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<KlondikeInstruction>; 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<KlondikeInstruction>; 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) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-12 12:43:47 -07:00
parent e0a858d4e8
commit 9bbb57134f
14 changed files with 311 additions and 810 deletions
+21 -51
View File
@@ -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<KlondikeInstruction>,
}
/// 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<AnyInstruction>,
pub saved_moves: Vec<KlondikeInstruction>,
}
#[cfg(feature = "test-support")]
@@ -249,10 +232,10 @@ impl<'de> Deserialize<'de> for GameState {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
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<SavedInstruction> {
/// 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<KlondikeInstruction> {
self.session
.history()
.iter()
.map(|snapshot| SavedInstruction::from(*snapshot.instruction()))
.map(|snapshot| *snapshot.instruction())
.collect()
}
+2 -264
View File
@@ -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<SkipCards> {
}
}
// ── 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<KlondikeInstruction>` (schema v4).
//
// The mirror types are retained for three reasons:
// 1. Schema v3 migration: `AnyInstruction` in `game_state.rs` uses
// `TryFrom<SavedInstruction> 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 06)")]
Tableau(u8),
#[error("invalid foundation index {0} (expected 03)")]
Foundation(u8),
#[error("invalid skip_cards value {0} (expected 012)")]
SkipCards(u8),
}
// ── From impls: KlondikeInstruction → Saved* ─────────────────────────────────
impl From<Tableau> for SavedTableau {
fn from(t: Tableau) -> Self {
Self(t as u8)
}
}
impl From<Foundation> for SavedFoundation {
fn from(f: Foundation) -> Self {
Self(f as u8)
}
}
impl From<SkipCards> for SavedSkipCards {
fn from(s: SkipCards) -> Self {
Self(s as u8)
}
}
impl From<KlondikePile> 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<TableauStack> for SavedTableauStack {
fn from(ts: TableauStack) -> Self {
Self {
tableau: ts.tableau.into(),
skip_cards: ts.skip_cards.into(),
}
}
}
impl From<KlondikePileStack> 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<DstFoundation> for SavedDstFoundation {
fn from(df: DstFoundation) -> Self {
Self {
src: df.src.into(),
foundation: df.foundation.into(),
}
}
}
impl From<DstTableau> for SavedDstTableau {
fn from(dt: DstTableau) -> Self {
Self {
src: dt.src.into(),
tableau: dt.tableau.into(),
}
}
}
impl From<KlondikeInstruction> 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<SavedTableau> for Tableau {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedTableau) -> Result<Self, Self::Error> {
tableau_from_index(s.0 as usize).ok_or(InvalidSavedInstruction::Tableau(s.0))
}
}
impl TryFrom<SavedFoundation> for Foundation {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedFoundation) -> Result<Self, Self::Error> {
foundation_from_slot(s.0).ok_or(InvalidSavedInstruction::Foundation(s.0))
}
}
impl TryFrom<SavedSkipCards> for SkipCards {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedSkipCards) -> Result<Self, Self::Error> {
skip_cards_from_count(s.0 as usize).ok_or(InvalidSavedInstruction::SkipCards(s.0))
}
}
impl TryFrom<SavedKlondikePile> for KlondikePile {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedKlondikePile) -> Result<Self, Self::Error> {
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<SavedTableauStack> for TableauStack {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedTableauStack) -> Result<Self, Self::Error> {
Ok(TableauStack {
tableau: s.tableau.try_into()?,
skip_cards: s.skip_cards.try_into()?,
})
}
}
impl TryFrom<SavedKlondikePileStack> for KlondikePileStack {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedKlondikePileStack) -> Result<Self, Self::Error> {
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<SavedDstFoundation> for DstFoundation {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedDstFoundation) -> Result<Self, Self::Error> {
Ok(DstFoundation {
src: s.src.try_into()?,
foundation: s.foundation.try_into()?,
})
}
}
impl TryFrom<SavedDstTableau> for DstTableau {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedDstTableau) -> Result<Self, Self::Error> {
Ok(DstTableau {
src: s.src.try_into()?,
tableau: s.tableau.try_into()?,
})
}
}
impl TryFrom<SavedInstruction> for KlondikeInstruction {
type Error = InvalidSavedInstruction;
fn try_from(s: SavedInstruction) -> Result<Self, Self::Error> {
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 {
+1 -118
View File
@@ -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`: 06
/// - `SavedFoundation`: 03
/// - `SavedSkipCards`: 012
#[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<Value = SavedTableau> {
(0u8..=6).prop_map(SavedTableau)
}
fn saved_foundation_strategy() -> impl Strategy<Value = SavedFoundation> {
(0u8..=3).prop_map(SavedFoundation)
}
fn saved_skip_cards_strategy() -> impl Strategy<Value = SavedSkipCards> {
(0u8..=12).prop_map(SavedSkipCards)
}
fn saved_klondike_pile_strategy() -> impl Strategy<Value = SavedKlondikePile> {
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<Value = SavedKlondikePileStack> {
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<Value = SavedInstruction> {
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)));
}
}