collective_run_if for SystemConfigs via anonymous system sets

Author: geieredgar

crates/bevy_app/src/app.rs

@@ -380,8 +380,10 @@ impl App {
     /// # fn system_a() {}
     /// # fn system_b() {}
     /// # fn system_c() {}
+    /// # fn should_run() -> bool { true }
     /// #
     /// app.add_systems(Update, (system_a, system_b, system_c));
+    /// app.add_systems(Update, (system_a, system_b).collective_run_if(should_run));
     /// ```
     pub fn add_systems<M>(
         &mut self,

crates/bevy_ecs/src/schedule/config.rs

@@ -59,6 +59,7 @@ pub enum SystemConfigs {
     SystemConfig(SystemConfig),
     Configs {
         configs: Vec<SystemConfigs>,
+        collective_conditions: Vec<BoxedCondition>,
         /// If `true`, adds `before -> after` ordering constraints between the successive elements.
         chained: bool,
     },
@@ -123,6 +124,20 @@ impl SystemConfigs {
         }
     }
 
+    fn collective_run_if_inner(&mut self, condition: BoxedCondition) {
+        match self {
+            SystemConfigs::SystemConfig(config) => {
+                config.conditions.push(condition);
+            }
+            SystemConfigs::Configs {
+                collective_conditions,
+                ..
+            } => {
+                collective_conditions.push(condition);
+            }
+        }
+    }
+
     fn distributive_run_if_inner<M>(&mut self, condition: impl Condition<M> + Clone) {
         match self {
             SystemConfigs::SystemConfig(config) => {
@@ -199,6 +214,40 @@ where
         self.into_configs().after(set)
     }
 
+    /// Add a single run condition for all contained systems.
+    ///
+    /// The [`Condition`] will be evaluated at most once (per schedule run),
+    /// when one of the contained systems is first run.
+    ///
+    /// This is equivalent to adding each system to an common set and configuring
+    /// the run condition on that set, as shown below:
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # use bevy_ecs::prelude::*;
+    /// # let mut app = Schedule::new();
+    /// # fn a() {}
+    /// # fn b() {}
+    /// # fn condition() -> bool { true }
+    /// # #[derive(SystemSet, Debug, Eq, PartialEq, Hash, Clone, Copy)]
+    /// # struct C;
+    /// app.add_systems((a, b).collective_run_if(condition));
+    /// app.add_systems((a, b).in_set(C)).configure_set(C.run_if(condition));
+    /// ```
+    ///
+    /// # Note
+    ///
+    /// Because the condition will only be evaluated once, there is no guarantee that the condition
+    /// is upheld after the first system has run. You need to make sure that no other systems that
+    /// could invalidate the condition are scheduled inbetween the first and last run system.
+    ///
+    /// Use [`distributive_run_if`](IntoSystemConfigs::distributive_run_if) if you want the
+    /// condition to be evaluated for each individual system, right before one is run.
+    fn collective_run_if<P>(self, condition: impl Condition<P>) -> SystemConfigs {
+        self.into_configs().collective_run_if(condition)
+    }
+
     /// Add a run condition to each contained system.
     ///
     /// Each system will receive its own clone of the [`Condition`] and will only run
@@ -319,6 +368,11 @@ impl IntoSystemConfigs<()> for SystemConfigs {
         self
     }
 
+    fn collective_run_if<M>(mut self, condition: impl Condition<M>) -> Self {
+        self.collective_run_if_inner(new_condition(condition));
+        self
+    }
+
     fn distributive_run_if<M>(mut self, condition: impl Condition<M> + Clone) -> SystemConfigs {
         self.distributive_run_if_inner(condition);
         self
@@ -364,6 +418,7 @@ macro_rules! impl_system_collection {
                 let ($($sys,)*) = self;
                 SystemConfigs::Configs {
                     configs: vec![$($sys.into_configs(),)*],
+                    collective_conditions: Vec::new(),
                     chained: false,
                 }
             }

crates/bevy_ecs/src/schedule/schedule.rs

@@ -349,6 +349,10 @@ impl SystemSetNode {
     pub fn is_system_type(&self) -> bool {
         self.inner.system_type().is_some()
     }
+
+    pub fn is_anonymous(&self) -> bool {
+        self.inner.is_anonymous()
+    }
 }
 
 /// A [`BoxedSystem`] with metadata, stored in a [`ScheduleGraph`].
@@ -525,7 +529,11 @@ impl ScheduleGraph {
                     }
                 }
             }
-            SystemConfigs::Configs { configs, chained } => {
+            SystemConfigs::Configs {
+                configs,
+                collective_conditions,
+                chained,
+            } => {
                 let mut config_iter = configs.into_iter();
                 let mut nodes_in_scope = Vec::new();
                 let mut densely_chained = true;
@@ -536,9 +544,26 @@ impl ScheduleGraph {
                             densely_chained: true
                         }
                     };
+                    let more_than_one_entry = config_iter.len() > 0;
+                    let anonymous_set = if more_than_one_entry && !collective_conditions.is_empty()
+                    {
+                        Some(AnonymousSet::new())
+                    } else {
+                        None
+                    };
+                    let prev = if let Some(anonymous_set) = anonymous_set {
+                        prev.in_set(anonymous_set)
+                    } else {
+                        prev
+                    };
                     let mut previous_result = self.add_systems_inner(prev, true);
                     densely_chained = previous_result.densely_chained;
                     for current in config_iter {
+                        let current = if let Some(anonymous_set) = anonymous_set {
+                            current.in_set(anonymous_set)
+                        } else {
+                            current
+                        };
                         let current_result = self.add_systems_inner(current, true);
                         densely_chained = densely_chained && current_result.densely_chained;
                         match (
@@ -608,7 +633,18 @@ impl ScheduleGraph {
                     }
                 } else {
                     let more_than_one_entry = config_iter.len() > 1;
+                    let anonymous_set = if more_than_one_entry && !collective_conditions.is_empty()
+                    {
+                        Some(AnonymousSet::new())
+                    } else {
+                        None
+                    };
                     for config in config_iter {
+                        let config = if let Some(anonymous_set) = anonymous_set {
+                            config.in_set(anonymous_set)
+                        } else {
+                            config
+                        };
                         let result = self.add_systems_inner(config, ancestor_chained);
                         densely_chained = densely_chained && result.densely_chained;
                         if ancestor_chained {
@@ -705,8 +741,7 @@ impl ScheduleGraph {
         match self.system_set_ids.get(set) {
             Some(set_id) => {
                 if id == set_id {
-                    let string = format!("{:?}", &set);
-                    return Err(ScheduleBuildError::HierarchyLoop(string));
+                    return Err(ScheduleBuildError::HierarchyLoop(self.get_node_name(id)));
                 }
             }
             None => {
@@ -742,8 +777,7 @@ impl ScheduleGraph {
             match self.system_set_ids.get(set) {
                 Some(set_id) => {
                     if id == set_id {
-                        let string = format!("{:?}", &set);
-                        return Err(ScheduleBuildError::DependencyLoop(string));
+                        return Err(ScheduleBuildError::DependencyLoop(self.get_node_name(id)));
                     }
                 }
                 None => {
@@ -1257,14 +1291,33 @@ impl ScheduleGraph {
                     name
                 }
             }
-            NodeId::Set(_) => self.system_sets[id.index()].name(),
+            NodeId::Set(_) => {
+                let set = &self.system_sets[id.index()];
+                if set.is_anonymous() {
+                    self.anonymous_set_name(id)
+                } else {
+                    set.name()
+                }
+            }
         };
         if self.settings.use_shortnames {
             name = bevy_utils::get_short_name(&name);
         }
         name
     }
 
+    fn anonymous_set_name(&self, id: &NodeId) -> String {
+        format!(
+            "({})",
+            self.hierarchy
+                .graph
+                .edges_directed(*id, Direction::Outgoing)
+                .map(|(_, member_id, _)| self.get_node_name(&member_id))
+                .reduce(|a, b| format!("{a}, {b}"))
+                .unwrap_or_default()
+        )
+    }
+
     fn get_node_kind(&self, id: &NodeId) -> &'static str {
         match id {
             NodeId::System(_) => "system",

crates/bevy_ecs/src/schedule/set.rs

@@ -2,6 +2,7 @@ use std::any::TypeId;
 use std::fmt::Debug;
 use std::hash::{Hash, Hasher};
 use std::marker::PhantomData;
+use std::sync::atomic::{AtomicUsize, Ordering};
 
 pub use bevy_ecs_macros::{ScheduleLabel, SystemSet};
 use bevy_utils::define_boxed_label;
@@ -23,6 +24,10 @@ pub trait SystemSet: DynHash + Debug + Send + Sync + 'static {
         None
     }
 
+    /// Returns `true` if this system set is an [`AnonymousSet`].
+    fn is_anonymous(&self) -> bool {
+        false
+    }
     /// Creates a boxed clone of the label corresponding to this system set.
     fn dyn_clone(&self) -> Box<dyn SystemSet>;
 }
@@ -102,6 +107,29 @@ impl<T> SystemSet for SystemTypeSet<T> {
     }
 }
 
+/// A [`SystemSet`] implicitly created when using
+/// [`Schedule::add_systems`](super::Schedule::add_systems).
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
+pub struct AnonymousSet(usize);
+
+static NEXT_ANONYMOUS_SET_ID: AtomicUsize = AtomicUsize::new(0);
+
+impl AnonymousSet {
+    pub(crate) fn new() -> Self {
+        Self(NEXT_ANONYMOUS_SET_ID.fetch_add(1, Ordering::Relaxed))
+    }
+}
+
+impl SystemSet for AnonymousSet {
+    fn is_anonymous(&self) -> bool {
+        true
+    }
+
+    fn dyn_clone(&self) -> Box<dyn SystemSet> {
+        Box::new(*self)
+    }
+}
+
 /// Types that can be converted into a [`SystemSet`].
 pub trait IntoSystemSet<Marker>: Sized {
     type Set: SystemSet;