Compare commits

...

2 Commits

Author SHA1 Message Date
funman300 366fd6d127 fix(engine): snap cards directly on window resize
CI / Test & Lint (push) Failing after 19s
CI / Release Build (push) Has been skipped
on_window_resized was firing StateChangedEvent on every WindowResized
event. That ran sync_cards_on_change → update_card_entity, which
inserts a CardAnim slide tween for every card whose target moves >1
unit. During a corner drag the resize fires every frame, retargeting
the slide each time from the cards' current mid-tween positions, so
cards never reach steady state — the visible "snap back and forth"
jitter reported during the 2026-04-29 smoke test.

Replace the StateChangedEvent emit with a direct snap path:

- Add LayoutSystem::UpdateOnResize SystemSet in layout.rs so cross-
  plugin ordering is explicit (Bevy's automatic conflict-based order
  only forces non-parallel execution, not a particular order).
- table_plugin::on_window_resized: drop the StateChangedEvent emit;
  mark the system in_set(LayoutSystem::UpdateOnResize). It already
  snaps backgrounds and pile markers directly, so this aligns cards
  with the same instant-snap policy.
- card_plugin: new snap_cards_on_window_resize system listens for
  WindowResized, runs .after(LayoutSystem::UpdateOnResize), writes
  fresh transforms via the existing card_positions() helper, and
  removes any in-flight CardAnim. It also reapplies the stock-empty
  indicator so the "↺" label's font_size (derived from
  layout.card_size.x) still rescales on resize.

Other StateChangedEvent listeners — start_settle_anim,
detect_auto_complete, clear_selection_on_state_change, check_no_moves,
reset_hint_cycle_on_state_change, clear_right_click_highlights — no
longer fire spuriously on resize. They should not fire on a layout
change anyway; that was a pre-existing minor bug masked by the
jitter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 22:44:08 +00:00
funman300 7a77c66f6d fix(engine): restore card to origin slot after rejected drop
When a drag was rejected, ShakeAnim was inserted on each dragged card
with origin_x = transform.translation.x — the drop-location X, not the
origin pile slot's X. tick_shake_anim restores translation.x to
origin_x at the end of the 0.3s shake, which fights the sync_cards
slide that StateChangedEvent triggers and pins the card at the drop
location. The visible symptom (reported during the 2026-04-29 smoke
test) was "the card returns to the slot beside the pile".

Compute the target X using the existing card_position() helper
against the origin pile and the card's stack_index, then save that as
ShakeAnim::origin_x. The shake now ends with the card at its correct
resting slot. Apply the same fix to both the mouse path (end_drag)
and the touch path (touch_end_drag), and update the existing Task #57
test to reflect the new contract (origin_x = origin slot X, not
drop-location X).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-04-29 22:29:20 +00:00
4 changed files with 144 additions and 35 deletions
+60 -1
View File
@@ -14,6 +14,7 @@ use std::collections::{HashMap, HashSet};
use bevy::color::Color;
use bevy::prelude::*;
use bevy::window::WindowResized;
use solitaire_core::card::{Card, Rank, Suit};
use solitaire_core::game_state::{DrawMode, GameState};
use solitaire_core::pile::PileType;
@@ -23,7 +24,7 @@ use solitaire_core::rules::{can_place_on_foundation, can_place_on_tableau};
use crate::animation_plugin::{CardAnim, EffectiveSlideDuration};
use crate::events::{CardFaceRevealedEvent, CardFlippedEvent, StateChangedEvent};
use crate::game_plugin::GameMutation;
use crate::layout::{Layout, LayoutResource};
use crate::layout::{Layout, LayoutResource, LayoutSystem};
use crate::pause_plugin::PausedResource;
use crate::resources::{DragState, GameStateResource};
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource};
@@ -191,6 +192,7 @@ impl Plugin for CardPlugin {
clear_right_click_highlights_on_state_change.after(GameMutation),
clear_right_click_highlights_on_pause,
update_stock_empty_indicator.after(GameMutation),
snap_cards_on_window_resize.after(LayoutSystem::UpdateOnResize),
),
);
}
@@ -1114,6 +1116,63 @@ fn update_stock_empty_indicator(
);
}
/// Snaps every card sprite to its target position when the window is resized.
///
/// This replaces the old "fire `StateChangedEvent` from `on_window_resized`"
/// path. That path went through `sync_cards_on_change` → `update_card_entity`,
/// which inserts a `CardAnim` slide tween whenever the card moves more than
/// 1 unit. During a corner drag, every frame's `WindowResized` event
/// retargeted the tween from the card's mid-slide position, so cards never
/// reached steady state — the visible "snap back and forth" jitter.
///
/// Cards now snap directly (no slide), matching the instant repositioning
/// already used for backgrounds and pile markers in
/// `table_plugin::on_window_resized`. Any in-flight `CardAnim` is removed so
/// it cannot keep writing the old target translation after the snap.
///
/// The "↺" stock-empty label's `font_size` is derived from
/// `layout.card_size.x`, so this system also reapplies the stock indicator —
/// otherwise the label would not rescale on resize once
/// `update_stock_empty_indicator` stopped firing on resize.
///
/// Scheduled `.after(LayoutSystem::UpdateOnResize)` so `LayoutResource` has
/// been refreshed by `table_plugin::on_window_resized` before this runs.
#[allow(clippy::type_complexity)]
fn snap_cards_on_window_resize(
mut events: MessageReader<WindowResized>,
mut commands: Commands,
game: Option<Res<GameStateResource>>,
layout: Option<Res<LayoutResource>>,
mut entities: Query<(Entity, &CardEntity, &mut Transform)>,
mut pile_markers: Query<(Entity, &PileMarker, &mut Sprite)>,
label_children: Query<(Entity, &ChildOf), With<StockEmptyLabel>>,
) {
if events.read().next().is_none() {
return;
}
let Some(game) = game else { return };
let Some(layout) = layout else { return };
let mut targets: HashMap<u32, (Vec2, f32)> = HashMap::new();
for (card, pos, z) in card_positions(&game.0, &layout.0) {
targets.insert(card.id, (pos, z));
}
for (entity, marker, mut transform) in &mut entities {
if let Some(&(pos, z)) = targets.get(&marker.card_id) {
transform.translation = Vec3::new(pos.x, pos.y, z);
commands.entity(entity).remove::<CardAnim>();
}
}
apply_stock_empty_indicator(
&mut commands,
&game.0,
&mut pile_markers,
&label_children,
&layout.0,
);
}
#[cfg(test)]
mod tests {
use super::*;
+63 -27
View File
@@ -694,16 +694,31 @@ fn end_drag(
count,
});
// Shake each dragged card so the player gets immediate
// visual feedback that the drop was rejected.
for &card_id in &drag.cards {
if let Some((entity, _, transform)) = card_entities
.iter()
.find(|(_, ce, _)| ce.card_id == card_id)
{
commands.entity(entity).insert(ShakeAnim {
elapsed: 0.0,
origin_x: transform.translation.x,
});
// visual feedback that the drop was rejected. ShakeAnim
// restores translation.x to origin_x at the end of the
// animation, so origin_x must be the target slot in the
// origin pile — using the current drag transform would
// pin the card at the drop location and fight the
// sync_cards slide that StateChangedEvent triggers
// (the symptom is "card lands beside the pile").
if let Some(origin_pile) = game.0.piles.get(&origin) {
for &card_id in &drag.cards {
let Some(stack_index) =
origin_pile.cards.iter().position(|c| c.id == card_id)
else {
continue;
};
let target_pos =
card_position(&game.0, &layout.0, &origin, stack_index);
if let Some((entity, _, _)) = card_entities
.iter()
.find(|(_, ce, _)| ce.card_id == card_id)
{
commands.entity(entity).insert(ShakeAnim {
elapsed: 0.0,
origin_x: target_pos.x,
});
}
}
}
}
@@ -911,14 +926,26 @@ fn touch_end_drag(
fired = true;
} else {
rejected.write(MoveRejectedEvent { from: origin.clone(), to: target, count });
for &card_id in &drag.cards {
if let Some((entity, _, transform)) =
card_entities.iter().find(|(_, ce, _)| ce.card_id == card_id)
{
commands.entity(entity).insert(ShakeAnim {
elapsed: 0.0,
origin_x: transform.translation.x,
});
// See `end_drag` (mouse path) for the rationale: ShakeAnim
// restores translation.x to origin_x, so origin_x must be
// the origin pile's slot, not the drop location.
if let Some(origin_pile) = game.0.piles.get(&origin) {
for &card_id in &drag.cards {
let Some(stack_index) =
origin_pile.cards.iter().position(|c| c.id == card_id)
else {
continue;
};
let target_pos =
card_position(&game.0, &layout.0, &origin, stack_index);
if let Some((entity, _, _)) =
card_entities.iter().find(|(_, ce, _)| ce.card_id == card_id)
{
commands.entity(entity).insert(ShakeAnim {
elapsed: 0.0,
origin_x: target_pos.x,
});
}
}
}
}
@@ -2025,7 +2052,11 @@ mod tests {
/// Verifies that `ShakeAnim` constructed for a rejected drag has the
/// correct initial values: `elapsed` starts at 0.0 and `origin_x` matches
/// the card's current transform X position.
/// the **target slot in the origin pile** (where the card will rest after
/// the rejection). Saving the drop-location X here was the root cause of
/// the "card lands beside the pile" bug — `tick_shake_anim` restores
/// `translation.x` to `origin_x` at the end of the shake, fighting the
/// `sync_cards` slide that `StateChangedEvent` triggers.
///
/// The Bevy ECS part (Commands + Query) is exercised at runtime; this test
/// covers the data path — that we build the component with the right values
@@ -2034,14 +2065,18 @@ mod tests {
fn shake_anim_for_rejected_drag_has_correct_initial_values() {
use crate::feedback_anim_plugin::ShakeAnim;
// Simulate the transform X that a dragged card would have at the
// moment the drag is released (could be anywhere on screen).
let current_x = 123.5_f32;
// 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`.
// 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: current_x,
origin_x: target_slot_x,
};
assert_eq!(
@@ -2049,9 +2084,10 @@ mod tests {
"ShakeAnim must start with elapsed=0.0 so the animation plays from the beginning"
);
assert!(
(anim.origin_x - current_x).abs() < 1e-6,
"ShakeAnim origin_x must match the card's transform X at drop time, \
expected {current_x}, got {}",
(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
);
}
+13 -1
View File
@@ -6,10 +6,22 @@
use std::collections::HashMap;
use bevy::math::Vec2;
use bevy::prelude::Resource;
use bevy::prelude::{Resource, SystemSet};
use solitaire_core::card::Suit;
use solitaire_core::pile::PileType;
/// Schedule labels for layout-related systems so cross-plugin ordering is
/// explicit instead of relying on Bevy's automatic resource-conflict ordering
/// (which only forces non-parallel execution, not a particular order).
#[derive(SystemSet, Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum LayoutSystem {
/// The system that updates [`LayoutResource`], the table background, and
/// pile markers in response to a `WindowResized` event. Card-snap systems
/// (in `card_plugin`) run `.after(LayoutSystem::UpdateOnResize)` so they
/// see the fresh layout.
UpdateOnResize,
}
/// Minimum supported window dimensions. Layout is still computed below this
/// size but cards will be small.
pub const MIN_WINDOW: Vec2 = Vec2::new(800.0, 600.0);
+8 -6
View File
@@ -10,7 +10,7 @@ use solitaire_core::card::Suit;
use solitaire_core::pile::PileType;
use crate::events::{HintVisualEvent, StateChangedEvent};
use crate::layout::{compute_layout, Layout, LayoutResource};
use crate::layout::{compute_layout, Layout, LayoutResource, LayoutSystem};
#[cfg(test)]
use crate::layout::TABLE_COLOUR;
use crate::settings_plugin::{SettingsChangedEvent, SettingsResource};
@@ -69,7 +69,7 @@ impl Plugin for TablePlugin {
.add_systems(
Update,
(
on_window_resized,
on_window_resized.in_set(LayoutSystem::UpdateOnResize),
apply_theme_on_settings_change,
apply_hint_pile_highlight,
tick_hint_pile_highlights,
@@ -275,12 +275,10 @@ fn spawn_pile_markers(commands: &mut Commands, layout: &Layout) {
}
}
#[allow(clippy::too_many_arguments)]
#[allow(clippy::type_complexity)]
fn on_window_resized(
mut events: MessageReader<WindowResized>,
mut layout_res: Option<ResMut<LayoutResource>>,
mut state_changed: MessageWriter<StateChangedEvent>,
mut backgrounds: Query<
(&mut Sprite, &mut Transform),
(With<TableBackground>, Without<PileMarker>),
@@ -311,8 +309,12 @@ fn on_window_resized(
}
}
// Reposition card sprites to the new layout.
state_changed.write(StateChangedEvent);
// Card sprites are repositioned by `card_plugin::snap_cards_on_window_resize`
// running `.after(LayoutSystem::UpdateOnResize)` — that system snaps card
// transforms directly to the new layout instead of going through
// `StateChangedEvent → sync_cards → CardAnim` which would retarget the
// slide tween every frame during a corner drag (the visible "snap back
// and forth" jitter).
}
// ---------------------------------------------------------------------------