Skip to content

Commit

Permalink
Add system.map(...) for transforming the output of a system (bevyen…
Browse files Browse the repository at this point in the history
…gine#8526)

# Objective

Any time we wish to transform the output of a system, we currently use
system piping to do so:

```rust
my_system.pipe(|In(x)| do_something(x))
```

Unfortunately, system piping is not a zero cost abstraction. Each call
to `.pipe` requires allocating two extra access sets: one for the second
system and one for the combined accesses of both systems. This also adds
extra work to each call to `update_archetype_component_access`, which
stacks as one adds multiple layers of system piping.

## Solution

Add the `AdapterSystem` abstraction: similar to `CombinatorSystem`, this
allows you to implement a trait to generically control how a system is
run and how its inputs and outputs are processed. Unlike
`CombinatorSystem`, this does not have any overhead when computing world
accesses which makes it ideal for simple operations such as inverting or
ignoring the output of a system.

Add the extension method `.map(...)`: this is similar to `.pipe(...)`,
only it accepts a closure as an argument instead of an `In<T>` system.

```rust
my_system.map(do_something)
```

This has the added benefit of making system names less messy: a system
that ignores its output will just be called `my_system`, instead of
`Pipe(my_system, ignore)`

---

## Changelog

TODO

## Migration Guide

The `system_adapter` functions have been deprecated: use `.map` instead,
which is a lightweight alternative to `.pipe`.

```rust
// Before:
my_system.pipe(system_adapter::ignore)
my_system.pipe(system_adapter::unwrap)
my_system.pipe(system_adapter::new(T::from))

// After:
my_system.map(std::mem::drop)
my_system.map(Result::unwrap)
my_system.map(T::from)

// Before:
my_system.pipe(system_adapter::info)
my_system.pipe(system_adapter::dbg)
my_system.pipe(system_adapter::warn)
my_system.pipe(system_adapter::error)

// After:
my_system.map(bevy_utils::info)
my_system.map(bevy_utils::dbg)
my_system.map(bevy_utils::warn)
my_system.map(bevy_utils::error)
```

---------

Co-authored-by: Alice Cecile <[email protected]>
  • Loading branch information
2 people authored and Shatur committed Aug 30, 2023
1 parent b9e6d15 commit ec84019
Show file tree
Hide file tree
Showing 6 changed files with 261 additions and 98 deletions.
6 changes: 4 additions & 2 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ pub mod prelude {
#[doc(hidden)]
#[cfg(feature = "bevy_reflect")]
pub use crate::reflect::{AppTypeRegistry, ReflectComponent, ReflectResource};
#[allow(deprecated)]
pub use crate::system::adapter::{
self as system_adapter, dbg, error, ignore, info, unwrap, warn,
};
#[doc(hidden)]
pub use crate::{
bundle::Bundle,
Expand All @@ -45,8 +49,6 @@ pub mod prelude {
OnEnter, OnExit, OnTransition, Schedule, Schedules, State, States, SystemSet,
},
system::{
adapter as system_adapter,
adapter::{dbg, error, ignore, info, unwrap, warn},
Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands,
ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction,
},
Expand Down
99 changes: 14 additions & 85 deletions crates/bevy_ecs/src/schedule/condition.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
use std::any::TypeId;
use std::borrow::Cow;
use std::ops::Not;

use crate::component::{self, ComponentId};
use crate::query::Access;
use crate::system::{CombinatorSystem, Combine, IntoSystem, ReadOnlySystem, System};
use crate::world::unsafe_world_cell::UnsafeWorldCell;
use crate::world::World;
use crate::system::{
Adapt, AdapterSystem, CombinatorSystem, Combine, IntoSystem, ReadOnlySystem, System,
};

/// A type-erased run condition stored in a [`Box`].
pub type BoxedCondition<In = ()> = Box<dyn ReadOnlySystem<In = In, Out = bool>>;
Expand Down Expand Up @@ -181,8 +178,6 @@ mod sealed {

/// A collection of [run conditions](Condition) that may be useful in any bevy app.
pub mod common_conditions {
use std::borrow::Cow;

use super::NotSystem;
use crate::{
change_detection::DetectChanges,
Expand Down Expand Up @@ -987,98 +982,32 @@ pub mod common_conditions {
{
let condition = IntoSystem::into_system(condition);
let name = format!("!{}", condition.name());
NotSystem::<T::System> {
condition,
name: Cow::Owned(name),
}
NotSystem::new(super::NotMarker, condition, name.into())
}
}

/// Invokes [`Not`] with the output of another system.
///
/// See [`common_conditions::not`] for examples.
#[derive(Clone)]
pub struct NotSystem<T: System>
where
T::Out: Not,
{
condition: T,
name: Cow<'static, str>,
}
impl<T: System> System for NotSystem<T>
pub type NotSystem<T> = AdapterSystem<NotMarker, T>;

/// Used with [`AdapterSystem`] to negate the output of a system via the [`Not`] operator.
#[doc(hidden)]
#[derive(Clone, Copy)]
pub struct NotMarker;

impl<T: System> Adapt<T> for NotMarker
where
T::Out: Not,
{
type In = T::In;
type Out = <T::Out as Not>::Output;

fn name(&self) -> Cow<'static, str> {
self.name.clone()
}

fn type_id(&self) -> TypeId {
TypeId::of::<T>()
}

fn component_access(&self) -> &Access<ComponentId> {
self.condition.component_access()
}

fn archetype_component_access(&self) -> &Access<crate::archetype::ArchetypeComponentId> {
self.condition.archetype_component_access()
}

fn is_send(&self) -> bool {
self.condition.is_send()
}

fn is_exclusive(&self) -> bool {
self.condition.is_exclusive()
}

unsafe fn run_unsafe(&mut self, input: Self::In, world: UnsafeWorldCell) -> Self::Out {
// SAFETY: The inner condition system asserts its own safety.
!self.condition.run_unsafe(input, world)
}

fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out {
!self.condition.run(input, world)
}

fn apply_deferred(&mut self, world: &mut World) {
self.condition.apply_deferred(world);
}

fn initialize(&mut self, world: &mut World) {
self.condition.initialize(world);
}

fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) {
self.condition.update_archetype_component_access(world);
}

fn check_change_tick(&mut self, change_tick: component::Tick) {
self.condition.check_change_tick(change_tick);
}

fn get_last_run(&self) -> component::Tick {
self.condition.get_last_run()
}

fn set_last_run(&mut self, last_run: component::Tick) {
self.condition.set_last_run(last_run);
fn adapt(&mut self, input: Self::In, run_system: impl FnOnce(T::In) -> T::Out) -> Self::Out {
!run_system(input)
}
}

// SAFETY: This trait is only implemented when the inner system is read-only.
// The `Not` condition does not modify access, and thus cannot transform a read-only system into one that is not.
unsafe impl<T> ReadOnlySystem for NotSystem<T>
where
T: ReadOnlySystem,
T::Out: Not,
{
}

/// Combines the outputs of two systems using the `&&` operator.
pub type AndThen<A, B> = CombinatorSystem<AndThenMarker, A, B>;

Expand Down
170 changes: 170 additions & 0 deletions crates/bevy_ecs/src/system/adapter_system.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
use std::borrow::Cow;

use super::{ReadOnlySystem, System};
use crate::world::unsafe_world_cell::UnsafeWorldCell;

/// Customizes the behavior of an [`AdapterSystem`]
///
/// # Examples
///
/// ```
/// # use bevy_ecs::prelude::*;
/// use bevy_ecs::system::{Adapt, AdapterSystem};
///
/// // A system adapter that inverts the result of a system.
/// // NOTE: Instead of manually implementing this, you can just use `bevy_ecs::schedule::common_conditions::not`.
/// pub type NotSystem<S> = AdapterSystem<NotMarker, S>;
///
/// // This struct is used to customize the behavior of our adapter.
/// pub struct NotMarker;
///
/// impl<S> Adapt<S> for NotMarker
/// where
/// S: System,
/// S::Out: std::ops::Not,
/// {
/// type In = S::In;
/// type Out = <S::Out as std::ops::Not>::Output;
///
/// fn adapt(
/// &mut self,
/// input: Self::In,
/// run_system: impl FnOnce(S::In) -> S::Out,
/// ) -> Self::Out {
/// !run_system(input)
/// }
/// }
/// # let mut world = World::new();
/// # let mut system = NotSystem::new(NotMarker, IntoSystem::into_system(|| false), "".into());
/// # system.initialize(&mut world);
/// # assert!(system.run((), &mut world));
/// ```
pub trait Adapt<S: System>: Send + Sync + 'static {
/// The [input](System::In) type for an [`AdapterSystem`].
type In;
/// The [output](System::Out) type for an [`AdapterSystem`].
type Out;

/// When used in an [`AdapterSystem`], this function customizes how the system
/// is run and how its inputs/outputs are adapted.
fn adapt(&mut self, input: Self::In, run_system: impl FnOnce(S::In) -> S::Out) -> Self::Out;
}

/// A [`System`] that takes the output of `S` and transforms it by applying `Func` to it.
#[derive(Clone)]
pub struct AdapterSystem<Func, S> {
func: Func,
system: S,
name: Cow<'static, str>,
}

impl<Func, S> AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: System,
{
/// Creates a new [`System`] that uses `func` to adapt `system`, via the [`Adapt`] trait.
pub const fn new(func: Func, system: S, name: Cow<'static, str>) -> Self {
Self { func, system, name }
}
}

impl<Func, S> System for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: System,
{
type In = Func::In;
type Out = Func::Out;

fn name(&self) -> Cow<'static, str> {
self.name.clone()
}

fn type_id(&self) -> std::any::TypeId {
std::any::TypeId::of::<Self>()
}

fn component_access(&self) -> &crate::query::Access<crate::component::ComponentId> {
self.system.component_access()
}

#[inline]
fn archetype_component_access(
&self,
) -> &crate::query::Access<crate::archetype::ArchetypeComponentId> {
self.system.archetype_component_access()
}

fn is_send(&self) -> bool {
self.system.is_send()
}

fn is_exclusive(&self) -> bool {
self.system.is_exclusive()
}

#[inline]
unsafe fn run_unsafe(&mut self, input: Self::In, world: UnsafeWorldCell) -> Self::Out {
// SAFETY: `system.run_unsafe` has the same invariants as `self.run_unsafe`.
self.func
.adapt(input, |input| self.system.run_unsafe(input, world))
}

#[inline]
fn run(&mut self, input: Self::In, world: &mut crate::prelude::World) -> Self::Out {
self.func
.adapt(input, |input| self.system.run(input, world))
}

#[inline]
fn apply_deferred(&mut self, world: &mut crate::prelude::World) {
self.system.apply_deferred(world);
}

fn initialize(&mut self, world: &mut crate::prelude::World) {
self.system.initialize(world);
}

#[inline]
fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) {
self.system.update_archetype_component_access(world);
}

fn check_change_tick(&mut self, change_tick: crate::component::Tick) {
self.system.check_change_tick(change_tick);
}

fn get_last_run(&self) -> crate::component::Tick {
self.system.get_last_run()
}

fn set_last_run(&mut self, last_run: crate::component::Tick) {
self.system.set_last_run(last_run);
}

fn default_system_sets(&self) -> Vec<Box<dyn crate::schedule::SystemSet>> {
self.system.default_system_sets()
}
}

// SAFETY: The inner system is read-only.
unsafe impl<Func, S> ReadOnlySystem for AdapterSystem<Func, S>
where
Func: Adapt<S>,
S: ReadOnlySystem,
{
}

impl<F, S, Out> Adapt<S> for F
where
S: System,
F: Send + Sync + 'static + FnMut(S::Out) -> Out,
{
type In = S::In;
type Out = Out;

fn adapt(&mut self, input: S::In, run_system: impl FnOnce(S::In) -> S::Out) -> Out {
self(run_system(input))
}
}
Loading

0 comments on commit ec84019

Please sign in to comment.