Obviate the need for RunSystem, and remove it (#3817)

# Objective

- Fixes #3300
- `RunSystem` is messy

## Solution

- Adds the trick theorised in #3300 (comment)

P.S. I also want this for an experimental refactoring of `Assets`, to remove the duplication of `Events<AssetEvent<T>>`


Co-authored-by: Carter Anderson <mcanders1@gmail.com>

crates/bevy_ecs/macros/src/lib.rs

@@ -382,7 +382,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream {
         #[doc(hidden)]
         #fetch_struct_visibility struct #fetch_struct_name<TSystemParamState, #punctuated_generic_idents> {
             state: TSystemParamState,
-            marker: std::marker::PhantomData<(#punctuated_generic_idents)>
+            marker: std::marker::PhantomData<fn()->(#punctuated_generic_idents)>
         }
 
         unsafe impl<TSystemParamState: #path::system::SystemParamState, #punctuated_generics> #path::system::SystemParamState for #fetch_struct_name<TSystemParamState, #punctuated_generic_idents> {

crates/bevy_ecs/src/system/function_system.rs

@@ -4,7 +4,7 @@ use crate::{
     query::{Access, FilteredAccessSet},
     system::{
         check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
-        SystemParamItem, SystemParamState,
+        SystemParamState,
     },
     world::{World, WorldId},
 };
@@ -46,11 +46,6 @@ impl SystemMeta {
     pub fn set_non_send(&mut self) {
         self.is_send = false;
     }
-
-    #[inline]
-    pub(crate) fn check_change_tick(&mut self, change_tick: u32) {
-        check_system_change_tick(&mut self.last_change_tick, change_tick, self.name.as_ref());
-    }
 }
 
 // TODO: Actually use this in FunctionSystem. We should probably only do this once Systems are constructed using a World reference
@@ -194,10 +189,6 @@ impl<Param: SystemParam> SystemState<Param> {
         self.world_id == world.id()
     }
 
-    pub(crate) fn new_archetype(&mut self, archetype: &Archetype) {
-        self.param_state.new_archetype(archetype, &mut self.meta);
-    }
-
     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();
@@ -236,74 +227,6 @@ impl<Param: SystemParam> SystemState<Param> {
     }
 }
 
-/// A trait for defining systems with a [`SystemParam`] associated type.
-///
-/// This facilitates the creation of systems that are generic over some trait
-/// and that use that trait's associated types as `SystemParam`s.
-pub trait RunSystem: Send + Sync + 'static {
-    /// The `SystemParam` type passed to the system when it runs.
-    type Param: SystemParam;
-
-    /// Runs the system.
-    fn run(param: SystemParamItem<Self::Param>);
-
-    /// Creates a concrete instance of the system for the specified `World`.
-    fn system(world: &mut World) -> ParamSystem<Self::Param> {
-        ParamSystem {
-            run: Self::run,
-            state: SystemState::new(world),
-        }
-    }
-}
-
-pub struct ParamSystem<P: SystemParam> {
-    state: SystemState<P>,
-    run: fn(SystemParamItem<P>),
-}
-
-impl<P: SystemParam + 'static> System for ParamSystem<P> {
-    type In = ();
-
-    type Out = ();
-
-    fn name(&self) -> Cow<'static, str> {
-        self.state.meta().name.clone()
-    }
-
-    fn new_archetype(&mut self, archetype: &Archetype) {
-        self.state.new_archetype(archetype);
-    }
-
-    fn component_access(&self) -> &Access<ComponentId> {
-        self.state.meta().component_access_set.combined_access()
-    }
-
-    fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
-        &self.state.meta().archetype_component_access
-    }
-
-    fn is_send(&self) -> bool {
-        self.state.meta().is_send()
-    }
-
-    unsafe fn run_unsafe(&mut self, _input: Self::In, world: &World) -> Self::Out {
-        let param = self.state.get_unchecked_manual(world);
-        (self.run)(param);
-    }
-
-    fn apply_buffers(&mut self, world: &mut World) {
-        self.state.apply(world);
-    }
-
-    fn initialize(&mut self, _world: &mut World) {
-        // already initialized by nature of the SystemState being constructed
-    }
-
-    fn check_change_tick(&mut self, change_tick: u32) {
-        self.state.meta.check_change_tick(change_tick);
-    }
-}
-
 /// 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

crates/bevy_ecs/src/system/system_param.rs

@@ -1254,6 +1254,131 @@ pub mod lifetimeless {
     pub type SCommands = crate::system::Commands<'static, 'static>;
 }
 
+/// A helper for using system parameters in generic contexts
+///
+/// This type is a [`SystemParam`] adapter which always has
+/// `Self::Fetch::Item == Self` (ignoring lifetimes for brevity),
+/// no matter the argument [`SystemParam`] (`P`) (other than
+/// that `P` must be `'static`)
+///
+/// This makes it useful for having arbitrary [`SystemParam`] type arguments
+/// to function systems, or for generic types using the [`derive@SystemParam`]
+/// derive:
+///
+/// ```
+/// # use bevy_ecs::prelude::*;
+/// use bevy_ecs::system::{SystemParam, StaticSystemParam};
+/// #[derive(SystemParam)]
+/// struct GenericParam<'w,'s, T: SystemParam + 'static> {
+///     field: StaticSystemParam<'w, 's, T>,
+/// }
+/// fn do_thing_generically<T: SystemParam + 'static>(t: StaticSystemParam<T>) {}
+///
+/// fn check_always_is_system<T: SystemParam + 'static>(){
+///     bevy_ecs::system::assert_is_system(do_thing_generically::<T>);
+/// }
+/// ```
+/// Note that in a real case you'd generally want
+/// additional bounds on `P`, for your use of the parameter
+/// to have a reason to be generic.
+///
+/// For example, using this would allow a type to be generic over
+/// whether a resource is accessed mutably or not, with
+/// impls being bounded on [`P: Deref<Target=MyType>`](Deref), and
+/// [`P: DerefMut<Target=MyType>`](DerefMut) depending on whether the
+/// method requires mutable access or not.
+///
+/// The method which doesn't use this type will not compile:
+/// ```compile_fail
+/// # use bevy_ecs::prelude::*;
+/// # use bevy_ecs::system::{SystemParam, StaticSystemParam};
+///
+/// fn do_thing_generically<T: SystemParam + 'static>(t: T) {}
+///
+/// #[derive(SystemParam)]
+/// struct GenericParam<'w,'s, T: SystemParam> {
+///     field: T,
+///     #[system_param(ignore)]
+///     // Use the lifetimes, as the `SystemParam` derive requires them
+///     phantom: core::marker::PhantomData<&'w &'s ()>
+/// }
+/// # fn check_always_is_system<T: SystemParam + 'static>(){
+/// #    bevy_ecs::system::assert_is_system(do_thing_generically::<T>);
+/// # }
+/// ```
+///
+pub struct StaticSystemParam<'w, 's, P: SystemParam>(SystemParamItem<'w, 's, P>);
+
+impl<'w, 's, P: SystemParam> Deref for StaticSystemParam<'w, 's, P> {
+    type Target = SystemParamItem<'w, 's, P>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl<'w, 's, P: SystemParam> DerefMut for StaticSystemParam<'w, 's, P> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl<'w, 's, P: SystemParam> StaticSystemParam<'w, 's, P> {
+    /// Get the value of the parameter
+    pub fn into_inner(self) -> SystemParamItem<'w, 's, P> {
+        self.0
+    }
+}
+
+/// The [`SystemParamState`] of [`SystemChangeTick`].
+pub struct StaticSystemParamState<S, P>(S, PhantomData<fn() -> P>);
+
+// Safe: This doesn't add any more reads, and the delegated fetch confirms it
+unsafe impl<'w, 's, S: ReadOnlySystemParamFetch, P> ReadOnlySystemParamFetch
+    for StaticSystemParamState<S, P>
+{
+}
+
+impl<'world, 'state, P: SystemParam + 'static> SystemParam
+    for StaticSystemParam<'world, 'state, P>
+{
+    type Fetch = StaticSystemParamState<P::Fetch, P>;
+}
+
+impl<'world, 'state, S: SystemParamFetch<'world, 'state>, P: SystemParam + 'static>
+    SystemParamFetch<'world, 'state> for StaticSystemParamState<S, P>
+where
+    P: SystemParam<Fetch = S>,
+{
+    type Item = StaticSystemParam<'world, 'state, P>;
+
+    unsafe fn get_param(
+        state: &'state mut Self,
+        system_meta: &SystemMeta,
+        world: &'world World,
+        change_tick: u32,
+    ) -> Self::Item {
+        // Safe: We properly delegate SystemParamState
+        StaticSystemParam(S::get_param(&mut state.0, system_meta, world, change_tick))
+    }
+}
+
+unsafe impl<'w, 's, S: SystemParamState, P: SystemParam + 'static> SystemParamState
+    for StaticSystemParamState<S, P>
+{
+    fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self {
+        Self(S::init(world, system_meta), PhantomData)
+    }
+
+    fn new_archetype(&mut self, archetype: &Archetype, system_meta: &mut SystemMeta) {
+        self.0.new_archetype(archetype, system_meta)
+    }
+
+    fn apply(&mut self, world: &mut World) {
+        self.0.apply(world)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::SystemParam;

crates/bevy_render/src/render_asset.rs

@@ -3,7 +3,7 @@ use bevy_app::{App, Plugin};
 use bevy_asset::{Asset, AssetEvent, Assets, Handle};
 use bevy_ecs::{
     prelude::*,
-    system::{lifetimeless::*, RunSystem, SystemParam, SystemParamItem},
+    system::{StaticSystemParam, SystemParam, SystemParamItem},
 };
 use bevy_utils::{HashMap, HashSet};
 use std::marker::PhantomData;
@@ -55,13 +55,12 @@ impl<A: RenderAsset> Default for RenderAssetPlugin<A> {
 impl<A: RenderAsset> Plugin for RenderAssetPlugin<A> {
     fn build(&self, app: &mut App) {
         if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
-            let prepare_asset_system = PrepareAssetSystem::<A>::system(&mut render_app.world);
             render_app
                 .init_resource::<ExtractedAssets<A>>()
                 .init_resource::<RenderAssets<A>>()
                 .init_resource::<PrepareNextFrameAssets<A>>()
                 .add_system_to_stage(RenderStage::Extract, extract_render_asset::<A>)
-                .add_system_to_stage(RenderStage::Prepare, prepare_asset_system);
+                .add_system_to_stage(RenderStage::Prepare, prepare_assets::<A>);
         }
     }
 }
@@ -122,14 +121,6 @@ fn extract_render_asset<A: RenderAsset>(
     });
 }
 
-/// Specifies all ECS data required by [`PrepareAssetSystem`].
-pub type RenderAssetParams<R> = (
-    SResMut<ExtractedAssets<R>>,
-    SResMut<RenderAssets<R>>,
-    SResMut<PrepareNextFrameAssets<R>>,
-    <R as RenderAsset>::Param,
-);
-
 // TODO: consider storing inside system?
 /// All assets that should be prepared next frame.
 pub struct PrepareNextFrameAssets<A: RenderAsset> {
@@ -146,38 +137,36 @@ impl<A: RenderAsset> Default for PrepareNextFrameAssets<A> {
 
 /// This system prepares all assets of the corresponding [`RenderAsset`] type
 /// which where extracted this frame for the GPU.
-pub struct PrepareAssetSystem<R: RenderAsset>(PhantomData<R>);
-
-impl<R: RenderAsset> RunSystem for PrepareAssetSystem<R> {
-    type Param = RenderAssetParams<R>;
-
-    fn run(
-        (mut extracted_assets, mut render_assets, mut prepare_next_frame, mut param): SystemParamItem<Self::Param>,
-    ) {
-        let mut queued_assets = std::mem::take(&mut prepare_next_frame.assets);
-        for (handle, extracted_asset) in queued_assets.drain(..) {
-            match R::prepare_asset(extracted_asset, &mut param) {
-                Ok(prepared_asset) => {
-                    render_assets.insert(handle, prepared_asset);
-                }
-                Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => {
-                    prepare_next_frame.assets.push((handle, extracted_asset));
-                }
+fn prepare_assets<R: RenderAsset>(
+    mut extracted_assets: ResMut<ExtractedAssets<R>>,
+    mut render_assets: ResMut<RenderAssets<R>>,
+    mut prepare_next_frame: ResMut<PrepareNextFrameAssets<R>>,
+    param: StaticSystemParam<<R as RenderAsset>::Param>,
+) {
+    let mut param = param.into_inner();
+    let mut queued_assets = std::mem::take(&mut prepare_next_frame.assets);
+    for (handle, extracted_asset) in queued_assets.drain(..) {
+        match R::prepare_asset(extracted_asset, &mut param) {
+            Ok(prepared_asset) => {
+                render_assets.insert(handle, prepared_asset);
+            }
+            Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => {
+                prepare_next_frame.assets.push((handle, extracted_asset));
             }
         }
+    }
 
-        for removed in std::mem::take(&mut extracted_assets.removed) {
-            render_assets.remove(&removed);
-        }
+    for removed in std::mem::take(&mut extracted_assets.removed) {
+        render_assets.remove(&removed);
+    }
 
-        for (handle, extracted_asset) in std::mem::take(&mut extracted_assets.extracted) {
-            match R::prepare_asset(extracted_asset, &mut param) {
-                Ok(prepared_asset) => {
-                    render_assets.insert(handle, prepared_asset);
-                }
-                Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => {
-                    prepare_next_frame.assets.push((handle, extracted_asset));
-                }
+    for (handle, extracted_asset) in std::mem::take(&mut extracted_assets.extracted) {
+        match R::prepare_asset(extracted_asset, &mut param) {
+            Ok(prepared_asset) => {
+                render_assets.insert(handle, prepared_asset);
+            }
+            Err(PrepareAssetError::RetryNextUpdate(extracted_asset)) => {
+                prepare_next_frame.assets.push((handle, extracted_asset));
             }
         }
     }