Explicit execution order ambiguities API (bevyengine#1469)

Explicit execution order ambiguities API.

Author: Ratysz

crates/bevy_ecs/src/schedule/stage.rs

@@ -482,9 +482,20 @@ fn topological_order(
 /// Returns vector containing all pairs of indices of systems with ambiguous execution order.
 /// Systems must be topologically sorted beforehand.
 fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
+    let mut ambiguity_set_labels = HashMap::default();
+    for set in systems.iter().flat_map(|c| c.ambiguity_sets()) {
+        let len = ambiguity_set_labels.len();
+        ambiguity_set_labels.entry(set).or_insert(len);
+    }
+    let mut all_ambiguity_sets = Vec::<FixedBitSet>::with_capacity(systems.len());
     let mut all_dependencies = Vec::<FixedBitSet>::with_capacity(systems.len());
     let mut all_dependants = Vec::<FixedBitSet>::with_capacity(systems.len());
     for (index, container) in systems.iter().enumerate() {
+        let mut ambiguity_sets = FixedBitSet::with_capacity(ambiguity_set_labels.len());
+        for set in container.ambiguity_sets() {
+            ambiguity_sets.insert(ambiguity_set_labels[set]);
+        }
+        all_ambiguity_sets.push(ambiguity_sets);
         let mut dependencies = FixedBitSet::with_capacity(systems.len());
         for &dependency in container.dependencies() {
             dependencies.union_with(&all_dependencies[dependency]);
@@ -522,7 +533,10 @@ fn find_ambiguities(systems: &[impl SystemContainer]) -> Vec<(usize, usize)> {
         for index_b in full_bitset.difference(&relations)
         /*.take(index_a)*/
         {
-            if !processed.contains(index_b) && !systems[index_a].is_compatible(&systems[index_b]) {
+            if !processed.contains(index_b)
+                && all_ambiguity_sets[index_a].is_disjoint(&all_ambiguity_sets[index_b])
+                && !systems[index_a].is_compatible(&systems[index_b])
+            {
                 ambiguities.push((index_a, index_b));
             }
         }
@@ -1208,6 +1222,27 @@ mod tests {
         );
         assert_eq!(ambiguities.len(), 2);
 
+        let mut stage = SystemStage::parallel()
+            .with_system(component.system().label("0"))
+            .with_system(
+                resource
+                    .system()
+                    .label("1")
+                    .after("0")
+                    .in_ambiguity_set("a"),
+            )
+            .with_system(empty.system().label("2"))
+            .with_system(component.system().label("3").after("2").before("4"))
+            .with_system(resource.system().label("4").in_ambiguity_set("a"));
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.parallel);
+        assert!(
+            ambiguities.contains(&("0".into(), "3".into()))
+                || ambiguities.contains(&("3".into(), "0".into()))
+        );
+        assert_eq!(ambiguities.len(), 1);
+
         let mut stage = SystemStage::parallel()
             .with_system(component.system().label("0").before("2"))
             .with_system(component.system().label("1").before("2"))
@@ -1248,6 +1283,30 @@ mod tests {
         );
         assert_eq!(ambiguities.len(), 1);
 
+        let mut stage = SystemStage::parallel()
+            .with_system(component.system().label("0").before("1").before("2"))
+            .with_system(component.system().label("1").in_ambiguity_set("a"))
+            .with_system(component.system().label("2").in_ambiguity_set("a"))
+            .with_system(component.system().label("3").after("1").after("2"));
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.parallel);
+        assert_eq!(ambiguities.len(), 0);
+
+        let mut stage = SystemStage::parallel()
+            .with_system(component.system().label("0").before("1").before("2"))
+            .with_system(component.system().label("1").in_ambiguity_set("a"))
+            .with_system(component.system().label("2").in_ambiguity_set("b"))
+            .with_system(component.system().label("3").after("1").after("2"));
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.parallel);
+        assert!(
+            ambiguities.contains(&("1".into(), "2".into()))
+                || ambiguities.contains(&("2".into(), "1".into()))
+        );
+        assert_eq!(ambiguities.len(), 1);
+
         let mut stage = SystemStage::parallel()
             .with_system(
                 component
@@ -1300,6 +1359,76 @@ mod tests {
         );
         assert_eq!(ambiguities.len(), 6);
 
+        let mut stage = SystemStage::parallel()
+            .with_system(
+                component
+                    .system()
+                    .label("0")
+                    .before("1")
+                    .before("2")
+                    .before("3")
+                    .before("4"),
+            )
+            .with_system(component.system().label("1").in_ambiguity_set("a"))
+            .with_system(component.system().label("2").in_ambiguity_set("a"))
+            .with_system(component.system().label("3").in_ambiguity_set("a"))
+            .with_system(component.system().label("4").in_ambiguity_set("a"))
+            .with_system(
+                component
+                    .system()
+                    .label("5")
+                    .after("1")
+                    .after("2")
+                    .after("3")
+                    .after("4"),
+            );
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.parallel);
+        assert_eq!(ambiguities.len(), 0);
+
+        let mut stage = SystemStage::parallel()
+            .with_system(
+                component
+                    .system()
+                    .label("0")
+                    .before("1")
+                    .before("2")
+                    .before("3")
+                    .before("4"),
+            )
+            .with_system(component.system().label("1").in_ambiguity_set("a"))
+            .with_system(component.system().label("2").in_ambiguity_set("a"))
+            .with_system(
+                component
+                    .system()
+                    .label("3")
+                    .in_ambiguity_set("a")
+                    .in_ambiguity_set("b"),
+            )
+            .with_system(component.system().label("4").in_ambiguity_set("b"))
+            .with_system(
+                component
+                    .system()
+                    .label("5")
+                    .after("1")
+                    .after("2")
+                    .after("3")
+                    .after("4"),
+            );
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.parallel);
+        assert!(
+            ambiguities.contains(&("1".into(), "4".into()))
+                || ambiguities.contains(&("4".into(), "1".into()))
+        );
+        assert!(
+            ambiguities.contains(&("2".into(), "4".into()))
+                || ambiguities.contains(&("4".into(), "2".into()))
+        );
+        assert_eq!(ambiguities.len(), 2);
+
         let mut stage = SystemStage::parallel()
             .with_system(empty.exclusive_system().label("0"))
             .with_system(empty.exclusive_system().label("1").after("0"))
@@ -1349,5 +1478,44 @@ mod tests {
                 || ambiguities.contains(&("5".into(), "2".into()))
         );
         assert_eq!(ambiguities.len(), 6);
+
+        let mut stage = SystemStage::parallel()
+            .with_system(empty.exclusive_system().label("0").before("1").before("3"))
+            .with_system(empty.exclusive_system().label("1").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("2").after("1"))
+            .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("4").after("3").before("5"))
+            .with_system(empty.exclusive_system().label("5").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("6").after("2").after("5"));
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start);
+        assert!(
+            ambiguities.contains(&("2".into(), "3".into()))
+                || ambiguities.contains(&("3".into(), "2".into()))
+        );
+        assert!(
+            ambiguities.contains(&("1".into(), "4".into()))
+                || ambiguities.contains(&("4".into(), "1".into()))
+        );
+        assert!(
+            ambiguities.contains(&("2".into(), "4".into()))
+                || ambiguities.contains(&("4".into(), "2".into()))
+        );
+        assert!(
+            ambiguities.contains(&("2".into(), "5".into()))
+                || ambiguities.contains(&("5".into(), "2".into()))
+        );
+        assert_eq!(ambiguities.len(), 4);
+
+        let mut stage = SystemStage::parallel()
+            .with_system(empty.exclusive_system().label("0").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("1").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("2").in_ambiguity_set("a"))
+            .with_system(empty.exclusive_system().label("3").in_ambiguity_set("a"));
+        stage.initialize_systems(&mut world, &mut resources);
+        stage.rebuild_orders_and_dependencies();
+        let ambiguities = find_ambiguities_labels(&stage.exclusive_at_start);
+        assert_eq!(ambiguities.len(), 0);
     }
 }

crates/bevy_ecs/src/schedule/system_container.rs

@@ -10,6 +10,7 @@ pub(super) trait SystemContainer {
     fn label(&self) -> &Option<Cow<'static, str>>;
     fn before(&self) -> &[Cow<'static, str>];
     fn after(&self) -> &[Cow<'static, str>];
+    fn ambiguity_sets(&self) -> &[Cow<'static, str>];
     fn is_compatible(&self, other: &Self) -> bool;
 }
 
@@ -20,6 +21,7 @@ pub(super) struct ExclusiveSystemContainer {
     label: Option<Cow<'static, str>>,
     before: Vec<Cow<'static, str>>,
     after: Vec<Cow<'static, str>>,
+    ambiguity_sets: Vec<Cow<'static, str>>,
 }
 
 impl ExclusiveSystemContainer {
@@ -31,6 +33,7 @@ impl ExclusiveSystemContainer {
             label: descriptor.label,
             before: descriptor.before,
             after: descriptor.after,
+            ambiguity_sets: descriptor.ambiguity_sets,
         }
     }
 
@@ -72,6 +75,10 @@ impl SystemContainer for ExclusiveSystemContainer {
         &self.after
     }
 
+    fn ambiguity_sets(&self) -> &[Cow<'static, str>] {
+        &self.ambiguity_sets
+    }
+
     fn is_compatible(&self, _: &Self) -> bool {
         false
     }
@@ -85,6 +92,7 @@ pub struct ParallelSystemContainer {
     label: Option<Cow<'static, str>>,
     before: Vec<Cow<'static, str>>,
     after: Vec<Cow<'static, str>>,
+    ambiguity_sets: Vec<Cow<'static, str>>,
 }
 
 impl SystemContainer for ParallelSystemContainer {
@@ -120,6 +128,10 @@ impl SystemContainer for ParallelSystemContainer {
         &self.after
     }
 
+    fn ambiguity_sets(&self) -> &[Cow<'static, str>] {
+        &self.ambiguity_sets
+    }
+
     fn is_compatible(&self, other: &Self) -> bool {
         self.system()
             .component_access()
@@ -144,6 +156,7 @@ impl ParallelSystemContainer {
             label: descriptor.label,
             before: descriptor.before,
             after: descriptor.after,
+            ambiguity_sets: descriptor.ambiguity_sets,
         }
     }
 

crates/bevy_ecs/src/schedule/system_descriptor.rs

@@ -75,6 +75,7 @@ pub struct ParallelSystemDescriptor {
     pub(crate) label: Option<Cow<'static, str>>,
     pub(crate) before: Vec<Cow<'static, str>>,
     pub(crate) after: Vec<Cow<'static, str>>,
+    pub(crate) ambiguity_sets: Vec<Cow<'static, str>>,
 }
 
 fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescriptor {
@@ -83,6 +84,7 @@ fn new_parallel_descriptor(system: BoxedSystem<(), ()>) -> ParallelSystemDescrip
         label: None,
         before: Vec::new(),
         after: Vec::new(),
+        ambiguity_sets: Vec::new(),
     }
 }
 
@@ -95,6 +97,10 @@ pub trait ParallelSystemDescriptorCoercion {
 
     /// Specifies that the system should run after the system with given label.
     fn after(self, label: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor;
+
+    /// Specifies that the system is exempt from execution order ambiguity detection
+    /// with other systems in this set.
+    fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor;
 }
 
 impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor {
@@ -112,6 +118,11 @@ impl ParallelSystemDescriptorCoercion for ParallelSystemDescriptor {
         self.after.push(label.into());
         self
     }
+
+    fn in_ambiguity_set(mut self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
+        self.ambiguity_sets.push(set.into());
+        self
+    }
 }
 
 impl<S> ParallelSystemDescriptorCoercion for S
@@ -129,6 +140,10 @@ where
     fn after(self, label: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
         new_parallel_descriptor(Box::new(self)).after(label)
     }
+
+    fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
+        new_parallel_descriptor(Box::new(self)).in_ambiguity_set(set)
+    }
 }
 
 impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> {
@@ -143,6 +158,10 @@ impl ParallelSystemDescriptorCoercion for BoxedSystem<(), ()> {
     fn after(self, label: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
         new_parallel_descriptor(self).after(label)
     }
+
+    fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ParallelSystemDescriptor {
+        new_parallel_descriptor(self).in_ambiguity_set(set)
+    }
 }
 
 #[derive(Debug, Clone, Copy)]
@@ -158,6 +177,7 @@ pub struct ExclusiveSystemDescriptor {
     pub(crate) label: Option<Cow<'static, str>>,
     pub(crate) before: Vec<Cow<'static, str>>,
     pub(crate) after: Vec<Cow<'static, str>>,
+    pub(crate) ambiguity_sets: Vec<Cow<'static, str>>,
     pub(crate) insertion_point: InsertionPoint,
 }
 
@@ -167,6 +187,7 @@ fn new_exclusive_descriptor(system: Box<dyn ExclusiveSystem>) -> ExclusiveSystem
         label: None,
         before: Vec::new(),
         after: Vec::new(),
+        ambiguity_sets: Vec::new(),
         insertion_point: InsertionPoint::AtStart,
     }
 }
@@ -181,6 +202,10 @@ pub trait ExclusiveSystemDescriptorCoercion {
     /// Specifies that the system should run after the system with given label.
     fn after(self, label: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor;
 
+    /// Specifies that the system is exempt from execution order ambiguity detection
+    /// with other systems in this set.
+    fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor;
+
     /// Specifies that the system should run with other exclusive systems at the start of stage.
     fn at_start(self) -> ExclusiveSystemDescriptor;
 
@@ -208,6 +233,11 @@ impl ExclusiveSystemDescriptorCoercion for ExclusiveSystemDescriptor {
         self
     }
 
+    fn in_ambiguity_set(mut self, set: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor {
+        self.ambiguity_sets.push(set.into());
+        self
+    }
+
     fn at_start(mut self) -> ExclusiveSystemDescriptor {
         self.insertion_point = InsertionPoint::AtStart;
         self
@@ -240,6 +270,10 @@ where
         new_exclusive_descriptor(Box::new(self)).after(label)
     }
 
+    fn in_ambiguity_set(self, set: impl Into<Cow<'static, str>>) -> ExclusiveSystemDescriptor {
+        new_exclusive_descriptor(Box::new(self)).in_ambiguity_set(set)
+    }
+
     fn at_start(self) -> ExclusiveSystemDescriptor {
         new_exclusive_descriptor(Box::new(self)).at_start()
     }

crates/bevy_transform/src/lib.rs

@@ -7,8 +7,7 @@ pub mod prelude {
 }
 
 use bevy_app::{prelude::*, startup_stage};
-use bevy_ecs::IntoSystem;
-use bevy_ecs::ParallelSystemDescriptorCoercion;
+use bevy_ecs::{IntoSystem, ParallelSystemDescriptorCoercion};
 use bevy_reflect::RegisterTypeBuilder;
 use prelude::{parent_update_system, Children, GlobalTransform, Parent, PreviousParent, Transform};