[Merged by Bors] - Add new SystemState and rename old SystemState to SystemMeta

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<TSystemParamState: #path::system::SystemParamState, #punctuated_generics> #path::system::SystemParamState for #fetch_struct_name<TSystemParamState, #punctuated_generic_idents> {
             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(),)*
                 }
             }

crates/bevy_ecs/src/system/into_system.rs → 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<ComponentId>,
     pub(crate) archetype_component_access: Access<ArchetypeComponentId>,
-    // 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<T>() -> Self {
         Self {
             name: std::any::type_name::<T>().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<Param: SystemParam> {
+    meta: SystemMeta,
+    param_state: <Param as SystemParam>::Fetch,
+    world_id: WorldId,
+    archetype_generation: ArchetypeGeneration,
+}
+
+impl<Param: SystemParam> SystemState<Param> {
+    pub fn new(world: &mut World) -> Self {
+        let config = <Param::Fetch as SystemParamState>::default_config();
+        Self::with_config(world, config)
+    }
+
+    pub fn with_config(
+        world: &mut World,
+        config: <Param::Fetch as SystemParamState>::Config,
+    ) -> Self {
+        let mut meta = SystemMeta::new::<Param>();
+        let param_state = <Param::Fetch as SystemParamState>::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) -> <Param::Fetch as SystemParamFetch<'a>>::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,
+    ) -> <Param::Fetch as SystemParamFetch<'a>>::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,
+    ) -> <Param::Fetch as SystemParamFetch<'a>>::Item {
+        let change_tick = world.increment_change_tick();
+        let param = <Param::Fetch as SystemParamFetch>::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<Param::Fetch>,
-    system_state: SystemState,
+    system_meta: SystemMeta,
     config: Option<<Param::Fetch as SystemParamState>::Config>,
     // NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
     #[allow(clippy::type_complexity)]
@@ -161,7 +270,7 @@ where
             func: self,
             param_state: None,
             config: Some(<Param::Fetch as SystemParamState>::default_config()),
-            system_state: SystemState::new::<F>(),
+            system_meta: SystemMeta::new::<F>(),
             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<ComponentId> {
-        &self.system_state.component_access_set.combined_access()
+        &self.system_meta.component_access_set.combined_access()
     }
 
     #[inline]
     fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
-        &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,28 +342,32 @@ where
     fn initialize(&mut self, world: &mut World) {
         self.param_state = Some(<Param::Fetch as SystemParamState>::init(
             world,
-            &mut self.system_state,
+            &mut self.system_meta,
             self.config.take().unwrap(),
         ));
     }
 
     #[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<In, Out, Param: SystemParam, Marker>: 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<Input>, $(<<$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),*)
             }
         }
     };