fix(engine): treat unplayable stock as softlock in has_legal_moves

Previous heuristic returned true whenever stock or waste held any
cards. Quat hit a state with 4 cards remaining and the stock kept
cycling but nothing was ever playable — the existing detection
counted "draw is available" as a legal move and the game just sat
there.

Replace the early return with a single pass over every card that
could ever be a move source: every Stock card, every Waste card, and
the face-up top of every Tableau column. For each, check whether it
can currently land on any Foundation or Tableau. Return true only if
*some* card anywhere can land *somewhere* — otherwise the player is
genuinely stuck no matter how many times they recycle the deck.

Tightened the existing fresh-game test name to reflect what it
actually proves (a fresh deal has playable moves, not "stock is
non-empty"). Added one new test reproducing Quat's exact case.

Reported by Quat.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-05 17:35:55 +00:00
parent 3eabc149a8
commit 271647265c
+80 -42
View File
@@ -510,58 +510,57 @@ fn handle_undo(
// Task #29 — No-moves detection // Task #29 — No-moves detection
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/// Returns `true` if the current game state has at least one legal move. /// Returns `true` if the current game state has at least one legal move
/// that could ever lead to progress.
/// ///
/// Considers: /// Considers a card "playable" if it's currently face-up on the top of
/// - Any non-empty Stock or Waste pile (draw / recycle is always available). /// the Waste or any Tableau, OR if it lives in the Stock or Waste pile
/// - Any face-up card on Waste or Tableau piles that can legally move to any /// at all (every card in those piles eventually rotates through the
/// Foundation or Tableau destination. /// Waste's top in both Draw-One and Draw-Three over the course of
/// recycling). For each such candidate, checks whether it can land on
/// any Foundation or any Tableau in the current state.
///
/// Returns `false` only when *no* card anywhere can land anywhere —
/// the player can keep drawing through the stock forever and nothing
/// will ever come of it. This treats "draw cycle with no useful drop"
/// as a softlock rather than as "legal moves remain", which the
/// previous heuristic incorrectly did (Quat hit this with 4 cards
/// remaining and the game just sat there).
pub fn has_legal_moves(game: &GameState) -> bool { pub fn has_legal_moves(game: &GameState) -> bool {
use solitaire_core::card::Card;
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};
// If stock or waste is non-empty, the player can always draw. let mut sources: Vec<Card> = Vec::new();
if !game.piles.get(&PileType::Stock).is_some_and(|p| p.cards.is_empty()) for ty in [PileType::Stock, PileType::Waste] {
|| !game.piles.get(&PileType::Waste).is_some_and(|p| p.cards.is_empty()) if let Some(p) = game.piles.get(&ty) {
sources.extend(p.cards.iter().cloned());
}
}
for i in 0..7_usize {
if let Some(t) = game.piles.get(&PileType::Tableau(i))
&& let Some(top) = t.cards.last().filter(|c| c.face_up)
{
sources.push(top.clone());
}
}
for card in &sources {
for slot in 0..4_u8 {
if let Some(dest) = game.piles.get(&PileType::Foundation(slot))
&& can_place_on_foundation(card, dest)
{ {
return true; return true;
} }
}
// Check each playable source pile.
let sources: Vec<PileType> = {
let mut v = vec![PileType::Waste];
for i in 0..7_usize { for i in 0..7_usize {
v.push(PileType::Tableau(i)); if let Some(dest) = game.piles.get(&PileType::Tableau(i))
} && can_place_on_tableau(card, dest)
v {
};
for from in &sources {
let Some(from_pile) = game.piles.get(from) else { continue };
let Some(card) = from_pile.cards.last().filter(|c| c.face_up) else { continue };
// Check foundation slots.
for slot in 0..4_u8 {
let dest = PileType::Foundation(slot);
if let Some(dest_pile) = game.piles.get(&dest)
&& can_place_on_foundation(card, dest_pile) {
return true;
}
}
// Check tableau piles.
for i in 0..7_usize {
let dest = PileType::Tableau(i);
if dest == *from {
continue;
}
if let Some(dest_pile) = game.piles.get(&dest)
&& can_place_on_tableau(card, dest_pile) {
return true; return true;
} }
} }
} }
false false
} }
@@ -1135,10 +1134,49 @@ mod tests {
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
#[test] #[test]
fn has_legal_moves_returns_true_when_stock_nonempty() { fn has_legal_moves_returns_true_for_fresh_game() {
// A fresh game has 24 cards in stock — draw is always available. // 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.
let game = GameState::new(42, DrawMode::DrawOne); let game = GameState::new(42, DrawMode::DrawOne);
assert!(has_legal_moves(&game), "draw is always available when stock is non-empty"); 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.
use solitaire_core::card::{Card, Rank, Suit};
let mut game = GameState::new(1, DrawMode::DrawOne);
for slot in 0..4_u8 {
game.piles.get_mut(&PileType::Foundation(slot)).unwrap().cards.clear();
}
for i in 0..7_usize {
game.piles.get_mut(&PileType::Tableau(i)).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();
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 });
}
assert!(
!has_legal_moves(&game),
"stock cards with no legal landing should count as softlock",
);
} }
#[test] #[test]