From e422e4f1f1d8cdecefd37692012663b2fc39797e Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 16:27:42 -0400 Subject: [PATCH 01/11] Group ambiguities with the same conflicts --- .../src/schedule/ambiguity_detection.rs | 330 +++++++++--------- 1 file changed, 168 insertions(+), 162 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index a9aa1de58b231..e407e9ecbf582 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -1,105 +1,11 @@ use bevy_utils::tracing::info; +use bevy_utils::HashMap; use fixedbitset::FixedBitSet; use crate::component::ComponentId; use crate::schedule::{SystemContainer, SystemStage}; use crate::world::World; -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -struct SystemOrderAmbiguity { - 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, -} - -/// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? -#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] -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. /// @@ -115,9 +21,10 @@ impl SystemStage { let mut last_segment_kind = None; for SystemOrderAmbiguity { - system_names: [system_a, system_b], - conflicts, segment, + conflicts, + system_names, + .. } in &ambiguities { // If the ambiguity occurred in a different segment than the previous one, write a header for the segment. @@ -126,7 +33,7 @@ impl SystemStage { last_segment_kind = Some(segment); } - writeln!(string, " -- {:?} and {:?}", system_a, system_b).unwrap(); + writeln!(string, " -- {system_names:?}").unwrap(); if !conflicts.is_empty() { writeln!(string, " conflicts: {conflicts:?}").unwrap(); @@ -147,80 +54,179 @@ impl SystemStage { /// /// 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)| { - 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, - ) - }, - ); - - let mut ambiguities: Vec<_> = at_start - .chain(parallel) - .chain(before_commands) - .chain(at_end) - .collect(); - ambiguities.sort(); - ambiguities + fn foo<'a>( + segment: SystemStageSegment, + container: &'a [impl SystemContainer], + world: &'a World, + ) -> impl Iterator + 'a { + let info = AmbiguityInfo::delineate(find_ambiguities(container)); + info.into_iter().map( + move |AmbiguityInfo { + conflicts, systems, .. + }| { + let conflicts = conflicts + .iter() + .map(|id| world.components().get_info(*id).unwrap().name().to_owned()) + .collect(); + let system_names = systems + .iter() + .map(|&SystemIndex(i)| container[i].name().to_string()) + .collect(); + SystemOrderAmbiguity { + segment, + conflicts, + system_names, + } + }, + ) + } + + foo(SystemStageSegment::Parallel, &self.parallel, world) + .chain(foo( + SystemStageSegment::ExclusiveAtStart, + &self.exclusive_at_start, + world, + )) + .chain(foo( + SystemStageSegment::ExclusiveBeforeCommands, + &self.exclusive_before_commands, + world, + )) + .chain(foo( + SystemStageSegment::ExclusiveAtEnd, + &self.exclusive_at_end, + world, + )) + .collect() } /// 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`. #[cfg(test)] - fn ambiguity_count(&self, world: &World) -> usize { - self.ambiguities(world).len() + fn ambiguity_count(&self, _world: &World) -> usize { + find_ambiguities(&self.parallel).len() + + find_ambiguities(&self.exclusive_at_start).len() + + find_ambiguities(&self.exclusive_before_commands).len() + + find_ambiguities(&self.exclusive_at_end).len() + } +} + +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +struct SystemOrderAmbiguity { + segment: SystemStageSegment, + conflicts: Vec, + system_names: Vec, +} + +/// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? +#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] +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", + } + } +} + +/// A set of systems that are all reported to be ambiguous with one another. +#[derive(PartialEq, Eq, PartialOrd, Ord)] +struct AmbiguityInfo { + // INVARIANT: `conflicts` is always sorted. + conflicts: Vec, + systems: Vec, +} + +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +struct SystemIndex(usize); + +impl AmbiguityInfo { + fn delineate( + pairs: impl IntoIterator)>, + ) -> Vec { + // Stores the pairs of system indices associated with each set of conflicts. + let mut pairs_by_conflicts = HashMap::new(); + for (system_a_index, system_b_index, mut conflicts) in pairs { + conflicts.sort(); + pairs_by_conflicts + .entry(conflicts) + .or_insert_with(Vec::new) + .push([system_a_index, system_b_index]); + } + + let mut ambiguity_sets = Vec::new(); + for (conflicts, pairs) in pairs_by_conflicts { + // Find all unique systems that have the same conflicts. + // Note that this does *not* mean they all conflict with one another. + let mut in_set: Vec<_> = pairs.iter().copied().flatten().collect(); + in_set.sort(); + in_set.dedup(); + + // adjacency marix for the entries of `in_set` + let mut adj: Vec = (0..in_set.len()) + .map(|i| { + let mut bitset = FixedBitSet::with_capacity(in_set.len()); + // enable the main diagonal + bitset.set(i, true); + bitset + }) + .collect(); + // the value `pairs` mapped as indices in `in_set`. + let mut pairs_as_indices = Vec::new(); + for &[a, b] in &pairs { + let a_index = in_set.iter().position(|&i| i == a).unwrap(); + let b_index = in_set.iter().position(|&i| i == b).unwrap(); + pairs_as_indices.push([a_index, b_index]); + adj[a_index].set(b_index, true); + adj[b_index].set(a_index, true); + } + + // Find decompose into "subgraphs" -- sets of systems that are all ambiguous with one another. + let mut subgraphs = Vec::new(); + for [a_index, b_index] in pairs_as_indices { + let intersection: FixedBitSet = adj[a_index].intersection(&adj[b_index]).collect(); + if intersection.count_ones(..) == 0 { + continue; + } + + for i in intersection.ones() { + adj[i].difference_with(&intersection); + // don't unset the main diagonal + adj[i].set(i, true); + } + + subgraphs.push(intersection); + } + + for subgraph in subgraphs { + ambiguity_sets.push(AmbiguityInfo { + conflicts: conflicts.clone(), + systems: subgraph.ones().map(|i| in_set[i]).collect(), + }); + } + } + ambiguity_sets } } /// Returns vector containing all pairs of indices of systems with ambiguous execution order, /// along with specific components that have triggered the warning. /// Systems must be topologically sorted beforehand. -fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec)> { +fn find_ambiguities( + systems: &[impl SystemContainer], +) -> Vec<(SystemIndex, SystemIndex, Vec)> { let mut all_dependencies = Vec::::with_capacity(systems.len()); let mut all_dependants = Vec::::with_capacity(systems.len()); for (index, container) in systems.iter().enumerate() { @@ -268,10 +274,10 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize, Vec< if let (Some(a), Some(b)) = (a_access, b_access) { let conflicts = a.get_conflicts(b); if !conflicts.is_empty() { - ambiguities.push((index_a, index_b, conflicts)); + ambiguities.push((SystemIndex(index_a), SystemIndex(index_b), conflicts)); } } else { - ambiguities.push((index_a, index_b, Vec::new())); + ambiguities.push((SystemIndex(index_a), SystemIndex(index_b), Vec::new())); } } } From cdcf4fa7c5b0f7b0ae377952b2513a67d512d885 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 17:31:34 -0400 Subject: [PATCH 02/11] Prettify ambiguity reporting --- .../src/schedule/ambiguity_detection.rs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index e407e9ecbf582..0ed7178b3f3f4 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -19,24 +19,25 @@ impl SystemStage { add an explicit dependency relation between some of these systems:\n" .to_owned(); - let mut last_segment_kind = None; - for SystemOrderAmbiguity { - segment, - conflicts, - system_names, - .. - } 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); + for (i, ambiguity) in ambiguities.iter().enumerate() { + let SystemOrderAmbiguity { + segment, + conflicts, + system_names, + .. + } = ambiguity; + + writeln!(string).unwrap(); + writeln!(string, "({i}) Ambiguity - {}", segment.desc()).unwrap(); + + for name in system_names { + writeln!(string, " * {name}").unwrap(); } - writeln!(string, " -- {system_names:?}").unwrap(); - - if !conflicts.is_empty() { - writeln!(string, " conflicts: {conflicts:?}").unwrap(); + writeln!(string).unwrap(); + writeln!(string, " Conflicts:").unwrap(); + for conflict in conflicts { + writeln!(string, " * {conflict}").unwrap(); } } From af09661ab95ade1a5f3c2cd283734c263ef5d7b1 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 17:57:51 -0400 Subject: [PATCH 03/11] Sort system names in groups --- 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 0ed7178b3f3f4..9e3e7e0a2cb3a 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -69,10 +69,11 @@ impl SystemStage { .iter() .map(|id| world.components().get_info(*id).unwrap().name().to_owned()) .collect(); - let system_names = systems + let mut system_names: Vec<_> = systems .iter() .map(|&SystemIndex(i)| container[i].name().to_string()) .collect(); + system_names.sort(); SystemOrderAmbiguity { segment, conflicts, From df6df6bd7a53414fd2cd5932658bde439e52228d Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 18:18:04 -0400 Subject: [PATCH 04/11] Fix a set fragmentation bug --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 6 ++++-- examples/games/alien_cake_addict.rs | 7 ++++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 9e3e7e0a2cb3a..89da7b8d50d68 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -195,13 +195,15 @@ impl AmbiguityInfo { adj[b_index].set(a_index, true); } - // Find decompose into "subgraphs" -- sets of systems that are all ambiguous with one another. + // Find sets of systems that are all ambiguous with one another. let mut subgraphs = Vec::new(); for [a_index, b_index] in pairs_as_indices { let intersection: FixedBitSet = adj[a_index].intersection(&adj[b_index]).collect(); - if intersection.count_ones(..) == 0 { + // If this pair has been included in another set, skip it. + if intersection.count_ones(..) <= 1 { continue; } + debug_assert!(intersection.count_ones(..) > 1); for i in intersection.ones() { adj[i].difference_with(&intersection); diff --git a/examples/games/alien_cake_addict.rs b/examples/games/alien_cake_addict.rs index c543c7c91d9a7..8748b405d54bb 100644 --- a/examples/games/alien_cake_addict.rs +++ b/examples/games/alien_cake_addict.rs @@ -2,7 +2,11 @@ use std::f32::consts::PI; -use bevy::{ecs::schedule::SystemSet, prelude::*, time::FixedTimestep}; +use bevy::{ + ecs::schedule::{ReportExecutionOrderAmbiguities, SystemSet}, + prelude::*, + time::FixedTimestep, +}; use rand::Rng; #[derive(Clone, Eq, PartialEq, Debug, Hash)] @@ -15,6 +19,7 @@ fn main() { App::new() .init_resource::() .add_plugins(DefaultPlugins) + .insert_resource(ReportExecutionOrderAmbiguities) .add_state(GameState::Playing) .add_startup_system(setup_cameras) .add_system_set(SystemSet::on_enter(GameState::Playing).with_system(setup)) From 2a0c97968fe861d9ccd9be03beaec2758b77a833 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 19:08:52 -0400 Subject: [PATCH 05/11] remove a redundant assert --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 89da7b8d50d68..dcaa63e621222 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -203,7 +203,6 @@ impl AmbiguityInfo { if intersection.count_ones(..) <= 1 { continue; } - debug_assert!(intersection.count_ones(..) > 1); for i in intersection.ones() { adj[i].difference_with(&intersection); From fe890d2e5b74eb53ff92bcdd9d9cd3c9b8eeaefb Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 19:38:21 -0400 Subject: [PATCH 06/11] make headings more extra --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index dcaa63e621222..e1c59f0c2e104 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -28,14 +28,19 @@ impl SystemStage { } = ambiguity; writeln!(string).unwrap(); - writeln!(string, "({i}) Ambiguity - {}", segment.desc()).unwrap(); + writeln!( + string, + "({i}) Ambiguous system ordering - {}", + segment.desc() + ) + .unwrap(); for name in system_names { writeln!(string, " * {name}").unwrap(); } writeln!(string).unwrap(); - writeln!(string, " Conflicts:").unwrap(); + writeln!(string, " Data access conflicts:").unwrap(); for conflict in conflicts { writeln!(string, " * {conflict}").unwrap(); } From 8972a1bca04af5287682b6ca3fcb92923c223362 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 19:46:46 -0400 Subject: [PATCH 07/11] make `ambiguity_count` call `ambiguities` --- .../bevy_ecs/src/schedule/ambiguity_detection.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index e1c59f0c2e104..255cc24e21782 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -111,11 +111,15 @@ impl SystemStage { /// /// The result may be incorrect if this stage has not been initialized with `world`. #[cfg(test)] - fn ambiguity_count(&self, _world: &World) -> usize { - find_ambiguities(&self.parallel).len() - + find_ambiguities(&self.exclusive_at_start).len() - + find_ambiguities(&self.exclusive_before_commands).len() - + find_ambiguities(&self.exclusive_at_end).len() + fn ambiguity_count(&self, world: &World) -> usize { + fn binomial_coefficient(n: usize, k: usize) -> usize { + (0..k).fold(1, |x, i| x * (n - i) / (i + 1)) + } + + self.ambiguities(world) + .iter() + .map(|a| binomial_coefficient(a.system_names.len(), 2)) + .sum() } } From 8eba46671117074725b3149bebb657d22deb835d Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 19:47:21 -0400 Subject: [PATCH 08/11] Revert a change to an example --- examples/games/alien_cake_addict.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/examples/games/alien_cake_addict.rs b/examples/games/alien_cake_addict.rs index 8748b405d54bb..c543c7c91d9a7 100644 --- a/examples/games/alien_cake_addict.rs +++ b/examples/games/alien_cake_addict.rs @@ -2,11 +2,7 @@ use std::f32::consts::PI; -use bevy::{ - ecs::schedule::{ReportExecutionOrderAmbiguities, SystemSet}, - prelude::*, - time::FixedTimestep, -}; +use bevy::{ecs::schedule::SystemSet, prelude::*, time::FixedTimestep}; use rand::Rng; #[derive(Clone, Eq, PartialEq, Debug, Hash)] @@ -19,7 +15,6 @@ fn main() { App::new() .init_resource::() .add_plugins(DefaultPlugins) - .insert_resource(ReportExecutionOrderAmbiguities) .add_state(GameState::Playing) .add_startup_system(setup_cameras) .add_system_set(SystemSet::on_enter(GameState::Playing).with_system(setup)) From 93dafc9d5b8d81e495e8a0d62b7f9d02c4245343 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Thu, 22 Sep 2022 19:56:15 -0400 Subject: [PATCH 09/11] improve a name --- crates/bevy_ecs/src/schedule/ambiguity_detection.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 255cc24e21782..2111690dbbb53 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -60,7 +60,7 @@ impl SystemStage { /// /// The result may be incorrect if this stage has not been initialized with `world`. fn ambiguities(&self, world: &World) -> Vec { - fn foo<'a>( + fn find_ambiguities_in<'a>( segment: SystemStageSegment, container: &'a [impl SystemContainer], world: &'a World, @@ -88,18 +88,18 @@ impl SystemStage { ) } - foo(SystemStageSegment::Parallel, &self.parallel, world) - .chain(foo( + find_ambiguities_in(SystemStageSegment::Parallel, &self.parallel, world) + .chain(find_ambiguities_in( SystemStageSegment::ExclusiveAtStart, &self.exclusive_at_start, world, )) - .chain(foo( + .chain(find_ambiguities_in( SystemStageSegment::ExclusiveBeforeCommands, &self.exclusive_before_commands, world, )) - .chain(foo( + .chain(find_ambiguities_in( SystemStageSegment::ExclusiveAtEnd, &self.exclusive_at_end, world, From 68aafec453daa286b734a991767f7758878cbd77 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Fri, 23 Sep 2022 11:49:17 -0400 Subject: [PATCH 10/11] Add a test case for grouping behavior --- .../src/schedule/ambiguity_detection.rs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 2111690dbbb53..16977b2e08016 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -300,6 +300,7 @@ fn find_ambiguities( #[cfg(test)] mod tests { + use super::*; // Required to make the derive macro behave use crate as bevy_ecs; use crate::event::Events; @@ -484,17 +485,29 @@ mod tests { let mut test_stage = SystemStage::parallel(); test_stage - // All 3 of these conflict with each other + // All 3 of these conflict with each other (`.at_start()` is the default configuration) .add_system(write_world_system.exclusive_system()) - .add_system(write_world_system.exclusive_system().at_end()) + .add_system(write_world_system.exclusive_system().at_start()) .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().at_end()) .add_system(write_world_system.exclusive_system().before_commands()); test_stage.run(&mut world); assert_eq!(test_stage.ambiguity_count(&world), 3); + assert_eq!( + test_stage.ambiguities(&world), + vec![SystemOrderAmbiguity { + segment: SystemStageSegment::ExclusiveAtStart, + conflicts: vec![], + system_names: vec![ + "bevy_ecs::schedule::ambiguity_detection::tests::res_system".to_owned(), + "bevy_ecs::schedule::ambiguity_detection::tests::write_world_system".to_owned(), + "bevy_ecs::schedule::ambiguity_detection::tests::write_world_system".to_owned(), + ] + }], + ); } // Tests for silencing and resolving ambiguities From eed091e895dfefb08013950bc07c56b173fb7981 Mon Sep 17 00:00:00 2001 From: JoJoJet Date: Fri, 23 Sep 2022 11:54:06 -0400 Subject: [PATCH 11/11] reduce churn --- .../src/schedule/ambiguity_detection.rs | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs index 16977b2e08016..3cc01961d2de3 100644 --- a/crates/bevy_ecs/src/schedule/ambiguity_detection.rs +++ b/crates/bevy_ecs/src/schedule/ambiguity_detection.rs @@ -6,6 +6,35 @@ use crate::component::ComponentId; use crate::schedule::{SystemContainer, SystemStage}; use crate::world::World; +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +struct SystemOrderAmbiguity { + segment: SystemStageSegment, + conflicts: Vec, + system_names: Vec, +} + +/// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? +#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] +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 SystemStage { /// Logs execution order ambiguities between systems. /// @@ -123,35 +152,6 @@ impl SystemStage { } } -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -struct SystemOrderAmbiguity { - segment: SystemStageSegment, - conflicts: Vec, - system_names: Vec, -} - -/// Which part of a [`SystemStage`] was a [`SystemOrderAmbiguity`] detected in? -#[derive(Debug, PartialEq, Eq, Clone, Copy, PartialOrd, Ord, Hash)] -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", - } - } -} - /// A set of systems that are all reported to be ambiguous with one another. #[derive(PartialEq, Eq, PartialOrd, Ord)] struct AmbiguityInfo {