From 253f40485c7f225830786c1937a72209b36c3124 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 15:57:41 -0400 Subject: [PATCH 01/13] Add `SystemOrderAmbiguity` struct --- .../src/schedule/ambiguity_detection.rs | 235 +++++++++++++----- 1 file changed, 178 insertions(+), 57 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index bca2450eb69ea..8bc29cf7273aa 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -5,75 +5,196 @@ use crate::component::ComponentId; use crate::schedule::{SystemContainer, SystemStage}; use crate::world::World; +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct SystemOrderAmbiguity { + // Note: In order for comparisons to work correctly, + // `system_names` and `conflicts` must be sorted at all times. + system_names: [String; 2], + conflicts: Vec, + pub segment: SystemStageSegment, +} + +/// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? +#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] +pub enum SystemStageSegment { + Parallel, + ExclusiveAtStart, + ExclusiveBeforeCommands, + ExclusiveAtEnd, +} + +impl SystemStageSegment { + pub fn desc(&self) -> &'static str { + match self { + SystemStageSegment::Parallel => "Parallel systems", + SystemStageSegment::ExclusiveAtStart => "Exclusive systems at start of stage", + SystemStageSegment::ExclusiveBeforeCommands => { + "Exclusive systems before commands of stage" + } + SystemStageSegment::ExclusiveAtEnd => "Exclusive systems at end of stage", + } + } +} + +impl SystemOrderAmbiguity { + fn from_raw( + system_a_index: usize, + system_b_index: usize, + component_ids: Vec, + segment: SystemStageSegment, + stage: &SystemStage, + world: &World, + ) -> Self { + use crate::schedule::graph_utils::GraphNode; + use SystemStageSegment::*; + + // TODO: blocked on https://github.com/bevyengine/bevy/pull/4166 + // We can't grab the system container generically, because .parallel_systems() + // and the exclusive equivalent return a different type, + // and SystemContainer is not object-safe + let (system_a_name, system_b_name) = match segment { + Parallel => { + let system_container = stage.parallel_systems(); + ( + system_container[system_a_index].name(), + system_container[system_b_index].name(), + ) + } + ExclusiveAtStart => { + let system_container = stage.exclusive_at_start_systems(); + ( + system_container[system_a_index].name(), + system_container[system_b_index].name(), + ) + } + ExclusiveBeforeCommands => { + let system_container = stage.exclusive_before_commands_systems(); + ( + system_container[system_a_index].name(), + system_container[system_b_index].name(), + ) + } + ExclusiveAtEnd => { + let system_container = stage.exclusive_at_end_systems(); + ( + system_container[system_a_index].name(), + system_container[system_b_index].name(), + ) + } + }; + + let mut system_names = [system_a_name.to_string(), system_b_name.to_string()]; + system_names.sort(); + + let mut conflicts: Vec<_> = component_ids + .iter() + .map(|id| world.components().get_info(*id).unwrap().name().to_owned()) + .collect(); + conflicts.sort(); + + Self { + system_names, + conflicts, + segment, + } + } +} + impl SystemStage { /// Logs execution order ambiguities between systems. System orders must be fresh. pub fn report_ambiguities(&self, world: &World) { debug_assert!(!self.systems_modified); use std::fmt::Write; - fn write_display_names_of_pairs( - string: &mut String, - systems: &[impl SystemContainer], - mut ambiguities: Vec<(usize, usize, Vec)>, - world: &World, - ) { - for (index_a, index_b, conflicts) in ambiguities.drain(..) { - writeln!( - string, - " -- {:?} and {:?}", - systems[index_a].name(), - systems[index_b].name() - ) - .unwrap(); - if !conflicts.is_empty() { - let names = conflicts - .iter() - .map(|id| world.components().get_info(*id).unwrap().name()) - .collect::>(); - writeln!(string, " conflicts: {:?}", names).unwrap(); - } - } - } - let parallel = find_ambiguities(&self.parallel); - let at_start = find_ambiguities(&self.exclusive_at_start); - let before_commands = find_ambiguities(&self.exclusive_before_commands); - let at_end = find_ambiguities(&self.exclusive_at_end); - if !(parallel.is_empty() - && at_start.is_empty() - && before_commands.is_empty() - && at_end.is_empty()) - { + let ambiguities = self.ambiguities(world); + if !ambiguities.is_empty() { let mut string = "Execution order ambiguities detected, you might want to \ add an explicit dependency relation between some of these systems:\n" .to_owned(); - if !parallel.is_empty() { - writeln!(string, " * Parallel systems:").unwrap(); - write_display_names_of_pairs(&mut string, &self.parallel, parallel, world); - } - if !at_start.is_empty() { - writeln!(string, " * Exclusive systems at start of stage:").unwrap(); - write_display_names_of_pairs( - &mut string, - &self.exclusive_at_start, - at_start, - world, - ); - } - if !before_commands.is_empty() { - writeln!(string, " * Exclusive systems before commands of stage:").unwrap(); - write_display_names_of_pairs( - &mut string, - &self.exclusive_before_commands, - before_commands, - world, - ); - } - if !at_end.is_empty() { - writeln!(string, " * Exclusive systems at end of stage:").unwrap(); - write_display_names_of_pairs(&mut string, &self.exclusive_at_end, at_end, world); + + let mut last_segment_kind = None; + + for SystemOrderAmbiguity { + system_names: [system_a, system_b], + conflicts, + segment, + } in &ambiguities + { + // If the ambiguity occurred in a different segment than the previous one, write a header for the segment. + if last_segment_kind != Some(segment) { + writeln!(string, " * {}:", segment.desc()).unwrap(); + last_segment_kind = Some(segment); + } + + writeln!(string, " -- {:?} and {:?}", system_a, system_b).unwrap(); + + if !conflicts.is_empty() { + writeln!(string, " conflicts: {conflicts:?}").unwrap(); + } } + info!("{}", string); } } + + fn ambiguities(&self, world: &World) -> Vec { + let parallel = find_ambiguities(&self.parallel).into_iter().map( + |(system_a_index, system_b_index, component_ids)| { + SystemOrderAmbiguity::from_raw( + system_a_index, + system_b_index, + component_ids.to_vec(), + SystemStageSegment::Parallel, + self, + world, + ) + }, + ); + + let at_start = find_ambiguities(&self.exclusive_at_start).into_iter().map( + |(system_a_index, system_b_index, component_ids)| { + SystemOrderAmbiguity::from_raw( + system_a_index, + system_b_index, + component_ids, + SystemStageSegment::ExclusiveAtStart, + self, + world, + ) + }, + ); + + let before_commands = find_ambiguities(&self.exclusive_before_commands) + .into_iter() + .map(|(system_a_index, system_b_index, component_ids)| { + SystemOrderAmbiguity::from_raw( + system_a_index, + system_b_index, + component_ids, + SystemStageSegment::ExclusiveBeforeCommands, + self, + world, + ) + }); + + let at_end = find_ambiguities(&self.exclusive_at_end).into_iter().map( + |(system_a_index, system_b_index, component_ids)| { + SystemOrderAmbiguity::from_raw( + system_a_index, + system_b_index, + component_ids, + SystemStageSegment::ExclusiveAtEnd, + self, + world, + ) + }, + ); + + at_start + .chain(parallel) + .chain(before_commands) + .chain(at_end) + .collect() + } } /// Returns vector containing all pairs of indices of systems with ambiguous execution order, From ad96b07eb90d5fc151e7c61ab997535be9e00c8b Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 16:03:45 -0400 Subject: [PATCH 02/13] Use a stable ordering for ambiguity lists --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 8bc29cf7273aa..f4feedce7103d 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -5,17 +5,17 @@ use crate::component::ComponentId; use crate::schedule::{SystemContainer, SystemStage}; use crate::world::World; -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct SystemOrderAmbiguity { + pub segment: SystemStageSegment, // Note: In order for comparisons to work correctly, // `system_names` and `conflicts` must be sorted at all times. system_names: [String; 2], conflicts: Vec, - pub segment: SystemStageSegment, } /// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? -#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] +#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] pub enum SystemStageSegment { Parallel, ExclusiveAtStart, @@ -112,7 +112,6 @@ impl SystemStage { .to_owned(); let mut last_segment_kind = None; - for SystemOrderAmbiguity { system_names: [system_a, system_b], conflicts, @@ -189,11 +188,13 @@ impl SystemStage { }, ); - at_start + let mut ambiguities: Vec<_> = at_start .chain(parallel) .chain(before_commands) .chain(at_end) - .collect() + .collect(); + ambiguities.sort(); + ambiguities } } From 993e220bc586b1b3f4a7aa38e9ff4b86e8d3222d Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 16:05:03 -0400 Subject: [PATCH 03/13] Copy over docs --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index f4feedce7103d..e4c9b5eed8fd4 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -135,6 +135,13 @@ impl SystemStage { } } + /// Returns all execution order ambiguities between systems + /// + /// Returns 4 vectors of ambiguities for each stage, in the following order: + /// - parallel + /// - exclusive at start, + /// - exclusive before commands + /// - exclusive at end fn ambiguities(&self, world: &World) -> Vec { let parallel = find_ambiguities(&self.parallel).into_iter().map( |(system_a_index, system_b_index, component_ids)| { From 03b2e6e32d7355f17a77e2090ec53ea562e7f672 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 17:15:48 -0400 Subject: [PATCH 04/13] Add unit tests --- .../src/schedule/ambiguity_detection.rs | 234 +++++++++++++++++- 1 file changed, 232 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index e4c9b5eed8fd4..6355647a27e35 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -135,14 +135,16 @@ impl SystemStage { } } - /// Returns all execution order ambiguities between systems + /// Returns all execution order ambiguities between systems. /// /// Returns 4 vectors of ambiguities for each stage, in the following order: /// - parallel /// - exclusive at start, /// - exclusive before commands /// - exclusive at end - fn ambiguities(&self, world: &World) -> Vec { + /// + /// This stage must have been initialized with `world`. + pub fn ambiguities(&self, world: &World) -> Vec { let parallel = find_ambiguities(&self.parallel).into_iter().map( |(system_a_index, system_b_index, component_ids)| { SystemOrderAmbiguity::from_raw( @@ -203,6 +205,12 @@ impl SystemStage { ambiguities.sort(); ambiguities } + + /// Returns the number of system order ambiguities between systems in this stage. + /// This stage must have been initialized with `world`. + pub fn n_ambiguities(&self, world: &World) -> usize { + self.ambiguities(world).len() + } } /// Returns vector containing all pairs of indices of systems with ambiguous execution order, @@ -267,3 +275,225 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec< } ambiguities } + +// Systems and TestResource are used in tests +#[allow(dead_code)] +#[cfg(test)] +mod tests { + // Required to make the derive macro behave + use crate as bevy_ecs; + use crate::event::Events; + use crate::prelude::*; + + #[derive(Resource)] + struct R; + + #[derive(Component)] + struct A; + + #[derive(Component)] + struct B; + + // An event type + struct E; + + fn empty_system() {} + fn res_system(_res: Res) {} + fn resmut_system(_res: ResMut) {} + fn nonsend_system(_ns: NonSend) {} + fn nonsendmut_system(_ns: NonSendMut) {} + fn read_component_system(_query: Query<&A>) {} + fn write_component_system(_query: Query<&mut A>) {} + fn with_filtered_component_system(_query: Query<&mut A, With>) {} + fn without_filtered_component_system(_query: Query<&mut A, Without>) {} + fn event_reader_system(_reader: EventReader) {} + fn event_writer_system(_writer: EventWriter) {} + fn event_resource_system(_events: ResMut>) {} + fn read_world_system(_world: &World) {} + fn write_world_system(_world: &mut World) {} + + // Tests for conflict detection + + #[test] + fn one_of_everything() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn().insert(A); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(empty_system) + // nonsendmut system deliberately conflicts with resmut system + .add_system(resmut_system) + .add_system(write_component_system) + .add_system(event_writer_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 0); + } + + #[test] + fn read_only() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn().insert(A); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(empty_system) + .add_system(empty_system) + .add_system(res_system) + .add_system(res_system) + .add_system(nonsend_system) + .add_system(nonsend_system) + .add_system(read_component_system) + .add_system(read_component_system) + .add_system(event_reader_system) + .add_system(event_reader_system) + .add_system(read_world_system) + .add_system(read_world_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 0); + } + + #[test] + fn read_world() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn().insert(A); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(empty_system) + .add_system(resmut_system) + .add_system(write_component_system) + .add_system(event_writer_system) + .add_system(read_world_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 3); + } + + #[test] + fn resources() { + let mut world = World::new(); + world.insert_resource(R); + + let mut test_stage = SystemStage::parallel(); + test_stage.add_system(resmut_system).add_system(res_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 1); + } + + #[test] + fn nonsend() { + let mut world = World::new(); + world.insert_resource(R); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(nonsendmut_system) + .add_system(nonsend_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 1); + } + + #[test] + fn components() { + let mut world = World::new(); + world.spawn().insert(A); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(read_component_system) + .add_system(write_component_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 1); + } + + #[test] + #[ignore = "Known failing but fix is non-trivial: https://github.com/bevyengine/bevy/issues/4381"] + fn filtered_components() { + let mut world = World::new(); + world.spawn().insert(A); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(with_filtered_component_system) + .add_system(without_filtered_component_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 0); + } + + #[test] + fn events() { + let mut world = World::new(); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + // All of these systems clash + .add_system(event_reader_system) + .add_system(event_writer_system) + .add_system(event_resource_system); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 3); + } + + #[test] + fn exclusive() { + let mut world = World::new(); + world.insert_resource(R); + world.spawn().insert(A); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + // All 3 of these conflict with each other + .add_system(write_world_system.exclusive_system()) + .add_system(write_world_system.exclusive_system().at_end()) + .add_system(res_system.exclusive_system()) + // These do not, as they're in different segments of the stage + .add_system(write_world_system.exclusive_system().at_start()) + .add_system(write_world_system.exclusive_system().before_commands()); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 3); + } + + // Tests for silencing and resolving ambiguities + + #[test] + fn before_and_after() { + let mut world = World::new(); + world.init_resource::>(); + + let mut test_stage = SystemStage::parallel(); + test_stage + .add_system(event_reader_system.before(event_writer_system)) + .add_system(event_writer_system) + .add_system(event_resource_system.after(event_writer_system)); + + test_stage.run(&mut world); + + assert_eq!(test_stage.n_ambiguities(&world), 0); + } +} From aea22534972c485455ab956c248a1162c3ea9cfc Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 17:17:21 -0400 Subject: [PATCH 05/13] Make `SystemOrderAmbiguity` private --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 6355647a27e35..6e8ce6d0ff9c3 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -6,7 +6,7 @@ use crate::schedule::{SystemContainer, SystemStage}; use crate::world::World; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct SystemOrderAmbiguity { +struct SystemOrderAmbiguity { pub segment: SystemStageSegment, // Note: In order for comparisons to work correctly, // `system_names` and `conflicts` must be sorted at all times. @@ -16,7 +16,7 @@ pub struct SystemOrderAmbiguity { /// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? #[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] -pub enum SystemStageSegment { +enum SystemStageSegment { Parallel, ExclusiveAtStart, ExclusiveBeforeCommands, @@ -144,7 +144,7 @@ impl SystemStage { /// - exclusive at end /// /// This stage must have been initialized with `world`. - pub fn ambiguities(&self, world: &World) -> Vec { + fn ambiguities(&self, world: &World) -> Vec { let parallel = find_ambiguities(&self.parallel).into_iter().map( |(system_a_index, system_b_index, component_ids)| { SystemOrderAmbiguity::from_raw( From c4750075136e1eeeef858512b5287c454d8774be Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 17:35:11 -0400 Subject: [PATCH 06/13] Make a fn private --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 6e8ce6d0ff9c3..5fcecb784e69c 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -208,7 +208,8 @@ impl SystemStage { /// Returns the number of system order ambiguities between systems in this stage. /// This stage must have been initialized with `world`. - pub fn n_ambiguities(&self, world: &World) -> usize { + #[allow(dead_code)] + fn n_ambiguities(&self, world: &World) -> usize { self.ambiguities(world).len() } } From 63b47f02e9c40e6b6338b5b97fe7b001603ba688 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 17:42:31 -0400 Subject: [PATCH 07/13] Un-silence a lint --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 5fcecb784e69c..a4a13d6775e5a 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -277,8 +277,6 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec< ambiguities } -// Systems and TestResource are used in tests -#[allow(dead_code)] #[cfg(test)] mod tests { // Required to make the derive macro behave From a0556f723de2a8b6d20e2e88499dff000553c134 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 18:17:42 -0400 Subject: [PATCH 08/13] Remove a useless pub --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index a4a13d6775e5a..78b3e33963524 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -7,7 +7,7 @@ use crate::world::World; #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] struct SystemOrderAmbiguity { - pub segment: SystemStageSegment, + segment: SystemStageSegment, // Note: In order for comparisons to work correctly, // `system_names` and `conflicts` must be sorted at all times. system_names: [String; 2], From ff0375d9ed0bf49349905debd2465a2ca0996ebf Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 18:24:22 -0400 Subject: [PATCH 09/13] Clarify some incorrect behavior --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 78b3e33963524..d90d622ae36ab 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -101,7 +101,9 @@ impl SystemOrderAmbiguity { } impl SystemStage { - /// Logs execution order ambiguities between systems. System orders must be fresh. + /// Logs execution order ambiguities between systems. + /// + /// The output may be incorrect if this stage has not been initialized with `world`. pub fn report_ambiguities(&self, world: &World) { debug_assert!(!self.systems_modified); use std::fmt::Write; @@ -143,7 +145,7 @@ impl SystemStage { /// - exclusive before commands /// - exclusive at end /// - /// This stage must have been initialized with `world`. + /// The result may be incorrect if this stage has not been initialized with `world`. fn ambiguities(&self, world: &World) -> Vec { let parallel = find_ambiguities(&self.parallel).into_iter().map( |(system_a_index, system_b_index, component_ids)| { @@ -207,7 +209,8 @@ impl SystemStage { } /// Returns the number of system order ambiguities between systems in this stage. - /// This stage must have been initialized with `world`. + /// + /// The result may be incorrect if this stage has not been initialized with `world`. #[allow(dead_code)] fn n_ambiguities(&self, world: &World) -> usize { self.ambiguities(world).len() From ec78500de4eb5c64f0cf114ebe73b5a5a42c977b Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 18:24:51 -0400 Subject: [PATCH 10/13] `n_ambiguityies` -> `ambiguity_count` --- .../src/schedule/ambiguity_detection.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index d90d622ae36ab..bb1c92de3fed6 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -212,7 +212,7 @@ impl SystemStage { /// /// The result may be incorrect if this stage has not been initialized with `world`. #[allow(dead_code)] - fn n_ambiguities(&self, world: &World) -> usize { + fn ambiguity_count(&self, world: &World) -> usize { self.ambiguities(world).len() } } @@ -333,7 +333,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 0); + assert_eq!(test_stage.ambiguity_count(&world), 0); } #[test] @@ -360,7 +360,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 0); + assert_eq!(test_stage.ambiguity_count(&world), 0); } #[test] @@ -380,7 +380,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 3); + assert_eq!(test_stage.ambiguity_count(&world), 3); } #[test] @@ -393,7 +393,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 1); + assert_eq!(test_stage.ambiguity_count(&world), 1); } #[test] @@ -408,7 +408,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 1); + assert_eq!(test_stage.ambiguity_count(&world), 1); } #[test] @@ -423,7 +423,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 1); + assert_eq!(test_stage.ambiguity_count(&world), 1); } #[test] @@ -439,7 +439,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 0); + assert_eq!(test_stage.ambiguity_count(&world), 0); } #[test] @@ -456,7 +456,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 3); + assert_eq!(test_stage.ambiguity_count(&world), 3); } #[test] @@ -478,7 +478,7 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 3); + assert_eq!(test_stage.ambiguity_count(&world), 3); } // Tests for silencing and resolving ambiguities @@ -496,6 +496,6 @@ mod tests { test_stage.run(&mut world); - assert_eq!(test_stage.n_ambiguities(&world), 0); + assert_eq!(test_stage.ambiguity_count(&world), 0); } } From d2d5135ed366e5723df1d8b5e6e1aadb95a4d881 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Wed, 21 Sep 2022 18:25:49 -0400 Subject: [PATCH 11/13] Use conditional compilation --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index bb1c92de3fed6..1b95b9eabab01 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -211,7 +211,7 @@ impl SystemStage { /// Returns the number of system order ambiguities between systems in this stage. /// /// The result may be incorrect if this stage has not been initialized with `world`. - #[allow(dead_code)] + #[cfg(test)] fn ambiguity_count(&self, world: &World) -> usize { self.ambiguities(world).len() } From e08cec59a5c59a0f07287abaa379d124495618b1 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 11:08:29 -0400 Subject: [PATCH 12/13] Remove some redundant empty system test cases --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 1b95b9eabab01..a9aa1de58b231 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -325,7 +325,6 @@ mod tests { let mut test_stage = SystemStage::parallel(); test_stage - .add_system(empty_system) // nonsendmut system deliberately conflicts with resmut system .add_system(resmut_system) .add_system(write_component_system) @@ -372,7 +371,6 @@ mod tests { let mut test_stage = SystemStage::parallel(); test_stage - .add_system(empty_system) .add_system(resmut_system) .add_system(write_component_system) .add_system(event_writer_system) From 0eaeef960654a122702d625ac870e00f78a00824 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 15:06:22 -0400 Subject: [PATCH 13/13] bump CI bump CI