fix(multi): resolve 14 bugs from second comprehensive review
Build and Deploy / build-and-push (push) Successful in 4m2s

Core (solitaire_core):
- fix(scoring): apply -15 penalty for Foundation→Tableau moves when
  take_from_foundation is enabled; update test
- fix(solver): is_won() validates full Ace→King suit sequence, not
  just card count — prevents hint system from emitting invalid paths

Engine — animation / layout:
- fix(animation): guard CardAnim advance against duration=0 to prevent
  NaN-poisoned Transform (analogous to CardAnimation's instant-snap path)
- fix(card_plugin): align TABLEAU_FAN_FRAC (0.25→0.18) and
  TABLEAU_FACEDOWN_FAN_FRAC (0.20→0.14) with layout.rs so the initial
  layout and first dynamic update produce identical fan spacing
- fix(layout): update tableau_fan_frac doc comment from 0.25→0.18

Engine — ECS / modal guards:
- fix(auto_complete): drive_auto_complete now checks PausedResource so
  cooldown does not tick while paused (prevents instant-move on unpause)
- fix(play_by_seed): handle_open_dialog checks global ModalScrim guard
  to prevent stacking over an existing modal
- fix(win_summary): spawn_win_summary_after_delay checks global
  ModalScrim guard; collect_session_achievements uses .next() not
  .last() to avoid draining the new_games stream

Engine — message registration:
- fix(leaderboard): register InfoToastEvent in LeaderboardPlugin::build
  so opt-in/opt-out toasts work under MinimalPlugins
- fix(replay_playback): register StateChangedEvent in
  ReplayPlaybackPlugin::build to prevent panic when used standalone

Security:
- fix(sync_setup): zero password SyncFieldBuffer immediately after
  spawning auth task — credential must not linger in ECS components

Server:
- fix(auth): replace MIME contains-chain with exact match for avatar
  upload; removes illusory starts_with guard and dead ALLOWED_IMAGE_TYPES

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-19 13:40:32 -07:00
parent a328059933
commit ea9dd848fd
12 changed files with 59 additions and 37 deletions
+9 -8
View File
@@ -9,9 +9,11 @@ use crate::pile::PileType;
pub fn score_move(from: &PileType, to: &PileType) -> i32 { pub fn score_move(from: &PileType, to: &PileType) -> i32 {
match to { match to {
PileType::Foundation(_) => 10, PileType::Foundation(_) => 10,
PileType::Tableau(_) => { PileType::Tableau(_) => match from {
if matches!(from, PileType::Waste) { 5 } else { 0 } PileType::Waste => 5,
} PileType::Foundation(_) => -15,
_ => 0,
},
_ => 0, _ => 0,
} }
} }
@@ -71,13 +73,12 @@ mod tests {
} }
#[test] #[test]
fn non_waste_to_tableau_scores_zero() { fn foundation_to_tableau_penalises_fifteen() {
// Foundation → Tableau is impossible in practice but must score 0. // Moving a card back off a foundation (take_from_foundation rule) costs -15.
assert_eq!(score_move(&PileType::Foundation(0), &PileType::Tableau(0)), 0); assert_eq!(score_move(&PileType::Foundation(0), &PileType::Tableau(0)), -15);
// Tableau → Tableau (restack) scores 0.
assert_eq!(score_move(&PileType::Tableau(1), &PileType::Tableau(2)), 0);
} }
#[test] #[test]
fn move_to_stock_or_waste_scores_zero() { fn move_to_stock_or_waste_scores_zero() {
// These destinations are illegal moves in practice, but the function // These destinations are illegal moves in practice, but the function
+9 -2
View File
@@ -298,9 +298,16 @@ impl SolverState {
} }
} }
/// True when every foundation slot has 13 cards. /// True when every foundation slot holds a complete Ace-through-King sequence.
fn is_won(&self) -> bool { fn is_won(&self) -> bool {
self.foundation.iter().all(|f| f.len() == 13) self.foundation.iter().all(|pile| {
pile.len() == 13
&& pile[0].rank == crate::card::Rank::Ace
&& pile.windows(2).all(|w| {
w[0].suit == w[1].suit
&& w[1].rank.value() == w[0].rank.value() + 1
})
})
} }
/// Returns the foundation slot that already claims `suit`, or the /// Returns the foundation slot that already claims `suit`, or the
+5
View File
@@ -258,6 +258,11 @@ fn advance_card_anims(
anim.delay = (anim.delay - dt).max(0.0); anim.delay = (anim.delay - dt).max(0.0);
continue; continue;
} }
if anim.duration <= 0.0 {
transform.translation = anim.target;
commands.entity(entity).remove::<CardAnim>();
continue;
}
anim.elapsed += dt; anim.elapsed += dt;
let t = (anim.elapsed / anim.duration).min(1.0); let t = (anim.elapsed / anim.duration).min(1.0);
// Curved interpolation using `MotionCurve::SmoothSnap` (cubic ease-out // Curved interpolation using `MotionCurve::SmoothSnap` (cubic ease-out
@@ -13,6 +13,7 @@ use bevy::prelude::*;
use crate::audio_plugin::{AudioState, SoundLibrary}; use crate::audio_plugin::{AudioState, SoundLibrary};
use crate::events::{MoveRequestEvent, StateChangedEvent}; use crate::events::{MoveRequestEvent, StateChangedEvent};
use crate::game_plugin::GameMutation; use crate::game_plugin::GameMutation;
use crate::pause_plugin::PausedResource;
use crate::resources::GameStateResource; use crate::resources::GameStateResource;
/// Volume amplitude used for the auto-complete activation chime. /// Volume amplitude used for the auto-complete activation chime.
@@ -111,11 +112,15 @@ fn drive_auto_complete(
mut state: ResMut<AutoCompleteState>, mut state: ResMut<AutoCompleteState>,
game: Res<GameStateResource>, game: Res<GameStateResource>,
time: Res<Time>, time: Res<Time>,
paused: Option<Res<PausedResource>>,
mut moves: MessageWriter<MoveRequestEvent>, mut moves: MessageWriter<MoveRequestEvent>,
) { ) {
if !state.active { if !state.active {
return; return;
} }
if paused.is_some_and(|p| p.0) {
return;
}
state.cooldown -= time.delta_secs(); state.cooldown -= time.delta_secs();
if state.cooldown > 0.0 { if state.cooldown > 0.0 {
+5 -3
View File
@@ -41,7 +41,9 @@ use crate::ui_theme::{
}; };
/// Fraction of card height used as vertical offset between face-up tableau cards. /// Fraction of card height used as vertical offset between face-up tableau cards.
pub const TABLEAU_FAN_FRAC: f32 = 0.25; /// Must match `layout::TABLEAU_FAN_FRAC` so the initial layout and the first
/// dynamic update from `update_tableau_fan_frac` produce identical spacing.
pub const TABLEAU_FAN_FRAC: f32 = 0.18;
/// Per-card vertical step for face-down tableau cards, as a fraction of /// Per-card vertical step for face-down tableau cards, as a fraction of
/// card height. Smaller than [`TABLEAU_FAN_FRAC`] because face-down cards /// card height. Smaller than [`TABLEAU_FAN_FRAC`] because face-down cards
@@ -51,11 +53,11 @@ pub const TABLEAU_FAN_FRAC: f32 = 0.25;
/// renderer creates a visible offset between the card face and where /// renderer creates a visible offset between the card face and where
/// clicks land. /// clicks land.
/// ///
/// Matches `layout::TABLEAU_FACEDOWN_FAN_FRAC` (0.20). Both constants must /// Matches `layout::TABLEAU_FACEDOWN_FAN_FRAC` (0.14). Both constants must
/// stay in sync; the layout constant drives the adaptive LayoutResource value /// stay in sync; the layout constant drives the adaptive LayoutResource value
/// used at runtime, while this one is the minimum floor used by /// used at runtime, while this one is the minimum floor used by
/// `update_tableau_fan_frac` when computing proportional updates. /// `update_tableau_fan_frac` when computing proportional updates.
pub const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.20; pub const TABLEAU_FACEDOWN_FAN_FRAC: f32 = 0.14;
/// 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. Public so other plugins /// non-tableau piles, so stacking is visible. Public so other plugins
+1 -1
View File
@@ -123,7 +123,7 @@ pub struct Layout {
pub pile_positions: HashMap<PileType, Vec2>, pub pile_positions: HashMap<PileType, Vec2>,
/// Per-step vertical offset fraction for face-up tableau cards, as a /// Per-step vertical offset fraction for face-up tableau cards, as a
/// fraction of `card_size.y`. On height-limited (desktop) windows this /// fraction of `card_size.y`. On height-limited (desktop) windows this
/// equals `TABLEAU_FAN_FRAC` (0.25); on width-limited (portrait phone) /// equals `TABLEAU_FAN_FRAC` (0.18); on width-limited (portrait phone)
/// windows it expands to fill the available vertical space so the tableau /// windows it expands to fill the available vertical space so the tableau
/// stretches to the bottom of the screen. Card rendering (`card_plugin`) /// stretches to the bottom of the screen. Card rendering (`card_plugin`)
/// and hit testing (`input_plugin`) both read from this field so they /// and hit testing (`input_plugin`) both read from this field so they
@@ -139,6 +139,7 @@ impl Plugin for LeaderboardPlugin {
.init_resource::<DisplayNameBuffer>() .init_resource::<DisplayNameBuffer>()
.add_message::<ToggleLeaderboardRequestEvent>() .add_message::<ToggleLeaderboardRequestEvent>()
.add_message::<WarningToastEvent>() .add_message::<WarningToastEvent>()
.add_message::<InfoToastEvent>()
// `MouseWheel` and `KeyboardInput` are emitted by Bevy's input // `MouseWheel` and `KeyboardInput` are emitted by Bevy's input
// plugin under `DefaultPlugins`; register them explicitly so all // plugin under `DefaultPlugins`; register them explicitly so all
// leaderboard systems run cleanly under `MinimalPlugins` in tests. // leaderboard systems run cleanly under `MinimalPlugins` in tests.
+3 -2
View File
@@ -138,12 +138,13 @@ fn handle_open_dialog(
mut requests: MessageReader<StartPlayBySeedRequestEvent>, mut requests: MessageReader<StartPlayBySeedRequestEvent>,
font_res: Option<Res<FontResource>>, font_res: Option<Res<FontResource>>,
existing: Query<(), With<PlayBySeedScreen>>, existing: Query<(), With<PlayBySeedScreen>>,
other_scrims: Query<(), (With<crate::ui_modal::ModalScrim>, Without<PlayBySeedScreen>)>,
) { ) {
if requests.read().count() == 0 { if requests.read().count() == 0 {
return; return;
} }
// Guard against double-spawn (e.g. two events in one frame). // Guard against double-spawn (e.g. two events in one frame) or stacking over another modal.
if !existing.is_empty() { if !existing.is_empty() || !other_scrims.is_empty() {
return; return;
} }
let font = font_res.as_deref(); let font = font_res.as_deref();
+1
View File
@@ -512,6 +512,7 @@ pub struct ReplayPlaybackPlugin;
impl Plugin for ReplayPlaybackPlugin { impl Plugin for ReplayPlaybackPlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
app.init_resource::<ReplayPlaybackState>() app.init_resource::<ReplayPlaybackState>()
.add_message::<StateChangedEvent>()
.add_systems( .add_systems(
Update, Update,
( (
+9 -1
View File
@@ -305,7 +305,7 @@ fn update_field_borders(
fn handle_auth_button( fn handle_auth_button(
login_q: Query<&Interaction, (Changed<Interaction>, With<SyncLoginButton>)>, login_q: Query<&Interaction, (Changed<Interaction>, With<SyncLoginButton>)>,
register_q: Query<&Interaction, (Changed<Interaction>, With<SyncRegisterButton>)>, register_q: Query<&Interaction, (Changed<Interaction>, With<SyncRegisterButton>)>,
fields: Query<(&SyncFieldKind, &SyncFieldBuffer)>, mut fields: Query<(&SyncFieldKind, &mut SyncFieldBuffer)>,
rt: Res<TokioRuntimeResource>, rt: Res<TokioRuntimeResource>,
mut pending: ResMut<PendingAuthTask>, mut pending: ResMut<PendingAuthTask>,
mut error_nodes: Query<(&mut Text, &mut TextColor), With<SyncAuthError>>, mut error_nodes: Query<(&mut Text, &mut TextColor), With<SyncAuthError>>,
@@ -392,6 +392,14 @@ fn handle_auth_button(
pending.task = Some(task); pending.task = Some(task);
pending.url = url; pending.url = url;
pending.username = username; pending.username = username;
// Zero the password buffer immediately — it must not linger in ECS
// components after the credential has been handed off to the async task.
for (kind, mut buf) in &mut fields {
if *kind == SyncFieldKind::Password {
buf.0.clear();
}
}
} }
/// Polls the in-flight auth task. On success updates settings + provider. /// Polls the in-flight auth task. On success updates settings + provider.
+4 -3
View File
@@ -508,7 +508,7 @@ fn collect_session_achievements(
) { ) {
// Reset on any new-game request (including mode switches via Z/X/C/T) so // Reset on any new-game request (including mode switches via Z/X/C/T) so
// achievements from the previous session are not carried into the next one. // achievements from the previous session are not carried into the next one.
if new_games.read().last().is_some() { if new_games.read().next().is_some() {
session.names.clear(); session.names.clear();
} }
for ev in unlocks.read() { for ev in unlocks.read() {
@@ -539,6 +539,7 @@ fn spawn_win_summary_after_delay(
settings: Option<Res<SettingsResource>>, settings: Option<Res<SettingsResource>>,
time: Res<Time>, time: Res<Time>,
overlays: Query<Entity, With<WinSummaryOverlay>>, overlays: Query<Entity, With<WinSummaryOverlay>>,
other_scrims: Query<(), (With<ModalScrim>, Without<WinSummaryOverlay>)>,
mut delay: Local<Option<f32>>, mut delay: Local<Option<f32>>,
) { ) {
// Process new win events. // Process new win events.
@@ -569,8 +570,8 @@ fn spawn_win_summary_after_delay(
*remaining -= time.delta_secs(); *remaining -= time.delta_secs();
if *remaining <= 0.0 { if *remaining <= 0.0 {
*delay = None; *delay = None;
// Only spawn if there is no overlay already. // Only spawn if no overlay of any kind is already visible.
if overlays.is_empty() { if overlays.is_empty() && other_scrims.is_empty() {
// Drain any XpAwardedEvents that arrived this frame but were // Drain any XpAwardedEvents that arrived this frame but were
// not yet consumed by `cache_win_data` (which may run later in // not yet consumed by `cache_win_data` (which may run later in
// the same schedule). Accumulating here ensures the modal // the same schedule). Accumulating here ensures the modal
+7 -17
View File
@@ -341,8 +341,6 @@ pub async fn get_me(
})) }))
} }
/// Allowed MIME types for uploaded avatars.
const ALLOWED_IMAGE_TYPES: &[&str] = &["image/jpeg", "image/png", "image/webp", "image/gif"];
/// Maximum avatar upload size in bytes (1 MB). /// Maximum avatar upload size in bytes (1 MB).
const AVATAR_MAX_BYTES: usize = 1024 * 1024; const AVATAR_MAX_BYTES: usize = 1024 * 1024;
@@ -361,23 +359,15 @@ pub async fn upload_avatar(
.and_then(|v| v.to_str().ok()) .and_then(|v| v.to_str().ok())
.unwrap_or("") .unwrap_or("")
.to_string(); .to_string();
let ext = if mime.contains("jpeg") || mime.contains("jpg") { let ext = match mime.as_str() {
"jpg" "image/jpeg" | "image/jpg" => "jpg",
} else if mime.contains("png") { "image/png" => "png",
"png" "image/webp" => "webp",
} else if mime.contains("webp") { "image/gif" => "gif",
"webp" _ => return Err(AppError::BadRequest(
} else if mime.contains("gif") {
"gif"
} else {
return Err(AppError::BadRequest(
"avatar must be image/jpeg, image/png, image/webp, or image/gif".into(), "avatar must be image/jpeg, image/png, image/webp, or image/gif".into(),
)); )),
}; };
if !ALLOWED_IMAGE_TYPES.iter().any(|t| mime.starts_with(t)) {
return Err(AppError::BadRequest("unsupported image type".into()));
}
if body.len() > AVATAR_MAX_BYTES { if body.len() > AVATAR_MAX_BYTES {
return Err(AppError::BadRequest("avatar must be ≤ 1 MB".into())); return Err(AppError::BadRequest("avatar must be ≤ 1 MB".into()));
} }