From 182fe3292ed108dd7a169df0f47d7fe88e89e716 Mon Sep 17 00:00:00 2001 From: James O'Brien Date: Tue, 21 May 2024 17:58:37 -0700 Subject: [PATCH] Implement a SystemBuilder for building SystemParams (#13123) # Objective - Implement a general purpose mechanism for building `SystemParam`. - Unblock the usage of dynamic queries in regular systems. ## Solution - Implement a `SystemBuilder` type. ## Examples Here are some simple test cases for the builder: ```rust fn local_system(local: Local) -> u64 { *local } fn query_system(query: Query<()>) -> usize { query.iter().count() } fn multi_param_system(a: Local, b: Local) -> u64 { *a + *b + 1 } #[test] fn local_builder() { let mut world = World::new(); let system = SystemBuilder::<()>::new(&mut world) .builder::>(|x| *x = 10) .build(local_system); let result = world.run_system_once(system); assert_eq!(result, 10); } #[test] fn query_builder() { let mut world = World::new(); world.spawn(A); world.spawn_empty(); let system = SystemBuilder::<()>::new(&mut world) .builder::>(|query| { query.with::(); }) .build(query_system); let result = world.run_system_once(system); assert_eq!(result, 1); } #[test] fn multi_param_builder() { let mut world = World::new(); world.spawn(A); world.spawn_empty(); let system = SystemBuilder::<()>::new(&mut world) .param::>() .param::>() .build(multi_param_system); let result = world.run_system_once(system); assert_eq!(result, 1); } ``` This will be expanded as this PR is iterated. --- crates/bevy_ecs/src/lib.rs | 3 +- crates/bevy_ecs/src/system/builder.rs | 214 ++++++++++++++++++ crates/bevy_ecs/src/system/function_system.rs | 41 +++- crates/bevy_ecs/src/system/mod.rs | 2 + crates/bevy_ecs/src/system/system_param.rs | 57 +++++ 5 files changed, 313 insertions(+), 4 deletions(-) create mode 100644 crates/bevy_ecs/src/system/builder.rs diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index ca6e42f3c9..e6cabde398 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -54,7 +54,8 @@ pub mod prelude { }, system::{ Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands, - ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction, + ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemBuilder, + SystemParamFunction, }, world::{EntityMut, EntityRef, EntityWorldMut, FromWorld, World}, }; diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs new file mode 100644 index 0000000000..f3daf8fb06 --- /dev/null +++ b/crates/bevy_ecs/src/system/builder.rs @@ -0,0 +1,214 @@ +use bevy_utils::all_tuples; + +use super::{ + BuildableSystemParam, FunctionSystem, Local, Res, ResMut, Resource, SystemMeta, SystemParam, + SystemParamFunction, SystemState, +}; +use crate::prelude::{FromWorld, Query, World}; +use crate::query::{QueryData, QueryFilter}; + +/// Builder struct used to construct state for [`SystemParam`] passed to a system. +/// +/// ``` +/// # use bevy_ecs::prelude::*; +/// # use bevy_ecs_macros::SystemParam; +/// # use bevy_ecs::system::RunSystemOnce; +/// # +/// # #[derive(Component)] +/// # struct A; +/// # +/// # #[derive(Component)] +/// # struct B; +/// # +/// # #[derive(Resource)] +/// # struct R; +/// # +/// # #[derive(SystemParam)] +/// # struct MyParam; +/// # +/// # let mut world = World::new(); +/// # world.insert_resource(R); +/// +/// fn my_system(res: Res, query: Query<&A>, param: MyParam) { +/// // ... +/// } +/// +/// // Create a builder from the world, helper methods exist to add `SystemParam`, +/// // alternatively use `.param::()` for any other `SystemParam` types. +/// let system = SystemBuilder::<()>::new(&mut world) +/// .resource::() +/// .query::<&A>() +/// .param::() +/// .build(my_system); +/// +/// // Parameters that the builder is initialised with will appear first in the arguments. +/// let system = SystemBuilder::<(Res, Query<&A>)>::new(&mut world) +/// .param::() +/// .build(my_system); +/// +/// // Parameters that implement `BuildableSystemParam` can use `.builder::()` to build in place. +/// let system = SystemBuilder::<()>::new(&mut world) +/// .resource::() +/// .builder::>(|builder| { builder.with::(); }) +/// .param::() +/// .build(my_system); +/// +/// world.run_system_once(system); +///``` +pub struct SystemBuilder<'w, T: SystemParam = ()> { + pub(crate) meta: SystemMeta, + pub(crate) state: T::State, + pub(crate) world: &'w mut World, +} + +impl<'w, T: SystemParam> SystemBuilder<'w, T> { + /// Construct a new builder with the default state for `T` + pub fn new(world: &'w mut World) -> Self { + let mut meta = SystemMeta::new::(); + Self { + state: T::init_state(world, &mut meta), + meta, + world, + } + } + + /// Construct the a system with the built params + pub fn build(self, func: F) -> FunctionSystem + where + F: SystemParamFunction, + { + FunctionSystem::from_builder(self, func) + } + + /// Return the constructed [`SystemState`] + pub fn state(self) -> SystemState { + SystemState::from_builder(self) + } +} + +macro_rules! impl_system_builder { + ($($curr: ident),*) => { + impl<'w, $($curr: SystemParam,)*> SystemBuilder<'w, ($($curr,)*)> { + /// Add `T` as a parameter built from the world + pub fn param(mut self) -> SystemBuilder<'w, ($($curr,)* T,)> { + #[allow(non_snake_case)] + let ($($curr,)*) = self.state; + SystemBuilder { + state: ($($curr,)* T::init_state(self.world, &mut self.meta),), + meta: self.meta, + world: self.world, + } + } + + /// Helper method for reading a [`Resource`] as a param, equivalent to `.param::>()` + pub fn resource(self) -> SystemBuilder<'w, ($($curr,)* Res<'static, T>,)> { + self.param::>() + } + + /// Helper method for mutably accessing a [`Resource`] as a param, equivalent to `.param::>()` + pub fn resource_mut(self) -> SystemBuilder<'w, ($($curr,)* ResMut<'static, T>,)> { + self.param::>() + } + + /// Helper method for adding a [`Local`] as a param, equivalent to `.param::>()` + pub fn local(self) -> SystemBuilder<'w, ($($curr,)* Local<'static, T>,)> { + self.param::>() + } + + /// Helper method for adding a [`Query`] as a param, equivalent to `.param::>()` + pub fn query(self) -> SystemBuilder<'w, ($($curr,)* Query<'static, 'static, D, ()>,)> { + self.query_filtered::() + } + + /// Helper method for adding a filtered [`Query`] as a param, equivalent to `.param::>()` + pub fn query_filtered(self) -> SystemBuilder<'w, ($($curr,)* Query<'static, 'static, D, F>,)> { + self.param::>() + } + + /// Add `T` as a parameter built with the given function + pub fn builder( + mut self, + func: impl FnOnce(&mut T::Builder<'_>), + ) -> SystemBuilder<'w, ($($curr,)* T,)> { + #[allow(non_snake_case)] + let ($($curr,)*) = self.state; + SystemBuilder { + state: ($($curr,)* T::build(self.world, &mut self.meta, func),), + meta: self.meta, + world: self.world, + } + } + } + }; +} + +all_tuples!(impl_system_builder, 0, 15, P); + +#[cfg(test)] +mod tests { + use crate as bevy_ecs; + use crate::prelude::{Component, Query}; + use crate::system::{Local, RunSystemOnce}; + + use super::*; + + #[derive(Component)] + struct A; + + fn local_system(local: Local) -> u64 { + *local + } + + fn query_system(query: Query<()>) -> usize { + query.iter().count() + } + + fn multi_param_system(a: Local, b: Local) -> u64 { + *a + *b + 1 + } + + #[test] + fn local_builder() { + let mut world = World::new(); + + let system = SystemBuilder::<()>::new(&mut world) + .builder::>(|x| *x = 10) + .build(local_system); + + let result = world.run_system_once(system); + assert_eq!(result, 10); + } + + #[test] + fn query_builder() { + let mut world = World::new(); + + world.spawn(A); + world.spawn_empty(); + + let system = SystemBuilder::<()>::new(&mut world) + .builder::>(|query| { + query.with::(); + }) + .build(query_system); + + let result = world.run_system_once(system); + assert_eq!(result, 1); + } + + #[test] + fn multi_param_builder() { + let mut world = World::new(); + + world.spawn(A); + world.spawn_empty(); + + let system = SystemBuilder::<()>::new(&mut world) + .local::() + .param::>() + .build(multi_param_system); + + let result = world.run_system_once(system); + assert_eq!(result, 1); + } +} diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index ac96075017..664f8e8681 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -14,7 +14,7 @@ use std::{borrow::Cow, marker::PhantomData}; #[cfg(feature = "trace")] use bevy_utils::tracing::{info_span, Span}; -use super::{In, IntoSystem, ReadOnlySystem}; +use super::{In, IntoSystem, ReadOnlySystem, SystemBuilder}; /// The metadata of a [`System`]. #[derive(Clone)] @@ -202,6 +202,16 @@ impl SystemState { } } + // Create a [`SystemState`] from a [`SystemBuilder`] + pub(crate) fn from_builder(builder: SystemBuilder) -> Self { + Self { + meta: builder.meta, + param_state: builder.state, + world_id: builder.world.id(), + archetype_generation: ArchetypeGeneration::initial(), + } + } + /// Gets the metadata for this instance. #[inline] pub fn meta(&self) -> &SystemMeta { @@ -397,6 +407,23 @@ where marker: PhantomData Marker>, } +impl FunctionSystem +where + F: SystemParamFunction, +{ + // Create a [`FunctionSystem`] from a [`SystemBuilder`] + pub(crate) fn from_builder(builder: SystemBuilder, func: F) -> Self { + Self { + func, + param_state: Some(builder.state), + system_meta: builder.meta, + world_id: Some(builder.world.id()), + archetype_generation: ArchetypeGeneration::initial(), + marker: PhantomData, + } + } +} + // De-initializes the cloned system. impl Clone for FunctionSystem where @@ -517,9 +544,17 @@ where #[inline] fn initialize(&mut self, world: &mut World) { - self.world_id = Some(world.id()); + if let Some(id) = self.world_id { + assert_eq!( + id, + world.id(), + "System built with a different world than the one it was added to.", + ); + } else { + self.world_id = Some(world.id()); + self.param_state = Some(F::Param::init_state(world, &mut self.system_meta)); + } self.system_meta.last_run = world.change_tick().relative_to(Tick::MAX); - self.param_state = Some(F::Param::init_state(world, &mut self.system_meta)); } fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) { diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 19785a7243..c96dee3e98 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -102,6 +102,7 @@ //! - [`()` (unit primitive type)](https://doc.rust-lang.org/stable/std/primitive.unit.html) mod adapter_system; +mod builder; mod combinator; mod commands; mod exclusive_function_system; @@ -117,6 +118,7 @@ mod system_registry; use std::{any::TypeId, borrow::Cow}; pub use adapter_system::*; +pub use builder::*; pub use combinator::*; pub use commands::*; pub use exclusive_function_system::*; diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index d85f50ae25..bdf29d2bb9 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -5,6 +5,7 @@ use crate::{ change_detection::{Ticks, TicksMut}, component::{ComponentId, ComponentTicks, Components, Tick}, entity::Entities, + prelude::QueryBuilder, query::{ Access, FilteredAccess, FilteredAccessSet, QueryData, QueryFilter, QueryState, ReadOnlyQueryData, @@ -175,6 +176,19 @@ pub unsafe trait SystemParam: Sized { ) -> Self::Item<'world, 'state>; } +/// A parameter that can be built with [`SystemBuilder`](crate::system::builder::SystemBuilder) +pub trait BuildableSystemParam: SystemParam { + /// A mutable reference to this type will be passed to the builder function + type Builder<'b>; + + /// Constructs [`SystemParam::State`] for `Self` using a given builder function + fn build( + world: &mut World, + meta: &mut SystemMeta, + func: impl FnOnce(&mut Self::Builder<'_>), + ) -> Self::State; +} + /// A [`SystemParam`] that only reads a given [`World`]. /// /// # Safety @@ -234,6 +248,35 @@ unsafe impl SystemParam for Qu } } +impl<'w, 's, D: QueryData + 'static, F: QueryFilter + 'static> BuildableSystemParam + for Query<'w, 's, D, F> +{ + type Builder<'b> = QueryBuilder<'b, D, F>; + + #[inline] + fn build( + world: &mut World, + system_meta: &mut SystemMeta, + build: impl FnOnce(&mut Self::Builder<'_>), + ) -> Self::State { + let mut builder = QueryBuilder::new(world); + build(&mut builder); + let state = builder.build(); + assert_component_access_compatibility( + &system_meta.name, + std::any::type_name::(), + std::any::type_name::(), + &system_meta.component_access_set, + &state.component_access, + world, + ); + system_meta + .component_access_set + .add(state.component_access.clone()); + state + } +} + fn assert_component_access_compatibility( system_name: &str, query_type: &'static str, @@ -777,6 +820,20 @@ unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> { } } +impl<'w, T: FromWorld + Send + 'static> BuildableSystemParam for Local<'w, T> { + type Builder<'b> = T; + + fn build( + world: &mut World, + _meta: &mut SystemMeta, + func: impl FnOnce(&mut Self::Builder<'_>), + ) -> Self::State { + let mut value = T::from_world(world); + func(&mut value); + SyncCell::new(value) + } +} + /// Types that can be used with [`Deferred`] in systems. /// This allows storing system-local data which is used to defer [`World`] mutations. ///