From 42a5f3bc3b671b9636c3bea8f3c781ed15d04699 Mon Sep 17 00:00:00 2001 From: funman300 Date: Mon, 6 Jul 2026 20:37:51 -0700 Subject: [PATCH] =?UTF-8?q?refactor(engine):=20first=20ambiguity=20burn-do?= =?UTF-8?q?wn=20batch=20=E2=80=94=20302=20=E2=86=92=20198=20pairs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two structural fixes from the #143 backlog: - Game-state ordering spine: the three pre-mutation GameStateResource writers (tick_elapsed_time, sync_settings_to_game, handle_restore_prompt) are now a deterministic chain before GameMutation, and the remaining unordered readers (update_selection_hud, handle_hint_button, tick_hint_highlight, handle_right_click, snap_cards_on_window_resize, sync_pile_marker_visibility, auto_save_game_state) are ordered after it. Readers now see the current frame's moves deterministically instead of racing the mutators. - NewGameRequestWriters set: every in-cluster writer of NewGameRequestEvent (buttons, modals, mode picker, seed poller, restore prompt) is registered in a shared set marked ambiguous with itself — writer-vs-writer append order is meaningless since consumers drain the whole queue. Out-of-cluster writers (home, challenge, time-attack, win-summary, play-by-seed, difficulty, stats plugins) can join the set when the test cluster grows. AMBIGUITY_BASELINE ratchets 302 → 198. Remaining backlog is dominated by the Sprite (72) / Transform (48) visual-domain cluster, which needs per-domain set architecture — next batch. Refs #143 Co-Authored-By: Claude Fable 5 --- solitaire_engine/src/card_plugin/mod.rs | 8 ++-- solitaire_engine/src/game_plugin/mod.rs | 57 +++++++++++++++++++++---- solitaire_engine/src/hud_plugin/mod.rs | 22 ++++++---- solitaire_engine/src/schedule_checks.rs | 2 +- solitaire_engine/src/table_plugin.rs | 3 +- 5 files changed, 70 insertions(+), 22 deletions(-) diff --git a/solitaire_engine/src/card_plugin/mod.rs b/solitaire_engine/src/card_plugin/mod.rs index 9f884f2..2e793a6 100644 --- a/solitaire_engine/src/card_plugin/mod.rs +++ b/solitaire_engine/src/card_plugin/mod.rs @@ -570,8 +570,8 @@ impl Plugin for CardPlugin { tick_flip_anim, update_drag_shadow, update_card_shadows_on_drag.after(sync_cards_on_change), - tick_hint_highlight, - handle_right_click, + tick_hint_highlight.after(GameMutation), + handle_right_click.after(GameMutation), tick_right_click_highlights, clear_right_click_highlights_on_state_change.after(GameMutation), clear_right_click_highlights_on_pause, @@ -580,7 +580,9 @@ impl Plugin for CardPlugin { .after(GameMutation) .run_if(resource_changed::), collect_resize_events.after(LayoutSystem::UpdateOnResize), - snap_cards_on_window_resize.after(collect_resize_events), + snap_cards_on_window_resize + .after(collect_resize_events) + .after(GameMutation), ), ); diff --git a/solitaire_engine/src/game_plugin/mod.rs b/solitaire_engine/src/game_plugin/mod.rs index 6f428c9..8826002 100644 --- a/solitaire_engine/src/game_plugin/mod.rs +++ b/solitaire_engine/src/game_plugin/mod.rs @@ -63,6 +63,18 @@ pub struct GameOverScreen; #[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] pub struct GameMutation; +/// System set for every writer of [`crate::events::NewGameRequestEvent`]. +/// +/// Many UI entry points fire this trigger (buttons, keyboard, modals, +/// mode pickers). Their relative append order within a frame is +/// meaningless — consumers drain the whole queue — so members are +/// registered `.in_set(NewGameRequestWriters).ambiguous_with(NewGameRequestWriters)` +/// to declare writer-vs-writer order irrelevant instead of leaving it as an +/// ambiguity (#143). Only ever combine with `.ambiguous_with` on the same +/// set; do NOT hang ordering edges off this set. +#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct NewGameRequestWriters; + /// Persistence path for the in-progress game state file. `None` disables I/O. #[derive(Resource, Debug, Clone)] pub struct GameStatePath(pub Option); @@ -208,7 +220,13 @@ impl Plugin for GamePlugin { .add_message::() // add_message is idempotent; SettingsPlugin also registers this. .add_message::() - .add_systems(Update, poll_pending_new_game_seed.before(GameMutation)) + .add_systems( + Update, + poll_pending_new_game_seed + .before(GameMutation) + .in_set(NewGameRequestWriters) + .ambiguous_with(NewGameRequestWriters), + ) .add_systems( Update, (handle_new_game, handle_draw, handle_move, handle_undo) @@ -217,19 +235,40 @@ impl Plugin for GamePlugin { ) .add_systems(Update, check_no_moves.after(GameMutation)) .add_systems(Update, record_replay_on_win.after(GameMutation)) - .add_systems(Update, handle_confirm_input.after(GameMutation)) - .add_systems(Update, handle_confirm_button_input.after(GameMutation)) - .add_systems(Update, handle_game_over_input.after(GameMutation)) - .add_systems(Update, handle_game_over_button_input.after(GameMutation)) + .add_systems( + Update, + ( + handle_confirm_input, + handle_confirm_button_input, + handle_game_over_input, + handle_game_over_button_input, + ) + .after(GameMutation) + .in_set(NewGameRequestWriters) + .ambiguous_with(NewGameRequestWriters), + ) // Restore prompt: spawn the modal once the splash is gone, // route Continue / New Game intents back into the existing // GameMutation flow. .add_systems(Update, spawn_restore_prompt_if_pending) - .add_systems(Update, handle_restore_prompt.before(GameMutation)) - .add_systems(Update, sync_settings_to_game.before(GameMutation)) + // All three pre-mutation game-state writers are chained: elapsed + // time ticks first, settings sync next, then the restore prompt — + // a deterministic spine instead of three unordered ResMut holders + // (ambiguity burn-down, #143). + .add_systems( + Update, + ( + tick_elapsed_time, + sync_settings_to_game, + handle_restore_prompt + .in_set(NewGameRequestWriters) + .ambiguous_with(NewGameRequestWriters), + ) + .chain() + .before(GameMutation), + ) .init_resource::() - .add_systems(Update, tick_elapsed_time) - .add_systems(Update, auto_save_game_state) + .add_systems(Update, auto_save_game_state.after(GameMutation)) .add_systems(Last, save_game_state_on_exit); } } diff --git a/solitaire_engine/src/hud_plugin/mod.rs b/solitaire_engine/src/hud_plugin/mod.rs index 2961401..21b5f7a 100644 --- a/solitaire_engine/src/hud_plugin/mod.rs +++ b/solitaire_engine/src/hud_plugin/mod.rs @@ -36,7 +36,7 @@ use crate::events::{ UndoRequestEvent, WinStreakMilestoneEvent, }; use crate::font_plugin::FontResource; -use crate::game_plugin::GameMutation; +use crate::game_plugin::{GameMutation, NewGameRequestWriters}; #[cfg(target_os = "android")] use crate::input_plugin::TouchDragSet; use crate::layout::HUD_BAND_HEIGHT; @@ -478,10 +478,12 @@ impl Plugin for HudPlugin { .add_systems(Update, announce_auto_complete.after(GameMutation)) .add_systems( Update, - update_selection_hud.run_if( - resource_exists_and_changed:: - .or(resource_exists_and_changed::), - ), + update_selection_hud + .after(GameMutation) + .run_if( + resource_exists_and_changed:: + .or(resource_exists_and_changed::), + ), ) .add_systems(Update, update_hud_typography) .add_systems( @@ -503,13 +505,17 @@ impl Plugin for HudPlugin { .add_systems( Update, ( - handle_new_game_button, + handle_new_game_button + .in_set(NewGameRequestWriters) + .ambiguous_with(NewGameRequestWriters), handle_undo_button, handle_pause_button, handle_help_button, - handle_hint_button, + handle_hint_button.after(GameMutation), handle_modes_button, - handle_mode_option_click, + handle_mode_option_click + .in_set(NewGameRequestWriters) + .ambiguous_with(NewGameRequestWriters), handle_modes_backdrop_click, close_modes_popover_on_escape, handle_menu_button, diff --git a/solitaire_engine/src/schedule_checks.rs b/solitaire_engine/src/schedule_checks.rs index 2a783f8..a06b12a 100644 --- a/solitaire_engine/src/schedule_checks.rs +++ b/solitaire_engine/src/schedule_checks.rs @@ -38,7 +38,7 @@ mod tests { /// ordering edge — add `.before`/`.after` (order matters) or /// `.ambiguous_with` (provably order-independent) at the registration /// site. When triage lowers the real count, lower this constant too. - const AMBIGUITY_BASELINE: usize = 302; + const AMBIGUITY_BASELINE: usize = 198; fn cluster_app() -> App { let mut app = App::new(); diff --git a/solitaire_engine/src/table_plugin.rs b/solitaire_engine/src/table_plugin.rs index dbe5e69..d3190da 100644 --- a/solitaire_engine/src/table_plugin.rs +++ b/solitaire_engine/src/table_plugin.rs @@ -11,6 +11,7 @@ use solitaire_core::{FOUNDATIONS, TABLEAUS}; use solitaire_core::Suit; use crate::events::{HintVisualEvent, StateChangedEvent}; +use crate::game_plugin::GameMutation; use crate::hud_plugin::HudVisibility; use crate::layout::{ Layout, LayoutResource, LayoutSystem, TABLE_COLOUR, apply_dynamic_tableau_fan, compute_layout, @@ -103,7 +104,7 @@ impl Plugin for TablePlugin { apply_theme_on_settings_change, apply_hint_pile_highlight, tick_hint_pile_highlights, - sync_pile_marker_visibility, + sync_pile_marker_visibility.after(GameMutation), ), ); }