forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

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

Browse Source 
# Objective

- Fixes #3300
- `RunSystem` is messy

## Solution

- Adds the trick theorised in https://github.com/bevyengine/bevy/issues/3300#issuecomment-991791234

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>
This commit is contained in:
Daniel McNab 2022-03-15 02:16:55 +00:00
parent a304fd9a99
commit 6e61fef67d
5 changed files with 167 additions and 142 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/bevy_ecs/macros/src/lib.rs
View File

@ -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> {

79
crates/bevy_ecs/src/system/function_system.rs
View File

@ -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

125
crates/bevy_ecs/src/system/system_param.rs
View File

@ -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;

67
crates/bevy_render/src/render_asset.rs
View File

@ -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));
}
}
}

36
crates/bevy_render/src/render_component.rs
View File

@ -9,10 +9,7 @@ use bevy_ecs::{
component::Component,
prelude::*,
query::{FilterFetch, QueryItem, WorldQuery},
system::{
lifetimeless::{Read, SCommands, SQuery},
RunSystem, SystemParamItem,
},
system::{lifetimeless::Read, StaticSystemParam},
};
use std::{marker::PhantomData, ops::Deref};
@ -147,9 +144,8 @@ where
<C::Filter as WorldQuery>::Fetch: FilterFetch,
{
fn build(&self, app: &mut App) {
let system = ExtractComponentSystem::<C>::system(&mut app.world);
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
render_app.add_system_to_stage(RenderStage::Extract, system);
render_app.add_system_to_stage(RenderStage::Extract, extract_components::<C>);
}
}
}
@ -165,25 +161,17 @@ impl<T: Asset> ExtractComponent for Handle<T> {
}
/// This system extracts all components of the corresponding [`ExtractComponent`] type.
pub struct ExtractComponentSystem<C: ExtractComponent>(PhantomData<C>);
impl<C: ExtractComponent> RunSystem for ExtractComponentSystem<C>
where
fn extract_components<C: ExtractComponent>(
mut commands: Commands,
mut previous_len: Local<usize>,
mut query: StaticSystemParam<Query<(Entity, C::Query), C::Filter>>,
) where
<C::Filter as WorldQuery>::Fetch: FilterFetch,
{
type Param = (
SCommands,
// the previous amount of extracted components
Local<'static, usize>,
SQuery<(Entity, C::Query), C::Filter>,
);
fn run((mut commands, mut previous_len, mut query): SystemParamItem<Self::Param>) {
let mut values = Vec::with_capacity(*previous_len);
for (entity, query_item) in query.iter_mut() {
values.push((entity, (C::extract_component(query_item),)));
}
*previous_len = values.len();
commands.insert_or_spawn_batch(values);
let mut values = Vec::with_capacity(*previous_len);
for (entity, query_item) in query.iter_mut() {
values.push((entity, (C::extract_component(query_item),)));
}
*previous_len = values.len();
commands.insert_or_spawn_batch(values);
}