funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 4 Pull Requests 1 Actions Packages Projects Releases 29 Wiki Activity

refactor(core): derive score/undo/recycle from upstream session stats

Browse Source 
Replace the bespoke WXP scoring engine with the upstream
card_game/klondike session stats, eliminating duplicated state that
could drift from the single source of truth.

score()/undo_count()/recycle_count() now read session.stats(); the -15
undo penalty is configured as SessionConfig::undo_penalty and applied by
the upstream score formula. Save schema bumped v4 -> v5 (the three
counters are no longer persisted -- they are rebuilt by replaying the
forward instruction history on load).

- Remove GameState fields score, undo_count, recycle_count (#87)
- Remove score_history / is_recycle_history undo journal (#86)
- Remove KlondikeAdapter::apply_undo_score and the score_for_* helpers,
  plus pre_instruction_score_delta / will_flip_tableau_source (#84)

These three issues are a single atomic change: each removed field/helper
is consumed by the same draw/apply_instruction/undo/serde/PartialEq
paths, so they cannot compile or pass tests in isolation.

Behaviour changes (intentional): the escalating recycle penalty and
per-step score floor are gone (upstream linear scoring, floored once at
0); recycle_count is now cumulative; undo_count resets across save/load.

Refs #84, #86, #87

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-06-11 10:36:31 -07:00
parent 9e3c6b06b0
commit 372b6423d8
10 changed files with 237 additions and 381 deletions
Download Patch File Download Diff File Expand all files Collapse all files
solitaire_core/src/game_state.rs
+171 -223
View File
@@ -21,10 +21,14 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
/// - 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.
/// - v4 (current): `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.
pub const GAME_STATE_SCHEMA_VERSION: u32 = 4;
/// - 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.
/// - 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
/// still carry those keys load fine — the extra fields are ignored.
pub const GAME_STATE_SCHEMA_VERSION: u32 = 5;
/// Default value for `GameState::schema_version` when deserialising older
/// save files that pre-date the field.
@@ -83,11 +87,8 @@ pub enum GameMode {
struct PersistedGameState {
pub draw_mode: DrawMode,
pub mode: GameMode,
pub score: i32,
pub elapsed_seconds: u64,
pub seed: u64,
pub undo_count: u32,
pub recycle_count: u32,
pub take_from_foundation: bool,
pub schema_version: u32,
pub saved_moves: Vec<KlondikeInstruction>,
@@ -109,20 +110,19 @@ enum AnyInstruction {
V3(SavedInstruction),
}
/// Input struct that accepts both schema v3 and v4 `saved_moves` formats.
/// Input struct that accepts schema v3, v4, and v5 `saved_moves` formats.
///
/// `recycle_count` is intentionally absent: the value is rebuilt from the
/// instruction replay so that stale counts (from the pre-Phase-3 undo drift
/// bug) are corrected on load. Serde ignores the field in the JSON.
/// `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.
#[derive(Debug, Clone, Deserialize)]
struct PersistedGameStateIn {
pub draw_mode: DrawMode,
#[serde(default)]
pub mode: GameMode,
pub score: i32,
pub elapsed_seconds: u64,
pub seed: u64,
pub undo_count: u32,
#[serde(default)]
pub take_from_foundation: bool,
#[serde(default = "schema_v1")]
@@ -161,33 +161,25 @@ pub struct TestPileState {
}
/// Full state of an in-progress Klondike Solitaire game.
///
/// Score, undo count, and recycle count are **not** stored here. They are
/// derived on demand from the upstream `card_game`/`klondike` session stats via
/// [`GameState::score`], [`GameState::undo_count`], and
/// [`GameState::recycle_count`]. The session is the single source of truth; the
/// −15 undo penalty is configured on the session ([`Self::session_config`]) and
/// applied by the upstream score formula.
#[derive(Debug, Clone)]
pub struct GameState {
/// Top-level mode (Classic / Zen).
pub mode: GameMode,
/// Current game score. Can be negative (undo penalties subtract from score).
pub score: i32,
/// Seconds elapsed since the game started, used for time-bonus scoring.
pub elapsed_seconds: u64,
/// RNG seed used to deal this game. Same seed always produces the same layout.
pub seed: u64,
/// Number of times `undo()` has been successfully invoked this game.
pub undo_count: u32,
/// Number of times the waste pile has been recycled back to stock this game.
pub recycle_count: u32,
/// When `true`, the player may move the top card of a foundation pile back
/// onto a compatible tableau column.
pub take_from_foundation: bool,
pub(crate) session: Session<Klondike>,
/// Score recorded immediately before each instruction was applied.
/// Parallel to `session.history()` during live play; used by `undo()` to
/// correctly restore the pre-move score before applying the undo penalty.
/// Empty after a load (can't be reconstructed from history alone).
score_history: Vec<i32>,
/// Whether each entry in `session.history()` was a stock recycle.
/// Parallel to `session.history()`; rebuilt from replay on load so that
/// `undo()` correctly decrements `recycle_count` even across save/load cycles.
is_recycle_history: Vec<bool>,
#[cfg(feature = "test-support")]
/// Test pile overrides. Always `None` in production runtime code.
pub test_pile_state: Option<TestPileState>,
@@ -197,14 +189,14 @@ impl PartialEq for GameState {
fn eq(&self, other: &Self) -> bool {
self.draw_mode() == other.draw_mode()
&& self.mode == other.mode
&& self.score == other.score
&& self.score() == other.score()
&& self.move_count() == other.move_count()
&& self.elapsed_seconds == other.elapsed_seconds
&& self.seed == other.seed
&& self.is_won() == other.is_won()
&& self.is_auto_completable() == other.is_auto_completable()
&& self.undo_count == other.undo_count
&& self.recycle_count == other.recycle_count
&& self.undo_count() == other.undo_count()
&& self.recycle_count() == other.recycle_count()
&& self.take_from_foundation == other.take_from_foundation
&& self.stock_cards() == other.stock_cards()
&& self.waste_cards() == other.waste_cards()
@@ -227,11 +219,8 @@ impl Serialize for GameState {
PersistedGameState {
draw_mode: self.draw_mode(),
mode: self.mode,
score: self.score,
elapsed_seconds: self.elapsed_seconds,
seed: self.seed,
undo_count: self.undo_count,
recycle_count: self.recycle_count,
take_from_foundation: self.take_from_foundation,
schema_version: GAME_STATE_SCHEMA_VERSION,
saved_moves: self.saved_moves(),
@@ -244,10 +233,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) and v4 (current,
// upstream named-variant serde). Reject everything else.
// Accept v3 (legacy u8-index format, auto-migrated), v4 (upstream
// named-variant serde), and v5 (current, derived stats). Reject the rest.
match persisted.schema_version {
3 | 4 => {}
3..=5 => {}
v => {
return Err(serde::de::Error::custom(format!(
"unsupported GameState schema version {v}"
@@ -257,30 +246,22 @@ impl<'de> Deserialize<'de> for GameState {
let mut game = Self {
mode: persisted.mode,
score: persisted.score,
elapsed_seconds: persisted.elapsed_seconds,
seed: persisted.seed,
undo_count: persisted.undo_count,
// Rebuilt from the replay loop below; persisted value may be stale
// due to the pre-Phase-3 undo drift bug.
recycle_count: 0,
take_from_foundation: persisted.take_from_foundation,
session: Self::new_session(persisted.seed, persisted.draw_mode),
// score_history cannot be faithfully rebuilt from the instruction
// history because live-play undo penalties are not recorded in
// saved_moves. Leave empty; undo() falls back to old behaviour for
// any move made before this load (see undo() for details).
score_history: Vec::new(),
// is_recycle_history IS rebuilt: recycle detection only needs the
// pre-instruction session state, which is available during replay.
is_recycle_history: Vec::new(),
#[cfg(feature = "test-support")]
test_pile_state: None,
};
// 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.
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::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 {
@@ -290,12 +271,6 @@ impl<'de> Deserialize<'de> for GameState {
}
};
// Detect recycle BEFORE processing so that the pre-instruction
// session state (face-down stock) is still available.
let is_recycle = matches!(instruction, KlondikeInstruction::RotateStock)
&& game.stock_cards().is_empty()
&& !game.waste_cards().is_empty();
if !game
.session
.state()
@@ -307,11 +282,6 @@ impl<'de> Deserialize<'de> for GameState {
));
}
game.session.process_instruction(instruction);
game.is_recycle_history.push(is_recycle);
if is_recycle {
game.recycle_count = game.recycle_count.saturating_add(1);
}
}
Ok(game)
@@ -328,15 +298,10 @@ impl GameState {
pub fn new_with_mode(seed: u64, draw_mode: DrawMode, mode: GameMode) -> Self {
Self {
mode,
score: 0,
elapsed_seconds: 0,
seed,
undo_count: 0,
recycle_count: 0,
take_from_foundation: true,
session: Self::new_session(seed, draw_mode),
score_history: Vec::new(),
is_recycle_history: Vec::new(),
#[cfg(feature = "test-support")]
test_pile_state: None,
}
@@ -351,6 +316,43 @@ impl GameState {
}
}
/// Current game score, derived from the upstream session stats.
///
/// The upstream score is a linear sum of move-type counts (foundation/
/// tableau/flip deltas) plus `undos * undo_penalty` (−15 each). Floored at 0
/// so the displayed score is never negative. Returns 0 in [`GameMode::Zen`],
/// where scoring is suppressed entirely.
///
/// Note: the win-time bonus (`compute_time_bonus`) is layered on by the
/// engine's win-summary, not included here — this is the in-play base score.
pub fn score(&self) -> i32 {
if self.mode == GameMode::Zen {
return 0;
}
self.session
.state()
.score(self.session.stats(), self.session.config())
.max(0)
}
/// Number of times `undo()` has been successfully invoked this game, read
/// from the upstream session stats.
///
/// Resets to 0 across a save/load cycle: only the forward instruction
/// history is persisted, so undone moves leave no trace to replay.
pub fn undo_count(&self) -> u32 {
self.session.stats().undos()
}
/// Number of times the waste pile has been recycled back to stock this game,
/// read from the upstream session stats.
///
/// This is a **cumulative** count — the upstream stat is not rolled back when
/// a recycle is undone, so it reflects total recycles ever performed.
pub fn recycle_count(&self) -> u32 {
self.session.stats().stats().recycle_count()
}
/// Total moves made this game (draws, recycles, and card moves), derived
/// from the session's instruction history length.
pub fn move_count(&self) -> u32 {
@@ -394,7 +396,9 @@ impl GameState {
fn session_config(draw_mode: DrawMode) -> SessionConfig<KlondikeConfig> {
SessionConfig {
inner: Self::replay_config(draw_mode),
undo_penalty: 0,
// The −15 WXP undo penalty is now applied by the upstream score
// formula (`undos * undo_penalty`) rather than by hand in `undo()`.
undo_penalty: -15,
..SessionConfig::default()
}
}
@@ -601,6 +605,67 @@ impl GameState {
state.move_count = Some(move_count);
}
/// Test-support helper: perform `n` real undos so [`Self::undo_count`]
/// reports `n`. Each iteration draws a card then immediately undoes it,
/// leaving the board unchanged but advancing the upstream `undos` counter.
///
/// Since `score`/`undo_count`/`recycle_count` are now derived from the
/// session stats rather than stored fields, tests drive the real session to
/// reach a desired stat instead of assigning the value directly.
#[cfg(feature = "test-support")]
pub fn force_test_undos(&mut self, n: u32) {
for _ in 0..n {
if self.draw().is_ok() {
let _ = self.undo();
}
}
}
/// Test-support helper: perform `n` real stock recycles so
/// [`Self::recycle_count`] reports `n`. Draws until the stock empties, then
/// draws once more to recycle, repeated `n` times.
#[cfg(feature = "test-support")]
pub fn force_test_recycles(&mut self, n: u32) {
for _ in 0..n {
let mut guard = 0;
while !self.stock_cards().is_empty() && guard < 200 {
guard += 1;
if self.draw().is_err() {
break;
}
}
// Stock now empty (waste full) — this draw recycles waste → stock.
let _ = self.draw();
}
}
/// Test-support helper: drive real moves until [`Self::score`] reaches at
/// least `target`, returning the resulting score. Prefers foundation moves
/// (+10 each) and falls back to the solver-priority move, so a modest target
/// is reached within a handful of moves on a typical deal.
#[cfg(feature = "test-support")]
pub fn force_test_score(&mut self, target: i32) -> i32 {
let mut guard = 0;
while self.score() < target && !self.is_won() && guard < 4000 {
guard += 1;
let instructions = self.possible_instructions();
let next = instructions
.iter()
.copied()
.find(|i| matches!(i, KlondikeInstruction::DstFoundation(_)))
.or_else(|| instructions.into_iter().next());
match next {
Some(instruction) => {
if self.apply_instruction(instruction).is_err() {
break;
}
}
None => break,
}
}
self.score()
}
/// Test-support helper: override face-down stock cards returned by
/// [`Self::stock_cards`].
#[cfg(feature = "test-support")]
@@ -672,79 +737,6 @@ impl GameState {
.ok_or_else(|| MoveError::RuleViolation("invalid tableau card count".into()))
}
fn will_flip_tableau_source(&self, from: KlondikePile, count: usize) -> bool {
let KlondikePile::Tableau(_) = from else {
return false;
};
let pile = self.pile(from);
if pile.is_empty() {
return false;
}
pile.len() > count && !pile[pile.len() - count - 1].1
}
/// Returns `(score_delta, is_recycle)` for `instruction` given the *current*
/// game state. Must be called **before** the instruction is applied to the
/// session; the helper reads pre-instruction pile state from `self`.
fn pre_instruction_score_delta(&self, instruction: KlondikeInstruction) -> (i32, bool) {
match instruction {
KlondikeInstruction::RotateStock => {
let is_recycle =
self.stock_cards().is_empty() && !self.waste_cards().is_empty();
if is_recycle {
let next_count = self.recycle_count.saturating_add(1);
let penalty = KlondikeAdapter::score_for_recycle_with_mode(
next_count,
self.draw_mode() == DrawMode::DrawThree,
self.mode,
);
(penalty, true)
} else {
(0, false)
}
}
KlondikeInstruction::DstFoundation(dst_foundation) => {
let from = dst_foundation.src;
let to = KlondikePile::Foundation(dst_foundation.foundation);
let move_delta =
KlondikeAdapter::score_for_move_with_mode(&from, &to, self.mode);
// DstFoundation always moves exactly 1 card.
let flip_bonus = if self.will_flip_tableau_source(from, 1) {
KlondikeAdapter::score_for_flip_with_mode(self.mode)
} else {
0
};
(move_delta + flip_bonus, false)
}
KlondikeInstruction::DstTableau(dst_tableau) => {
let (from, count) = match dst_tableau.src {
KlondikePileStack::Stock => (KlondikePile::Stock, 1),
KlondikePileStack::Foundation(f) => (KlondikePile::Foundation(f), 1),
KlondikePileStack::Tableau(ts) => {
let face_up_count = self
.session
.state()
.state()
.state()
.tableau_face_up_cards(ts.tableau)
.len();
let count = face_up_count.saturating_sub(ts.skip_cards as usize);
(KlondikePile::Tableau(ts.tableau), count)
}
};
let to = KlondikePile::Tableau(dst_tableau.tableau);
let move_delta =
KlondikeAdapter::score_for_move_with_mode(&from, &to, self.mode);
let flip_bonus = if self.will_flip_tableau_source(from, count) {
KlondikeAdapter::score_for_flip_with_mode(self.mode)
} else {
0
};
(move_delta + flip_bonus, false)
}
}
}
fn instruction_for_move(
&self,
from: KlondikePile,
@@ -899,19 +891,10 @@ impl GameState {
return Err(MoveError::StockEmpty);
}
let (score_delta, is_recycle) =
self.pre_instruction_score_delta(KlondikeInstruction::RotateStock);
self.score_history.push(self.score);
self.is_recycle_history.push(is_recycle);
// The session tracks score components and recycle_count as it processes
// the instruction; no local bookkeeping required.
self.session
.process_instruction(KlondikeInstruction::RotateStock);
if is_recycle {
self.recycle_count = self.recycle_count.saturating_add(1);
}
self.score = (self.score + score_delta).max(0);
Ok(())
}
@@ -949,8 +932,9 @@ impl GameState {
/// instruction form — solver hints, auto-complete, replay, and the property
/// tests. User drag-and-drop enters through [`Self::move_cards`], which is a
/// thin adapter that converts pile coordinates to an instruction and
/// delegates here, so the move bookkeeping (rule validation, score history,
/// recycle accounting, undo snapshot) lives in exactly one place.
/// delegates here, so the move bookkeeping (rule validation, the undo
/// snapshot, and the session's score/recycle stats) lives in exactly one
/// place.
///
/// Returns [`MoveError::RuleViolation`] if the instruction is illegal in the
/// current position, or [`MoveError::GameAlreadyWon`] once the game is over.
@@ -972,21 +956,17 @@ impl GameState {
return Err(MoveError::RuleViolation("move violates rules".into()));
}
let (score_delta, is_recycle) = self.pre_instruction_score_delta(instruction);
self.score_history.push(self.score);
self.is_recycle_history.push(is_recycle);
// The session records the move snapshot and updates score/recycle stats.
self.session.process_instruction(instruction);
if is_recycle {
self.recycle_count = self.recycle_count.saturating_add(1);
}
self.score = (self.score + score_delta).max(0);
Ok(())
}
/// Restore the most recent undo snapshot and apply the undo score penalty (-15).
/// Restore the most recent undo snapshot.
///
/// The −15 undo penalty is applied by the upstream score formula
/// (`undos * undo_penalty`), and the session increments its `undos` counter,
/// so this method only has to delegate to [`Session::undo`] after the mode
/// guards. See [`Self::score`] / [`Self::undo_count`] for the derived values.
pub fn undo(&mut self) -> Result<(), MoveError> {
if self.is_won() {
return Err(MoveError::GameAlreadyWon);
@@ -1000,23 +980,7 @@ impl GameState {
return Err(MoveError::UndoStackEmpty);
}
// Pop the pre-instruction score for the move being undone. Falls back
// to self.score (= old behaviour) when score_history is empty, which
// happens for moves made before a save/load cycle because undo
// penalties aren't reflected in the saved instruction history.
let pre_move_score = self.score_history.pop().unwrap_or(self.score);
let was_recycle = self.is_recycle_history.pop().unwrap_or(false);
self.session.undo();
if was_recycle {
self.recycle_count = self.recycle_count.saturating_sub(1);
}
// Apply the undo penalty to the pre-move score, not the post-move score.
// This correctly reverses any recycle or move penalty that was applied
// before adding the −15 undo penalty.
self.score = KlondikeAdapter::apply_undo_score(pre_move_score, self.mode);
self.undo_count = self.undo_count.saturating_add(1);
Ok(())
}
@@ -1210,56 +1174,40 @@ mod tests {
}
#[test]
fn recycle_count_decrements_when_recycle_is_undone() {
fn recycle_count_is_cumulative_and_not_rolled_back_on_undo() {
// Upstream `KlondikeStats::recycle_count` counts every recycle ever
// performed; it is intentionally NOT decremented when a recycle is
// undone (the session restores the board but leaves the stat). This is
// the post-migration semantics: a cumulative count, not a net count.
let mut game = game_at_first_recycle().expect("could not reach recycle");
let count_after_recycle = game.recycle_count;
assert_eq!(count_after_recycle, 1, "first recycle should give count=1");
assert_eq!(game.recycle_count(), 1, "first recycle should give count=1");
game.undo().expect("undo should succeed");
assert_eq!(
game.recycle_count, 0,
"recycle_count must decrement back to 0 after undoing the recycle",
game.recycle_count(),
1,
"recycle_count is cumulative: undoing a recycle does not roll it back",
);
}
#[test]
fn score_recycle_penalty_is_reversed_on_undo() {
// Reach the second recycle (count=2, Draw-1) so there is a −100 penalty.
let mut game = game_at_first_recycle().expect("could not reach first recycle");
// Draw until stock is empty again so we can do a second recycle.
let mut second_recycle_done = false;
for _ in 0..200 {
if game.stock_cards().is_empty() && !game.waste_cards().is_empty() {
let score_before_second_recycle = game.score;
game.draw().expect("second recycle should succeed");
assert_eq!(game.recycle_count, 2);
// The second recycle in Draw-1 mode costs −100.
let expected_after = (score_before_second_recycle - 100).max(0);
assert_eq!(
game.score, expected_after,
"second Draw-1 recycle must apply −100 penalty",
);
// Undo: score should recover to (score_before_second_recycle − 15).max(0),
// NOT to (score_after_recycle − 15).max(0).
game.undo().expect("undo of second recycle should succeed");
let expected_after_undo = (score_before_second_recycle - 15).max(0);
assert_eq!(
game.score, expected_after_undo,
"undoing a penalised recycle must reverse the recycle penalty \
before applying the −15 undo penalty",
);
assert_eq!(
game.recycle_count, 1,
"recycle_count must also be decremented on undo",
);
second_recycle_done = true;
break;
}
let _ = game.draw();
fn undo_applies_minus_15_penalty_via_upstream_score() {
// A foundation move scores +10 upstream; undoing it nets the move score
// back to 0 and adds the −15 undo penalty, which `score()` floors at 0.
let mut game = GameState::new(1, DrawMode::DrawOne);
// Find and play any scoring move, then undo it.
let scoring_move = game
.possible_instructions()
.into_iter()
.find(|i| matches!(i, KlondikeInstruction::DstFoundation(_)));
if let Some(instruction) = scoring_move {
game.apply_instruction(instruction)
.expect("scoring move should apply");
assert!(game.score() > 0, "a foundation move should raise the score");
game.undo().expect("undo should succeed");
assert_eq!(game.undo_count(), 1, "undo increments the upstream counter");
// base score returns to 0, minus 15 undo penalty, floored at 0.
assert_eq!(game.score(), 0, "score floors at 0 after the undo penalty");
}
assert!(second_recycle_done, "could not reach second recycle in test");
}
#[test]