refactor(engine): ambiguity burn-down batch 4 — zero ambiguities, gate enforced
Test / test (pull_request) Failing after 17m5s

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 <noreply@anthropic.com>
This commit is contained in:
funman300
2026-07-06 21:13:58 -07:00
parent be478acde7
commit b402c01918
8 changed files with 136 additions and 37 deletions
+8 -1
View File
@@ -46,6 +46,11 @@ pub struct AutoCompleteState {
/// Plugin that drives the auto-complete sequence. /// Plugin that drives the auto-complete sequence.
pub struct AutoCompletePlugin; 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 { impl Plugin for AutoCompletePlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
app.init_resource::<AutoCompleteState>() app.init_resource::<AutoCompleteState>()
@@ -58,7 +63,9 @@ impl Plugin for AutoCompletePlugin {
drive_auto_complete, drive_auto_complete,
) )
.chain() .chain()
.after(GameMutation), .in_set(AutoComplete)
.after(GameMutation)
.before(crate::card_plugin::BoardVisuals),
); );
} }
} }
+25 -4
View File
@@ -75,6 +75,17 @@ pub struct GameMutation;
#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)] #[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)]
pub struct NewGameRequestWriters; 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. /// Persistence path for the in-progress game state file. `None` disables I/O.
#[derive(Resource, Debug, Clone)] #[derive(Resource, Debug, Clone)]
pub struct GameStatePath(pub Option<PathBuf>); pub struct GameStatePath(pub Option<PathBuf>);
@@ -233,7 +244,14 @@ impl Plugin for GamePlugin {
.chain() .chain()
.in_set(GameMutation), .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, record_replay_on_win.after(GameMutation))
.add_systems( .add_systems(
Update, Update,
@@ -244,14 +262,16 @@ impl Plugin for GamePlugin {
handle_game_over_button_input, handle_game_over_button_input,
) )
.after(GameMutation) .after(GameMutation)
.before(crate::ui_focus::FocusKeys)
.in_set(NewGameRequestWriters) .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, // Restore prompt: spawn the modal once the splash is gone,
// route Continue / New Game intents back into the existing // route Continue / New Game intents back into the existing
// GameMutation flow. // GameMutation flow.
.add_systems(Update, spawn_restore_prompt_if_pending) // All pre-mutation game-state writers are chained: elapsed
// All three pre-mutation game-state writers are chained: elapsed
// time ticks first, settings sync next, then the restore prompt — // time ticks first, settings sync next, then the restore prompt —
// a deterministic spine instead of three unordered ResMut holders // a deterministic spine instead of three unordered ResMut holders
// (ambiguity burn-down, #143). // (ambiguity burn-down, #143).
@@ -260,6 +280,7 @@ impl Plugin for GamePlugin {
( (
tick_elapsed_time, tick_elapsed_time,
sync_settings_to_game, sync_settings_to_game,
spawn_restore_prompt_if_pending,
handle_restore_prompt handle_restore_prompt
.in_set(NewGameRequestWriters) .in_set(NewGameRequestWriters)
.ambiguous_with(NewGameRequestWriters), .ambiguous_with(NewGameRequestWriters),
+56 -15
View File
@@ -154,6 +154,13 @@ pub struct HudColumn;
#[derive(Component, Debug)] #[derive(Component, Debug)]
pub struct HudActionBar; 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. /// 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. /// Used by `resize_action_bar_labels` to update font size on window resize.
#[derive(Component, Debug)] #[derive(Component, Debug)]
@@ -468,31 +475,56 @@ impl Plugin for HudPlugin {
// defensively so the HUD plugin works standalone in tests. // defensively so the HUD plugin works standalone in tests.
.add_message::<WindowResized>() .add_message::<WindowResized>()
.add_systems(Startup, (spawn_hud_band, spawn_hud, spawn_action_buttons, spawn_hud_avatar)) .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( .add_systems(
Update, Update,
apply_hud_visibility.before(LayoutSystem::UpdateOnResize), (
update_hud,
update_selection_hud.run_if(
resource_exists_and_changed::<SelectionState>
.or(resource_exists_and_changed::<GameStateResource>),
),
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( .add_systems(
Update, Update,
( (
update_hud_avatar.after(crate::settings_plugin::SettingsMutation), 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( .add_systems(
Update, Update,
update_selection_hud announce_auto_complete
.after(GameMutation) .after(GameMutation)
.run_if( .after(crate::auto_complete_plugin::AutoComplete)
resource_exists_and_changed::<SelectionState> .in_set(crate::game_plugin::InfoToastWriters)
.or(resource_exists_and_changed::<GameStateResource>), .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( .add_systems(
Update, Update,
( (
@@ -521,10 +553,16 @@ impl Plugin for HudPlugin {
handle_new_game_button handle_new_game_button
.in_set(NewGameRequestWriters) .in_set(NewGameRequestWriters)
.ambiguous_with(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_pause_button,
handle_help_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_modes_button,
handle_mode_option_click handle_mode_option_click
.in_set(NewGameRequestWriters) .in_set(NewGameRequestWriters)
@@ -536,7 +574,10 @@ impl Plugin for HudPlugin {
handle_menu_backdrop_click, handle_menu_backdrop_click,
close_menu_popover_on_escape, close_menu_popover_on_escape,
paint_action_buttons, paint_action_buttons,
), )
.chain()
.in_set(HudButtons)
.before(crate::ui_focus::FocusKeys),
) )
// Fade lives in `Last` so it always overrides whatever the // Fade lives in `Last` so it always overrides whatever the
// hover/paint pass set on `BackgroundColor` this frame. // hover/paint pass set on `BackgroundColor` this frame.
+14 -11
View File
@@ -32,13 +32,14 @@ mod tests {
use crate::ui_focus::UiFocusPlugin; use crate::ui_focus::UiFocusPlugin;
use crate::ui_modal::UiModalPlugin; use crate::ui_modal::UiModalPlugin;
/// Legacy ambiguity backlog measured 2026-07-06 (issue #143). This /// The backlog (302 pairs on 2026-07-06) was burned down to ZERO the
/// number may only decrease. If your change trips this assertion you /// same day (#143, PRs #146#149) — this is now a hard gate. If your
/// have added a pair of systems with conflicting data access and no /// change trips this assertion you have added a pair of systems with
/// ordering edge add `.before`/`.after` (order matters) or /// conflicting data access and no ordering edge: add `.before`/`.after`
/// `.ambiguous_with` (provably order-independent) at the registration /// where order matters, or `.ambiguous_with` the relevant domain set
/// site. When triage lowers the real count, lower this constant too. /// (BoardVisuals, MarkerVisuals, UiTextFx, HudButtons, writer sets)
const AMBIGUITY_BASELINE: usize = 47; /// where it provably does not. Do not raise this constant.
const AMBIGUITY_BASELINE: usize = 0;
fn cluster_app() -> App { fn cluster_app() -> App {
let mut app = App::new(); let mut app = App::new();
@@ -96,10 +97,12 @@ mod tests {
} }
}; };
assert!( assert_eq!(
count <= AMBIGUITY_BASELINE, count, AMBIGUITY_BASELINE,
"system-order ambiguities grew: {count} > baseline {AMBIGUITY_BASELINE}. \ "system-order ambiguities changed from the enforced baseline. \
Add .before/.after or .ambiguous_with at the new registration site.", Add .before/.after or .ambiguous_with at the new registration site \
(or, if the count legitimately dropped below a nonzero baseline, \
lower AMBIGUITY_BASELINE).",
); );
} }
} }
+8 -3
View File
@@ -387,7 +387,9 @@ impl Plugin for SettingsPlugin {
// frame's settings transitively (ambiguity burn-down, #143). // frame's settings transitively (ambiguity burn-down, #143).
.configure_sets( .configure_sets(
Update, Update,
SettingsMutation.before(crate::game_plugin::GameMutation), SettingsMutation
.after(crate::layout::LayoutSystem::UpdateOnResize)
.before(crate::game_plugin::GameMutation),
) )
.add_systems( .add_systems(
Update, Update,
@@ -402,10 +404,13 @@ impl Plugin for SettingsPlugin {
.add_systems( .add_systems(
Update, Update,
( (
toggle_settings_screen, toggle_settings_screen
.before(crate::ui_focus::FocusKeys)
.ambiguous_with(crate::hud_plugin::HudButtons),
scroll_settings_panel, scroll_settings_panel,
crate::ui_modal::touch_scroll_panel::<SettingsPanelScrollable>, crate::ui_modal::touch_scroll_panel::<SettingsPanelScrollable>,
), )
.chain(),
); );
if self.ui_enabled { if self.ui_enabled {
+8
View File
@@ -85,6 +85,13 @@ pub struct HintPileHighlight {
/// Registers the table background and pile-marker rendering. /// Registers the table background and pile-marker rendering.
pub struct TablePlugin; 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 { impl Plugin for TablePlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
// Register WindowResized so the plugin works under MinimalPlugins in // Register WindowResized so the plugin works under MinimalPlugins in
@@ -111,6 +118,7 @@ impl Plugin for TablePlugin {
sync_pile_marker_visibility.after(GameMutation), sync_pile_marker_visibility.after(GameMutation),
) )
.chain() .chain()
.in_set(MarkerVisuals)
.after(crate::card_plugin::BoardVisuals), .after(crate::card_plugin::BoardVisuals),
), ),
); );
+15 -2
View File
@@ -117,6 +117,13 @@ pub struct FocusedButton(pub Option<Entity>);
/// gains keyboard navigation without per-plugin wiring. /// gains keyboard navigation without per-plugin wiring.
pub struct UiFocusPlugin; 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 { impl Plugin for UiFocusPlugin {
fn build(&self, app: &mut App) { fn build(&self, app: &mut App) {
app.init_resource::<FocusedButton>() app.init_resource::<FocusedButton>()
@@ -147,8 +154,14 @@ impl Plugin for UiFocusPlugin {
( (
sync_focus_on_mouse_click, sync_focus_on_mouse_click,
clear_hud_focus_on_unhover, clear_hud_focus_on_unhover,
handle_focus_keys, handle_focus_keys
update_focus_overlay, .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 pulse_focus_overlay
.after(crate::settings_plugin::SettingsMutation) .after(crate::settings_plugin::SettingsMutation)
.in_set(crate::ui_theme::UiTextFx) .in_set(crate::ui_theme::UiTextFx)
+2 -1
View File
@@ -699,7 +699,8 @@ impl Plugin for UiModalPlugin {
.after(crate::settings_plugin::SettingsMutation) .after(crate::settings_plugin::SettingsMutation)
.in_set(crate::ui_theme::UiTextFx) .in_set(crate::ui_theme::UiTextFx)
.ambiguous_with(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 // Click-outside-to-dismiss is independent of the open
// animation chain — it reads `just_pressed(Left)` and runs // animation chain — it reads `just_pressed(Left)` and runs