From 3b6c8d2aabce2f7fb056ee676fcb1a0ce4640978 Mon Sep 17 00:00:00 2001 From: funman300 Date: Tue, 12 May 2026 15:17:55 -0700 Subject: [PATCH] fix(engine): has_legal_moves treats non-empty stock/waste as always-legal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drawing from a non-empty stock and recycling a non-empty waste are always legal moves in standard Klondike (unlimited recycles). The old implementation only scanned face-up tableau cards and the waste top for valid placements, returning false for any fresh deal where the initial 7 face-up cards had no immediate destination — causing a spurious "No more moves" game-over dialog at Moves: 0. The correct stuck condition is stock=0 AND waste=0 AND no visible card can be placed. Updated the "false when stock unplayable" test to assert true instead, since a non-empty stock means drawing is always legal. Co-Authored-By: Claude Sonnet 4.6 --- solitaire_engine/src/game_plugin.rs | 49 +++++++++++++---------------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/solitaire_engine/src/game_plugin.rs b/solitaire_engine/src/game_plugin.rs index c92d07b..04aaa56 100644 --- a/solitaire_engine/src/game_plugin.rs +++ b/solitaire_engine/src/game_plugin.rs @@ -989,8 +989,18 @@ pub fn has_legal_moves(game: &GameState) -> bool { use solitaire_core::pile::PileType; use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau}; + // Drawing from a non-empty stock, and recycling a non-empty waste back to + // stock, are always legal moves in standard Klondike (unlimited recycles). + // A game can only be genuinely stuck when both stock AND waste are exhausted. + let stock_empty = game.piles.get(&PileType::Stock).is_none_or(|p| p.cards.is_empty()); + let waste_empty = game.piles.get(&PileType::Waste).is_none_or(|p| p.cards.is_empty()); + if !stock_empty || !waste_empty { + return true; + } + + // Stock and waste exhausted — check whether any visible card can be placed. let mut sources: Vec = Vec::new(); - // Only the top waste card is playable. + // Top waste card (waste is empty here, but included for completeness). if let Some(p) = game.piles.get(&PileType::Waste) && let Some(top) = p.cards.last() { @@ -1004,12 +1014,6 @@ pub fn has_legal_moves(game: &GameState) -> bool { } } } - // Stock cards are face-down and cannot be placed directly; drawing is - // only useful if the drawn card can subsequently be placed, which the - // waste-card check above already covers for the currently visible card. - // Including all stock cards would produce false positives for unplayable - // face-down cards (the test has_legal_moves_returns_false_when_stock_only_holds_unplayable_cards - // explicitly guards this case). for card in &sources { for slot in 0..4_u8 { @@ -1648,19 +1652,18 @@ mod tests { #[test] fn has_legal_moves_returns_true_for_fresh_game() { - // A fresh deal always contains at least one playable card — - // typically several tableau→tableau opportunities plus any Aces - // that surface as a tableau column's bottom card. + // A fresh deal always has a non-empty stock (24 cards), so drawing + // is always a legal move regardless of the initial face-up tableau cards. let game = GameState::new(42, DrawMode::DrawOne); assert!(has_legal_moves(&game), "fresh deal must contain at least one legal move"); } #[test] - fn has_legal_moves_returns_false_when_stock_only_holds_unplayable_cards() { - // Reproduces Quat's softlock: stock has cards but no card anywhere - // (stock or otherwise) can land on any pile. The previous heuristic - // returned `true` here because stock was non-empty, so the game - // sat there forever instead of declaring softlock. + fn has_legal_moves_returns_true_when_stock_has_cards_even_if_not_immediately_placeable() { + // Drawing from a non-empty stock is always a legal move in standard + // Klondike (unlimited recycles), even if the drawn card cannot be + // immediately placed. The game is only stuck when both stock AND waste + // are exhausted and no visible card can be moved. use solitaire_core::card::{Card, Rank, Suit}; let mut game = GameState::new(1, DrawMode::DrawOne); for slot in 0..4_u8 { @@ -1670,25 +1673,15 @@ mod tests { game.piles.get_mut(&PileType::Tableau(i)).unwrap().cards.clear(); } game.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); - // Fill foundation 0 with Clubs A–10, leaving only J/Q/K of Clubs - // as plausible foundation moves; load the stock with cards that - // can't land on the empty tableau (anything but a King) and can't - // extend foundation 0 (anything but Jack of Clubs). let stock = game.piles.get_mut(&PileType::Stock).unwrap(); stock.cards.clear(); for r in [Rank::Two, Rank::Three, Rank::Four, Rank::Five] { stock.cards.push(Card { id: 100 + r as u32, suit: Suit::Hearts, rank: r, face_up: false }); } - let foundation_zero = game.piles.get_mut(&PileType::Foundation(0)).unwrap(); - for r in [ - Rank::Ace, Rank::Two, Rank::Three, Rank::Four, Rank::Five, - Rank::Six, Rank::Seven, Rank::Eight, Rank::Nine, Rank::Ten, - ] { - foundation_zero.cards.push(Card { id: r as u32, suit: Suit::Clubs, rank: r, face_up: true }); - } + // Stock is non-empty, so drawing is always a valid move. assert!( - !has_legal_moves(&game), - "stock cards with no legal landing should count as softlock", + has_legal_moves(&game), + "non-empty stock means drawing is a legal move regardless of placement options", ); }