From 5846b081c70d9e26aa41742ce85fa632ddc09235 Mon Sep 17 00:00:00 2001 From: Ratys Date: Fri, 5 Mar 2021 18:41:12 +0300 Subject: [PATCH 1/6] Many-to-many system labels proof of concept. --- crates/bevy_ecs/src/schedule/stage.rs | 113 +++++++++--------- .../bevy_ecs/src/schedule/system_container.rs | 28 +++-- .../src/schedule/system_descriptor.rs | 12 +- 3 files changed, 76 insertions(+), 77 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 7a601438bd0bf..e22969f69355a 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -229,9 +229,6 @@ impl SystemStage { Err(LabelNotFound(label)) => { panic!("No parallel system with label {:?} in stage.", label) } - Err(DuplicateLabel(label)) => { - panic!("Label {:?} already used by a parallel system.", label) - } Err(GraphCycles(labels)) => { panic!( "Found a dependency cycle in parallel systems: {:?}.", @@ -247,12 +244,6 @@ impl SystemStage { label ) } - Err(DuplicateLabel(label)) => { - panic!( - "Label {:?} already used by an exclusive system at start of stage.", - label - ) - } Err(GraphCycles(labels)) => { panic!( "Found a dependency cycle in exclusive systems at start of stage: {:?}.", @@ -268,12 +259,6 @@ impl SystemStage { label ) } - Err(DuplicateLabel(label)) => { - panic!( - "Label {:?} already used by an exclusive system before commands of stage.", - label - ) - } Err(GraphCycles(labels)) => { panic!( "Found a dependency cycle in exclusive systems before commands of stage: {:?}.", @@ -289,12 +274,6 @@ impl SystemStage { label ) } - Err(DuplicateLabel(label)) => { - panic!( - "Label {:?} already used by an exclusive system at end of stage.", - label - ) - } Err(GraphCycles(labels)) => { panic!( "Found a dependency cycle in exclusive systems at end of stage: {:?}.", @@ -362,7 +341,6 @@ impl SystemStage { enum DependencyGraphError { LabelNotFound(Box), - DuplicateLabel(Box), GraphCycles(Vec>), } @@ -392,45 +370,48 @@ fn sort_systems(systems: &mut Vec) -> Result<(), Dependenc fn build_dependency_graph( systems: &[impl SystemContainer], ) -> Result>, DependencyGraphError> { - let mut labels = HashMap::::default(); - for (label, index) in systems.iter().enumerate().filter_map(|(index, container)| { + let mut labels = HashMap::::default(); + for (label, index) in systems.iter().enumerate().flat_map(|(index, container)| { container - .label() - .as_ref() + .labels() + .iter() .cloned() - .map(|label| (label, index)) + .map(move |label| (label, index)) }) { - if labels.contains_key(&label) { - return Err(DependencyGraphError::DuplicateLabel(label)); - } - labels.insert(label, index); + labels + .entry(label) + .or_insert_with(|| FixedBitSet::with_capacity(systems.len())) + .insert(index); } - let mut graph = HashMap::default(); + let mut graph = HashMap::with_capacity_and_hasher(systems.len(), Default::default()); for (system_index, container) in systems.iter().enumerate() { - let dependencies = graph.entry(system_index).or_insert_with(Vec::new); + let dependencies = graph + .entry(system_index) + .or_insert_with(|| FixedBitSet::with_capacity(systems.len())); for label in container.after() { match labels.get(label) { - Some(dependency) => { - if !dependencies.contains(dependency) { - dependencies.push(*dependency); - } - } + Some(new_dependencies) => dependencies.extend(new_dependencies.ones()), None => return Err(DependencyGraphError::LabelNotFound(label.clone())), } } for label in container.before() { match labels.get(label) { - Some(dependant) => { - let dependencies = graph.entry(*dependant).or_insert_with(Vec::new); - if !dependencies.contains(&system_index) { - dependencies.push(system_index); + Some(dependants) => { + for dependant in dependants.ones() { + graph + .entry(dependant) + .or_insert_with(|| FixedBitSet::with_capacity(systems.len())) + .insert(system_index); } } None => return Err(DependencyGraphError::LabelNotFound(label.clone())), } } } - Ok(graph) + Ok(graph + .drain() + .map(|(system, dependencies)| (system, dependencies.ones().collect())) + .collect()) } /// Generates a topological order for the given graph. @@ -749,20 +730,30 @@ mod tests { } #[test] - #[should_panic( - expected = "Label \"empty\" already used by an exclusive system at start of stage." - )] - fn exclusive_duplicate_label() { + fn exclusive_multiple_labels() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(empty.exclusive_system().at_end().label("empty")) - .with_system(empty.exclusive_system().before_commands().label("empty")); + .with_system( + make_exclusive(1) + .exclusive_system() + .label("first") + .after("0"), + ) + .with_system(make_exclusive(2).exclusive_system().after("first")) + .with_system( + make_exclusive(0) + .exclusive_system() + .label("first") + .label("0"), + ); stage.run(&mut world); - let mut stage = SystemStage::parallel() - .with_system(empty.exclusive_system().label("empty")) - .with_system(empty.exclusive_system().label("empty")); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 0, 1, 2] + ); } #[test] @@ -949,14 +940,20 @@ mod tests { } #[test] - #[should_panic(expected = "Label \"empty\" already used by a parallel system.")] - fn parallel_duplicate_label() { + fn parallel_multiple_labels() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(empty.system().label("empty")) - .with_system(empty.system().label("empty")); + .with_system(make_parallel!(1).system().label("first").after("0")) + .with_system(make_parallel!(2).system().after("first")) + .with_system(make_parallel!(0).system().label("first").label("0")); + stage.run(&mut world); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 0, 1, 2] + ); } #[test] @@ -1140,8 +1137,8 @@ mod tests { .drain(..) .map(|(index_a, index_b)| { ( - systems[index_a].label().clone().unwrap(), - systems[index_b].label().clone().unwrap(), + systems[index_a].labels()[0].clone(), + systems[index_b].labels()[0].clone(), ) }) .collect() diff --git a/crates/bevy_ecs/src/schedule/system_container.rs b/crates/bevy_ecs/src/schedule/system_container.rs index 28f313904deee..df16c40f211c5 100644 --- a/crates/bevy_ecs/src/schedule/system_container.rs +++ b/crates/bevy_ecs/src/schedule/system_container.rs @@ -12,7 +12,7 @@ pub(super) trait SystemContainer { fn dependencies(&self) -> &[usize]; fn set_dependencies(&mut self, dependencies: impl IntoIterator); fn system_set(&self) -> usize; - fn label(&self) -> &Option; + fn labels(&self) -> &[BoxedSystemLabel]; fn before(&self) -> &[BoxedSystemLabel]; fn after(&self) -> &[BoxedSystemLabel]; fn ambiguity_sets(&self) -> &[BoxedAmbiguitySetLabel]; @@ -23,7 +23,7 @@ pub(super) struct ExclusiveSystemContainer { system: Box, dependencies: Vec, set: usize, - label: Option, + labels: Vec, before: Vec, after: Vec, ambiguity_sets: Vec, @@ -35,7 +35,7 @@ impl ExclusiveSystemContainer { system: descriptor.system, dependencies: Vec::new(), set, - label: descriptor.label, + labels: descriptor.labels, before: descriptor.before, after: descriptor.after, ambiguity_sets: descriptor.ambiguity_sets, @@ -49,8 +49,9 @@ impl ExclusiveSystemContainer { impl SystemContainer for ExclusiveSystemContainer { fn display_name(&self) -> Cow<'static, str> { - self.label - .as_ref() + // TODO: sensible display names. + self.labels + .get(0) .map(|l| Cow::Owned(format!("{:?}", l))) .unwrap_or_else(|| self.system.name()) } @@ -68,8 +69,8 @@ impl SystemContainer for ExclusiveSystemContainer { self.set } - fn label(&self) -> &Option { - &self.label + fn labels(&self) -> &[BoxedSystemLabel] { + &self.labels } fn before(&self) -> &[BoxedSystemLabel] { @@ -94,7 +95,7 @@ pub struct ParallelSystemContainer { pub(crate) should_run: bool, dependencies: Vec, set: usize, - label: Option, + labels: Vec, before: Vec, after: Vec, ambiguity_sets: Vec, @@ -102,8 +103,9 @@ pub struct ParallelSystemContainer { impl SystemContainer for ParallelSystemContainer { fn display_name(&self) -> Cow<'static, str> { - self.label - .as_ref() + // TODO: sensible display names. + self.labels + .get(0) .map(|l| Cow::Owned(format!("{:?}", l))) .unwrap_or_else(|| self.system().name()) } @@ -121,8 +123,8 @@ impl SystemContainer for ParallelSystemContainer { self.set } - fn label(&self) -> &Option { - &self.label + fn labels(&self) -> &[BoxedSystemLabel] { + &self.labels } fn before(&self) -> &[BoxedSystemLabel] { @@ -154,7 +156,7 @@ impl ParallelSystemContainer { should_run: false, set, dependencies: Vec::new(), - label: descriptor.label, + labels: descriptor.labels, before: descriptor.before, after: descriptor.after, ambiguity_sets: descriptor.ambiguity_sets, diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index ec1c3c508fa3c..cab2d8b7e595f 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -79,7 +79,7 @@ impl From for SystemDescriptor { /// Encapsulates a parallel system and information on when it run in a `SystemStage`. pub struct ParallelSystemDescriptor { pub(crate) system: BoxedSystem<(), ()>, - pub(crate) label: Option, + pub(crate) labels: Vec, pub(crate) before: Vec, pub(crate) after: Vec, pub(crate) ambiguity_sets: Vec, @@ -88,7 +88,7 @@ pub struct ParallelSystemDescriptor { fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor { ParallelSystemDescriptor { system, - label: None, + labels: Vec::new(), before: Vec::new(), after: Vec::new(), ambiguity_sets: Vec::new(), @@ -112,7 +112,7 @@ pub trait ParallelSystemDescriptorCoercion { impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor { fn label(mut self, label: impl SystemLabel) -> ParallelSystemDescriptor { - self.label = Some(Box::new(label)); + self.labels.push(Box::new(label)); self } @@ -181,7 +181,7 @@ pub(crate) enum InsertionPoint { /// Encapsulates an exclusive system and information on when it run in a `SystemStage`. pub struct ExclusiveSystemDescriptor { pub(crate) system: Box, - pub(crate) label: Option, + pub(crate) labels: Vec, pub(crate) before: Vec, pub(crate) after: Vec, pub(crate) ambiguity_sets: Vec, @@ -191,7 +191,7 @@ pub struct ExclusiveSystemDescriptor { fn new_exclusive_descriptor(system: Box) -> ExclusiveSystemDescriptor { ExclusiveSystemDescriptor { system, - label: None, + labels: Vec::new(), before: Vec::new(), after: Vec::new(), ambiguity_sets: Vec::new(), @@ -226,7 +226,7 @@ pub trait ExclusiveSystemDescriptorCoercion { impl ExclusiveSystemDescriptorCoercion for ExclusiveSystemDescriptor { fn label(mut self, label: impl SystemLabel) -> ExclusiveSystemDescriptor { - self.label = Some(Box::new(label)); + self.labels.push(Box::new(label)); self } From 601d8e499da38d94590e10c103adee3619f4b782 Mon Sep 17 00:00:00 2001 From: Ratys Date: Sat, 6 Mar 2021 18:57:25 +0300 Subject: [PATCH 2/6] Better graph cycle reporting; made unknown labels a warning; made display names always resolve to systems' names, renamed relevant methods accordingly; minor rearrangements in system_container. --- crates/bevy_ecs/src/schedule/stage.rs | 246 ++++++++---------- .../bevy_ecs/src/schedule/system_container.rs | 100 ++++--- 2 files changed, 157 insertions(+), 189 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index e22969f69355a..5ee74809766ea 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -2,12 +2,15 @@ use crate::{ schedule::{ BoxedSystemLabel, ExclusiveSystemContainer, InsertionPoint, ParallelExecutor, ParallelSystemContainer, ParallelSystemExecutor, RunCriteria, ShouldRun, - SingleThreadedExecutor, SystemContainer, SystemDescriptor, SystemLabel, SystemSet, + SingleThreadedExecutor, SystemContainer, SystemDescriptor, SystemSet, }, system::System, world::{World, WorldId}, }; -use bevy_utils::{tracing::info, HashMap, HashSet}; +use bevy_utils::{ + tracing::{info, warn}, + HashMap, HashSet, +}; use downcast_rs::{impl_downcast, Downcast}; use fixedbitset::FixedBitSet; use std::borrow::Cow; @@ -223,64 +226,40 @@ impl SystemStage { && self.uninitialized_before_commands.is_empty() && self.uninitialized_at_end.is_empty() ); - use DependencyGraphError::*; - match sort_systems(&mut self.parallel) { - Ok(()) => (), - Err(LabelNotFound(label)) => { - panic!("No parallel system with label {:?} in stage.", label) - } - Err(GraphCycles(labels)) => { - panic!( - "Found a dependency cycle in parallel systems: {:?}.", - labels - ) - } - } - match sort_systems(&mut self.exclusive_at_start) { - Ok(()) => (), - Err(LabelNotFound(label)) => { - panic!( - "No exclusive system with label {:?} at start of stage.", - label - ) - } - Err(GraphCycles(labels)) => { - panic!( - "Found a dependency cycle in exclusive systems at start of stage: {:?}.", - labels - ) - } - } - match sort_systems(&mut self.exclusive_before_commands) { - Ok(()) => (), - Err(LabelNotFound(label)) => { - panic!( - "No exclusive system with label {:?} before commands of stage.", - label - ) - } - Err(GraphCycles(labels)) => { - panic!( - "Found a dependency cycle in exclusive systems before commands of stage: {:?}.", - labels - ) - } - } - match sort_systems(&mut self.exclusive_at_end) { - Ok(()) => (), - Err(LabelNotFound(label)) => { - panic!( - "No exclusive system with label {:?} at end of stage.", - label - ) - } - Err(GraphCycles(labels)) => { - panic!( - "Found a dependency cycle in exclusive systems at end of stage: {:?}.", - labels - ) + fn sort_systems_unwrap( + systems: &mut Vec, + systems_description: &'static str, + ) { + if let Err(DependencyGraphError::GraphCycles(cycle)) = sort_systems(systems) { + use std::fmt::Write; + let mut string = format!("Found a dependency cycle in {}:", systems_description); + writeln!(string).unwrap(); + for (name, labels) in &cycle { + writeln!(string, " - {}", name).unwrap(); + writeln!( + string, + " wants to be after (because of labels {:?})", + labels + ) + .unwrap(); + } + writeln!(string, " - {}", cycle[0].0).unwrap(); + panic!(string); } } + sort_systems_unwrap(&mut self.parallel, "parallel systems"); + sort_systems_unwrap( + &mut self.exclusive_at_start, + "exclusive systems at start of stage", + ); + sort_systems_unwrap( + &mut self.exclusive_before_commands, + "exclusive systems before commands of stage", + ); + sort_systems_unwrap( + &mut self.exclusive_at_end, + "exclusive systems at end of stage", + ); } /// Logs execution order ambiguities between systems. System orders must be fresh. @@ -296,8 +275,8 @@ impl SystemStage { writeln!( string, " -- {:?} and {:?}", - systems[index_a].display_name(), - systems[index_b].display_name() + systems[index_a].name(), + systems[index_b].name() ) .unwrap(); } @@ -340,13 +319,12 @@ impl SystemStage { } enum DependencyGraphError { - LabelNotFound(Box), - GraphCycles(Vec>), + GraphCycles(Vec<(Cow<'static, str>, Vec)>), } /// Sorts given system containers topologically and populates their resolved dependencies. fn sort_systems(systems: &mut Vec) -> Result<(), DependencyGraphError> { - let mut graph = build_dependency_graph(systems)?; + let mut graph = build_dependency_graph(systems); let order = topological_order(systems, &graph)?; let mut order_inverted = order.iter().enumerate().collect::>(); order_inverted.sort_unstable_by_key(|(_, &key)| key); @@ -355,8 +333,8 @@ fn sort_systems(systems: &mut Vec) -> Result<(), Dependenc graph .get_mut(&index) .unwrap() - .drain(..) - .map(|index| order_inverted[index].0), + .drain() + .map(|(index, _)| order_inverted[index].0), ); } let mut temp = systems.drain(..).map(Some).collect::>(); @@ -369,8 +347,8 @@ fn sort_systems(systems: &mut Vec) -> Result<(), Dependenc /// Constructs a dependency graph of given system containers. fn build_dependency_graph( systems: &[impl SystemContainer], -) -> Result>, DependencyGraphError> { - let mut labels = HashMap::::default(); +) -> HashMap>> { + let mut labelled_systems = HashMap::::default(); for (label, index) in systems.iter().enumerate().flat_map(|(index, container)| { container .labels() @@ -378,81 +356,103 @@ fn build_dependency_graph( .cloned() .map(move |label| (label, index)) }) { - labels + labelled_systems .entry(label) .or_insert_with(|| FixedBitSet::with_capacity(systems.len())) .insert(index); } let mut graph = HashMap::with_capacity_and_hasher(systems.len(), Default::default()); for (system_index, container) in systems.iter().enumerate() { - let dependencies = graph - .entry(system_index) - .or_insert_with(|| FixedBitSet::with_capacity(systems.len())); + let dependencies = graph.entry(system_index).or_insert_with(HashMap::default); for label in container.after() { - match labels.get(label) { - Some(new_dependencies) => dependencies.extend(new_dependencies.ones()), - None => return Err(DependencyGraphError::LabelNotFound(label.clone())), + match labelled_systems.get(label) { + Some(new_dependencies) => { + for dependency in new_dependencies.ones() { + dependencies + .entry(dependency) + .or_insert_with(HashSet::default) + .insert(label.clone()); + } + } + None => warn!( + "System {} wants to be after unknown system label: {:?}", + systems[system_index].name(), + label + ), } } for label in container.before() { - match labels.get(label) { + match labelled_systems.get(label) { Some(dependants) => { for dependant in dependants.ones() { graph .entry(dependant) - .or_insert_with(|| FixedBitSet::with_capacity(systems.len())) - .insert(system_index); + .or_insert_with(HashMap::default) + .entry(system_index) + .or_insert_with(HashSet::default) + .insert(label.clone()); } } - None => return Err(DependencyGraphError::LabelNotFound(label.clone())), + None => warn!( + "System {} wants to be before unknown system label: {:?}", + systems[system_index].name(), + label + ), } } } - Ok(graph - .drain() - .map(|(system, dependencies)| (system, dependencies.ones().collect())) - .collect()) + graph } /// Generates a topological order for the given graph. fn topological_order( - systems: &[impl SystemContainer], - graph: &HashMap>, + _systems: &[impl SystemContainer], + graph: &HashMap>>, ) -> Result, DependencyGraphError> { fn check_if_cycles_and_visit( node: &usize, - graph: &HashMap>, + graph: &HashMap>>, sorted: &mut Vec, unvisited: &mut HashSet, - current: &mut HashSet, + current: &mut Vec, ) -> bool { if current.contains(node) { return true; } else if !unvisited.remove(node) { return false; } - current.insert(*node); - for dependency in graph.get(node).unwrap() { + current.push(*node); + for dependency in graph.get(node).unwrap().keys() { if check_if_cycles_and_visit(dependency, &graph, sorted, unvisited, current) { return true; } } sorted.push(*node); - current.remove(node); + current.pop(); false } let mut sorted = Vec::with_capacity(graph.len()); - let mut current = HashSet::with_capacity_and_hasher(graph.len(), Default::default()); + let mut current = Vec::with_capacity(graph.len()); let mut unvisited = HashSet::with_capacity_and_hasher(graph.len(), Default::default()); unvisited.extend(graph.keys().cloned()); while let Some(node) = unvisited.iter().next().cloned() { if check_if_cycles_and_visit(&node, graph, &mut sorted, &mut unvisited, &mut current) { - return Err(DependencyGraphError::GraphCycles( - current - .iter() - .map(|index| systems[*index].display_name()) - .collect::>(), + let mut cycle = Vec::new(); + for index in 0..current.len() - 1 { + cycle.push(( + _systems[current[index]].name(), + graph[¤t[index]][¤t[index + 1]] + .iter() + .cloned() + .collect(), + )); + } + let last = *current.last().unwrap(); + cycle.push(( + _systems[last].name(), + graph[&last][¤t[0]].iter().cloned().collect(), )); + return Err(DependencyGraphError::GraphCycles(cycle)); } } Ok(sorted) @@ -650,8 +650,6 @@ mod tests { }}; } - fn empty() {} - fn resettable_run_once(mut has_ran: ResMut) -> ShouldRun { if !*has_ran { *has_ran = true; @@ -718,17 +716,6 @@ mod tests { ); } - #[test] - #[should_panic(expected = "No exclusive system with label \"empty\" at start of stage.")] - fn exclusive_unknown_label() { - let mut world = World::new(); - world.insert_resource(Vec::::new()); - let mut stage = SystemStage::parallel() - .with_system(empty.exclusive_system().at_end().label("empty")) - .with_system(empty.exclusive_system().after("empty")); - stage.run(&mut world); - } - #[test] fn exclusive_multiple_labels() { let mut world = World::new(); @@ -928,17 +915,6 @@ mod tests { stage.run(&mut world); } - #[test] - #[should_panic(expected = "No parallel system with label \"empty\" in stage.")] - fn parallel_unknown_label() { - let mut world = World::new(); - world.insert_resource(Vec::::new()); - let mut stage = SystemStage::parallel() - .with_system(empty.system()) - .with_system(empty.system().after("empty")); - stage.run(&mut world); - } - #[test] fn parallel_multiple_labels() { let mut world = World::new(); @@ -1130,7 +1106,7 @@ mod tests { fn ambiguity_detection() { use super::{find_ambiguities, SystemContainer}; - fn find_ambiguities_labels( + fn find_ambiguities_first_labels( systems: &[impl SystemContainer], ) -> Vec<(BoxedSystemLabel, BoxedSystemLabel)> { find_ambiguities(systems) @@ -1168,7 +1144,7 @@ mod tests { .with_system(component.system().label("4")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("4"))) || ambiguities.contains(&(Box::new("4"), Box::new("1"))) @@ -1183,7 +1159,7 @@ mod tests { .with_system(resource.system().label("4")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("4"))) || ambiguities.contains(&(Box::new("4"), Box::new("1"))) @@ -1208,7 +1184,7 @@ mod tests { .with_system(resource.system().label("4")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("0"), Box::new("3"))) || ambiguities.contains(&(Box::new("3"), Box::new("0"))) @@ -1233,7 +1209,7 @@ mod tests { .with_system(resource.system().label("4").in_ambiguity_set("a")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("0"), Box::new("3"))) || ambiguities.contains(&(Box::new("3"), Box::new("0"))) @@ -1246,7 +1222,7 @@ mod tests { .with_system(component.system().label("2")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("0"), Box::new("1"))) || ambiguities.contains(&(Box::new("1"), Box::new("0"))) @@ -1259,7 +1235,7 @@ mod tests { .with_system(component.system().label("2").after("0")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("2"))) || ambiguities.contains(&(Box::new("2"), Box::new("1"))) @@ -1273,7 +1249,7 @@ mod tests { .with_system(component.system().label("3").after("1").after("2")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("2"))) || ambiguities.contains(&(Box::new("2"), Box::new("1"))) @@ -1287,7 +1263,7 @@ mod tests { .with_system(component.system().label("3").after("1").after("2")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert_eq!(ambiguities.len(), 0); let mut stage = SystemStage::parallel() @@ -1297,7 +1273,7 @@ mod tests { .with_system(component.system().label("3").after("1").after("2")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("2"))) || ambiguities.contains(&(Box::new("2"), Box::new("1"))) @@ -1329,7 +1305,7 @@ mod tests { ); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("2"))) || ambiguities.contains(&(Box::new("2"), Box::new("1"))) @@ -1381,7 +1357,7 @@ mod tests { ); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert_eq!(ambiguities.len(), 0); let mut stage = SystemStage::parallel() @@ -1415,7 +1391,7 @@ mod tests { ); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.parallel); + let ambiguities = find_ambiguities_first_labels(&stage.parallel); assert!( ambiguities.contains(&(Box::new("1"), Box::new("4"))) || ambiguities.contains(&(Box::new("4"), Box::new("1"))) @@ -1449,7 +1425,7 @@ mod tests { .with_system(empty.exclusive_system().label("6").after("2").after("5")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start); + let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); assert!( ambiguities.contains(&(Box::new("1"), Box::new("3"))) || ambiguities.contains(&(Box::new("3"), Box::new("1"))) @@ -1486,7 +1462,7 @@ mod tests { .with_system(empty.exclusive_system().label("6").after("2").after("5")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start); + let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); assert!( ambiguities.contains(&(Box::new("2"), Box::new("3"))) || ambiguities.contains(&(Box::new("3"), Box::new("2"))) @@ -1512,7 +1488,7 @@ mod tests { .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a")); stage.initialize_systems(&mut world); stage.rebuild_orders_and_dependencies(); - let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start); + let ambiguities = find_ambiguities_first_labels(&stage.exclusive_at_start); assert_eq!(ambiguities.len(), 0); } diff --git a/crates/bevy_ecs/src/schedule/system_container.rs b/crates/bevy_ecs/src/schedule/system_container.rs index df16c40f211c5..60cc8e6f8b778 100644 --- a/crates/bevy_ecs/src/schedule/system_container.rs +++ b/crates/bevy_ecs/src/schedule/system_container.rs @@ -8,7 +8,7 @@ use crate::{ use std::{borrow::Cow, ptr::NonNull}; pub(super) trait SystemContainer { - fn display_name(&self) -> Cow<'static, str>; + fn name(&self) -> Cow<'static, str>; fn dependencies(&self) -> &[usize]; fn set_dependencies(&mut self, dependencies: impl IntoIterator); fn system_set(&self) -> usize; @@ -48,12 +48,8 @@ impl ExclusiveSystemContainer { } impl SystemContainer for ExclusiveSystemContainer { - fn display_name(&self) -> Cow<'static, str> { - // TODO: sensible display names. - self.labels - .get(0) - .map(|l| Cow::Owned(format!("{:?}", l))) - .unwrap_or_else(|| self.system.name()) + fn name(&self) -> Cow<'static, str> { + self.system.name() } fn dependencies(&self) -> &[usize] { @@ -101,51 +97,6 @@ pub struct ParallelSystemContainer { ambiguity_sets: Vec, } -impl SystemContainer for ParallelSystemContainer { - fn display_name(&self) -> Cow<'static, str> { - // TODO: sensible display names. - self.labels - .get(0) - .map(|l| Cow::Owned(format!("{:?}", l))) - .unwrap_or_else(|| self.system().name()) - } - - fn dependencies(&self) -> &[usize] { - &self.dependencies - } - - fn set_dependencies(&mut self, dependencies: impl IntoIterator) { - self.dependencies.clear(); - self.dependencies.extend(dependencies); - } - - fn system_set(&self) -> usize { - self.set - } - - fn labels(&self) -> &[BoxedSystemLabel] { - &self.labels - } - - fn before(&self) -> &[BoxedSystemLabel] { - &self.before - } - - fn after(&self) -> &[BoxedSystemLabel] { - &self.after - } - - fn ambiguity_sets(&self) -> &[BoxedAmbiguitySetLabel] { - &self.ambiguity_sets - } - - fn is_compatible(&self, other: &Self) -> bool { - self.system() - .component_access() - .is_compatible(other.system().component_access()) - } -} - unsafe impl Send for ParallelSystemContainer {} unsafe impl Sync for ParallelSystemContainer {} @@ -163,8 +114,8 @@ impl ParallelSystemContainer { } } - pub fn display_name(&self) -> Cow<'static, str> { - SystemContainer::display_name(self) + pub fn name(&self) -> Cow<'static, str> { + SystemContainer::name(self) } pub fn system(&self) -> &dyn System { @@ -192,3 +143,44 @@ impl ParallelSystemContainer { &self.dependencies } } + +impl SystemContainer for ParallelSystemContainer { + fn name(&self) -> Cow<'static, str> { + self.system().name() + } + + fn dependencies(&self) -> &[usize] { + &self.dependencies + } + + fn set_dependencies(&mut self, dependencies: impl IntoIterator) { + self.dependencies.clear(); + self.dependencies.extend(dependencies); + } + + fn system_set(&self) -> usize { + self.set + } + + fn labels(&self) -> &[BoxedSystemLabel] { + &self.labels + } + + fn before(&self) -> &[BoxedSystemLabel] { + &self.before + } + + fn after(&self) -> &[BoxedSystemLabel] { + &self.after + } + + fn ambiguity_sets(&self) -> &[BoxedAmbiguitySetLabel] { + &self.ambiguity_sets + } + + fn is_compatible(&self, other: &Self) -> bool { + self.system() + .component_access() + .is_compatible(other.system().component_access()) + } +} From 807a72a131be006cf3cdec20e80667819fbbab5f Mon Sep 17 00:00:00 2001 From: Ratys Date: Sat, 6 Mar 2021 21:51:11 +0300 Subject: [PATCH 3/6] More tests, updated docs. --- crates/bevy_ecs/src/schedule/stage.rs | 167 +++++++++++++----- .../src/schedule/system_descriptor.rs | 17 +- 2 files changed, 131 insertions(+), 53 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 5ee74809766ea..85c351c3cae72 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -406,7 +406,7 @@ fn build_dependency_graph( /// Generates a topological order for the given graph. fn topological_order( - _systems: &[impl SystemContainer], + systems: &[impl SystemContainer], graph: &HashMap>>, ) -> Result, DependencyGraphError> { fn check_if_cycles_and_visit( @@ -440,7 +440,7 @@ fn topological_order( let mut cycle = Vec::new(); for index in 0..current.len() - 1 { cycle.push(( - _systems[current[index]].name(), + systems[current[index]].name(), graph[¤t[index]][¤t[index + 1]] .iter() .cloned() @@ -449,7 +449,7 @@ fn topological_order( } let last = *current.last().unwrap(); cycle.push(( - _systems[last].name(), + systems[last].name(), graph[&last][¤t[0]].iter().cloned().collect(), )); return Err(DependencyGraphError::GraphCycles(cycle)); @@ -717,23 +717,13 @@ mod tests { } #[test] - fn exclusive_multiple_labels() { + fn exclusive_after() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system( - make_exclusive(1) - .exclusive_system() - .label("first") - .after("0"), - ) - .with_system(make_exclusive(2).exclusive_system().after("first")) - .with_system( - make_exclusive(0) - .exclusive_system() - .label("first") - .label("0"), - ); + .with_system(make_exclusive(1).exclusive_system().label("1").after("0")) + .with_system(make_exclusive(2).exclusive_system().after("1")) + .with_system(make_exclusive(0).exclusive_system().label("0")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); @@ -744,13 +734,13 @@ mod tests { } #[test] - fn exclusive_after() { + fn exclusive_before() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_exclusive(1).exclusive_system().label("1").after("0")) - .with_system(make_exclusive(2).exclusive_system().after("1")) - .with_system(make_exclusive(0).exclusive_system().label("0")); + .with_system(make_exclusive(1).exclusive_system().label("1").before("2")) + .with_system(make_exclusive(2).exclusive_system().label("2")) + .with_system(make_exclusive(0).exclusive_system().before("1")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); @@ -761,30 +751,55 @@ mod tests { } #[test] - fn exclusive_before() { + fn exclusive_mixed() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_exclusive(1).exclusive_system().label("1").before("2")) .with_system(make_exclusive(2).exclusive_system().label("2")) - .with_system(make_exclusive(0).exclusive_system().before("1")); + .with_system(make_exclusive(1).exclusive_system().after("0").before("2")) + .with_system(make_exclusive(0).exclusive_system().label("0")) + .with_system(make_exclusive(4).exclusive_system().label("4")) + .with_system(make_exclusive(3).exclusive_system().after("2").before("4")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); assert_eq!( *world.get_resource::>().unwrap(), - vec![0, 1, 2, 0, 1, 2] + vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] ); } #[test] - fn exclusive_mixed() { + fn exclusive_multiple_labels() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_exclusive(2).exclusive_system().label("2")) - .with_system(make_exclusive(1).exclusive_system().after("0").before("2")) - .with_system(make_exclusive(0).exclusive_system().label("0")) + .with_system( + make_exclusive(1) + .exclusive_system() + .label("first") + .after("0"), + ) + .with_system(make_exclusive(2).exclusive_system().after("first")) + .with_system( + make_exclusive(0) + .exclusive_system() + .label("first") + .label("0"), + ); + stage.run(&mut world); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); + stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 0, 1, 2] + ); + + world.get_resource_mut::>().unwrap().clear(); + let mut stage = SystemStage::parallel() + .with_system(make_exclusive(2).exclusive_system().after("01").label("2")) + .with_system(make_exclusive(1).exclusive_system().label("01").after("0")) + .with_system(make_exclusive(0).exclusive_system().label("01").label("0")) .with_system(make_exclusive(4).exclusive_system().label("4")) .with_system(make_exclusive(3).exclusive_system().after("2").before("4")); stage.run(&mut world); @@ -794,6 +809,32 @@ mod tests { *world.get_resource::>().unwrap(), vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] ); + + world.get_resource_mut::>().unwrap().clear(); + let mut stage = SystemStage::parallel() + .with_system(make_exclusive(2).exclusive_system().label("234").label("2")) + .with_system( + make_exclusive(1) + .exclusive_system() + .before("234") + .after("0"), + ) + .with_system(make_exclusive(0).exclusive_system().label("0")) + .with_system(make_exclusive(4).exclusive_system().label("234").label("4")) + .with_system( + make_exclusive(3) + .exclusive_system() + .label("234") + .after("2") + .before("4"), + ); + stage.run(&mut world); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); + stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] + ); } #[test] @@ -916,13 +957,13 @@ mod tests { } #[test] - fn parallel_multiple_labels() { + fn parallel_after() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_parallel!(1).system().label("first").after("0")) - .with_system(make_parallel!(2).system().after("first")) - .with_system(make_parallel!(0).system().label("first").label("0")); + .with_system(make_parallel!(1).system().after("0").label("1")) + .with_system(make_parallel!(2).system().after("1")) + .with_system(make_parallel!(0).system().label("0")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); @@ -933,13 +974,13 @@ mod tests { } #[test] - fn parallel_after() { + fn parallel_before() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_parallel!(1).system().after("0").label("1")) - .with_system(make_parallel!(2).system().after("1")) - .with_system(make_parallel!(0).system().label("0")); + .with_system(make_parallel!(1).system().label("1").before("2")) + .with_system(make_parallel!(2).system().label("2")) + .with_system(make_parallel!(0).system().before("1")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); @@ -950,30 +991,45 @@ mod tests { } #[test] - fn parallel_before() { + fn parallel_mixed() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_parallel!(1).system().label("1").before("2")) .with_system(make_parallel!(2).system().label("2")) - .with_system(make_parallel!(0).system().before("1")); + .with_system(make_parallel!(1).system().after("0").before("2")) + .with_system(make_parallel!(0).system().label("0")) + .with_system(make_parallel!(4).system().label("4")) + .with_system(make_parallel!(3).system().after("2").before("4")); stage.run(&mut world); stage.set_executor(Box::new(SingleThreadedExecutor::default())); stage.run(&mut world); assert_eq!( *world.get_resource::>().unwrap(), - vec![0, 1, 2, 0, 1, 2] + vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] ); } #[test] - fn parallel_mixed() { + fn parallel_multiple_labels() { let mut world = World::new(); world.insert_resource(Vec::::new()); let mut stage = SystemStage::parallel() - .with_system(make_parallel!(2).system().label("2")) - .with_system(make_parallel!(1).system().after("0").before("2")) - .with_system(make_parallel!(0).system().label("0")) + .with_system(make_parallel!(1).system().label("first").after("0")) + .with_system(make_parallel!(2).system().after("first")) + .with_system(make_parallel!(0).system().label("first").label("0")); + stage.run(&mut world); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); + stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 0, 1, 2] + ); + + world.get_resource_mut::>().unwrap().clear(); + let mut stage = SystemStage::parallel() + .with_system(make_parallel!(2).system().after("01").label("2")) + .with_system(make_parallel!(1).system().label("01").after("0")) + .with_system(make_parallel!(0).system().label("01").label("0")) .with_system(make_parallel!(4).system().label("4")) .with_system(make_parallel!(3).system().after("2").before("4")); stage.run(&mut world); @@ -983,6 +1039,27 @@ mod tests { *world.get_resource::>().unwrap(), vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] ); + + world.get_resource_mut::>().unwrap().clear(); + let mut stage = SystemStage::parallel() + .with_system(make_parallel!(2).system().label("234").label("2")) + .with_system(make_parallel!(1).system().before("234").after("0")) + .with_system(make_parallel!(0).system().label("0")) + .with_system(make_parallel!(4).system().label("234").label("4")) + .with_system( + make_parallel!(3) + .system() + .label("234") + .after("2") + .before("4"), + ); + stage.run(&mut world); + stage.set_executor(Box::new(SingleThreadedExecutor::default())); + stage.run(&mut world); + assert_eq!( + *world.get_resource::>().unwrap(), + vec![0, 1, 2, 3, 4, 0, 1, 2, 3, 4] + ); } #[test] diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index cab2d8b7e595f..798385d58c521 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -13,8 +13,9 @@ use crate::{ /// * At end, accepts exclusive systems; runs after parallel systems' command buffers have /// been applied. /// -/// All systems can have a label attached to them; other systems in the same group can then specify -/// that they have to run before or after the system with that label using the `before` and `after` methods. +/// All systems can have one or several labels attached to them; other systems in the same group +/// can then specify that they have to run before or after systems with that label using the +/// `before` and `after` methods. /// /// # Example /// ``` @@ -96,13 +97,13 @@ fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescrip } pub trait ParallelSystemDescriptorCoercion { - /// Assigns a label to the system. + /// Assigns a label to the system; there can be more than one, and it doesn't have to be unique. fn label(self, label: impl SystemLabel) -> ParallelSystemDescriptor; - /// Specifies that the system should run before the system with given label. + /// Specifies that the system should run before systems with the given label. fn before(self, label: impl SystemLabel) -> ParallelSystemDescriptor; - /// Specifies that the system should run after the system with given label. + /// Specifies that the system should run after systems with the given label. fn after(self, label: impl SystemLabel) -> ParallelSystemDescriptor; /// Specifies that the system is exempt from execution order ambiguity detection @@ -200,13 +201,13 @@ fn new_exclusive_descriptor(system: Box) -> ExclusiveSystem } pub trait ExclusiveSystemDescriptorCoercion { - /// Assigns a label to the system. + /// Assigns a label to the system; there can be more than one, and it doesn't have to be unique. fn label(self, label: impl SystemLabel) -> ExclusiveSystemDescriptor; - /// Specifies that the system should run before the system with given label. + /// Specifies that the system should run before systems with the given label. fn before(self, label: impl SystemLabel) -> ExclusiveSystemDescriptor; - /// Specifies that the system should run after the system with given label. + /// Specifies that the system should run after systems with the given label. fn after(self, label: impl SystemLabel) -> ExclusiveSystemDescriptor; /// Specifies that the system is exempt from execution order ambiguity detection From 0297855c0907fba6bb681996398b8ea45c729262 Mon Sep 17 00:00:00 2001 From: Ratys Date: Sat, 6 Mar 2021 22:27:48 +0300 Subject: [PATCH 4/6] Rust 2021. --- crates/bevy_ecs/src/schedule/stage.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 85c351c3cae72..4e5e28fd49014 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -244,7 +244,7 @@ impl SystemStage { .unwrap(); } writeln!(string, " - {}", cycle[0].0).unwrap(); - panic!(string); + panic!("{}", string); } } sort_systems_unwrap(&mut self.parallel, "parallel systems"); From 2dcd38fbb9545d7b8ace541c8f56d40856b4a056 Mon Sep 17 00:00:00 2001 From: Ratys Date: Sun, 7 Mar 2021 20:53:27 +0300 Subject: [PATCH 5/6] Use `.window()` instead of indexing. --- crates/bevy_ecs/src/schedule/stage.rs | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 4e5e28fd49014..f9f488071ef84 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -438,20 +438,16 @@ fn topological_order( while let Some(node) = unvisited.iter().next().cloned() { if check_if_cycles_and_visit(&node, graph, &mut sorted, &mut unvisited, &mut current) { let mut cycle = Vec::new(); - for index in 0..current.len() - 1 { + let last_window = [*current.last().unwrap(), current[0]]; + let mut windows = current + .windows(2) + .chain(std::iter::once(&last_window as &[usize])); + while let Some(&[dependant, dependency]) = windows.next() { cycle.push(( - systems[current[index]].name(), - graph[¤t[index]][¤t[index + 1]] - .iter() - .cloned() - .collect(), + systems[dependant].name(), + graph[&dependant][&dependency].iter().cloned().collect(), )); } - let last = *current.last().unwrap(); - cycle.push(( - systems[last].name(), - graph[&last][¤t[0]].iter().cloned().collect(), - )); return Err(DependencyGraphError::GraphCycles(cycle)); } } From e00f6529e22488d39f95d647f1f7cc8ff6e37548 Mon Sep 17 00:00:00 2001 From: Ratys Date: Tue, 9 Mar 2021 09:19:38 +0300 Subject: [PATCH 6/6] Doc tweak; minor refactor. --- crates/bevy_ecs/src/schedule/stage.rs | 12 ++++++------ crates/bevy_ecs/src/schedule/system_descriptor.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index f9f488071ef84..a205c4a7d1997 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -232,19 +232,19 @@ impl SystemStage { ) { if let Err(DependencyGraphError::GraphCycles(cycle)) = sort_systems(systems) { use std::fmt::Write; - let mut string = format!("Found a dependency cycle in {}:", systems_description); - writeln!(string).unwrap(); + let mut message = format!("Found a dependency cycle in {}:", systems_description); + writeln!(message).unwrap(); for (name, labels) in &cycle { - writeln!(string, " - {}", name).unwrap(); + writeln!(message, " - {}", name).unwrap(); writeln!( - string, + message, " wants to be after (because of labels {:?})", labels ) .unwrap(); } - writeln!(string, " - {}", cycle[0].0).unwrap(); - panic!("{}", string); + writeln!(message, " - {}", cycle[0].0).unwrap(); + panic!("{}", message); } } sort_systems_unwrap(&mut self.parallel, "parallel systems"); diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index 798385d58c521..ce2adeda4512a 100644 --- a/crates/bevy_ecs/src/schedule/system_descriptor.rs +++ b/crates/bevy_ecs/src/schedule/system_descriptor.rs @@ -13,7 +13,7 @@ use crate::{ /// * At end, accepts exclusive systems; runs after parallel systems' command buffers have /// been applied. /// -/// All systems can have one or several labels attached to them; other systems in the same group +/// Systems can have one or more labels attached to them; other systems in the same group /// can then specify that they have to run before or after systems with that label using the /// `before` and `after` methods. ///