From 3d9f97d6fa0033602d02f56b5fc5dfd497a0e3d8 Mon Sep 17 00:00:00 2001 From: Christian Hughes Date: Sat, 29 Nov 2025 13:27:12 -0600 Subject: [PATCH] Accept boxed conditions for `run_if`, and cleanup `IntoScheduleConfigs` --- crates/bevy_ecs/src/schedule/condition.rs | 52 +++- crates/bevy_ecs/src/schedule/config.rs | 302 +++++++++------------- crates/bevy_ecs/src/schedule/schedule.rs | 4 +- 3 files changed, 179 insertions(+), 179 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/condition.rs b/crates/bevy_ecs/src/schedule/condition.rs index c6a5c99b8cc68..ab779b8ef723e 100644 --- a/crates/bevy_ecs/src/schedule/condition.rs +++ b/crates/bevy_ecs/src/schedule/condition.rs @@ -377,6 +377,33 @@ impl SystemCondition for F where { } +/// Conversion trait to turn something into a [`BoxedCondition`]. +/// +/// This trait is automatically implemented for all types that implement +/// [`SystemCondition`], as well as for [`BoxedCondition`] itself. +pub trait IntoBoxedCondition { + /// Converts `self` into a boxed run condition. + fn into_boxed_condition(this: Self) -> BoxedCondition; +} + +#[doc(hidden)] +pub struct UnboxedSystemMarker; + +impl IntoBoxedCondition<(UnboxedSystemMarker, Marker), In> for S +where + S: SystemCondition, +{ + fn into_boxed_condition(this: Self) -> BoxedCondition { + Box::new(IntoSystem::into_system(this)) + } +} + +impl IntoBoxedCondition<(), In> for BoxedCondition { + fn into_boxed_condition(this: Self) -> BoxedCondition { + this + } +} + /// A collection of [run conditions](SystemCondition) that may be useful in any bevy app. pub mod common_conditions { use super::{NotSystem, SystemCondition}; @@ -1249,6 +1276,8 @@ where #[cfg(test)] mod tests { + use alloc::boxed::Box; + use super::{common_conditions::*, SystemCondition}; use crate::error::{BevyError, DefaultErrorHandler, ErrorContext}; use crate::{ @@ -1256,7 +1285,7 @@ mod tests { component::Component, message::Message, query::With, - schedule::{IntoScheduleConfigs, Schedule}, + schedule::{BoxedCondition, IntoScheduleConfigs, Schedule}, system::{IntoSystem, Local, Res, System}, world::World, }; @@ -1434,4 +1463,25 @@ mod tests { .configure_sets(Set.run_if(condition)); schedule.run(&mut world); } + + #[test] + fn boxed_run_if() { + #[derive(Resource, Default)] + struct MyResource; + + let mut world = World::new(); + world.init_resource::(); + let mut schedule = Schedule::default(); + + let condition: BoxedCondition = + Box::new(IntoSystem::into_system(resource_exists::)); + + schedule.add_systems(increment_counter.run_if(condition)); + + schedule.run(&mut world); + assert_eq!(world.resource::().0, 0); + world.init_resource::(); + schedule.run(&mut world); + assert_eq!(world.resource::().0, 1); + } } diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index a8dbfc810b638..fcf3fa149246a 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -7,34 +7,11 @@ use crate::{ condition::{BoxedCondition, SystemCondition}, graph::{Ambiguity, Dependency, DependencyKind, GraphInfo}, set::{InternedSystemSet, IntoSystemSet, SystemSet}, - Chain, + Chain, IntoBoxedCondition, }, - system::{BoxedSystem, IntoSystem, ScheduleSystem, System}, + system::{BoxedSystem, IntoSystem, ScheduleSystem, System, SystemInput}, }; -fn new_condition(condition: impl SystemCondition) -> BoxedCondition { - let condition_system = IntoSystem::into_system(condition); - assert!( - condition_system.is_send(), - "SystemCondition `{}` accesses `NonSend` resources. This is not currently supported.", - condition_system.name() - ); - - Box::new(condition_system) -} - -fn ambiguous_with(graph_info: &mut GraphInfo, set: InternedSystemSet) { - match &mut graph_info.ambiguous_with { - detection @ Ambiguity::Check => { - *detection = Ambiguity::IgnoreWithSet(vec![set]); - } - Ambiguity::IgnoreWithSet(ambiguous_with) => { - ambiguous_with.push(set); - } - Ambiguity::IgnoreAll => (), - } -} - /// Stores data to differentiate different schedulable structs. pub trait Schedulable { /// Additional data used to configure independent scheduling. Stored in [`ScheduleConfig`]. @@ -112,117 +89,134 @@ pub enum ScheduleConfigs { impl> ScheduleConfigs { /// Adds a new boxed system set to the systems. + #[deprecated(since = "0.18.0", note = "Use `add_in_set` instead.")] pub fn in_set_inner(&mut self, set: InternedSystemSet) { + self.add_in_set(set); + } + + /// `&mut Self` version of [`in_set`](IntoScheduleConfigs::in_set). + pub fn add_in_set(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); + assert!( + set.system_type().is_none(), + "adding arbitrary systems to a system type set is not allowed" + ); match self { Self::ScheduleConfig(config) => { config.metadata.hierarchy.push(set); } Self::Configs { configs, .. } => { for config in configs { - config.in_set_inner(set); + config.add_in_set(set); } } } } - fn before_inner(&mut self, set: InternedSystemSet) { + fn add_dependency(&mut self, dependency: &mut impl FnMut() -> Dependency) { match self { Self::ScheduleConfig(config) => { - config - .metadata - .dependencies - .push(Dependency::new(DependencyKind::Before, set)); + config.metadata.dependencies.push(dependency()); } Self::Configs { configs, .. } => { for config in configs { - config.before_inner(set); + config.add_dependency(dependency); } } } } - fn after_inner(&mut self, set: InternedSystemSet) { - match self { - Self::ScheduleConfig(config) => { - config - .metadata - .dependencies - .push(Dependency::new(DependencyKind::After, set)); - } - Self::Configs { configs, .. } => { - for config in configs { - config.after_inner(set); - } - } - } + /// `&mut Self` version of [`before`](IntoScheduleConfigs::before). + pub fn add_before(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); + self.add_dependency(&mut || Dependency::new(DependencyKind::Before, set)); } - fn before_ignore_deferred_inner(&mut self, set: InternedSystemSet) { - match self { - Self::ScheduleConfig(config) => { - config - .metadata - .dependencies - .push(Dependency::new(DependencyKind::Before, set).add_config(IgnoreDeferred)); - } - Self::Configs { configs, .. } => { - for config in configs { - config.before_ignore_deferred_inner(set.intern()); - } - } - } + /// `&mut Self` version of [`after`](IntoScheduleConfigs::after). + pub fn add_after(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); + self.add_dependency(&mut || Dependency::new(DependencyKind::After, set)); + } + + /// `&mut Self` version of [`before_ignore_deferred`](IntoScheduleConfigs::before_ignore_deferred). + pub fn add_before_ignore_deferred(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); + self.add_dependency(&mut || { + Dependency::new(DependencyKind::Before, set).add_config(IgnoreDeferred) + }); + } + + /// `&mut Self` version of [`after_ignore_deferred`](IntoScheduleConfigs::after_ignore_deferred). + pub fn add_after_ignore_deferred(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); + self.add_dependency(&mut || { + Dependency::new(DependencyKind::After, set).add_config(IgnoreDeferred) + }); } - fn after_ignore_deferred_inner(&mut self, set: InternedSystemSet) { + /// `&mut Self` version of [`distributive_run_if`](IntoScheduleConfigs::distributive_run_if). + pub fn add_distributive_run_if(&mut self, condition: impl SystemCondition + Clone) { match self { Self::ScheduleConfig(config) => { - config - .metadata - .dependencies - .push(Dependency::new(DependencyKind::After, set).add_config(IgnoreDeferred)); + let condition = Box::new(IntoSystem::into_system(condition)); + check_send(&*condition); + config.conditions.push(condition); } Self::Configs { configs, .. } => { for config in configs { - config.after_ignore_deferred_inner(set.intern()); + config.add_distributive_run_if(condition.clone()); } } } } - fn distributive_run_if_inner(&mut self, condition: impl SystemCondition + Clone) { + /// `&mut Self` version of [`run_if`](IntoScheduleConfigs::run_if). + pub fn add_run_if(&mut self, condition: impl IntoBoxedCondition) { + let condition = IntoBoxedCondition::into_boxed_condition(condition); + check_send(&*condition); match self { Self::ScheduleConfig(config) => { - config.conditions.push(new_condition(condition)); + config.conditions.push(condition); } - Self::Configs { configs, .. } => { - for config in configs { - config.distributive_run_if_inner(condition.clone()); - } + Self::Configs { + collective_conditions, + .. + } => { + collective_conditions.push(condition); } } } - fn ambiguous_with_inner(&mut self, set: InternedSystemSet) { + /// `&mut Self` version of [`ambiguous_with`](IntoScheduleConfigs::ambiguous_with). + pub fn add_ambiguous_with(&mut self, set: impl IntoSystemSet) { + let set = set.into_system_set().intern(); match self { - Self::ScheduleConfig(config) => { - ambiguous_with(&mut config.metadata, set); - } + Self::ScheduleConfig(config) => match &mut config.metadata.ambiguous_with { + detection @ Ambiguity::Check => { + *detection = Ambiguity::IgnoreWithSet(vec![set]); + } + Ambiguity::IgnoreWithSet(ambiguous_with) => { + ambiguous_with.push(set); + } + Ambiguity::IgnoreAll => (), + }, Self::Configs { configs, .. } => { for config in configs { - config.ambiguous_with_inner(set); + config.add_ambiguous_with(set); } } } } - fn ambiguous_with_all_inner(&mut self) { + /// `&mut Self` version of [`ambiguous_with_all`](IntoScheduleConfigs::ambiguous_with_all). + pub fn set_ambiguous_with_all(&mut self) { match self { Self::ScheduleConfig(config) => { config.metadata.ambiguous_with = Ambiguity::IgnoreAll; } Self::Configs { configs, .. } => { for config in configs { - config.ambiguous_with_all_inner(); + config.set_ambiguous_with_all(); } } } @@ -232,41 +226,43 @@ impl> ScheduleConfig /// /// This is useful if you have a run condition whose concrete type is unknown. /// Prefer `run_if` for run conditions whose type is known at compile time. + #[deprecated( + since = "0.18.0", + note = "`run_if` now accepts boxed conditions directly, and `add_run_if` exists for `&mut config` usage." + )] pub fn run_if_dyn(&mut self, condition: BoxedCondition) { - match self { - Self::ScheduleConfig(config) => { - config.conditions.push(condition); - } - Self::Configs { - collective_conditions, - .. - } => { - collective_conditions.push(condition); - } - } + self.add_run_if(condition); } - fn chain_inner(mut self) -> Self { - match &mut self { + /// `&mut Self` version of [`chain`](IntoScheduleConfigs::chain). + pub fn set_chained(&mut self) { + match self { Self::ScheduleConfig(_) => { /* no op */ } Self::Configs { metadata, .. } => { metadata.set_chained(); } }; - self } - fn chain_ignore_deferred_inner(mut self) -> Self { - match &mut self { + /// `&mut Self` version of [`chain_ignore_deferred`](IntoScheduleConfigs::chain_ignore_deferred). + pub fn set_chained_ignore_deferred(&mut self) { + match self { Self::ScheduleConfig(_) => { /* no op */ } Self::Configs { metadata, .. } => { metadata.set_chained_with_config(IgnoreDeferred); } } - self } } +fn check_send(condition: &dyn System) { + assert!( + condition.is_send(), + "Run condition `{}` accesses `NonSend` resources. This is not currently supported.", + condition.name() + ); +} + /// Types that can convert into a [`ScheduleConfigs`]. /// /// This trait is implemented for "systems" (functions whose arguments all implement @@ -320,7 +316,9 @@ pub trait IntoScheduleConfigs ScheduleConfigs { - self.into_configs().in_set(set) + let mut configs = self.into_configs(); + configs.add_in_set(set); + configs } /// Runs before all systems in `set`. If `self` has any systems that produce [`Commands`](crate::system::Commands) @@ -332,7 +330,9 @@ pub trait IntoScheduleConfigs(self, set: impl IntoSystemSet) -> ScheduleConfigs { - self.into_configs().before(set) + let mut configs = self.into_configs(); + configs.add_before(set); + configs } /// Run after all systems in `set`. If `set` has any systems that produce [`Commands`](crate::system::Commands) @@ -359,7 +359,9 @@ pub trait IntoScheduleConfigs(self, set: impl IntoSystemSet) -> ScheduleConfigs { - self.into_configs().after(set) + let mut configs = self.into_configs(); + configs.add_after(set); + configs } /// Run before all systems in `set`. @@ -367,7 +369,9 @@ pub trait IntoScheduleConfigs(self, set: impl IntoSystemSet) -> ScheduleConfigs { - self.into_configs().before_ignore_deferred(set) + let mut configs = self.into_configs(); + configs.add_before_ignore_deferred(set); + configs } /// Run after all systems in `set`. @@ -375,7 +379,9 @@ pub trait IntoScheduleConfigs(self, set: impl IntoSystemSet) -> ScheduleConfigs { - self.into_configs().after_ignore_deferred(set) + let mut configs = self.into_configs(); + configs.add_after_ignore_deferred(set); + configs } /// Add a run condition to each contained system. @@ -412,7 +418,9 @@ pub trait IntoScheduleConfigs + Clone, ) -> ScheduleConfigs { - self.into_configs().distributive_run_if(condition) + let mut configs = self.into_configs(); + configs.add_distributive_run_if(condition); + configs } /// Run the systems only if the [`SystemCondition`] is `true`. @@ -445,20 +453,26 @@ pub trait IntoScheduleConfigs(self, condition: impl SystemCondition) -> ScheduleConfigs { - self.into_configs().run_if(condition) + fn run_if(self, condition: impl IntoBoxedCondition) -> ScheduleConfigs { + let mut configs = self.into_configs(); + configs.add_run_if(condition); + configs } /// 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) -> ScheduleConfigs { - self.into_configs().ambiguous_with(set) + let mut configs = self.into_configs(); + configs.add_ambiguous_with(set); + configs } /// Suppress warnings and errors that would result from these systems having ambiguities /// (conflicting access but indeterminate order) with any other system. fn ambiguous_with_all(self) -> ScheduleConfigs { - self.into_configs().ambiguous_with_all() + let mut configs = self.into_configs(); + configs.set_ambiguous_with_all(); + configs } /// Treat this collection as a sequence of systems. @@ -469,7 +483,9 @@ pub trait IntoScheduleConfigs ScheduleConfigs { - self.into_configs().chain() + let mut configs = self.into_configs(); + configs.set_chained(); + configs } /// Treat this collection as a sequence of systems. @@ -478,7 +494,9 @@ pub trait IntoScheduleConfigs ScheduleConfigs { - self.into_configs().chain_ignore_deferred() + let mut configs = self.into_configs(); + configs.set_chained_ignore_deferred(); + configs } } @@ -488,74 +506,6 @@ impl> IntoScheduleCo fn into_configs(self) -> Self { self } - - #[track_caller] - fn in_set(mut self, set: impl SystemSet) -> Self { - assert!( - set.system_type().is_none(), - "adding arbitrary systems to a system type set is not allowed" - ); - - self.in_set_inner(set.intern()); - - self - } - - fn before(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.before_inner(set.intern()); - self - } - - fn after(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.after_inner(set.intern()); - self - } - - fn before_ignore_deferred(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.before_ignore_deferred_inner(set.intern()); - self - } - - fn after_ignore_deferred(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.after_ignore_deferred_inner(set.intern()); - self - } - - fn distributive_run_if( - mut self, - condition: impl SystemCondition + Clone, - ) -> ScheduleConfigs { - self.distributive_run_if_inner(condition); - self - } - - fn run_if(mut self, condition: impl SystemCondition) -> ScheduleConfigs { - self.run_if_dyn(new_condition(condition)); - self - } - - fn ambiguous_with(mut self, set: impl IntoSystemSet) -> Self { - let set = set.into_system_set(); - self.ambiguous_with_inner(set.intern()); - self - } - - fn ambiguous_with_all(mut self) -> Self { - self.ambiguous_with_all_inner(); - self - } - - fn chain(self) -> Self { - self.chain_inner() - } - - fn chain_ignore_deferred(self) -> Self { - self.chain_ignore_deferred_inner() - } } impl IntoScheduleConfigs for F diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 3119f322cbfc0..43f45032e0c1e 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -767,12 +767,12 @@ impl ScheduleGraph { if !collective_conditions.is_empty() { if let [config] = configs { for condition in collective_conditions { - config.run_if_dyn(condition); + config.add_run_if(condition); } } else { let set = self.create_anonymous_set(); for config in configs.iter_mut() { - config.in_set_inner(set.intern()); + config.add_in_set(set); } let mut set_config = InternedSystemSet::into_config(set.intern()); set_config.conditions.extend(collective_conditions);