Merge pull request 'fix(engine): double-click moves exactly the clicked run (#158)' (#167) from fix/158-double-click-exact-stack into master

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:
+/// The move covers exactly the face-up run headed by the clicked card —
-/// 1. Move the single **top** card to its best foundation (or tableau) destination.
+/// see [`auto_move_for_run`].
-/// 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.
 ///
-/// When a multi-card stack double-click finds no legal destination (Priority 2
+/// When the clicked run has no legal destination, fires `MoveRejectedEvent`
-/// returns `None`), fires `MoveRejectedEvent` with `from == to == pile` so the
+/// with `from == to == pile` so the invalid-move sound plays and the source
-/// invalid-move sound plays and the source pile cards shake as feedback.
+/// 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, count)) = auto_move_for_run(clicked_card, &pile, &game.0, card_ids.len())
-        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())
         {
             moves.write(MoveRequestEvent {
                 from: pile,
@@ -1460,14 +1468,10 @@ fn handle_double_click(
             return;
         }
 
-        // Both priorities failed — play the invalid-move sound and shake
+        // No legal destination for the clicked run — play the invalid-move
-        // the source pile as feedback. `MoveRejectedEvent` with
+        // sound and shake the source pile as feedback. `MoveRejectedEvent`
-        // `from == to` routes the shake to the source pile (which
+        // with `from == to` routes the shake to the source pile (which
-        // `start_shake_anim` reads from `ev.to`). Pre-fix, this branch
+        // `start_shake_anim` reads from `ev.to`).
-        // 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.
         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:
+/// The move covers exactly the face-up run headed by the tapped card —
-/// 1. Single top card to its best foundation (or tableau).
+/// see [`auto_move_for_run`]. Fires `MoveRejectedEvent` for audio + shake
-/// 2. Whole face-up run to best tableau column when no single-card move exists.
+/// feedback when the tapped run has no legal destination.
-/// 3. `MoveRejectedEvent` for audio + shake feedback when no legal move found.
 #[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)
+        let Some((_, found_face_up)) = pile_cards.iter().find(|(c, _)| c == top_card) else {
-        else {
             return;
         };
         if !*found_face_up {
@@ -1591,35 +1593,10 @@ fn handle_double_tap(
 
         // --- One-tap auto-move (original behaviour) ---
 
-        // Priority 1: move single top card.
+        // Move exactly the run headed by the tapped card.
-        if let Some(dest) = best_destination(found_card, &game.0) {
+        if let Some(tapped_card) = drag.cards.first()
-            for (entity, ce, mut sprite) in card_sprites.iter_mut() {
+            && let Some((dest, count)) =
-                if ce.card == *top_card {
+                auto_move_for_run(tapped_card, tapped_pile, &game.0, drag.cards.len())
-                    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,
-            });
-            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) {
@@ -1636,7 +1613,6 @@ fn handle_double_tap(
             });
             return;
         }
-        }
 
         rejected.write(MoveRejectedEvent {
             from: *tapped_pile,

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
 // -----------------------------------------------------------------------