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

Allow higher order systems (#4833)

Browse Source 
# Objective

- Higher order system could not be created by users.
- However, a simple change to `SystemParamFunction` allows this.
- Higher order systems in this case mean functions which return systems created using other systems, such as `chain` (which is basically equivalent to map)

## Solution

- Change `SystemParamFunction` to be a safe abstraction over `FnMut([In<In>,] ...params)->Out`.
- Note that I believe `SystemParamFunction` should not have been counted as part of our public api before this PR.
    - This is because its only use was an unsafe function without an actionable safety comment.
    - The safety comment was basically 'call this within bevy code'.
    - I also believe that there are no external users in its current form. 
        - A quick search on Google and in the discord confirmed this.

## See also

- https://github.com/bevyengine/bevy/pull/4666, which uses this and subsumes the example here

---

## Changelog

### Added

- `SystemParamFunction`, which can be used to create higher order systems.
This commit is contained in:
Daniel McNab 2022-05-30 17:59:20 +00:00
parent c46691c04a
commit 80b08ea45d
2 changed files with 100 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -7,7 +7,7 @@ use crate::{
schedule::SystemLabel,
system::{
check_system_change_tick, ReadOnlySystemParamFetch, System, SystemParam, SystemParamFetch,
SystemParamState,
SystemParamItem, SystemParamState,
},
world::{World, WorldId},
};
@ -346,6 +346,16 @@ where
}
}
impl<In, Out, Param, Marker, F> FunctionSystem<In, Out, Param, Marker, F>
where
Param: SystemParam,
{
/// Message shown when a system isn't initialised
// When lines get too long, rustfmt can sometimes refuse to format them.
// Work around this by storing the message separately.
const PARAM_MESSAGE: &'static str = "System's param_state was not found. Did you forget to initialize this system before running it?";
}
impl<In, Out, Param, Marker, F> System for FunctionSystem<In, Out, Param, Marker, F>
where
In: 'static,
@ -380,20 +390,25 @@ where
#[inline]
unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out {
let change_tick = world.increment_change_tick();
let out = self.func.run(
input,
self.param_state.as_mut().expect("System's param_state was not found. Did you forget to initialize this system before running it?"),
// Safety:
// We update the archetype component access correctly based on `Param`'s requirements
// in `update_archetype_component_access`.
// Our caller upholds the requirements.
let params = <Param as SystemParam>::Fetch::get_param(
self.param_state.as_mut().expect(Self::PARAM_MESSAGE),
&self.system_meta,
world,
change_tick,
);
let out = self.func.run(input, params);
self.system_meta.last_change_tick = change_tick;
out
}
#[inline]
fn apply_buffers(&mut self, world: &mut World) {
let param_state = self.param_state.as_mut().expect("System's param_state was not found. Did you forget to initialize this system before running it?");
let param_state = self.param_state.as_mut().expect(Self::PARAM_MESSAGE);
param_state.apply(world);
}
@ -474,19 +489,71 @@ impl<T> SystemLabel for SystemTypeIdLabel<T> {
}
/// A trait implemented for all functions that can be used as [`System`]s.
///
/// This trait can be useful for making your own systems which accept other systems,
/// sometimes called higher order systems.
///
/// This should be used in combination with [`ParamSet`] when calling other systems
/// within your system.
/// Using [`ParamSet`] in this case avoids [`SystemParam`] collisions.
///
/// # Example
///
/// To create something like [`ChainSystem`], but in entirely safe code.
///
/// ```rust
/// use std::num::ParseIntError;
///
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::system::{SystemParam, SystemParamItem};
///
/// // Unfortunately, we need all of these generics. `A` is the first system, with its
/// // parameters and marker type required for coherence. `B` is the second system, and
/// // the other generics are for the input/output types of A and B.
/// /// Chain creates a new system which calls `a`, then calls `b` with the output of `a`
/// pub fn chain<AIn, Shared, BOut, A, AParam, AMarker, B, BParam, BMarker>(
/// mut a: A,
/// mut b: B,
/// ) -> impl FnMut(In<AIn>, ParamSet<(SystemParamItem<AParam>, SystemParamItem<BParam>)>) -> BOut
/// where
/// // We need A and B to be systems, add those bounds
/// A: SystemParamFunction<AIn, Shared, AParam, AMarker>,
/// B: SystemParamFunction<Shared, BOut, BParam, BMarker>,
/// AParam: SystemParam,
/// BParam: SystemParam,
/// {
/// // The type of `params` is inferred based on the return of this function above
/// move |In(a_in), mut params| {
/// let shared = a.run(a_in, params.p0());
/// b.run(shared, params.p1())
/// }
/// }
///
/// // Usage example for `chain`:
/// fn main() {
/// let mut world = World::default();
/// world.insert_resource(Message("42".to_string()));
///
/// // chain the `parse_message_system`'s output into the `filter_system`s input
/// let mut chained_system = IntoSystem::into_system(chain(parse_message, filter));
/// chained_system.initialize(&mut world);
/// assert_eq!(chained_system.run((), &mut world), Some(42));
/// }
///
/// struct Message(String);
///
/// fn parse_message(message: Res<Message>) -> Result<usize, ParseIntError> {
/// message.0.parse::<usize>()
/// }
///
/// fn filter(In(result): In<Result<usize, ParseIntError>>) -> Option<usize> {
/// result.ok().filter(|&n| n < 100)
/// }
/// ```
/// [`ChainSystem`]: crate::system::ChainSystem
/// [`ParamSet`]: crate::system::ParamSet
pub trait SystemParamFunction<In, Out, Param: SystemParam, Marker>: Send + Sync + 'static {
/// # 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_meta: &SystemMeta,
world: &World,
change_tick: u32,
) -> Out;
fn run(&mut self, input: In, param_value: SystemParamItem<Param>) -> Out;
}
macro_rules! impl_system_function {
@ -496,12 +563,13 @@ macro_rules! impl_system_function {
where
for <'a> &'a mut Func:
FnMut($($param),*) -> Out +
FnMut($(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out, Out: 'static
FnMut($(SystemParamItem<$param>),*) -> Out, Out: 'static
{
#[inline]
unsafe fn run(&mut self, _input: (), state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out {
// Yes, this is strange, but rustc fails to compile this impl
// without using this function.
fn run(&mut self, _input: (), param_value: SystemParamItem< ($($param,)*)>) -> Out {
// Yes, this is strange, but `rustc` fails to compile this impl
// without using this function. It fails to recognise that `func`
// is a function, potentially because of the multiple impls of `FnMut`
#[allow(clippy::too_many_arguments)]
fn call_inner<Out, $($param,)*>(
mut f: impl FnMut($($param,)*)->Out,
@ -509,7 +577,7 @@ macro_rules! impl_system_function {
)->Out{
f($($param,)*)
}
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick);
let ($($param,)*) = param_value;
call_inner(self, $($param),*)
}
}
@ -522,7 +590,7 @@ macro_rules! impl_system_function {
FnMut(In<Input>, $(<<$param as SystemParam>::Fetch as SystemParamFetch>::Item),*) -> Out, Out: 'static
{
#[inline]
unsafe fn run(&mut self, input: Input, state: &mut <($($param,)*) as SystemParam>::Fetch, system_meta: &SystemMeta, world: &World, change_tick: u32) -> Out {
fn run(&mut self, input: Input, param_value: SystemParamItem< ($($param,)*)>) -> Out {
#[allow(clippy::too_many_arguments)]
fn call_inner<Input, Out, $($param,)*>(
mut f: impl FnMut(In<Input>, $($param,)*)->Out,
@ -531,13 +599,15 @@ macro_rules! impl_system_function {
)->Out{
f(input, $($param,)*)
}
let ($($param,)*) = <<($($param,)*) as SystemParam>::Fetch as SystemParamFetch>::get_param(state, system_meta, world, change_tick);
let ($($param,)*) = param_value;
call_inner(self, In(input), $($param),*)
}
}
};
}
// Note that we rely on the highest impl to be <= the highest order of the tuple impls
// of `SystemParam` created.
all_tuples!(impl_system_function, 0, 16, F);
/// Used to implicitly convert systems to their default labels. For example, it will convert

6
crates/bevy_ecs/src/system/mod.rs
View File

@ -83,9 +83,15 @@ pub use system::*;
pub use system_chaining::*;
pub use system_param::*;
/// Ensure that a given function is a system
///
/// This should be used when writing doc examples,
/// to confirm that systems used in an example are
/// valid systems
pub fn assert_is_system<In, Out, Params, S: IntoSystem<In, Out, Params>>(sys: S) {
if false {
// Check it can be converted into a system
// TODO: This should ensure that the system has no conflicting system params
IntoSystem::into_system(sys);
}
}