diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index d908794..8b26a14 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -193,7 +193,7 @@ impl GameState { is_auto_completable: false, undo_count: 0, recycle_count: 0, - take_from_foundation: true, + take_from_foundation: false, schema_version: GAME_STATE_SCHEMA_VERSION, undo_stack: VecDeque::new(), } @@ -407,7 +407,7 @@ impl GameState { self.score = if self.mode == GameMode::Zen { 0 } else { - (snapshot.score + scoring_undo()).max(0) + (self.score + scoring_undo()).max(0) }; self.move_count = snapshot.move_count; self.is_won = false; @@ -416,12 +416,25 @@ impl GameState { 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 { - (0..4_u8).all(|slot| { - self.piles - .get(&PileType::Foundation(slot)) - .is_some_and(|p| p.cards.len() == 13) + (0..4_u8).all(|slot| self.is_valid_foundation_pile(slot)) + } + + 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] - fn take_from_foundation_allowed_by_default() { - let mut g = setup_take_from_foundation_game(); - assert!(g.take_from_foundation, "standard Klondike allows take-from-foundation by default"); - 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); + fn take_from_foundation_disabled_by_default() { + let g = setup_take_from_foundation_game(); + assert!(!g.take_from_foundation, "take_from_foundation is off by default (non-standard rule)"); } #[test] diff --git a/solitaire_engine/src/achievement_plugin.rs b/solitaire_engine/src/achievement_plugin.rs index b7f2b92..22f0351 100644 --- a/solitaire_engine/src/achievement_plugin.rs +++ b/solitaire_engine/src/achievement_plugin.rs @@ -32,7 +32,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath}; use crate::stats_plugin::{StatsResource, StatsUpdate}; use crate::ui_modal::{ spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, - ScrimDismissible, + ModalScrim, ScrimDismissible, }; use crate::ui_theme::{ ACCENT_PRIMARY, BORDER_SUBTLE, STATE_SUCCESS, TEXT_DISABLED, TEXT_PRIMARY, TEXT_SECONDARY, @@ -162,93 +162,91 @@ fn evaluate_on_win( mut achievements: ResMut, mut progress: ResMut, ) { - let Some(ev) = wins.read().last() else { - return; - }; - - let ctx = AchievementContext { - games_played: stats.0.games_played, - games_won: stats.0.games_won, - win_streak_current: stats.0.win_streak_current, - best_single_score: stats.0.best_single_score, - lifetime_score: stats.0.lifetime_score, - draw_three_wins: stats.0.draw_three_wins, - daily_challenge_streak: progress.0.daily_challenge_streak, - last_win_score: ev.score, - last_win_time_seconds: ev.time_seconds, - last_win_used_undo: game.0.undo_count > 0, - 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; + for ev in wins.read() { + let ctx = AchievementContext { + games_played: stats.0.games_played, + games_won: stats.0.games_won, + win_streak_current: stats.0.win_streak_current, + best_single_score: stats.0.best_single_score, + lifetime_score: stats.0.lifetime_score, + draw_three_wins: stats.0.draw_three_wins, + daily_challenge_streak: progress.0.daily_challenge_streak, + last_win_score: ev.score, + last_win_time_seconds: ev.time_seconds, + last_win_used_undo: game.0.undo_count > 0, + 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, }; - if record.unlocked { + + let hits = check_achievements(&ctx); + if hits.is_empty() { continue; } - 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 => {} - } + 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 { + continue; } - 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. @@ -391,6 +389,7 @@ fn toggle_achievements_screen( achievements: Res, font_res: Option>, screens: Query>, + other_modal_scrims: Query<(), (With, Without)>, ) { let button_clicked = requests.read().count() > 0; if !keys.just_pressed(KeyCode::KeyA) && !button_clicked { @@ -398,7 +397,7 @@ fn toggle_achievements_screen( } if let Ok(entity) = screens.single() { commands.entity(entity).despawn(); - } else { + } else if other_modal_scrims.is_empty() { spawn_achievements_screen(&mut commands, &achievements.0, font_res.as_deref()); } } diff --git a/solitaire_engine/src/auto_complete_plugin.rs b/solitaire_engine/src/auto_complete_plugin.rs index feb7404..b3ab749 100644 --- a/solitaire_engine/src/auto_complete_plugin.rs +++ b/solitaire_engine/src/auto_complete_plugin.rs @@ -72,9 +72,14 @@ fn detect_auto_complete( if game.0.is_auto_completable && !state.active { state.active = true; 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. diff --git a/solitaire_engine/src/card_animation/mod.rs b/solitaire_engine/src/card_animation/mod.rs index 0632fe2..325cb14 100644 --- a/solitaire_engine/src/card_animation/mod.rs +++ b/solitaire_engine/src/card_animation/mod.rs @@ -142,6 +142,13 @@ impl Plugin for CardAnimationPlugin { update_frame_time_diagnostics, // Advance active 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. advance_animation_chains, // Interaction visuals (run after animation for final positions). diff --git a/solitaire_engine/src/cursor_plugin.rs b/solitaire_engine/src/cursor_plugin.rs index 5531528..cf46431 100644 --- a/solitaire_engine/src/cursor_plugin.rs +++ b/solitaire_engine/src/cursor_plugin.rs @@ -382,8 +382,8 @@ fn update_drop_target_overlays( /// for everything else it is card-sized. Replicated here rather than /// imported because `pile_drop_rect` is private to `input_plugin` and /// this overlay is the only other consumer. -fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> (Vec2, Vec2) { - let centre = layout.pile_positions[pile]; +fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> Option<(Vec2, Vec2)> { + let centre = layout.pile_positions.get(pile).copied()?; if matches!(pile, PileType::Tableau(_)) { let card_count = game.piles.get(pile).map_or(0, |p| p.cards.len()); 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 span_height = top_edge - bottom_edge; let new_centre_y = (top_edge + bottom_edge) / 2.0; - return ( + return Some(( Vec2::new(centre.x, new_centre_y), 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 @@ -410,7 +410,10 @@ fn spawn_drop_target_overlay( layout: &Layout, 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; commands diff --git a/solitaire_engine/src/feedback_anim_plugin.rs b/solitaire_engine/src/feedback_anim_plugin.rs index 6dcd80a..e6bfa27 100644 --- a/solitaire_engine/src/feedback_anim_plugin.rs +++ b/solitaire_engine/src/feedback_anim_plugin.rs @@ -210,10 +210,15 @@ impl Plugin for FeedbackAnimPlugin { start_shake_anim.after(GameMutation), tick_shake_anim, 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, start_deal_anim.after(GameMutation), start_foundation_flourish.after(GameMutation), - tick_foundation_flourish, ), ); } diff --git a/solitaire_engine/src/font_plugin.rs b/solitaire_engine/src/font_plugin.rs index e16e434..8098554 100644 --- a/solitaire_engine/src/font_plugin.rs +++ b/solitaire_engine/src/font_plugin.rs @@ -31,8 +31,15 @@ fn load_font(fonts: Option>>, mut commands: Commands) { // Assets). FontPlugin in that context is a no-op — consumers // already query `Option>` and degrade cleanly. let Some(mut fonts) = fonts else { return }; - let font = Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec()) - .expect("bundled FiraMono failed to parse — binary is corrupt"); + let font = match Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec()) { + 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); commands.insert_resource(FontResource(handle)); } diff --git a/solitaire_engine/src/help_plugin.rs b/solitaire_engine/src/help_plugin.rs index 04d77e2..1c85b5a 100644 --- a/solitaire_engine/src/help_plugin.rs +++ b/solitaire_engine/src/help_plugin.rs @@ -13,7 +13,7 @@ use crate::font_plugin::FontResource; use crate::hud_plugin::ANDROID_HINT_LABEL; use crate::ui_modal::{ 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}; #[cfg(not(target_os = "android"))] @@ -67,6 +67,7 @@ fn toggle_help_screen( keys: Res>, mut requests: MessageReader, screens: Query>, + other_modal_scrims: Query<(), (With, Without)>, font_res: Option>, ) { // 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() { commands.entity(entity).despawn(); - } else { + } else if other_modal_scrims.is_empty() { spawn_help_screen(&mut commands, font_res.as_deref()); } } diff --git a/solitaire_engine/src/hud_plugin.rs b/solitaire_engine/src/hud_plugin.rs index 48dff13..87b543e 100644 --- a/solitaire_engine/src/hud_plugin.rs +++ b/solitaire_engine/src/hud_plugin.rs @@ -870,12 +870,12 @@ fn spawn_action_buttons( ); #[cfg(not(target_os = "android"))] let labels = ( - "Menu \u{25BE}", + "Menu \u{2193}", "Undo", "Pause", "Help", "Hint", - "Modes \u{25BE}", + "Modes \u{2193}", "New Game", ); diff --git a/solitaire_engine/src/input_plugin.rs b/solitaire_engine/src/input_plugin.rs index 45c0eb7..2d729db 100644 --- a/solitaire_engine/src/input_plugin.rs +++ b/solitaire_engine/src/input_plugin.rs @@ -47,6 +47,7 @@ use crate::game_plugin::{ConfirmNewGameScreen, GameMutation, RestorePromptScreen use crate::pause_plugin::PausedResource; use crate::progress_plugin::ProgressResource; use crate::layout::{Layout, LayoutResource}; +use crate::replay_playback::ReplayPlaybackState; use crate::resources::{DragState, GameInputConsumedResource, GameStateResource, HintCycleIndex}; use crate::selection_plugin::SelectionState; use crate::time_attack_plugin::TimeAttackResource; @@ -175,11 +176,20 @@ fn handle_keyboard_core( mut zen_requests: MessageReader, confirm_screens: Query<(), With>, restore_prompts: Query<(), With>, + replay_state: Option>, ) { if paused.is_some_and(|p| p.0) { 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) { ev.undo.write(UndoRequestEvent); } diff --git a/solitaire_engine/src/leaderboard_plugin.rs b/solitaire_engine/src/leaderboard_plugin.rs index 2d7628a..44e95e5 100644 --- a/solitaire_engine/src/leaderboard_plugin.rs +++ b/solitaire_engine/src/leaderboard_plugin.rs @@ -21,7 +21,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath}; use crate::sync_plugin::SyncProviderResource; use crate::ui_modal::{ spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant, - ScrimDismissible, + ModalScrim, ScrimDismissible, }; use crate::ui_theme::{ ACCENT_PRIMARY, BG_ELEVATED, BORDER_SUBTLE, RADIUS_SM, STATE_INFO, @@ -714,6 +714,7 @@ fn data_cell( fn handle_set_display_name_button( button_q: Query<&Interaction, (Changed, With)>, existing: Query<(), With>, + other_modal_scrims: Query<(), (With, Without)>, mut commands: Commands, settings: Option>, font_res: Option>, @@ -725,6 +726,9 @@ fn handle_set_display_name_button( if !existing.is_empty() { return; // already open } + if !other_modal_scrims.is_empty() { + return; // Another modal is already visible. + } buf.0 = settings .as_ref() .and_then(|s| s.0.leaderboard_display_name.clone()) diff --git a/solitaire_engine/src/play_by_seed_plugin.rs b/solitaire_engine/src/play_by_seed_plugin.rs index a34170a..ad663b0 100644 --- a/solitaire_engine/src/play_by_seed_plugin.rs +++ b/solitaire_engine/src/play_by_seed_plugin.rs @@ -411,7 +411,11 @@ fn handle_confirm( new_game.write(NewGameRequestEvent { seed: Some(seed), 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 { @@ -566,7 +570,9 @@ mod tests { assert_eq!(fired.len(), 1); assert_eq!(fired[0].seed, Some(42)); 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. assert!(!dialog_present(&mut app)); diff --git a/solitaire_engine/src/replay_overlay.rs b/solitaire_engine/src/replay_overlay.rs index 020d528..0b9db90 100644 --- a/solitaire_engine/src/replay_overlay.rs +++ b/solitaire_engine/src/replay_overlay.rs @@ -28,7 +28,7 @@ use chrono::Datelike; use crate::font_plugin::FontResource; use crate::layout::LayoutResource; -use crate::events::{DrawRequestEvent, MoveRequestEvent, UndoRequestEvent}; +use crate::events::{DrawRequestEvent, MoveRequestEvent, StateChangedEvent, UndoRequestEvent}; use crate::replay_playback::{ step_backwards_replay_playback, step_replay_playback, stop_replay_playback, toggle_pause_replay_playback, ReplayPlaybackState, @@ -476,6 +476,7 @@ impl Plugin for ReplayOverlayPlugin { .add_message::() .add_message::() .add_message::() + .add_message::() .add_systems( Update, ( @@ -1884,6 +1885,7 @@ fn handle_pause_keyboard( /// resets to 0 on key release so the next fresh press fires /// immediately. This matches the mockup's `[← →] scrub` /// terminology while keeping single-press = single-step semantics. +#[allow(clippy::too_many_arguments)] fn handle_arrow_keyboard( keys: Option>>, time: Res