Skip to content

Commit 4dc8fb4

Browse files
authored
Change the ambiguity_detection test to be more strict. (#23846)
# Objective - Fixes #23843 by preventing these systems from recurring inside Bevy. - Note this does not fix this for users of Bevy. They would need to setup their own "strict" checking. ## Solution - Disable `auto_insert_apply_deferred` in the `ambiguity_detection` example. Now these stages won't accidentally resolve these ambiguities, so they will be detected! - Stop running the app in `ambiguity_detection` and just manually initialize the schedules. - We have to do this otherwise the schedules just panic because they depend on various resource initializations to be executed by commands (which were previously being applied by the automatically inserted `apply_deferred`). ### Caveats: - The ambiguity_detection CI test now won't detect ambiguities in runtime-added systems. - This is not a pattern we use today, and I'd encourage us to **delete** systems instead so that tooling can grab the systems before the app runs. - To clarify, this is **not** ambiguity detection in general - just Bevy's CI test that is affected. ## Testing - Ran the examples and all the ambiguities have been fixed in previous PRs.
1 parent c7f2038 commit 4dc8fb4

1 file changed

Lines changed: 31 additions & 8 deletions

File tree

tests/ecs/ambiguity_detection.rs

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,21 @@ fn main() {
3535
let sub_app = app.sub_app_mut(bevy_render::RenderApp);
3636
configure_ambiguity_detection(sub_app);
3737

38+
// Make sure all the system stuff is added.
3839
app.finish();
3940
app.cleanup();
40-
app.update();
4141

42-
let main_app_ambiguities = count_ambiguities(app.main());
42+
let main_app_ambiguities = count_ambiguities(app.main_mut());
4343
assert_eq!(
4444
main_app_ambiguities.total(),
4545
0,
4646
"Main app has unexpected ambiguities among the following schedules: \n{main_app_ambiguities:#?}.",
4747
);
48-
let render_app_ambiguities = count_ambiguities(app.sub_app(bevy_render::RenderApp));
48+
49+
let render_app = app.sub_app_mut(bevy_render::RenderApp);
50+
// Initialize the MainWorld so the render world systems don't fail initialization.
51+
render_app.init_resource::<bevy_render::MainWorld>();
52+
let render_app_ambiguities = count_ambiguities(render_app);
4953
assert_eq!(
5054
render_app_ambiguities.total(),
5155
0,
@@ -69,19 +73,38 @@ fn configure_ambiguity_detection(sub_app: &mut SubApp) {
6973
schedule.set_build_settings(ScheduleBuildSettings {
7074
// NOTE: you can change this to `LogLevel::Ignore` to easily see the current number of ambiguities.
7175
ambiguity_detection: LogLevel::Warn,
76+
// With auto-inserted apply_deferred stages, these can cause two ambiguous systems to
77+
// become accidentally ordered by one of the apply_deferred stages. Disabling requires
78+
// us to meet a higher bar. We don't just want no ambiguities - we also don't want
79+
// changes to systems or the auto-insert code from "creating" new ambiguities (by
80+
// reordering the graph). However, the cost is that the graph is no longer runnable,
81+
// since Bevy crates often rely on auto-insert apply_deferred to not panic (e.g.,
82+
// because a resource wasn't inserted).
83+
auto_insert_apply_deferred: false,
7284
use_shortnames: false,
7385
..default()
7486
});
7587
}
7688
}
7789

7890
/// Returns the number of conflicting systems per schedule.
79-
fn count_ambiguities(sub_app: &SubApp) -> AmbiguitiesCount {
80-
let schedules = sub_app.world().resource::<Schedules>();
91+
fn count_ambiguities(sub_app: &mut SubApp) -> AmbiguitiesCount {
92+
let schedule_labels = sub_app
93+
.world()
94+
.resource::<Schedules>()
95+
.iter()
96+
.map(|(_, schedule)| schedule.label())
97+
.collect::<Vec<_>>();
8198
let mut ambiguities = <HashMap<_, _>>::default();
82-
for (_, schedule) in schedules.iter() {
83-
let ambiguities_in_schedule = schedule.graph().conflicting_systems().len();
84-
ambiguities.insert(schedule.label(), ambiguities_in_schedule);
99+
for label in schedule_labels {
100+
let ambiguities_in_schedule =
101+
sub_app
102+
.world_mut()
103+
.schedule_scope(label, |world, schedule| {
104+
schedule.initialize(world).unwrap().unwrap();
105+
schedule.graph().conflicting_systems().len()
106+
});
107+
ambiguities.insert(label, ambiguities_in_schedule);
85108
}
86109
AmbiguitiesCount(ambiguities)
87110
}

0 commit comments

Comments
 (0)