From 6037596cc097a308dff32ca7a2ec2017f50fd9d4 Mon Sep 17 00:00:00 2001 From: funman300 Date: Wed, 6 May 2026 20:00:05 -0700 Subject: [PATCH] fix(engine): double-click move animation no longer plays twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A successful double-click was rendering the slide-to-destination animation twice — once from the first press's MoveRequestEvent landing, and again from the release's StateChangedEvent racing the in-flight CardAnim and replacing it from the mid-animation position. The frame trace: Frame N (second press): handle_double_click → MoveRequestEvent (queued) start_drag → DragState set, drag.committed = false (start_drag never mutates Transform; the card is still visually in place) handle_move → applies the move, fires StateChangedEvent sync_cards_on_change → cur ≠ target, inserts CardAnim slide (animation #1 starts) Frames N+1, N+2, …: follow_drag idles (drag uncommitted, cursor not moving) CardAnim animates the card from old to new pile Frame N+K (release): end_drag → drag.committed = false branch: drag.clear() + StateChangedEvent ← CULPRIT sync_cards_on_change → sees the card mid-CardAnim (cur ≠ target), replaces CardAnim with a fresh one starting at the current mid-position (animation #2 visibly restarts the slide) The fix is one line: drop the StateChangedEvent write in the uncommitted-drag branch of end_drag. The defensive resync was never needed there — start_drag only mutates the DragState resource on press, never card transforms, so an uncommitted drag has no visual side effect to undo. The committed-drag branch (line 762) keeps its StateChangedEvent write since snap-back from a real drag does need a resync. Existing tests pass unchanged. The bug only manifested in the specific timing of double-click → quick-release before animation-complete; an integration test would require driving mouse press/release across several frames with a dispatched GameMutation pass between, which is heavier than the fix warrants. Workspace: 1170 passing tests / 0 failing. cargo clippy --workspace --all-targets -- -D warnings clean. Co-Authored-By: Claude Opus 4.7 --- solitaire_engine/src/input_plugin.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/solitaire_engine/src/input_plugin.rs b/solitaire_engine/src/input_plugin.rs index e394cf0..c22a6e5 100644 --- a/solitaire_engine/src/input_plugin.rs +++ b/solitaire_engine/src/input_plugin.rs @@ -644,10 +644,23 @@ fn end_drag( } // If the drag was never committed (user tapped without moving far enough), - // treat it as a click: just cancel the pending drag and resync card positions. + // treat it as a click: cancel the pending drag and exit. We deliberately + // do NOT fire `StateChangedEvent` here — `start_drag` only mutates the + // `DragState` resource on press, never card transforms, so an uncommitted + // drag has no visual side effect to undo. + // + // Firing one would race a CardAnim that's already in flight on the same + // card. Specifically: on a successful double-click, `handle_double_click` + // fires `MoveRequestEvent`, `start_drag` picks the card up the same + // frame (uncommitted), and `handle_move` queues a `StateChangedEvent` → + // `sync_cards_on_change` starts a slide animation. When the player + // releases the button mid-slide, `end_drag` would fire a second + // `StateChangedEvent`, `sync_cards_on_change` would see the card mid- + // animation (`cur != target`), and replace the in-flight CardAnim with + // a fresh one — restarting the slide and reading on screen as the move + // animation playing twice. if !drag.committed { drag.clear(); - changed.write(StateChangedEvent); return; } let Some(layout) = layout else {