From 57bd070ec893333e36342e4b9ad66d570ef00b35 Mon Sep 17 00:00:00 2001 From: Edgar Geier Date: Sat, 18 Feb 2023 21:33:09 +0000 Subject: [PATCH] Add `distributive_run_if` to `IntoSystemConfigs` (#7724) # Objective - Fixes #7659. ## Solution - This PR extracted the `distributive_run_if` part of #7676, because it does not require the controversial introduction of anonymous system sets. - The distinctive name should make the user aware about the differences between `IntoSystemConfig::run_if` and `IntoSystemConfigs::distributive_run_if`. - The documentation explains in detail the consequences of using the API and possible pit falls when using it. - A test demonstrates the possibility of changing the condition result, resulting in some of the systems not being run. --- ## Changelog ### Added - Add `distributive_run_if` to `IntoSystemConfigs` to enable adding a run condition to each system when using `add_systems`. --- crates/bevy_ecs/src/schedule/config.rs | 42 ++++++++++++++++++++++++++ crates/bevy_ecs/src/schedule/mod.rs | 26 ++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 956626549455a..28fe775acd903 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -520,6 +520,40 @@ where self.into_configs().after(set) } + /// Add a run condition to each contained system. + /// + /// Each system will receive its own clone of the [`Condition`] and will only run + /// if the `Condition` is true. + /// + /// Each individual condition will be evaluated at most once (per schedule run), + /// right before the corresponding system prepares to run. + /// + /// This is equivalent to calling [`run_if`](IntoSystemConfig::run_if) on each individual + /// system, as shown below: + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # let mut app = Schedule::new(); + /// # fn a() {} + /// # fn b() {} + /// # fn condition() -> bool { true } + /// app.add_systems((a, b).distributive_run_if(condition)); + /// app.add_systems((a.run_if(condition), b.run_if(condition))); + /// ``` + /// + /// # Note + /// + /// Because the conditions are evaluated separately for each system, there is no guarantee + /// that all evaluations in a single schedule run will yield the same result. If another + /// system is run inbetween two evaluations it could cause the result of the condition to change. + /// + /// Use [`run_if`](IntoSystemSetConfig::run_if) on a [`SystemSet`] if you want to make sure + /// that either all or none of the systems are run, or you don't want to evaluate the run + /// condition for each contained system separately. + fn distributive_run_if

(self, condition: impl Condition

+ Clone) -> SystemConfigs { + self.into_configs().distributive_run_if(condition) + } + /// Suppress warnings and errors that would result from these systems having ambiguities /// (conflicting access but indeterminate order) with systems in `set`. fn ambiguous_with(self, set: impl IntoSystemSet) -> SystemConfigs { @@ -603,6 +637,14 @@ impl IntoSystemConfigs<()> for SystemConfigs { self } + fn distributive_run_if

(mut self, condition: impl Condition

+ Clone) -> SystemConfigs { + for config in &mut self.systems { + config.conditions.push(new_condition(condition.clone())); + } + + self + } + fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { let set = set.into_system_set(); for config in &mut self.systems { diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 4a2cbf332083b..fa7a2b72844a9 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -222,6 +222,32 @@ mod tests { assert_eq!(world.resource::().0, vec![0]); } + #[test] + fn systems_with_distributive_condition() { + let mut world = World::default(); + let mut schedule = Schedule::default(); + + world.insert_resource(RunConditionBool(true)); + world.init_resource::(); + + fn change_condition(mut condition: ResMut) { + condition.0 = false; + } + + schedule.add_systems( + ( + make_function_system(0), + change_condition, + make_function_system(1), + ) + .chain() + .distributive_run_if(|condition: Res| condition.0), + ); + + schedule.run(&mut world); + assert_eq!(world.resource::().0, vec![0]); + } + #[test] fn run_exclusive_system_with_condition() { let mut world = World::default();