Skip to content

Commit

Permalink
Simplify generics for the SystemParamFunction trait (#7675)
Browse files Browse the repository at this point in the history
# Objective

The `SystemParamFunction` (and `ExclusiveSystemParamFunction`) trait is very cumbersome to use, due to it requiring four generic type parameters. These are currently all used as marker parameters to satisfy rust's trait coherence rules.

### Example (before)

```rust
pub fn pipe<AIn, Shared, BOut, A, AParam, AMarker, B, BParam, BMarker>(
    mut system_a: A,
    mut system_b: B,
) -> impl FnMut(In<AIn>, ParamSet<(AParam, BParam)>) -> BOut
where
    A: SystemParamFunction<AIn, Shared, AParam, AMarker>,
    B: SystemParamFunction<Shared, BOut, BParam, BMarker>,
    AParam: SystemParam,
    BParam: SystemParam,
```

## Solution

Turn the `In`, `Out`, and `Param` generics into associated types. Merge the marker types together to retain coherence.

### Example (after)

```rust
pub fn pipe<A, B, AMarker, BMarker>(
    mut system_a: A,
    mut system_b: B,
) -> impl FnMut(In<A::In>, ParamSet<(A::Param, B::Param)>) -> B::Out
where
    A: SystemParamFunction<AMarker>,
    B: SystemParamFunction<BMarker, In = A::Out>,
```

---

## Changelog

+ Simplified the `SystemParamFunction` and `ExclusiveSystemParamFunction` traits.

## Migration Guide

For users of the `SystemParamFunction` trait, the generic type parameters `In`, `Out`, and `Param` have been turned into associated types. The same has been done with the `ExclusiveSystemParamFunction` trait.
  • Loading branch information
JoJoJet committed Feb 15, 2023
1 parent 11c7e58 commit 4f57f38
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 78 deletions.
14 changes: 5 additions & 9 deletions crates/bevy_ecs/src/schedule/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use bevy_utils::define_boxed_label;
use bevy_utils::label::DynHash;

use crate::system::{
ExclusiveSystemParam, ExclusiveSystemParamFunction, IsExclusiveFunctionSystem,
IsFunctionSystem, SystemParam, SystemParamFunction,
ExclusiveSystemParamFunction, IsExclusiveFunctionSystem, IsFunctionSystem, SystemParamFunction,
};

define_boxed_label!(ScheduleLabel);
Expand Down Expand Up @@ -130,10 +129,9 @@ impl<S: SystemSet> IntoSystemSet<()> for S {
}

// systems
impl<In, Out, Param, Marker, F> IntoSystemSet<(IsFunctionSystem, In, Out, Param, Marker)> for F
impl<Marker, F> IntoSystemSet<(IsFunctionSystem, Marker)> for F
where
Param: SystemParam,
F: SystemParamFunction<In, Out, Param, Marker>,
F: SystemParamFunction<Marker>,
{
type Set = SystemTypeSet<Self>;

Expand All @@ -144,11 +142,9 @@ where
}

// exclusive systems
impl<In, Out, Param, Marker, F> IntoSystemSet<(IsExclusiveFunctionSystem, In, Out, Param, Marker)>
for F
impl<Marker, F> IntoSystemSet<(IsExclusiveFunctionSystem, Marker)> for F
where
Param: ExclusiveSystemParam,
F: ExclusiveSystemParamFunction<In, Out, Param, Marker>,
F: ExclusiveSystemParamFunction<Marker>,
{
type Set = SystemTypeSet<Self>;

Expand Down
63 changes: 35 additions & 28 deletions crates/bevy_ecs/src/system/exclusive_function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,30 +18,26 @@ use std::{any::TypeId, borrow::Cow, marker::PhantomData};
/// [`ExclusiveSystemParam`]s.
///
/// [`ExclusiveFunctionSystem`] must be `.initialized` before they can be run.
pub struct ExclusiveFunctionSystem<In, Out, Param, Marker, F>
pub struct ExclusiveFunctionSystem<Marker, F>
where
Param: ExclusiveSystemParam,
F: ExclusiveSystemParamFunction<Marker>,
{
func: F,
param_state: Option<Param::State>,
param_state: Option<<F::Param as ExclusiveSystemParam>::State>,
system_meta: SystemMeta,
world_id: Option<WorldId>,
// NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
marker: PhantomData<fn(In) -> (Out, Marker)>,
marker: PhantomData<fn() -> Marker>,
}

pub struct IsExclusiveFunctionSystem;

impl<In, Out, Param, Marker, F> IntoSystem<In, Out, (IsExclusiveFunctionSystem, Param, Marker)>
for F
impl<Marker, F> IntoSystem<F::In, F::Out, (IsExclusiveFunctionSystem, Marker)> for F
where
In: 'static,
Out: 'static,
Param: ExclusiveSystemParam + 'static,
Marker: 'static,
F: ExclusiveSystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
F: ExclusiveSystemParamFunction<Marker>,
{
type System = ExclusiveFunctionSystem<In, Out, Param, Marker, F>;
type System = ExclusiveFunctionSystem<Marker, F>;
fn into_system(func: Self) -> Self::System {
ExclusiveFunctionSystem {
func,
Expand All @@ -55,16 +51,13 @@ where

const PARAM_MESSAGE: &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 ExclusiveFunctionSystem<In, Out, Param, Marker, F>
impl<Marker, F> System for ExclusiveFunctionSystem<Marker, F>
where
In: 'static,
Out: 'static,
Param: ExclusiveSystemParam + 'static,
Marker: 'static,
F: ExclusiveSystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
F: ExclusiveSystemParamFunction<Marker>,
{
type In = In;
type Out = Out;
type In = F::In;
type Out = F::Out;

#[inline]
fn name(&self) -> Cow<'static, str> {
Expand Down Expand Up @@ -103,7 +96,7 @@ where
let saved_last_tick = world.last_change_tick;
world.last_change_tick = self.system_meta.last_change_tick;

let params = Param::get_param(
let params = F::Param::get_param(
self.param_state.as_mut().expect(PARAM_MESSAGE),
&self.system_meta,
);
Expand Down Expand Up @@ -141,7 +134,7 @@ where
fn initialize(&mut self, world: &mut World) {
self.world_id = Some(world.id());
self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE);
self.param_state = Some(Param::init(world, &mut self.system_meta));
self.param_state = Some(F::Param::init(world, &mut self.system_meta));
}

fn update_archetype_component_access(&mut self, _world: &World) {}
Expand All @@ -165,27 +158,38 @@ where
///
/// This trait can be useful for making your own systems which accept other systems,
/// sometimes called higher order systems.
pub trait ExclusiveSystemParamFunction<In, Out, Param: ExclusiveSystemParam, Marker>:
Send + Sync + 'static
{
pub trait ExclusiveSystemParamFunction<Marker>: Send + Sync + 'static {
/// The input type to this system. See [`System::In`].
type In;

/// The return type of this system. See [`System::Out`].
type Out;

/// The [`ExclusiveSystemParam`]/s defined by this system's `fn` parameters.
type Param: ExclusiveSystemParam;

/// Executes this system once. See [`System::run`].
fn run(
&mut self,
world: &mut World,
input: In,
param_value: ExclusiveSystemParamItem<Param>,
) -> Out;
input: Self::In,
param_value: ExclusiveSystemParamItem<Self::Param>,
) -> Self::Out;
}

macro_rules! impl_exclusive_system_function {
($($param: ident),*) => {
#[allow(non_snake_case)]
impl<Out, Func: Send + Sync + 'static, $($param: ExclusiveSystemParam),*> ExclusiveSystemParamFunction<(), Out, ($($param,)*), ()> for Func
impl<Out, Func: Send + Sync + 'static, $($param: ExclusiveSystemParam),*> ExclusiveSystemParamFunction<((), Out, $($param,)*)> for Func
where
for <'a> &'a mut Func:
FnMut(&mut World, $($param),*) -> Out +
FnMut(&mut World, $(ExclusiveSystemParamItem<$param>),*) -> Out,
Out: 'static,
{
type In = ();
type Out = Out;
type Param = ($($param,)*);
#[inline]
fn run(&mut self, world: &mut World, _in: (), param_value: ExclusiveSystemParamItem< ($($param,)*)>) -> Out {
// Yes, this is strange, but `rustc` fails to compile this impl
Expand All @@ -204,13 +208,16 @@ macro_rules! impl_exclusive_system_function {
}
}
#[allow(non_snake_case)]
impl<Input, Out, Func: Send + Sync + 'static, $($param: ExclusiveSystemParam),*> ExclusiveSystemParamFunction<Input, Out, ($($param,)*), InputMarker> for Func
impl<Input, Out, Func: Send + Sync + 'static, $($param: ExclusiveSystemParam),*> ExclusiveSystemParamFunction<(Input, Out, $($param,)* InputMarker)> for Func
where
for <'a> &'a mut Func:
FnMut(In<Input>, &mut World, $($param),*) -> Out +
FnMut(In<Input>, &mut World, $(ExclusiveSystemParamItem<$param>),*) -> Out,
Out: 'static,
{
type In = Input;
type Out = Out;
type Param = ($($param,)*);
#[inline]
fn run(&mut self, world: &mut World, input: Input, param_value: ExclusiveSystemParamItem< ($($param,)*)>) -> Out {
// Yes, this is strange, but `rustc` fails to compile this impl
Expand Down
85 changes: 44 additions & 41 deletions crates/bevy_ecs/src/system/function_system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,30 +371,27 @@ pub struct InputMarker;
/// becomes the functions [`In`] tagged parameter or `()` if no such parameter exists.
///
/// [`FunctionSystem`] must be `.initialized` before they can be run.
pub struct FunctionSystem<In, Out, Param, Marker, F>
pub struct FunctionSystem<Marker, F>
where
Param: SystemParam,
F: SystemParamFunction<Marker>,
{
func: F,
param_state: Option<Param::State>,
param_state: Option<<F::Param as SystemParam>::State>,
system_meta: SystemMeta,
world_id: Option<WorldId>,
archetype_generation: ArchetypeGeneration,
// NOTE: PhantomData<fn()-> T> gives this safe Send/Sync impls
marker: PhantomData<fn(In) -> (Out, Marker)>,
marker: PhantomData<fn() -> Marker>,
}

pub struct IsFunctionSystem;

impl<In, Out, Param, Marker, F> IntoSystem<In, Out, (IsFunctionSystem, Param, Marker)> for F
impl<Marker, F> IntoSystem<F::In, F::Out, (IsFunctionSystem, Marker)> for F
where
In: 'static,
Out: 'static,
Param: SystemParam + 'static,
Marker: 'static,
F: SystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
F: SystemParamFunction<Marker>,
{
type System = FunctionSystem<In, Out, Param, Marker, F>;
type System = FunctionSystem<Marker, F>;
fn into_system(func: Self) -> Self::System {
FunctionSystem {
func,
Expand All @@ -407,26 +404,23 @@ where
}
}

impl<In, Out, Param, Marker, F> FunctionSystem<In, Out, Param, Marker, F>
impl<Marker, F> FunctionSystem<Marker, F>
where
Param: SystemParam,
F: SystemParamFunction<Marker>,
{
/// 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>
impl<Marker, F> System for FunctionSystem<Marker, F>
where
In: 'static,
Out: 'static,
Param: SystemParam + 'static,
Marker: 'static,
F: SystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
F: SystemParamFunction<Marker>,
{
type In = In;
type Out = Out;
type In = F::In;
type Out = F::Out;

#[inline]
fn name(&self) -> Cow<'static, str> {
Expand Down Expand Up @@ -466,7 +460,7 @@ where
// 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::get_param(
let params = F::Param::get_param(
self.param_state.as_mut().expect(Self::PARAM_MESSAGE),
&self.system_meta,
world,
Expand All @@ -488,14 +482,14 @@ where
#[inline]
fn apply_buffers(&mut self, world: &mut World) {
let param_state = self.param_state.as_mut().expect(Self::PARAM_MESSAGE);
Param::apply(param_state, &self.system_meta, world);
F::Param::apply(param_state, &self.system_meta, world);
}

#[inline]
fn initialize(&mut self, world: &mut World) {
self.world_id = Some(world.id());
self.system_meta.last_change_tick = world.change_tick().wrapping_sub(MAX_CHANGE_AGE);
self.param_state = Some(Param::init_state(world, &mut self.system_meta));
self.param_state = Some(F::Param::init_state(world, &mut self.system_meta));
}

fn update_archetype_component_access(&mut self, world: &World) {
Expand All @@ -507,7 +501,7 @@ where

for archetype_index in archetype_index_range {
let param_state = self.param_state.as_mut().unwrap();
Param::new_archetype(
F::Param::new_archetype(
param_state,
&archetypes[ArchetypeId::new(archetype_index)],
&mut self.system_meta,
Expand All @@ -531,13 +525,11 @@ where
}

/// SAFETY: `F`'s param is `ReadOnlySystemParam`, so this system will only read from the world.
unsafe impl<In, Out, Param, Marker, F> ReadOnlySystem for FunctionSystem<In, Out, Param, Marker, F>
unsafe impl<Marker, F> ReadOnlySystem for FunctionSystem<Marker, F>
where
In: 'static,
Out: 'static,
Param: ReadOnlySystemParam + 'static,
Marker: 'static,
F: SystemParamFunction<In, Out, Param, Marker> + Send + Sync + 'static,
F: SystemParamFunction<Marker>,
F::Param: ReadOnlySystemParam,
{
}

Expand All @@ -560,20 +552,15 @@ where
/// 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`.
/// /// Pipe creates a new system which calls `a`, then calls `b` with the output of `a`
/// pub fn pipe<AIn, Shared, BOut, A, AParam, AMarker, B, BParam, BMarker>(
/// pub fn pipe<A, B, AMarker, BMarker>(
/// mut a: A,
/// mut b: B,
/// ) -> impl FnMut(In<AIn>, ParamSet<(AParam, BParam)>) -> BOut
/// ) -> impl FnMut(In<A::In>, ParamSet<(A::Param, B::Param)>) -> B::Out
/// 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,
/// A: SystemParamFunction<AMarker>,
/// B: SystemParamFunction<BMarker, In = A::Out>,
/// {
/// // The type of `params` is inferred based on the return of this function above
/// move |In(a_in), mut params| {
Expand Down Expand Up @@ -606,19 +593,32 @@ where
/// ```
/// [`PipeSystem`]: crate::system::PipeSystem
/// [`ParamSet`]: crate::system::ParamSet
pub trait SystemParamFunction<In, Out, Param: SystemParam, Marker>: Send + Sync + 'static {
fn run(&mut self, input: In, param_value: SystemParamItem<Param>) -> Out;
pub trait SystemParamFunction<Marker>: Send + Sync + 'static {
/// The input type to this system. See [`System::In`].
type In;

/// The return type of this system. See [`System::Out`].
type Out;

/// The [`SystemParam`]/s used by this system to access the [`World`].
type Param: SystemParam;

/// Executes this system once. See [`System::run`] or [`System::run_unsafe`].
fn run(&mut self, input: Self::In, param_value: SystemParamItem<Self::Param>) -> Self::Out;
}

macro_rules! impl_system_function {
($($param: ident),*) => {
#[allow(non_snake_case)]
impl<Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<(), Out, ($($param,)*), ()> for Func
impl<Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<((), Out, $($param,)*)> for Func
where
for <'a> &'a mut Func:
FnMut($($param),*) -> Out +
FnMut($(SystemParamItem<$param>),*) -> Out, Out: 'static
{
type In = ();
type Out = Out;
type Param = ($($param,)*);
#[inline]
fn run(&mut self, _input: (), param_value: SystemParamItem< ($($param,)*)>) -> Out {
// Yes, this is strange, but `rustc` fails to compile this impl
Expand All @@ -637,12 +637,15 @@ macro_rules! impl_system_function {
}

#[allow(non_snake_case)]
impl<Input, Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<Input, Out, ($($param,)*), InputMarker> for Func
impl<Input, Out, Func: Send + Sync + 'static, $($param: SystemParam),*> SystemParamFunction<(Input, Out, $($param,)* InputMarker)> for Func
where
for <'a> &'a mut Func:
FnMut(In<Input>, $($param),*) -> Out +
FnMut(In<Input>, $(SystemParamItem<$param>),*) -> Out, Out: 'static
{
type In = Input;
type Out = Out;
type Param = ($($param,)*);
#[inline]
fn run(&mut self, input: Input, param_value: SystemParamItem< ($($param,)*)>) -> Out {
#[allow(clippy::too_many_arguments)]
Expand Down

0 comments on commit 4f57f38

Please sign in to comment.