diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index 1d4cb94c2a7c0..27383ff71ee55 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -221,40 +221,45 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream { type Fetch = QuerySetState<(#(QueryState<#query, #filter>,)*)>; } - // SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemState. If any QueryState conflicts + // SAFE: All Queries are constrained to ReadOnlyFetch, so World is only read + unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> ReadOnlySystemParamFetch for QuerySetState<(#(QueryState<#query, #filter>,)*)> + where #(#query::Fetch: ReadOnlyFetch,)* #(#filter::Fetch: FilterFetch,)* + { } + + // SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemMeta. If any QueryState conflicts // with any prior access, a panic will occur. unsafe impl<#(#query: WorldQuery + 'static,)* #(#filter: WorldQuery + 'static,)*> SystemParamState for QuerySetState<(#(QueryState<#query, #filter>,)*)> where #(#filter::Fetch: FilterFetch,)* { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, config: Self::Config) -> Self { + fn init(world: &mut World, system_meta: &mut SystemMeta, config: Self::Config) -> Self { #( let mut #query = QueryState::<#query, #filter>::new(world); assert_component_access_compatibility( - &system_state.name, + &system_meta.name, std::any::type_name::<#query>(), std::any::type_name::<#filter>(), - &system_state.component_access_set, + &system_meta.component_access_set, &#query.component_access, world, ); )* #( - system_state + system_meta .component_access_set .add(#query.component_access.clone()); - system_state + system_meta .archetype_component_access .extend(&#query.archetype_component_access); )* QuerySetState((#(#query,)*)) } - fn new_archetype(&mut self, archetype: &Archetype, system_state: &mut SystemState) { + fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) { let (#(#query,)*) = &mut self.0; #( #query.new_archetype(archetype); - system_state + system_meta .archetype_component_access .extend(&#query.archetype_component_access); )* @@ -271,12 +276,12 @@ pub fn impl_query_set(_input: TokenStream) -> TokenStream { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { let (#(#query,)*) = &state.0; - QuerySet((#(Query::new(world, #query, system_state.last_change_tick, change_tick),)*)) + QuerySet((#(Query::new(world, #query, system_meta.last_change_tick, change_tick),)*)) } } @@ -388,15 +393,15 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { unsafe impl #path::system::SystemParamState for #fetch_struct_name { type Config = TSystemParamState::Config; - fn init(world: &mut #path::world::World, system_state: &mut #path::system::SystemState, config: Self::Config) -> Self { + fn init(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta, config: Self::Config) -> Self { Self { - state: TSystemParamState::init(world, system_state, config), + state: TSystemParamState::init(world, system_meta, config), marker: std::marker::PhantomData, } } - fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_state: &mut #path::system::SystemState) { - self.state.new_archetype(archetype, system_state) + fn new_archetype(&mut self, archetype: &#path::archetype::Archetype, system_meta: &mut #path::system::SystemMeta) { + self.state.new_archetype(archetype, system_meta) } fn default_config() -> TSystemParamState::Config { @@ -412,12 +417,12 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { type Item = #struct_name#ty_generics; unsafe fn get_param( state: &'a mut Self, - system_state: &'a #path::system::SystemState, + system_meta: &#path::system::SystemMeta, world: &'a #path::world::World, change_tick: u32, ) -> Self::Item { #struct_name { - #(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_state, world, change_tick),)* + #(#fields: <<#field_types as SystemParam>::Fetch as #path::system::SystemParamFetch>::get_param(&mut state.state.#field_indices, system_meta, world, change_tick),)* #(#ignored_fields: <#ignored_field_types>::default(),)* } } diff --git a/crates/bevy_ecs/src/system/into_system.rs b/crates/bevy_ecs/src/system/function_system.rs similarity index 55% rename from crates/bevy_ecs/src/system/into_system.rs rename to crates/bevy_ecs/src/system/function_system.rs index 97f3e0f62f0f2..414ba2c3fec26 100644 --- a/crates/bevy_ecs/src/system/into_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -1,28 +1,29 @@ use crate::{ - archetype::{Archetype, ArchetypeComponentId}, + archetype::{Archetype, ArchetypeComponentId, ArchetypeGeneration, ArchetypeId}, component::ComponentId, query::{Access, FilteredAccessSet}, system::{ - check_system_change_tick, System, SystemId, SystemParam, SystemParamFetch, SystemParamState, + check_system_change_tick, ReadOnlySystemParamFetch, System, SystemId, SystemParam, + SystemParamFetch, SystemParamState, }, - world::World, + world::{World, WorldId}, }; use bevy_ecs_macros::all_tuples; use std::{borrow::Cow, marker::PhantomData}; -/// The state of a [`System`]. -pub struct SystemState { +/// The metadata of a [`System`]. +pub struct SystemMeta { pub(crate) id: SystemId, pub(crate) name: Cow<'static, str>, pub(crate) component_access_set: FilteredAccessSet, pub(crate) archetype_component_access: Access, - // NOTE: this must be kept private. making a SystemState non-send is irreversible to prevent + // NOTE: this must be kept private. making a SystemMeta non-send is irreversible to prevent // SystemParams from overriding each other is_send: bool, pub(crate) last_change_tick: u32, } -impl SystemState { +impl SystemMeta { fn new() -> Self { Self { name: std::any::type_name::().into(), @@ -49,6 +50,114 @@ impl SystemState { } } +// TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference +// (to avoid the need for unwrapping to retrieve SystemMeta) +/// Holds on to persistent state required to drive [`SystemParam`] for a [`System`]. +pub struct SystemState { + meta: SystemMeta, + param_state: ::Fetch, + world_id: WorldId, + archetype_generation: ArchetypeGeneration, +} + +impl SystemState { + pub fn new(world: &mut World) -> Self { + let config = ::default_config(); + Self::with_config(world, config) + } + + pub fn with_config( + world: &mut World, + config: ::Config, + ) -> Self { + let mut meta = SystemMeta::new::(); + let param_state = ::init(world, &mut meta, config); + Self { + meta, + param_state, + world_id: world.id(), + archetype_generation: ArchetypeGeneration::initial(), + } + } + + #[inline] + pub fn meta(&self) -> &SystemMeta { + &self.meta + } + + /// Retrieve the [`SystemParam`] values. This can only be called when all parameters are read-only. + #[inline] + pub fn get<'a>(&'a mut self, world: &'a World) -> >::Item + where + Param::Fetch: ReadOnlySystemParamFetch, + { + self.validate_world_and_update_archetypes(world); + // SAFE: Param is read-only and doesn't allow mutable access to World. It also matches the World this SystemState was created with. + unsafe { self.get_unchecked_manual(world) } + } + + /// Retrieve the mutable [`SystemParam`] values. + #[inline] + pub fn get_mut<'a>( + &'a mut self, + world: &'a mut World, + ) -> >::Item { + self.validate_world_and_update_archetypes(world); + // SAFE: World is uniquely borrowed and matches the World this SystemState was created with. + unsafe { self.get_unchecked_manual(world) } + } + + /// Applies all state queued up for [`SystemParam`] values. For example, this will apply commands queued up + /// by a [`Commands`](`super::Commands`) parameter to the given [`World`]. + /// 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(world); + } + + #[inline] + pub fn matches_world(&self, world: &World) -> bool { + self.world_id == world.id() + } + + fn validate_world_and_update_archetypes(&mut self, world: &World) { + assert!(self.matches_world(world), "Encountered a mismatched World. A SystemState cannot be used with Worlds other than the one it was created with."); + let archetypes = world.archetypes(); + let new_generation = archetypes.generation(); + let old_generation = std::mem::replace(&mut self.archetype_generation, new_generation); + let archetype_index_range = old_generation.value()..new_generation.value(); + + for archetype_index in archetype_index_range { + self.param_state.new_archetype( + &archetypes[ArchetypeId::new(archetype_index)], + &mut self.meta, + ); + } + } + + /// Retrieve the [`SystemParam`] values. This will not update archetypes automatically. + /// + /// # Safety + /// This call might access any of the input parameters in a way that violates Rust's mutability rules. Make sure the data + /// access is safe in the context of global [`World`] access. The passed-in [`World`] _must_ be the [`World`] the [`SystemState`] was + /// created with. + #[inline] + pub unsafe fn get_unchecked_manual<'a>( + &'a mut self, + world: &'a World, + ) -> >::Item { + let change_tick = world.increment_change_tick(); + let param = ::get_param( + &mut self.param_state, + &self.meta, + world, + change_tick, + ); + self.meta.last_change_tick = change_tick; + param + } +} + /// Conversion trait to turn something into a [`System`]. /// /// Use this to get a system from a function. Also note that every system implements this trait as @@ -116,7 +225,7 @@ where { func: F, param_state: Option, - system_state: SystemState, + system_meta: SystemMeta, config: Option<::Config>, // NOTE: PhantomData T> gives this safe Send/Sync impls #[allow(clippy::type_complexity)] @@ -161,7 +270,7 @@ where func: self, param_state: None, config: Some(::default_config()), - system_state: SystemState::new::(), + system_meta: SystemMeta::new::(), marker: PhantomData, } } @@ -180,33 +289,33 @@ where #[inline] fn name(&self) -> Cow<'static, str> { - self.system_state.name.clone() + self.system_meta.name.clone() } #[inline] fn id(&self) -> SystemId { - self.system_state.id + self.system_meta.id } #[inline] fn new_archetype(&mut self, archetype: &Archetype) { let param_state = self.param_state.as_mut().unwrap(); - param_state.new_archetype(archetype, &mut self.system_state); + param_state.new_archetype(archetype, &mut self.system_meta); } #[inline] fn component_access(&self) -> &Access { - &self.system_state.component_access_set.combined_access() + &self.system_meta.component_access_set.combined_access() } #[inline] fn archetype_component_access(&self) -> &Access { - &self.system_state.archetype_component_access + &self.system_meta.archetype_component_access } #[inline] fn is_send(&self) -> bool { - self.system_state.is_send + self.system_meta.is_send } #[inline] @@ -215,11 +324,11 @@ where let out = self.func.run( input, self.param_state.as_mut().unwrap(), - &self.system_state, + &self.system_meta, world, change_tick, ); - self.system_state.last_change_tick = change_tick; + self.system_meta.last_change_tick = change_tick; out } @@ -233,7 +342,7 @@ where fn initialize(&mut self, world: &mut World) { self.param_state = Some(::init( world, - &mut self.system_state, + &mut self.system_meta, self.config.take().unwrap(), )); } @@ -241,20 +350,24 @@ where #[inline] fn check_change_tick(&mut self, change_tick: u32) { check_system_change_tick( - &mut self.system_state.last_change_tick, + &mut self.system_meta.last_change_tick, change_tick, - self.system_state.name.as_ref(), + self.system_meta.name.as_ref(), ); } } /// A trait implemented for all functions that can be used as [`System`]s. pub trait SystemParamFunction: Send + Sync + 'static { - fn run( + /// # 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. + unsafe fn run( &mut self, input: In, state: &mut Param::Fetch, - system_state: &SystemState, + system_meta: &SystemMeta, world: &World, change_tick: u32, ) -> Out; @@ -270,11 +383,9 @@ macro_rules! impl_system_function { FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static { #[inline] - fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out { - unsafe { - let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick); - self($($param),*) - } + unsafe fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out { + let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick); + self($($param),*) } } @@ -286,11 +397,9 @@ macro_rules! impl_system_function { FnMut(In, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out + Send + Sync + 'static, Out: 'static { #[inline] - fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_state: &SystemState, world: &World, change_tick: u32) -> Out { - unsafe { - let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_state, world, change_tick); - self(In(input), $($param),*) - } + unsafe fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out { + let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick); + self(In(input), $($param),*) } } }; diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index b10ec03c6506e..06fdf53ce6eaa 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -1,6 +1,6 @@ mod commands; mod exclusive_system; -mod into_system; +mod function_system; mod query; #[allow(clippy::module_inception)] mod system; @@ -9,7 +9,7 @@ mod system_param; pub use commands::*; pub use exclusive_system::*; -pub use into_system::*; +pub use function_system::*; pub use query::*; pub use system::*; pub use system_chaining::*; @@ -28,7 +28,7 @@ mod tests { schedule::{Schedule, Stage, SystemStage}, system::{ IntoExclusiveSystem, IntoSystem, Local, Query, QuerySet, RemovedComponents, Res, - ResMut, System, + ResMut, System, SystemState, }, world::{FromWorld, World}, }; @@ -485,4 +485,121 @@ mod tests { x.initialize(&mut world); y.initialize(&mut world); } + + #[test] + fn read_system_state() { + #[derive(Eq, PartialEq, Debug)] + struct A(usize); + + #[derive(Eq, PartialEq, Debug)] + struct B(usize); + + let mut world = World::default(); + world.insert_resource(A(42)); + world.spawn().insert(B(7)); + + let mut system_state: SystemState<(Res, Query<&B>, QuerySet<(Query<&C>, Query<&D>)>)> = + SystemState::new(&mut world); + let (a, query, _) = system_state.get(&world); + assert_eq!(*a, A(42), "returned resource matches initial value"); + assert_eq!( + *query.single().unwrap(), + B(7), + "returned component matches initial value" + ); + } + + #[test] + fn write_system_state() { + #[derive(Eq, PartialEq, Debug)] + struct A(usize); + + #[derive(Eq, PartialEq, Debug)] + struct B(usize); + + let mut world = World::default(); + world.insert_resource(A(42)); + world.spawn().insert(B(7)); + + let mut system_state: SystemState<(ResMut, Query<&mut B>)> = + SystemState::new(&mut world); + + // The following line shouldn't compile because the parameters used are not ReadOnlySystemParam + // let (a, query) = system_state.get(&world); + + let (a, mut query) = system_state.get_mut(&mut world); + assert_eq!(*a, A(42), "returned resource matches initial value"); + assert_eq!( + *query.single_mut().unwrap(), + B(7), + "returned component matches initial value" + ); + } + + #[test] + fn system_state_change_detection() { + #[derive(Eq, PartialEq, Debug)] + struct A(usize); + + let mut world = World::default(); + let entity = world.spawn().insert(A(1)).id(); + + let mut system_state: SystemState>> = SystemState::new(&mut world); + { + let query = system_state.get(&world); + assert_eq!(*query.single().unwrap(), A(1)); + } + + { + let query = system_state.get(&world); + assert!(query.single().is_err()); + } + + world.entity_mut(entity).get_mut::().unwrap().0 = 2; + { + let query = system_state.get(&world); + assert_eq!(*query.single().unwrap(), A(2)); + } + } + + #[test] + #[should_panic] + fn system_state_invalid_world() { + let mut world = World::default(); + let mut system_state = SystemState::>::new(&mut world); + let mismatched_world = World::default(); + system_state.get(&mismatched_world); + } + + #[test] + fn system_state_archetype_update() { + #[derive(Eq, PartialEq, Debug)] + struct A(usize); + + #[derive(Eq, PartialEq, Debug)] + struct B(usize); + + let mut world = World::default(); + world.spawn().insert(A(1)); + + let mut system_state = SystemState::>::new(&mut world); + { + let query = system_state.get(&world); + assert_eq!( + query.iter().collect::>(), + vec![&A(1)], + "exactly one component returned" + ); + } + + world.spawn().insert_bundle((A(2), B(2))); + { + let query = system_state.get(&world); + assert_eq!( + query.iter().collect::>(), + vec![&A(1), &A(2)], + "components from both archetypes returned" + ); + } + } } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 07a8499971396..95dd4cee1bc83 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -5,8 +5,10 @@ use crate::{ change_detection::Ticks, component::{Component, ComponentId, ComponentTicks, Components}, entity::{Entities, Entity}, - query::{FilterFetch, FilteredAccess, FilteredAccessSet, QueryState, WorldQuery}, - system::{CommandQueue, Commands, Query, SystemState}, + query::{ + FilterFetch, FilteredAccess, FilteredAccessSet, QueryState, ReadOnlyFetch, WorldQuery, + }, + system::{CommandQueue, Commands, Query, SystemMeta}, world::{FromWorld, World}, }; pub use bevy_ecs_macros::SystemParam; @@ -46,7 +48,7 @@ pub trait SystemParam: Sized { /// /// # Safety /// -/// It is the implementor's responsibility to ensure `system_state` is populated with the _exact_ +/// It is the implementor's responsibility to ensure `system_meta` is populated with the _exact_ /// [`World`] access used by the `SystemParamState` (and associated [`SystemParamFetch`]). /// Additionally, it is the implementor's responsibility to ensure there is no /// conflicting access across all SystemParams. @@ -64,14 +66,20 @@ pub unsafe trait SystemParamState: Send + Sync + 'static { /// See [`FunctionSystem::config`](super::FunctionSystem::config) /// for more information and examples. type Config: Send + Sync; - fn init(world: &mut World, system_state: &mut SystemState, config: Self::Config) -> Self; + fn init(world: &mut World, system_meta: &mut SystemMeta, config: Self::Config) -> Self; #[inline] - fn new_archetype(&mut self, _archetype: &Archetype, _system_state: &mut SystemState) {} + fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) {} #[inline] fn apply(&mut self, _world: &mut World) {} fn default_config() -> Self::Config; } +/// A [`SystemParamFetch`] that only reads a given [`World`]. +/// +/// # Safety +/// This must only be implemented for [`SystemParamFetch`] impls that exclusively read the World passed in to [`SystemParamFetch::get_param`] +pub unsafe trait ReadOnlySystemParamFetch {} + pub trait SystemParamFetch<'a>: SystemParamState { type Item; /// # Safety @@ -80,14 +88,12 @@ pub trait SystemParamFetch<'a>: SystemParamState { /// access is safe in the context of the system scheduler. unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item; } -pub struct QueryFetch(PhantomData<(Q, F)>); - impl<'a, Q: WorldQuery + 'static, F: WorldQuery + 'static> SystemParam for Query<'a, Q, F> where F::Fetch: FilterFetch, @@ -95,7 +101,15 @@ where type Fetch = QueryState; } -// SAFE: Relevant query ComponentId and ArchetypeComponentId access is applied to SystemState. If +// SAFE: QueryState is constrained to read-only fetches, so it only reads World. +unsafe impl ReadOnlySystemParamFetch for QueryState +where + Q::Fetch: ReadOnlyFetch, + F::Fetch: FilterFetch, +{ +} + +// SAFE: 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 where @@ -103,28 +117,28 @@ where { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { let state = QueryState::new(world); assert_component_access_compatibility( - &system_state.name, + &system_meta.name, std::any::type_name::(), std::any::type_name::(), - &system_state.component_access_set, + &system_meta.component_access_set, &state.component_access, world, ); - system_state + system_meta .component_access_set .add(state.component_access.clone()); - system_state + system_meta .archetype_component_access .extend(&state.archetype_component_access); state } - fn new_archetype(&mut self, archetype: &Archetype, system_state: &mut SystemState) { + fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) { self.new_archetype(archetype); - system_state + system_meta .archetype_component_access .extend(&self.archetype_component_access); } @@ -141,11 +155,11 @@ where #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { - Query::new(world, state, system_state.last_change_tick, change_tick) + Query::new(world, state, system_meta.last_change_tick, change_tick) } } @@ -189,6 +203,9 @@ pub struct Res<'w, T: Component> { change_tick: u32, } +// SAFE: Res only reads a single World resource +unsafe impl ReadOnlySystemParamFetch for ResState {} + impl<'w, T: Component> Debug for Res<'w, T> where T: Debug, @@ -238,18 +255,18 @@ impl<'a, T: Component> SystemParam for Res<'a, T> { type Fetch = ResState; } -// SAFE: Res ComponentId and ArchetypeComponentId access is applied to SystemState. If this Res +// SAFE: 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 { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { let component_id = world.initialize_resource::(); - let combined_access = system_state.component_access_set.combined_access_mut(); + let combined_access = system_meta.component_access_set.combined_access_mut(); if combined_access.has_write(component_id) { panic!( "Res<{}> in system {} conflicts with a previous ResMut<{0}> access. Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } combined_access.add_read(component_id); @@ -257,7 +274,7 @@ unsafe impl SystemParamState for ResState { let archetype_component_id = resource_archetype .get_archetype_component_id(component_id) .unwrap(); - system_state + system_meta .archetype_component_access .add_read(archetype_component_id); Self { @@ -275,7 +292,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -284,14 +301,14 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResState { .unwrap_or_else(|| { panic!( "Resource requested by {} does not exist: {}", - system_state.name, + system_meta.name, std::any::type_name::() ) }); Res { value: &*column.get_data_ptr().cast::().as_ptr(), ticks: column.get_ticks_unchecked(0), - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, } } @@ -304,11 +321,14 @@ impl<'a, T: Component> SystemParam for Option> { type Fetch = OptionResState; } +// SAFE: Only reads a single World resource +unsafe impl ReadOnlySystemParamFetch for OptionResState {} + unsafe impl SystemParamState for OptionResState { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { - Self(ResState::init(world, system_state, ())) + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { + Self(ResState::init(world, system_meta, ())) } fn default_config() {} @@ -320,7 +340,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -329,7 +349,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResState { .map(|column| Res { value: &*column.get_data_ptr().cast::().as_ptr(), ticks: column.get_ticks_unchecked(0), - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, }) } @@ -345,22 +365,22 @@ impl<'a, T: Component> SystemParam for ResMut<'a, T> { type Fetch = ResMutState; } -// SAFE: Res ComponentId and ArchetypeComponentId access is applied to SystemState. If this Res +// SAFE: 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 { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { let component_id = world.initialize_resource::(); - let combined_access = system_state.component_access_set.combined_access_mut(); + let combined_access = system_meta.component_access_set.combined_access_mut(); if combined_access.has_write(component_id) { panic!( "ResMut<{}> in system {} conflicts with a previous ResMut<{0}> access. Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } else if combined_access.has_read(component_id) { panic!( "ResMut<{}> in system {} conflicts with a previous Res<{0}> access. Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } combined_access.add_write(component_id); @@ -368,7 +388,7 @@ unsafe impl SystemParamState for ResMutState { let archetype_component_id = resource_archetype .get_archetype_component_id(component_id) .unwrap(); - system_state + system_meta .archetype_component_access .add_write(archetype_component_id); Self { @@ -386,7 +406,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -395,7 +415,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResMutState { .unwrap_or_else(|| { panic!( "Resource requested by {} does not exist: {}", - system_state.name, + system_meta.name, std::any::type_name::() ) }); @@ -403,7 +423,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for ResMutState { value: value.value, ticks: Ticks { component_ticks: value.ticks.component_ticks, - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, }, } @@ -420,8 +440,8 @@ impl<'a, T: Component> SystemParam for Option> { unsafe impl SystemParamState for OptionResMutState { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { - Self(ResMutState::init(world, system_state, ())) + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { + Self(ResMutState::init(world, system_meta, ())) } fn default_config() {} @@ -433,7 +453,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -443,7 +463,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for OptionResMutState { value: value.value, ticks: Ticks { component_ticks: value.ticks.component_ticks, - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, }, }) @@ -454,11 +474,14 @@ impl<'a> SystemParam for Commands<'a> { type Fetch = CommandQueue; } +// SAFE: Commands only accesses internal state +unsafe impl ReadOnlySystemParamFetch for CommandQueue {} + // SAFE: only local state is accessed unsafe impl SystemParamState for CommandQueue { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Default::default() } @@ -475,7 +498,7 @@ impl<'a> SystemParamFetch<'a> for CommandQueue { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -511,6 +534,9 @@ impl<'a> SystemParamFetch<'a> for CommandQueue { /// ``` pub struct Local<'a, T: Component>(&'a mut T); +// SAFE: Local only accesses internal state +unsafe impl ReadOnlySystemParamFetch for LocalState {} + impl<'a, T: Component> Debug for Local<'a, T> where T: Debug, @@ -547,7 +573,7 @@ impl<'a, T: Component + FromWorld> SystemParam for Local<'a, T> { unsafe impl SystemParamState for LocalState { type Config = Option; - fn init(world: &mut World, _system_state: &mut SystemState, config: Self::Config) -> Self { + fn init(world: &mut World, _system_meta: &mut SystemMeta, config: Self::Config) -> Self { Self(config.unwrap_or_else(|| T::from_world(world))) } @@ -562,7 +588,7 @@ impl<'a, T: Component + FromWorld> SystemParamFetch<'a> for LocalState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, _world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -601,6 +627,9 @@ impl<'a, T> RemovedComponents<'a, T> { } } +// SAFE: Only reads World components +unsafe impl ReadOnlySystemParamFetch for RemovedComponentsState {} + /// The [`SystemParamState`] of [`RemovedComponents`]. pub struct RemovedComponentsState { component_id: ComponentId, @@ -616,7 +645,7 @@ impl<'a, T: Component> SystemParam for RemovedComponents<'a, T> { unsafe impl SystemParamState for RemovedComponentsState { type Config = (); - fn init(world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self { component_id: world.components.get_or_insert_id::(), marker: PhantomData, @@ -632,7 +661,7 @@ impl<'a, T: Component> SystemParamFetch<'a> for RemovedComponentsState { #[inline] unsafe fn get_param( state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -661,6 +690,9 @@ pub struct NonSend<'w, T> { change_tick: u32, } +// SAFE: Only reads a single World non-send resource +unsafe impl ReadOnlySystemParamFetch for NonSendState {} + impl<'w, T> Debug for NonSend<'w, T> where T: Debug, @@ -703,20 +735,20 @@ impl<'a, T: 'static> SystemParam for NonSend<'a, T> { type Fetch = NonSendState; } -// SAFE: NonSendComponentId and ArchetypeComponentId access is applied to SystemState. If this +// SAFE: 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 { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { - system_state.set_non_send(); + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { + system_meta.set_non_send(); let component_id = world.initialize_non_send_resource::(); - let combined_access = system_state.component_access_set.combined_access_mut(); + let combined_access = system_meta.component_access_set.combined_access_mut(); if combined_access.has_write(component_id) { panic!( "NonSend<{}> in system {} conflicts with a previous mutable resource access ({0}). Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } combined_access.add_read(component_id); @@ -724,7 +756,7 @@ unsafe impl SystemParamState for NonSendState { let archetype_component_id = resource_archetype .get_archetype_component_id(component_id) .unwrap(); - system_state + system_meta .archetype_component_access .add_read(archetype_component_id); Self { @@ -742,7 +774,7 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -752,7 +784,7 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendState { .unwrap_or_else(|| { panic!( "Non-send resource requested by {} does not exist: {}", - system_state.name, + system_meta.name, std::any::type_name::() ) }); @@ -760,7 +792,7 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendState { NonSend { value: &*column.get_data_ptr().cast::().as_ptr(), ticks: column.get_ticks_unchecked(0).clone(), - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, } } @@ -831,24 +863,24 @@ impl<'a, T: 'static> SystemParam for NonSendMut<'a, T> { type Fetch = NonSendMutState; } -// SAFE: NonSendMut ComponentId and ArchetypeComponentId access is applied to SystemState. If this +// SAFE: 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 { type Config = (); - fn init(world: &mut World, system_state: &mut SystemState, _config: Self::Config) -> Self { - system_state.set_non_send(); + fn init(world: &mut World, system_meta: &mut SystemMeta, _config: Self::Config) -> Self { + system_meta.set_non_send(); let component_id = world.components.get_or_insert_non_send_resource_id::(); - let combined_access = system_state.component_access_set.combined_access_mut(); + let combined_access = system_meta.component_access_set.combined_access_mut(); if combined_access.has_write(component_id) { panic!( "NonSendMut<{}> in system {} conflicts with a previous mutable resource access ({0}). Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } else if combined_access.has_read(component_id) { panic!( "NonSendMut<{}> in system {} conflicts with a previous immutable resource access ({0}). Allowing this would break Rust's mutability rules. Consider removing the duplicate access.", - std::any::type_name::(), system_state.name); + std::any::type_name::(), system_meta.name); } combined_access.add_write(component_id); @@ -856,7 +888,7 @@ unsafe impl SystemParamState for NonSendMutState { let archetype_component_id = resource_archetype .get_archetype_component_id(component_id) .unwrap(); - system_state + system_meta .archetype_component_access .add_write(archetype_component_id); Self { @@ -874,7 +906,7 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendMutState { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { @@ -884,14 +916,14 @@ impl<'a, T: 'static> SystemParamFetch<'a> for NonSendMutState { .unwrap_or_else(|| { panic!( "Non-send resource requested by {} does not exist: {}", - system_state.name, + system_meta.name, std::any::type_name::() ) }); NonSendMut { value: &mut *column.get_data_ptr().cast::().as_ptr(), ticks: &mut *column.get_ticks_mut_ptr_unchecked(0), - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, } } @@ -901,6 +933,9 @@ impl<'a> SystemParam for &'a Archetypes { type Fetch = ArchetypesState; } +// SAFE: Only reads World archetypes +unsafe impl ReadOnlySystemParamFetch for ArchetypesState {} + /// The [`SystemParamState`] of [`Archetypes`]. pub struct ArchetypesState; @@ -908,7 +943,7 @@ pub struct ArchetypesState; unsafe impl SystemParamState for ArchetypesState { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self } @@ -921,7 +956,7 @@ impl<'a> SystemParamFetch<'a> for ArchetypesState { #[inline] unsafe fn get_param( _state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -933,6 +968,9 @@ impl<'a> SystemParam for &'a Components { type Fetch = ComponentsState; } +// SAFE: Only reads World components +unsafe impl ReadOnlySystemParamFetch for ComponentsState {} + /// The [`SystemParamState`] of [`Components`]. pub struct ComponentsState; @@ -940,7 +978,7 @@ pub struct ComponentsState; unsafe impl SystemParamState for ComponentsState { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self } @@ -953,7 +991,7 @@ impl<'a> SystemParamFetch<'a> for ComponentsState { #[inline] unsafe fn get_param( _state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -965,6 +1003,9 @@ impl<'a> SystemParam for &'a Entities { type Fetch = EntitiesState; } +// SAFE: Only reads World entities +unsafe impl ReadOnlySystemParamFetch for EntitiesState {} + /// The [`SystemParamState`] of [`Entities`]. pub struct EntitiesState; @@ -972,7 +1013,7 @@ pub struct EntitiesState; unsafe impl SystemParamState for EntitiesState { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self } @@ -985,7 +1026,7 @@ impl<'a> SystemParamFetch<'a> for EntitiesState { #[inline] unsafe fn get_param( _state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -997,6 +1038,9 @@ impl<'a> SystemParam for &'a Bundles { type Fetch = BundlesState; } +// SAFE: Only reads World bundles +unsafe impl ReadOnlySystemParamFetch for BundlesState {} + /// The [`SystemParamState`] of [`Bundles`]. pub struct BundlesState; @@ -1004,7 +1048,7 @@ pub struct BundlesState; unsafe impl SystemParamState for BundlesState { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self } @@ -1017,7 +1061,7 @@ impl<'a> SystemParamFetch<'a> for BundlesState { #[inline] unsafe fn get_param( _state: &'a mut Self, - _system_state: &'a SystemState, + _system_meta: &SystemMeta, world: &'a World, _change_tick: u32, ) -> Self::Item { @@ -1031,6 +1075,9 @@ pub struct SystemChangeTick { pub change_tick: u32, } +// SAFE: Only reads internal system state +unsafe impl ReadOnlySystemParamFetch for SystemChangeTickState {} + impl SystemParam for SystemChangeTick { type Fetch = SystemChangeTickState; } @@ -1041,7 +1088,7 @@ pub struct SystemChangeTickState {} unsafe impl SystemParamState for SystemChangeTickState { type Config = (); - fn init(_world: &mut World, _system_state: &mut SystemState, _config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, _config: Self::Config) -> Self { Self {} } @@ -1053,12 +1100,12 @@ impl<'a> SystemParamFetch<'a> for SystemChangeTickState { unsafe fn get_param( _state: &mut Self, - system_state: &SystemState, + system_meta: &SystemMeta, _world: &World, change_tick: u32, ) -> Self::Item { SystemChangeTick { - last_change_tick: system_state.last_change_tick, + last_change_tick: system_meta.last_change_tick, change_tick, } } @@ -1069,6 +1116,10 @@ macro_rules! impl_system_param_tuple { impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { type Fetch = ($($param::Fetch,)*); } + + // SAFE: tuple consists only of ReadOnlySystemParamFetches + unsafe impl<$($param: ReadOnlySystemParamFetch),*> ReadOnlySystemParamFetch for ($($param,)*) {} + #[allow(unused_variables)] #[allow(non_snake_case)] impl<'a, $($param: SystemParamFetch<'a>),*> SystemParamFetch<'a> for ($($param,)*) { @@ -1077,13 +1128,13 @@ macro_rules! impl_system_param_tuple { #[inline] unsafe fn get_param( state: &'a mut Self, - system_state: &'a SystemState, + system_meta: &SystemMeta, world: &'a World, change_tick: u32, ) -> Self::Item { let ($($param,)*) = state; - ($($param::get_param($param, system_state, world, change_tick),)*) + ($($param::get_param($param, system_meta, world, change_tick),)*) } } @@ -1092,15 +1143,15 @@ macro_rules! impl_system_param_tuple { unsafe impl<$($param: SystemParamState),*> SystemParamState for ($($param,)*) { type Config = ($(<$param as SystemParamState>::Config,)*); #[inline] - fn init(_world: &mut World, _system_state: &mut SystemState, config: Self::Config) -> Self { + fn init(_world: &mut World, _system_meta: &mut SystemMeta, config: Self::Config) -> Self { let ($($param,)*) = config; - (($($param::init(_world, _system_state, $param),)*)) + (($($param::init(_world, _system_meta, $param),)*)) } #[inline] - fn new_archetype(&mut self, _archetype: &Archetype, _system_state: &mut SystemState) { + fn new_archetype(&mut self, _archetype: &Archetype, _system_meta: &mut SystemMeta) { let ($($param,)*) = self; - $($param.new_archetype(_archetype, _system_state);)* + $($param.new_archetype(_archetype, _system_meta);)* } #[inline]