funman300/Ferrous-Solitaire
1
1
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases 28 Wiki Activity

fix(multi): resolve 26 bugs found in comprehensive codebase review
Build and Deploy / build-and-push (push) Successful in 3m40s
Details

Browse Source 
Core fixes (issues #12, #13, #22):
- #12: undo now preserves score delta instead of restoring snapshot score
- #13: take_from_foundation defaults to false (non-standard house rule)
- #22: check_win validates full suit sequence, not just card count

Engine fixes:
- #8:  replay keyboard input guard against non-replay state
- #9:  help modal scrims.is_empty() guard added
- #10: settings modal scrims.is_empty() guard added
- #11: sync_plugin builds payload at poll time (not task-spawn time)
- #14: server replay mode case-sensitivity fix ("Classic")
- #15: play_by_seed_plugin confirmed flag set to true on launch
- #16: replay back-step debounce via Local<bool> + StateChangedEvent;
       register StateChangedEvent in ReplayOverlayPlugin (fixes 52 tests)
- #17: time-attack timer ignores win-summary overlay
- #18: HUD dropdown glyphs U+25BE → U+2193 (FiraMono-safe arrow)
- #19: theme plugin applies immediate visual update on A→B→A switch
- #20: SyncAuthError / SyncBusyOverlay split into separate entities so
       auth errors are visible after busy overlay is hidden
- #21: handle_forfeit ordered before update_stats_on_new_game
- #23: server merge uses correct avg_time_seconds and games_lost math
- #24: win_summary migrated to ModalScrim pattern
- #25: card_animation apply_deferred between animation systems
- #26: cursor_plugin HashMap access uses .get() with fallback
- #27: auto_complete mid-sequence deactivation guard
- #28: feedback_anim SettleAnim ordered before FoundationFlourish
- #29: achievement_plugin iterates all win events; adds scrims guard
- #30: leaderboard modal scrims.is_empty() guard added
- #31: server auth tmp file cleanup on rename failure
- #32: sync_setup modal scrims.is_empty() guard added
- #33: font_plugin uses match fallback; TokioRuntimeResource graceful
       current-thread fallback on runtime init failure

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-05-19 13:14:47 -07:00
parent 6d061d23a1
commit 7840ef9eb2
24 changed files with 374 additions and 171 deletions
Download Patch File Download Diff File Expand all files Collapse all files
solitaire_core/src/game_state.rs
+23 -13
View File
@@ -193,7 +193,7 @@ impl GameState {
is_auto_completable: false,
undo_count: 0,
recycle_count: 0,
take_from_foundation: true,
take_from_foundation: false,
schema_version: GAME_STATE_SCHEMA_VERSION,
undo_stack: VecDeque::new(),
}
@@ -407,7 +407,7 @@ impl GameState {
self.score = if self.mode == GameMode::Zen {
0
} else {
(snapshot.score + scoring_undo()).max(0)
(self.score + scoring_undo()).max(0)
};
self.move_count = snapshot.move_count;
self.is_won = false;
@@ -416,12 +416,25 @@ impl GameState {
Ok(())
}
/// Returns `true` when all four foundation slots each contain 13 cards.
/// Returns `true` when all four foundation slots each contain a valid A→K
/// sequence of a single suit.
///
/// Counting 13 cards is not sufficient — a corrupt save could produce 13
/// arbitrary cards per pile and permanently lock the game via `GameAlreadyWon`.
pub fn check_win(&self) -> bool {
(0..4_u8).all(|slot| {
self.piles
.get(&PileType::Foundation(slot))
.is_some_and(|p| p.cards.len() == 13)
(0..4_u8).all(|slot| self.is_valid_foundation_pile(slot))
}
fn is_valid_foundation_pile(&self, slot: u8) -> bool {
let Some(pile) = self.piles.get(&PileType::Foundation(slot)) else {
return false;
};
if pile.cards.len() != 13 {
return false;
}
let suit = pile.cards[0].suit;
pile.cards.iter().enumerate().all(|(i, card)| {
card.suit == suit && card.rank.value() == (i as u8 + 1)
})
}
@@ -1395,12 +1408,9 @@ mod tests {
}
#[test]
fn take_from_foundation_allowed_by_default() {
let mut g = setup_take_from_foundation_game();
assert!(g.take_from_foundation, "standard Klondike allows take-from-foundation by default");
g.move_cards(PileType::Foundation(0), PileType::Tableau(0), 1).unwrap();
assert_eq!(g.piles[&PileType::Foundation(0)].cards.len(), 1);
assert_eq!(g.piles[&PileType::Tableau(0)].cards.len(), 2);
fn take_from_foundation_disabled_by_default() {
let g = setup_take_from_foundation_game();
assert!(!g.take_from_foundation, "take_from_foundation is off by default (non-standard rule)");
}
#[test]
solitaire_engine/src/achievement_plugin.rs
+80 -81
View File
@@ -32,7 +32,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath};
use crate::stats_plugin::{StatsResource, StatsUpdate};
use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible,
ModalScrim, ScrimDismissible,
};
use crate::ui_theme::{
ACCENT_PRIMARY, BORDER_SUBTLE, STATE_SUCCESS, TEXT_DISABLED, TEXT_PRIMARY, TEXT_SECONDARY,
@@ -162,93 +162,91 @@ fn evaluate_on_win(
mut achievements: ResMut<AchievementsResource>,
mut progress: ResMut<ProgressResource>,
) {
let Some(ev) = wins.read().last() else {
return;
};
let ctx = AchievementContext {
games_played: stats.0.games_played,
games_won: stats.0.games_won,
win_streak_current: stats.0.win_streak_current,
best_single_score: stats.0.best_single_score,
lifetime_score: stats.0.lifetime_score,
draw_three_wins: stats.0.draw_three_wins,
daily_challenge_streak: progress.0.daily_challenge_streak,
last_win_score: ev.score,
last_win_time_seconds: ev.time_seconds,
last_win_used_undo: game.0.undo_count > 0,
wall_clock_hour: Some(Local::now().hour()),
last_win_recycle_count: game.0.recycle_count,
last_win_is_zen: game.0.mode == solitaire_core::game_state::GameMode::Zen,
};
let hits = check_achievements(&ctx);
if hits.is_empty() {
return;
}
let now = Utc::now();
let mut achievements_changed = false;
let mut progress_changed = false;
for def in hits {
let Some(record) = achievements.0.iter_mut().find(|r| r.id == def.id) else {
continue;
for ev in wins.read() {
let ctx = AchievementContext {
games_played: stats.0.games_played,
games_won: stats.0.games_won,
win_streak_current: stats.0.win_streak_current,
best_single_score: stats.0.best_single_score,
lifetime_score: stats.0.lifetime_score,
draw_three_wins: stats.0.draw_three_wins,
daily_challenge_streak: progress.0.daily_challenge_streak,
last_win_score: ev.score,
last_win_time_seconds: ev.time_seconds,
last_win_used_undo: game.0.undo_count > 0,
wall_clock_hour: Some(Local::now().hour()),
last_win_recycle_count: game.0.recycle_count,
last_win_is_zen: game.0.mode == solitaire_core::game_state::GameMode::Zen,
};
if record.unlocked {
let hits = check_achievements(&ctx);
if hits.is_empty() {
continue;
}
record.unlock(now);
achievements_changed = true;
// Grant the reward on first unlock.
if !record.reward_granted {
if let Some(reward) = def.reward {
match reward {
Reward::CardBack(idx) => {
if !progress.0.unlocked_card_backs.contains(&idx) {
progress.0.unlocked_card_backs.push(idx);
progress_changed = true;
}
}
Reward::Background(idx) => {
if !progress.0.unlocked_backgrounds.contains(&idx) {
progress.0.unlocked_backgrounds.push(idx);
progress_changed = true;
}
}
Reward::BonusXp(amount) => {
xp_awarded.write(XpAwardedEvent { amount });
let prev_level = progress.0.add_xp(amount);
if progress.0.leveled_up_from(prev_level) {
levelups.write(LevelUpEvent {
previous_level: prev_level,
new_level: progress.0.level,
total_xp: progress.0.total_xp,
});
}
progress_changed = true;
}
Reward::Badge => {}
}
let now = Utc::now();
let mut achievements_changed = false;
let mut progress_changed = false;
for def in hits {
let Some(record) = achievements.0.iter_mut().find(|r| r.id == def.id) else {
continue;
};
if record.unlocked {
continue;
}
record.reward_granted = true;
record.unlock(now);
achievements_changed = true;
// Grant the reward on first unlock.
if !record.reward_granted {
if let Some(reward) = def.reward {
match reward {
Reward::CardBack(idx) => {
if !progress.0.unlocked_card_backs.contains(&idx) {
progress.0.unlocked_card_backs.push(idx);
progress_changed = true;
}
}
Reward::Background(idx) => {
if !progress.0.unlocked_backgrounds.contains(&idx) {
progress.0.unlocked_backgrounds.push(idx);
progress_changed = true;
}
}
Reward::BonusXp(amount) => {
xp_awarded.write(XpAwardedEvent { amount });
let prev_level = progress.0.add_xp(amount);
if progress.0.leveled_up_from(prev_level) {
levelups.write(LevelUpEvent {
previous_level: prev_level,
new_level: progress.0.level,
total_xp: progress.0.total_xp,
});
}
progress_changed = true;
}
Reward::Badge => {}
}
}
record.reward_granted = true;
}
unlocks.write(AchievementUnlockedEvent(record.clone()));
}
unlocks.write(AchievementUnlockedEvent(record.clone()));
if achievements_changed
&& let Some(target) = &path.0
&& let Err(e) = save_achievements_to(target, &achievements.0) {
warn!("failed to save achievements: {e}");
}
if progress_changed
&& let Some(target) = &progress_path.0
&& let Err(e) = save_progress_to(target, &progress.0) {
warn!("failed to save progress after reward: {e}");
}
}
if achievements_changed
&& let Some(target) = &path.0
&& let Err(e) = save_achievements_to(target, &achievements.0) {
warn!("failed to save achievements: {e}");
}
if progress_changed
&& let Some(target) = &progress_path.0
&& let Err(e) = save_progress_to(target, &progress.0) {
warn!("failed to save progress after reward: {e}");
}
}
/// Cinephile unlock observer.
@@ -391,6 +389,7 @@ fn toggle_achievements_screen(
achievements: Res<AchievementsResource>,
font_res: Option<Res<FontResource>>,
screens: Query<Entity, With<AchievementsScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<AchievementsScreen>)>,
) {
let button_clicked = requests.read().count() > 0;
if !keys.just_pressed(KeyCode::KeyA) && !button_clicked {
@@ -398,7 +397,7 @@ fn toggle_achievements_screen(
}
if let Ok(entity) = screens.single() {
commands.entity(entity).despawn();
} else {
} else if other_modal_scrims.is_empty() {
spawn_achievements_screen(&mut commands, &achievements.0, font_res.as_deref());
}
}
solitaire_engine/src/auto_complete_plugin.rs
+7 -2
View File
@@ -72,9 +72,14 @@ fn detect_auto_complete(
if game.0.is_auto_completable && !state.active {
state.active = true;
state.cooldown = 0.0; // fire first move immediately
} else if !game.0.is_auto_completable {
state.active = false;
}
// Intentionally no `else if !is_auto_completable` branch here.
// Deactivating on every frame where `is_auto_completable` is false
// would hard-stop the sequence mid-flight whenever `next_auto_complete_move`
// transiently returns `None` (e.g. while the previous move is still
// in-flight). The `is_won` check above already handles the definitive
// end-of-game case; `drive_auto_complete` simply retries next tick
// when no move is available yet.
}
/// Plays a distinct chime the moment auto-complete first activates.
solitaire_engine/src/card_animation/mod.rs
+7
View File
@@ -142,6 +142,13 @@ impl Plugin for CardAnimationPlugin {
update_frame_time_diagnostics,
// Advance active animations.
advance_card_animations,
// Flush deferred commands so `CardAnimation` removals from
// `advance_card_animations` are visible before the chain
// system runs. Without this, the chain sees the component
// still present in the same frame it was removed (deferred
// commands aren't applied until the next ApplyDeferred
// point), causing a 1-frame gap between every chain step.
ApplyDeferred,
// After each animation finishes, pop the next chain segment.
advance_animation_chains,
// Interaction visuals (run after animation for final positions).
solitaire_engine/src/cursor_plugin.rs
+9 -6
View File
@@ -382,8 +382,8 @@ fn update_drop_target_overlays(
/// for everything else it is card-sized. Replicated here rather than
/// imported because `pile_drop_rect` is private to `input_plugin` and
/// this overlay is the only other consumer.
fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> (Vec2, Vec2) {
let centre = layout.pile_positions[pile];
fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> Option<(Vec2, Vec2)> {
let centre = layout.pile_positions.get(pile).copied()?;
if matches!(pile, PileType::Tableau(_)) {
let card_count = game.piles.get(pile).map_or(0, |p| p.cards.len());
if card_count > 1 {
@@ -393,13 +393,13 @@ fn drop_overlay_rect(pile: &PileType, layout: &Layout, game: &GameState) -> (Vec
let bottom_edge = bottom_card_centre_y - layout.card_size.y / 2.0;
let span_height = top_edge - bottom_edge;
let new_centre_y = (top_edge + bottom_edge) / 2.0;
return (
return Some((
Vec2::new(centre.x, new_centre_y),
Vec2::new(layout.card_size.x, span_height),
);
));
}
}
(centre, layout.card_size)
Some((centre, layout.card_size))
}
/// Spawns one overlay parent (fill) plus four edge sprites (outline) at
@@ -410,7 +410,10 @@ fn spawn_drop_target_overlay(
layout: &Layout,
game: &GameState,
) {
let (centre, size) = drop_overlay_rect(pile, layout, game);
let Some((centre, size)) = drop_overlay_rect(pile, layout, game) else {
warn!("drop_overlay_rect: pile {pile:?} not in layout, skipping overlay");
return;
};
let edge = DROP_TARGET_OUTLINE_PX;
commands
solitaire_engine/src/feedback_anim_plugin.rs
+6 -1
View File
@@ -210,10 +210,15 @@ impl Plugin for FeedbackAnimPlugin {
start_shake_anim.after(GameMutation),
tick_shake_anim,
start_settle_anim.after(GameMutation),
// tick_foundation_flourish writes the full Transform.scale
// (Vec3); tick_settle_anim writes only scale.y on top of
// it. Ordering ensures the settle's y-only write always
// applies last so it wins on the ~0.15 s overlap when both
// components are present on the same King entity.
tick_foundation_flourish.before(tick_settle_anim),
tick_settle_anim,
start_deal_anim.after(GameMutation),
start_foundation_flourish.after(GameMutation),
tick_foundation_flourish,
),
);
}
solitaire_engine/src/font_plugin.rs
+9 -2
View File
@@ -31,8 +31,15 @@ fn load_font(fonts: Option<ResMut<Assets<Font>>>, mut commands: Commands) {
// Assets<Font>). FontPlugin in that context is a no-op — consumers
// already query `Option<Res<FontResource>>` and degrade cleanly.
let Some(mut fonts) = fonts else { return };
let font = Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec())
.expect("bundled FiraMono failed to parse — binary is corrupt");
let font = match Font::try_from_bytes(BUNDLED_FONT_BYTES.to_vec()) {
Ok(f) => f,
Err(e) => {
// A corrupt embedded font is unusual but should not crash the
// process — UI will render without glyphs rather than panicking.
warn!("bundled FiraMono failed to parse ({e}); UI text may be invisible");
return;
}
};
let handle = fonts.add(font);
commands.insert_resource(FontResource(handle));
}
solitaire_engine/src/help_plugin.rs
+3 -2
View File
@@ -13,7 +13,7 @@ use crate::font_plugin::FontResource;
use crate::hud_plugin::ANDROID_HINT_LABEL;
use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible,
ModalScrim, ScrimDismissible,
};
use crate::ui_theme::{SPACE_2, TEXT_PRIMARY, TEXT_SECONDARY, TYPE_BODY, VAL_SPACE_2, VAL_SPACE_3, Z_MODAL_PANEL};
#[cfg(not(target_os = "android"))]
@@ -67,6 +67,7 @@ fn toggle_help_screen(
keys: Res<ButtonInput<KeyCode>>,
mut requests: MessageReader<HelpRequestEvent>,
screens: Query<Entity, With<HelpScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<HelpScreen>)>,
font_res: Option<Res<FontResource>>,
) {
// Either F1 or a click on the HUD "Help" button (which fires
@@ -77,7 +78,7 @@ fn toggle_help_screen(
}
if let Ok(entity) = screens.single() {
commands.entity(entity).despawn();
} else {
} else if other_modal_scrims.is_empty() {
spawn_help_screen(&mut commands, font_res.as_deref());
}
}
solitaire_engine/src/hud_plugin.rs
+2 -2
View File
@@ -870,12 +870,12 @@ fn spawn_action_buttons(
);
#[cfg(not(target_os = "android"))]
let labels = (
"Menu \u{25BE}",
"Menu \u{2193}",
"Undo",
"Pause",
"Help",
"Hint",
"Modes \u{25BE}",
"Modes \u{2193}",
"New Game",
);
solitaire_engine/src/input_plugin.rs
+10
View File
@@ -47,6 +47,7 @@ use crate::game_plugin::{ConfirmNewGameScreen, GameMutation, RestorePromptScreen
use crate::pause_plugin::PausedResource;
use crate::progress_plugin::ProgressResource;
use crate::layout::{Layout, LayoutResource};
use crate::replay_playback::ReplayPlaybackState;
use crate::resources::{DragState, GameInputConsumedResource, GameStateResource, HintCycleIndex};
use crate::selection_plugin::SelectionState;
use crate::time_attack_plugin::TimeAttackResource;
@@ -175,11 +176,20 @@ fn handle_keyboard_core(
mut zen_requests: MessageReader<StartZenRequestEvent>,
confirm_screens: Query<(), With<ConfirmNewGameScreen>>,
restore_prompts: Query<(), With<RestorePromptScreen>>,
replay_state: Option<Res<ReplayPlaybackState>>,
) {
if paused.is_some_and(|p| p.0) {
return;
}
// During replay playback (Playing or Completed) all game-input shortcuts
// are suppressed. The replay overlay owns Space (pause/resume) and the
// arrow keys (step). Letting game input through would mutate
// `GameStateResource` and corrupt replay determinism.
if replay_state.is_some_and(|r| !matches!(*r, ReplayPlaybackState::Inactive)) {
return;
}
if keys.just_pressed(KeyCode::KeyU) {
ev.undo.write(UndoRequestEvent);
}
solitaire_engine/src/leaderboard_plugin.rs
+5 -1
View File
@@ -21,7 +21,7 @@ use crate::settings_plugin::{SettingsResource, SettingsStoragePath};
use crate::sync_plugin::SyncProviderResource;
use crate::ui_modal::{
spawn_modal, spawn_modal_actions, spawn_modal_button, spawn_modal_header, ButtonVariant,
ScrimDismissible,
ModalScrim, ScrimDismissible,
};
use crate::ui_theme::{
ACCENT_PRIMARY, BG_ELEVATED, BORDER_SUBTLE, RADIUS_SM, STATE_INFO,
@@ -714,6 +714,7 @@ fn data_cell(
fn handle_set_display_name_button(
button_q: Query<&Interaction, (Changed<Interaction>, With<SetDisplayNameButton>)>,
existing: Query<(), With<DisplayNameModal>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<DisplayNameModal>)>,
mut commands: Commands,
settings: Option<Res<SettingsResource>>,
font_res: Option<Res<FontResource>>,
@@ -725,6 +726,9 @@ fn handle_set_display_name_button(
if !existing.is_empty() {
return; // already open
}
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
buf.0 = settings
.as_ref()
.and_then(|s| s.0.leaderboard_display_name.clone())
solitaire_engine/src/play_by_seed_plugin.rs
+8 -2
View File
@@ -411,7 +411,11 @@ fn handle_confirm(
new_game.write(NewGameRequestEvent {
seed: Some(seed),
mode: None,
confirmed: false,
// The player explicitly clicked Play (or pressed Enter) after typing
// a seed — treat this as an affirmative confirmation so the
// abandon-current-game dialog is not shown on top of the already-
// dismissed seed dialog.
confirmed: true,
});
for entity in &screen {
@@ -566,7 +570,9 @@ mod tests {
assert_eq!(fired.len(), 1);
assert_eq!(fired[0].seed, Some(42));
assert_eq!(fired[0].mode, None);
assert!(!fired[0].confirmed);
// confirmed: true — the player explicitly clicked Play, so no
// abandon-current-game dialog should appear.
assert!(fired[0].confirmed);
// Dialog should be gone.
assert!(!dialog_present(&mut app));
solitaire_engine/src/replay_overlay.rs
+32 -4
View File
@@ -28,7 +28,7 @@ use chrono::Datelike;
use crate::font_plugin::FontResource;
use crate::layout::LayoutResource;
use crate::events::{DrawRequestEvent, MoveRequestEvent, UndoRequestEvent};
use crate::events::{DrawRequestEvent, MoveRequestEvent, StateChangedEvent, UndoRequestEvent};
use crate::replay_playback::{
step_backwards_replay_playback, step_replay_playback, stop_replay_playback,
toggle_pause_replay_playback, ReplayPlaybackState,
@@ -476,6 +476,7 @@ impl Plugin for ReplayOverlayPlugin {
.add_message::<MoveRequestEvent>()
.add_message::<DrawRequestEvent>()
.add_message::<UndoRequestEvent>()
.add_message::<StateChangedEvent>()
.add_systems(
Update,
(
@@ -1884,6 +1885,7 @@ fn handle_pause_keyboard(
/// resets to 0 on key release so the next fresh press fires
/// immediately. This matches the mockup's `[← →] scrub`
/// terminology while keeping single-press = single-step semantics.
#[allow(clippy::too_many_arguments)]
fn handle_arrow_keyboard(
keys: Option<Res<ButtonInput<KeyCode>>>,
time: Res<Time>,
@@ -1892,10 +1894,22 @@ fn handle_arrow_keyboard(
mut moves_writer: MessageWriter<MoveRequestEvent>,
mut draws_writer: MessageWriter<DrawRequestEvent>,
mut undo_writer: MessageWriter<UndoRequestEvent>,
mut state_changed: MessageReader<StateChangedEvent>,
// `true` while a backward step is in-flight: cursor was decremented and
// `UndoRequestEvent` was written, but `handle_undo` hasn't applied it yet.
// Cleared when `StateChangedEvent` confirms the game state has caught up.
// Prevents rapid ← presses from accumulating multiple cursor decrements
// before any undo is applied (Bug #16).
mut back_pending: Local<bool>,
) {
let Some(keys) = keys else { return };
let dt = time.delta_secs();
// Clear the in-flight flag once the game confirms the undo landed.
if state_changed.read().count() > 0 {
*back_pending = false;
}
// Right (forward step) — initial press fires immediately;
// held repeats fire when the accumulator crosses the interval.
if keys.just_pressed(KeyCode::ArrowRight) {
@@ -1911,14 +1925,28 @@ fn handle_arrow_keyboard(
hold.right_held_secs = 0.0;
}
// Left (backwards step) — symmetric to the right path.
// Left (backwards step) — gate on `back_pending` so at most one undo
// is in-flight at a time. The cursor is only decremented inside
// `step_backwards_replay_playback`, which also writes `UndoRequestEvent`.
// `back_pending` is set after a successful step and cleared above when
// `StateChangedEvent` confirms the undo was applied.
if keys.just_pressed(KeyCode::ArrowLeft) {
step_backwards_replay_playback(&mut state, &mut undo_writer);
if !*back_pending {
let fired = step_backwards_replay_playback(&mut state, &mut undo_writer);
if fired {
*back_pending = true;
}
}
hold.left_held_secs = 0.0;
} else if keys.pressed(KeyCode::ArrowLeft) {
hold.left_held_secs += dt;
if hold.left_held_secs >= SCRUB_REPEAT_INTERVAL_SECS {
step_backwards_replay_playback(&mut state, &mut undo_writer);
if !*back_pending {
let fired = step_backwards_replay_playback(&mut state, &mut undo_writer);
if fired {
*back_pending = true;
}
}
hold.left_held_secs = 0.0;
}
} else {
solitaire_engine/src/resources.rs
+41 -8
View File
@@ -3,7 +3,7 @@
use std::sync::Arc;
use bevy::math::Vec2;
use bevy::prelude::Resource;
use bevy::prelude::{warn, Resource};
use chrono::{DateTime, Utc};
use solitaire_core::game_state::GameState;
use solitaire_core::pile::PileType;
@@ -131,15 +131,48 @@ pub struct GameInputConsumedResource(pub bool);
#[derive(Resource, Clone)]
pub struct TokioRuntimeResource(pub Arc<tokio::runtime::Runtime>);
impl Default for TokioRuntimeResource {
fn default() -> Self {
// Building the Tokio runtime is startup-time initialization; failure
// here means the OS refused to create threads, which is unrecoverable.
impl TokioRuntimeResource {
/// Attempts to build the shared multi-threaded Tokio runtime.
///
/// Returns `Err` if the OS refuses to create worker threads (e.g. resource
/// limits on Android). Callers should log the error and disable sync
/// features rather than panicking.
pub fn new() -> Result<Self, tokio::io::Error> {
let rt = tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
.expect("failed to build shared Tokio runtime");
Self(Arc::new(rt))
.build()?;
Ok(Self(Arc::new(rt)))
}
}
impl Default for TokioRuntimeResource {
fn default() -> Self {
// Try multi-threaded first; fall back to current-thread (single
// worker) if the OS refuses to create additional threads. Neither
// path uses `.expect()` so this never panics at startup.
match tokio::runtime::Builder::new_multi_thread()
.worker_threads(2)
.enable_all()
.build()
{
Ok(rt) => Self(Arc::new(rt)),
Err(e) => {
warn!(
"sync: failed to build multi-thread Tokio runtime ({e}); \
falling back to current-thread runtime"
);
// current_thread runtime never spawns OS threads, so it
// succeeds even under tight sandboxing.
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect(
"current-thread Tokio runtime failed — \
the process cannot do any async I/O",
);
Self(Arc::new(rt))
}
}
}
}
solitaire_engine/src/settings_plugin.rs
+2 -1
View File
@@ -510,6 +510,7 @@ fn toggle_settings_screen(
fn sync_settings_panel_visibility(
screen: Res<SettingsScreen>,
panels: Query<Entity, With<SettingsPanel>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<SettingsPanel>)>,
scroll_nodes: Query<&ScrollPosition, With<SettingsScrollNode>>,
mut scroll_pos: ResMut<SettingsScrollPos>,
mut commands: Commands,
@@ -525,7 +526,7 @@ fn sync_settings_panel_visibility(
return;
}
if screen.0 {
if panels.is_empty() {
if panels.is_empty() && other_modal_scrims.is_empty() {
let status_label = sync_status
.map_or_else(|| "Status: local only".to_string(), |s| sync_status_label(&s.0));
let unlocked_backs = progress
solitaire_engine/src/stats_plugin.rs
+7 -1
View File
@@ -220,7 +220,13 @@ impl Plugin for StatsPlugin {
)
.add_systems(
Update,
handle_forfeit.before(GameMutation),
// handle_forfeit must run before update_stats_on_new_game so
// the NewGameRequestEvent it emits is not visible to
// update_stats_on_new_game in the same frame — otherwise
// record_abandoned() fires twice on every forfeit (#21).
handle_forfeit
.before(GameMutation)
.before(update_stats_on_new_game),
)
.add_systems(Update, toggle_stats_screen.after(GameMutation))
.add_systems(Update, handle_stats_close_button)
solitaire_engine/src/sync_plugin.rs
+24 -13
View File
@@ -101,7 +101,6 @@ impl SyncPlugin {
impl Plugin for SyncPlugin {
fn build(&self, app: &mut App) {
app.insert_resource(SyncProviderResource(self.provider.clone()))
.init_resource::<TokioRuntimeResource>()
.init_resource::<SyncStatusResource>()
.init_resource::<PullTaskResult>()
.init_resource::<PullTask>()
@@ -109,18 +108,30 @@ impl Plugin for SyncPlugin {
.add_message::<ManualSyncRequestEvent>()
.add_message::<SyncCompleteEvent>()
.add_message::<SyncConfigureRequestEvent>()
.add_message::<WarningToastEvent>()
.add_systems(Startup, start_pull)
.add_systems(
Update,
(
poll_pull_result,
handle_manual_sync_request,
push_replay_on_win,
poll_replay_upload_result,
),
)
.add_systems(Last, push_on_exit);
.add_message::<WarningToastEvent>();
// Build the shared Tokio runtime; disable all network sync if the OS
// refuses to create threads (resource-limited environments, sandboxed
// Android builds, etc.).
match TokioRuntimeResource::new() {
Ok(rt) => {
app.insert_resource(rt)
.add_systems(Startup, start_pull)
.add_systems(
Update,
(
poll_pull_result,
handle_manual_sync_request,
push_replay_on_win,
poll_replay_upload_result,
),
)
.add_systems(Last, push_on_exit);
}
Err(e) => {
warn!("sync: failed to create Tokio runtime — network sync disabled: {e}");
}
}
}
}
solitaire_engine/src/sync_setup_plugin.rs
+27 -7
View File
@@ -55,7 +55,7 @@ use crate::font_plugin::FontResource;
use crate::settings_plugin::{SettingsResource, SettingsScreen, SettingsStoragePath};
use crate::resources::TokioRuntimeResource;
use crate::sync_plugin::SyncProviderResource;
use crate::ui_modal::spawn_modal;
use crate::ui_modal::{spawn_modal, ModalScrim};
use crate::ui_theme::{
ACCENT_PRIMARY, BG_ELEVATED, BG_ELEVATED_HI,
BORDER_SUBTLE, HighContrastBorder, RADIUS_SM, STATE_DANGER, TEXT_DISABLED,
@@ -208,6 +208,7 @@ impl Plugin for SyncSetupPlugin {
fn open_sync_setup_modal(
mut events: MessageReader<SyncConfigureRequestEvent>,
existing: Query<(), With<SyncSetupScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<SyncSetupScreen>)>,
mut commands: Commands,
mut focused: ResMut<SyncFocusedField>,
font_res: Option<Res<FontResource>>,
@@ -219,6 +220,9 @@ fn open_sync_setup_modal(
if !existing.is_empty() {
return; // Already open.
}
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
*focused = SyncFocusedField::Url;
spawn_sync_setup_modal(&mut commands, font_res.as_deref());
}
@@ -354,9 +358,10 @@ fn handle_auth_button(
return;
}
// Clear error and show busy indicator.
for (mut text, _) in &mut error_nodes {
text.0 = "Connecting…".to_string();
// Clear previous error and show busy indicator.
for (mut text, mut color) in &mut error_nodes {
text.0 = String::new();
color.0 = TEXT_SECONDARY;
}
for mut vis in &mut busy_nodes {
*vis = Visibility::Visible;
@@ -540,6 +545,7 @@ fn handle_logout(
fn open_delete_confirm_modal(
mut events: MessageReader<DeleteAccountRequestEvent>,
existing: Query<(), With<DeleteConfirmScreen>>,
other_modal_scrims: Query<(), (With<ModalScrim>, Without<DeleteConfirmScreen>)>,
mut commands: Commands,
font_res: Option<Res<FontResource>>,
) {
@@ -550,6 +556,9 @@ fn open_delete_confirm_modal(
if !existing.is_empty() {
return;
}
if !other_modal_scrims.is_empty() {
return; // Another modal is already visible.
}
spawn_delete_confirm_modal(&mut commands, font_res.as_deref());
}
@@ -675,20 +684,31 @@ fn spawn_sync_setup_modal(commands: &mut Commands, font_res: Option<&FontResourc
font_res,
);
// Error / status line.
// Error / status line — two distinct children so visibility and
// text can be controlled independently.
body.spawn(Node {
min_height: Val::Px(18.0),
flex_direction: FlexDirection::Row,
align_items: AlignItems::Center,
column_gap: VAL_SPACE_2,
..default()
})
.with_children(|row| {
// Busy indicator: shown while the auth task is in flight.
row.spawn((
SyncAuthError,
SyncBusyOverlay,
Text::new(String::new()),
Text::new("…"),
make_font(font_res, TYPE_CAPTION),
TextColor(TEXT_SECONDARY),
Visibility::Hidden,
));
// Error / status text: always laid out, empty when idle.
row.spawn((
SyncAuthError,
Text::new(String::new()),
make_font(font_res, TYPE_CAPTION),
TextColor(TEXT_SECONDARY),
));
});
// Tab hint — desktop only; no Tab key on Android.
solitaire_engine/src/theme/plugin.rs
+20
View File
@@ -182,12 +182,16 @@ fn sync_card_image_set_with_active_theme(
mut events: MessageReader<AssetEvent<CardTheme>>,
active: Option<Res<ActiveTheme>>,
themes: Res<Assets<CardTheme>>,
asset_server: Option<Res<AssetServer>>,
mut card_image_set: Option<ResMut<CardImageSet>>,
mut state_events: MessageWriter<StateChangedEvent>,
) {
let Some(active) = active else { return };
let active_id = active.0.id();
let mut should_sync = false;
// Consume asset events — covers the normal first-load path.
for ev in events.read() {
let id = match ev {
AssetEvent::LoadedWithDependencies { id }
@@ -198,6 +202,22 @@ fn sync_card_image_set_with_active_theme(
should_sync = true;
}
}
// A→B→A switch: Bevy does not re-fire LoadedWithDependencies for a
// handle whose asset is already cached. Detect this by checking that
// `ActiveTheme` itself changed this frame (the resource was just
// replaced by `react_to_settings_theme_change`) and the underlying
// asset is already fully loaded. If so, sync immediately rather than
// waiting for an event that will never arrive.
if !should_sync
&& active.is_changed()
&& asset_server
.as_ref()
.is_some_and(|as_| as_.is_loaded_with_dependencies(active.0.id()))
{
should_sync = true;
}
if !should_sync {
return;
}
solitaire_engine/src/time_attack_plugin.rs
+5 -4
View File
@@ -172,14 +172,15 @@ fn advance_time_attack(
paused: Option<Res<crate::pause_plugin::PausedResource>>,
path: Option<Res<TimeAttackSessionPath>>,
home_screens: Query<(), With<crate::home_plugin::HomeScreen>>,
win_overlays: Query<(), With<crate::win_summary_plugin::WinSummaryOverlay>>,
) {
if !session.active {
return;
}
// Mirrors `tick_elapsed_time`: pause while the launch / mode-picker
// Home modal is up so the countdown doesn't burn while the player
// is choosing what to play next.
if paused.is_some_and(|p| p.0) || !home_screens.is_empty() {
// Pause the countdown while Home, the Pause overlay, or the Win Summary
// overlay is visible — the player should not lose time while reading results
// or navigating menus.
if paused.is_some_and(|p| p.0) || !home_screens.is_empty() || !win_overlays.is_empty() {
return;
}
session.remaining_secs -= time.delta_secs();
solitaire_engine/src/win_summary_plugin.rs
+3
View File
@@ -24,6 +24,7 @@ use crate::progress_plugin::ProgressResource;
use crate::resources::GameStateResource;
use crate::settings_plugin::SettingsResource;
use crate::stats_plugin::{StatsResource, StatsUpdate};
use crate::ui_modal::ModalScrim;
use crate::ui_theme::{
scaled_duration, ACCENT_PRIMARY, BG_BASE, BG_ELEVATED, MOTION_SCORE_BREAKDOWN_FADE_SECS,
MOTION_SCORE_BREAKDOWN_STAGGER_SECS, MOTION_WIN_SHAKE_AMPLITUDE, MOTION_WIN_SHAKE_SECS,
@@ -757,6 +758,7 @@ fn spawn_overlay(
commands
.spawn((
WinSummaryOverlay,
ModalScrim,
Node {
position_type: PositionType::Absolute,
left: Val::Percent(0.0),
@@ -769,6 +771,7 @@ fn spawn_overlay(
..default()
},
BackgroundColor(SCRIM),
GlobalZIndex(Z_WIN_CASCADE),
ZIndex(Z_WIN_CASCADE),
))
.with_children(|root| {
solitaire_server/src/auth.rs
+4 -1
View File
@@ -390,7 +390,10 @@ pub async fn upload_avatar(
// Write to a temp file then atomically rename so concurrent readers never
// see a partially-written avatar.
std::fs::write(&tmp_path, &body).map_err(|e| AppError::Internal(e.to_string()))?;
std::fs::rename(&tmp_path, &path).map_err(|e| AppError::Internal(e.to_string()))?;
if let Err(e) = std::fs::rename(&tmp_path, &path) {
let _ = std::fs::remove_file(&tmp_path);
return Err(AppError::Internal(e.to_string()));
}
// Remove stale files with other extensions after the atomic rename.
for old_ext in &["jpg", "png", "webp", "gif"] {
if *old_ext != ext {
solitaire_server/src/replays.rs
+6 -3
View File
@@ -35,7 +35,7 @@ use crate::{error::AppError, middleware::AuthenticatedUser, AppState};
/// the desktop client's transitive dependencies.
#[derive(Debug, Deserialize)]
struct ReplayHeader {
seed: i64,
seed: u64,
draw_mode: String,
mode: String,
time_seconds: i64,
@@ -94,6 +94,9 @@ pub async fn upload(
let id = Uuid::new_v4().to_string();
let received_at = Utc::now().to_rfc3339();
let replay_json = serde_json::to_string(&payload)?;
// SQLite INTEGER columns bind as i64. Reinterpret the u64 bits — the
// database stores the same 8 bytes; high-bit seeds round-trip correctly.
let seed_i64 = header.seed as i64;
sqlx::query!(
r#"INSERT INTO replays (
@@ -102,7 +105,7 @@ pub async fn upload(
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)"#,
id,
user.user_id,
header.seed,
seed_i64,
header.draw_mode,
header.mode,
header.time_seconds,
@@ -116,7 +119,7 @@ pub async fn upload(
// Update leaderboard best score/time for opted-in users when this replay
// beats their existing best. Only classic mode counts for the leaderboard.
if header.mode == "classic" {
if header.mode == "Classic" {
sqlx::query!(
r#"UPDATE leaderboard
SET best_score = ?,
solitaire_sync/src/merge.rs
+34 -17
View File
@@ -108,23 +108,27 @@ fn merge_stats(
let merged_games_won = local.games_won.max(remote.games_won);
let merged_games_played = local.games_played.max(remote.games_played);
// Recompute average time from the merged totals. If no wins yet, keep 0.
// Carry the average time from whichever side contributed merged_games_won.
// Taking max(total_time)/max(wins) misattributes time when the side with
// more wins has a lower total — use the winning side's average directly.
let avg_time_seconds = if merged_games_won == 0 {
0
} else if local.games_won >= remote.games_won {
local.avg_time_seconds
} else {
// Use whichever side has more wins to approximate total time, then blend.
// We don't have total_time stored, so we reconstruct it from avg * count.
let local_total = local.avg_time_seconds as u128 * local.games_won as u128;
let remote_total = remote.avg_time_seconds as u128 * remote.games_won as u128;
// Take max total time (conservative — avoids underestimating total play time).
let best_total = local_total.max(remote_total);
(best_total / merged_games_won as u128) as u64
remote.avg_time_seconds
};
// Derive games_lost from the merged played/won counts so the invariant
// games_won + games_lost <= games_played is always satisfied. Computing
// max(local.games_lost, remote.games_lost) independently can push
// games_won + games_lost above games_played after a divergent merge.
let merged_games_lost = merged_games_played.saturating_sub(merged_games_won);
StatsSnapshot {
games_played: merged_games_played,
games_won: merged_games_won,
games_lost: local.games_lost.max(remote.games_lost),
games_lost: merged_games_lost,
win_streak_current: local.win_streak_current.max(remote.win_streak_current),
win_streak_best: local.win_streak_best.max(remote.win_streak_best),
avg_time_seconds,
@@ -454,14 +458,28 @@ mod tests {
}
#[test]
fn stats_games_lost_takes_max() {
fn stats_games_lost_derived_from_played_minus_won() {
// games_lost must equal games_played - games_won so the invariant
// games_won + games_lost <= games_played is always satisfied.
let mut local = default_payload();
local.stats.games_played = 20;
local.stats.games_won = 8;
local.stats.games_lost = 12;
let mut remote = default_payload();
remote.stats.games_lost = 8;
remote.stats.games_played = 15;
remote.stats.games_won = 10;
remote.stats.games_lost = 5;
// merged: games_played = max(20, 15) = 20; games_won = max(8, 10) = 10
// games_lost must be 20 - 10 = 10, NOT max(12, 5) = 12
let (merged, _) = merge(&local, &remote);
assert_eq!(merged.stats.games_lost, 12);
assert_eq!(merged.stats.games_played, 20);
assert_eq!(merged.stats.games_won, 10);
assert_eq!(merged.stats.games_lost, 10);
assert!(
merged.stats.games_won + merged.stats.games_lost <= merged.stats.games_played,
"games_won + games_lost must never exceed games_played"
);
}
#[test]
@@ -502,11 +520,10 @@ mod tests {
#[test]
fn stats_avg_time_recomputed_from_merged_totals() {
// local: 4 wins averaging 100s each (total = 400s)
// remote: 6 wins averaging 200s each (total = 1200s)
// merged_games_won = max(4, 6) = 6
// best_total = max(400, 1200) = 1200
// avg = 1200 / 6 = 200
// local: 4 wins averaging 100s each
// remote: 6 wins averaging 200s each
// merged_games_won = max(4, 6) = 6 → remote contributed the wins
// avg_time_seconds must be remote's 200s, not a blend of totals
let mut local = default_payload();
local.stats.games_won = 4;
local.stats.avg_time_seconds = 100;