fix(wasm,stats): surface replay errors to JS, deduplicate win events per frame (#65, #69)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Cargo.lock

@@ -7087,6 +7087,7 @@ dependencies = [
  "serde_json",
  "solitaire_core",
  "wasm-bindgen",
+ "web-sys",
 ]
 
 [[package]]

solitaire_engine/src/stats_plugin.rs

@@ -8,12 +8,12 @@
 
 use std::path::PathBuf;
 
-use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
 use bevy::input::ButtonInput;
+use bevy::input::mouse::{MouseScrollUnit, MouseWheel};
 use bevy::prelude::*;
 use solitaire_data::{
-    load_replay_history_from, load_stats_from, replay_history_path, save_stats_to,
+    PlayerProgress, Replay, ReplayHistory, StatsExt, StatsSnapshot, WEEKLY_GOALS,
-    stats_file_path, PlayerProgress, Replay, ReplayHistory, StatsExt, StatsSnapshot, WEEKLY_GOALS,
+    load_replay_history_from, load_stats_from, replay_history_path, save_stats_to, stats_file_path,
 };
 
 use crate::auto_complete_plugin::AutoCompleteState;
@@ -22,21 +22,20 @@ use crate::events::{
     ForfeitEvent, GameWonEvent, InfoToastEvent, NewGameRequestEvent, ToggleStatsRequestEvent,
     WinStreakMilestoneEvent,
 };
+use crate::font_plugin::FontResource;
 use crate::game_plugin::GameMutation;
 use crate::platform::ClipboardBackendResource;
 use crate::progress_plugin::ProgressResource;
-use crate::font_plugin::FontResource;
 use crate::resources::GameStateResource;
 use crate::time_attack_plugin::TimeAttackResource;
 use crate::ui_modal::{
-    spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
+    ButtonVariant, ModalButton, ScrimDismissible, spawn_modal, spawn_modal_actions,
-    ModalButton, ScrimDismissible,
+    spawn_modal_button, spawn_modal_header,
 };
 use crate::ui_theme::{
     ACCENT_PRIMARY, BG_ELEVATED_HI, BORDER_SUBTLE, HighContrastBorder, RADIUS_SM, STATE_INFO,
     STATE_WARNING, STREAK_MILESTONES, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY, TYPE_BODY_LG,
-    TYPE_CAPTION, TYPE_HEADLINE, VAL_SPACE_1, VAL_SPACE_2, VAL_SPACE_3, VAL_SPACE_4,
+    TYPE_CAPTION, TYPE_HEADLINE, VAL_SPACE_1, VAL_SPACE_2, VAL_SPACE_3, VAL_SPACE_4, Z_MODAL_PANEL,
-    Z_MODAL_PANEL,
 };
 
 /// Bevy resource wrapping the current stats.
@@ -211,10 +210,7 @@ impl Plugin for StatsPlugin {
             // constraints (win_summary_plugin: cache_win_data.before(StatsUpdate)),
             // and a system cannot be both inside a set and individually before a
             // set-level ordering constraint.
-            .add_systems(
+            .add_systems(Update, update_stats_on_new_game.before(GameMutation))
-                Update,
-                update_stats_on_new_game.before(GameMutation),
-            )
             .add_systems(
                 Update,
                 update_stats_on_win.after(GameMutation).in_set(StatsUpdate),
@@ -231,10 +227,7 @@ impl Plugin for StatsPlugin {
             )
             .add_systems(Update, toggle_stats_screen.after(GameMutation))
             .add_systems(Update, handle_stats_close_button)
-            .add_systems(
+            .add_systems(Update, refresh_replay_history_on_win.after(GameMutation))
-                Update,
-                refresh_replay_history_on_win.after(GameMutation),
-            )
             .add_systems(Update, handle_watch_replay_button)
             .add_systems(Update, handle_copy_share_link_button)
             .add_systems(
@@ -247,7 +240,10 @@ impl Plugin for StatsPlugin {
                     .chain(),
             )
             .add_systems(Update, scroll_stats_panel)
-            .add_systems(Update, crate::ui_modal::touch_scroll_panel::<StatsScrollable>);
+            .add_systems(
+                Update,
+                crate::ui_modal::touch_scroll_panel::<StatsScrollable>,
+            );
     }
 }
 
@@ -289,9 +285,11 @@ fn refresh_replay_history_on_win(
     path: Res<LatestReplayPath>,
 ) {
     // Only re-load when at least one win actually fired.
-    if wins.read().next().is_none() {
+    let mut win_events = wins.read();
+    if win_events.next().is_none() {
         return;
     }
+    win_events.for_each(|_| {});
     let Some(p) = path.0.as_deref() else {
         return;
     };
@@ -415,7 +413,11 @@ fn handle_replay_selector_buttons(
     if prev_pressed {
         // Step toward older replays — wrap to the oldest when at the
         // newest (index 0).
-        selected.0 = if selected.0 == 0 { len - 1 } else { selected.0 - 1 };
+        selected.0 = if selected.0 == 0 {
+            len - 1
+        } else {
+            selected.0 - 1
+        };
     }
     if next_pressed {
         // Step toward more recent replays — wrap to the newest when at
@@ -523,7 +525,12 @@ fn update_stats_on_win(
     mut milestone: MessageWriter<WinStreakMilestoneEvent>,
     mut toast: MessageWriter<InfoToastEvent>,
 ) {
-    for ev in events.read() {
+    let mut win_events = events.read();
+    let Some(ev) = win_events.next() else {
+        return;
+    };
+    win_events.for_each(|_| {});
+
     let prev_streak = stats.0.win_streak_current;
     stats
         .0
@@ -542,13 +549,10 @@ fn update_stats_on_win(
     // the highest milestone.
     if let Some(crossed) = streak_milestone_crossed(prev_streak, new_streak) {
         milestone.write(WinStreakMilestoneEvent { streak: crossed });
-            toast.write(InfoToastEvent(format!(
+        toast.write(InfoToastEvent(format!("Win streak: {crossed}! \u{1F525}")));
-                "Win streak: {crossed}! \u{1F525}"
-            )));
     }
     persist(&path, &stats.0, "win");
 }
-}
 
 /// Returns the milestone value that the player just crossed, if any.
 ///
@@ -695,14 +699,46 @@ fn spawn_stats_screen(
     // mix of "0" counters and "—" sentinels (which feels buggy).
     let is_first_launch = stats.games_played == 0;
     let dash = "\u{2014}".to_string();
-    let win_rate_str    = if is_first_launch { dash.clone() } else { format_win_rate(stats) };
+    let win_rate_str = if is_first_launch {
-    let played_str      = if is_first_launch { dash.clone() } else { format_stat_value(stats.games_played) };
+        dash.clone()
-    let won_str         = if is_first_launch { dash.clone() } else { format_stat_value(stats.games_won) };
+    } else {
-    let lost_str        = if is_first_launch { dash.clone() } else { format_stat_value(stats.games_lost) };
+        format_win_rate(stats)
-    let fastest_str     = if is_first_launch { dash.clone() } else { format_fastest_win(stats.fastest_win_seconds) };
+    };
-    let avg_time_str    = if is_first_launch { dash.clone() } else { format_avg_time(stats) };
+    let played_str = if is_first_launch {
-    let best_score_str  = if is_first_launch { dash.clone() } else { format_optional_u32(stats.best_single_score) };
+        dash.clone()
-    let best_streak_str = if is_first_launch { dash.clone() } else { format_stat_value(stats.win_streak_best) };
+    } else {
+        format_stat_value(stats.games_played)
+    };
+    let won_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_stat_value(stats.games_won)
+    };
+    let lost_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_stat_value(stats.games_lost)
+    };
+    let fastest_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_fastest_win(stats.fastest_win_seconds)
+    };
+    let avg_time_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_avg_time(stats)
+    };
+    let best_score_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_optional_u32(stats.best_single_score)
+    };
+    let best_streak_str = if is_first_launch {
+        dash.clone()
+    } else {
+        format_stat_value(stats.win_streak_best)
+    };
 
     let font_handle = font_res.map(|f| f.0.clone()).unwrap_or_default();
     let font_section = TextFont {
@@ -882,7 +918,8 @@ fn spawn_stats_screen(
 
             // --- Time Attack section ---
             if let Some(ta) = time_attack
-                && ta.active {
+                && ta.active
+            {
                 let mins = (ta.remaining_secs / 60.0).floor() as u64;
                 let secs = (ta.remaining_secs % 60.0).floor() as u64;
                 body.spawn((
@@ -1197,7 +1234,11 @@ fn xp_to_next_level_label(total_xp: u64, level: u32) -> String {
     };
     let span = xp_next - xp_current;
     let done = total_xp.saturating_sub(xp_current).min(span);
-    let pct = if span == 0 { 100 } else { done.saturating_mul(100).checked_div(span).unwrap_or(100) };
+    let pct = if span == 0 {
+        100
+    } else {
+        done.saturating_mul(100).checked_div(span).unwrap_or(100)
+    };
     let remaining = span - done;
     format!("{remaining} XP ({pct}%)")
 }
@@ -1291,8 +1332,34 @@ mod tests {
         app.update();
 
         let stats = &app.world().resource::<StatsResource>().0;
-        assert_eq!(stats.draw_three_wins, 1, "draw_three_wins must increment for DrawThree mode");
+        assert_eq!(
-        assert_eq!(stats.draw_one_wins, 0, "draw_one_wins must not increment for DrawThree mode");
+            stats.draw_three_wins, 1,
+            "draw_three_wins must increment for DrawThree mode"
+        );
+        assert_eq!(
+            stats.draw_one_wins, 0,
+            "draw_one_wins must not increment for DrawThree mode"
+        );
+    }
+
+    #[test]
+    fn multiple_win_events_in_one_frame_increment_once() {
+        let mut app = headless_app();
+        app.world_mut().write_message(GameWonEvent {
+            score: 1000,
+            time_seconds: 120,
+        });
+        app.world_mut().write_message(GameWonEvent {
+            score: 1500,
+            time_seconds: 90,
+        });
+        app.update();
+
+        let stats = &app.world().resource::<StatsResource>().0;
+        assert_eq!(stats.games_won, 1);
+        assert_eq!(stats.games_played, 1);
+        assert_eq!(stats.best_single_score, 1000);
+        assert_eq!(stats.fastest_win_seconds, 120);
     }
 
     #[test]
@@ -1304,8 +1371,11 @@ mod tests {
             .0
             .move_count = 3;
 
-        app.world_mut()
+        app.world_mut().write_message(NewGameRequestEvent {
-            .write_message(NewGameRequestEvent { seed: Some(999), mode: None, confirmed: false });
+            seed: Some(999),
+            mode: None,
+            confirmed: false,
+        });
         app.update();
 
         let stats = &app.world().resource::<StatsResource>().0;
@@ -1317,8 +1387,11 @@ mod tests {
     #[test]
     fn new_game_without_moves_does_not_record_abandoned() {
         let mut app = headless_app();
-        app.world_mut()
+        app.world_mut().write_message(NewGameRequestEvent {
-            .write_message(NewGameRequestEvent { seed: Some(42), mode: None, confirmed: false });
+            seed: Some(42),
+            mode: None,
+            confirmed: false,
+        });
         app.update();
 
         let stats = &app.world().resource::<StatsResource>().0;
@@ -1629,10 +1702,7 @@ mod tests {
 
         let events = app.world().resource::<Messages<InfoToastEvent>>();
         let mut reader = events.get_cursor();
-        let messages: Vec<&str> = reader
+        let messages: Vec<&str> = reader.read(events).map(|e| e.0.as_str()).collect();
-            .read(events)
-            .map(|e| e.0.as_str())
-            .collect();
 
         assert!(
             messages.contains(&"Streak of 3 broken!"),
@@ -1658,10 +1728,7 @@ mod tests {
 
         let events = app.world().resource::<Messages<InfoToastEvent>>();
         let mut reader = events.get_cursor();
-        let messages: Vec<&str> = reader
+        let messages: Vec<&str> = reader.read(events).map(|e| e.0.as_str()).collect();
-            .read(events)
-            .map(|e| e.0.as_str())
-            .collect();
 
         assert!(
             !messages.iter().any(|m| m.contains("broken")),
@@ -1820,8 +1887,7 @@ mod tests {
         let texts: Vec<String> = q.iter(app.world()).map(|t| t.0.clone()).collect();
         assert_eq!(texts.len(), 1);
         assert_eq!(
-            texts[0],
+            texts[0], "Replay 1 / 1",
-            "Replay 1 / 1",
             "caption must show '1 / 1' for a single-replay history"
         );
     }

solitaire_wasm/Cargo.toml

@@ -24,6 +24,7 @@ console_error_panic_hook = { version = "0.1", optional = true }
 # `solitaire_core`'s deps with wasm-only flags.
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 getrandom = { version = "0.3", features = ["wasm_js"] }
+web-sys = { version = "0.3", features = ["console"] }
 
 [features]
 default = ["console_error_panic_hook"]

solitaire_wasm/src/lib.rs

@@ -21,6 +21,7 @@
 use chrono::NaiveDate;
 use serde::{Deserialize, Serialize};
 use solitaire_core::card::Suit;
+use solitaire_core::error::MoveError;
 use solitaire_core::game_state::{DrawMode, GameMode, GameState};
 use solitaire_core::pile::PileType;
 use wasm_bindgen::prelude::*;
@@ -108,6 +109,14 @@ pub struct ReplayPlayer {
     step_idx: usize,
 }
 
+fn log_replay_move_error(err: &MoveError) {
+    #[cfg(target_arch = "wasm32")]
+    web_sys::console::error_1(&format!("Replay move failed: {:?}", err).into());
+
+    #[cfg(not(target_arch = "wasm32"))]
+    eprintln!("Replay move failed: {:?}", err);
+}
+
 // Native-callable methods. Used by both the wasm-bindgen interface
 // below and by unit tests, which can't go through `serde_wasm_bindgen`
 // (it panics on non-wasm targets).
@@ -118,8 +127,7 @@ impl ReplayPlayer {
     pub fn from_json(replay_json: &str) -> Result<Self, String> {
         let replay: Replay =
             serde_json::from_str(replay_json).map_err(|e| format!("invalid replay JSON: {e}"))?;
-        let game =
+        let game = GameState::new_with_mode(replay.seed, replay.draw_mode, replay.mode);
-            GameState::new_with_mode(replay.seed, replay.draw_mode, replay.mode);
         Ok(Self {
             game,
             moves: replay.moves,
@@ -127,18 +135,18 @@ impl ReplayPlayer {
         })
     }
 
-    /// Apply the next move. Returns `None` once the list is exhausted.
+    /// Apply the next move. Returns `Ok(None)` once the list is exhausted.
-    pub fn step_native(&mut self) -> Option<StateSnapshot> {
+    pub fn step_native(&mut self) -> Result<Option<StateSnapshot>, MoveError> {
         if self.step_idx >= self.moves.len() {
-            return None;
+            return Ok(None);
         }
         let mv = self.moves[self.step_idx].clone();
-        let _ = match mv {
+        match mv {
-            ReplayMove::Move { from, to, count } => self.game.move_cards(from, to, count),
+            ReplayMove::Move { from, to, count } => self.game.move_cards(from, to, count)?,
-            ReplayMove::StockClick => self.game.draw(),
+            ReplayMove::StockClick => self.game.draw()?,
-        };
+        }
         self.step_idx += 1;
-        Some(self.snapshot())
+        Ok(Some(self.snapshot()))
     }
 
     fn snapshot(&self) -> StateSnapshot {
@@ -205,12 +213,19 @@ impl ReplayPlayer {
     /// once the move list is exhausted.
     ///
     /// Returns `null` (not an exception) when the replay is finished.
+    /// Throws `"replay_desync"` when the next recorded move is illegal for
+    /// the current state, and logs the underlying core error to the JS console.
     /// Throws a JS string exception on serialisation failure.
     pub fn step(&mut self) -> Result<JsValue, JsValue> {
         match self.step_native() {
-            Some(snap) => serde_wasm_bindgen::to_value(&snap)
+            Ok(Some(snap)) => {
-                .map_err(|e| JsValue::from_str(&e.to_string())),
+                serde_wasm_bindgen::to_value(&snap).map_err(|e| JsValue::from_str(&e.to_string()))
-            None => Ok(JsValue::NULL),
+            }
+            Ok(None) => Ok(JsValue::NULL),
+            Err(e) => {
+                log_replay_move_error(&e);
+                Err(JsValue::from_str("replay_desync"))
+            }
         }
     }
 
@@ -282,8 +297,16 @@ impl SolitaireGame {
                 .unwrap_or_default()
         };
         let has_moves = {
-            let stock_empty = self.game.piles.get(&PileType::Stock).is_none_or(|p| p.cards.is_empty());
+            let stock_empty = self
-            let waste_empty = self.game.piles.get(&PileType::Waste).is_none_or(|p| p.cards.is_empty());
+                .game
+                .piles
+                .get(&PileType::Stock)
+                .is_none_or(|p| p.cards.is_empty());
+            let waste_empty = self
+                .game
+                .piles
+                .get(&PileType::Waste)
+                .is_none_or(|p| p.cards.is_empty());
             !stock_empty || !waste_empty || !self.game.possible_instructions().is_empty()
         };
         GameSnapshot {
@@ -383,8 +406,7 @@ impl SolitaireGame {
     ///
     /// Throws a JS string exception on serialisation failure.
     pub fn state(&self) -> Result<JsValue, JsValue> {
-        serde_wasm_bindgen::to_value(&self.snap())
+        serde_wasm_bindgen::to_value(&self.snap()).map_err(|e| JsValue::from_str(&e.to_string()))
-            .map_err(|e| JsValue::from_str(&e.to_string()))
     }
 
     /// The seed used to deal this game.
@@ -435,8 +457,7 @@ impl SolitaireGame {
     /// Use [`SolitaireGame::from_saved`] to restore it. The returned string is
     /// opaque — callers should treat it as a blob and store/restore it verbatim.
     pub fn serialize(&self) -> Result<String, JsValue> {
-        serde_json::to_string(&self.game)
+        serde_json::to_string(&self.game).map_err(|e| JsValue::from_str(&e.to_string()))
-            .map_err(|e| JsValue::from_str(&e.to_string()))
     }
 
     /// Restore a game from a JSON string previously produced by [`SolitaireGame::serialize`].
@@ -509,11 +530,27 @@ mod tests {
     #[test]
     fn steps_advance_then_terminate() {
         let mut player = ReplayPlayer::from_json(&sample_replay_json()).expect("valid JSON");
-        assert!(player.step_native().is_some());
+        assert!(
+            player
+                .step_native()
+                .expect("first move should apply")
+                .is_some()
+        );
         assert_eq!(player.step_idx, 1);
-        assert!(player.step_native().is_some());
+        assert!(
+            player
+                .step_native()
+                .expect("second move should apply")
+                .is_some()
+        );
         assert_eq!(player.step_idx, 2);
-        assert!(player.step_native().is_none(), "no further steps");
+        assert!(
+            player
+                .step_native()
+                .expect("replay should be exhausted")
+                .is_none(),
+            "no further steps"
+        );
     }
 
     /// Malformed JSON returns an error rather than panicking.
@@ -523,6 +560,35 @@ mod tests {
         assert!(result.is_err());
     }
 
+    #[test]
+    fn invalid_replay_move_returns_error_without_advancing() {
+        let replay = Replay {
+            schema_version: 2,
+            seed: 42,
+            draw_mode: DrawMode::DrawOne,
+            mode: GameMode::Classic,
+            time_seconds: 60,
+            final_score: 100,
+            recorded_at: NaiveDate::from_ymd_opt(2026, 5, 2).expect("valid date"),
+            moves: vec![ReplayMove::Move {
+                from: PileType::Waste,
+                to: PileType::Foundation(0),
+                count: 1,
+            }],
+        };
+        let json = serde_json::to_string(&replay).expect("replay serialises");
+        let mut player = ReplayPlayer::from_json(&json).expect("valid JSON");
+
+        let err = player
+            .step_native()
+            .expect_err("illegal replay move must surface an error");
+        assert_eq!(err, MoveError::EmptySource);
+        assert_eq!(
+            player.step_idx, 0,
+            "desync must not advance the replay cursor"
+        );
+    }
+
     // -------------------------------------------------------------------------
     // Winning-sequence step-through
     // -------------------------------------------------------------------------
@@ -686,8 +752,7 @@ mod tests {
             mode: GameMode::Classic,
             time_seconds: 300,
             final_score: 0,
-            recorded_at: NaiveDate::from_ymd_opt(2026, 5, 12)
+            recorded_at: NaiveDate::from_ymd_opt(2026, 5, 12).expect("2026-05-12 is a valid date"),
-                .expect("2026-05-12 is a valid date"),
             moves: winning_moves.clone(),
         };
         let json = serde_json::to_string(&replay).expect("replay serialises to JSON cleanly");
@@ -702,7 +767,10 @@ mod tests {
         );
 
         let mut last_snap: Option<StateSnapshot> = None;
-        while let Some(snap) = player.step_native() {
+        while let Some(snap) = player
+            .step_native()
+            .expect("solver-generated replay must stay in sync")
+        {
             last_snap = Some(snap);
         }
 
@@ -719,7 +787,10 @@ mod tests {
             "step_idx after the last move must equal the total move count"
         );
         assert!(
-            player.step_native().is_none(),
+            player
+                .step_native()
+                .expect("winning replay should still be exhausted")
+                .is_none(),
             "step_native must return None once all moves are exhausted"
         );
     }