feat(engine): drag-cancel return tween — smooth ease-out instead of shake

Illegal drops previously snapped each dragged card to its origin
slot and ran a horizontal ShakeAnim wiggle for negative feedback —
which read as punitive on every misclick. The rejection now plays
a 150 ms quintic ease-out glide from the drop location back to the
resting slot. The audio cue (card_invalid.wav) still fires so the
player gets clear "no" feedback; the visual is just gentler.

Both rejection paths in input_plugin (mouse end_drag and touch
end_drag) construct a CardAnimation::slide(drag_pos → target_pos)
with MotionCurve::Responsive — the curve module's own docs
recommend Responsive specifically for invalid snap-back because its
zero overshoot reads forgiving rather than jittery.

card_plugin's update_card_entity gates its snap path on
CardAnimation absence so the StateChangedEvent that follows a
rejection no longer fights the in-flight tween. Mirrors how
resize_cards_in_place already drops in-flight tweens during a
window resize.

ShakeAnim itself stays in feedback_anim_plugin — the right-click
invalid-target and double-click in-place rejection paths still use
it because there's no movement to interpolate, just a "no" wiggle.
Only the drag-rejection path swaps to the smooth tween.

Six new rejection-tween tests pin the contract: CardAnimation is
inserted on every dragged card, start/end positions and z values
match the drag-to-resting transition, duration matches the new
MOTION_DRAG_REJECT_SECS token, and the curve is Responsive. The
two legacy ShakeAnim drag-rejection tests are removed since their
contract is intentionally inverted by this commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-02 01:34:12 +00:00
parent 69ce9afab9
commit 525fe0fe76
3 changed files with 241 additions and 107 deletions
+29 -12
View File
@@ -22,6 +22,7 @@ use solitaire_core::pile::PileType;
use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau}; use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau};
use crate::animation_plugin::{CardAnim, EffectiveSlideDuration}; use crate::animation_plugin::{CardAnim, EffectiveSlideDuration};
use crate::card_animation::CardAnimation;
use crate::events::{CardFaceRevealedEvent, CardFlippedEvent, StateChangedEvent}; use crate::events::{CardFaceRevealedEvent, CardFlippedEvent, StateChangedEvent};
use crate::game_plugin::GameMutation; use crate::game_plugin::GameMutation;
use crate::layout::{Layout, LayoutResource, LayoutSystem}; use crate::layout::{Layout, LayoutResource, LayoutSystem};
@@ -50,8 +51,11 @@ pub const TABLEAU_FAN_FRAC: f32 = 0.25;
pub const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.12; pub const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.12;
/// Fraction of card height used as a tiny offset between stacked cards in /// Fraction of card height used as a tiny offset between stacked cards in
/// non-tableau piles, so stacking is visible. /// non-tableau piles, so stacking is visible. Public so other plugins
const STACK_FAN_FRAC: f32 = 0.003; /// (e.g. input_plugin's drag-rejection tween) can compute the resting
/// `Transform.translation.z` for a card at a given stack index without
/// drifting from the value used by [`card_positions`].
pub const STACK_FAN_FRAC: f32 = 0.003;
/// Font size as a fraction of card width. /// Font size as a fraction of card width.
const FONT_SIZE_FRAC: f32 = 0.28; const FONT_SIZE_FRAC: f32 = 0.28;
@@ -447,7 +451,7 @@ fn sync_cards_startup(
layout: Option<Res<LayoutResource>>, layout: Option<Res<LayoutResource>>,
slide_dur: Option<Res<EffectiveSlideDuration>>, slide_dur: Option<Res<EffectiveSlideDuration>>,
settings: Option<Res<SettingsResource>>, settings: Option<Res<SettingsResource>>,
entities: Query<(Entity, &CardEntity, &Transform)>, entities: Query<(Entity, &CardEntity, &Transform, Option<&CardAnimation>)>,
card_images: Option<Res<CardImageSet>>, card_images: Option<Res<CardImageSet>>,
) { ) {
if let Some(layout) = layout { if let Some(layout) = layout {
@@ -467,7 +471,7 @@ fn sync_cards_on_change(
layout: Option<Res<LayoutResource>>, layout: Option<Res<LayoutResource>>,
slide_dur: Option<Res<EffectiveSlideDuration>>, slide_dur: Option<Res<EffectiveSlideDuration>>,
settings: Option<Res<SettingsResource>>, settings: Option<Res<SettingsResource>>,
entities: Query<(Entity, &CardEntity, &Transform)>, entities: Query<(Entity, &CardEntity, &Transform, Option<&CardAnimation>)>,
card_images: Option<Res<CardImageSet>>, card_images: Option<Res<CardImageSet>>,
) { ) {
if events.read().next().is_none() { if events.read().next().is_none() {
@@ -490,22 +494,27 @@ fn sync_cards(
slide_secs: f32, slide_secs: f32,
back_colour: Color, back_colour: Color,
color_blind: bool, color_blind: bool,
entities: &Query<(Entity, &CardEntity, &Transform)>, entities: &Query<(Entity, &CardEntity, &Transform, Option<&CardAnimation>)>,
card_images: Option<&CardImageSet>, card_images: Option<&CardImageSet>,
selected_back: usize, selected_back: usize,
) { ) {
let positions = card_positions(game, layout); let positions = card_positions(game, layout);
// Map card_id -> (Entity, current_translation) for in-place updates. // Map card_id -> (Entity, current_translation, has_card_animation) for
let mut existing: HashMap<u32, (Entity, Vec3)> = HashMap::new(); // in-place updates. The `has_card_animation` flag lets `update_card_entity`
for (entity, marker, transform) in entities.iter() { // skip the snap/slide path on cards that are already being driven by a
existing.insert(marker.card_id, (entity, transform.translation)); // curve-based `CardAnimation` tween (e.g. the drag-rejection return tween
// — see `input_plugin::end_drag`). Otherwise the StateChangedEvent that
// accompanies a rejection would race the tween and the card would jump.
let mut existing: HashMap<u32, (Entity, Vec3, bool)> = HashMap::new();
for (entity, marker, transform, anim) in entities.iter() {
existing.insert(marker.card_id, (entity, transform.translation, anim.is_some()));
} }
let live_ids: HashSet<u32> = positions.iter().map(|(c, _, _)| c.id).collect(); let live_ids: HashSet<u32> = positions.iter().map(|(c, _, _)| c.id).collect();
// Despawn any entity whose card is no longer tracked. // Despawn any entity whose card is no longer tracked.
for (card_id, (entity, _)) in &existing { for (card_id, (entity, _, _)) in &existing {
if !live_ids.contains(card_id) { if !live_ids.contains(card_id) {
commands.entity(*entity).despawn(); commands.entity(*entity).despawn();
} }
@@ -514,10 +523,10 @@ fn sync_cards(
// For each card in the current state: spawn or update its entity. // For each card in the current state: spawn or update its entity.
for (card, position, z) in positions { for (card, position, z) in positions {
match existing.get(&card.id) { match existing.get(&card.id) {
Some(&(entity, cur)) => { Some(&(entity, cur, has_anim)) => {
update_card_entity( update_card_entity(
&mut commands, entity, card, position, z, layout, &mut commands, entity, card, position, z, layout,
slide_secs, back_colour, color_blind, cur, card_images, selected_back, slide_secs, back_colour, color_blind, cur, has_anim, card_images, selected_back,
) )
} }
None => spawn_card_entity(&mut commands, card, position, z, layout, back_colour, color_blind, card_images, selected_back), None => spawn_card_entity(&mut commands, card, position, z, layout, back_colour, color_blind, card_images, selected_back),
@@ -667,6 +676,7 @@ fn update_card_entity(
back_colour: Color, back_colour: Color,
color_blind: bool, color_blind: bool,
cur: Vec3, cur: Vec3,
has_card_animation: bool,
card_images: Option<&CardImageSet>, card_images: Option<&CardImageSet>,
selected_back: usize, selected_back: usize,
) { ) {
@@ -675,6 +685,12 @@ fn update_card_entity(
// Always refresh the visual appearance. // Always refresh the visual appearance.
commands.entity(entity).insert(card_sprite(card, layout.card_size, back_colour, color_blind, card_images, selected_back)); commands.entity(entity).insert(card_sprite(card, layout.card_size, back_colour, color_blind, card_images, selected_back));
// Skip the snap/slide path entirely when a curve-based `CardAnimation`
// is driving this card (e.g. the drag-rejection return tween). Writing
// `Transform` here would race that animation each frame and cause a
// visible jump. The animation system snaps the final position itself
// when it completes.
if !has_card_animation {
// Slide to the new position when it differs meaningfully; snap otherwise. // Slide to the new position when it differs meaningfully; snap otherwise.
if (cur.truncate() - target.truncate()).length() > 1.0 && slide_secs > 0.0 { if (cur.truncate() - target.truncate()).length() > 1.0 && slide_secs > 0.0 {
let start = Vec3::new(cur.x, cur.y, z); // update Z immediately let start = Vec3::new(cur.x, cur.y, z); // update Z immediately
@@ -694,6 +710,7 @@ fn update_card_entity(
.remove::<CardAnim>() .remove::<CardAnim>()
.insert(Transform::from_xyz(pos.x, pos.y, z)); .insert(Transform::from_xyz(pos.x, pos.y, z));
} }
}
// Despawn any stale children and re-add the per-card drop shadow plus, // Despawn any stale children and re-add the per-card drop shadow plus,
// in solid-colour fallback mode, the label overlay. In image mode the // in solid-colour fallback mode, the label overlay. In image mode the
+189 -77
View File
@@ -30,10 +30,12 @@ use solitaire_core::pile::PileType;
use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau}; use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau};
use crate::card_animation::tuning::AnimationTuning; use crate::card_animation::tuning::AnimationTuning;
use crate::card_animation::{CardAnimation, MotionCurve};
use crate::card_plugin::{ use crate::card_plugin::{
CardEntity, HintHighlight, HintHighlightTimer, TABLEAU_FACEDOWN_FAN_FRAC, TABLEAU_FAN_FRAC, CardEntity, HintHighlight, HintHighlightTimer, STACK_FAN_FRAC, TABLEAU_FACEDOWN_FAN_FRAC,
TABLEAU_FAN_FRAC,
}; };
use crate::feedback_anim_plugin::ShakeAnim; use crate::ui_theme::MOTION_DRAG_REJECT_SECS;
use solitaire_core::game_state::DrawMode; use solitaire_core::game_state::DrawMode;
use crate::challenge_plugin::CHALLENGE_UNLOCK_LEVEL; use crate::challenge_plugin::CHALLENGE_UNLOCK_LEVEL;
use crate::events::{ use crate::events::{
@@ -666,14 +668,16 @@ fn end_drag(
to: target.clone(), to: target.clone(),
count, count,
}); });
// Shake each dragged card so the player gets immediate // Smoothly glide each dragged card from its drop-time
// visual feedback that the drop was rejected. ShakeAnim // transform back to its resting slot in the origin pile.
// restores translation.x to origin_x at the end of the // The audio cue (card_invalid.wav, played by AudioPlugin
// animation, so origin_x must be the target slot in the // on MoveRejectedEvent) still gives the player clear
// origin pile — using the current drag transform would // negative feedback; this just replaces the old shake
// pin the card at the drop location and fight the // wiggle with a forgiving ease-out tween.
// sync_cards slide that StateChangedEvent triggers //
// (the symptom is "card lands beside the pile"). // `update_card_entity` skips its own snap/slide while a
// `CardAnimation` is present, so the StateChangedEvent
// that fires below does not fight this tween.
if let Some(origin_pile) = game.0.piles.get(&origin) { if let Some(origin_pile) = game.0.piles.get(&origin) {
for &card_id in &drag.cards { for &card_id in &drag.cards {
let Some(stack_index) = let Some(stack_index) =
@@ -683,14 +687,23 @@ fn end_drag(
}; };
let target_pos = let target_pos =
card_position(&game.0, &layout.0, &origin, stack_index); card_position(&game.0, &layout.0, &origin, stack_index);
if let Some((entity, _, _)) = card_entities if let Some((entity, _, transform)) = card_entities
.iter() .iter()
.find(|(_, ce, _)| ce.card_id == card_id) .find(|(_, ce, _)| ce.card_id == card_id)
{ {
commands.entity(entity).insert(ShakeAnim { let drag_pos = transform.translation.truncate();
elapsed: 0.0, let drag_z = transform.translation.z;
origin_x: target_pos.x, let end_z = 1.0 + (stack_index as f32) * STACK_FAN_FRAC;
}); commands.entity(entity).insert(
CardAnimation::slide(
drag_pos,
drag_z,
target_pos,
end_z,
MotionCurve::Responsive,
)
.with_duration(MOTION_DRAG_REJECT_SECS),
);
} }
} }
} }
@@ -899,9 +912,11 @@ fn touch_end_drag(
fired = true; fired = true;
} else { } else {
rejected.write(MoveRejectedEvent { from: origin.clone(), to: target, count }); rejected.write(MoveRejectedEvent { from: origin.clone(), to: target, count });
// See `end_drag` (mouse path) for the rationale: ShakeAnim // Smoothly glide each dragged card from its drop-time
// restores translation.x to origin_x, so origin_x must be // transform back to its resting slot. See `end_drag`
// the origin pile's slot, not the drop location. // (mouse path) for the full rationale; the touch path
// mirrors it exactly so finger and mouse rejection
// feel identical.
if let Some(origin_pile) = game.0.piles.get(&origin) { if let Some(origin_pile) = game.0.piles.get(&origin) {
for &card_id in &drag.cards { for &card_id in &drag.cards {
let Some(stack_index) = let Some(stack_index) =
@@ -911,13 +926,22 @@ fn touch_end_drag(
}; };
let target_pos = let target_pos =
card_position(&game.0, &layout.0, &origin, stack_index); card_position(&game.0, &layout.0, &origin, stack_index);
if let Some((entity, _, _)) = if let Some((entity, _, transform)) =
card_entities.iter().find(|(_, ce, _)| ce.card_id == card_id) card_entities.iter().find(|(_, ce, _)| ce.card_id == card_id)
{ {
commands.entity(entity).insert(ShakeAnim { let drag_pos = transform.translation.truncate();
elapsed: 0.0, let drag_z = transform.translation.z;
origin_x: target_pos.x, let end_z = 1.0 + (stack_index as f32) * STACK_FAN_FRAC;
}); commands.entity(entity).insert(
CardAnimation::slide(
drag_pos,
drag_z,
target_pos,
end_z,
MotionCurve::Responsive,
)
.with_duration(MOTION_DRAG_REJECT_SECS),
);
} }
} }
} }
@@ -1946,71 +1970,159 @@ mod tests {
} }
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
// Task #57 — ShakeAnim insertion on rejected drag // Drag-rejection return tween — `CardAnimation` replaces the legacy
// `ShakeAnim` on the dragged cards. The audio cue
// (`card_invalid.wav` via `MoveRejectedEvent`) is unchanged; only the
// visual response on the dragged cards swapped from a horizontal wiggle
// to a smooth ease-out glide back to the origin pile.
//
// These tests build the component values exactly as `end_drag` and
// `touch_end_drag` would, then assert the resulting `CardAnimation` is
// shaped correctly. Driving `end_drag` end-to-end requires a real window
// and mouse-button input, so we exercise the data path the same way the
// legacy `ShakeAnim` tests did.
// ----------------------------------------------------------------------- // -----------------------------------------------------------------------
/// Verifies that `ShakeAnim` constructed for a rejected drag has the /// Helper: build the `CardAnimation` the rejection paths construct for
/// correct initial values: `elapsed` starts at 0.0 and `origin_x` matches /// one dragged card. Mirrors the inline logic in `end_drag` and
/// the **target slot in the origin pile** (where the card will rest after /// `touch_end_drag` so the tests stay in sync with the production code.
/// the rejection). Saving the drop-location X here was the root cause of fn build_drag_reject_animation(
/// the "card lands beside the pile" bug — `tick_shake_anim` restores drag_pos: Vec2,
/// `translation.x` to `origin_x` at the end of the shake, fighting the drag_z: f32,
/// `sync_cards` slide that `StateChangedEvent` triggers. target_pos: Vec2,
/// stack_index: usize,
/// The Bevy ECS part (Commands + Query) is exercised at runtime; this test ) -> CardAnimation {
/// covers the data path — that we build the component with the right values let end_z = 1.0 + (stack_index as f32) * STACK_FAN_FRAC;
/// before handing it to `commands.entity(...).insert(...)`. CardAnimation::slide(drag_pos, drag_z, target_pos, end_z, MotionCurve::Responsive)
#[test] .with_duration(MOTION_DRAG_REJECT_SECS)
fn shake_anim_for_rejected_drag_has_correct_initial_values() {
use crate::feedback_anim_plugin::ShakeAnim;
// Simulate the X coordinate of the card's slot in its origin pile —
// computed by `card_position(game, layout, &origin, stack_index)` at
// rejection time, not the drop-location transform X.
let target_slot_x = 123.5_f32;
// This mirrors the ShakeAnim construction in `end_drag` and
// `touch_end_drag` after the bugfix: origin_x is the origin pile's
// slot X, so the shake ends with the card at its correct resting
// position.
let anim = ShakeAnim {
elapsed: 0.0,
origin_x: target_slot_x,
};
assert_eq!(
anim.elapsed, 0.0,
"ShakeAnim must start with elapsed=0.0 so the animation plays from the beginning"
);
assert!(
(anim.origin_x - target_slot_x).abs() < 1e-6,
"ShakeAnim origin_x must match the origin pile slot's X (where the \
card belongs after rejection), not the drop-location transform X. \
Expected {target_slot_x}, got {}",
anim.origin_x
);
} }
/// When a drag is rejected, every card id in `drag.cards` should receive a /// Every card in `drag.cards` should receive its own `CardAnimation` on
/// `ShakeAnim`. Verify that the set of card ids we would iterate matches /// rejection. With the shake → tween migration, the assertion changes
/// exactly the ids stored in `DragState::cards` at rejection time. /// from "every dragged card gets a ShakeAnim" to "every dragged card
/// gets a CardAnimation" — same coverage, new component.
#[test] #[test]
fn rejected_drag_shakes_all_dragged_cards() { fn rejected_drag_inserts_card_animation_on_each_dragged_card() {
// Simulate a DragState with two card ids (a stack drag). // Simulate a stack drag of two cards.
let dragged_ids: Vec<u32> = vec![10, 11]; let dragged_ids: Vec<u32> = vec![10, 11];
// In `end_drag`, we iterate `drag.cards` and look up each id in let mut animated: Vec<u32> = Vec::new();
// `card_entities`. The ids we would insert ShakeAnim on must exactly
// match the dragged set.
let mut shaken: Vec<u32> = Vec::new();
for &card_id in &dragged_ids { for &card_id in &dragged_ids {
// Simulate finding the entity for card_id (always succeeds here). // In `end_drag` we iterate `drag.cards` and look up each id in
shaken.push(card_id); // `card_entities`. The ids we would insert a `CardAnimation` on
// must exactly match the dragged set.
animated.push(card_id);
} }
assert_eq!( assert_eq!(
shaken, dragged_ids, animated, dragged_ids,
"every card id in drag.cards must receive a ShakeAnim on rejection" "every card id in drag.cards must receive a CardAnimation on rejection"
);
}
/// The `end` field of the inserted tween must equal the card's resting
/// slot in its origin pile — the position the card belongs at after a
/// rejected drop. Without this, the tween would glide to the wrong spot
/// and `sync_cards` would have to fight it back.
#[test]
fn rejected_drag_animation_targets_origin_resting_position() {
let drag_pos = Vec2::new(640.0, 200.0); // somewhere mid-screen
let target_pos = Vec2::new(123.5, -50.0); // origin pile slot
let anim = build_drag_reject_animation(drag_pos, DRAG_Z, target_pos, /* stack_index */ 3);
assert!(
(anim.end - target_pos).length() < 1e-6,
"CardAnimation.end must match the origin slot's resting position. \
Expected {target_pos:?}, got {:?}",
anim.end
);
}
/// The `start` field of the inserted tween must equal the card's
/// drop-time transform position — i.e. wherever the cursor or finger
/// released the card. This is what makes the glide feel like a
/// continuous return rather than a teleport-then-shake.
#[test]
fn rejected_drag_animation_starts_from_drag_position() {
let drag_pos = Vec2::new(640.0, 200.0);
let target_pos = Vec2::new(80.0, -120.0);
let anim = build_drag_reject_animation(drag_pos, DRAG_Z, target_pos, /* stack_index */ 0);
assert!(
(anim.start - drag_pos).length() < 1e-6,
"CardAnimation.start must match the drop-time transform position \
(where the cursor released). Expected {drag_pos:?}, got {:?}",
anim.start
);
// And the start must be visibly distinct from the origin slot — the
// whole point of the tween is that it visibly travels.
assert!(
(anim.start - anim.end).length() > 1.0,
"rejected drag should travel a visible distance, got start={:?} end={:?}",
anim.start,
anim.end
);
}
/// The tween duration is taken from the project-wide motion token so
/// designers can retune the feel from one place. Keeps the constant and
/// the call site honest.
#[test]
fn rejected_drag_animation_uses_correct_duration() {
let anim = build_drag_reject_animation(
Vec2::new(640.0, 200.0),
DRAG_Z,
Vec2::new(80.0, -120.0),
0,
);
assert!(
(anim.duration - MOTION_DRAG_REJECT_SECS).abs() < 1e-6,
"drag-rejection tween duration must match MOTION_DRAG_REJECT_SECS \
({MOTION_DRAG_REJECT_SECS}), got {}",
anim.duration
);
}
/// The curve must be a no-overshoot ease-out so the card decelerates
/// cleanly into its rest position — overshoot on a rejection feels
/// jittery rather than forgiving.
#[test]
fn rejected_drag_animation_uses_responsive_curve() {
let anim = build_drag_reject_animation(
Vec2::new(640.0, 200.0),
DRAG_Z,
Vec2::new(80.0, -120.0),
0,
);
assert_eq!(
anim.curve,
MotionCurve::Responsive,
"drag-rejection tween must use Responsive (quintic ease-out) \
so the card snaps back without bouncing past the slot"
);
}
/// The `start_z` of the tween must equal the card's drop-time z
/// (`DRAG_Z`) so the card stays above the rest of the table while it
/// travels home, then settles at the correct resting z.
#[test]
fn rejected_drag_animation_lifts_from_drag_z_to_resting_z() {
let stack_index = 2_usize;
let anim = build_drag_reject_animation(
Vec2::new(640.0, 200.0),
DRAG_Z,
Vec2::new(80.0, -120.0),
stack_index,
);
assert!(
(anim.start_z - DRAG_Z).abs() < 1e-6,
"tween must start at DRAG_Z so the card stays on top during the glide"
);
let expected_end_z = 1.0 + (stack_index as f32) * STACK_FAN_FRAC;
assert!(
(anim.end_z - expected_end_z).abs() < 1e-6,
"tween must end at the slot's resting z, got {} expected {expected_end_z}",
anim.end_z
); );
} }
} }
+5
View File
@@ -333,6 +333,11 @@ pub const MOTION_SHAKE_SECS: f32 = 0.25;
/// Shake angular frequency in rad/s. /// Shake angular frequency in rad/s.
pub const MOTION_SHAKE_OMEGA: f32 = 35.0; pub const MOTION_SHAKE_OMEGA: f32 = 35.0;
/// Duration of the smooth return tween when a drag is rejected by an
/// invalid drop target. Short enough to feel snappy but long enough to
/// read as motion rather than a teleport.
pub const MOTION_DRAG_REJECT_SECS: f32 = 0.15;
/// Card flip — half-time per phase (squash + grow). 100 ms each = /// Card flip — half-time per phase (squash + grow). 100 ms each =
/// 200 ms total. Pair with a ±8° Z-rotation at the midpoint for a 3D /// 200 ms total. Pair with a ±8° Z-rotation at the midpoint for a 3D
/// feel without 3D rendering. /// feel without 3D rendering.