diff --git a/solitaire_engine/src/input_plugin/mod.rs b/solitaire_engine/src/input_plugin/mod.rs index 0577bf3..5d71bbd 100644 --- a/solitaire_engine/src/input_plugin/mod.rs +++ b/solitaire_engine/src/input_plugin/mod.rs @@ -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>, @@ -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, @@ -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, diff --git a/solitaire_engine/src/input_plugin/tests.rs b/solitaire_engine/src/input_plugin/tests.rs index 9471c2b..612ecb0 100644 --- a/solitaire_engine/src/input_plugin/tests.rs +++ b/solitaire_engine/src/input_plugin/tests.rs @@ -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 // -----------------------------------------------------------------------