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 6936ca4aff3a6..de6baac8550f8 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,85 @@ 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 new file mode 100644 index 0000000000000..3bf1f82dbc79b --- /dev/null +++ b/crates/bevy_ecs/macros/src/system_composition.rs @@ -0,0 +1,128 @@ +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 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,)*)))); + + 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/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/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 937911ca834c1..bd640224ac1ad 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -1,23 +1,29 @@ 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, + BoxedSystem, DynSystemParam, DynSystemParamState, FromInput, FunctionSystem, If, + IntoResult, IntoSystem, Local, ParamSet, Query, ReadOnlySystem, System, SystemInput, + SystemMeta, SystemParam, SystemParamFunction, SystemParamValidationError, SystemRunner, + SystemRunnerState, }, world::{ - FilteredResources, FilteredResourcesBuilder, FilteredResourcesMut, - FilteredResourcesMutBuilder, FromWorld, World, + unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, + FilteredResourcesBuilder, FilteredResourcesMut, FilteredResourcesMutBuilder, FromWorld, + World, }, }; -use core::fmt::Debug; +use alloc::borrow::Cow; +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 +125,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. @@ -202,6 +219,288 @@ impl ParamBuilder { ) -> impl SystemParamBuilder> { Self } + + /// 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))) + } +} + +/// 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, +{ + #[inline] + /// 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::(), + }, + } + } + + #[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 +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`, @@ -616,6 +915,37 @@ unsafe impl> SystemParamBuilder> } } +/// A [`SystemParamBuilder`] for a [`SystemRunner`] +pub struct SystemRunnerBuilder(Box); + +impl SystemRunnerBuilder { + /// Returns a `SystemRunnerBuilder` created from a given 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, Sys: System + ?Sized> + 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/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 5f6653be39834..af143b2071416 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}, }; @@ -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 { @@ -234,15 +251,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 +267,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 +331,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 +493,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, { @@ -485,7 +503,7 @@ where state: Option>, system_meta: SystemMeta, // NOTE: PhantomData T> gives this safe Send/Sync impls - marker: PhantomData (Marker, Out)>, + marker: PhantomData (Marker, Out)>, } /// The state of a [`FunctionSystem`], which must be initialized with @@ -500,10 +518,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 +545,7 @@ where } // De-initializes the cloned system. -impl Clone for FunctionSystem +impl Clone for FunctionSystem where F: SystemParamFunction + Clone, { @@ -535,23 +566,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 +620,7 @@ impl IntoResult for Never { } } -impl FunctionSystem +impl FunctionSystem where F: SystemParamFunction, { @@ -607,13 +631,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 +662,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 +775,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, + >, { } @@ -799,8 +831,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)); /// } diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index 429f4df018ed6..c37a08cd65be1 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. /// @@ -294,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, }; @@ -327,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(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); + } } 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`]. diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 80d449017d2eb..4342cecda7c0c 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -11,7 +11,7 @@ use crate::{ }, resource::Resource, storage::ResourceData, - system::{Query, Single, SystemMeta}, + system::{Query, ReadOnlySystem, RunSystemError, Single, System, SystemInput, SystemMeta}, world::{ unsafe_world_cell::UnsafeWorldCell, DeferredWorld, FilteredResources, FilteredResourcesMut, FromWorld, World, @@ -2765,6 +2765,177 @@ unsafe impl SystemParam for FilteredResourcesMut<'_, '_> { } } +/// 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 Sys, +} + +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, Sys> SystemRunner<'w, 's, (), Out, Sys> +where + Sys: System + ?Sized, +{ + /// Run the system. + #[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, Out, Sys> SystemParam for SystemRunner<'w, 's, In, Out, Sys> +where + In: SystemInput, + Sys: System + ?Sized, +{ + type State = SystemRunnerState; + + type Item<'world, 'state> = SystemRunner<'world, 'state, In, Out, Sys>; + + #[inline] + fn init_state(_world: &mut World) -> Self::State { + SystemRunnerState { + system: None, + access: FilteredAccessSet::default(), + } + } + + #[inline] + fn init_access( + state: &Self::State, + system_meta: &mut SystemMeta, + component_access_set: &mut FilteredAccessSet, + world: &mut World, + ) { + 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); + } + } + + #[inline] + unsafe fn validate_param( + state: &mut Self::State, + _system_meta: &SystemMeta, + _world: UnsafeWorldCell, + ) -> Result<(), SystemParamValidationError> { + 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/function_system_generics.md b/release-content/migration-guides/function_system_generics.md new file mode 100644 index 0000000000000..a302dd72cf329 --- /dev/null +++ b/release-content/migration-guides/function_system_generics.md @@ -0,0 +1,21 @@ +--- +title: "FunctionSystem Generics" +authors: ["@ecoskey"] +pull_requests: [21917] +--- + +`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. 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) + } +} +```