From 1efdbb7e3ea2c7226385eb457123322430891b1d Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Sat, 7 Jan 2023 23:20:32 +0000 Subject: [PATCH] Remove the `SystemParamState` trait and remove types like `ResState` (#6919) Spiritual successor to #5205. Actual successor to #6865. # Objective Currently, system params are defined using three traits: `SystemParam`, `ReadOnlySystemParam`, `SystemParamState`. The behavior for each param is specified by the `SystemParamState` trait, while `SystemParam` simply defers to the state. Splitting the traits in this way makes it easier to implement within macros, but it increases the cognitive load. Worst of all, this approach requires each `MySystemParam` to have a public `MySystemParamState` type associated with it. ## Solution * Merge the trait `SystemParamState` into `SystemParam`. * Remove all trivial `SystemParam` state types. * `OptionNonSendMutState`: you will not be missed. --- - [x] Fix/resolve the remaining test failure. ## Changelog * Removed the trait `SystemParamState`, merging its functionality into `SystemParam`. ## Migration Guide **Note**: this should replace the migration guide for #6865. This is relative to Bevy 0.9, not main. The traits `SystemParamState` and `SystemParamFetch` have been removed, and their functionality has been transferred to `SystemParam`. ```rust // Before (0.9) impl SystemParam for MyParam<'_, '_> { type State = MyParamState; } unsafe impl SystemParamState for MyParamState { fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { ... } } unsafe impl<'w, 's> SystemParamFetch<'w, 's> for MyParamState { type Item = MyParam<'w, 's>; fn get_param(&mut self, ...) -> Self::Item; } unsafe impl ReadOnlySystemParamFetch for MyParamState { } // After (0.10) unsafe impl SystemParam for MyParam<'_, '_> { type State = MyParamState; type Item<'w, 's> = MyParam<'w, 's>; fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { ... } fn get_param<'w, 's>(state: &mut Self::State, ...) -> Self::Item<'w, 's>; } unsafe impl ReadOnlySystemParam for MyParam<'_, '_> { } ``` The trait `ReadOnlySystemParamFetch` has been replaced with `ReadOnlySystemParam`. ```rust // Before unsafe impl ReadOnlySystemParamFetch for MyParamState {} // After unsafe impl ReadOnlySystemParam for MyParam<'_, '_> {} ``` --- crates/bevy_ecs/macros/src/lib.rs | 94 ++- .../src/system/commands/parallel_scope.rs | 21 +- .../src/system/exclusive_function_system.rs | 11 +- .../src/system/exclusive_system_param.rs | 70 +-- crates/bevy_ecs/src/system/function_system.rs | 36 +- crates/bevy_ecs/src/system/system_param.rs | 564 ++++++------------ crates/bevy_render/src/extract_param.rs | 45 +- 7 files changed, 301 insertions(+), 540 deletions(-) diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index b697c9d0ba..1990f89107 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -216,7 +216,6 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { let mut tokens = TokenStream::new(); let max_params = 8; let params = get_idents(|i| format!("P{i}"), max_params); - let params_state = get_idents(|i| format!("PF{i}"), max_params); let metas = get_idents(|i| format!("m{i}"), max_params); let mut param_fn_muts = Vec::new(); for (i, param) in params.iter().enumerate() { @@ -238,7 +237,7 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { // Conflicting params in ParamSet are not accessible at the same time // ParamSets are guaranteed to not conflict with other SystemParams unsafe { - <#param::State as SystemParamState>::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick) + #param::get_param(&mut self.param_states.#index, &self.system_meta, self.world, self.change_tick) } } }); @@ -246,36 +245,29 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { for param_count in 1..=max_params { let param = ¶ms[0..param_count]; - let param_state = ¶ms_state[0..param_count]; let meta = &metas[0..param_count]; let param_fn_mut = ¶m_fn_muts[0..param_count]; tokens.extend(TokenStream::from(quote! { - impl<'w, 's, #(#param: SystemParam,)*> SystemParam for ParamSet<'w, 's, (#(#param,)*)> - { - type State = ParamSetState<(#(#param::State,)*)>; - } - - // SAFETY: All parameters are constrained to ReadOnlyState, so World is only read - + // SAFETY: All parameters are constrained to ReadOnlySystemParam, so World is only read unsafe impl<'w, 's, #(#param,)*> ReadOnlySystemParam for ParamSet<'w, 's, (#(#param,)*)> where #(#param: ReadOnlySystemParam,)* { } // SAFETY: Relevant parameter ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any ParamState conflicts // with any prior access, a panic will occur. - - unsafe impl<#(#param_state: SystemParamState,)*> SystemParamState for ParamSetState<(#(#param_state,)*)> + unsafe impl<'_w, '_s, #(#param: SystemParam,)*> SystemParam for ParamSet<'_w, '_s, (#(#param,)*)> { - type Item<'w, 's> = ParamSet<'w, 's, (#(<#param_state as SystemParamState>::Item::<'w, 's>,)*)>; + type State = (#(#param::State,)*); + type Item<'w, 's> = ParamSet<'w, 's, (#(#param,)*)>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { #( // Pretend to add each param to the system alone, see if it conflicts let mut #meta = system_meta.clone(); #meta.component_access_set.clear(); #meta.archetype_component_access.clear(); - #param_state::init(world, &mut #meta); - let #param = #param_state::init(world, &mut system_meta.clone()); + #param::init_state(world, &mut #meta); + let #param = #param::init_state(world, &mut system_meta.clone()); )* #( system_meta @@ -285,29 +277,26 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { .archetype_component_access .extend(&#meta.archetype_component_access); )* - ParamSetState((#(#param,)*)) + (#(#param,)*) } - fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) { - let (#(#param,)*) = &mut self.0; - #( - #param.new_archetype(archetype, system_meta); - )* + fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { + <(#(#param,)*) as SystemParam>::new_archetype(state, archetype, system_meta); } - fn apply(&mut self, system_meta: &SystemMeta, world: &mut World) { - self.0.apply(system_meta, world) + fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { + <(#(#param,)*) as SystemParam>::apply(state, system_meta, world); } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { ParamSet { - param_states: &mut state.0, + param_states: state, system_meta: system_meta.clone(), world, change_tick, @@ -317,7 +306,6 @@ pub fn impl_param_set(_input: TokenStream) -> TokenStream { impl<'w, 's, #(#param: SystemParam,)*> ParamSet<'w, 's, (#(#param,)*)> { - #(#param_fn_mut)* } })); @@ -411,7 +399,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } } - let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + let (_impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let lifetimeless_generics: Vec<_> = generics .params @@ -419,6 +407,12 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { .filter(|g| !matches!(g, GenericParam::Lifetime(_))) .collect(); + let mut shadowed_lifetimes: Vec<_> = generics.lifetimes().map(|x| x.lifetime.clone()).collect(); + for lifetime in &mut shadowed_lifetimes { + let shadowed_ident = format_ident!("_{}", lifetime.ident); + lifetime.ident = shadowed_ident; + } + let mut punctuated_generics = Punctuated::<_, Token![,]>::new(); punctuated_generics.extend(lifetimeless_generics.iter().map(|g| match g { GenericParam::Type(g) => GenericParam::Type(TypeParam { @@ -432,15 +426,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { _ => unreachable!(), })); - let mut punctuated_generics_no_bounds = punctuated_generics.clone(); - for g in &mut punctuated_generics_no_bounds { - match g { - GenericParam::Type(g) => g.bounds.clear(), - GenericParam::Lifetime(g) => g.bounds.clear(), - GenericParam::Const(_) => {} - } - } - let mut punctuated_generic_idents = Punctuated::<_, Token![,]>::new(); punctuated_generic_idents.extend(lifetimeless_generics.iter().map(|g| match g { GenericParam::Type(g) => &g.ident, @@ -479,10 +464,6 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { // The struct can still be accessed via SystemParam::State, e.g. EventReaderState can be accessed via // as SystemParam>::State const _: () = { - impl #impl_generics #path::system::SystemParam for #struct_name #ty_generics #where_clause { - type State = FetchState<'static, 'static, #punctuated_generic_idents>; - } - #[doc(hidden)] #state_struct_visibility struct FetchState <'w, 's, #(#lifetimeless_generics,)*> { state: (#(<#tuple_types as #path::system::SystemParam>::State,)*), @@ -492,34 +473,33 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { )>, } - unsafe impl<#punctuated_generics> #path::system::SystemParamState for - FetchState<'static, 'static, #punctuated_generic_idents> - #where_clause { - type Item<'w, 's> = #struct_name #ty_generics; + unsafe impl<'w, 's, #punctuated_generics> #path::system::SystemParam for #struct_name #ty_generics #where_clause { + type State = FetchState<'static, 'static, #punctuated_generic_idents>; + type Item<'_w, '_s> = #struct_name <#(#shadowed_lifetimes,)* #punctuated_generic_idents>; - fn init(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self { - Self { - state: #path::system::SystemParamState::init(world, system_meta), + fn init_state(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self::State { + FetchState { + state: <(#(#tuple_types,)*) as #path::system::SystemParam>::init_state(world, system_meta), marker: std::marker::PhantomData, } } - fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) { - self.state.new_archetype(archetype, system_meta) + fn new_archetype(state: &mut Self::State, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) { + <(#(#tuple_types,)*) as #path::system::SystemParam>::new_archetype(&mut state.state, archetype, system_meta) } - fn apply(&mut self, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) { - self.state.apply(system_meta, world) + fn apply(state: &mut Self::State, system_meta: &#path::system::SystemMeta, world: &mut #path::world::World) { + <(#(#tuple_types,)*) as #path::system::SystemParam>::apply(&mut state.state, system_meta, world); } - unsafe fn get_param<'w, 's>( - state: &'s mut Self, + unsafe fn get_param<'w2, 's2>( + state: &'s2 mut Self::State, system_meta: &#path::system::SystemMeta, - world: &'w #path::world::World, + world: &'w2 #path::world::World, change_tick: u32, - ) -> Self::Item<'w, 's> { + ) -> Self::Item<'w2, 's2> { let (#(#tuple_patterns,)*) = < - <(#(#tuple_types,)*) as #path::system::SystemParam>::State as #path::system::SystemParamState + (#(#tuple_types,)*) as #path::system::SystemParam >::get_param(&mut state.state, system_meta, world, change_tick); #struct_name { #(#fields: #field_locals,)* diff --git a/crates/bevy_ecs/src/system/commands/parallel_scope.rs b/crates/bevy_ecs/src/system/commands/parallel_scope.rs index 72c8118aa5..5975e9d668 100644 --- a/crates/bevy_ecs/src/system/commands/parallel_scope.rs +++ b/crates/bevy_ecs/src/system/commands/parallel_scope.rs @@ -5,14 +5,14 @@ use thread_local::ThreadLocal; use crate::{ entity::Entities, prelude::World, - system::{SystemMeta, SystemParam, SystemParamState}, + system::{SystemMeta, SystemParam}, }; use super::{CommandQueue, Commands}; +/// The internal [`SystemParam`] state of the [`ParallelCommands`] type #[doc(hidden)] #[derive(Default)] -/// The internal [`SystemParamState`] of the [`ParallelCommands`] type pub struct ParallelCommandsState { thread_local_storage: ThreadLocal>, } @@ -48,30 +48,27 @@ pub struct ParallelCommands<'w, 's> { entities: &'w Entities, } -impl SystemParam for ParallelCommands<'_, '_> { - type State = ParallelCommandsState; -} - // SAFETY: no component or resource access to report -unsafe impl SystemParamState for ParallelCommandsState { +unsafe impl SystemParam for ParallelCommands<'_, '_> { + type State = ParallelCommandsState; type Item<'w, 's> = ParallelCommands<'w, 's>; - fn init(_: &mut World, _: &mut crate::system::SystemMeta) -> Self { - Self::default() + fn init_state(_: &mut World, _: &mut crate::system::SystemMeta) -> Self::State { + ParallelCommandsState::default() } - fn apply(&mut self, _system_meta: &SystemMeta, world: &mut World) { + fn apply(state: &mut Self::State, _system_meta: &SystemMeta, world: &mut World) { #[cfg(feature = "trace")] let _system_span = bevy_utils::tracing::info_span!("system_commands", name = _system_meta.name()) .entered(); - for cq in &mut self.thread_local_storage { + for cq in &mut state.thread_local_storage { cq.get_mut().apply(world); } } unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, _: &crate::system::SystemMeta, world: &'w World, _: u32, diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 8b1607c758..272b9bce17 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -6,7 +6,7 @@ use crate::{ schedule::{SystemLabel, SystemLabelId}, system::{ check_system_change_tick, AsSystemLabel, ExclusiveSystemParam, ExclusiveSystemParamItem, - ExclusiveSystemParamState, IntoSystem, System, SystemMeta, SystemTypeIdLabel, + IntoSystem, System, SystemMeta, SystemTypeIdLabel, }, world::{World, WorldId}, }; @@ -94,7 +94,7 @@ where let saved_last_tick = world.last_change_tick; world.last_change_tick = self.system_meta.last_change_tick; - let params = ::State::get_param( + let params = Param::get_param( self.param_state.as_mut().expect(PARAM_MESSAGE), &self.system_meta, ); @@ -122,17 +122,14 @@ where #[inline] fn apply_buffers(&mut self, world: &mut World) { let param_state = self.param_state.as_mut().expect(PARAM_MESSAGE); - param_state.apply(world); + Param::apply(param_state, world); } #[inline] fn initialize(&mut self, world: &mut World) { self.world_id = Some(world.id()); self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); - self.param_state = Some(::init( - world, - &mut self.system_meta, - )); + self.param_state = Some(Param::init(world, &mut self.system_meta)); } fn update_archetype_component_access(&mut self, _world: &World) {} diff --git a/crates/bevy_ecs/src/system/exclusive_system_param.rs b/crates/bevy_ecs/src/system/exclusive_system_param.rs index 6c52738901..4b63426604 100644 --- a/crates/bevy_ecs/src/system/exclusive_system_param.rs +++ b/crates/bevy_ecs/src/system/exclusive_system_param.rs @@ -1,108 +1,89 @@ use crate::{ prelude::{FromWorld, QueryState}, query::{ReadOnlyWorldQuery, WorldQuery}, - system::{Local, LocalState, SystemMeta, SystemParam, SystemState}, + system::{Local, SystemMeta, SystemParam, SystemState}, world::World, }; use bevy_ecs_macros::all_tuples; use bevy_utils::synccell::SyncCell; pub trait ExclusiveSystemParam: Sized { - type State: ExclusiveSystemParamState; -} + type State: Send + Sync + 'static; + type Item<'s>: ExclusiveSystemParam; -pub type ExclusiveSystemParamItem<'s, P> = - <

::State as ExclusiveSystemParamState>::Item<'s>; - -/// The state of a [`SystemParam`]. -pub trait ExclusiveSystemParamState: Send + Sync + 'static { - type Item<'s>: ExclusiveSystemParam; - - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self; + fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self::State; #[inline] - fn apply(&mut self, _world: &mut World) {} + fn apply(_state: &mut Self::State, _world: &mut World) {} - fn get_param<'s>(state: &'s mut Self, system_meta: &SystemMeta) -> Self::Item<'s>; + fn get_param<'s>(state: &'s mut Self::State, system_meta: &SystemMeta) -> Self::Item<'s>; } +pub type ExclusiveSystemParamItem<'s, P> =

::Item<'s>; + impl<'a, Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> ExclusiveSystemParam for &'a mut QueryState { type State = QueryState; -} - -impl ExclusiveSystemParamState - for QueryState -{ type Item<'s> = &'s mut QueryState; - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { + fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { QueryState::new(world) } - fn get_param<'s>(state: &'s mut Self, _system_meta: &SystemMeta) -> Self::Item<'s> { + fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { state } } impl<'a, P: SystemParam + 'static> ExclusiveSystemParam for &'a mut SystemState

{ type State = SystemState

; -} - -impl ExclusiveSystemParamState for SystemState

{ type Item<'s> = &'s mut SystemState

; - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { + fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { SystemState::new(world) } - fn get_param<'s>(state: &'s mut Self, _system_meta: &SystemMeta) -> Self::Item<'s> { + fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { state } } -impl<'s, T: FromWorld + Send + Sync + 'static> ExclusiveSystemParam for Local<'s, T> { - type State = LocalState; -} - -impl ExclusiveSystemParamState for LocalState { +impl<'_s, T: FromWorld + Send + Sync + 'static> ExclusiveSystemParam for Local<'_s, T> { + type State = SyncCell; type Item<'s> = Local<'s, T>; - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self(SyncCell::new(T::from_world(world))) + fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + SyncCell::new(T::from_world(world)) } - fn get_param<'s>(state: &'s mut Self, _system_meta: &SystemMeta) -> Self::Item<'s> { - Local(state.0.get()) + fn get_param<'s>(state: &'s mut Self::State, _system_meta: &SystemMeta) -> Self::Item<'s> { + Local(state.get()) } } macro_rules! impl_exclusive_system_param_tuple { ($($param: ident),*) => { - impl<$($param: ExclusiveSystemParam),*> ExclusiveSystemParam for ($($param,)*) { - type State = ($($param::State,)*); - } - #[allow(unused_variables)] #[allow(non_snake_case)] - impl<$($param: ExclusiveSystemParamState),*> ExclusiveSystemParamState for ($($param,)*) { + impl<$($param: ExclusiveSystemParam),*> ExclusiveSystemParam for ($($param,)*) { + type State = ($($param::State,)*); type Item<'s> = ($($param::Item<'s>,)*); #[inline] - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { (($($param::init(_world, _system_meta),)*)) } #[inline] - fn apply(&mut self, _world: &mut World) { - let ($($param,)*) = self; - $($param.apply(_world);)* + fn apply(state: &mut Self::State, _world: &mut World) { + let ($($param,)*) = state; + $($param::apply($param, _world);)* } #[inline] #[allow(clippy::unused_unit)] fn get_param<'s>( - state: &'s mut Self, + state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Self::Item<'s> { @@ -110,7 +91,6 @@ macro_rules! impl_exclusive_system_param_tuple { ($($param::get_param($param, system_meta),)*) } } - }; } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index 2cc88f9e30..f99ae67b15 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -5,10 +5,7 @@ use crate::{ prelude::FromWorld, query::{Access, FilteredAccessSet}, schedule::{SystemLabel, SystemLabelId}, - system::{ - check_system_change_tick, ReadOnlySystemParam, System, SystemParam, SystemParamItem, - SystemParamState, - }, + system::{check_system_change_tick, ReadOnlySystemParam, System, SystemParam, SystemParamItem}, world::{World, WorldId}, }; use bevy_ecs_macros::all_tuples; @@ -141,7 +138,7 @@ impl SystemMeta { /// ``` pub struct SystemState { meta: SystemMeta, - param_state: ::State, + param_state: Param::State, world_id: WorldId, archetype_generation: ArchetypeGeneration, } @@ -150,7 +147,7 @@ impl SystemState { pub fn new(world: &mut World) -> Self { let mut meta = SystemMeta::new::(); meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); - let param_state = ::init(world, &mut meta); + let param_state = Param::init_state(world, &mut meta); Self { meta, param_state, @@ -188,7 +185,7 @@ impl SystemState { /// This function should be called manually after the values returned by [`SystemState::get`] and [`SystemState::get_mut`] /// are finished being used. pub fn apply(&mut self, world: &mut World) { - self.param_state.apply(&self.meta, world); + Param::apply(&mut self.param_state, &self.meta, world); } #[inline] @@ -204,7 +201,8 @@ impl SystemState { let archetype_index_range = old_generation.value()..new_generation.value(); for archetype_index in archetype_index_range { - self.param_state.new_archetype( + Param::new_archetype( + &mut self.param_state, &archetypes[ArchetypeId::new(archetype_index)], &mut self.meta, ); @@ -223,12 +221,7 @@ impl SystemState { world: &'w World, ) -> SystemParamItem<'w, 's, Param> { let change_tick = world.increment_change_tick(); - let param = ::get_param( - &mut self.param_state, - &self.meta, - world, - change_tick, - ); + let param = Param::get_param(&mut self.param_state, &self.meta, world, change_tick); self.meta.last_change_tick = change_tick; param } @@ -400,7 +393,7 @@ where // We update the archetype component access correctly based on `Param`'s requirements // in `update_archetype_component_access`. // Our caller upholds the requirements. - let params = ::State::get_param( + let params = Param::get_param( self.param_state.as_mut().expect(Self::PARAM_MESSAGE), &self.system_meta, world, @@ -422,17 +415,14 @@ where #[inline] fn apply_buffers(&mut self, world: &mut World) { let param_state = self.param_state.as_mut().expect(Self::PARAM_MESSAGE); - param_state.apply(&self.system_meta, world); + Param::apply(param_state, &self.system_meta, world); } #[inline] fn initialize(&mut self, world: &mut World) { self.world_id = Some(world.id()); self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE); - self.param_state = Some(::init( - world, - &mut self.system_meta, - )); + self.param_state = Some(Param::init_state(world, &mut self.system_meta)); } fn update_archetype_component_access(&mut self, world: &World) { @@ -443,7 +433,9 @@ where let archetype_index_range = old_generation.value()..new_generation.value(); for archetype_index in archetype_index_range { - self.param_state.as_mut().unwrap().new_archetype( + let param_state = self.param_state.as_mut().unwrap(); + Param::new_archetype( + param_state, &archetypes[ArchetypeId::new(archetype_index)], &mut self.system_meta, ); @@ -514,7 +506,7 @@ impl Copy for SystemTypeIdLabel {} /// pub fn pipe( /// mut a: A, /// mut b: B, -/// ) -> impl FnMut(In, ParamSet<(SystemParamItem, SystemParamItem)>) -> BOut +/// ) -> impl FnMut(In, ParamSet<(AParam, BParam)>) -> BOut /// where /// // We need A and B to be systems, add those bounds /// A: SystemParamFunction, diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 7751995ef1..814ac6316e 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -72,7 +72,7 @@ use std::{ /// /// ```text /// expected ... [ParamType] -/// found associated type `<<[ParamType] as SystemParam>::State as SystemParamState>::Item<'_, '_>` +/// found associated type `<[ParamType] as SystemParam>::Item<'_, '_>` /// ``` /// where `[ParamType]` is the type of one of your fields. /// To solve this error, you can wrap the field of type `[ParamType]` with [`StaticSystemParam`] @@ -81,7 +81,7 @@ use std::{ /// ## Details /// /// The derive macro requires that the [`SystemParam`] implementation of -/// each field `F`'s [`State`](`SystemParam::State`)'s [`Item`](`SystemParamState::Item`) is itself `F` +/// each field `F`'s [`Item`](`SystemParam::Item`)'s is itself `F` /// (ignoring lifetimes for simplicity). /// This assumption is due to type inference reasons, so that the derived [`SystemParam`] can be /// used as an argument to a function system. @@ -121,35 +121,46 @@ use std::{ /// /// [`SyncCell`]: bevy_utils::synccell::SyncCell /// [`Exclusive`]: https://doc.rust-lang.org/nightly/std/sync/struct.Exclusive.html -pub trait SystemParam: Sized { - type State: SystemParamState; -} - -pub type SystemParamItem<'w, 's, P> = <

::State as SystemParamState>::Item<'w, 's>; - -/// The state of a [`SystemParam`]. /// /// # Safety /// -/// It is the implementor's responsibility to ensure `system_meta` is populated with the _exact_ -/// [`World`] access used by the [`SystemParamState`]. -/// Additionally, it is the implementor's responsibility to ensure there is no -/// conflicting access across all [`SystemParam`]'s. -pub unsafe trait SystemParamState: Send + Sync + 'static { - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self; +/// The implementor must ensure that [`SystemParam::init_state`] correctly registers all +/// [`World`] accesses used by this [`SystemParam`] with the provided [`system_meta`](SystemMeta). +pub unsafe trait SystemParam: Sized { + /// Used to store data which persists across invocations of a system. + type State: Send + Sync + 'static; + + /// The item type returned when constructing this system param. + /// The value of this associated type should be `Self`, instantiated with new lifetimes. + /// + /// You could think of `SystemParam::Item<'w, 's>` as being an *operation* that changes the lifetimes bound to `Self`. + type Item<'world, 'state>: SystemParam; + + /// Registers any [`World`] access used by this [`SystemParam`] + /// and creates a new instance of this param's [`State`](Self::State). + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State; + + /// For the specified [`Archetype`], registers the components accessed by this [`SystemParam`] (if applicable). #[inline] - fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) {} + fn new_archetype( + _state: &mut Self::State, + _archetype: &Archetype, + _system_meta: &mut SystemMeta, + ) { + } + + /// Applies any deferred mutations stored in this [`SystemParam`]'s state. + /// This is used to apply [`Commands`] at the end of a stage. #[inline] #[allow(unused_variables)] - fn apply(&mut self, system_meta: &SystemMeta, _world: &mut World) {} + fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {} - type Item<'world, 'state>: SystemParam; /// # Safety /// - /// This call might access any of the input parameters in an unsafe way. Make sure the data - /// access is safe in the context of the system scheduler. + /// This call might use any of the [`World`] accesses that were registered in [`Self::init_state`]. + /// You must ensure that none of those accesses conflict with any other [`SystemParam`]s running in parallel with this one. unsafe fn get_param<'world, 'state>( - state: &'state mut Self, + state: &'state mut Self::State, system_meta: &SystemMeta, world: &'world World, change_tick: u32, @@ -159,14 +170,11 @@ pub unsafe trait SystemParamState: Send + Sync + 'static { /// A [`SystemParam`] that only reads a given [`World`]. /// /// # Safety -/// This must only be implemented for [`SystemParam`] impls that exclusively read the World passed in to [`SystemParamState::get_param`] +/// This must only be implemented for [`SystemParam`] impls that exclusively read the World passed in to [`SystemParam::get_param`] pub unsafe trait ReadOnlySystemParam: SystemParam {} -impl<'w, 's, Q: WorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> SystemParam - for Query<'w, 's, Q, F> -{ - type State = QueryState; -} +/// Shorthand way of accessing the associated type [`SystemParam::Item`] for a given [`SystemParam`]. +pub type SystemParamItem<'w, 's, P> =

::Item<'w, 's>; // SAFETY: QueryState is constrained to read-only fetches, so it only reads World. unsafe impl<'w, 's, Q: ReadOnlyWorldQuery + 'static, F: ReadOnlyWorldQuery + 'static> @@ -175,13 +183,14 @@ unsafe impl<'w, 's, Q: ReadOnlyWorldQuery + 'static, F: ReadOnlyWorldQuery + 'st } // SAFETY: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If -// this QueryState conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState - for QueryState +// this Query conflicts with any prior access, a panic will occur. +unsafe impl SystemParam + for Query<'_, '_, Q, F> { + type State = QueryState; type Item<'w, 's> = Query<'w, 's, Q, F>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let state = QueryState::new(world); assert_component_access_compatibility( &system_meta.name, @@ -200,16 +209,16 @@ unsafe impl SystemPara state } - fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) { - self.new_archetype(archetype); + fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { + state.new_archetype(archetype); system_meta .archetype_component_access - .extend(&self.archetype_component_access); + .extend(&state.archetype_component_access); } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, @@ -353,8 +362,6 @@ pub struct ParamSet<'w, 's, T: SystemParam> { system_meta: SystemMeta, change_tick: u32, } -/// The [`SystemParamState`] of [`ParamSet`]. -pub struct ParamSetState(T); impl_param_set!(); @@ -410,9 +417,6 @@ pub struct Res<'w, T: Resource> { change_tick: u32, } -// SAFETY: Res only reads a single World resource -unsafe impl<'w, T: Resource> ReadOnlySystemParam for Res<'w, T> {} - impl<'w, T: Resource> Debug for Res<'w, T> where T: Debug, @@ -491,23 +495,16 @@ where } } -/// The [`SystemParamState`] of [`Res`]. -#[doc(hidden)] -pub struct ResState { - component_id: ComponentId, - marker: PhantomData, -} - -impl<'a, T: Resource> SystemParam for Res<'a, T> { - type State = ResState; -} +// SAFETY: Res only reads a single World resource +unsafe impl<'a, T: Resource> ReadOnlySystemParam for Res<'a, T> {} // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResState { +unsafe impl<'a, T: Resource> SystemParam for Res<'a, T> { + type State = ComponentId; type Item<'w, 's> = Res<'w, T>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access(); assert!( @@ -526,21 +523,19 @@ unsafe impl SystemParamState for ResState { system_meta .archetype_component_access .add_read(archetype_component_id); - Self { - component_id, - marker: PhantomData, - } + + component_id } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { let (ptr, ticks) = world - .get_resource_with_ticks(state.component_id) + .get_resource_with_ticks(component_id) .unwrap_or_else(|| { panic!( "Resource requested by {} does not exist: {}", @@ -558,36 +553,27 @@ unsafe impl SystemParamState for ResState { } } -/// The [`SystemParamState`] of [`Option>`]. -/// See: [`Res`] -#[doc(hidden)] -pub struct OptionResState(ResState); - -impl<'a, T: Resource> SystemParam for Option> { - type State = OptionResState; -} - // SAFETY: Only reads a single World resource unsafe impl<'a, T: Resource> ReadOnlySystemParam for Option> {} -// SAFETY: this impl defers to `ResState`, which initializes -// and validates the correct world access -unsafe impl SystemParamState for OptionResState { +// SAFETY: this impl defers to `Res`, which initializes and validates the correct world access. +unsafe impl<'a, T: Resource> SystemParam for Option> { + type State = ComponentId; type Item<'w, 's> = Option>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self(ResState::init(world, system_meta)) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + Res::::init_state(world, system_meta) } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world - .get_resource_with_ticks(state.0.component_id) + .get_resource_with_ticks(component_id) .map(|(ptr, ticks)| Res { value: ptr.deref(), added: ticks.added.deref(), @@ -598,23 +584,13 @@ unsafe impl SystemParamState for OptionResState { } } -/// The [`SystemParamState`] of [`ResMut`]. -#[doc(hidden)] -pub struct ResMutState { - component_id: ComponentId, - marker: PhantomData, -} - -impl<'a, T: Resource> SystemParam for ResMut<'a, T> { - type State = ResMutState; -} - // SAFETY: Res ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this Res // conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for ResMutState { +unsafe impl<'a, T: Resource> SystemParam for ResMut<'a, T> { + type State = ComponentId; type Item<'w, 's> = ResMut<'w, T>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let component_id = world.initialize_resource::(); let combined_access = system_meta.component_access_set.combined_access(); if combined_access.has_write(component_id) { @@ -636,21 +612,19 @@ unsafe impl SystemParamState for ResMutState { system_meta .archetype_component_access .add_write(archetype_component_id); - Self { - component_id, - marker: PhantomData, - } + + component_id } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { let value = world - .get_resource_unchecked_mut_with_id(state.component_id) + .get_resource_unchecked_mut_with_id(component_id) .unwrap_or_else(|| { panic!( "Resource requested by {} does not exist: {}", @@ -670,33 +644,24 @@ unsafe impl SystemParamState for ResMutState { } } -/// The [`SystemParamState`] of [`Option>`]. -/// See: [`ResMut`] -#[doc(hidden)] -pub struct OptionResMutState(ResMutState); - -impl<'a, T: Resource> SystemParam for Option> { - type State = OptionResMutState; -} - -// SAFETY: this impl defers to `ResMutState`, which initializes -// and validates the correct world access -unsafe impl SystemParamState for OptionResMutState { +// SAFETY: this impl defers to `ResMut`, which initializes and validates the correct world access. +unsafe impl<'a, T: Resource> SystemParam for Option> { + type State = ComponentId; type Item<'w, 's> = Option>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self(ResMutState::init(world, system_meta)) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + ResMut::::init_state(world, system_meta) } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world - .get_resource_unchecked_mut_with_id(state.0.component_id) + .get_resource_unchecked_mut_with_id(component_id) .map(|value| ResMut { value: value.value, ticks: Ticks { @@ -709,32 +674,29 @@ unsafe impl SystemParamState for OptionResMutState { } } -impl<'w, 's> SystemParam for Commands<'w, 's> { - type State = CommandQueue; -} - // SAFETY: Commands only accesses internal state unsafe impl<'w, 's> ReadOnlySystemParam for Commands<'w, 's> {} // SAFETY: only local state is accessed -unsafe impl SystemParamState for CommandQueue { +unsafe impl SystemParam for Commands<'_, '_> { + type State = CommandQueue; type Item<'w, 's> = Commands<'w, 's>; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { Default::default() } - fn apply(&mut self, _system_meta: &SystemMeta, world: &mut World) { + fn apply(state: &mut Self::State, _system_meta: &SystemMeta, world: &mut World) { #[cfg(feature = "trace")] let _system_span = bevy_utils::tracing::info_span!("system_commands", name = _system_meta.name()) .entered(); - self.apply(world); + state.apply(world); } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -746,19 +708,12 @@ unsafe impl SystemParamState for CommandQueue { /// SAFETY: only reads world unsafe impl<'w> ReadOnlySystemParam for &'w World {} -/// The [`SystemParamState`] of [`&World`](crate::world::World). -#[doc(hidden)] -pub struct WorldState; - -impl<'w> SystemParam for &'w World { - type State = WorldState; -} - // SAFETY: `read_all` access is set and conflicts result in a panic -unsafe impl SystemParamState for WorldState { +unsafe impl SystemParam for &'_ World { + type State = (); type Item<'w, 's> = &'w World; - fn init(_world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(_world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let mut access = Access::default(); access.read_all(); if !system_meta @@ -780,12 +735,10 @@ unsafe impl SystemParamState for WorldState { panic!("&World conflicts with a previous mutable system parameter. Allowing this would break Rust's mutability rules"); } system_meta.component_access_set.add(filtered_access); - - WorldState } unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -895,30 +848,23 @@ where } } -/// The [`SystemParamState`] of [`Local`]. -#[doc(hidden)] -pub struct LocalState(pub(crate) SyncCell); - -impl<'s, T: FromWorld + Send + 'static> SystemParam for Local<'s, T> { - type State = LocalState; -} - // SAFETY: only local state is accessed -unsafe impl SystemParamState for LocalState { +unsafe impl<'a, T: FromWorld + Send + 'static> SystemParam for Local<'a, T> { + type State = SyncCell; type Item<'w, 's> = Local<'s, T>; - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self(SyncCell::new(T::from_world(world))) + fn init_state(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + SyncCell::new(T::from_world(world)) } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, _system_meta: &SystemMeta, _world: &'w World, _change_tick: u32, ) -> Self::Item<'w, 's> { - Local(state.0.get()) + Local(state.get()) } } @@ -980,39 +926,26 @@ impl<'a, T: Component> IntoIterator for &'a RemovedComponents<'a, T> { // SAFETY: Only reads World components unsafe impl<'a, T: Component> ReadOnlySystemParam for RemovedComponents<'a, T> {} -/// The [`SystemParamState`] of [`RemovedComponents`]. -#[doc(hidden)] -pub struct RemovedComponentsState { - component_id: ComponentId, - marker: PhantomData, -} - -impl<'a, T: Component> SystemParam for RemovedComponents<'a, T> { - type State = RemovedComponentsState; -} - // SAFETY: no component access. removed component entity collections can be read in parallel and are // never mutably borrowed during system execution -unsafe impl SystemParamState for RemovedComponentsState { +unsafe impl<'a, T: Component> SystemParam for RemovedComponents<'a, T> { + type State = ComponentId; type Item<'w, 's> = RemovedComponents<'w, T>; - fn init(world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self { - component_id: world.init_component::(), - marker: PhantomData, - } + fn init_state(world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + world.init_component::() } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, ) -> Self::Item<'w, 's> { RemovedComponents { world, - component_id: state.component_id, + component_id, marker: PhantomData, } } @@ -1083,23 +1016,13 @@ impl<'a, T> From> for NonSend<'a, T> { } } -/// The [`SystemParamState`] of [`NonSend`]. -#[doc(hidden)] -pub struct NonSendState { - component_id: ComponentId, - marker: PhantomData T>, -} - -impl<'a, T: 'static> SystemParam for NonSend<'a, T> { - type State = NonSendState; -} - // SAFETY: NonSendComponentId and ArchetypeComponentId access is applied to SystemMeta. If this // NonSend conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for NonSendState { +unsafe impl<'a, T: 'static> SystemParam for NonSend<'a, T> { + type State = ComponentId; type Item<'w, 's> = NonSend<'w, T>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { system_meta.set_non_send(); let component_id = world.initialize_non_send_resource::(); @@ -1120,22 +1043,20 @@ unsafe impl SystemParamState for NonSendState { system_meta .archetype_component_access .add_read(archetype_component_id); - Self { - component_id, - marker: PhantomData, - } + + component_id } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world.validate_non_send_access::(); let (ptr, ticks) = world - .get_resource_with_ticks(state.component_id) + .get_resource_with_ticks(component_id) .unwrap_or_else(|| { panic!( "Non-send resource requested by {} does not exist: {}", @@ -1153,37 +1074,25 @@ unsafe impl SystemParamState for NonSendState { } } -/// The [`SystemParamState`] of [`Option>`]. -/// See: [`NonSend`] -#[doc(hidden)] -pub struct OptionNonSendState(NonSendState); - -impl<'w, T: 'static> SystemParam for Option> { - type State = OptionNonSendState; -} - -// SAFETY: Only reads a single non-send resource -unsafe impl<'w, T: 'static> ReadOnlySystemParam for Option> {} - -// SAFETY: this impl defers to `NonSendState`, which initializes -// and validates the correct world access -unsafe impl SystemParamState for OptionNonSendState { +// SAFETY: this impl defers to `NonSend`, which initializes and validates the correct world access. +unsafe impl SystemParam for Option> { + type State = ComponentId; type Item<'w, 's> = Option>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self(NonSendState::init(world, system_meta)) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + NonSend::::init_state(world, system_meta) } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world.validate_non_send_access::(); world - .get_resource_with_ticks(state.0.component_id) + .get_resource_with_ticks(component_id) .map(|(ptr, ticks)| NonSend { value: ptr.deref(), ticks: ticks.read(), @@ -1193,23 +1102,16 @@ unsafe impl SystemParamState for OptionNonSendState { } } -/// The [`SystemParamState`] of [`NonSendMut`]. -#[doc(hidden)] -pub struct NonSendMutState { - component_id: ComponentId, - marker: PhantomData T>, -} - -impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { - type State = NonSendMutState; -} +// SAFETY: Only reads a single non-send resource +unsafe impl<'a, T: 'static> ReadOnlySystemParam for NonSendMut<'a, T> {} // SAFETY: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemMeta. If this // NonSendMut conflicts with any prior access, a panic will occur. -unsafe impl SystemParamState for NonSendMutState { +unsafe impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { + type State = ComponentId; type Item<'w, 's> = NonSendMut<'w, T>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { system_meta.set_non_send(); let component_id = world.initialize_non_send_resource::(); @@ -1233,22 +1135,20 @@ unsafe impl SystemParamState for NonSendMutState { system_meta .archetype_component_access .add_write(archetype_component_id); - Self { - component_id, - marker: PhantomData, - } + + component_id } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world.validate_non_send_access::(); let (ptr, ticks) = world - .get_resource_with_ticks(state.component_id) + .get_resource_with_ticks(component_id) .unwrap_or_else(|| { panic!( "Non-send resource requested by {} does not exist: {}", @@ -1263,34 +1163,25 @@ unsafe impl SystemParamState for NonSendMutState { } } -/// The [`SystemParamState`] of [`Option>`]. -/// See: [`NonSendMut`] -#[doc(hidden)] -pub struct OptionNonSendMutState(NonSendMutState); - -impl<'a, T: 'static> SystemParam for Option> { - type State = OptionNonSendMutState; -} - -// SAFETY: this impl defers to `NonSendMutState`, which initializes -// and validates the correct world access -unsafe impl SystemParamState for OptionNonSendMutState { +// SAFETY: this impl defers to `NonSendMut`, which initializes and validates the correct world access. +unsafe impl<'a, T: 'static> SystemParam for Option> { + type State = ComponentId; type Item<'w, 's> = Option>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self(NonSendMutState::init(world, system_meta)) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + NonSendMut::::init_state(world, system_meta) } #[inline] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + &mut component_id: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { world.validate_non_send_access::(); world - .get_resource_with_ticks(state.0.component_id) + .get_resource_with_ticks(component_id) .map(|(ptr, ticks)| NonSendMut { value: ptr.assert_unique().deref_mut(), ticks: Ticks::from_tick_cells(ticks, system_meta.last_change_tick, change_tick), @@ -1298,28 +1189,19 @@ unsafe impl SystemParamState for OptionNonSendMutState { } } -impl<'a> SystemParam for &'a Archetypes { - type State = ArchetypesState; -} - // SAFETY: Only reads World archetypes unsafe impl<'a> ReadOnlySystemParam for &'a Archetypes {} -/// The [`SystemParamState`] of [`Archetypes`]. -#[doc(hidden)] -pub struct ArchetypesState; - // SAFETY: no component value access -unsafe impl SystemParamState for ArchetypesState { +unsafe impl<'a> SystemParam for &'a Archetypes { + type State = (); type Item<'w, 's> = &'w Archetypes; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self - } + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} #[inline] unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -1328,28 +1210,19 @@ unsafe impl SystemParamState for ArchetypesState { } } -impl<'a> SystemParam for &'a Components { - type State = ComponentsState; -} - // SAFETY: Only reads World components unsafe impl<'a> ReadOnlySystemParam for &'a Components {} -/// The [`SystemParamState`] of [`Components`]. -#[doc(hidden)] -pub struct ComponentsState; - // SAFETY: no component value access -unsafe impl SystemParamState for ComponentsState { +unsafe impl<'a> SystemParam for &'a Components { + type State = (); type Item<'w, 's> = &'w Components; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self - } + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} #[inline] unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -1358,28 +1231,19 @@ unsafe impl SystemParamState for ComponentsState { } } -impl<'a> SystemParam for &'a Entities { - type State = EntitiesState; -} - // SAFETY: Only reads World entities unsafe impl<'a> ReadOnlySystemParam for &'a Entities {} -/// The [`SystemParamState`] of [`Entities`]. -#[doc(hidden)] -pub struct EntitiesState; - // SAFETY: no component value access -unsafe impl SystemParamState for EntitiesState { +unsafe impl<'a> SystemParam for &'a Entities { + type State = (); type Item<'w, 's> = &'w Entities; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self - } + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} #[inline] unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -1388,28 +1252,19 @@ unsafe impl SystemParamState for EntitiesState { } } -impl<'a> SystemParam for &'a Bundles { - type State = BundlesState; -} - // SAFETY: Only reads World bundles unsafe impl<'a> ReadOnlySystemParam for &'a Bundles {} -/// The [`SystemParamState`] of [`Bundles`]. -#[doc(hidden)] -pub struct BundlesState; - // SAFETY: no component value access -unsafe impl SystemParamState for BundlesState { +unsafe impl<'a> SystemParam for &'a Bundles { + type State = (); type Item<'w, 's> = &'w Bundles; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self - } + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} #[inline] unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, _system_meta: &SystemMeta, world: &'w World, _change_tick: u32, @@ -1450,24 +1305,15 @@ impl SystemChangeTick { // SAFETY: Only reads internal system state unsafe impl ReadOnlySystemParam for SystemChangeTick {} -impl SystemParam for SystemChangeTick { - type State = SystemChangeTickState; -} - -/// The [`SystemParamState`] of [`SystemChangeTick`]. -#[doc(hidden)] -pub struct SystemChangeTickState {} - -// SAFETY: `SystemParamTickState` doesn't require any world access -unsafe impl SystemParamState for SystemChangeTickState { +// SAFETY: `SystemChangeTick` doesn't require any world access +unsafe impl SystemParam for SystemChangeTick { + type State = (); type Item<'w, 's> = SystemChangeTick; - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - Self {} - } + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State {} unsafe fn get_param<'w, 's>( - _state: &'s mut Self, + _state: &'s mut Self::State, system_meta: &SystemMeta, _world: &'w World, change_tick: u32, @@ -1526,79 +1372,61 @@ impl<'s> std::fmt::Display for SystemName<'s> { } } -impl<'s> SystemParam for SystemName<'s> { - type State = SystemNameState; +// SAFETY: no component value access +unsafe impl SystemParam for SystemName<'_> { + type State = Cow<'static, str>; + type Item<'w, 's> = SystemName<'s>; + + fn init_state(_world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + system_meta.name.clone() + } + + #[inline] + unsafe fn get_param<'w, 's>( + name: &'s mut Self::State, + _system_meta: &SystemMeta, + _world: &'w World, + _change_tick: u32, + ) -> Self::Item<'w, 's> { + SystemName { name } + } } // SAFETY: Only reads internal system state unsafe impl<'s> ReadOnlySystemParam for SystemName<'s> {} -/// The [`SystemParamState`] of [`SystemName`]. -#[doc(hidden)] -pub struct SystemNameState { - name: Cow<'static, str>, -} - -// SAFETY: no component value access -unsafe impl SystemParamState for SystemNameState { - type Item<'w, 's> = SystemName<'s>; - - fn init(_world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self { - name: system_meta.name.clone(), - } - } - - #[inline] - unsafe fn get_param<'w, 's>( - state: &'s mut Self, - _system_meta: &SystemMeta, - _world: &'w World, - _change_tick: u32, - ) -> Self::Item<'w, 's> { - SystemName { - name: state.name.as_ref(), - } - } -} - macro_rules! impl_system_param_tuple { ($($param: ident),*) => { - impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { - type State = ($($param::State,)*); - } // SAFETY: tuple consists only of ReadOnlySystemParams unsafe impl<$($param: ReadOnlySystemParam),*> ReadOnlySystemParam for ($($param,)*) {} - - // SAFETY: implementors of each `SystemParamState` in the tuple have validated their impls + // SAFETY: implementors of each `SystemParam` in the tuple have validated their impls #[allow(clippy::undocumented_unsafe_blocks)] // false positive by clippy #[allow(non_snake_case)] - unsafe impl<$($param: SystemParamState),*> SystemParamState for ($($param,)*) { + unsafe impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { + type State = ($($param::State,)*); type Item<'w, 's> = ($($param::Item::<'w, 's>,)*); #[inline] - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self { - (($($param::init(_world, _system_meta),)*)) + fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { + (($($param::init_state(_world, _system_meta),)*)) } #[inline] - fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) { - let ($($param,)*) = self; - $($param.new_archetype(_archetype, _system_meta);)* + fn new_archetype(($($param,)*): &mut Self::State, _archetype: &Archetype, _system_meta: &mut SystemMeta) { + $($param::new_archetype($param, _archetype, _system_meta);)* } #[inline] - fn apply(&mut self, _system_meta: &SystemMeta, _world: &mut World) { - let ($($param,)*) = self; - $($param.apply(_system_meta, _world);)* + fn apply(($($param,)*): &mut Self::State, _system_meta: &SystemMeta, _world: &mut World) { + $($param::apply($param, _system_meta, _world);)* } #[inline] #[allow(clippy::unused_unit)] unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, _system_meta: &SystemMeta, _world: &'w World, _change_tick: u32, @@ -1698,48 +1526,37 @@ impl<'w, 's, P: SystemParam> StaticSystemParam<'w, 's, P> { } } -/// The [`SystemParamState`] of [`StaticSystemParam`]. -#[doc(hidden)] -pub struct StaticSystemParamState(S, PhantomData P>); - // SAFETY: This doesn't add any more reads, and the delegated fetch confirms it unsafe impl<'w, 's, P: ReadOnlySystemParam + 'static> ReadOnlySystemParam for StaticSystemParam<'w, 's, P> { } -impl<'world, 'state, P: SystemParam + 'static> SystemParam - for StaticSystemParam<'world, 'state, P> -{ - type State = StaticSystemParamState; -} - -// SAFETY: all methods are just delegated to `S`'s `SystemParamState` implementation -unsafe impl + 'static> SystemParamState - for StaticSystemParamState -{ +// SAFETY: all methods are just delegated to `P`'s `SystemParam` implementation +unsafe impl SystemParam for StaticSystemParam<'_, '_, P> { + type State = P::State; type Item<'world, 'state> = StaticSystemParam<'world, 'state, P>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { - Self(S::init(world, system_meta), PhantomData) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + P::init_state(world, system_meta) } - fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) { - self.0.new_archetype(archetype, system_meta); + fn new_archetype(state: &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { + P::new_archetype(state, archetype, system_meta); } - fn apply(&mut self, system_meta: &SystemMeta, world: &mut World) { - self.0.apply(system_meta, world); + fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) { + P::apply(state, system_meta, world); } unsafe fn get_param<'world, 'state>( - state: &'state mut Self, + state: &'state mut Self::State, system_meta: &SystemMeta, world: &'world World, change_tick: u32, ) -> Self::Item<'world, 'state> { - // SAFETY: We properly delegate SystemParamState - StaticSystemParam(S::get_param(&mut state.0, system_meta, world, change_tick)) + // SAFETY: Defer to the safety of P::SystemParam + StaticSystemParam(P::get_param(state, system_meta, world, change_tick)) } } @@ -1805,6 +1622,13 @@ mod tests { crate::system::assert_is_system(long_system); } + #[derive(SystemParam)] + struct MyParam<'w, T: Resource, Marker: 'static> { + _foo: Res<'w, T>, + #[system_param(ignore)] + marker: PhantomData, + } + #[derive(SystemParam)] pub struct UnitParam; diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs index b3d4ba665a..4038a71aa6 100644 --- a/crates/bevy_render/src/extract_param.rs +++ b/crates/bevy_render/src/extract_param.rs @@ -1,10 +1,7 @@ use crate::MainWorld; use bevy_ecs::{ prelude::*, - system::{ - ReadOnlySystemParam, ResState, SystemMeta, SystemParam, SystemParamItem, SystemParamState, - SystemState, - }, + system::{ReadOnlySystemParam, SystemMeta, SystemParam, SystemParamItem, SystemState}, }; use std::ops::{Deref, DerefMut}; @@ -49,42 +46,36 @@ where item: SystemParamItem<'w, 's, P>, } -impl<'w, 's, P> SystemParam for Extract<'w, 's, P> +#[doc(hidden)] +pub struct ExtractState { + state: SystemState

, + main_world_state: as SystemParam>::State, +} + +// SAFETY: only accesses MainWorld resource with read only system params using Res, +// which is initialized in init() +unsafe impl

SystemParam for Extract<'_, '_, P> where P: ReadOnlySystemParam, { type State = ExtractState

; -} - -#[doc(hidden)] -pub struct ExtractState { - state: SystemState

, - main_world_state: ResState, -} - -// SAFETY: only accesses MainWorld resource with read only system params using ResState, -// which is initialized in init() -unsafe impl SystemParamState for ExtractState

-where - P: ReadOnlySystemParam + 'static, -{ type Item<'w, 's> = Extract<'w, 's, P>; - fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { let mut main_world = world.resource_mut::(); - Self { + ExtractState { state: SystemState::new(&mut main_world), - main_world_state: ResState::init(world, system_meta), + main_world_state: Res::::init_state(world, system_meta), } } unsafe fn get_param<'w, 's>( - state: &'s mut Self, + state: &'s mut Self::State, system_meta: &SystemMeta, world: &'w World, change_tick: u32, ) -> Self::Item<'w, 's> { - let main_world = ResState::::get_param( + let main_world = Res::::get_param( &mut state.main_world_state, system_meta, world, @@ -95,7 +86,7 @@ where } } -impl<'w, 's, P: SystemParam> Deref for Extract<'w, 's, P> +impl<'w, 's, P> Deref for Extract<'w, 's, P> where P: ReadOnlySystemParam, { @@ -107,7 +98,7 @@ where } } -impl<'w, 's, P: SystemParam> DerefMut for Extract<'w, 's, P> +impl<'w, 's, P> DerefMut for Extract<'w, 's, P> where P: ReadOnlySystemParam, { @@ -117,7 +108,7 @@ where } } -impl<'a, 'w, 's, P: SystemParam> IntoIterator for &'a Extract<'w, 's, P> +impl<'a, 'w, 's, P> IntoIterator for &'a Extract<'w, 's, P> where P: ReadOnlySystemParam, &'a SystemParamItem<'w, 's, P>: IntoIterator,