fix(multi): resolve 26 bugs found in comprehensive codebase review
Build and Deploy / build-and-push (push) Successful in 3m40s

Core fixes (issues #12, #13, #22):
- #12: undo now preserves score delta instead of restoring snapshot score
- #13: take_from_foundation defaults to false (non-standard house rule)
- #22: check_win validates full suit sequence, not just card count

Engine fixes:
- #8:  replay keyboard input guard against non-replay state
- #9:  help modal scrims.is_empty() guard added
- #10: settings modal scrims.is_empty() guard added
- #11: sync_plugin builds payload at poll time (not task-spawn time)
- #14: server replay mode case-sensitivity fix ("Classic")
- #15: play_by_seed_plugin confirmed flag set to true on launch
- #16: replay back-step debounce via Local<bool> + StateChangedEvent;
       register StateChangedEvent in ReplayOverlayPlugin (fixes 52 tests)
- #17: time-attack timer ignores win-summary overlay
- #18: HUD dropdown glyphs U+25BE → U+2193 (FiraMono-safe arrow)
- #19: theme plugin applies immediate visual update on A→B→A switch
- #20: SyncAuthError / SyncBusyOverlay split into separate entities so
       auth errors are visible after busy overlay is hidden
- #21: handle_forfeit ordered before update_stats_on_new_game
- #23: server merge uses correct avg_time_seconds and games_lost math
- #24: win_summary migrated to ModalScrim pattern
- #25: card_animation apply_deferred between animation systems
- #26: cursor_plugin HashMap access uses .get() with fallback
- #27: auto_complete mid-sequence deactivation guard
- #28: feedback_anim SettleAnim ordered before FoundationFlourish
- #29: achievement_plugin iterates all win events; adds scrims guard
- #30: leaderboard modal scrims.is_empty() guard added
- #31: server auth tmp file cleanup on rename failure
- #32: sync_setup modal scrims.is_empty() guard added
- #33: font_plugin uses match fallback; TokioRuntimeResource graceful
       current-thread fallback on runtime init failure

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-19 13:14:47 -07:00
parent 6d061d23a1
commit 7840ef9eb2
24 changed files with 374 additions and 171 deletions
+23 -13
View File
@@ -193,7 +193,7 @@ impl GameState {
is_auto_completable: false, is_auto_completable: false,
undo_count: 0, undo_count: 0,
recycle_count: 0, recycle_count: 0,
take_from_foundation: true, take_from_foundation: false,
schema_version: GAME_STATE_SCHEMA_VERSION, schema_version: GAME_STATE_SCHEMA_VERSION,
undo_stack: VecDeque::new(), undo_stack: VecDeque::new(),
} }
@@ -407,7 +407,7 @@ impl GameState {
self.score = if self.mode == GameMode::Zen { self.score = if self.mode == GameMode::Zen {
0 0
} else { } else {
(snapshot.score + scoring_undo()).max(0) (self.score + scoring_undo()).max(0)
}; };
self.move_count = snapshot.move_count; self.move_count = snapshot.move_count;
self.is_won = false; self.is_won = false;
@@ -416,12 +416,25 @@ impl GameState {
Ok(()) Ok(())
} }
/// Returns `true` when all four foundation slots each contain 13 cards. /// Returns `true` when all four foundation slots each contain a valid A→K
/// sequence of a single suit.
///
/// Counting 13 cards is not sufficient — a corrupt save could produce 13
/// arbitrary cards per pile and permanently lock the game via `GameAlreadyWon`.
pub fn check_win(&self) -> bool { pub fn check_win(&self) -> bool {
(0..4_u8).all(|slot| { (0..4_u8).all(|slot| self.is_valid_foundation_pile(slot))
self.piles }
.get(&PileType::Foundation(slot))
.is_some_and(|p| p.cards.len() == 13) fn is_valid_foundation_pile(&self, slot: u8) -> bool {
let Some(pile) = self.piles.get(&PileType::Foundation(slot)) else {
return false;
};
if pile.cards.len() != 13 {
return false;
}
let suit = pile.cards[0].suit;
pile.cards.iter().enumerate().all(|(i, card)| {
card.suit == suit && card.rank.value() == (i as u8 + 1)
}) })
} }
@@ -1395,12 +1408,9 @@ mod tests {
} }
#[test] #[test]
fn take_from_foundation_allowed_by_default() { fn take_from_foundation_disabled_by_default() {
let mut g = setup_take_from_foundation_game(); let g = setup_take_from_foundation_game();
assert!(g.take_from_foundation, "standard Klondike allows take-from-foundation by default"); assert!(!g.take_from_foundation, "take_from_foundation is off by default (non-standard rule)");
g.move_cards(PileType::Foundation(0), PileType::Tableau(0), 1).unwrap();
assert_eq!(g.piles[&PileType::Foundation(0)].cards.len(), 1);
assert_eq!(g.piles[&PileType::Tableau(0)].cards.len(), 2);
} }
#[test] #[test]
+80 -81
View File
@@ -32,7 +32,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath};
use crate::stats_plugin::{StatsResource, StatsUpdate}; use crate::stats_plugin::{StatsResource, StatsUpdate};
use crate::ui_modal::{ use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible, ModalScrim, ScrimDismissible,
}; };
use crate::ui_theme::{ use crate::ui_theme::{
ACCENT_PRIMARY, BORDER_SUBTLE, STATE_SUCCESS, TEXT_DISABLED, TEXT_PRIMARY, TEXT_SECONDARY, ACCENT_PRIMARY, BORDER_SUBTLE, STATE_SUCCESS, TEXT_DISABLED, TEXT_PRIMARY, TEXT_SECONDARY,
@@ -162,93 +162,91 @@ fn evaluate_on_win(
mut achievements: ResMut<AchievementsResource>, mut achievements: ResMut<AchievementsResource>,
mut progress: ResMut<ProgressResource>, mut progress: ResMut<ProgressResource>,
) { ) {
let Some(ev) = wins.read().last() else { for ev in wins.read() {
return; let ctx = AchievementContext {
}; games_played: stats.0.games_played,
games_won: stats.0.games_won,
let ctx = AchievementContext { win_streak_current: stats.0.win_streak_current,
games_played: stats.0.games_played, best_single_score: stats.0.best_single_score,
games_won: stats.0.games_won, lifetime_score: stats.0.lifetime_score,
win_streak_current: stats.0.win_streak_current, draw_three_wins: stats.0.draw_three_wins,
best_single_score: stats.0.best_single_score, daily_challenge_streak: progress.0.daily_challenge_streak,
lifetime_score: stats.0.lifetime_score, last_win_score: ev.score,
draw_three_wins: stats.0.draw_three_wins, last_win_time_seconds: ev.time_seconds,
daily_challenge_streak: progress.0.daily_challenge_streak, last_win_used_undo: game.0.undo_count > 0,
last_win_score: ev.score, wall_clock_hour: Some(Local::now().hour()),
last_win_time_seconds: ev.time_seconds, last_win_recycle_count: game.0.recycle_count,
last_win_used_undo: game.0.undo_count > 0, last_win_is_zen: game.0.mode == solitaire_core::game_state::GameMode::Zen,
wall_clock_hour: Some(Local::now().hour()),
last_win_recycle_count: game.0.recycle_count,
last_win_is_zen: game.0.mode == solitaire_core::game_state::GameMode::Zen,
};
let hits = check_achievements(&ctx);
if hits.is_empty() {
return;
}
let now = Utc::now();
let mut achievements_changed = false;
let mut progress_changed = false;
for def in hits {
let Some(record) = achievements.0.iter_mut().find(|r| r.id == def.id) else {
continue;
}; };
if record.unlocked {
let hits = check_achievements(&ctx);
if hits.is_empty() {
continue; continue;
} }
record.unlock(now);
achievements_changed = true;
// Grant the reward on first unlock. let now = Utc::now();
if !record.reward_granted { let mut achievements_changed = false;
if let Some(reward) = def.reward { let mut progress_changed = false;
match reward {
Reward::CardBack(idx) => { for def in hits {
if !progress.0.unlocked_card_backs.contains(&idx) { let Some(record) = achievements.0.iter_mut().find(|r| r.id == def.id) else {
progress.0.unlocked_card_backs.push(idx); continue;
progress_changed = true; };
} if record.unlocked {
} continue;
Reward::Background(idx) => {
if !progress.0.unlocked_backgrounds.contains(&idx) {
progress.0.unlocked_backgrounds.push(idx);
progress_changed = true;
}
}
Reward::BonusXp(amount) => {
xp_awarded.write(XpAwardedEvent { amount });
let prev_level = progress.0.add_xp(amount);
if progress.0.leveled_up_from(prev_level) {
levelups.write(LevelUpEvent {
previous_level: prev_level,
new_level: progress.0.level,
total_xp: progress.0.total_xp,
});
}
progress_changed = true;
}
Reward::Badge => {}
}
} }
record.reward_granted = true; record.unlock(now);
achievements_changed = true;
// Grant the reward on first unlock.
if !record.reward_granted {
if let Some(reward) = def.reward {
match reward {
Reward::CardBack(idx) => {
if !progress.0.unlocked_card_backs.contains(&idx) {
progress.0.unlocked_card_backs.push(idx);
progress_changed = true;
}
}
Reward::Background(idx) => {
if !progress.0.unlocked_backgrounds.contains(&idx) {
progress.0.unlocked_backgrounds.push(idx);
progress_changed = true;
}
}
Reward::BonusXp(amount) => {
xp_awarded.write(XpAwardedEvent { amount });
let prev_level = progress.0.add_xp(amount);
if progress.0.leveled_up_from(prev_level) {
levelups.write(LevelUpEvent {
previous_level: prev_level,
new_level: progress.0.level,
total_xp: progress.0.total_xp,
});
}
progress_changed = true;
}
Reward::Badge => {}
}
}
record.reward_granted = true;
}
unlocks.write(AchievementUnlockedEvent(record.clone()));
} }
unlocks.write(AchievementUnlockedEvent(record.clone())); if achievements_changed
&& let Some(target) = &path.0
&& let Err(e) = save_achievements_to(target, &achievements.0) {
warn!("failed to save achievements: {e}");
}
if progress_changed
&& let Some(target) = &progress_path.0
&& let Err(e) = save_progress_to(target, &progress.0) {
warn!("failed to save progress after reward: {e}");
}
} }
if achievements_changed
&& let Some(target) = &path.0
&& let Err(e) = save_achievements_to(target, &achievements.0) {
warn!("failed to save achievements: {e}");
}
if progress_changed
&& let Some(target) = &progress_path.0
&& let Err(e) = save_progress_to(target, &progress.0) {
warn!("failed to save progress after reward: {e}");
}
} }
/// Cinephile unlock observer. /// Cinephile unlock observer.
@@ -391,6 +389,7 @@ fn toggle_achievements_screen(
achievements: Res<AchievementsResource>, achievements: Res<AchievementsResource>,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
screens: Query<Entity, With<AchievementsScreen>>, screens: Query<Entity, With<AchievementsScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<AchievementsScreen>)>,
) { ) {
let button_clicked = requests.read().count() > 0; let button_clicked = requests.read().count() > 0;
if !keys.just_pressed(KeyCode::KeyA) && !button_clicked { if !keys.just_pressed(KeyCode::KeyA) && !button_clicked {
@@ -398,7 +397,7 @@ fn toggle_achievements_screen(
} }
if let Ok(entity) = screens.single() { if let Ok(entity) = screens.single() {
commands.entity(entity).despawn(); commands.entity(entity).despawn();
} else { } else if other_modal_scrims.is_empty() {
spawn_achievements_screen(&mut commands, &achievements.0, font_res.as_deref()); spawn_achievements_screen(&mut commands, &achievements.0, font_res.as_deref());
} }
} }
+7 -2
View File
@@ -72,9 +72,14 @@ fn detect_auto_complete(
if game.0.is_auto_completable && !state.active { if game.0.is_auto_completable && !state.active {
state.active = true; state.active = true;
state.cooldown = 0.0; // fire first move immediately state.cooldown = 0.0; // fire first move immediately
} else if !game.0.is_auto_completable {
state.active = false;
} }
// Intentionally no `else if !is_auto_completable` branch here.
// Deactivating on every frame where `is_auto_completable` is false
// would hard-stop the sequence mid-flight whenever `next_auto_complete_move`
// transiently returns `None` (e.g. while the previous move is still
// in-flight). The `is_won` check above already handles the definitive
// end-of-game case; `drive_auto_complete` simply retries next tick
// when no move is available yet.
} }
/// Plays a distinct chime the moment auto-complete first activates. /// Plays a distinct chime the moment auto-complete first activates.
@@ -142,6 +142,13 @@ impl Plugin for CardAnimationPlugin {
update_frame_time_diagnostics, update_frame_time_diagnostics,
// Advance active animations. // Advance active animations.
advance_card_animations, advance_card_animations,
// Flush deferred commands so `CardAnimation` removals from
// `advance_card_animations` are visible before the chain
// system runs. Without this, the chain sees the component
// still present in the same frame it was removed (deferred
// commands aren't applied until the next ApplyDeferred
// point), causing a 1-frame gap between every chain step.
ApplyDeferred,
// After each animation finishes, pop the next chain segment. // After each animation finishes, pop the next chain segment.
advance_animation_chains, advance_animation_chains,
// Interaction visuals (run after animation for final positions). // Interaction visuals (run after animation for final positions).
+9 -6
View File
@@ -382,8 +382,8 @@ fn update_drop_target_overlays(
/// for everything else it is card-sized. Replicated here rather than /// for everything else it is card-sized. Replicated here rather than
/// imported because `pile_drop_rect` is private to `input_plugin` and /// imported because `pile_drop_rect` is private to `input_plugin` and
/// this overlay is the only other consumer. /// this overlay is the only other consumer.
fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> (Vec2, Vec2) { fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> Option<(Vec2, Vec2)> {
let centre = layout.pile_positions[pile]; let centre = layout.pile_positions.get(pile).copied()?;
if matches!(pile, PileType::Tableau(_)) { if matches!(pile, PileType::Tableau(_)) {
let card_count = game.piles.get(pile).map_or(0, |p| p.cards.len()); let card_count = game.piles.get(pile).map_or(0, |p| p.cards.len());
if card_count > 1 { if card_count > 1 {
@@ -393,13 +393,13 @@ fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> (Vec
let bottom_edge = bottom_card_centre_y - layout.card_size.y / 2.0; let bottom_edge = bottom_card_centre_y - layout.card_size.y / 2.0;
let span_height = top_edge - bottom_edge; let span_height = top_edge - bottom_edge;
let new_centre_y = (top_edge + bottom_edge) / 2.0; let new_centre_y = (top_edge + bottom_edge) / 2.0;
return ( return Some((
Vec2::new(centre.x, new_centre_y), Vec2::new(centre.x, new_centre_y),
Vec2::new(layout.card_size.x, span_height), Vec2::new(layout.card_size.x, span_height),
); ));
} }
} }
(centre, layout.card_size) Some((centre, layout.card_size))
} }
/// Spawns one overlay parent (fill) plus four edge sprites (outline) at /// Spawns one overlay parent (fill) plus four edge sprites (outline) at
@@ -410,7 +410,10 @@ fn spawn_drop_target_overlay(
layout: &Layout, layout: &Layout,
game: &GameState, game: &GameState,
) { ) {
let (centre, size) = drop_overlay_rect(pile, layout, game); let Some((centre, size)) = drop_overlay_rect(pile, layout, game) else {
warn!("drop_overlay_rect: pile {pile:?} not in layout, skipping overlay");
return;
};
let edge = DROP_TARGET_OUTLINE_PX; let edge = DROP_TARGET_OUTLINE_PX;
commands commands
+6 -1
View File
@@ -210,10 +210,15 @@ impl Plugin for FeedbackAnimPlugin {
start_shake_anim.after(GameMutation), start_shake_anim.after(GameMutation),
tick_shake_anim, tick_shake_anim,
start_settle_anim.after(GameMutation), start_settle_anim.after(GameMutation),
// tick_foundation_flourish writes the full Transform.scale
// (Vec3); tick_settle_anim writes only scale.y on top of
// it. Ordering ensures the settle's y-only write always
// applies last so it wins on the ~0.15 s overlap when both
// components are present on the same King entity.
tick_foundation_flourish.before(tick_settle_anim),
tick_settle_anim, tick_settle_anim,
start_deal_anim.after(GameMutation), start_deal_anim.after(GameMutation),
start_foundation_flourish.after(GameMutation), start_foundation_flourish.after(GameMutation),
tick_foundation_flourish,
), ),
); );
} }
+9 -2
View File
@@ -31,8 +31,15 @@ fn load_font(fonts: Option<ResMut<Assets<Font>>>, mut commands: Commands) {
// Assets<Font>). FontPlugin in that context is a no-op — consumers // Assets<Font>). FontPlugin in that context is a no-op — consumers
// already query `Option<Res<FontResource>>` and degrade cleanly. // already query `Option<Res<FontResource>>` and degrade cleanly.
let Some(mut fonts) = fonts else { return }; let Some(mut fonts) = fonts else { return };
let font = Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec()) let font = match Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec()) {
.expect("bundled FiraMono failed to parse — binary is corrupt"); Ok(f) => f,
Err(e) => {
// A corrupt embedded font is unusual but should not crash the
// process — UI will render without glyphs rather than panicking.
warn!("bundled FiraMono failed to parse ({e}); UI text may be invisible");
return;
}
};
let handle = fonts.add(font); let handle = fonts.add(font);
commands.insert_resource(FontResource(handle)); commands.insert_resource(FontResource(handle));
} }
+3 -2
View File
@@ -13,7 +13,7 @@ use crate::font_plugin::FontResource;
use crate::hud_plugin::ANDROID_HINT_LABEL; use crate::hud_plugin::ANDROID_HINT_LABEL;
use crate::ui_modal::{ use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible, ModalScrim, ScrimDismissible,
}; };
use crate::ui_theme::{SPACE_2, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY, VAL_SPACE_2, VAL_SPACE_3, Z_MODAL_PANEL}; use crate::ui_theme::{SPACE_2, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY, VAL_SPACE_2, VAL_SPACE_3, Z_MODAL_PANEL};
#[cfg(not(target_os = "android"))] #[cfg(not(target_os = "android"))]
@@ -67,6 +67,7 @@ fn toggle_help_screen(
keys: Res<ButtonInput<KeyCode>>, keys: Res<ButtonInput<KeyCode>>,
mut requests: MessageReader<HelpRequestEvent>, mut requests: MessageReader<HelpRequestEvent>,
screens: Query<Entity, With<HelpScreen>>, screens: Query<Entity, With<HelpScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<HelpScreen>)>,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
) { ) {
// Either F1 or a click on the HUD "Help" button (which fires // Either F1 or a click on the HUD "Help" button (which fires
@@ -77,7 +78,7 @@ fn toggle_help_screen(
} }
if let Ok(entity) = screens.single() { if let Ok(entity) = screens.single() {
commands.entity(entity).despawn(); commands.entity(entity).despawn();
} else { } else if other_modal_scrims.is_empty() {
spawn_help_screen(&mut commands, font_res.as_deref()); spawn_help_screen(&mut commands, font_res.as_deref());
} }
} }
+2 -2
View File
@@ -870,12 +870,12 @@ fn spawn_action_buttons(
); );
#[cfg(not(target_os = "android"))] #[cfg(not(target_os = "android"))]
let labels = ( let labels = (
"Menu \u{25BE}", "Menu \u{2193}",
"Undo", "Undo",
"Pause", "Pause",
"Help", "Help",
"Hint", "Hint",
"Modes \u{25BE}", "Modes \u{2193}",
"New Game", "New Game",
); );
+10
View File
@@ -47,6 +47,7 @@ use crate::game_plugin::{ConfirmNewGameScreen, GameMutation, RestorePromptScreen
use crate::pause_plugin::PausedResource; use crate::pause_plugin::PausedResource;
use crate::progress_plugin::ProgressResource; use crate::progress_plugin::ProgressResource;
use crate::layout::{Layout, LayoutResource}; use crate::layout::{Layout, LayoutResource};
use crate::replay_playback::ReplayPlaybackState;
use crate::resources::{DragState, GameInputConsumedResource, GameStateResource, HintCycleIndex}; use crate::resources::{DragState, GameInputConsumedResource, GameStateResource, HintCycleIndex};
use crate::selection_plugin::SelectionState; use crate::selection_plugin::SelectionState;
use crate::time_attack_plugin::TimeAttackResource; use crate::time_attack_plugin::TimeAttackResource;
@@ -175,11 +176,20 @@ fn handle_keyboard_core(
mut zen_requests: MessageReader<StartZenRequestEvent>, mut zen_requests: MessageReader<StartZenRequestEvent>,
confirm_screens: Query<(), With<ConfirmNewGameScreen>>, confirm_screens: Query<(), With<ConfirmNewGameScreen>>,
restore_prompts: Query<(), With<RestorePromptScreen>>, restore_prompts: Query<(), With<RestorePromptScreen>>,
replay_state: Option<Res<ReplayPlaybackState>>,
) { ) {
if paused.is_some_and(|p| p.0) { if paused.is_some_and(|p| p.0) {
return; return;
} }
// During replay playback (Playing or Completed) all game-input shortcuts
// are suppressed. The replay overlay owns Space (pause/resume) and the
// arrow keys (step). Letting game input through would mutate
// `GameStateResource` and corrupt replay determinism.
if replay_state.is_some_and(|r| !matches!(*r, ReplayPlaybackState::Inactive)) {
return;
}
if keys.just_pressed(KeyCode::KeyU) { if keys.just_pressed(KeyCode::KeyU) {
ev.undo.write(UndoRequestEvent); ev.undo.write(UndoRequestEvent);
} }
+5 -1
View File
@@ -21,7 +21,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath};
use crate::sync_plugin::SyncProviderResource; use crate::sync_plugin::SyncProviderResource;
use crate::ui_modal::{ use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible, ModalScrim, ScrimDismissible,
}; };
use crate::ui_theme::{ use crate::ui_theme::{
ACCENT_PRIMARY, BG_ELEVATED, BORDER_SUBTLE, RADIUS_SM, STATE_INFO, ACCENT_PRIMARY, BG_ELEVATED, BORDER_SUBTLE, RADIUS_SM, STATE_INFO,
@@ -714,6 +714,7 @@ fn data_cell(
fn handle_set_display_name_button( fn handle_set_display_name_button(
button_q: Query<&Interaction, (Changed<Interaction>, With<SetDisplayNameButton>)>, button_q: Query<&Interaction, (Changed<Interaction>, With<SetDisplayNameButton>)>,
existing: Query<(), With<DisplayNameModal>>, existing: Query<(), With<DisplayNameModal>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<DisplayNameModal>)>,
mut commands: Commands, mut commands: Commands,
settings: Option<Res<SettingsResource>>, settings: Option<Res<SettingsResource>>,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
@@ -725,6 +726,9 @@ fn handle_set_display_name_button(
if !existing.is_empty() { if !existing.is_empty() {
return; // already open return; // already open
} }
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
buf.0 = settings buf.0 = settings
.as_ref() .as_ref()
.and_then(|s| s.0.leaderboard_display_name.clone()) .and_then(|s| s.0.leaderboard_display_name.clone())
+8 -2
View File
@@ -411,7 +411,11 @@ fn handle_confirm(
new_game.write(NewGameRequestEvent { new_game.write(NewGameRequestEvent {
seed: Some(seed), seed: Some(seed),
mode: None, mode: None,
confirmed: false, // The player explicitly clicked Play (or pressed Enter) after typing
// a seed — treat this as an affirmative confirmation so the
// abandon-current-game dialog is not shown on top of the already-
// dismissed seed dialog.
confirmed: true,
}); });
for entity in &screen { for entity in &screen {
@@ -566,7 +570,9 @@ mod tests {
assert_eq!(fired.len(), 1); assert_eq!(fired.len(), 1);
assert_eq!(fired[0].seed, Some(42)); assert_eq!(fired[0].seed, Some(42));
assert_eq!(fired[0].mode, None); assert_eq!(fired[0].mode, None);
assert!(!fired[0].confirmed); // confirmed: true — the player explicitly clicked Play, so no
// abandon-current-game dialog should appear.
assert!(fired[0].confirmed);
// Dialog should be gone. // Dialog should be gone.
assert!(!dialog_present(&mut app)); assert!(!dialog_present(&mut app));
+32 -4
View File
@@ -28,7 +28,7 @@ use chrono::Datelike;
use crate::font_plugin::FontResource; use crate::font_plugin::FontResource;
use crate::layout::LayoutResource; use crate::layout::LayoutResource;
use crate::events::{DrawRequestEvent, MoveRequestEvent, UndoRequestEvent}; use crate::events::{DrawRequestEvent, MoveRequestEvent, StateChangedEvent, UndoRequestEvent};
use crate::replay_playback::{ use crate::replay_playback::{
step_backwards_replay_playback, step_replay_playback, stop_replay_playback, step_backwards_replay_playback, step_replay_playback, stop_replay_playback,
toggle_pause_replay_playback, ReplayPlaybackState, toggle_pause_replay_playback, ReplayPlaybackState,
@@ -476,6 +476,7 @@ impl Plugin for ReplayOverlayPlugin {
.add_message::<MoveRequestEvent>() .add_message::<MoveRequestEvent>()
.add_message::<DrawRequestEvent>() .add_message::<DrawRequestEvent>()
.add_message::<UndoRequestEvent>() .add_message::<UndoRequestEvent>()
.add_message::<StateChangedEvent>()
.add_systems( .add_systems(
Update, Update,
( (
@@ -1884,6 +1885,7 @@ fn handle_pause_keyboard(
/// resets to 0 on key release so the next fresh press fires /// resets to 0 on key release so the next fresh press fires
/// immediately. This matches the mockup's `[← →] scrub` /// immediately. This matches the mockup's `[← →] scrub`
/// terminology while keeping single-press = single-step semantics. /// terminology while keeping single-press = single-step semantics.
#[allow(clippy::too_many_arguments)]
fn handle_arrow_keyboard( fn handle_arrow_keyboard(
keys: Option<Res<ButtonInput<KeyCode>>>, keys: Option<Res<ButtonInput<KeyCode>>>,
time: Res<Time>, time: Res<Time>,
@@ -1892,10 +1894,22 @@ fn handle_arrow_keyboard(
mut moves_writer: MessageWriter<MoveRequestEvent>, mut moves_writer: MessageWriter<MoveRequestEvent>,
mut draws_writer: MessageWriter<DrawRequestEvent>, mut draws_writer: MessageWriter<DrawRequestEvent>,
mut undo_writer: MessageWriter<UndoRequestEvent>, mut undo_writer: MessageWriter<UndoRequestEvent>,
mut state_changed: MessageReader<StateChangedEvent>,
// `true` while a backward step is in-flight: cursor was decremented and
// `UndoRequestEvent` was written, but `handle_undo` hasn't applied it yet.
// Cleared when `StateChangedEvent` confirms the game state has caught up.
// Prevents rapid ← presses from accumulating multiple cursor decrements
// before any undo is applied (Bug #16).
mut back_pending: Local<bool>,
) { ) {
let Some(keys) = keys else { return }; let Some(keys) = keys else { return };
let dt = time.delta_secs(); let dt = time.delta_secs();
// Clear the in-flight flag once the game confirms the undo landed.
if state_changed.read().count() > 0 {
*back_pending = false;
}
// Right (forward step) — initial press fires immediately; // Right (forward step) — initial press fires immediately;
// held repeats fire when the accumulator crosses the interval. // held repeats fire when the accumulator crosses the interval.
if keys.just_pressed(KeyCode::ArrowRight) { if keys.just_pressed(KeyCode::ArrowRight) {
@@ -1911,14 +1925,28 @@ fn handle_arrow_keyboard(
hold.right_held_secs = 0.0; hold.right_held_secs = 0.0;
} }
// Left (backwards step) — symmetric to the right path. // Left (backwards step) — gate on `back_pending` so at most one undo
// is in-flight at a time. The cursor is only decremented inside
// `step_backwards_replay_playback`, which also writes `UndoRequestEvent`.
// `back_pending` is set after a successful step and cleared above when
// `StateChangedEvent` confirms the undo was applied.
if keys.just_pressed(KeyCode::ArrowLeft) { if keys.just_pressed(KeyCode::ArrowLeft) {
step_backwards_replay_playback(&mut state, &mut undo_writer); if !*back_pending {
let fired = step_backwards_replay_playback(&mut state, &mut undo_writer);
if fired {
*back_pending = true;
}
}
hold.left_held_secs = 0.0; hold.left_held_secs = 0.0;
} else if keys.pressed(KeyCode::ArrowLeft) { } else if keys.pressed(KeyCode::ArrowLeft) {
hold.left_held_secs += dt; hold.left_held_secs += dt;
if hold.left_held_secs >= SCRUB_REPEAT_INTERVAL_SECS { if hold.left_held_secs >= SCRUB_REPEAT_INTERVAL_SECS {
step_backwards_replay_playback(&mut state, &mut undo_writer); if !*back_pending {
let fired = step_backwards_replay_playback(&mut state, &mut undo_writer);
if fired {
*back_pending = true;
}
}
hold.left_held_secs = 0.0; hold.left_held_secs = 0.0;
} }
} else { } else {
+41 -8
View File
@@ -3,7 +3,7 @@
use std::sync::Arc; use std::sync::Arc;
use bevy::math::Vec2; use bevy::math::Vec2;
use bevy::prelude::Resource; use bevy::prelude::{warn, Resource};
use chrono::{DateTime, Utc}; use chrono::{DateTime, Utc};
use solitaire_core::game_state::GameState; use solitaire_core::game_state::GameState;
use solitaire_core::pile::PileType; use solitaire_core::pile::PileType;
@@ -131,15 +131,48 @@ pub struct GameInputConsumedResource(pub bool);
#[derive(Resource, Clone)] #[derive(Resource, Clone)]
pub struct TokioRuntimeResource(pub Arc<tokio::runtime::Runtime>); pub struct TokioRuntimeResource(pub Arc<tokio::runtime::Runtime>);
impl Default for TokioRuntimeResource { impl TokioRuntimeResource {
fn default() -> Self { /// Attempts to build the shared multi-threaded Tokio runtime.
// Building the Tokio runtime is startup-time initialization; failure ///
// here means the OS refused to create threads, which is unrecoverable. /// Returns `Err` if the OS refuses to create worker threads (e.g. resource
/// limits on Android). Callers should log the error and disable sync
/// features rather than panicking.
pub fn new() -> Result<Self, tokio::io::Error> {
let rt = tokio::runtime::Builder::new_multi_thread() let rt = tokio::runtime::Builder::new_multi_thread()
.worker_threads(2) .worker_threads(2)
.enable_all() .enable_all()
.build() .build()?;
.expect("failed to build shared Tokio runtime"); Ok(Self(Arc::new(rt)))
Self(Arc::new(rt)) }
}
impl Default for TokioRuntimeResource {
fn default() -> Self {
// Try multi-threaded first; fall back to current-thread (single
// worker) if the OS refuses to create additional threads. Neither
// path uses `.expect()` so this never panics at startup.
match tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
{
Ok(rt) => Self(Arc::new(rt)),
Err(e) => {
warn!(
"sync: failed to build multi-thread Tokio runtime ({e}); \
falling back to current-thread runtime"
);
// current_thread runtime never spawns OS threads, so it
// succeeds even under tight sandboxing.
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect(
"current-thread Tokio runtime failed — \
the process cannot do any async I/O",
);
Self(Arc::new(rt))
}
}
} }
} }
+2 -1
View File
@@ -510,6 +510,7 @@ fn toggle_settings_screen(
fn sync_settings_panel_visibility( fn sync_settings_panel_visibility(
screen: Res<SettingsScreen>, screen: Res<SettingsScreen>,
panels: Query<Entity, With<SettingsPanel>>, panels: Query<Entity, With<SettingsPanel>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<SettingsPanel>)>,
scroll_nodes: Query<&ScrollPosition, With<SettingsScrollNode>>, scroll_nodes: Query<&ScrollPosition, With<SettingsScrollNode>>,
mut scroll_pos: ResMut<SettingsScrollPos>, mut scroll_pos: ResMut<SettingsScrollPos>,
mut commands: Commands, mut commands: Commands,
@@ -525,7 +526,7 @@ fn sync_settings_panel_visibility(
return; return;
} }
if screen.0 { if screen.0 {
if panels.is_empty() { if panels.is_empty() && other_modal_scrims.is_empty() {
let status_label = sync_status let status_label = sync_status
.map_or_else(|| "Status: local only".to_string(), |s| sync_status_label(&s.0)); .map_or_else(|| "Status: local only".to_string(), |s| sync_status_label(&s.0));
let unlocked_backs = progress let unlocked_backs = progress
+7 -1
View File
@@ -220,7 +220,13 @@ impl Plugin for StatsPlugin {
) )
.add_systems( .add_systems(
Update, Update,
handle_forfeit.before(GameMutation), // handle_forfeit must run before update_stats_on_new_game so
// the NewGameRequestEvent it emits is not visible to
// update_stats_on_new_game in the same frame — otherwise
// record_abandoned() fires twice on every forfeit (#21).
handle_forfeit
.before(GameMutation)
.before(update_stats_on_new_game),
) )
.add_systems(Update, toggle_stats_screen.after(GameMutation)) .add_systems(Update, toggle_stats_screen.after(GameMutation))
.add_systems(Update, handle_stats_close_button) .add_systems(Update, handle_stats_close_button)
+24 -13
View File
@@ -101,7 +101,6 @@ impl SyncPlugin {
impl Plugin for SyncPlugin { impl Plugin for SyncPlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
app.insert_resource(SyncProviderResource(self.provider.clone())) app.insert_resource(SyncProviderResource(self.provider.clone()))
.init_resource::<TokioRuntimeResource>()
.init_resource::<SyncStatusResource>() .init_resource::<SyncStatusResource>()
.init_resource::<PullTaskResult>() .init_resource::<PullTaskResult>()
.init_resource::<PullTask>() .init_resource::<PullTask>()
@@ -109,18 +108,30 @@ impl Plugin for SyncPlugin {
.add_message::<ManualSyncRequestEvent>() .add_message::<ManualSyncRequestEvent>()
.add_message::<SyncCompleteEvent>() .add_message::<SyncCompleteEvent>()
.add_message::<SyncConfigureRequestEvent>() .add_message::<SyncConfigureRequestEvent>()
.add_message::<WarningToastEvent>() .add_message::<WarningToastEvent>();
.add_systems(Startup, start_pull)
.add_systems( // Build the shared Tokio runtime; disable all network sync if the OS
Update, // refuses to create threads (resource-limited environments, sandboxed
( // Android builds, etc.).
poll_pull_result, match TokioRuntimeResource::new() {
handle_manual_sync_request, Ok(rt) => {
push_replay_on_win, app.insert_resource(rt)
poll_replay_upload_result, .add_systems(Startup, start_pull)
), .add_systems(
) Update,
.add_systems(Last, push_on_exit); (
poll_pull_result,
handle_manual_sync_request,
push_replay_on_win,
poll_replay_upload_result,
),
)
.add_systems(Last, push_on_exit);
}
Err(e) => {
warn!("sync: failed to create Tokio runtime — network sync disabled: {e}");
}
}
} }
} }
+27 -7
View File
@@ -55,7 +55,7 @@ use crate::font_plugin::FontResource;
use crate::settings_plugin::{SettingsResource, SettingsScreen, SettingsStoragePath}; use crate::settings_plugin::{SettingsResource, SettingsScreen, SettingsStoragePath};
use crate::resources::TokioRuntimeResource; use crate::resources::TokioRuntimeResource;
use crate::sync_plugin::SyncProviderResource; use crate::sync_plugin::SyncProviderResource;
use crate::ui_modal::spawn_modal; use crate::ui_modal::{spawn_modal, ModalScrim};
use crate::ui_theme::{ use crate::ui_theme::{
ACCENT_PRIMARY, BG_ELEVATED, BG_ELEVATED_HI, ACCENT_PRIMARY, BG_ELEVATED, BG_ELEVATED_HI,
BORDER_SUBTLE, HighContrastBorder, RADIUS_SM, STATE_DANGER, TEXT_DISABLED, BORDER_SUBTLE, HighContrastBorder, RADIUS_SM, STATE_DANGER, TEXT_DISABLED,
@@ -208,6 +208,7 @@ impl Plugin for SyncSetupPlugin {
fn open_sync_setup_modal( fn open_sync_setup_modal(
mut events: MessageReader<SyncConfigureRequestEvent>, mut events: MessageReader<SyncConfigureRequestEvent>,
existing: Query<(), With<SyncSetupScreen>>, existing: Query<(), With<SyncSetupScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<SyncSetupScreen>)>,
mut commands: Commands, mut commands: Commands,
mut focused: ResMut<SyncFocusedField>, mut focused: ResMut<SyncFocusedField>,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
@@ -219,6 +220,9 @@ fn open_sync_setup_modal(
if !existing.is_empty() { if !existing.is_empty() {
return; // Already open. return; // Already open.
} }
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
*focused = SyncFocusedField::Url; *focused = SyncFocusedField::Url;
spawn_sync_setup_modal(&mut commands, font_res.as_deref()); spawn_sync_setup_modal(&mut commands, font_res.as_deref());
} }
@@ -354,9 +358,10 @@ fn handle_auth_button(
return; return;
} }
// Clear error and show busy indicator. // Clear previous error and show busy indicator.
for (mut text, _) in &mut error_nodes { for (mut text, mut color) in &mut error_nodes {
text.0 = "Connecting…".to_string(); text.0 = String::new();
color.0 = TEXT_SECONDARY;
} }
for mut vis in &mut busy_nodes { for mut vis in &mut busy_nodes {
*vis = Visibility::Visible; *vis = Visibility::Visible;
@@ -540,6 +545,7 @@ fn handle_logout(
fn open_delete_confirm_modal( fn open_delete_confirm_modal(
mut events: MessageReader<DeleteAccountRequestEvent>, mut events: MessageReader<DeleteAccountRequestEvent>,
existing: Query<(), With<DeleteConfirmScreen>>, existing: Query<(), With<DeleteConfirmScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<DeleteConfirmScreen>)>,
mut commands: Commands, mut commands: Commands,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
) { ) {
@@ -550,6 +556,9 @@ fn open_delete_confirm_modal(
if !existing.is_empty() { if !existing.is_empty() {
return; return;
} }
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
spawn_delete_confirm_modal(&mut commands, font_res.as_deref()); spawn_delete_confirm_modal(&mut commands, font_res.as_deref());
} }
@@ -675,20 +684,31 @@ fn spawn_sync_setup_modal(commands: &mut Commands, font_res: Option<&FontResourc
font_res, font_res,
); );
// Error / status line. // Error / status line — two distinct children so visibility and
// text can be controlled independently.
body.spawn(Node { body.spawn(Node {
min_height: Val::Px(18.0), min_height: Val::Px(18.0),
flex_direction: FlexDirection::Row,
align_items: AlignItems::Center,
column_gap: VAL_SPACE_2,
..default() ..default()
}) })
.with_children(|row| { .with_children(|row| {
// Busy indicator: shown while the auth task is in flight.
row.spawn(( row.spawn((
SyncAuthError,
SyncBusyOverlay, SyncBusyOverlay,
Text::new(String::new()), Text::new(""),
make_font(font_res, TYPE_CAPTION), make_font(font_res, TYPE_CAPTION),
TextColor(TEXT_SECONDARY), TextColor(TEXT_SECONDARY),
Visibility::Hidden, Visibility::Hidden,
)); ));
// Error / status text: always laid out, empty when idle.
row.spawn((
SyncAuthError,
Text::new(String::new()),
make_font(font_res, TYPE_CAPTION),
TextColor(TEXT_SECONDARY),
));
}); });
// Tab hint — desktop only; no Tab key on Android. // Tab hint — desktop only; no Tab key on Android.
+20
View File
@@ -182,12 +182,16 @@ fn sync_card_image_set_with_active_theme(
mut events: MessageReader<AssetEvent<CardTheme>>, mut events: MessageReader<AssetEvent<CardTheme>>,
active: Option<Res<ActiveTheme>>, active: Option<Res<ActiveTheme>>,
themes: Res<Assets<CardTheme>>, themes: Res<Assets<CardTheme>>,
asset_server: Option<Res<AssetServer>>,
mut card_image_set: Option<ResMut<CardImageSet>>, mut card_image_set: Option<ResMut<CardImageSet>>,
mut state_events: MessageWriter<StateChangedEvent>, mut state_events: MessageWriter<StateChangedEvent>,
) { ) {
let Some(active) = active else { return }; let Some(active) = active else { return };
let active_id = active.0.id(); let active_id = active.0.id();
let mut should_sync = false; let mut should_sync = false;
// Consume asset events — covers the normal first-load path.
for ev in events.read() { for ev in events.read() {
let id = match ev { let id = match ev {
AssetEvent::LoadedWithDependencies { id } AssetEvent::LoadedWithDependencies { id }
@@ -198,6 +202,22 @@ fn sync_card_image_set_with_active_theme(
should_sync = true; should_sync = true;
} }
} }
// A→B→A switch: Bevy does not re-fire LoadedWithDependencies for a
// handle whose asset is already cached. Detect this by checking that
// `ActiveTheme` itself changed this frame (the resource was just
// replaced by `react_to_settings_theme_change`) and the underlying
// asset is already fully loaded. If so, sync immediately rather than
// waiting for an event that will never arrive.
if !should_sync
&& active.is_changed()
&& asset_server
.as_ref()
.is_some_and(|as_| as_.is_loaded_with_dependencies(active.0.id()))
{
should_sync = true;
}
if !should_sync { if !should_sync {
return; return;
} }
+5 -4
View File
@@ -172,14 +172,15 @@ fn advance_time_attack(
paused: Option<Res<crate::pause_plugin::PausedResource>>, paused: Option<Res<crate::pause_plugin::PausedResource>>,
path: Option<Res<TimeAttackSessionPath>>, path: Option<Res<TimeAttackSessionPath>>,
home_screens: Query<(), With<crate::home_plugin::HomeScreen>>, home_screens: Query<(), With<crate::home_plugin::HomeScreen>>,
win_overlays: Query<(), With<crate::win_summary_plugin::WinSummaryOverlay>>,
) { ) {
if !session.active { if !session.active {
return; return;
} }
// Mirrors `tick_elapsed_time`: pause while the launch / mode-picker // Pause the countdown while Home, the Pause overlay, or the Win Summary
// Home modal is up so the countdown doesn't burn while the player // overlay is visible — the player should not lose time while reading results
// is choosing what to play next. // or navigating menus.
if paused.is_some_and(|p| p.0) || !home_screens.is_empty() { if paused.is_some_and(|p| p.0) || !home_screens.is_empty() || !win_overlays.is_empty() {
return; return;
} }
session.remaining_secs -= time.delta_secs(); session.remaining_secs -= time.delta_secs();
@@ -24,6 +24,7 @@ use crate::progress_plugin::ProgressResource;
use crate::resources::GameStateResource; use crate::resources::GameStateResource;
use crate::settings_plugin::SettingsResource; use crate::settings_plugin::SettingsResource;
use crate::stats_plugin::{StatsResource, StatsUpdate}; use crate::stats_plugin::{StatsResource, StatsUpdate};
use crate::ui_modal::ModalScrim;
use crate::ui_theme::{ use crate::ui_theme::{
scaled_duration, ACCENT_PRIMARY, BG_BASE, BG_ELEVATED, MOTION_SCORE_BREAKDOWN_FADE_SECS, scaled_duration, ACCENT_PRIMARY, BG_BASE, BG_ELEVATED, MOTION_SCORE_BREAKDOWN_FADE_SECS,
MOTION_SCORE_BREAKDOWN_STAGGER_SECS, MOTION_WIN_SHAKE_AMPLITUDE, MOTION_WIN_SHAKE_SECS, MOTION_SCORE_BREAKDOWN_STAGGER_SECS, MOTION_WIN_SHAKE_AMPLITUDE, MOTION_WIN_SHAKE_SECS,
@@ -757,6 +758,7 @@ fn spawn_overlay(
commands commands
.spawn(( .spawn((
WinSummaryOverlay, WinSummaryOverlay,
ModalScrim,
Node { Node {
position_type: PositionType::Absolute, position_type: PositionType::Absolute,
left: Val::Percent(0.0), left: Val::Percent(0.0),
@@ -769,6 +771,7 @@ fn spawn_overlay(
..default() ..default()
}, },
BackgroundColor(SCRIM), BackgroundColor(SCRIM),
GlobalZIndex(Z_WIN_CASCADE),
ZIndex(Z_WIN_CASCADE), ZIndex(Z_WIN_CASCADE),
)) ))
.with_children(|root| { .with_children(|root| {
+4 -1
View File
@@ -390,7 +390,10 @@ pub async fn upload_avatar(
// Write to a temp file then atomically rename so concurrent readers never // Write to a temp file then atomically rename so concurrent readers never
// see a partially-written avatar. // see a partially-written avatar.
std::fs::write(&tmp_path, &body).map_err(|e| AppError::Internal(e.to_string()))?; std::fs::write(&tmp_path, &body).map_err(|e| AppError::Internal(e.to_string()))?;
std::fs::rename(&tmp_path, &path).map_err(|e| AppError::Internal(e.to_string()))?; if let Err(e) = std::fs::rename(&tmp_path, &path) {
let _ = std::fs::remove_file(&tmp_path);
return Err(AppError::Internal(e.to_string()));
}
// Remove stale files with other extensions after the atomic rename. // Remove stale files with other extensions after the atomic rename.
for old_ext in &["jpg", "png", "webp", "gif"] { for old_ext in &["jpg", "png", "webp", "gif"] {
if *old_ext != ext { if *old_ext != ext {
+6 -3
View File
@@ -35,7 +35,7 @@ use crate::{error::AppError, middleware::AuthenticatedUser, AppState};
/// the desktop client's transitive dependencies. /// the desktop client's transitive dependencies.
#[derive(Debug, Deserialize)] #[derive(Debug, Deserialize)]
struct ReplayHeader { struct ReplayHeader {
seed: i64, seed: u64,
draw_mode: String, draw_mode: String,
mode: String, mode: String,
time_seconds: i64, time_seconds: i64,
@@ -94,6 +94,9 @@ pub async fn upload(
let id = Uuid::new_v4().to_string(); let id = Uuid::new_v4().to_string();
let received_at = Utc::now().to_rfc3339(); let received_at = Utc::now().to_rfc3339();
let replay_json = serde_json::to_string(&payload)?; let replay_json = serde_json::to_string(&payload)?;
// SQLite INTEGER columns bind as i64. Reinterpret the u64 bits — the
// database stores the same 8 bytes; high-bit seeds round-trip correctly.
let seed_i64 = header.seed as i64;
sqlx::query!( sqlx::query!(
r#"INSERT INTO replays ( r#"INSERT INTO replays (
@@ -102,7 +105,7 @@ pub async fn upload(
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"#, ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"#,
id, id,
user.user_id, user.user_id,
header.seed, seed_i64,
header.draw_mode, header.draw_mode,
header.mode, header.mode,
header.time_seconds, header.time_seconds,
@@ -116,7 +119,7 @@ pub async fn upload(
// Update leaderboard best score/time for opted-in users when this replay // Update leaderboard best score/time for opted-in users when this replay
// beats their existing best. Only classic mode counts for the leaderboard. // beats their existing best. Only classic mode counts for the leaderboard.
if header.mode == "classic" { if header.mode == "Classic" {
sqlx::query!( sqlx::query!(
r#"UPDATE leaderboard r#"UPDATE leaderboard
SET best_score = ?, SET best_score = ?,
+34 -17
View File
@@ -108,23 +108,27 @@ fn merge_stats(
let merged_games_won = local.games_won.max(remote.games_won); let merged_games_won = local.games_won.max(remote.games_won);
let merged_games_played = local.games_played.max(remote.games_played); let merged_games_played = local.games_played.max(remote.games_played);
// Recompute average time from the merged totals. If no wins yet, keep 0. // Carry the average time from whichever side contributed merged_games_won.
// Taking max(total_time)/max(wins) misattributes time when the side with
// more wins has a lower total — use the winning side's average directly.
let avg_time_seconds = if merged_games_won == 0 { let avg_time_seconds = if merged_games_won == 0 {
0 0
} else if local.games_won >= remote.games_won {
local.avg_time_seconds
} else { } else {
// Use whichever side has more wins to approximate total time, then blend. remote.avg_time_seconds
// We don't have total_time stored, so we reconstruct it from avg * count.
let local_total = local.avg_time_seconds as u128 * local.games_won as u128;
let remote_total = remote.avg_time_seconds as u128 * remote.games_won as u128;
// Take max total time (conservative — avoids underestimating total play time).
let best_total = local_total.max(remote_total);
(best_total / merged_games_won as u128) as u64
}; };
// Derive games_lost from the merged played/won counts so the invariant
// games_won + games_lost <= games_played is always satisfied. Computing
// max(local.games_lost, remote.games_lost) independently can push
// games_won + games_lost above games_played after a divergent merge.
let merged_games_lost = merged_games_played.saturating_sub(merged_games_won);
StatsSnapshot { StatsSnapshot {
games_played: merged_games_played, games_played: merged_games_played,
games_won: merged_games_won, games_won: merged_games_won,
games_lost: local.games_lost.max(remote.games_lost), games_lost: merged_games_lost,
win_streak_current: local.win_streak_current.max(remote.win_streak_current), win_streak_current: local.win_streak_current.max(remote.win_streak_current),
win_streak_best: local.win_streak_best.max(remote.win_streak_best), win_streak_best: local.win_streak_best.max(remote.win_streak_best),
avg_time_seconds, avg_time_seconds,
@@ -454,14 +458,28 @@ mod tests {
} }
#[test] #[test]
fn stats_games_lost_takes_max() { fn stats_games_lost_derived_from_played_minus_won() {
// games_lost must equal games_played - games_won so the invariant
// games_won + games_lost <= games_played is always satisfied.
let mut local = default_payload(); let mut local = default_payload();
local.stats.games_played = 20;
local.stats.games_won = 8;
local.stats.games_lost = 12; local.stats.games_lost = 12;
let mut remote = default_payload(); let mut remote = default_payload();
remote.stats.games_lost = 8; remote.stats.games_played = 15;
remote.stats.games_won = 10;
remote.stats.games_lost = 5;
// merged: games_played = max(20, 15) = 20; games_won = max(8, 10) = 10
// games_lost must be 20 - 10 = 10, NOT max(12, 5) = 12
let (merged, _) = merge(&local, &remote); let (merged, _) = merge(&local, &remote);
assert_eq!(merged.stats.games_lost, 12); assert_eq!(merged.stats.games_played, 20);
assert_eq!(merged.stats.games_won, 10);
assert_eq!(merged.stats.games_lost, 10);
assert!(
merged.stats.games_won + merged.stats.games_lost <= merged.stats.games_played,
"games_won + games_lost must never exceed games_played"
);
} }
#[test] #[test]
@@ -502,11 +520,10 @@ mod tests {
#[test] #[test]
fn stats_avg_time_recomputed_from_merged_totals() { fn stats_avg_time_recomputed_from_merged_totals() {
// local: 4 wins averaging 100s each (total = 400s) // local: 4 wins averaging 100s each
// remote: 6 wins averaging 200s each (total = 1200s) // remote: 6 wins averaging 200s each
// merged_games_won = max(4, 6) = 6 // merged_games_won = max(4, 6) = 6 → remote contributed the wins
// best_total = max(400, 1200) = 1200 // avg_time_seconds must be remote's 200s, not a blend of totals
// avg = 1200 / 6 = 200
let mut local = default_payload(); let mut local = default_payload();
local.stats.games_won = 4; local.stats.games_won = 4;
local.stats.avg_time_seconds = 100; local.stats.avg_time_seconds = 100;