diff --git a/solitaire_core/Cargo.toml b/solitaire_core/Cargo.toml index f44f2c0..555c1e8 100644 --- a/solitaire_core/Cargo.toml +++ b/solitaire_core/Cargo.toml @@ -8,6 +8,9 @@ edition.workspace = true default = [] test-support = [] +[dev-dependencies] +proptest = "1" + [dependencies] serde = { workspace = true } thiserror = { workspace = true } diff --git a/solitaire_core/src/game_state.rs b/solitaire_core/src/game_state.rs index f143e9f..3d7c749 100644 --- a/solitaire_core/src/game_state.rs +++ b/solitaire_core/src/game_state.rs @@ -874,6 +874,16 @@ impl GameState { pub fn compute_time_bonus(&self) -> i32 { scoring_time_bonus(self.elapsed_seconds) } + + /// Read-only access to the underlying [`card_game::Session`] for this deal. + /// + /// Exposes `session.history()` (deterministic replay) and `session.solve()` + /// (DFS solver) to crates outside `solitaire_core` without surfacing the + /// mutable field. Internal code that needs to mutate the session accesses + /// the `pub(crate)` field directly. + pub fn session(&self) -> &Session { + &self.session + } } #[cfg(test)] diff --git a/solitaire_core/src/klondike_adapter.rs b/solitaire_core/src/klondike_adapter.rs index 9ecf189..93502a0 100644 --- a/solitaire_core/src/klondike_adapter.rs +++ b/solitaire_core/src/klondike_adapter.rs @@ -1,19 +1,15 @@ //! Adapter bridging `solitaire_core` types to the upstream `klondike` crate. //! -//! # Current scope (integration steps 1–4) -//! //! [`KlondikeAdapter`] is a pure helper namespace for: //! - building [`KlondikeConfig`] from Ferrous settings //! - translating between local and upstream types //! - applying Ferrous-specific scoring policy on top of upstream defaults //! -//! # Not yet implemented -//! -//! - Live [`klondike::Klondike`] shadow state (requires pile-mapping, step 2). -//! - Move validation via klondike's rule engine (step 2). -//! - DFS solver via [`klondike::KlondikeState`] (step 6, now delegated to upstream). +//! All `From` / `TryFrom` conversions between `solitaire_core` product types and +//! upstream `card_game` / `klondike` types live here so that the product modules +//! (`card`, `pile`, etc.) remain free of upstream dependencies. -use card_game::{Card as KlCard, Rank as KlRank, Suit as KlSuit}; +use card_game::Card as KlCard; use klondike::{ DrawStockConfig, DstFoundation, DstTableau, Foundation, KlondikeConfig, KlondikeInstruction, KlondikePile, KlondikePileStack, MoveFromFoundationConfig, ScoringConfig, SkipCards, Tableau, @@ -21,6 +17,7 @@ use klondike::{ }; use serde::{Deserialize, Serialize}; +use crate::card; use crate::game_state::{DrawMode, GameMode}; /// Bridges `solitaire_core` game config and scoring to the upstream `klondike` crate. @@ -74,10 +71,10 @@ impl KlondikeAdapter { /// Score delta for undo: −15. /// - /// [`card_game::Session`] handles this via `SessionConfig::undo_penalty` - /// (default −15). We mirror the constant here so `GameState` can apply it - /// in its snapshot-based undo path without owning a `Session`. - pub const fn score_for_undo() -> i32 { + /// This is a Ferrous product policy — `card_game::SessionConfig::undo_penalty` + /// defaults to 0; the solver overrides it to 0 explicitly. The −15 WXP penalty + /// is applied here by `GameState` on every undo. + pub fn score_for_undo() -> i32 { -15 } @@ -161,6 +158,37 @@ impl KlondikeAdapter { // ── Type-conversion utilities ───────────────────────────────────────────── +impl From for card::Suit { + fn from(s: card_game::Suit) -> Self { + match s { + card_game::Suit::Clubs => Self::Clubs, + card_game::Suit::Diamonds => Self::Diamonds, + card_game::Suit::Hearts => Self::Hearts, + card_game::Suit::Spades => Self::Spades, + } + } +} + +impl From for card::Rank { + fn from(r: card_game::Rank) -> Self { + match r { + card_game::Rank::Ace => Self::Ace, + card_game::Rank::Two => Self::Two, + card_game::Rank::Three => Self::Three, + card_game::Rank::Four => Self::Four, + card_game::Rank::Five => Self::Five, + card_game::Rank::Six => Self::Six, + card_game::Rank::Seven => Self::Seven, + card_game::Rank::Eight => Self::Eight, + card_game::Rank::Nine => Self::Nine, + card_game::Rank::Ten => Self::Ten, + card_game::Rank::Jack => Self::Jack, + card_game::Rank::Queen => Self::Queen, + card_game::Rank::King => Self::King, + } + } +} + /// Convert a zero-based tableau index (0..=6) into [`Tableau`]. pub fn tableau_from_index(index: usize) -> Option { match index { @@ -206,37 +234,21 @@ pub fn skip_cards_from_count(skip: usize) -> Option { } } -/// Convert [`card_game::Suit`] back to our [`crate::card::Suit`]. -pub(crate) fn suit_from_kl(suit: KlSuit) -> crate::card::Suit { - match suit { - KlSuit::Clubs => crate::card::Suit::Clubs, - KlSuit::Diamonds => crate::card::Suit::Diamonds, - KlSuit::Hearts => crate::card::Suit::Hearts, - KlSuit::Spades => crate::card::Suit::Spades, - } -} - -/// Convert [`card_game::Rank`] back to our [`crate::card::Rank`]. -pub(crate) fn rank_from_kl(rank: KlRank) -> crate::card::Rank { - crate::card::Rank::RANKS - .into_iter() - .find(|r| r.value() == rank as u8) - .expect("KlRank 1-13 always maps to a valid Rank") -} - -/// Convert a [`card_game::Card`] back to our [`crate::card::Card`], assigning -/// a stable `id` derived from the suit and rank (0–51, Clubs-first ordering). +/// Convert a [`card_game::Card`] to a [`card::Card`], assigning a stable `id` +/// derived from suit and rank (0–51, Clubs-first ordering). /// /// The id is consistent for the same logical card across all reconstructions. -pub fn card_from_kl(card: &KlCard) -> crate::card::Card { - let suit = suit_from_kl(card.suit()); - let rank = rank_from_kl(card.rank()); - let suit_index = crate::card::Suit::SUITS - .iter() - .position(|s| *s == suit) - .expect("suit always in SUITS") as u32; +pub fn card_from_kl(kl_card: &KlCard) -> card::Card { + let suit: card::Suit = kl_card.suit().into(); + let rank: card::Rank = kl_card.rank().into(); + let suit_index = match suit { + card::Suit::Clubs => 0, + card::Suit::Diamonds => 1, + card::Suit::Hearts => 2, + card::Suit::Spades => 3, + }; let id = suit_index * 13 + (rank.value() as u32 - 1); - crate::card::Card { + card::Card { id, suit, rank, diff --git a/solitaire_core/src/lib.rs b/solitaire_core/src/lib.rs index 2d3755c..353496f 100644 --- a/solitaire_core/src/lib.rs +++ b/solitaire_core/src/lib.rs @@ -5,3 +5,11 @@ pub mod game_state; pub mod klondike_adapter; pub mod pile; pub mod solver; + +// Re-export upstream types that cross the solitaire_core API boundary so +// callers can import from one place without a direct `klondike` / `card_game` dep. +pub use card_game::Session; +pub use klondike::{Foundation, Klondike, KlondikePile, Tableau}; + +#[cfg(test)] +mod proptest_tests; diff --git a/solitaire_core/src/pile.rs b/solitaire_core/src/pile.rs index e18ebf6..52bdf3e 100644 --- a/solitaire_core/src/pile.rs +++ b/solitaire_core/src/pile.rs @@ -1,12 +1,19 @@ use crate::card::{Card, Suit}; use klondike::KlondikePile; -/// A named collection of cards in a specific board position. +/// Read-only projection of a single Klondike pile, rebuilt from [`GameState`] on every sync. +/// +/// `Pile` is a **data-transfer type**, not a game-state owner. Only the engine's +/// sync system may populate `cards`; no game logic should mutate this struct directly. +/// [`GameState`] is always the authoritative source of truth. +/// +/// [`GameState`]: crate::game_state::GameState #[derive(Debug, Clone, PartialEq, Eq)] pub struct Pile { /// Which logical Klondike pile this is. pub pile_type: KlondikePile, /// Cards in the pile, bottom-to-top stacking order. Last element is the top card. + /// Populated by the sync system; do not mutate from game-logic code. pub cards: Vec, } diff --git a/solitaire_core/src/proptest_tests.rs b/solitaire_core/src/proptest_tests.rs new file mode 100644 index 0000000..31c9197 --- /dev/null +++ b/solitaire_core/src/proptest_tests.rs @@ -0,0 +1,224 @@ +use klondike::{Foundation, KlondikePile, Tableau}; +use proptest::prelude::*; + +use crate::game_state::{DrawMode, GameState}; + +// --------------------------------------------------------------------------- +// Shared helpers +// --------------------------------------------------------------------------- + +/// Collect all card IDs across every pile in a fixed traversal order: +/// stock → waste → foundations 1–4 → tableaux 1–7. +/// +/// The order is deterministic for a given game state, so two calls on +/// equivalent states produce identical Vec outputs — the right fingerprint +/// for undo-reversibility checks. +fn all_card_ids(game: &GameState) -> Vec { + let foundations = [ + Foundation::Foundation1, + Foundation::Foundation2, + Foundation::Foundation3, + Foundation::Foundation4, + ]; + let tableaux = [ + Tableau::Tableau1, + Tableau::Tableau2, + Tableau::Tableau3, + Tableau::Tableau4, + Tableau::Tableau5, + Tableau::Tableau6, + Tableau::Tableau7, + ]; + + let mut ids: Vec = game.stock_cards().iter().map(|c| c.id).collect(); + ids.extend(game.waste_cards().iter().map(|c| c.id)); + for f in &foundations { + ids.extend( + game.pile(KlondikePile::Foundation(*f)) + .iter() + .map(|c| c.id), + ); + } + for t in &tableaux { + ids.extend(game.pile(KlondikePile::Tableau(*t)).iter().map(|c| c.id)); + } + ids +} + +fn draw_mode_strategy() -> impl Strategy { + prop_oneof![Just(DrawMode::DrawOne), Just(DrawMode::DrawThree)] +} + +/// Apply a sequence of random actions to a game, silently ignoring errors. +/// +/// Each action is `(draw_flag, move_index)`: +/// - `draw_flag = true` → call `game.draw()` +/// - `draw_flag = false` → pick the `move_index % len`th legal move from +/// `possible_instructions()` and execute it. +/// +/// `possible_instructions()` may return `(Stock, Stock, 1)` for the +/// RotateStock / draw action. `move_cards(Stock, Stock, 1)` is rejected by +/// the `from == to` guard, so those are dispatched to `game.draw()`. +fn apply_random_actions(game: &mut GameState, actions: &[(bool, usize)]) { + for &(do_draw, idx) in actions { + if do_draw { + let _ = game.draw(); + } else { + let instructions = game.possible_instructions(); + if instructions.is_empty() { + continue; + } + let (from, to, count) = instructions[idx % instructions.len()]; + if from == to { + let _ = game.draw(); + } else { + let _ = game.move_cards(from, to, count); + } + } + } +} + +/// Apply one move from `possible_instructions()` (or a draw if no move is +/// available), using `move_idx` to select among the legal options. +/// Returns `true` when a move was successfully applied. +fn apply_one_move(game: &mut GameState, move_idx: usize) -> bool { + if game.is_won { + return false; + } + let instructions = game.possible_instructions(); + if instructions.is_empty() { + return game.draw().is_ok(); + } + let (from, to, count) = instructions[move_idx % instructions.len()]; + if from == to { + game.draw().is_ok() + } else { + game.move_cards(from, to, count).is_ok() + } +} + +// --------------------------------------------------------------------------- +// Properties +// --------------------------------------------------------------------------- + +proptest! { + /// All 52 card IDs must be present exactly once across every pile after + /// any reachable sequence of draw + move_cards actions. + /// + /// Catches two bug classes at once: + /// - Card loss (fewer than 52 unique IDs after the sequence). + /// - Card duplication (52 total but deduplication reduces the set). + #[test] + fn all_52_cards_always_present( + seed in any::(), + draw_mode in draw_mode_strategy(), + actions in prop::collection::vec((any::(), 0usize..200), 0..30), + ) { + let mut game = GameState::new(seed, draw_mode); + apply_random_actions(&mut game, &actions); + + let mut ids = all_card_ids(&game); + prop_assert_eq!(ids.len(), 52, "card count ≠ 52 (got {})", ids.len()); + ids.sort_unstable(); + ids.dedup(); + prop_assert_eq!( + ids.len(), 52, + "duplicate card IDs found after dedup — a card was cloned" + ); + } + + /// `GameState::new(seed, draw_mode)` must be deterministic: two calls + /// with the same arguments must produce identical initial pile layouts. + /// + /// Pins that the deal is seeded from `seed` alone and not from any + /// implicit source like wall-clock time or global state. + #[test] + fn deal_is_deterministic( + seed in any::(), + draw_mode in draw_mode_strategy(), + ) { + let a = GameState::new(seed, draw_mode); + let b = GameState::new(seed, draw_mode); + prop_assert_eq!( + all_card_ids(&a), + all_card_ids(&b), + "same seed + draw_mode produced different deals", + ); + } + + /// After applying any single legal move and immediately undoing it, the + /// pile layout and move_count must be identical to their pre-move values. + /// + /// `setup_actions` drives the game to an arbitrary mid-game position; + /// `move_idx` selects which legal move to apply and then undo. + /// + /// The score is intentionally excluded: `undo()` applies a −15 penalty + /// that is by design, not a regression. + #[test] + fn undo_restores_pile_layout_and_move_count( + seed in any::(), + draw_mode in draw_mode_strategy(), + setup_actions in prop::collection::vec((any::(), 0usize..200), 0..20), + move_idx in 0usize..200, + ) { + let mut game = GameState::new(seed, draw_mode); + apply_random_actions(&mut game, &setup_actions); + + // Snapshot the state before the move. + let before_ids = all_card_ids(&game); + let before_move_count = game.move_count; + + // Apply one move. + if !apply_one_move(&mut game, move_idx) || game.is_won { + return Ok(()); // nothing to undo + } + + // Undo and verify. + prop_assert!( + game.undo().is_ok(), + "undo must succeed immediately after a successful move", + ); + prop_assert_eq!( + all_card_ids(&game), + before_ids, + "pile layout after undo differs from the pre-move snapshot", + ); + prop_assert_eq!( + game.move_count, + before_move_count, + "move_count after undo must equal the pre-move value", + ); + } + + /// Every move returned by `possible_instructions()` must succeed when + /// applied via `move_cards()`. + /// + /// `possible_instructions()` and `move_cards()` both validate moves + /// through the same upstream rule engine. This property ensures no + /// drift has opened up between what the engine reports as legal and + /// what it actually accepts. + #[test] + fn legal_moves_always_succeed( + seed in any::(), + draw_mode in draw_mode_strategy(), + setup_actions in prop::collection::vec((any::(), 0usize..200), 0..20), + ) { + let mut game = GameState::new(seed, draw_mode); + apply_random_actions(&mut game, &setup_actions); + + for (from, to, count) in game.possible_instructions() { + // Clone so each move is tried from the same starting state. + let mut trial = game.clone(); + let result = if from == to { + trial.draw() + } else { + trial.move_cards(from, to, count) + }; + prop_assert!( + result.is_ok(), + "possible_instructions() reported ({from:?} → {to:?} ×{count}) \ + as legal but the call returned Err: {result:?}", + ); + } + } +} diff --git a/solitaire_core/src/solver.rs b/solitaire_core/src/solver.rs index 5691eb2..ef0f36f 100644 --- a/solitaire_core/src/solver.rs +++ b/solitaire_core/src/solver.rs @@ -84,13 +84,7 @@ pub fn try_solve_from_state(state: &GameState, config: &SolverConfig) -> SolveOu } fn solve_game_state(initial: &GameState, config: &SolverConfig) -> SolveOutcome { - // Keep solver latency bounded even when callers pass very large budgets. - // This preserves responsiveness for async engine paths and keeps - // "winnable-only" seed search from stalling on pathological states. - let effective_state_budget = config.state_budget.min(5_000); - let effective_move_budget = config.move_budget.min(5_000); - - if effective_state_budget == 0 { + if config.state_budget == 0 { return SolveOutcome { result: SolverResult::Inconclusive, first_move: None, @@ -109,10 +103,10 @@ fn solve_game_state(initial: &GameState, config: &SolverConfig) -> SolveOutcome let solver_config = SessionConfig { inner: KlondikeAdapter::config_for(initial.draw_mode, initial.take_from_foundation), undo_penalty: 0, - solve_moves_budget: effective_move_budget, - solve_states_budget: effective_state_budget as u64, + solve_moves_budget: config.move_budget, + solve_states_budget: config.state_budget as u64, }; - let solver_session = Session::new(initial.session.state().state().clone(), solver_config); + let solver_session = Session::new(initial.session().state().state().clone(), solver_config); match solver_session.solve() { Ok(Some(solution)) => { @@ -245,4 +239,44 @@ mod tests { assert_eq!(outcome.result, SolverResult::Inconclusive); assert!(outcome.first_move.is_none()); } + + #[test] + fn budget_is_passed_through_not_clamped() { + // 0xD1FF_0000_0000_0012 is a Medium-tier catalog seed: Inconclusive at + // the Easy budget (1 000 states) but Winnable at Medium (5 000 states). + // Differing results confirm solve_game_state passes the caller's + // state_budget unchanged to the underlying solver. + let easy = SolverConfig { move_budget: 1_000, state_budget: 1_000 }; + let medium = SolverConfig { move_budget: 5_000, state_budget: 5_000 }; + assert_eq!( + try_solve(0xD1FF_0000_0000_0012, DrawMode::DrawOne, &easy), + SolverResult::Inconclusive, + ); + assert_eq!( + try_solve(0xD1FF_0000_0000_0012, DrawMode::DrawOne, &medium), + SolverResult::Winnable, + ); + } + + #[test] + fn budget_above_five_thousand_is_not_clamped() { + // 0xD1FF_0000_0000_00DE is a hard catalog seed: Inconclusive at 5 000 + // states but Winnable at 50 000. Before this fix, solve_game_state + // applied `config.state_budget.min(5_000)` internally, so a 50k config + // was silently reduced to 5k — making both calls return Inconclusive and + // preventing the generator from certifying Hard/Expert/Grandmaster seeds. + // This assertion fails if the cap is re-introduced. + let below_cap = SolverConfig { move_budget: 5_000, state_budget: 5_000 }; + let above_cap = SolverConfig { move_budget: 50_000, state_budget: 50_000 }; + assert_eq!( + try_solve(0xD1FF_0000_0000_00DE, DrawMode::DrawOne, &below_cap), + SolverResult::Inconclusive, + "seed must be Inconclusive at 5 000 states", + ); + assert_eq!( + try_solve(0xD1FF_0000_0000_00DE, DrawMode::DrawOne, &above_cap), + SolverResult::Winnable, + "seed must be Winnable at 50 000 states — re-introducing the 5k cap would break this", + ); + } }