58c2dfd0a9
Test / test (pull_request) Successful in 28m7s
New SettingsMutation set: the per-frame settings mutators (handle_volume_keys → record_window_geometry_changes → persist_window_geometry_after_debounce) run as a deterministic chain ordered before GameMutation, so every reader already after GameMutation observes the current frame's settings transitively. The four readers outside that ordering (modal enter-speed chain, focus-ring pulse, HUD avatar, and the game plugin's pre-mutation chain) are ordered after the set explicitly. SettingsResource ambiguities: 21 → 0. Baseline ratchets 198 → 171. Refs #143 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
106 lines
4.4 KiB
Rust
106 lines
4.4 KiB
Rust
//! Schedule hygiene checks (issue #143).
|
|
//!
|
|
//! Bevy runs systems with conflicting data access in nondeterministic order
|
|
//! unless an ordering edge exists. Nothing enforced this historically, and
|
|
//! the headless gameplay cluster carries a large legacy backlog of such
|
|
//! pairs — most benign (event/resource writers that never observably race),
|
|
//! but unproven. Rather than a permanently red hard-error test or no test
|
|
//! at all, this is a RATCHET: the count may only go down. New conflicting
|
|
//! pairs fail CI immediately; the backlog can be triaged incrementally
|
|
//! (add `.before`/`.after` where order matters, `.ambiguous_with` where it
|
|
//! provably doesn't, then lower `AMBIGUITY_BASELINE`).
|
|
//!
|
|
//! To see the offending pair names, add the `bevy_debug` / debug feature to
|
|
//! the bevy dev-dependency (system names are stripped otherwise) and set
|
|
//! `ambiguity_detection: LogLevel::Error` manually.
|
|
//!
|
|
//! Scope note: `CoreGamePlugin` (the full composition) performs real
|
|
//! storage/platform I/O in `build` and cannot run in a unit test; the
|
|
//! cluster below is the same one the engine's headless behaviour tests use.
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use bevy::ecs::schedule::{LogLevel, ScheduleBuildSettings};
|
|
use bevy::prelude::*;
|
|
|
|
use crate::auto_complete_plugin::AutoCompletePlugin;
|
|
use crate::card_plugin::CardPlugin;
|
|
use crate::game_plugin::GamePlugin;
|
|
use crate::hud_plugin::HudPlugin;
|
|
use crate::settings_plugin::SettingsPlugin;
|
|
use crate::table_plugin::TablePlugin;
|
|
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 = 171;
|
|
|
|
fn cluster_app() -> App {
|
|
let mut app = App::new();
|
|
app.add_plugins(MinimalPlugins)
|
|
.add_plugins(GamePlugin)
|
|
.add_plugins(TablePlugin)
|
|
.add_plugins(CardPlugin)
|
|
.add_plugins(HudPlugin)
|
|
.add_plugins(AutoCompletePlugin)
|
|
.add_plugins(UiModalPlugin)
|
|
.add_plugins(UiFocusPlugin)
|
|
.add_plugins(SettingsPlugin::headless());
|
|
app.init_resource::<ButtonInput<KeyCode>>();
|
|
app
|
|
}
|
|
|
|
#[test]
|
|
fn update_schedule_ambiguities_do_not_grow() {
|
|
let mut app = cluster_app();
|
|
app.edit_schedule(Update, |s| {
|
|
s.set_build_settings(ScheduleBuildSettings {
|
|
ambiguity_detection: LogLevel::Error,
|
|
..Default::default()
|
|
});
|
|
});
|
|
|
|
// With LogLevel::Error the first schedule build panics when any
|
|
// ambiguity exists, and the panic message begins with the pair
|
|
// count. Catch it and ratchet on that count. (Parsing a panic
|
|
// message is brittle by design — if a Bevy upgrade rewords it,
|
|
// this test fails loudly and the parse below needs one-line
|
|
// maintenance, which is preferable to losing the ratchet.)
|
|
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
|
|
app.update();
|
|
}));
|
|
|
|
let count = match result {
|
|
Ok(()) => 0,
|
|
Err(payload) => {
|
|
let msg = payload
|
|
.downcast_ref::<String>()
|
|
.cloned()
|
|
.or_else(|| payload.downcast_ref::<&str>().map(|s| (*s).to_string()))
|
|
.unwrap_or_default();
|
|
let parsed = msg
|
|
.split(" pairs of systems")
|
|
.next()
|
|
.and_then(|prefix| prefix.split_whitespace().last())
|
|
.and_then(|n| n.parse::<usize>().ok());
|
|
parsed.unwrap_or_else(|| {
|
|
panic!(
|
|
"ambiguity panic message no longer parseable (Bevy upgrade?): {msg}"
|
|
)
|
|
})
|
|
}
|
|
};
|
|
|
|
assert!(
|
|
count <= AMBIGUITY_BASELINE,
|
|
"system-order ambiguities grew: {count} > baseline {AMBIGUITY_BASELINE}. \
|
|
Add .before/.after or .ambiguous_with at the new registration site.",
|
|
);
|
|
}
|
|
}
|