From c45bc476344093367f64b039f94d9d6d163392e2 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Mon, 17 Nov 2025 21:44:00 -0800 Subject: [PATCH 01/10] add FromInput trait --- crates/bevy_ecs/src/schedule/set.rs | 11 +- crates/bevy_ecs/src/system/function_system.rs | 113 ++++++++++-------- crates/bevy_ecs/src/system/input.rs | 24 ++++ 3 files changed, 93 insertions(+), 55 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/set.rs b/crates/bevy_ecs/src/schedule/set.rs index 7c2d718ed6be8..53c6e1565600e 100644 --- a/crates/bevy_ecs/src/schedule/set.rs +++ b/crates/bevy_ecs/src/schedule/set.rs @@ -14,8 +14,8 @@ use crate::{ define_label, intern::Interned, system::{ - ExclusiveFunctionSystem, ExclusiveSystemParamFunction, FunctionSystem, IntoResult, - IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, + ExclusiveFunctionSystem, ExclusiveSystemParamFunction, FromInput, FunctionSystem, + IntoResult, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction, }, }; @@ -291,14 +291,13 @@ impl IntoSystemSet<()> for S { impl IntoSystemSet<(IsFunctionSystem, Marker)> for F where Marker: 'static, - F::Out: IntoResult<()>, - F: SystemParamFunction, + F: SystemParamFunction, Out: IntoResult<()>>, { - type Set = SystemTypeSet>; + type Set = SystemTypeSet>; #[inline] fn into_system_set(self) -> Self::Set { - SystemTypeSet::>::new() + SystemTypeSet::>::new() } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 5f6653be39834..8ca5cea704d92 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -6,8 +6,8 @@ use crate::{ query::FilteredAccessSet, schedule::{InternedSystemSet, SystemSet}, system::{ - check_system_change_tick, ReadOnlySystemParam, System, SystemIn, SystemInput, SystemParam, - SystemParamItem, + check_system_change_tick, FromInput, ReadOnlySystemParam, System, SystemIn, SystemInput, + SystemParam, SystemParamItem, }, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World, WorldId}, }; @@ -234,15 +234,15 @@ macro_rules! impl_build_system { /// You can use [`SystemState::build_system_with_input()`] if you have input, or [`SystemState::build_any_system()`] if you don't need type inference. pub fn build_system< InnerOut: IntoResult, - Out: 'static, + Out, Marker, F: FnMut($(SystemParamItem<$param>),*) -> InnerOut - + SystemParamFunction + + SystemParamFunction > ( self, func: F, - ) -> FunctionSystem + ) -> FunctionSystem { self.build_any_system(func) } @@ -250,17 +250,20 @@ macro_rules! impl_build_system { /// Create a [`FunctionSystem`] from a [`SystemState`]. /// This method signature allows type inference of closure parameters for a system with input. /// You can use [`SystemState::build_system()`] if you have no input, or [`SystemState::build_any_system()`] if you don't need type inference. + #[inline] pub fn build_system_with_input< - Input: SystemInput, + InnerIn: SystemInput + FromInput, + In: SystemInput, InnerOut: IntoResult, - Out: 'static, + Out, Marker, - F: FnMut(Input, $(SystemParamItem<$param>),*) -> InnerOut - + SystemParamFunction, - >( + F: FnMut(InnerIn, $(SystemParamItem<$param>),*) -> InnerOut + + SystemParamFunction + > + ( self, func: F, - ) -> FunctionSystem { + ) -> FunctionSystem { self.build_any_system(func) } } @@ -311,22 +314,20 @@ impl SystemState { /// Create a [`FunctionSystem`] from a [`SystemState`]. /// This method signature allows any system function, but the compiler will not perform type inference on closure parameters. /// You can use [`SystemState::build_system()`] or [`SystemState::build_system_with_input()`] to get type inference on parameters. - pub fn build_any_system(self, func: F) -> FunctionSystem + #[inline] + pub fn build_any_system(self, func: F) -> FunctionSystem where - F: SystemParamFunction>, + In: SystemInput, + F: SystemParamFunction, Out: IntoResult, Param = Param>, { - FunctionSystem { + FunctionSystem::new( func, - #[cfg(feature = "hotpatching")] - current_ptr: subsecond::HotFn::current(>::run) - .ptr_address(), - state: Some(FunctionSystemState { + self.meta, + Some(FunctionSystemState { param: self.param_state, world_id: self.world_id, }), - system_meta: self.meta, - marker: PhantomData, - } + ) } /// Gets the metadata for this instance. @@ -475,7 +476,7 @@ impl FromWorld for SystemState { /// /// The [`Clone`] implementation for [`FunctionSystem`] returns a new instance which /// is NOT initialized. The cloned system must also be `.initialized` before it can be run. -pub struct FunctionSystem +pub struct FunctionSystem where F: SystemParamFunction, { @@ -484,8 +485,8 @@ where current_ptr: subsecond::HotFnPtr, state: Option>, system_meta: SystemMeta, - // NOTE: PhantomData T> gives this safe Send/Sync impls - marker: PhantomData (Marker, Out)>, + // NOTE: PhantomData T> gives this safe Send/Sync impls + marker: PhantomData (Marker, Out)>, } /// The state of a [`FunctionSystem`], which must be initialized with @@ -500,10 +501,23 @@ struct FunctionSystemState { world_id: WorldId, } -impl FunctionSystem +impl FunctionSystem where F: SystemParamFunction, { + #[inline] + fn new(func: F, system_meta: SystemMeta, state: Option>) -> Self { + Self { + func, + #[cfg(feature = "hotpatching")] + current_ptr: subsecond::HotFn::current(>::run) + .ptr_address(), + state, + system_meta, + marker: PhantomData, + } + } + /// Return this system with a new name. /// /// Useful to give closure systems more readable and unique names for debugging and tracing. @@ -514,7 +528,7 @@ where } // De-initializes the cloned system. -impl Clone for FunctionSystem +impl Clone for FunctionSystem where F: SystemParamFunction + Clone, { @@ -535,23 +549,16 @@ where #[doc(hidden)] pub struct IsFunctionSystem; -impl IntoSystem for F +impl IntoSystem for F where - Out: 'static, Marker: 'static, - F: SystemParamFunction>, + In: SystemInput + 'static, + Out: 'static, + F: SystemParamFunction, Out: IntoResult>, { - type System = FunctionSystem; + type System = FunctionSystem; fn into_system(func: Self) -> Self::System { - FunctionSystem { - func, - #[cfg(feature = "hotpatching")] - current_ptr: subsecond::HotFn::current(>::run) - .ptr_address(), - state: None, - system_meta: SystemMeta::new::(), - marker: PhantomData, - } + FunctionSystem::new(func, SystemMeta::new::(), None) } } @@ -596,7 +603,7 @@ impl IntoResult for Never { } } -impl FunctionSystem +impl FunctionSystem where F: SystemParamFunction, { @@ -607,13 +614,14 @@ where "System's state was not found. Did you forget to initialize this system before running it?"; } -impl System for FunctionSystem +impl System for FunctionSystem where Marker: 'static, + In: SystemInput + 'static, Out: 'static, - F: SystemParamFunction>, + F: SystemParamFunction, Out: IntoResult>, { - type In = F::In; + type In = In; type Out = Out; #[inline] @@ -637,6 +645,8 @@ where let change_tick = world.increment_change_tick(); + let input = F::In::from_inner(input); + let state = self.state.as_mut().expect(Self::ERROR_UNINITIALIZED); assert_eq!(state.world_id, world.id(), "Encountered a mismatched World. A System cannot be used with Worlds other than the one it was initialized with."); // SAFETY: @@ -748,12 +758,17 @@ where } /// SAFETY: `F`'s param is [`ReadOnlySystemParam`], so this system will only read from the world. -unsafe impl ReadOnlySystem for FunctionSystem +unsafe impl ReadOnlySystem for FunctionSystem where Marker: 'static, + In: SystemInput + 'static, Out: 'static, - F: SystemParamFunction>, - F::Param: ReadOnlySystemParam, + F: SystemParamFunction< + Marker, + In: FromInput, + Out: IntoResult, + Param: ReadOnlySystemParam, + >, { } @@ -935,7 +950,7 @@ mod tests { use core::any::TypeId; - let system = IntoSystem::into_system(function); + let system = IntoSystem::::into_system(function); assert_eq!( system.type_id(), @@ -951,13 +966,13 @@ mod tests { assert_ne!( system.type_id(), - IntoSystem::into_system(reference_system).type_id(), + IntoSystem::<(), (), _>::into_system(reference_system).type_id(), "Different systems should have different TypeIds" ); } fn function_system() {} - test(function_system); + test::<_, (), (), _>(function_system); } } diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index 429f4df018ed6..d9ab4a7e712e7 100644 --- a/crates/bevy_ecs/src/system/input.rs +++ b/crates/bevy_ecs/src/system/input.rs @@ -55,6 +55,30 @@ pub trait SystemInput: Sized { /// Shorthand way to get the [`System::In`] for a [`System`] as a [`SystemInput::Inner`]. pub type SystemIn<'a, S> = <::In as SystemInput>::Inner<'a>; +/// A type that may be constructed from the input of a [`System`]. +/// This is used to allow systems whose first parameter is a `StaticSystemInput` +/// to take an `In` as input, and can be implemented for user types to allow +/// similar conversions. +pub trait FromInput: SystemInput { + /// Converts the system input's inner representation into this type's + /// inner representation. + fn from_inner<'i>(inner: In::Inner<'i>) -> Self::Inner<'i>; +} + +impl FromInput for In { + #[inline] + fn from_inner<'i>(inner: In::Inner<'i>) -> Self::Inner<'i> { + inner + } +} + +impl<'a, In: SystemInput> FromInput for StaticSystemInput<'a, In> { + #[inline] + fn from_inner<'i>(inner: In::Inner<'i>) -> Self::Inner<'i> { + inner + } +} + /// A [`SystemInput`] type which denotes that a [`System`] receives /// an input value of type `T` from its caller. /// From f104b1ff1291a545fad41261958cb4c01545c07f Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sun, 23 Nov 2025 11:19:19 -0800 Subject: [PATCH 02/10] tests --- crates/bevy_ecs/src/system/function_system.rs | 8 ++++---- crates/bevy_ecs/src/system/input.rs | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 8ca5cea704d92..875d2683d896a 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -485,7 +485,7 @@ where current_ptr: subsecond::HotFnPtr, state: Option>, system_meta: SystemMeta, - // NOTE: PhantomData T> gives this safe Send/Sync impls + // NOTE: PhantomData T> gives this safe Send/Sync impls marker: PhantomData (Marker, Out)>, } @@ -950,7 +950,7 @@ mod tests { use core::any::TypeId; - let system = IntoSystem::::into_system(function); + let system = IntoSystem::into_system(function); assert_eq!( system.type_id(), @@ -966,13 +966,13 @@ mod tests { assert_ne!( system.type_id(), - IntoSystem::<(), (), _>::into_system(reference_system).type_id(), + IntoSystem::into_system(reference_system).type_id(), "Different systems should have different TypeIds" ); } fn function_system() {} - test::<_, (), (), _>(function_system); + test(function_system); } } diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index d9ab4a7e712e7..b9143f2079bea 100644 --- a/crates/bevy_ecs/src/system/input.rs +++ b/crates/bevy_ecs/src/system/input.rs @@ -318,7 +318,7 @@ all_tuples!( #[cfg(test)] mod tests { use crate::{ - system::{In, InMut, InRef, IntoSystem, System}, + system::{assert_is_system, In, InMut, InRef, IntoSystem, StaticSystemInput, System}, world::World, }; @@ -351,4 +351,19 @@ mod tests { by_mut.run((&mut a, b), &mut world).unwrap(); assert_eq!(a, 36); } + + #[test] + fn compatible_input() { + fn takes_usize(In(a): In) -> usize { + a + } + + fn takes_static_usize(StaticSystemInput(In(b)): StaticSystemInput>) -> usize { + b + } + + assert_is_system::, usize, _>(takes_usize); + // test if StaticSystemInput is compatible with its inner type + assert_is_system::, usize, _>(takes_static_usize); + } } From 5b40252379e4088a65ae9f8c4d9dcf3ffc8c519a Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sat, 22 Nov 2025 11:55:58 -0800 Subject: [PATCH 03/10] docs and migration guide --- .../function_system_generics.md | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 release-content/migration-guides/function_system_generics.md diff --git a/release-content/migration-guides/function_system_generics.md b/release-content/migration-guides/function_system_generics.md new file mode 100644 index 0000000000000..d9ade91543274 --- /dev/null +++ b/release-content/migration-guides/function_system_generics.md @@ -0,0 +1,21 @@ +--- +title: "FunctionSystem Generics" +authors: ["@ecoskey"] +pull_requests: [todo!()] +--- + +`FunctionSystem` now has a new generic parameter: `In`. + +Old: `FunctionSystem` +New: `FunctionSystem` + +Additionally, there's an extra bound on the `System` and `IntoSystem` impls +related to `FunctionSystem`: + +`::In: FromInput` + +This enabled systems to take as input any *compatible* type, in addition to the +exact one specified by the system function. This shouldn't impact users at all +since it only adds functionality, but users writing heavily generic code may +want to add a similar bound. See `function_system.rs` to see how it works in +practice. From e606143aac5e22c31a6da3f02500e4e1381886a9 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sun, 23 Nov 2025 12:01:15 -0800 Subject: [PATCH 04/10] fix pr number --- release-content/migration-guides/function_system_generics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/release-content/migration-guides/function_system_generics.md b/release-content/migration-guides/function_system_generics.md index d9ade91543274..a302dd72cf329 100644 --- a/release-content/migration-guides/function_system_generics.md +++ b/release-content/migration-guides/function_system_generics.md @@ -1,7 +1,7 @@ --- title: "FunctionSystem Generics" authors: ["@ecoskey"] -pull_requests: [todo!()] +pull_requests: [21917] --- `FunctionSystem` now has a new generic parameter: `In`. From 826d1c0f6fa92d01b25c7355211b9b307a076060 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Sun, 23 Nov 2025 12:15:51 -0800 Subject: [PATCH 05/10] fix tests --- crates/bevy_ecs/src/system/input.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index b9143f2079bea..c37a08cd65be1 100644 --- a/crates/bevy_ecs/src/system/input.rs +++ b/crates/bevy_ecs/src/system/input.rs @@ -358,7 +358,7 @@ mod tests { a } - fn takes_static_usize(StaticSystemInput(In(b)): StaticSystemInput>) -> usize { + fn takes_static_usize(StaticSystemInput(b): StaticSystemInput>) -> usize { b } From dfb39d0b0d12c9f17ad8c08ca2350d8076b337ff Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 27 Nov 2025 21:28:49 -0800 Subject: [PATCH 06/10] fix tests --- crates/bevy_ecs/src/system/function_system.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 875d2683d896a..fda251373c355 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -814,8 +814,10 @@ where /// let mut world = World::default(); /// world.insert_resource(Message("42".to_string())); /// -/// // pipe the `parse_message_system`'s output into the `filter_system`s input -/// let mut piped_system = IntoSystem::into_system(pipe(parse_message, filter)); +/// // pipe the `parse_message_system`'s output into the `filter_system`s input. +/// // Type annotations should only needed when using `StaticSystemInput` as input +/// // AND the input type isn't constrained by nearby code. +/// let mut piped_system = IntoSystem::<(), Option, _>::into_system(pipe(parse_message, filter)); /// piped_system.initialize(&mut world); /// assert_eq!(piped_system.run((), &mut world).unwrap(), Some(42)); /// } From 83844acf50c3c3695c7a6ca27d39feba830c877c Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 6 Nov 2025 21:45:07 -0800 Subject: [PATCH 07/10] Simplify system building with BuilderSystem --- crates/bevy_ecs/src/system/builder.rs | 277 +++++++++++++++++- crates/bevy_ecs/src/system/function_system.rs | 17 ++ 2 files changed, 287 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 937911ca834c1..1744f2da881b7 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -1,23 +1,27 @@ use alloc::{boxed::Box, vec::Vec}; use bevy_platform::cell::SyncCell; +use bevy_utils::prelude::DebugName; use variadics_please::all_tuples; use crate::{ + change_detection::{CheckChangeTicks, Tick}, prelude::QueryBuilder, - query::{QueryData, QueryFilter, QueryState}, + query::{FilteredAccessSet, QueryData, QueryFilter, QueryState}, resource::Resource, system::{ - DynSystemParam, DynSystemParamState, If, Local, ParamSet, Query, SystemParam, - SystemParamValidationError, + DynSystemParam, DynSystemParamState, FromInput, FunctionSystem, If, IntoResult, IntoSystem, + Local, ParamSet, Query, ReadOnlySystem, System, SystemInput, SystemMeta, SystemParam, + SystemParamFunction, SystemParamValidationError, }, world::{ - FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut, - FilteredResourcesMutBuilder, FromWorld, World, + unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, + FilteredResourcesBuilder, FilteredResourcesMut, FilteredResourcesMutBuilder, FromWorld, + World, }, }; -use core::fmt::Debug; +use core::{fmt::Debug, marker::PhantomData, mem}; -use super::{Res, ResMut, SystemState}; +use super::{Res, ResMut, RunSystemError, SystemState, SystemStateFlags}; /// A builder that can create a [`SystemParam`]. /// @@ -119,6 +123,17 @@ pub unsafe trait SystemParamBuilder: Sized { fn build_state(self, world: &mut World) -> SystemState

{ SystemState::from_builder(world, self) } + + /// Create a [`System`] from a [`SystemParamBuilder`] + fn build_system( + self, + func: Func, + ) -> IntoBuilderSystem + where + Func: SystemParamFunction, + { + IntoBuilderSystem::new(self, func) + } } /// A [`SystemParamBuilder`] for any [`SystemParam`] that uses its default initialization. @@ -204,6 +219,254 @@ impl ParamBuilder { } } +/// A marker type used to distinguish builder systems from plain function systems. +#[doc(hidden)] +pub struct IsBuilderSystem; + +/// An [`IntoSystem`] creating an instance of [`BuilderSystem`] +pub struct IntoBuilderSystem +where + Func: SystemParamFunction, + Builder: SystemParamBuilder, +{ + builder: Builder, + func: Func, + _data: PhantomData (Marker, Out)>, +} + +impl IntoBuilderSystem +where + Func: SystemParamFunction, + Builder: SystemParamBuilder, +{ + /// Returns a new [`IntoBuilderSystem`] given a system param builder and system function + pub fn new(builder: Builder, func: Func) -> Self { + Self { + builder, + func, + _data: PhantomData, + } + } +} + +impl IntoSystem + for IntoBuilderSystem +where + Marker: 'static, + In: SystemInput + 'static, + Out: 'static, + Func: SystemParamFunction, Out: IntoResult>, + Builder: SystemParamBuilder + Send + Sync + 'static, +{ + type System = BuilderSystem; + + fn into_system(this: Self) -> Self::System { + BuilderSystem::new(this.builder, this.func) + } +} + +/// A [`System`] created from a [`SystemParamBuilder`] whose state is not +/// initialized until the first run. +pub struct BuilderSystem +where + Func: SystemParamFunction, + Builder: SystemParamBuilder, +{ + inner: BuilderSystemInner, +} + +impl BuilderSystem +where + Func: SystemParamFunction, + Builder: SystemParamBuilder, +{ + /// Returns a new `BuilderSystem` given a system param builder and a system function + pub fn new(builder: Builder, func: Func) -> Self { + Self { + inner: BuilderSystemInner::Uninitialized { + builder, + func, + meta: SystemMeta::new::(), + }, + } + } +} + +enum BuilderSystemInner +where + Func: SystemParamFunction, + Builder: SystemParamBuilder, +{ + /// A properly initialized system whose state has been constructed + Initialized { + system: FunctionSystem, + }, + /// An unititialized system, whose state hasn't been constructed from + /// the param builder yet + Uninitialized { + builder: Builder, + func: Func, + meta: SystemMeta, + }, + // This only exists as a variant to use with `mem::replace` in `initialize`. + // If this state is ever observed outside `initialize`, then a `panic!` + // interrupted initialization, leaving this system in an invalid state. + Invalid, +} + +impl System for BuilderSystem +where + Marker: 'static, + In: SystemInput + 'static, + Out: 'static, + Func: SystemParamFunction, Out: IntoResult>, + Builder: SystemParamBuilder + Send + Sync + 'static, +{ + type In = In; + + type Out = Out; + + #[inline] + fn name(&self) -> DebugName { + match &self.inner { + BuilderSystemInner::Initialized { system } => system.name(), + BuilderSystemInner::Uninitialized { meta, .. } => meta.name().clone(), + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn flags(&self) -> SystemStateFlags { + match &self.inner { + BuilderSystemInner::Initialized { system, .. } => system.flags(), + BuilderSystemInner::Uninitialized { meta, .. } => meta.flags(), + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + unsafe fn run_unsafe( + &mut self, + input: super::SystemIn<'_, Self>, + world: UnsafeWorldCell, + ) -> Result { + match &mut self.inner { + // SAFETY: requirements upheld by the caller. + BuilderSystemInner::Initialized { system, .. } => unsafe { + system.run_unsafe(input, world) + }, + BuilderSystemInner::Uninitialized { .. } => panic!( + "BuilderSystem {} was not initialized before calling run_unsafe.", + self.name() + ), + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[cfg(feature = "hotpatching")] + #[inline] + fn refresh_hotpatch(&mut self) { + match &mut self.inner { + BuilderSystemInner::Initialized { system, .. } => system.refresh_hotpatch(), + BuilderSystemInner::Uninitialized { .. } => {} + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn apply_deferred(&mut self, world: &mut World) { + match &mut self.inner { + BuilderSystemInner::Initialized { system, .. } => system.apply_deferred(world), + BuilderSystemInner::Uninitialized { .. } => {} + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn queue_deferred(&mut self, world: DeferredWorld) { + match &mut self.inner { + BuilderSystemInner::Initialized { system, .. } => system.queue_deferred(world), + BuilderSystemInner::Uninitialized { .. } => {} + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + unsafe fn validate_param_unsafe( + &mut self, + world: UnsafeWorldCell, + ) -> Result<(), SystemParamValidationError> { + match &mut self.inner { + // SAFETY: requirements upheld by the caller. + BuilderSystemInner::Initialized { system, .. } => unsafe { + system.validate_param_unsafe(world) + }, + BuilderSystemInner::Uninitialized { .. } => Ok(()), + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn initialize(&mut self, world: &mut World) -> FilteredAccessSet { + let inner = mem::replace(&mut self.inner, BuilderSystemInner::Invalid); + match inner { + BuilderSystemInner::Initialized { mut system } => { + let access = system.initialize(world); + self.inner = BuilderSystemInner::Initialized { system }; + access + } + BuilderSystemInner::Uninitialized { builder, func, .. } => { + let mut system = builder.build_state(world).build_any_system(func); + let access = system.initialize(world); + self.inner = BuilderSystemInner::Initialized { system }; + access + } + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn check_change_tick(&mut self, check: CheckChangeTicks) { + match &mut self.inner { + BuilderSystemInner::Initialized { system, .. } => system.check_change_tick(check), + BuilderSystemInner::Uninitialized { .. } => {} + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn get_last_run(&self) -> Tick { + match &self.inner { + BuilderSystemInner::Initialized { system, .. } => system.get_last_run(), + BuilderSystemInner::Uninitialized { meta, .. } => meta.get_last_run(), + BuilderSystemInner::Invalid => unreachable!(), + } + } + + #[inline] + fn set_last_run(&mut self, last_run: Tick) { + match &mut self.inner { + BuilderSystemInner::Initialized { system, .. } => system.set_last_run(last_run), + BuilderSystemInner::Uninitialized { meta, .. } => meta.set_last_run(last_run), + BuilderSystemInner::Invalid => unreachable!(), + } + } +} + +// SAFETY: if the wrapped system is read-only, so is this one +unsafe impl ReadOnlySystem + for BuilderSystem +where + Marker: 'static, + In: SystemInput + 'static, + Out: 'static, + Func: SystemParamFunction, Out: IntoResult>, + Builder: SystemParamBuilder + Send + Sync + 'static, + // the important bound + FunctionSystem: ReadOnlySystem, +{ +} + // SAFETY: Any `QueryState` for the correct world is valid for `Query::State`, // and we check the world during `build`. unsafe impl<'w, 's, D: QueryData + 'static, F: QueryFilter + 'static> diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index fda251373c355..af143b2071416 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -64,6 +64,11 @@ impl SystemMeta { &self.name } + /// Returns the system's state flags + pub fn flags(&self) -> SystemStateFlags { + self.flags + } + /// Sets the name of this system. /// /// Useful to give closure systems more readable and unique names for debugging and tracing. @@ -79,6 +84,18 @@ impl SystemMeta { self.name = new_name.into(); } + /// Gets the last time this system was run. + #[inline] + pub fn get_last_run(&self) -> Tick { + self.last_run + } + + /// Sets the last time this system was run. + #[inline] + pub fn set_last_run(&mut self, last_run: Tick) { + self.last_run = last_run; + } + /// Returns true if the system is [`Send`]. #[inline] pub fn is_send(&self) -> bool { From 312519a33563f15ac96b0b684c4ffc63dcd5ef8e Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Wed, 5 Nov 2025 21:13:52 -0800 Subject: [PATCH 08/10] Add SystemRunner --- crates/bevy_ecs/src/system/builder.rs | 40 ++++++- crates/bevy_ecs/src/system/system_param.rs | 115 ++++++++++++++++++++- 2 files changed, 150 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 1744f2da881b7..3999d9ed35285 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -9,9 +9,10 @@ use crate::{ query::{FilteredAccessSet, QueryData, QueryFilter, QueryState}, resource::Resource, system::{ - DynSystemParam, DynSystemParamState, FromInput, FunctionSystem, If, IntoResult, IntoSystem, - Local, ParamSet, Query, ReadOnlySystem, System, SystemInput, SystemMeta, SystemParam, - SystemParamFunction, SystemParamValidationError, + BoxedSystem, DynSystemParam, DynSystemParamState, FromInput, FunctionSystem, If, + IntoResult, IntoSystem, Local, ParamSet, Query, ReadOnlySystem, System, SystemInput, + SystemMeta, SystemParam, SystemParamFunction, SystemParamValidationError, SystemRunner, + SystemRunnerState, }, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, @@ -217,6 +218,13 @@ impl ParamBuilder { ) -> impl SystemParamBuilder> { Self } + + /// Helper method for adding a [`SystemRunner`] as a param, equivalent to [`SystemRunnerBuilder::from_system`] + pub fn system<'w, 's, In: SystemInput + 'static, Out: 'static, Marker>( + system: impl IntoSystem, + ) -> impl SystemParamBuilder> { + SystemRunnerBuilder::from_system(system) + } } /// A marker type used to distinguish builder systems from plain function systems. @@ -879,6 +887,32 @@ unsafe impl> SystemParamBuilder> } } +/// A [`SystemParamBuilder`] for a [`SystemRunner`] +pub struct SystemRunnerBuilder( + BoxedSystem, +); + +impl SystemRunnerBuilder { + /// Returns a `SystemRunnerBuilder` created from a given system. + pub fn from_system, M>(system: S) -> Self { + Self(Box::new(S::into_system(system))) + } +} + +// SAFETY: the state returned is always valid. In particular the access always +// matches the contained system. +unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> + SystemParamBuilder> for SystemRunnerBuilder +{ + fn build(mut self, world: &mut World) -> SystemRunnerState { + let access = self.0.initialize(world); + SystemRunnerState { + system: Some(self.0), + access, + } + } +} + #[cfg(test)] mod tests { use crate::{ diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 80d449017d2eb..e829dd1261283 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -3,7 +3,7 @@ use crate::{ archetype::Archetypes, bundle::Bundles, change_detection::{ComponentTicksMut, ComponentTicksRef, Tick}, - component::{ComponentId, Components}, + component::{self, ComponentId, Components}, entity::{Entities, EntityAllocator}, query::{ Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QuerySingleError, @@ -11,7 +11,7 @@ use crate::{ }, resource::Resource, storage::ResourceData, - system::{Query, Single, SystemMeta}, + system::{BoxedSystem, Query, RunSystemError, Single, System, SystemInput, SystemMeta}, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut, FromWorld, World, @@ -26,6 +26,7 @@ use core::{ any::Any, fmt::{Debug, Display}, marker::PhantomData, + mem, ops::{Deref, DerefMut}, }; use thiserror::Error; @@ -2765,6 +2766,116 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { } } +pub struct SystemRunner<'w, 's, In: SystemInput = (), Out = ()> { + world: UnsafeWorldCell<'w>, + system: &'s mut dyn System, +} + +impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemRunner<'w, 's, In, Out> { + #[inline] + pub fn run_with(&mut self, input: In::Inner<'_>) -> Result { + // SAFETY: + // - all accesses are properly declared in `init_access` + unsafe { self.system.run_unsafe(input, self.world) } + } +} + +impl<'w, 's, Out: 'static> SystemRunner<'w, 's, (), Out> { + #[inline] + pub fn run(&mut self) -> Result { + self.run_with(()) + } +} + +/// The [`SystemParam::State`] for a [`SystemRunner`]. +pub struct SystemRunnerState { + pub(crate) system: Option>, + pub(crate) access: FilteredAccessSet, +} + +// SAFETY: SystemRunner registers all accesses and panics if a conflict is detected. +unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemParam + for SystemRunner<'w, 's, In, Out> +{ + type State = SystemRunnerState; + + type Item<'world, 'state> = SystemRunner<'world, 'state, In, Out>; + + #[inline] + fn init_state(_world: &mut World) -> Self::State { + SystemRunnerState { + system: None, + access: FilteredAccessSet::default(), + } + } + + fn init_access( + state: &Self::State, + system_meta: &mut SystemMeta, + component_access_set: &mut FilteredAccessSet, + world: &mut World, + ) { + //TODO: does this handle exclusive systems properly? + let conflicts = component_access_set.get_conflicts(&state.access); + if !conflicts.is_empty() { + let system_name = system_meta.name(); + let mut accesses = conflicts.format_conflict_list(world); + // Access list may be empty (if access to all components requested) + if !accesses.is_empty() { + accesses.push(' '); + } + panic!("error[B0001]: SystemRunner({}) in system {system_name} accesses component(s) {accesses}in a way that conflicts with a previous system parameter. Consider using `Without` to create disjoint system accesses or using a `ParamSet`. See: https://bevy.org/learn/errors/b0001", state.system.as_ref().map(|sys| sys.name()).unwrap_or(DebugName::borrowed(""))); + } + + component_access_set.extend(state.access.clone()); + } + + #[inline] + unsafe fn get_param<'world, 'state>( + state: &'state mut Self::State, + _system_meta: &SystemMeta, + world: UnsafeWorldCell<'world>, + _change_tick: Tick, + ) -> Self::Item<'world, 'state> { + SystemRunner { + world, + system: state.system.as_deref_mut().expect("SystemRunner accessed with invalid state. This should have been caught by `validate_param`"), + } + } + + #[inline] + fn apply(state: &mut Self::State, _system_meta: &SystemMeta, world: &mut World) { + if let Some(sys) = &mut state.system { + sys.apply_deferred(world); + } + } + + #[inline] + fn queue(state: &mut Self::State, _system_meta: &SystemMeta, world: DeferredWorld) { + if let Some(sys) = &mut state.system { + sys.queue_deferred(world); + } + } + + unsafe fn validate_param( + state: &mut Self::State, + _system_meta: &SystemMeta, + world: UnsafeWorldCell, + ) -> Result<(), SystemParamValidationError> { + state.system.as_mut().ok_or(SystemParamValidationError { + skipped: false, + message: "SystemRunner must be manually initialized, for example with the system builder API.".into(), + param: DebugName::type_name::(), + field: "".into(), + }).and_then(|sys| { + // SAFETY: caller asserts that `world` has all accesses declared in `init_access`, + // which match the underlying system when `state.system` is `Ok`, and otherwise + // this branch will not be entered. + unsafe { sys.validate_param_unsafe(world) } + }) + } +} + /// An error that occurs when a system parameter is not valid, /// used by system executors to determine what to do with a system. /// From 180bb224f36ff18d188b80207980bebaa368950a Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Fri, 7 Nov 2025 12:35:27 -0800 Subject: [PATCH 09/10] system composition macro --- crates/bevy_ecs/macros/src/lib.rs | 11 ++ .../bevy_ecs/macros/src/system_composition.rs | 121 ++++++++++++++++++ crates/bevy_ecs/src/system/mod.rs | 2 + 3 files changed, 134 insertions(+) create mode 100644 crates/bevy_ecs/macros/src/system_composition.rs diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 6936ca4aff3a6..f71d4bc10cd1f 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -9,6 +9,7 @@ mod event; mod message; mod query_data; mod query_filter; +mod system_composition; mod world_query; use crate::{ @@ -717,3 +718,13 @@ pub fn derive_from_world(input: TokenStream) -> TokenStream { } }) } + +#[proc_macro] +pub fn compose(input: TokenStream) -> TokenStream { + system_composition::compose(input, false) +} + +#[proc_macro] +pub fn compose_with(input: TokenStream) -> TokenStream { + system_composition::compose(input, true) +} diff --git a/crates/bevy_ecs/macros/src/system_composition.rs b/crates/bevy_ecs/macros/src/system_composition.rs new file mode 100644 index 0000000000000..cbfe83c57c0c3 --- /dev/null +++ b/crates/bevy_ecs/macros/src/system_composition.rs @@ -0,0 +1,121 @@ +use core::mem; + +use bevy_macro_utils::ensure_no_collision; +use proc_macro::TokenStream; +use quote::{format_ident, quote}; +use syn::{ + parse::{Parse, ParseStream}, + parse_macro_input, parse_quote, parse_quote_spanned, + spanned::Spanned, + visit_mut::VisitMut, + Expr, ExprMacro, Ident, Pat, Path, Token, +}; + +use crate::bevy_ecs_path; + +struct ExpandSystemCalls { + call_ident: Ident, + systems_ident: Ident, + system_paths: Vec, +} + +struct SystemCall { + path: Path, + _comma: Option, + input: Option>, +} + +impl Parse for SystemCall { + fn parse(input: ParseStream) -> syn::Result { + let path = input.parse()?; + let comma: Option = input.parse()?; + let input = comma.as_ref().and_then(|_| input.parse().ok()); + Ok(Self { + path, + _comma: comma, + input, + }) + } +} + +impl VisitMut for ExpandSystemCalls { + fn visit_expr_mut(&mut self, i: &mut Expr) { + if let Expr::Macro(ExprMacro { attrs: _, mac }) = i + && mac.path.is_ident(&self.call_ident) + { + let call = match mac.parse_body::() { + Ok(call) => call, + Err(err) => { + *i = Expr::Verbatim(err.into_compile_error()); + return; + } + }; + + let call_index = match self.system_paths.iter().position(|p| p == &call.path) { + Some(i) => i, + None => { + let len = self.system_paths.len(); + self.system_paths.push(call.path.clone()); + len + } + }; + + let systems_ident = &self.systems_ident; + let system_accessor = format_ident!("p{}", call_index); + let expr: Expr = match &call.input { + Some(input) => { + parse_quote_spanned!(mac.span()=> #systems_ident.#system_accessor().run_with(#input)) + } + None => { + parse_quote_spanned!(mac.span()=> #systems_ident.#system_accessor().run()) + } + }; + *i = expr; + } else { + syn::visit_mut::visit_expr_mut(self, i); + } + } +} + +pub fn compose(input: TokenStream, has_input: bool) -> TokenStream { + let bevy_ecs_path = bevy_ecs_path(); + let call_ident = format_ident!("run"); + let systems_ident = ensure_no_collision(format_ident!("__systems"), input.clone()); + let mut expr_closure = parse_macro_input!(input as syn::ExprClosure); + + let mut visitor = ExpandSystemCalls { + call_ident, + systems_ident: systems_ident.clone(), + system_paths: Vec::new(), + }; + + syn::visit_mut::visit_expr_closure_mut(&mut visitor, &mut expr_closure); + + let runner_types: Vec = visitor + .system_paths + .iter() + .map(|path| parse_quote_spanned!(path.span()=> #bevy_ecs_path::system::SystemRunner<_, _>)) + .collect(); + + let mut builders: Vec = vec![ + parse_quote!(#bevy_ecs_path::system::builder::ParamBuilder); + if has_input { + expr_closure.inputs.len() - 1 + } else { + expr_closure.inputs.len() + } + ]; + let system_builders: Vec = visitor.system_paths.iter().map(|path| parse_quote_spanned!(path.span()=> #bevy_ecs_path::system::ParamBuilder::system(#path))).collect(); + builders.push(parse_quote!(#bevy_ecs_path::system::ParamSetBuilder((#(#system_builders,)*)))); + + expr_closure.inputs.push(Pat::Type( + parse_quote!(mut #systems_ident: #bevy_ecs_path::system::ParamSet<(#(#runner_types,)*)>), + )); + + TokenStream::from(quote! { + #bevy_ecs_path::system::SystemParamBuilder::build_system( + (#(#builders,)*), + #expr_closure + ) + }) +} diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index eeecf4b7ac5db..6e97a69104b31 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -153,6 +153,8 @@ pub use system_name::*; pub use system_param::*; pub use system_registry::*; +pub use bevy_ecs_macros::{compose, compose_with}; + use crate::world::{FromWorld, World}; /// Conversion trait to turn something into a [`System`]. From 8ac61d3312d738a5e5dc5f7b8b773ca1eb70f661 Mon Sep 17 00:00:00 2001 From: Emerson Coskey Date: Thu, 6 Nov 2025 21:52:14 -0800 Subject: [PATCH 10/10] docs + various fixes --- crates/bevy_ecs/macros/Cargo.toml | 2 +- crates/bevy_ecs/macros/src/lib.rs | 72 +++++++++++ .../bevy_ecs/macros/src/system_composition.rs | 27 +++-- crates/bevy_ecs/src/system/builder.rs | 61 +++++++--- crates/bevy_ecs/src/system/system_param.rs | 112 ++++++++++++++---- examples/ecs/system_piping.rs | 31 +++-- .../migration-guides/system_input_unwrap.md | 9 ++ .../release-notes/system_composition.md | 49 ++++++++ 8 files changed, 303 insertions(+), 60 deletions(-) create mode 100644 release-content/migration-guides/system_input_unwrap.md create mode 100644 release-content/release-notes/system_composition.md diff --git a/crates/bevy_ecs/macros/Cargo.toml b/crates/bevy_ecs/macros/Cargo.toml index c70c258e6d74f..1103128a8b88a 100644 --- a/crates/bevy_ecs/macros/Cargo.toml +++ b/crates/bevy_ecs/macros/Cargo.toml @@ -11,7 +11,7 @@ proc-macro = true [dependencies] bevy_macro_utils = { path = "../../bevy_macro_utils", version = "0.18.0-dev" } -syn = { version = "2.0.108", features = ["full", "extra-traits"] } +syn = { version = "2.0.108", features = ["full", "extra-traits", "visit-mut"] } quote = "1.0" proc-macro2 = "1.0" [lints] diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index f71d4bc10cd1f..de6baac8550f8 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -719,11 +719,83 @@ pub fn derive_from_world(input: TokenStream) -> TokenStream { }) } +/// Automatically compose systems together with function syntax. +/// +/// This macro provides some nice syntax on top of the `SystemRunner` `SystemParam` +/// to allow running systems inside other systems. Overall, the macro accepts normal +/// closure syntax: +/// +/// ```ignore +/// let system_a = |world: &mut World| { 10 }; +/// let system_b = |a: In, world: &mut World| { println!("{}", *a + 12) }; +/// compose! { +/// || -> Result<(), RunSystemError> { +/// let a = run!(system-a)?; +/// run!(system_b); +/// } +/// } +/// ``` +/// +/// What's special is that the macro will expand any invocations of `run!()` into +/// calls to `SystemRunner::run` or `SystemRunner::run_with`. The `run!()` accepts +/// two parameters: first, a system identifier (or a path to one), and second, an +/// optional input to invoke the system with. +/// +/// Notes: +/// 1. All system runners are passed through a `ParamSet`, so invoked systems will +/// not conflict with each other. However, invoked systems may still conflict +/// with system params in the outer closure. +/// +/// 2. `run!` will not accept expressions that evaluate to systems, only direct +/// identifiers or paths. So, if you want to call something like: +/// +/// ```ignore +/// run!(|query: Query<(&A, &B, &mut C)>| { ... })` +/// ``` +/// +/// Assign the expression to a variable first. #[proc_macro] pub fn compose(input: TokenStream) -> TokenStream { system_composition::compose(input, false) } +/// Automatically compose systems together with function syntax. +/// +/// Unlike [`compose`], this macro allows generating systems that take input. +/// +/// This macro provides some nice syntax on top of the `SystemRunner` `SystemParam` +/// to allow running systems inside other systems. Overall, the macro accepts normal +/// closure syntax: +/// +/// ```ignore +/// let system_a = |input: In, world: &mut World| { *input + 10 }; +/// let system_b = |a: In, world: &mut World| { println!("{}", *a + 12) }; +/// compose_with! { +/// |input: In| -> Result<(), RunSystemError> { +/// let a = run!(system_a, input)?; +/// run!(system_b); +/// } +/// } +/// ``` +/// +/// What's special is that the macro will expand any invocations of `run!()` into +/// calls to `SystemRunner::run` or `SystemRunner::run_with`. The `run!()` accepts +/// two parameters: first, a system identifier (or a path to one), and second, an +/// optional input to invoke the system with. +/// +/// Notes: +/// 1. All system runners are passed through a `ParamSet`, so invoked systems will +/// not conflict with each other. However, invoked systems may still conflict +/// with system params in the outer closure. +/// +/// 2. `run!` will not accept expressions that evaluate to systems, only direct +/// identifiers or paths. So, if you want to call something like: +/// +/// ```ignore +/// run!(|query: Query<(&A, &B, &mut C)>| { ... })` +/// ``` +/// +/// Assign the expression to a variable first. #[proc_macro] pub fn compose_with(input: TokenStream) -> TokenStream { system_composition::compose(input, true) diff --git a/crates/bevy_ecs/macros/src/system_composition.rs b/crates/bevy_ecs/macros/src/system_composition.rs index cbfe83c57c0c3..3bf1f82dbc79b 100644 --- a/crates/bevy_ecs/macros/src/system_composition.rs +++ b/crates/bevy_ecs/macros/src/system_composition.rs @@ -1,5 +1,3 @@ -use core::mem; - use bevy_macro_utils::ensure_no_collision; use proc_macro::TokenStream; use quote::{format_ident, quote}; @@ -94,17 +92,26 @@ pub fn compose(input: TokenStream, has_input: bool) -> TokenStream { let runner_types: Vec = visitor .system_paths .iter() - .map(|path| parse_quote_spanned!(path.span()=> #bevy_ecs_path::system::SystemRunner<_, _>)) + .map(|path| parse_quote_spanned!(path.span()=> #bevy_ecs_path::system::SystemRunner<_, _, _>)) .collect(); - let mut builders: Vec = vec![ - parse_quote!(#bevy_ecs_path::system::builder::ParamBuilder); - if has_input { - expr_closure.inputs.len() - 1 - } else { - expr_closure.inputs.len() + let param_count = if has_input { + if expr_closure.inputs.is_empty() { + return TokenStream::from( + syn::Error::new_spanned( + &expr_closure.inputs, + "closure must have at least one parameter", + ) + .into_compile_error(), + ); } - ]; + expr_closure.inputs.len() - 1 + } else { + expr_closure.inputs.len() + }; + + let mut builders: Vec = + vec![parse_quote!(#bevy_ecs_path::system::ParamBuilder); param_count]; let system_builders: Vec = visitor.system_paths.iter().map(|path| parse_quote_spanned!(path.span()=> #bevy_ecs_path::system::ParamBuilder::system(#path))).collect(); builders.push(parse_quote!(#bevy_ecs_path::system::ParamSetBuilder((#(#system_builders,)*)))); diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 3999d9ed35285..bd640224ac1ad 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -20,6 +20,7 @@ use crate::{ World, }, }; +use alloc::borrow::Cow; use core::{fmt::Debug, marker::PhantomData, mem}; use super::{Res, ResMut, RunSystemError, SystemState, SystemStateFlags}; @@ -219,11 +220,26 @@ impl ParamBuilder { Self } - /// Helper method for adding a [`SystemRunner`] as a param, equivalent to [`SystemRunnerBuilder::from_system`] - pub fn system<'w, 's, In: SystemInput + 'static, Out: 'static, Marker>( - system: impl IntoSystem, - ) -> impl SystemParamBuilder> { - SystemRunnerBuilder::from_system(system) + /// Helper method for adding a [`SystemRunner`] as a param, equivalent to [`SystemRunnerBuilder::new`] + pub fn system<'w, 's, In, Out, Marker, Sys>( + system: Sys, + ) -> impl SystemParamBuilder> + where + In: SystemInput, + Sys: IntoSystem, + { + SystemRunnerBuilder::new(IntoSystem::into_system(system)) + } + + /// Helper method for adding a [`SystemRunner`] as a param, equivalent to [`SystemRunnerBuilder::boxed`] + pub fn dyn_system<'w, 's, In, Out, Marker, Sys>( + system: Sys, + ) -> impl SystemParamBuilder> + where + In: SystemInput, + Sys: IntoSystem, + { + SystemRunnerBuilder::boxed(Box::new(IntoSystem::into_system(system))) } } @@ -288,6 +304,7 @@ where Func: SystemParamFunction, Builder: SystemParamBuilder, { + #[inline] /// Returns a new `BuilderSystem` given a system param builder and a system function pub fn new(builder: Builder, func: Func) -> Self { Self { @@ -298,6 +315,17 @@ where }, } } + + #[inline] + /// Returns this `BuilderSystem` with a custom name. + pub fn with_name(mut self, name: impl Into>) -> Self { + if let BuilderSystemInner::Uninitialized { meta, .. } = &mut self.inner { + meta.set_name(name); + } else { + panic!("Called with_name() on an already initialized BuilderSystem"); + } + self + } } enum BuilderSystemInner @@ -888,23 +916,28 @@ unsafe impl> SystemParamBuilder> } /// A [`SystemParamBuilder`] for a [`SystemRunner`] -pub struct SystemRunnerBuilder( - BoxedSystem, -); +pub struct SystemRunnerBuilder(Box); -impl SystemRunnerBuilder { +impl SystemRunnerBuilder { /// Returns a `SystemRunnerBuilder` created from a given system. - pub fn from_system, M>(system: S) -> Self { - Self(Box::new(S::into_system(system))) + pub fn new(system: Sys) -> Self { + Self(Box::new(system)) + } +} + +impl SystemRunnerBuilder> { + /// Returns a `SystemRunnerBuilder` created from a boxed system. + pub fn boxed(system: BoxedSystem) -> Self { + Self(system) } } // SAFETY: the state returned is always valid. In particular the access always // matches the contained system. -unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> - SystemParamBuilder> for SystemRunnerBuilder +unsafe impl<'w, 's, Sys: System + ?Sized> + SystemParamBuilder> for SystemRunnerBuilder { - fn build(mut self, world: &mut World) -> SystemRunnerState { + fn build(mut self, world: &mut World) -> SystemRunnerState { let access = self.0.initialize(world); SystemRunnerState { system: Some(self.0), diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index e829dd1261283..4342cecda7c0c 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -3,7 +3,7 @@ use crate::{ archetype::Archetypes, bundle::Bundles, change_detection::{ComponentTicksMut, ComponentTicksRef, Tick}, - component::{self, ComponentId, Components}, + component::{ComponentId, Components}, entity::{Entities, EntityAllocator}, query::{ Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QuerySingleError, @@ -11,7 +11,7 @@ use crate::{ }, resource::Resource, storage::ResourceData, - system::{BoxedSystem, Query, RunSystemError, Single, System, SystemInput, SystemMeta}, + system::{Query, ReadOnlySystem, RunSystemError, Single, System, SystemInput, SystemMeta}, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut, FromWorld, World, @@ -26,7 +26,6 @@ use core::{ any::Any, fmt::{Debug, Display}, marker::PhantomData, - mem, ops::{Deref, DerefMut}, }; use thiserror::Error; @@ -2766,21 +2765,78 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { } } -pub struct SystemRunner<'w, 's, In: SystemInput = (), Out = ()> { +/// A [`SystemParam`] that allows running other systems. +/// +/// To be useful, this must be configured using a [`SystemRunnerBuilder`](crate::system::SystemRunnerBuilder) +/// to build the system using a [`SystemParamBuilder`](crate::prelude::SystemParamBuilder). +/// +/// Also see the macros [`compose`](crate::system::compose) and [`compose_with`](crate::system::compose_with) +/// for some nice syntax on top of this API. +/// +/// # Examples +/// +/// ``` +/// # use bevy_ecs::{prelude::*, system::*}; +/// # +/// # #[derive(Component)] +/// # struct A; +/// # +/// # #[derive(Component)] +/// # struct B; +/// # let mut world = World::new(); +/// # +/// fn count_a(a: Query<&A>) -> usize { +/// a.count() +/// } +/// +/// fn count_b(b: Query<&B>) -> usize { +/// b.count() +/// } +/// +/// let get_sum = ( +/// ParamBuilder::system(count_a), +/// ParamBuilder::system(count_b) +/// ) +/// .build_state(&mut world) +/// .build_system( +/// |mut run_a: SystemRunner<(), usize>, mut run_b: SystemRunner<(), usize>| -> Result { +/// let a = run_a.run()?; +/// let b = run_b.run()?; +/// Ok(a + b) +/// } +/// ); +/// ``` +pub struct SystemRunner<'w, 's, In = (), Out = (), Sys = dyn System> +where + In: SystemInput, + Sys: System + ?Sized, +{ world: UnsafeWorldCell<'w>, - system: &'s mut dyn System, + system: &'s mut Sys, } -impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemRunner<'w, 's, In, Out> { +impl<'w, 's, In, Out, Sys> SystemRunner<'w, 's, In, Out, Sys> +where + In: SystemInput, + Sys: System + ?Sized, +{ + /// Run the system with input. #[inline] pub fn run_with(&mut self, input: In::Inner<'_>) -> Result { + // SAFETY: + // - all accesses are properly declared in `init_access` + unsafe { self.system.validate_param_unsafe(self.world)? }; // SAFETY: // - all accesses are properly declared in `init_access` unsafe { self.system.run_unsafe(input, self.world) } } } -impl<'w, 's, Out: 'static> SystemRunner<'w, 's, (), Out> { +impl<'w, 's, Out, Sys> SystemRunner<'w, 's, (), Out, Sys> +where + Sys: System + ?Sized, +{ + /// Run the system. #[inline] pub fn run(&mut self) -> Result { self.run_with(()) @@ -2788,18 +2844,20 @@ impl<'w, 's, Out: 'static> SystemRunner<'w, 's, (), Out> { } /// The [`SystemParam::State`] for a [`SystemRunner`]. -pub struct SystemRunnerState { - pub(crate) system: Option>, +pub struct SystemRunnerState { + pub(crate) system: Option>, pub(crate) access: FilteredAccessSet, } // SAFETY: SystemRunner registers all accesses and panics if a conflict is detected. -unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemParam - for SystemRunner<'w, 's, In, Out> +unsafe impl<'w, 's, In, Out, Sys> SystemParam for SystemRunner<'w, 's, In, Out, Sys> +where + In: SystemInput, + Sys: System + ?Sized, { - type State = SystemRunnerState; + type State = SystemRunnerState; - type Item<'world, 'state> = SystemRunner<'world, 'state, In, Out>; + type Item<'world, 'state> = SystemRunner<'world, 'state, In, Out, Sys>; #[inline] fn init_state(_world: &mut World) -> Self::State { @@ -2809,13 +2867,13 @@ unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemParam } } + #[inline] fn init_access( state: &Self::State, system_meta: &mut SystemMeta, component_access_set: &mut FilteredAccessSet, world: &mut World, ) { - //TODO: does this handle exclusive systems properly? let conflicts = component_access_set.get_conflicts(&state.access); if !conflicts.is_empty() { let system_name = system_meta.name(); @@ -2857,25 +2915,27 @@ unsafe impl<'w, 's, In: SystemInput + 'static, Out: 'static> SystemParam } } + #[inline] unsafe fn validate_param( state: &mut Self::State, _system_meta: &SystemMeta, - world: UnsafeWorldCell, + _world: UnsafeWorldCell, ) -> Result<(), SystemParamValidationError> { - state.system.as_mut().ok_or(SystemParamValidationError { - skipped: false, - message: "SystemRunner must be manually initialized, for example with the system builder API.".into(), - param: DebugName::type_name::(), - field: "".into(), - }).and_then(|sys| { - // SAFETY: caller asserts that `world` has all accesses declared in `init_access`, - // which match the underlying system when `state.system` is `Ok`, and otherwise - // this branch will not be entered. - unsafe { sys.validate_param_unsafe(world) } - }) + let err = SystemParamValidationError::invalid::( + "SystemRunner must be manually initialized, for example with the system builder API.", + ); + state.system.as_ref().map(|_| ()).ok_or(err) } } +// SAFETY: if the internal system is readonly, this param can't mutate the world. +unsafe impl<'w, 's, In, Out, Sys> ReadOnlySystemParam for SystemRunner<'w, 's, In, Out, Sys> +where + In: SystemInput, + Sys: System + ReadOnlySystem + ?Sized, +{ +} + /// An error that occurs when a system parameter is not valid, /// used by system executors to determine what to do with a system. /// diff --git a/examples/ecs/system_piping.rs b/examples/ecs/system_piping.rs index a3607c085de6d..d7e35ef93920b 100644 --- a/examples/ecs/system_piping.rs +++ b/examples/ecs/system_piping.rs @@ -1,6 +1,7 @@ //! Illustrates how to make a single system from multiple functions running in sequence, //! passing the output of the first into the input of the next. +use bevy::ecs::system::{compose, RunSystemError}; use bevy::prelude::*; use std::num::ParseIntError; @@ -21,17 +22,29 @@ fn main() { parse_message_system.pipe(handler_system), data_pipe_system.map(|out| info!("{out}")), parse_message_system.map(|out| debug!("{out:?}")), - warning_pipe_system.map(|out| { - if let Err(err) = out { - error!("{err}"); + parse_message_system.map(drop), + // You can also use the `compose!` macro to pipe systems together! + // This is even more powerful, but might be harder to use with + // fully generic code. See the docs on `compose!` and `compose_with!` + // for more info. + compose! { + || -> Result<(), RunSystemError> { + let out = run!(warning_pipe_system)?; + if let Err(err) = out { + error!("{err}"); + } + Ok(()) } - }), - parse_error_message_system.map(|out| { - if let Err(err) = out { - error!("{err}"); + }, + compose! { + || -> Result<(), RunSystemError> { + let out = run!(parse_error_message_system)?; + if let Err(err) = out { + error!("{err}"); + } + Ok(()) } - }), - parse_message_system.map(drop), + }, ), ) .run(); diff --git a/release-content/migration-guides/system_input_unwrap.md b/release-content/migration-guides/system_input_unwrap.md new file mode 100644 index 0000000000000..f555775ffd3f3 --- /dev/null +++ b/release-content/migration-guides/system_input_unwrap.md @@ -0,0 +1,9 @@ +--- +title: "New required method on SystemInput" +pull_requests: [21811] +--- + +Custom implementations of the `SystemInput` trait will need to implement a new +required method: `unwrap`. Like `wrap`, it converts between the inner input item +and the wrapper, but in the opposite direction. In most cases it should be +trivial to add. diff --git a/release-content/release-notes/system_composition.md b/release-content/release-notes/system_composition.md new file mode 100644 index 0000000000000..89efb956353d7 --- /dev/null +++ b/release-content/release-notes/system_composition.md @@ -0,0 +1,49 @@ +--- +title: System Composition +authors: ["@ecoskey"] +pull_requests: [21811] +--- + +## `SystemRunner` SystemParam + +We've been working on some new tools to make composing multiple ECS systems together +even easier. Bevy 0.18 introduces the `SystemRunner` `SystemParam`, allowing running +systems inside other systems! + +```rust +fn count_a(a: Query<&A>) -> usize { + a.count() +} + +fn count_b(b: Query<&B>) -> usize { + b.count() +} + +let get_sum = ( + ParamBuilder::system(count_a), + ParamBuilder::system(count_b) +) +.build_system( + |mut run_a: SystemRunner<(), usize>, mut run_b: SystemRunner<(), usize>| -> Result { + let a = run_a.run()?; + let b = run_b.run()?; + Ok(a + b) + } +); +``` + +## `compose!` and `compose_with!` + +With this new API we've also added some nice macro syntax to go on top. The `compose!` +and `compose_with!` macros will automatically transform a provided closure, making +the new `SystemRunner` params seamless to use. + +```rust +compose! { + || -> Result { + let a = run!(count_a)?; + let b = run!(count_b)?; + Ok(a + b) + } +} +```