fix(core): save files store the deal via upstream serializers (schema v6) #171

Merged
funman300 merged 1 commits from fix/save-schema-v6-session-recording into master 2026-07-10 18:12:28 +00:00
2 changed files with 155 additions and 46 deletions
+145 -41
View File
@@ -22,11 +22,17 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
/// 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`).
/// - v5 (current): `score`, `undo_count`, and `recycle_count` are no longer
/// - v5: `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
/// still carry those keys load fine — the extra fields are ignored.
pub const GAME_STATE_SCHEMA_VERSION: u32 = 5;
/// - v6 (current): `saved_moves` replaced by `recording`, the upstream
/// `card_game` session serialisation carrying the dealt board explicitly.
/// Loading no longer re-deals from `seed`, so in-progress saves survive
/// RNG/upstream upgrades that change the seed→deal mapping (the failure
/// class that PR #170 fixed for replays). v4/v5 files still load through
/// the legacy seed path and are rewritten as v6 on next save.
pub const GAME_STATE_SCHEMA_VERSION: u32 = 6;
/// Default move budget for a solvability check. Matches the winnable-deal retry
/// loop in the engine.
@@ -95,8 +101,8 @@ pub enum GameMode {
Difficulty(DifficultyLevel),
}
/// Output struct for schema v4 serialisation. `saved_moves` uses upstream
/// `KlondikeInstruction` serde, which produces named enum variants.
/// Output struct for schema v6 serialisation. `recording` is the upstream
/// session serialisation (config + dealt board + instruction list).
#[derive(Debug, Clone, Serialize)]
struct PersistedGameState {
pub draw_mode: DrawStockConfig,
@@ -105,15 +111,16 @@ struct PersistedGameState {
pub seed: u64,
pub take_from_foundation: bool,
pub schema_version: u32,
pub saved_moves: Vec<KlondikeInstruction>,
pub recording: SessionRecording,
}
/// Input struct that accepts schema v4 and v5 `saved_moves` formats.
/// Input struct that accepts schema v4, v5, and v6 save formats.
///
/// `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
/// v6 files carry `recording` (the upstream session serialisation, deal
/// included); v4/v5 files carry `saved_moves` and rebuild the deal from
/// `seed`. `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 {
@@ -126,7 +133,12 @@ struct PersistedGameStateIn {
pub take_from_foundation: bool,
#[serde(default = "schema_v1")]
pub schema_version: u32,
/// v4/v5 only. Replayed against a seed-dealt board.
#[serde(default)]
pub saved_moves: Vec<KlondikeInstruction>,
/// v6 only. Carries the dealt board explicitly.
#[serde(default)]
pub recording: Option<SessionRecording>,
}
#[cfg(feature = "test-support")]
@@ -222,7 +234,7 @@ impl Serialize for GameState {
seed: self.seed,
take_from_foundation: self.take_from_foundation,
schema_version: GAME_STATE_SCHEMA_VERSION,
saved_moves: self.saved_moves(),
recording: self.recording(),
}
.serialize(serializer)
}
@@ -232,34 +244,51 @@ impl<'de> Deserialize<'de> for GameState {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let persisted = PersistedGameStateIn::deserialize(deserializer)?;
// 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 {
4 | 5 => {}
// Accept v6 (current, recording-based), plus v4/v5 (legacy seed-dealt
// saves — loadable while the seed→deal mapping they were written
// under still holds; rewritten as v6 on the next save). v3 (legacy
// u8-index format) and all others are rejected.
let (mut game, instructions) = match persisted.schema_version {
6 => {
let Some(recording) = persisted.recording else {
return Err(serde::de::Error::custom(
"v6 save file is missing the session recording",
));
};
// The dealt board and session config come from the recording
// itself; `seed` is presentation metadata only.
Self::from_recording(&recording, persisted.seed, persisted.mode)
}
4 | 5 => (
Self {
mode: persisted.mode,
elapsed_seconds: 0,
seed: persisted.seed,
take_from_foundation: true,
session: Self::new_session(persisted.seed, persisted.draw_mode),
#[cfg(feature = "test-support")]
test_pile_state: None,
},
persisted.saved_moves,
),
v => {
return Err(serde::de::Error::custom(format!(
"unsupported GameState schema version {v}"
)));
}
}
let mut game = Self {
mode: persisted.mode,
elapsed_seconds: persisted.elapsed_seconds,
seed: persisted.seed,
take_from_foundation: persisted.take_from_foundation,
session: Self::new_session(persisted.seed, persisted.draw_mode),
#[cfg(feature = "test-support")]
test_pile_state: None,
};
game.mode = persisted.mode;
game.elapsed_seconds = persisted.elapsed_seconds;
game.take_from_foundation = persisted.take_from_foundation;
// Replay the saved instruction history. The upstream session tracks
// score components and recycle_count as it processes each move, so the
// derived stats are correct once replay completes. `undo_count()` resets
// to 0 across save/load because undone moves are not part of the saved
// forward history.
// Replay the saved instruction history with validation — a tampered
// or corrupt file must surface an error, not a silently wrong board.
// The upstream session tracks score components and recycle_count as
// it processes each move, so the derived stats are correct once
// replay completes. `undo_count()` resets 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 instruction in persisted.saved_moves {
for instruction in instructions {
if !game
.session
.state()
@@ -524,16 +553,6 @@ impl GameState {
KlondikeAdapter::config_for(self.draw_mode(), self.take_from_foundation)
}
/// Collects the session instruction history as upstream types for schema v4
/// serialisation.
fn saved_moves(&self) -> Vec<KlondikeInstruction> {
self.session
.history()
.iter()
.map(|snapshot| *snapshot.instruction())
.collect()
}
/// Returns the deterministic instruction history for the current deal as
/// upstream [`KlondikeInstruction`] values.
///
@@ -1697,6 +1716,91 @@ mod tests {
assert_eq!(replayed.move_count(), game.move_count());
}
/// v6 save files carry the deal in the recording; the `seed` field is
/// presentation metadata. Corrupting it must not change the loaded board.
#[test]
fn v6_save_load_ignores_seed_for_dealing() {
let game = game_with_some_moves(51);
let mut value = serde_json::to_value(&game).expect("serialise");
assert_eq!(value["schema_version"], 6);
value["seed"] = serde_json::Value::from(0xDEAD_BEEF_u64);
let loaded: GameState =
serde_json::from_value(value).expect("v6 save with wrong seed must still load");
assert_eq!(loaded.stock_cards(), game.stock_cards());
assert_eq!(loaded.waste_cards(), game.waste_cards());
for index in 0..7 {
let t = GameState::tableau_from_index(index).expect("tableau index");
assert_eq!(
loaded.pile(KlondikePile::Tableau(t)),
game.pile(KlondikePile::Tableau(t)),
"tableau {index} differs"
);
}
assert_eq!(loaded.move_count(), game.move_count());
assert_eq!(loaded.score(), game.score());
}
/// Legacy v5 files (seed + saved_moves, no recording) must still load
/// through the seed-dealt path until players resave as v6.
#[test]
fn v5_legacy_save_still_loads() {
let game = game_with_some_moves(145);
let v5 = serde_json::json!({
"draw_mode": game.draw_mode(),
"mode": game.mode,
"elapsed_seconds": 12,
"seed": game.seed,
"take_from_foundation": true,
"schema_version": 5,
"saved_moves": game.instruction_history(),
});
let loaded: GameState =
serde_json::from_value(v5).expect("v5 save must load via the legacy seed path");
assert_eq!(loaded.move_count(), game.move_count());
assert_eq!(loaded.stock_cards(), game.stock_cards());
assert_eq!(loaded.elapsed_seconds, 12);
}
/// A v6 file without the recording payload is malformed and must be
/// rejected with an error rather than silently re-dealt from the seed.
#[test]
fn v6_save_without_recording_is_rejected() {
let game = GameState::new(7, DrawStockConfig::DrawOne);
let mut value = serde_json::to_value(&game).expect("serialise");
value.as_object_mut().expect("object").remove("recording");
let err = serde_json::from_value::<GameState>(value)
.expect_err("v6 save without recording must fail");
assert!(err.to_string().contains("recording"), "got: {err}");
}
/// A recording whose instruction list is invalid for its own deal must
/// be rejected on load — tampered or corrupt files surface an error,
/// never a silently wrong board.
#[test]
fn v6_save_with_invalid_history_is_rejected() {
use klondike::{DstFoundation, Foundation, KlondikePile};
// A foundation move from an empty tableau is never legal on a fresh
// deal's first instruction slot for this source shape.
let bogus = KlondikeInstruction::DstFoundation(DstFoundation {
src: KlondikePile::Foundation(Foundation::Foundation1),
foundation: Foundation::Foundation2,
});
let recording =
SessionRecording::from_instructions_unchecked(7, DrawStockConfig::DrawOne, [bogus]);
let v6 = serde_json::json!({
"draw_mode": DrawStockConfig::DrawOne,
"mode": GameMode::Classic,
"elapsed_seconds": 0,
"seed": 7,
"take_from_foundation": true,
"schema_version": 6,
"recording": recording,
});
let err =
serde_json::from_value::<GameState>(v6).expect_err("invalid history must be rejected");
assert!(err.to_string().contains("invalid"), "got: {err}");
}
#[test]
fn from_recording_of_fresh_deal_returns_empty_instructions() {
let game = GameState::new(7, DrawStockConfig::DrawThree);
+10 -5
View File
@@ -477,7 +477,7 @@ mod tests {
/// again must reproduce byte-identical JSON. `undo_count` deliberately resets
/// to 0 on load because only the forward instruction history is persisted.
#[test]
fn game_state_v5_mid_game_round_trip() {
fn game_state_v6_mid_game_round_trip() {
use solitaire_core::KlondikeInstruction;
use solitaire_core::game_state::GameState;
@@ -518,11 +518,16 @@ mod tests {
save_game_state_to(&path, &gs).expect("save");
// Verify the file carries the v5 schema marker.
// Verify the file carries the v6 schema marker and the recording.
let json = fs::read_to_string(&path).expect("read json");
let parsed: serde_json::Value = serde_json::from_str(&json).expect("parse saved json");
assert_eq!(
parsed["schema_version"], 6,
"saved file must use schema version 6",
);
assert!(
json.contains("\"schema_version\"") && json.contains('5'),
"saved file must use schema version 5",
parsed["recording"].is_object(),
"saved file must embed the session recording",
);
let loaded =
@@ -530,7 +535,7 @@ mod tests {
// The forward instruction history round-trips, so the reconstructed board
// re-serialises to byte-identical JSON.
let path_reload = gs_path("v5_mid_game_reload");
let path_reload = gs_path("v6_mid_game_reload");
let _ = fs::remove_file(&path_reload);
save_game_state_to(&path_reload, &loaded).expect("re-save loaded");
assert_eq!(