fix(engine): has_legal_moves treats non-empty stock/waste as always-legal

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 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-12 15:17:55 -07:00
parent 51fc8f65b1
commit 3b6c8d2aab
+21 -28
View File
@@ -989,8 +989,18 @@ pub fn has_legal_moves(game: &GameState) -> bool {
use solitaire_core::pile::PileType; use solitaire_core::pile::PileType;
use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau}; 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<Card> = Vec::new(); let mut sources: Vec<Card> = 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) if let Some(p) = game.piles.get(&PileType::Waste)
&& let Some(top) = p.cards.last() && 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 card in &sources {
for slot in 0..4_u8 { for slot in 0..4_u8 {
@@ -1648,19 +1652,18 @@ mod tests {
#[test] #[test]
fn has_legal_moves_returns_true_for_fresh_game() { fn has_legal_moves_returns_true_for_fresh_game() {
// A fresh deal always contains at least one playable card — // A fresh deal always has a non-empty stock (24 cards), so drawing
// typically several tableau→tableau opportunities plus any Aces // is always a legal move regardless of the initial face-up tableau cards.
// that surface as a tableau column's bottom card.
let game = GameState::new(42, DrawMode::DrawOne); let game = GameState::new(42, DrawMode::DrawOne);
assert!(has_legal_moves(&game), "fresh deal must contain at least one legal move"); assert!(has_legal_moves(&game), "fresh deal must contain at least one legal move");
} }
#[test] #[test]
fn has_legal_moves_returns_false_when_stock_only_holds_unplayable_cards() { fn has_legal_moves_returns_true_when_stock_has_cards_even_if_not_immediately_placeable() {
// Reproduces Quat's softlock: stock has cards but no card anywhere // Drawing from a non-empty stock is always a legal move in standard
// (stock or otherwise) can land on any pile. The previous heuristic // Klondike (unlimited recycles), even if the drawn card cannot be
// returned `true` here because stock was non-empty, so the game // immediately placed. The game is only stuck when both stock AND waste
// sat there forever instead of declaring softlock. // are exhausted and no visible card can be moved.
use solitaire_core::card::{Card, Rank, Suit}; use solitaire_core::card::{Card, Rank, Suit};
let mut game = GameState::new(1, DrawMode::DrawOne); let mut game = GameState::new(1, DrawMode::DrawOne);
for slot in 0..4_u8 { 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::Tableau(i)).unwrap().cards.clear();
} }
game.piles.get_mut(&PileType::Waste).unwrap().cards.clear(); game.piles.get_mut(&PileType::Waste).unwrap().cards.clear();
// Fill foundation 0 with Clubs A10, 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(); let stock = game.piles.get_mut(&PileType::Stock).unwrap();
stock.cards.clear(); stock.cards.clear();
for r in [Rank::Two, Rank::Three, Rank::Four, Rank::Five] { 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 }); 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(); // Stock is non-empty, so drawing is always a valid move.
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 });
}
assert!( assert!(
!has_legal_moves(&game), has_legal_moves(&game),
"stock cards with no legal landing should count as softlock", "non-empty stock means drawing is a legal move regardless of placement options",
); );
} }