Compare commits

...

1 Commits

Author SHA1 Message Date
funman300 adbcb8f59a fix(engine): double-click moves exactly the clicked run (#158)
Test / test (pull_request) Successful in 36m48s
Double-click/tap auto-move previously tried the run's top card alone
before the full run headed by the clicked card, so a top card with a
foundation move hijacked the intended whole-stack move. Both handlers
now share auto_move_for_run: a lone top card goes foundation-first, a
multi-card run moves whole to a tableau or not at all. The double-click
key is now the clicked card, so two clicks on different cards of the
same stack no longer register as a double-click.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 12:55:43 -07:00
2 changed files with 163 additions and 75 deletions
+51 -75
View File
@@ -1370,17 +1370,37 @@ pub fn best_tableau_destination_for_stack(
None
}
/// Decide the auto-move for the face-up run headed by the clicked/tapped card.
///
/// The move covers **exactly** `run_len` cards — the run from the clicked
/// card to the top of its pile. A lone top card goes to its best foundation
/// (or tableau) destination; a multi-card run goes whole to the best tableau
/// column. Runs larger or smaller than the clicked one are never considered.
///
/// Returns `(destination, count)`, or `None` when the clicked run has no
/// legal destination.
pub fn auto_move_for_run(
clicked_card: &Card,
pile: &KlondikePile,
game: &GameState,
run_len: usize,
) -> Option<(KlondikePile, usize)> {
if run_len == 1 {
best_destination(clicked_card, game).map(|dest| (dest, 1))
} else {
best_tableau_destination_for_stack(clicked_card, pile, game, run_len)
}
}
/// System that detects double-clicks on face-up cards and fires `MoveRequestEvent`
/// to the best legal destination.
///
/// Move priority:
/// 1. Move the single **top** card to its best foundation (or tableau) destination.
/// 2. If no single-card move exists and the clicked card is the base of a
/// multi-card face-up stack, move the whole stack to the best tableau column.
/// The move covers exactly the face-up run headed by the clicked card —
/// see [`auto_move_for_run`].
///
/// When a multi-card stack double-click finds no legal destination (Priority 2
/// returns `None`), fires `MoveRejectedEvent` with `from == to == pile` so the
/// invalid-move sound plays and the source pile cards shake as feedback.
/// When the clicked run has no legal destination, fires `MoveRejectedEvent`
/// with `from == to == pile` so the invalid-move sound plays and the source
/// pile cards shake as feedback.
#[allow(clippy::too_many_arguments)]
fn handle_double_click(
buttons: Res<ButtonInput<MouseButton>>,
@@ -1411,7 +1431,11 @@ fn handle_double_click(
return;
};
// The topmost card in the draggable run — used as the double-click key.
// The clicked card heads the run and keys the double-click: two clicks
// on different cards of the same stack are not a double-click.
let Some(clicked_card) = card_ids.first() else {
return;
};
let Some(top_card) = card_ids.last() else {
return;
};
@@ -1426,31 +1450,15 @@ fn handle_double_click(
let now = time.elapsed_secs();
let prev = last_click
.get(top_card)
.get(clicked_card)
.copied()
.unwrap_or(f32::NEG_INFINITY);
if now - prev <= DOUBLE_CLICK_WINDOW {
// Double-click confirmed.
last_click.remove(top_card);
last_click.remove(clicked_card);
// Priority 1: move the single top card (foundation preferred, then tableau).
if let Some(dest) = best_destination(top_card, &game.0) {
moves.write(MoveRequestEvent {
from: pile,
to: dest,
count: 1,
});
return;
}
// Priority 2: if the player clicked the base of a multi-card face-up
// stack (card_ids.len() > 1), try moving the whole stack to another
// tableau column.
if card_ids.len() > 1
&& let Some((bottom_card, _)) = pile_cards.get(stack_index)
&& let Some((dest, count)) =
best_tableau_destination_for_stack(bottom_card, &pile, &game.0, card_ids.len())
if let Some((dest, count)) = auto_move_for_run(clicked_card, &pile, &game.0, card_ids.len())
{
moves.write(MoveRequestEvent {
from: pile,
@@ -1460,14 +1468,10 @@ fn handle_double_click(
return;
}
// Both priorities failed — play the invalid-move sound and shake
// the source pile as feedback. `MoveRejectedEvent` with
// `from == to` routes the shake to the source pile (which
// `start_shake_anim` reads from `ev.to`). Pre-fix, this branch
// only fired for multi-card stacks, so a double-click on a
// single card with no legal destination did nothing — no
// sound, no shake. Now both single-card and stack misses get
// the same feedback.
// No legal destination for the clicked run — play the invalid-move
// sound and shake the source pile as feedback. `MoveRejectedEvent`
// with `from == to` routes the shake to the source pile (which
// `start_shake_anim` reads from `ev.to`).
rejected.write(MoveRejectedEvent {
from: pile,
to: pile,
@@ -1475,7 +1479,7 @@ fn handle_double_click(
});
} else {
// Single click — record the time.
last_click.insert(top_card.clone(), now);
last_click.insert(clicked_card.clone(), now);
}
}
@@ -1491,10 +1495,9 @@ fn handle_double_click(
/// `cards`, and `origin_pile`; once `touch_end_drag` fires those fields
/// are cleared and the tap/drag distinction is permanently lost.
///
/// Move priority:
/// 1. Single top card to its best foundation (or tableau).
/// 2. Whole face-up run to best tableau column when no single-card move exists.
/// 3. `MoveRejectedEvent` for audio + shake feedback when no legal move found.
/// The move covers exactly the face-up run headed by the tapped card —
/// see [`auto_move_for_run`]. Fires `MoveRejectedEvent` for audio + shake
/// feedback when the tapped run has no legal destination.
#[allow(clippy::too_many_arguments)]
fn handle_double_tap(
mut touch_events: MessageReader<TouchInput>,
@@ -1554,8 +1557,7 @@ fn handle_double_tap(
return;
}
let Some((found_card, found_face_up)) = pile_cards.iter().find(|(c, _)| c == top_card)
else {
let Some((_, found_face_up)) = pile_cards.iter().find(|(c, _)| c == top_card) else {
return;
};
if !*found_face_up {
@@ -1591,53 +1593,27 @@ fn handle_double_tap(
// --- One-tap auto-move (original behaviour) ---
// Priority 1: move single top card.
if let Some(dest) = best_destination(found_card, &game.0) {
// Move exactly the run headed by the tapped card.
if let Some(tapped_card) = drag.cards.first()
&& let Some((dest, count)) =
auto_move_for_run(tapped_card, tapped_pile, &game.0, drag.cards.len())
{
for (entity, ce, mut sprite) in card_sprites.iter_mut() {
if ce.card == *top_card {
if drag.cards.contains(&ce.card) {
sprite.color = STATE_SUCCESS;
commands.entity(entity).insert(HintHighlight {
remaining: DOUBLE_TAP_FLASH_SECS,
});
break;
}
}
moves.write(MoveRequestEvent {
from: *tapped_pile,
to: dest,
count: 1,
count,
});
return;
}
// Priority 2: move whole face-up stack to best tableau column.
if drag.cards.len() > 1 {
let stack_index = pile_cards.len() - drag.cards.len();
if let Some((bottom_card, _)) = pile_cards.get(stack_index)
&& let Some((dest, count)) = best_tableau_destination_for_stack(
bottom_card,
tapped_pile,
&game.0,
drag.cards.len(),
)
{
for (entity, ce, mut sprite) in card_sprites.iter_mut() {
if drag.cards.contains(&ce.card) {
sprite.color = STATE_SUCCESS;
commands.entity(entity).insert(HintHighlight {
remaining: DOUBLE_TAP_FLASH_SECS,
});
}
}
moves.write(MoveRequestEvent {
from: *tapped_pile,
to: dest,
count,
});
return;
}
}
rejected.write(MoveRejectedEvent {
from: *tapped_pile,
to: *tapped_pile,
+112
View File
@@ -429,6 +429,118 @@ fn best_tableau_destination_for_stack_returns_none_when_no_legal_move() {
);
}
// -----------------------------------------------------------------------
// auto_move_for_run pure-function tests (issue #158)
// -----------------------------------------------------------------------
//
// These need real positions — `can_move_cards` validates against the live
// session, not the `set_test_*` overlays. Seeds 51 and 145 both deal an
// Ace and its Two on tableau tops plus an opposite-color Three elsewhere,
// letting two moves build a face-up [Three, Two] run whose top card is
// foundation-eligible (the bait the pre-#158 code would take).
/// Deal `seed`, send the Ace on tableau 1 to its foundation, then stack the
/// matching Two from `two_from` onto the opposite-color Three on `run_on`.
/// Returns the game with a 2-card face-up run on `run_on`.
fn deal_run_with_foundation_bait(seed: u64, two_from: Tableau, run_on: Tableau) -> GameState {
let mut game = GameState::new(seed, DrawStockConfig::DrawOne);
let (ace, _) = game
.pile(KlondikePile::Tableau(Tableau::Tableau1))
.last()
.cloned()
.expect("seed deals a card on tableau 1");
let foundation = best_destination(&ace, &game).expect("ace has a foundation home");
game.move_cards(KlondikePile::Tableau(Tableau::Tableau1), foundation, 1)
.expect("ace moves to foundation");
game.move_cards(
KlondikePile::Tableau(two_from),
KlondikePile::Tableau(run_on),
1,
)
.expect("two stacks onto three");
game
}
#[test]
fn auto_move_for_run_moves_exact_clicked_run_not_top_card() {
let game = deal_run_with_foundation_bait(51, Tableau::Tableau6, Tableau::Tableau5);
let run_pile = KlondikePile::Tableau(Tableau::Tableau5);
let cards = game.pile(run_pile);
let (top, _) = cards.last().cloned().expect("run pile has cards");
let (clicked, _) = cards[cards.len() - 2].clone();
// The bait: the lone top card has a foundation move available.
assert!(
matches!(
best_destination(&top, &game),
Some(KlondikePile::Foundation(_))
),
"precondition: run top card must be foundation-eligible"
);
// Clicking the run base must move exactly the 2-card run to a tableau.
match auto_move_for_run(&clicked, &run_pile, &game, 2) {
Some((KlondikePile::Tableau(dest), 2)) => assert_ne!(dest, Tableau::Tableau5),
other => panic!("expected a whole-run tableau move, got {other:?}"),
}
}
#[test]
fn auto_move_for_run_rejects_when_clicked_run_cannot_move() {
let game = deal_run_with_foundation_bait(145, Tableau::Tableau4, Tableau::Tableau7);
let run_pile = KlondikePile::Tableau(Tableau::Tableau7);
let cards = game.pile(run_pile);
let (top, _) = cards.last().cloned().expect("run pile has cards");
let (clicked, _) = cards[cards.len() - 2].clone();
assert!(
matches!(
best_destination(&top, &game),
Some(KlondikePile::Foundation(_))
),
"precondition: run top card must be foundation-eligible"
);
// The 2-card run has no legal home — the top card's foundation move
// must NOT be taken as a substitute.
assert_eq!(
auto_move_for_run(&clicked, &run_pile, &game, 2),
None,
"an immovable clicked run must not fall back to a top-card move"
);
}
#[test]
fn auto_move_for_run_single_card_prefers_foundation() {
// Seed 51 after the Ace reaches the foundation: the Two on tableau 6
// is a lone face-up card that could go to the foundation OR onto the
// Three on tableau 5. Foundation must win.
let mut game = GameState::new(51, DrawStockConfig::DrawOne);
let (ace, _) = game
.pile(KlondikePile::Tableau(Tableau::Tableau1))
.last()
.cloned()
.expect("seed 51 deals a card on tableau 1");
let foundation = best_destination(&ace, &game).expect("ace has a foundation home");
game.move_cards(KlondikePile::Tableau(Tableau::Tableau1), foundation, 1)
.expect("ace moves to foundation");
let two_pile = KlondikePile::Tableau(Tableau::Tableau6);
let (two, _) = game.pile(two_pile).last().cloned().expect("two on top");
assert!(
game.can_move_cards(&two_pile, &KlondikePile::Tableau(Tableau::Tableau5), 1),
"precondition: a tableau destination also exists"
);
assert!(
matches!(
auto_move_for_run(&two, &two_pile, &game, 1),
Some((KlondikePile::Foundation(_), 1))
),
"a lone top card still prefers the foundation"
);
}
// -----------------------------------------------------------------------
// Task #28 — find_hint pure-function tests
// -----------------------------------------------------------------------