From d51130d4ab0ceefd8cc02d2963ac9c6705e66c1a Mon Sep 17 00:00:00 2001 From: Alexander Sepity Date: Tue, 9 Mar 2021 23:08:34 +0000 Subject: [PATCH] Many-to-many system labels (#1576) * Systems can now have more than one label attached to them. * System labels no longer have to be unique in the stage. Code like this is now possible: ```rust SystemStage::parallel() .with_system(system_0.system().label("group one").label("first")) .with_system(system_1.system().label("group one").after("first")) .with_system(system_2.system().after("group one")) ``` I've opted to use only the system name in ambiguity reporting, which previously was only a fallback; this, obviously, is because labels aren't one-to-one with systems anymore. We could allow users to name systems to improve this; we'll then have to think about whether or not we want to allow using the name as a label (this would, effectively, introduce implicit labelling, not all implications of which are clear to me yet wrt many-to-many labels). Dependency cycle errors are reported using the system names and only the labels that form the cycle, with each system-system "edge" in the cycle represented as one or several labels. Slightly unrelated: `.before()` and `.after()` with a label not attached to any system no longer crashes, and logs a warning instead. This is necessary to, for example, allow plugins to specify execution order with systems of potentially missing other plugins. --- crates/bevy_ecs/src/schedule/stage.rs | 434 ++++++++++-------- .../bevy_ecs/src/schedule/system_container.rs | 112 +++-- .../src/schedule/system_descriptor.rs | 29 +- 3 files changed, 308 insertions(+), 267 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index 94306c5f6affd..77caaf8aa1a81 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,85 +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(DuplicateLabel(label)) => { - panic!("Label {:?} already used by a parallel system.", 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(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: {:?}.", - 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(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: {:?}.", - 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(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: {:?}.", - 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 message = format!("Found a dependency cycle in {}:", systems_description); + writeln!(message).unwrap(); + for (name, labels) in &cycle { + writeln!(message, " - {}", name).unwrap(); + writeln!( + message, + " wants to be after (because of labels {:?})", + labels + ) + .unwrap(); + } + writeln!(message, " - {}", cycle[0].0).unwrap(); + panic!("{}", message); } } + 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. @@ -317,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(); } @@ -361,14 +319,12 @@ impl SystemStage { } enum DependencyGraphError { - LabelNotFound(Box), - DuplicateLabel(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); @@ -377,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::>(); @@ -391,87 +347,108 @@ 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(); - for (label, index) in systems.iter().enumerate().filter_map(|(index, container)| { +) -> HashMap>> { + let mut labelled_systems = 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); + labelled_systems + .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(HashMap::default); for label in container.after() { - match labels.get(label) { - Some(dependency) => { - if !dependencies.contains(dependency) { - dependencies.push(*dependency); + 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 => return Err(DependencyGraphError::LabelNotFound(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) { - Some(dependant) => { - let dependencies = graph.entry(*dependant).or_insert_with(Vec::new); - if !dependencies.contains(&system_index) { - dependencies.push(system_index); + match labelled_systems.get(label) { + Some(dependants) => { + for dependant in dependants.ones() { + graph + .entry(dependant) + .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) + graph } /// Generates a topological order for the given graph. fn topological_order( systems: &[impl SystemContainer], - graph: &HashMap>, + 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(); + 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[dependant].name(), + graph[&dependant][&dependency].iter().cloned().collect(), + )); + } + return Err(DependencyGraphError::GraphCycles(cycle)); } } Ok(sorted) @@ -669,8 +646,6 @@ mod tests { }}; } - fn empty() {} - fn resettable_run_once(mut has_ran: ResMut) -> ShouldRun { if !*has_ran { *has_ran = true; @@ -737,34 +712,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] - #[should_panic( - expected = "Label \"empty\" already used by an exclusive system at start of stage." - )] - fn exclusive_duplicate_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().before_commands().label("empty")); - stage.run(&mut world); - let mut stage = SystemStage::parallel() - .with_system(empty.exclusive_system().label("empty")) - .with_system(empty.exclusive_system().label("empty")); - stage.run(&mut world); - } - #[test] fn exclusive_after() { let mut world = World::new(); @@ -818,6 +765,74 @@ mod tests { ); } + #[test] + fn exclusive_multiple_labels() { + 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"), + ); + 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); + 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] + ); + + 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] fn exclusive_redundant_constraints() { let mut world = World::new(); @@ -937,28 +952,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] - #[should_panic(expected = "Label \"empty\" already used by a parallel system.")] - fn parallel_duplicate_label() { - 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")); - stage.run(&mut world); - } - #[test] fn parallel_after() { let mut world = World::new(); @@ -1012,6 +1005,59 @@ mod tests { ); } + #[test] + fn parallel_multiple_labels() { + 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")); + 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); + 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] + ); + + 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] fn parallel_redundant_constraints() { let mut world = World::new(); @@ -1133,15 +1179,15 @@ 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) .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() @@ -1171,7 +1217,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"))) @@ -1186,7 +1232,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"))) @@ -1211,7 +1257,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"))) @@ -1236,7 +1282,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"))) @@ -1249,7 +1295,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"))) @@ -1262,7 +1308,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"))) @@ -1276,7 +1322,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"))) @@ -1290,7 +1336,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() @@ -1300,7 +1346,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"))) @@ -1332,7 +1378,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"))) @@ -1384,7 +1430,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() @@ -1418,7 +1464,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"))) @@ -1452,7 +1498,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"))) @@ -1489,7 +1535,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"))) @@ -1515,7 +1561,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 28f313904deee..60cc8e6f8b778 100644 --- a/crates/bevy_ecs/src/schedule/system_container.rs +++ b/crates/bevy_ecs/src/schedule/system_container.rs @@ -8,11 +8,11 @@ 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; - 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, @@ -48,11 +48,8 @@ impl ExclusiveSystemContainer { } impl SystemContainer for ExclusiveSystemContainer { - fn display_name(&self) -> Cow<'static, str> { - self.label - .as_ref() - .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] { @@ -68,8 +65,8 @@ impl SystemContainer for ExclusiveSystemContainer { self.set } - fn label(&self) -> &Option { - &self.label + fn labels(&self) -> &[BoxedSystemLabel] { + &self.labels } fn before(&self) -> &[BoxedSystemLabel] { @@ -94,56 +91,12 @@ pub struct ParallelSystemContainer { pub(crate) should_run: bool, dependencies: Vec, set: usize, - label: Option, + labels: Vec, before: Vec, after: Vec, ambiguity_sets: Vec, } -impl SystemContainer for ParallelSystemContainer { - fn display_name(&self) -> Cow<'static, str> { - self.label - .as_ref() - .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 label(&self) -> &Option { - &self.label - } - - 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 {} @@ -154,15 +107,15 @@ 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, } } - 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 { @@ -190,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()) + } +} diff --git a/crates/bevy_ecs/src/schedule/system_descriptor.rs b/crates/bevy_ecs/src/schedule/system_descriptor.rs index ec1c3c508fa3c..ce2adeda4512a 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. +/// 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. /// /// # Example /// ``` @@ -79,7 +80,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 +89,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(), @@ -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 @@ -112,7 +113,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 +182,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 +192,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(), @@ -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 @@ -226,7 +227,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 }