From 9bbb57134f7b87ca7ec7c84936d0608a81021934 Mon Sep 17 00:00:00 2001 From: funman300 Date: Fri, 12 Jun 2026 12:43:47 -0700 Subject: [PATCH] refactor: persist replay/save moves as KlondikeInstruction, not pile coords (#89) Pile-position types (Tableau, Foundation, KlondikePile, KlondikePileStack) are runtime-only and have no serde upstream. Per Rhys's guidance, the persistence layer now stores the moves (KlondikeInstruction) rather than board coordinates, decoding back to runtime pile positions on demand. Core / data: - game_state: instruction_history() -> Vec; add instruction_to_piles() and apply_instruction(); drop AnyInstruction. - klondike_adapter: delete the entire Saved* serde mirror section (SavedTableau/Foundation/SkipCards/KlondikePile/TableauStack/ KlondikePileStack/DstFoundation/DstTableau/SavedInstruction). - replay: drop the bespoke ReplayMove serde mirror; Replay.moves is now Vec; REPLAY_SCHEMA_VERSION 2 -> 3. - storage: game_state save format v3 rejected (v4/v5 only). Engine / wasm consumers: - record via KlondikeInstruction (stock click = RotateStock). - playback decodes each instruction to (from, to, count) against the live state via instruction_to_piles, then fires the canonical event; undecodable instructions are skipped with a warning, never panic. - remove all use solitaire_data::ReplayMove and Saved* imports. Workspace check, clippy -D warnings, and the full test suite all pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- solitaire_core/src/game_state.rs | 72 ++--- solitaire_core/src/klondike_adapter.rs | 266 +----------------- solitaire_core/src/proptest_tests.rs | 119 +------- solitaire_data/src/lib.rs | 2 +- solitaire_data/src/replay.rs | 115 ++++---- solitaire_data/src/storage.rs | 31 +- solitaire_engine/src/achievement_plugin.rs | 6 +- solitaire_engine/src/game_plugin.rs | 73 ++--- solitaire_engine/src/replay_overlay/format.rs | 50 ++-- solitaire_engine/src/replay_overlay/input.rs | 24 +- solitaire_engine/src/replay_overlay/tests.rs | 34 ++- solitaire_engine/src/replay_overlay/update.rs | 16 +- solitaire_engine/src/replay_playback.rs | 140 ++++----- solitaire_wasm/src/lib.rs | 173 +++--------- 14 files changed, 311 insertions(+), 810 deletions(-) diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index 4d0942b..4691d32 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -1,7 +1,6 @@ use crate::error::MoveError; use crate::klondike_adapter::{ - KlondikeAdapter, SavedInstruction, - foundation_from_slot as adapter_foundation_from_slot, + KlondikeAdapter, foundation_from_slot as adapter_foundation_from_slot, skip_cards_from_count as adapter_skip_cards_from_count, tableau_from_index as adapter_tableau_from_index, }; @@ -19,11 +18,10 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; /// History: /// - v1: `Foundation(Suit)` keys. /// - v2: `Foundation(u8)` slot keys; claimed suit derived from the bottom card. -/// - v3: session-backed save files using local `SavedInstruction` mirror types -/// with u8 indices for enum variants. +/// - v3 (rejected): session-backed save files using local mirror types with u8 +/// indices for enum variants. No longer loadable — v3 files are discarded. /// - v4: `saved_moves` uses upstream `KlondikeInstruction` serde with named enum -/// variants (e.g. `"Foundation1"` instead of `0`). v3 files are auto-migrated -/// on load via `AnyInstruction` transparent deserialization. +/// variants (e.g. `"Foundation1"` instead of `0`). /// - v5 (current): `score`, `undo_count`, and `recycle_count` are no longer /// persisted. They are derived from the upstream `card_game`/`klondike` session /// stats, which are rebuilt by replaying `saved_moves` on load. Older files that @@ -110,28 +108,13 @@ struct PersistedGameState { pub saved_moves: Vec, } -/// Transparent migration wrapper for deserialisation. +/// Input struct that accepts schema v4 and v5 `saved_moves` formats. /// -/// Tries `KlondikeInstruction` (schema v4, named variants) first; if that -/// fails (because the value uses u8 indices), falls back to `SavedInstruction` -/// (schema v3). Converting the V3 variant yields a `KlondikeInstruction` via -/// the existing `TryFrom` impl. -/// -/// `SavedInstruction` remains `pub` in `klondike_adapter` because -/// `solitaire_data::ReplayMove` and the WASM replay layer depend on it. -#[derive(Debug, Clone, Deserialize)] -#[serde(untagged)] -enum AnyInstruction { - V4(KlondikeInstruction), - V3(SavedInstruction), -} - -/// Input struct that accepts schema v3, v4, and v5 `saved_moves` formats. -/// -/// `score`, `undo_count`, and `recycle_count` are intentionally absent: all -/// three are rebuilt by replaying the instruction history through the upstream -/// session stats. Older save files (v3/v4) still carry those keys; serde ignores -/// them. +/// `saved_moves` is deserialised directly as upstream `KlondikeInstruction` +/// (named-variant serde). `score`, `undo_count`, and `recycle_count` are +/// intentionally absent: all three are rebuilt by replaying the instruction +/// history through the upstream session stats. Older v4 save files still carry +/// those keys; serde ignores them. #[derive(Debug, Clone, Deserialize)] struct PersistedGameStateIn { pub draw_mode: DrawStockConfig, @@ -143,7 +126,7 @@ struct PersistedGameStateIn { pub take_from_foundation: bool, #[serde(default = "schema_v1")] pub schema_version: u32, - pub saved_moves: Vec, + pub saved_moves: Vec, } #[cfg(feature = "test-support")] @@ -249,10 +232,10 @@ impl<'de> Deserialize<'de> for GameState { fn deserialize>(deserializer: D) -> Result { let persisted = PersistedGameStateIn::deserialize(deserializer)?; - // Accept v3 (legacy u8-index format, auto-migrated), v4 (upstream - // named-variant serde), and v5 (current, derived stats). Reject the rest. + // Accept v4 (upstream named-variant serde) and v5 (current, derived + // stats). v3 (legacy u8-index format) and all others are rejected. match persisted.schema_version { - 3..=5 => {} + 4 | 5 => {} v => { return Err(serde::de::Error::custom(format!( "unsupported GameState schema version {v}" @@ -276,17 +259,7 @@ impl<'de> Deserialize<'de> for GameState { // to 0 across save/load because undone moves are not part of the saved // forward history. let replay_config = Self::replay_config(persisted.draw_mode); - for any in persisted.saved_moves { - // AnyInstruction::V4 arrives directly from upstream serde (schema v4+). - // AnyInstruction::V3 was serialised with u8 indices (schema v3) and is - // converted here via the existing TryFrom impl. - let instruction = match any { - AnyInstruction::V4(i) => i, - AnyInstruction::V3(s) => { - KlondikeInstruction::try_from(s).map_err(serde::de::Error::custom)? - } - }; - + for instruction in persisted.saved_moves { if !game .session .state() @@ -440,20 +413,17 @@ impl GameState { } /// Returns the deterministic instruction history for the current deal as - /// legacy mirror types. + /// upstream [`KlondikeInstruction`] values. /// /// Combined with [`GameState::seed`] and [`GameState::draw_mode`], this - /// sequence is sufficient to replay the game state exactly. - /// - /// Returns [`SavedInstruction`] (u8-index mirror types) for backward - /// compatibility with the WASM replay layer and `solitaire_data::ReplayMove` - /// format. New code that does not need serde should prefer - /// `session().history()` directly. - pub fn instruction_history(&self) -> Vec { + /// sequence is sufficient to replay the game state exactly. Consumers + /// record these directly (they serialise via `KlondikeInstruction`'s + /// compact serde) and play them back via [`GameState::apply_instruction`]. + pub fn instruction_history(&self) -> Vec { self.session .history() .iter() - .map(|snapshot| SavedInstruction::from(*snapshot.instruction())) + .map(|snapshot| *snapshot.instruction()) .collect() } diff --git a/solitaire_core/src/klondike_adapter.rs b/solitaire_core/src/klondike_adapter.rs index 6c666dc..9114e96 100644 --- a/solitaire_core/src/klondike_adapter.rs +++ b/solitaire_core/src/klondike_adapter.rs @@ -10,11 +10,9 @@ //! (`card`, `pile`, etc.) remain free of upstream dependencies. use klondike::{ - DrawStockConfig, DstFoundation, DstTableau, Foundation, KlondikeConfig, KlondikeInstruction, - KlondikePile, KlondikePileStack, MoveFromFoundationConfig, ScoringConfig, SkipCards, Tableau, - TableauStack, + DrawStockConfig, Foundation, KlondikeConfig, MoveFromFoundationConfig, ScoringConfig, + SkipCards, Tableau, }; -use serde::{Deserialize, Serialize}; /// Bridges `solitaire_core` game config and scoring to the upstream `klondike` crate. /// @@ -84,266 +82,6 @@ pub fn skip_cards_from_count(skip: usize) -> Option { } } -// ── Legacy serde mirror types (kept for backward compatibility) ─────────────── -// -// These types were introduced when upstream `klondike` had no serde feature. -// Mainline `klondike` now provides full serde support (with a hand-written -// compact `KlondikeInstruction` impl), and `GameState` serialises -// `saved_moves` directly as `Vec` (schema v4). -// -// The mirror types are retained for three reasons: -// 1. Schema v3 migration: `AnyInstruction` in `game_state.rs` uses -// `TryFrom for KlondikeInstruction` to parse old save -// files with u8 indices and replay them. -// 2. `solitaire_data::ReplayMove` uses `SavedKlondikePile` as its serde -// type; changing it would break the on-disk replay format (schema v2). -// 3. `solitaire_wasm` mirrors `ReplayMove` using the same types so that -// replay JSON is cross-compatible between the desktop and browser builds. -// -// These types should not be used for new serialisation concerns. If the -// ReplayMove format is ever bumped to a new schema, migrate those callers to -// `KlondikePile` / `KlondikePileStack` and the types here can then be deleted. - -/// A `Serialize` + `Deserialize` mirror of [`klondike::Tableau`] (0 = Tableau1 … 6 = Tableau7). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedTableau(pub u8); - -/// A `Serialize` + `Deserialize` mirror of [`klondike::Foundation`] (0 = Foundation1 … 3 = Foundation4). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedFoundation(pub u8); - -/// A `Serialize` + `Deserialize` mirror of [`klondike::SkipCards`] (0 = Skip0 … 12 = Skip12). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedSkipCards(pub u8); - -/// A `Serialize` + `Deserialize` mirror of [`klondike::KlondikePile`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub enum SavedKlondikePile { - Tableau(SavedTableau), - Stock, - Foundation(SavedFoundation), -} - -/// A `Serialize` + `Deserialize` mirror of [`klondike::TableauStack`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedTableauStack { - pub tableau: SavedTableau, - pub skip_cards: SavedSkipCards, -} - -/// A `Serialize` + `Deserialize` mirror of [`klondike::KlondikePileStack`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub enum SavedKlondikePileStack { - Tableau(SavedTableauStack), - Stock, - Foundation(SavedFoundation), -} - -/// A `Serialize` + `Deserialize` mirror of [`klondike::DstFoundation`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedDstFoundation { - pub src: SavedKlondikePile, - pub foundation: SavedFoundation, -} - -/// A `Serialize` + `Deserialize` mirror of [`klondike::DstTableau`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub struct SavedDstTableau { - pub src: SavedKlondikePileStack, - pub tableau: SavedTableau, -} - -/// A `Serialize` + `Deserialize` mirror of [`klondike::KlondikeInstruction`]. -/// -/// Convert to/from the upstream type with: -/// ```ignore -/// let saved = SavedInstruction::from(instruction); -/// let instruction = KlondikeInstruction::try_from(saved)?; -/// ``` -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -pub enum SavedInstruction { - DstFoundation(SavedDstFoundation), - DstTableau(SavedDstTableau), - RotateStock, -} - -/// Error returned when a [`SavedInstruction`] contains an out-of-range numeric value -/// and cannot be converted back to a [`klondike::KlondikeInstruction`]. -#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] -pub enum InvalidSavedInstruction { - #[error("invalid tableau index {0} (expected 0–6)")] - Tableau(u8), - #[error("invalid foundation index {0} (expected 0–3)")] - Foundation(u8), - #[error("invalid skip_cards value {0} (expected 0–12)")] - SkipCards(u8), -} - -// ── From impls: KlondikeInstruction → Saved* ───────────────────────────────── - -impl From for SavedTableau { - fn from(t: Tableau) -> Self { - Self(t as u8) - } -} - -impl From for SavedFoundation { - fn from(f: Foundation) -> Self { - Self(f as u8) - } -} - -impl From for SavedSkipCards { - fn from(s: SkipCards) -> Self { - Self(s as u8) - } -} - -impl From for SavedKlondikePile { - fn from(p: KlondikePile) -> Self { - match p { - KlondikePile::Tableau(t) => Self::Tableau(t.into()), - KlondikePile::Stock => Self::Stock, - KlondikePile::Foundation(f) => Self::Foundation(f.into()), - } - } -} - -impl From for SavedTableauStack { - fn from(ts: TableauStack) -> Self { - Self { - tableau: ts.tableau.into(), - skip_cards: ts.skip_cards.into(), - } - } -} - -impl From for SavedKlondikePileStack { - fn from(ps: KlondikePileStack) -> Self { - match ps { - KlondikePileStack::Tableau(ts) => Self::Tableau(ts.into()), - KlondikePileStack::Stock => Self::Stock, - KlondikePileStack::Foundation(f) => Self::Foundation(f.into()), - } - } -} - -impl From for SavedDstFoundation { - fn from(df: DstFoundation) -> Self { - Self { - src: df.src.into(), - foundation: df.foundation.into(), - } - } -} - -impl From for SavedDstTableau { - fn from(dt: DstTableau) -> Self { - Self { - src: dt.src.into(), - tableau: dt.tableau.into(), - } - } -} - -impl From for SavedInstruction { - fn from(i: KlondikeInstruction) -> Self { - match i { - KlondikeInstruction::RotateStock => Self::RotateStock, - KlondikeInstruction::DstFoundation(df) => Self::DstFoundation(df.into()), - KlondikeInstruction::DstTableau(dt) => Self::DstTableau(dt.into()), - } - } -} - -// ── TryFrom impls: Saved* → KlondikeInstruction ────────────────────────────── - -impl TryFrom for Tableau { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedTableau) -> Result { - tableau_from_index(s.0 as usize).ok_or(InvalidSavedInstruction::Tableau(s.0)) - } -} - -impl TryFrom for Foundation { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedFoundation) -> Result { - foundation_from_slot(s.0).ok_or(InvalidSavedInstruction::Foundation(s.0)) - } -} - -impl TryFrom for SkipCards { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedSkipCards) -> Result { - skip_cards_from_count(s.0 as usize).ok_or(InvalidSavedInstruction::SkipCards(s.0)) - } -} - -impl TryFrom for KlondikePile { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedKlondikePile) -> Result { - Ok(match s { - SavedKlondikePile::Tableau(t) => KlondikePile::Tableau(t.try_into()?), - SavedKlondikePile::Stock => KlondikePile::Stock, - SavedKlondikePile::Foundation(f) => KlondikePile::Foundation(f.try_into()?), - }) - } -} - -impl TryFrom for TableauStack { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedTableauStack) -> Result { - Ok(TableauStack { - tableau: s.tableau.try_into()?, - skip_cards: s.skip_cards.try_into()?, - }) - } -} - -impl TryFrom for KlondikePileStack { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedKlondikePileStack) -> Result { - Ok(match s { - SavedKlondikePileStack::Tableau(ts) => KlondikePileStack::Tableau(ts.try_into()?), - SavedKlondikePileStack::Stock => KlondikePileStack::Stock, - SavedKlondikePileStack::Foundation(f) => KlondikePileStack::Foundation(f.try_into()?), - }) - } -} - -impl TryFrom for DstFoundation { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedDstFoundation) -> Result { - Ok(DstFoundation { - src: s.src.try_into()?, - foundation: s.foundation.try_into()?, - }) - } -} - -impl TryFrom for DstTableau { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedDstTableau) -> Result { - Ok(DstTableau { - src: s.src.try_into()?, - tableau: s.tableau.try_into()?, - }) - } -} - -impl TryFrom for KlondikeInstruction { - type Error = InvalidSavedInstruction; - fn try_from(s: SavedInstruction) -> Result { - Ok(match s { - SavedInstruction::RotateStock => KlondikeInstruction::RotateStock, - SavedInstruction::DstFoundation(df) => { - KlondikeInstruction::DstFoundation(df.try_into()?) - } - SavedInstruction::DstTableau(dt) => KlondikeInstruction::DstTableau(dt.try_into()?), - }) - } -} - /// Time bonus added to the score on a win: `700_000 / elapsed_seconds`. /// Returns 0 when `elapsed_seconds` is 0 to avoid division by zero. pub fn compute_time_bonus(elapsed_seconds: u64) -> i32 { diff --git a/solitaire_core/src/proptest_tests.rs b/solitaire_core/src/proptest_tests.rs index 3ec7cc4..03417a5 100644 --- a/solitaire_core/src/proptest_tests.rs +++ b/solitaire_core/src/proptest_tests.rs @@ -1,13 +1,8 @@ use card_game::{Card, Game}; -use klondike::{DrawStockConfig, Foundation, KlondikePile, KlondikeInstruction, SkipCards, Tableau}; +use klondike::{DrawStockConfig, Foundation, KlondikePile, Tableau}; use proptest::prelude::*; use crate::game_state::GameState; -use crate::klondike_adapter::{ - InvalidSavedInstruction, SavedDstFoundation, SavedDstTableau, SavedFoundation, - SavedInstruction, SavedKlondikePile, SavedKlondikePileStack, SavedSkipCards, SavedTableau, - SavedTableauStack, -}; // --------------------------------------------------------------------------- // Shared helpers @@ -261,116 +256,4 @@ proptest! { } } - // ------------------------------------------------------------------------- - // SavedInstruction ↔ KlondikeInstruction round-trip - // ------------------------------------------------------------------------- - - /// Every valid `SavedInstruction` survives a round-trip through - /// `KlondikeInstruction::try_from(SavedInstruction::from(original))`. - /// - /// Covers all three variants (`RotateStock`, `DstFoundation`, `DstTableau`) - /// and all legal sub-field ranges: - /// - `SavedTableau`: 0–6 - /// - `SavedFoundation`: 0–3 - /// - `SavedSkipCards`: 0–12 - #[test] - fn saved_instruction_round_trip( - instruction in saved_instruction_strategy(), - ) { - let klondike = KlondikeInstruction::try_from(instruction); - prop_assert!( - klondike.is_ok(), - "TryFrom failed for valid SavedInstruction {instruction:?}: {:?}", - klondike.err(), - ); - let saved_again = SavedInstruction::from(klondike.expect("checked above")); - prop_assert_eq!( - saved_again, - instruction, - "round-trip produced a different SavedInstruction", - ); - } -} - -// --------------------------------------------------------------------------- -// Proptest strategies for SavedInstruction and its sub-types -// --------------------------------------------------------------------------- - -fn saved_tableau_strategy() -> impl Strategy { - (0u8..=6).prop_map(SavedTableau) -} - -fn saved_foundation_strategy() -> impl Strategy { - (0u8..=3).prop_map(SavedFoundation) -} - -fn saved_skip_cards_strategy() -> impl Strategy { - (0u8..=12).prop_map(SavedSkipCards) -} - -fn saved_klondike_pile_strategy() -> impl Strategy { - prop_oneof![ - saved_tableau_strategy().prop_map(SavedKlondikePile::Tableau), - Just(SavedKlondikePile::Stock), - saved_foundation_strategy().prop_map(SavedKlondikePile::Foundation), - ] -} - -fn saved_klondike_pile_stack_strategy() -> impl Strategy { - prop_oneof![ - (saved_tableau_strategy(), saved_skip_cards_strategy()).prop_map(|(tableau, skip_cards)| { - SavedKlondikePileStack::Tableau(SavedTableauStack { tableau, skip_cards }) - }), - Just(SavedKlondikePileStack::Stock), - saved_foundation_strategy().prop_map(SavedKlondikePileStack::Foundation), - ] -} - -fn saved_instruction_strategy() -> impl Strategy { - prop_oneof![ - Just(SavedInstruction::RotateStock), - (saved_klondike_pile_strategy(), saved_foundation_strategy()).prop_map( - |(src, foundation)| { - SavedInstruction::DstFoundation(SavedDstFoundation { src, foundation }) - } - ), - (saved_klondike_pile_stack_strategy(), saved_tableau_strategy()).prop_map( - |(src, tableau)| { - SavedInstruction::DstTableau(SavedDstTableau { src, tableau }) - } - ), - ] -} - -// --------------------------------------------------------------------------- -// Boundary error unit tests (exact out-of-range values) -// --------------------------------------------------------------------------- - -#[cfg(test)] -mod saved_instruction_boundary_tests { - use super::*; - - #[test] - fn saved_tableau_7_is_invalid() { - let result = Tableau::try_from(SavedTableau(7)); - assert_eq!(result, Err(InvalidSavedInstruction::Tableau(7))); - } - - #[test] - fn saved_tableau_255_is_invalid() { - let result = Tableau::try_from(SavedTableau(255)); - assert_eq!(result, Err(InvalidSavedInstruction::Tableau(255))); - } - - #[test] - fn saved_foundation_4_is_invalid() { - let result = Foundation::try_from(SavedFoundation(4)); - assert_eq!(result, Err(InvalidSavedInstruction::Foundation(4))); - } - - #[test] - fn saved_skip_cards_13_is_invalid() { - let result = SkipCards::try_from(SavedSkipCards(13)); - assert_eq!(result, Err(InvalidSavedInstruction::SkipCards(13))); - } } diff --git a/solitaire_data/src/lib.rs b/solitaire_data/src/lib.rs index 8b52881..415597e 100644 --- a/solitaire_data/src/lib.rs +++ b/solitaire_data/src/lib.rs @@ -163,7 +163,7 @@ pub use sync_client::{SolitaireServerClient, provider_for_backend}; pub mod replay; pub use replay::{ REPLAY_HISTORY_CAP, REPLAY_HISTORY_SCHEMA_VERSION, REPLAY_SCHEMA_VERSION, Replay, - ReplayHistory, ReplayMove, append_replay_to_history, load_replay_history_from, + ReplayHistory, append_replay_to_history, load_replay_history_from, migrate_legacy_latest_replay, replay_history_path, save_replay_history_to, }; // `latest_replay_path` is still consumed by the engine's one-shot legacy diff --git a/solitaire_data/src/replay.rs b/solitaire_data/src/replay.rs index 86d9480..4dad2d7 100644 --- a/solitaire_data/src/replay.rs +++ b/solitaire_data/src/replay.rs @@ -12,13 +12,22 @@ //! carries any other version so older replays are silently dropped instead //! of crashing the loader. //! -//! The recording is intentionally minimal — only [`ReplayMove`] entries -//! that successfully advanced the game. `Undo` is **not** recorded: a -//! replay represents the canonical path the player ultimately took to win, -//! so backed-out missteps simply do not appear in the move list. The -//! starting deal is not stored either — the [`seed`](Replay::seed) + +//! The recording is intentionally minimal — only the +//! [`KlondikeInstruction`](solitaire_core::KlondikeInstruction) inputs that +//! successfully advanced the game. `Undo` is **not** recorded: a replay +//! represents the canonical path the player ultimately took to win, so +//! backed-out missteps simply do not appear in the move list. The starting +//! deal is not stored either — the [`seed`](Replay::seed) + //! [`draw_mode`](Replay::draw_mode) + [`mode`](Replay::mode) are sufficient //! for `GameState::new_with_mode` to rebuild the identical layout. +//! +//! Each recorded move is the player's atomic *input*, not its outcome. +//! `KlondikeInstruction::RotateStock` covers every click on the stock pile; +//! the engine resolves draw-vs-recycle deterministically from the current +//! stock state during playback, so the same input always produces the same +//! effect on the same starting deal. Runtime-only pile-position types are +//! never serialised — the instruction itself serialises via its compact +//! upstream serde representation. use std::fs; use std::io; @@ -26,8 +35,7 @@ use std::path::{Path, PathBuf}; use chrono::NaiveDate; use serde::{Deserialize, Serialize}; -use solitaire_core::{DrawStockConfig, game_state::GameMode}; -use solitaire_core::klondike_adapter::SavedKlondikePile; +use solitaire_core::{DrawStockConfig, KlondikeInstruction, game_state::GameMode}; const LATEST_REPLAY_FILE_NAME: &str = "latest_replay.json"; const REPLAY_HISTORY_FILE_NAME: &str = "replays.json"; @@ -65,14 +73,17 @@ fn history_schema_v0() -> u32 { /// seeing a broken one. /// /// History: -/// - v1: initial release. `ReplayMove` had separate `Draw` and `Recycle` +/// - v1: initial release. The move type had separate `Draw` and `Recycle` /// variants which carried the *outcome* of a stock interaction rather /// than the player's atomic input. -/// - v2 (current): `Draw` + `Recycle` collapsed into a single `StockClick` -/// variant. The engine resolves draw-vs-recycle deterministically from -/// the current stock state, so the input alone is sufficient and the -/// replay model now stores atomic player inputs end-to-end. -pub const REPLAY_SCHEMA_VERSION: u32 = 2; +/// - v2: `Draw` + `Recycle` collapsed into a single `StockClick` variant. +/// - v3 (current): the bespoke `ReplayMove` serde mirror was dropped. Moves +/// are now stored directly as upstream +/// [`KlondikeInstruction`](solitaire_core::KlondikeInstruction) (compact +/// int serde); `StockClick` is now `RotateStock`. Pile-position types are +/// runtime-only and are never serialised. v1/v2 files fail to deserialise +/// and are discarded by the loader. +pub const REPLAY_SCHEMA_VERSION: u32 = 3; /// Default value for [`Replay::schema_version`] when deserialising files /// that pre-date the field. Any value other than [`REPLAY_SCHEMA_VERSION`] @@ -81,32 +92,6 @@ fn schema_v0() -> u32 { 0 } -/// One atomic player input recorded during a winning game, in the order -/// it was applied to the live `GameState`. -/// -/// `Undo` is intentionally absent — see the module-level docs. -/// -/// The variants represent *inputs*, not outcomes. `StockClick` covers -/// every player click on the stock pile; the engine then resolves -/// draw-vs-recycle deterministically from the current state during both -/// recording and playback, so the same input always produces the same -/// effect on the same starting deal. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum ReplayMove { - /// A successful `move_cards(from, to, count)` call. - Move { - /// Source pile. - from: SavedKlondikePile, - /// Destination pile. - to: SavedKlondikePile, - /// Number of cards moved. - count: usize, - }, - /// A click on the stock pile. Resolves to a draw when stock is - /// non-empty and to a waste→stock recycle when stock is empty. - StockClick, -} - /// A complete recording of a single winning game. /// /// Replays are reconstructed by rebuilding a fresh @@ -134,9 +119,11 @@ pub struct Replay { pub final_score: i32, /// ISO-8601 date the win was recorded. pub recorded_at: NaiveDate, - /// Ordered move list. Each entry is what the player did, replayable - /// against a fresh `GameState` constructed from the seed. - pub moves: Vec, + /// Ordered move list. Each entry is the atomic + /// [`KlondikeInstruction`](solitaire_core::KlondikeInstruction) the player + /// issued, replayable against a fresh `GameState` constructed from the + /// seed via `GameState::apply_instruction`. + pub moves: Vec, /// Public share URL for this replay on the active sync backend, set /// by `sync_plugin::poll_replay_upload_result` when the upload /// task resolves. `None` when the player won on a local-only @@ -185,7 +172,7 @@ impl Replay { time_seconds: u64, final_score: i32, recorded_at: NaiveDate, - moves: Vec, + moves: Vec, ) -> Self { Self { schema_version: REPLAY_SCHEMA_VERSION, @@ -442,7 +429,9 @@ pub fn migrate_legacy_latest_replay(latest_path: &Path, history_path: &Path) { #[allow(deprecated)] mod tests { use super::*; - use solitaire_core::klondike_adapter::{SavedFoundation, SavedTableau}; + use klondike::{ + DstFoundation, DstTableau, Foundation, KlondikePile, KlondikePileStack, Tableau, + }; use std::env; fn tmp_path(name: &str) -> PathBuf { @@ -459,18 +448,16 @@ mod tests { 5_120, date, vec![ - ReplayMove::StockClick, - ReplayMove::Move { - from: SavedKlondikePile::Stock, - to: SavedKlondikePile::Tableau(SavedTableau(3)), - count: 1, - }, - ReplayMove::StockClick, - ReplayMove::Move { - from: SavedKlondikePile::Tableau(SavedTableau(3)), - to: SavedKlondikePile::Foundation(SavedFoundation(0)), - count: 1, - }, + KlondikeInstruction::RotateStock, + KlondikeInstruction::DstTableau(DstTableau { + src: KlondikePileStack::Stock, + tableau: Tableau::Tableau4, + }), + KlondikeInstruction::RotateStock, + KlondikeInstruction::DstFoundation(DstFoundation { + src: KlondikePile::Tableau(Tableau::Tableau4), + foundation: Foundation::Foundation1, + }), ], ) } @@ -601,7 +588,7 @@ mod tests { 60, id, date, - vec![ReplayMove::StockClick], + vec![KlondikeInstruction::RotateStock], ) } @@ -837,9 +824,11 @@ mod tests { let path = tmp_path("legacy_no_win_move_index"); let _ = fs::remove_file(&path); - // Hand-rolled minimal v2 replay JSON with no win_move_index field. - let v2_no_field = r#"{ - "schema_version": 2, + // Hand-rolled minimal current-schema replay JSON with no + // win_move_index field — the additive field must still default to None. + let no_field = format!( + r#"{{ + "schema_version": {schema}, "seed": 1, "draw_mode": "DrawOne", "mode": "Classic", @@ -847,8 +836,10 @@ mod tests { "final_score": 100, "recorded_at": "2026-05-02", "moves": [] - }"#; - fs::write(&path, v2_no_field).expect("write fixture"); + }}"#, + schema = REPLAY_SCHEMA_VERSION, + ); + fs::write(&path, no_field).expect("write fixture"); let loaded = load_latest_replay_from(&path).expect("load"); assert_eq!(loaded.win_move_index, None); diff --git a/solitaire_data/src/storage.rs b/solitaire_data/src/storage.rs index a272043..d734eda 100644 --- a/solitaire_data/src/storage.rs +++ b/solitaire_data/src/storage.rs @@ -583,24 +583,16 @@ mod tests { ); } - /// A schema v3 save (instruction history using u8 indices) must load - /// successfully and be transparently migrated to schema v4. - /// - /// This verifies the `AnyInstruction` untagged deserialization migration - /// path. v3 files with `RotateStock` (unit variant, format-identical in - /// v3 and v4) load correctly and report `schema_version == 4` after load. - /// The `SavedInstruction` boundary tests in `proptest_tests.rs` cover the - /// u8-to-named conversion for `DstFoundation` / `DstTableau` indices. + /// A schema v3 save (instruction history using the old u8-index mirror + /// types) is no longer loadable. The legacy migration path was dropped, + /// so any file claiming `schema_version: 3` must be rejected and the + /// player started on a fresh game. #[test] - fn game_state_v3_migrates_to_v4() { - use solitaire_core::game_state::GameState; - - let path = gs_path("v3_migrate"); + fn game_state_v3_is_rejected() { + let path = gs_path("v3_reject"); let _ = fs::remove_file(&path); // Hand-crafted schema v3 JSON: one RotateStock (draw) instruction. - // RotateStock serialises as the string "RotateStock" in both v3 and v4, - // so this exercises the schema version acceptance code path. let v3_json = r#"{ "draw_mode": "DrawOne", "mode": "Classic", @@ -615,13 +607,12 @@ mod tests { }"#; fs::write(&path, v3_json).expect("write v3 fixture"); - let loaded = load_game_state_from(&path) - .expect("schema v3 must be accepted and migrated to v4"); + assert!( + load_game_state_from(&path).is_none(), + "schema v3 must be rejected (no migration path)", + ); - // The loaded game should match a fresh game that had one draw applied. - let mut expected = GameState::new(42, DrawStockConfig::DrawOne); - expected.draw().expect("draw must succeed on a fresh game"); - assert_eq!(loaded, expected, "migrated v3 game state must match equivalent v4 state"); + let _ = fs::remove_file(&path); } /// Schema v2 stored raw pile arrays and undo snapshots (no instruction diff --git a/solitaire_engine/src/achievement_plugin.rs b/solitaire_engine/src/achievement_plugin.rs index 5d987f2..7c7a882 100644 --- a/solitaire_engine/src/achievement_plugin.rs +++ b/solitaire_engine/src/achievement_plugin.rs @@ -1393,8 +1393,8 @@ mod tests { use crate::replay_playback::ReplayPlaybackState; use chrono::NaiveDate; - use solitaire_core::{DrawStockConfig, game_state::GameMode}; - use solitaire_data::{Replay, ReplayMove}; + use solitaire_core::{DrawStockConfig, KlondikeInstruction, game_state::GameMode}; + use solitaire_data::Replay; /// Headless app variant that injects a default `ReplayPlaybackState` /// directly (no `ReplayPlaybackPlugin`) so we can drive the resource @@ -1414,7 +1414,7 @@ mod tests { 10, 100, NaiveDate::from_ymd_opt(2026, 5, 5).expect("valid date"), - vec![ReplayMove::StockClick], + vec![KlondikeInstruction::RotateStock], ) } diff --git a/solitaire_engine/src/game_plugin.rs b/solitaire_engine/src/game_plugin.rs index e822129..3ccdeab 100644 --- a/solitaire_engine/src/game_plugin.rs +++ b/solitaire_engine/src/game_plugin.rs @@ -15,11 +15,11 @@ use bevy::tasks::{AsyncComputeTaskPool, Task, futures_lite::future}; use bevy::window::AppLifecycle; use solitaire_core::KlondikePile; use solitaire_core::{DrawStockConfig, game_state::{GameMode, GameState}}; -use solitaire_core::{DEFAULT_SOLVE_MOVES_BUDGET, DEFAULT_SOLVE_STATES_BUDGET}; +use solitaire_core::{DEFAULT_SOLVE_MOVES_BUDGET, DEFAULT_SOLVE_STATES_BUDGET, KlondikeInstruction}; #[allow(deprecated)] use solitaire_data::latest_replay_path; use solitaire_data::{ - Replay, ReplayMove, SOLVER_DEAL_RETRY_CAP, append_replay_to_history, delete_game_state_at, + Replay, SOLVER_DEAL_RETRY_CAP, append_replay_to_history, delete_game_state_at, game_state_file_path, load_game_state_from, migrate_legacy_latest_replay, replay_history_path, save_game_state_to, }; @@ -105,18 +105,21 @@ pub struct RestoreContinueButton; #[derive(Component, Debug)] pub struct RestoreNewGameButton; -/// In-memory accumulator for [`ReplayMove`] entries during the current -/// game. Cleared on every new-game start; frozen into a [`Replay`] and -/// flushed to disk by [`record_replay_on_win`] when the player wins. +/// In-memory accumulator for [`KlondikeInstruction`] entries during the +/// current game. Cleared on every new-game start; frozen into a [`Replay`] +/// and flushed to disk by [`record_replay_on_win`] when the player wins. /// /// Recording captures only successful state-mutating events the player /// drove (`MoveRequestEvent`, `DrawRequestEvent`). `UndoRequestEvent` is /// intentionally not recorded — see [`solitaire_data::replay`] for the -/// design rationale. +/// design rationale. Each entry is the atomic player input as a +/// [`KlondikeInstruction`] (a stock click is +/// [`KlondikeInstruction::RotateStock`]); pile-position types are +/// runtime-only and never persisted. #[derive(Resource, Debug, Default, Clone)] pub struct RecordingReplay { - /// Ordered list of moves applied so far this game. - pub moves: Vec, + /// Ordered list of instructions applied so far this game. + pub moves: Vec, } impl RecordingReplay { @@ -851,7 +854,7 @@ fn handle_draw( // the click happens — re-executing on the same starting // deal produces the same effect, so the input alone is // sufficient to recover the move on playback. - recording.moves.push(ReplayMove::StockClick); + recording.moves.push(KlondikeInstruction::RotateStock); changed.write(StateChangedEvent); } Err(e) => warn!("draw rejected: {e}"), @@ -889,11 +892,17 @@ fn handle_move( // Record the move in the in-flight replay buffer. Done // first so the entry is captured even if a subsequent // event-write or pile-lookup happens to bail out below. - recording.moves.push(ReplayMove::Move { - from: ev.from.into(), - to: ev.to.into(), - count: ev.count, - }); + // `move_cards` resolved the pile coordinates to a + // `KlondikeInstruction` and pushed it onto the session + // history; recover that exact instruction from the tail + // (no clone — the instruction is `Copy`). Pile-position + // types are runtime-only, so we persist the instruction + // rather than the (from, to, count) triple. + if let Some(instruction) = + game.0.session().history().last().map(|s| *s.instruction()) + { + recording.moves.push(instruction); + } // Fire flip event if the candidate card is now face-up. if let Some(fcard) = flip_candidate && pile_cards(&game.0, &ev.from) @@ -1301,7 +1310,6 @@ fn save_game_state_on_exit( mod tests { use super::*; use solitaire_core::{Foundation, KlondikePile, Tableau}; - use solitaire_core::klondike_adapter::{SavedKlondikePile, SavedTableau}; /// Build a minimal headless `App` with just `GamePlugin` installed. /// Disables persistence and overrides the seed so tests are deterministic @@ -2102,7 +2110,7 @@ mod tests { 1, "only the draw is recorded; the undo does not erase it nor add a new entry", ); - assert!(matches!(recording.moves[0], ReplayMove::StockClick)); + assert!(matches!(recording.moves[0], KlondikeInstruction::RotateStock)); } /// Starting a new game wipes the recording so the next deal begins @@ -2154,16 +2162,16 @@ mod tests { let mut app = test_app(7654); app.insert_resource(ReplayPath(Some(path.clone()))); - // Push two recorded moves manually so we can verify they survive - // the freeze/save round-trip without having to drive a real win. + // Push two recorded instructions manually so we can verify they + // survive the freeze/save round-trip without having to drive a + // real win. Both are `RotateStock` — the only instruction + // constructible without the runtime-only `klondike` pile-stack + // types (which the engine intentionally does not depend on); the + // round-trip shape is identical for any instruction variant. { let mut recording = app.world_mut().resource_mut::(); - recording.moves.push(ReplayMove::StockClick); - recording.moves.push(ReplayMove::Move { - from: SavedKlondikePile::Stock, - to: SavedKlondikePile::Tableau(SavedTableau(2)), - count: 1, - }); + recording.moves.push(KlondikeInstruction::RotateStock); + recording.moves.push(KlondikeInstruction::RotateStock); } // Fire the win event the engine emits when the last foundation @@ -2197,15 +2205,8 @@ mod tests { "time_seconds must come from the win event" ); assert_eq!(loaded.moves.len(), 2, "every recorded move must round-trip"); - assert!(matches!(loaded.moves[0], ReplayMove::StockClick)); - match &loaded.moves[1] { - ReplayMove::Move { from, to, count } => { - assert_eq!(*from, SavedKlondikePile::Stock); - assert_eq!(*to, SavedKlondikePile::Tableau(SavedTableau(2))); - assert_eq!(*count, 1); - } - other => panic!("second entry must be a Move, got {other:?}"), - } + assert!(matches!(loaded.moves[0], KlondikeInstruction::RotateStock)); + assert!(matches!(loaded.moves[1], KlondikeInstruction::RotateStock)); #[cfg(not(target_arch = "wasm32"))] let _ = std::fs::remove_file(&path); @@ -2229,7 +2230,7 @@ mod tests { { let mut recording = app.world_mut().resource_mut::(); recording.moves.clear(); - recording.moves.push(ReplayMove::StockClick); + recording.moves.push(KlondikeInstruction::RotateStock); } app.world_mut().write_message(GameWonEvent { score: 100, @@ -2241,8 +2242,8 @@ mod tests { { let mut recording = app.world_mut().resource_mut::(); recording.moves.clear(); - recording.moves.push(ReplayMove::StockClick); - recording.moves.push(ReplayMove::StockClick); + recording.moves.push(KlondikeInstruction::RotateStock); + recording.moves.push(KlondikeInstruction::RotateStock); } app.world_mut().write_message(GameWonEvent { score: 200, diff --git a/solitaire_engine/src/replay_overlay/format.rs b/solitaire_engine/src/replay_overlay/format.rs index d3e141b..226f360 100644 --- a/solitaire_engine/src/replay_overlay/format.rs +++ b/solitaire_engine/src/replay_overlay/format.rs @@ -1,10 +1,8 @@ use super::ReplayPlaybackState; use chrono::Datelike; -use solitaire_core::{Foundation, KlondikePile, Tableau}; -use solitaire_core::{Card, Rank, Suit}; use solitaire_core::game_state::GameState; -use solitaire_core::klondike_adapter::SavedKlondikePile; -use solitaire_data::ReplayMove; +use solitaire_core::{Card, Rank, Suit}; +use solitaire_core::{Foundation, KlondikeInstruction, KlondikePile, Tableau}; /// Pure helper — formats the `GAME #YYYY-DDD` caption for the given /// state. Returns `None` for `Inactive` / `Completed` (the replay is @@ -60,12 +58,6 @@ pub(crate) fn format_pile(p: &KlondikePile) -> String { } } -pub(crate) fn format_saved_pile(p: &SavedKlondikePile) -> String { - KlondikePile::try_from(*p) - .map(|pile| format_pile(&pile)) - .unwrap_or_else(|_| "unknown pile".to_string()) -} - fn foundation_number(foundation: Foundation) -> u8 { match foundation { Foundation::Foundation1 => 1, @@ -87,20 +79,36 @@ fn tableau_number(tableau: Tableau) -> u8 { } } -/// Pure helper — formats a [`ReplayMove`] as the body of a -/// move-log row. `StockClick` reads as `"stock cycle"`; `Move` -/// reads as `"{from} → {to}"` using [`format_pile`] for both -/// endpoints. The `count` field is omitted from the row body — -/// at row scale it adds visual noise without meaningful +/// Pure helper — formats a [`KlondikeInstruction`] as the body of a +/// move-log row. `RotateStock` reads as `"stock cycle"`; a `Dst*` +/// instruction reads as `"{from} → {to}"` using [`format_pile`] for +/// each nameable endpoint. The card count is omitted from the row +/// body — at row scale it adds visual noise without meaningful /// information for the typical 1-card moves. -pub(crate) fn format_move_body(m: &ReplayMove) -> String { - match m { - ReplayMove::StockClick => "stock cycle".to_string(), - ReplayMove::Move { from, to, .. } => { +/// +/// The destination pile is always recoverable directly from the +/// instruction. The source pile is shown when it is statically +/// nameable (a `DstFoundation` carries a [`KlondikePile`] source); +/// a `DstTableau`'s source is the runtime-only `KlondikePileStack` +/// type — not re-exported across the `solitaire_core` boundary and so +/// not pattern-matchable here — so its row renders `"→ {to}"` without +/// a leading source label. Faithful full-coordinate decoding lives in +/// [`GameState::instruction_to_piles`] on the playback path; the +/// move-log is a display-only digest. +pub(crate) fn format_move_body(instruction: &KlondikeInstruction) -> String { + match instruction { + KlondikeInstruction::RotateStock => "stock cycle".to_string(), + KlondikeInstruction::DstFoundation(dst) => { format!( "{} \u{2192} {}", - format_saved_pile(from), - format_saved_pile(to) + format_pile(&dst.src), + format_pile(&KlondikePile::Foundation(dst.foundation)) + ) + } + KlondikeInstruction::DstTableau(dst) => { + format!( + "\u{2192} {}", + format_pile(&KlondikePile::Tableau(dst.tableau)) ) } } diff --git a/solitaire_engine/src/replay_overlay/input.rs b/solitaire_engine/src/replay_overlay/input.rs index a7199ee..46ea0d1 100644 --- a/solitaire_engine/src/replay_overlay/input.rs +++ b/solitaire_engine/src/replay_overlay/input.rs @@ -8,6 +8,7 @@ use crate::replay_playback::{ ReplayPlaybackState, step_backwards_replay_playback, step_replay_playback, stop_replay_playback, toggle_pause_replay_playback, }; +use crate::resources::GameStateResource; /// Per-arrow-key time-since-last-fire accumulators that drive the /// continuous-scrub repeat behaviour for held arrow keys. Each @@ -1033,6 +1034,7 @@ pub(crate) fn handle_pause_button( /// guard lives inside `step_replay_playback`. pub(crate) fn handle_step_button( mut state: ResMut, + game: Option>, mut moves_writer: MessageWriter, mut draws_writer: MessageWriter, buttons: Query<&Interaction, (With, Changed)>, @@ -1040,7 +1042,12 @@ pub(crate) fn handle_step_button( if !buttons.iter().any(|i| *i == Interaction::Pressed) { return; } - step_replay_playback(&mut state, &mut moves_writer, &mut draws_writer); + step_replay_playback( + &mut state, + game.as_deref(), + &mut moves_writer, + &mut draws_writer, + ); } /// Repaints the Pause / Resume button's label whenever @@ -1112,6 +1119,7 @@ pub(crate) fn handle_pause_keyboard( pub(crate) fn handle_arrow_keyboard( keys: Option>>, time: Res