From b402c01918b38b38d9a22ca45ce7ff7b70b93302 Mon Sep 17 00:00:00 2001 From: funman300 Date: Mon, 6 Jul 2026 21:13:58 -0700 Subject: [PATCH] =?UTF-8?q?refactor(engine):=20ambiguity=20burn-down=20bat?= =?UTF-8?q?ch=204=20=E2=80=94=20zero=20ambiguities,=20gate=20enforced?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears the final 47 pairs and turns the ratchet into a hard gate (AMBIGUITY_BASELINE = 0, assert_eq): - HudButtons: the 14 HUD button/popover handlers run as one chain, before ui_focus::FocusKeys — Esc/keyboard consumption order is now defined (restore prompt → buttons/popovers → focus navigation → settings toggle) instead of scheduler-dependent. - HUD text updaters (update_hud, update_selection_hud, update_won_previously) chained, in UiTextFx, after the new AutoComplete set (update_hud reads AutoCompleteState). - restore_hud_on_modal → apply_hud_visibility chained before UpdateOnResize: HudVisibility writes, application, and the layout read happen in a fixed order. - New writer sets UndoRequestWriters / InfoToastWriters (same self-ambiguous pattern as NewGameRequestWriters). - Logic-before-paint: check_no_moves and the AutoComplete chain order before BoardVisuals; SettingsMutation after UpdateOnResize. - MarkerVisuals set wraps the table painter chain; chrome fx declare disjointness from it. - update_hud_typography after BoardVisuals; avatar/settings-toggle interaction handlers declared disjoint from HudButtons. 302 → 198 → 171 → 47 → 0 in four batches, one day. New systems now fail CI unless they declare their ordering or their disjointness. Closes #143 Co-Authored-By: Claude Fable 5 --- solitaire_engine/src/auto_complete_plugin.rs | 9 ++- solitaire_engine/src/game_plugin/mod.rs | 29 ++++++-- solitaire_engine/src/hud_plugin/mod.rs | 71 +++++++++++++++----- solitaire_engine/src/schedule_checks.rs | 25 ++++--- solitaire_engine/src/settings_plugin/mod.rs | 11 ++- solitaire_engine/src/table_plugin.rs | 8 +++ solitaire_engine/src/ui_focus.rs | 17 ++++- solitaire_engine/src/ui_modal.rs | 3 +- 8 files changed, 136 insertions(+), 37 deletions(-) diff --git a/solitaire_engine/src/auto_complete_plugin.rs b/solitaire_engine/src/auto_complete_plugin.rs index b73efae..033b77d 100644 --- a/solitaire_engine/src/auto_complete_plugin.rs +++ b/solitaire_engine/src/auto_complete_plugin.rs @@ -46,6 +46,11 @@ pub struct AutoCompleteState { /// Plugin that drives the auto-complete sequence. pub struct AutoCompletePlugin; +/// Set wrapping the auto-complete detect/drive chain; HUD readers of +/// [`AutoCompleteState`] order themselves after it (#143). +#[derive(bevy::ecs::schedule::SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct AutoComplete; + impl Plugin for AutoCompletePlugin { fn build(&self, app: &mut App) { app.init_resource::() @@ -58,7 +63,9 @@ impl Plugin for AutoCompletePlugin { drive_auto_complete, ) .chain() - .after(GameMutation), + .in_set(AutoComplete) + .after(GameMutation) + .before(crate::card_plugin::BoardVisuals), ); } } diff --git a/solitaire_engine/src/game_plugin/mod.rs b/solitaire_engine/src/game_plugin/mod.rs index 83baa47..1f4ee8d 100644 --- a/solitaire_engine/src/game_plugin/mod.rs +++ b/solitaire_engine/src/game_plugin/mod.rs @@ -75,6 +75,17 @@ pub struct GameMutation; #[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] pub struct NewGameRequestWriters; +/// Self-ambiguous set for writers of `UndoRequestEvent` — same rationale as +/// [`NewGameRequestWriters`]: consumers drain the queue, append order is +/// meaningless (#143). +#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct UndoRequestWriters; + +/// Self-ambiguous set for writers of `InfoToastEvent` — toasts queue in +/// arrival order and any same-frame order is fine (#143). +#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct InfoToastWriters; + /// Persistence path for the in-progress game state file. `None` disables I/O. #[derive(Resource, Debug, Clone)] pub struct GameStatePath(pub Option); @@ -233,7 +244,14 @@ impl Plugin for GamePlugin { .chain() .in_set(GameMutation), ) - .add_systems(Update, check_no_moves.after(GameMutation)) + .add_systems( + Update, + check_no_moves + .after(GameMutation) + .before(crate::card_plugin::BoardVisuals) + .in_set(InfoToastWriters) + .ambiguous_with(InfoToastWriters), + ) .add_systems(Update, record_replay_on_win.after(GameMutation)) .add_systems( Update, @@ -244,14 +262,16 @@ impl Plugin for GamePlugin { handle_game_over_button_input, ) .after(GameMutation) + .before(crate::ui_focus::FocusKeys) .in_set(NewGameRequestWriters) - .ambiguous_with(NewGameRequestWriters), + .ambiguous_with(NewGameRequestWriters) + .in_set(UndoRequestWriters) + .ambiguous_with(UndoRequestWriters), ) // 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) - // All three pre-mutation game-state writers are chained: elapsed + // All 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). @@ -260,6 +280,7 @@ impl Plugin for GamePlugin { ( tick_elapsed_time, sync_settings_to_game, + spawn_restore_prompt_if_pending, handle_restore_prompt .in_set(NewGameRequestWriters) .ambiguous_with(NewGameRequestWriters), diff --git a/solitaire_engine/src/hud_plugin/mod.rs b/solitaire_engine/src/hud_plugin/mod.rs index 8c1d4cd..1f8f1ed 100644 --- a/solitaire_engine/src/hud_plugin/mod.rs +++ b/solitaire_engine/src/hud_plugin/mod.rs @@ -154,6 +154,13 @@ pub struct HudColumn; #[derive(Component, Debug)] pub struct HudActionBar; +/// Set wrapping the chained HUD button/popover interaction systems. Other +/// keyboard consumers order themselves around it (e.g. +/// [`crate::ui_focus::FocusKeys`] runs after) so input-consumption order is +/// deterministic (#143). +#[derive(bevy::ecs::schedule::SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct HudButtons; + /// Marker on the text node inside each touch-layout action-bar button. /// Used by `resize_action_bar_labels` to update font size on window resize. #[derive(Component, Debug)] @@ -468,31 +475,56 @@ impl Plugin for HudPlugin { // defensively so the HUD plugin works standalone in tests. .add_message::() .add_systems(Startup, (spawn_hud_band, spawn_hud, spawn_action_buttons, spawn_hud_avatar)) - .add_systems(Update, update_hud.after(GameMutation)) + // HUD text updaters run as one deterministic chain (they write + // disjoint Text nodes, but Bevy can't prove it); update_hud also + // reads AutoCompleteState, so the chain sits after the + // auto-complete detect/drive chain (#143). .add_systems( Update, - apply_hud_visibility.before(LayoutSystem::UpdateOnResize), + ( + update_hud, + update_selection_hud.run_if( + resource_exists_and_changed:: + .or(resource_exists_and_changed::), + ), + update_won_previously, + ) + .chain() + .after(GameMutation) + .after(crate::auto_complete_plugin::AutoComplete) + .in_set(UiTextFx) + .ambiguous_with(UiTextFx), + ) + // HUD chrome visibility: modal-restore writes HudVisibility, the + // applier consumes it, and the layout recompute reads it — a + // fixed chain instead of three racing systems (#143). + .add_systems( + Update, + (restore_hud_on_modal, apply_hud_visibility) + .chain() + .before(LayoutSystem::UpdateOnResize), ) - .add_systems(Update, restore_hud_on_modal) .add_systems( Update, ( update_hud_avatar.after(crate::settings_plugin::SettingsMutation), - handle_avatar_button, + handle_avatar_button.ambiguous_with(HudButtons), ), ) - .add_systems(Update, update_won_previously.after(GameMutation)) - .add_systems(Update, announce_auto_complete.after(GameMutation)) .add_systems( Update, - update_selection_hud + announce_auto_complete .after(GameMutation) - .run_if( - resource_exists_and_changed:: - .or(resource_exists_and_changed::), - ), + .after(crate::auto_complete_plugin::AutoComplete) + .in_set(crate::game_plugin::InfoToastWriters) + .ambiguous_with(crate::game_plugin::InfoToastWriters), + ) + // Typography rescale touches HUD TextFont only, but orders after + // the board painters that resize card/label text (#143). + .add_systems( + Update, + update_hud_typography.after(crate::card_plugin::BoardVisuals), ) - .add_systems(Update, update_hud_typography) .add_systems( Update, ( @@ -521,10 +553,16 @@ impl Plugin for HudPlugin { handle_new_game_button .in_set(NewGameRequestWriters) .ambiguous_with(NewGameRequestWriters), - handle_undo_button, + handle_undo_button + .in_set(crate::game_plugin::UndoRequestWriters) + .ambiguous_with(crate::game_plugin::UndoRequestWriters) + .before(GameMutation), handle_pause_button, handle_help_button, - handle_hint_button.after(GameMutation), + handle_hint_button + .after(GameMutation) + .in_set(crate::game_plugin::InfoToastWriters) + .ambiguous_with(crate::game_plugin::InfoToastWriters), handle_modes_button, handle_mode_option_click .in_set(NewGameRequestWriters) @@ -536,7 +574,10 @@ impl Plugin for HudPlugin { handle_menu_backdrop_click, close_menu_popover_on_escape, paint_action_buttons, - ), + ) + .chain() + .in_set(HudButtons) + .before(crate::ui_focus::FocusKeys), ) // Fade lives in `Last` so it always overrides whatever the // hover/paint pass set on `BackgroundColor` this frame. diff --git a/solitaire_engine/src/schedule_checks.rs b/solitaire_engine/src/schedule_checks.rs index 378d5e7..a384600 100644 --- a/solitaire_engine/src/schedule_checks.rs +++ b/solitaire_engine/src/schedule_checks.rs @@ -32,13 +32,14 @@ mod tests { use crate::ui_focus::UiFocusPlugin; use crate::ui_modal::UiModalPlugin; - /// Legacy ambiguity backlog measured 2026-07-06 (issue #143). This - /// number may only decrease. If your change trips this assertion you - /// have added a pair of systems with conflicting data access and no - /// 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 = 47; + /// The backlog (302 pairs on 2026-07-06) was burned down to ZERO the + /// same day (#143, PRs #146–#149) — this is now a hard gate. If your + /// change trips this assertion you have added a pair of systems with + /// conflicting data access and no ordering edge: add `.before`/`.after` + /// where order matters, or `.ambiguous_with` the relevant domain set + /// (BoardVisuals, MarkerVisuals, UiTextFx, HudButtons, writer sets) + /// where it provably does not. Do not raise this constant. + const AMBIGUITY_BASELINE: usize = 0; fn cluster_app() -> App { let mut app = App::new(); @@ -96,10 +97,12 @@ mod tests { } }; - assert!( - count <= AMBIGUITY_BASELINE, - "system-order ambiguities grew: {count} > baseline {AMBIGUITY_BASELINE}. \ - Add .before/.after or .ambiguous_with at the new registration site.", + assert_eq!( + count, AMBIGUITY_BASELINE, + "system-order ambiguities changed from the enforced baseline. \ + Add .before/.after or .ambiguous_with at the new registration site \ + (or, if the count legitimately dropped below a nonzero baseline, \ + lower AMBIGUITY_BASELINE).", ); } } diff --git a/solitaire_engine/src/settings_plugin/mod.rs b/solitaire_engine/src/settings_plugin/mod.rs index 9aae03b..225af2f 100644 --- a/solitaire_engine/src/settings_plugin/mod.rs +++ b/solitaire_engine/src/settings_plugin/mod.rs @@ -387,7 +387,9 @@ impl Plugin for SettingsPlugin { // frame's settings transitively (ambiguity burn-down, #143). .configure_sets( Update, - SettingsMutation.before(crate::game_plugin::GameMutation), + SettingsMutation + .after(crate::layout::LayoutSystem::UpdateOnResize) + .before(crate::game_plugin::GameMutation), ) .add_systems( Update, @@ -402,10 +404,13 @@ impl Plugin for SettingsPlugin { .add_systems( Update, ( - toggle_settings_screen, + toggle_settings_screen + .before(crate::ui_focus::FocusKeys) + .ambiguous_with(crate::hud_plugin::HudButtons), scroll_settings_panel, crate::ui_modal::touch_scroll_panel::, - ), + ) + .chain(), ); if self.ui_enabled { diff --git a/solitaire_engine/src/table_plugin.rs b/solitaire_engine/src/table_plugin.rs index 0448976..f1fd9cd 100644 --- a/solitaire_engine/src/table_plugin.rs +++ b/solitaire_engine/src/table_plugin.rs @@ -85,6 +85,13 @@ pub struct HintPileHighlight { /// Registers the table background and pile-marker rendering. pub struct TablePlugin; +/// Set wrapping the pile-marker painter chain (theme, hint highlights, +/// visibility). Runs after [`crate::card_plugin::BoardVisuals`]; chrome-fx +/// systems that touch `Visibility` on UI entities declare themselves +/// ambiguous with it (#143). +#[derive(bevy::ecs::schedule::SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct MarkerVisuals; + impl Plugin for TablePlugin { fn build(&self, app: &mut App) { // Register WindowResized so the plugin works under MinimalPlugins in @@ -111,6 +118,7 @@ impl Plugin for TablePlugin { sync_pile_marker_visibility.after(GameMutation), ) .chain() + .in_set(MarkerVisuals) .after(crate::card_plugin::BoardVisuals), ), ); diff --git a/solitaire_engine/src/ui_focus.rs b/solitaire_engine/src/ui_focus.rs index 9cd3fc9..aea9b5f 100644 --- a/solitaire_engine/src/ui_focus.rs +++ b/solitaire_engine/src/ui_focus.rs @@ -117,6 +117,13 @@ pub struct FocusedButton(pub Option); /// gains keyboard navigation without per-plugin wiring. pub struct UiFocusPlugin; +/// Set on [`handle_focus_keys`], the focus-ring keyboard navigator. It runs +/// AFTER every app-level keyboard consumer (HUD buttons/popovers, restore +/// prompt, settings toggle) so Esc/Tab consumption order is defined instead +/// of scheduler-dependent (#143). +#[derive(bevy::ecs::schedule::SystemSet, Debug, Clone, PartialEq, Eq, Hash)] +pub struct FocusKeys; + impl Plugin for UiFocusPlugin { fn build(&self, app: &mut App) { app.init_resource::() @@ -147,8 +154,14 @@ impl Plugin for UiFocusPlugin { ( sync_focus_on_mouse_click, clear_hud_focus_on_unhover, - handle_focus_keys, - update_focus_overlay, + handle_focus_keys + .in_set(FocusKeys) + .after(crate::game_plugin::GameMutation), + update_focus_overlay + .in_set(crate::ui_theme::UiTextFx) + .ambiguous_with(crate::ui_theme::UiTextFx) + .ambiguous_with(crate::card_plugin::BoardVisuals) + .ambiguous_with(crate::table_plugin::MarkerVisuals), pulse_focus_overlay .after(crate::settings_plugin::SettingsMutation) .in_set(crate::ui_theme::UiTextFx) diff --git a/solitaire_engine/src/ui_modal.rs b/solitaire_engine/src/ui_modal.rs index 3def57a..cb94708 100644 --- a/solitaire_engine/src/ui_modal.rs +++ b/solitaire_engine/src/ui_modal.rs @@ -699,7 +699,8 @@ impl Plugin for UiModalPlugin { .after(crate::settings_plugin::SettingsMutation) .in_set(crate::ui_theme::UiTextFx) .ambiguous_with(crate::ui_theme::UiTextFx) - .ambiguous_with(crate::card_plugin::BoardVisuals), + .ambiguous_with(crate::card_plugin::BoardVisuals) + .ambiguous_with(crate::hud_plugin::HudButtons), ); // Click-outside-to-dismiss is independent of the open // animation chain — it reads `just_pressed(Left)` and runs