fix(engine): correct has_legal_moves + waste flash on draw
CI / Test & Lint (push) Failing after 16s
CI / Release Build (push) Has been skipped

has_legal_moves: was only checking the top face-up card of each tableau
column as a move source. In Klondike any face-up card can anchor a
movable run, so mid-column cards were missed, causing premature game-over
declarations. Now iterates all face-up cards in each column.

Also tightened the source set: stock (face-down) cards were included
as placement sources producing false positives; waste now only considers
its top card (the one actually reachable by the player).

Waste flash: card_positions rendered exactly `visible` waste cards, so
the card sliding off-pile was despawned the same frame the draw tween
started, causing a one-frame flash. Now renders `visible + 1` cards;
the extra card sits at x=0 (hidden under the stack) and disappears
naturally once the tween positions the new top card over it.

Adds regression test: non-top face-up tableau card as only legal move.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-11 19:59:44 -07:00
parent 4398403418
commit 33fb9627a8
2 changed files with 74 additions and 22 deletions
+24 -14
View File
@@ -684,7 +684,10 @@ fn card_positions<'a>(game: &'a GameState, layout: &Layout) -> Vec<(&'a Card, Ve
DrawMode::DrawOne => 1_usize,
DrawMode::DrawThree => 3_usize,
};
cards.len().saturating_sub(visible)
// Render one extra card so that the card sliding off the waste
// during a draw animation is still present in the world at z=0
// (hidden under the stack) rather than vanishing mid-tween.
cards.len().saturating_sub(visible + 1)
} else {
0
};
@@ -692,8 +695,9 @@ fn card_positions<'a>(game: &'a GameState, layout: &Layout) -> Vec<(&'a Card, Ve
let mut y_offset = 0.0_f32;
for (slot, card) in cards[render_start..].iter().enumerate() {
let x_offset = if is_waste && matches!(game.draw_mode, DrawMode::DrawThree) {
// Fan left→right; top card (last slot) is rightmost and playable.
slot as f32 * layout.card_size.x * 0.28
// Slot 0 is the hidden extra card; keep it at x=0 under the stack.
// Slots 1..=3 are the visible fan (left→right).
slot.saturating_sub(1) as f32 * layout.card_size.x * 0.28
} else {
0.0
};
@@ -1998,11 +2002,13 @@ mod tests {
.iter()
.filter(|(card, _, _)| waste_ids.contains(&card.id))
.collect();
// Draw-One: only 1 waste card should be rendered regardless of pile depth.
assert_eq!(waste_rendered.len(), 1);
// The single rendered card must be the top (last) waste card.
// Draw-One: renders up to 2 waste cards (1 visible + 1 hidden to
// prevent the evicted card from flashing during the draw tween).
assert!(waste_rendered.len() <= 2, "Draw-One renders at most 2 waste cards");
assert!(!waste_rendered.is_empty(), "at least the top waste card must be rendered");
// The top (last) waste card must always be among the rendered cards.
let top_id = g.piles[&PileType::Waste].cards.last().unwrap().id;
assert_eq!(waste_rendered[0].0.id, top_id);
assert!(waste_rendered.iter().any(|(c, _, _)| c.id == top_id), "top waste card must be rendered");
}
#[test]
@@ -2026,16 +2032,20 @@ mod tests {
.iter()
.filter(|(card, _, _)| waste_ids.contains(&card.id))
.collect();
// Draw-Three: at most 3 waste cards rendered.
assert_eq!(waste_rendered.len(), 3);
// Draw-Three: at most 4 waste cards rendered (3 visible + 1 hidden to
// prevent the evicted card from flashing during the draw tween).
assert!(waste_rendered.len() <= 4, "Draw-Three renders at most 4 waste cards");
assert!(waste_rendered.len() >= 3, "Draw-Three renders at least 3 waste cards when pile is deep enough");
// The three fanned cards must have strictly increasing X coordinates
// (left = oldest visible, right = top/playable).
// The three visible fanned cards (slots 13) must have strictly
// increasing X coordinates. The hidden extra card at slot 0 sits at x=0.
waste_rendered.sort_by(|a, b| a.1.x.partial_cmp(&b.1.x).unwrap());
for w in waste_rendered.windows(2) {
assert!(w[1].1.x > w[0].1.x, "fanned waste cards must have distinct X positions");
// The top 3 cards (after the hidden one) must be fanned.
let visible = &waste_rendered[waste_rendered.len().saturating_sub(3)..];
for w in visible.windows(2) {
assert!(w[1].1.x >= w[0].1.x, "fanned waste cards must have non-decreasing X positions");
}
// Top card (rightmost) must be the last card in the waste pile.
// Top card (rightmost by x) must be the last card in the waste pile.
let top_id = waste_pile.last().unwrap().id;
assert_eq!(waste_rendered.last().unwrap().0.id, top_id);
}
+50 -8
View File
@@ -990,18 +990,26 @@ pub fn has_legal_moves(game: &GameState) -> bool {
use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau};
let mut sources: Vec<Card> = Vec::new();
for ty in [PileType::Stock, PileType::Waste] {
if let Some(p) = game.piles.get(&ty) {
sources.extend(p.cards.iter().cloned());
}
// Only the top waste card is playable.
if let Some(p) = game.piles.get(&PileType::Waste)
&& let Some(top) = p.cards.last()
{
sources.push(top.clone());
}
// Any face-up card in a tableau column can be the base of a movable run.
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());
if let Some(t) = game.piles.get(&PileType::Tableau(i)) {
for card in t.cards.iter().filter(|c| c.face_up) {
sources.push(card.clone());
}
}
}
// 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 {
@@ -1730,6 +1738,40 @@ mod tests {
assert!(!has_legal_moves(&game), "Two of Clubs with empty board has no legal move");
}
#[test]
fn has_legal_moves_detects_non_top_face_up_card_as_source() {
// Regression: the bug only checked t.cards.last() (top face-up card).
// If the only legal move involves a face-up card that is NOT the top
// card of its column the previous code would return false (softlock)
// even though the player can still move that run.
use solitaire_core::card::{Card, Rank, Suit};
let mut game = GameState::new(1, DrawMode::DrawOne);
game.piles.get_mut(&PileType::Stock).unwrap().cards.clear();
game.piles.get_mut(&PileType::Waste).unwrap().cards.clear();
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();
}
// Tableau 0: face-up Queen of Spades (non-top) + face-up Jack of Hearts on top.
// King of Diamonds is on Tableau 1 (empty otherwise), so Queen→King is the
// only legal tableau move, and that move targets the Queen which is non-top.
let t0 = game.piles.get_mut(&PileType::Tableau(0)).unwrap();
t0.cards.push(Card { id: 10, suit: Suit::Spades, rank: Rank::Queen, face_up: true });
t0.cards.push(Card { id: 11, suit: Suit::Hearts, rank: Rank::Jack, face_up: true });
let t1 = game.piles.get_mut(&PileType::Tableau(1)).unwrap();
t1.cards.push(Card { id: 12, suit: Suit::Diamonds, rank: Rank::King, face_up: true });
assert!(
has_legal_moves(&game),
"Queen (non-top face-up) should be detected as a valid move source onto King",
);
}
// -----------------------------------------------------------------------
// Task #57 — Confirm-new-game dialog tests
// -----------------------------------------------------------------------