fix(engine): double-click moves exactly the clicked run (#158) #167
@@ -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,
|
||||
|
||||
@@ -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
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user