Add AND/OR combinators for run conditions (#7605)

# Objective

Fix #7584.

## Solution

Add an abstraction for creating custom system combinators with minimal boilerplate. Use this to implement AND/OR combinators. Use this to simplify the implementation of `PipeSystem`.

## Example

Feel free to bikeshed on the syntax.

I chose the names `and_then`/`or_else` to emphasize the fact that these short-circuit, while I chose method syntax to empasize that the arguments are *not* treated equally.

```rust
app.add_systems((
    my_system.run_if(resource_exists::<R>().and_then(resource_equals(R(0)))),
    our_system.run_if(resource_exists::<R>().or_else(resource_exists::<S>())),
));
```

---

## Todo

- [ ] Decide on a syntax
- [x] Write docs
- [x] Write tests

## Changelog

+ Added the extension methods `.and_then(...)` and `.or_else(...)` to run conditions, which allows combining run conditions with short-circuiting behavior.
+ Added the trait `Combine`, which can be used with the new `CombinatorSystem` to create system combinators with custom behavior.
This commit is contained in:
JoJoJet 2023-02-20 18:16:11 +00:00
parent e2c77fee03
commit 0c98f9a225
6 changed files with 425 additions and 137 deletions

View file

@ -41,9 +41,10 @@ pub mod prelude {
query::{Added, AnyOf, Changed, Or, QueryState, With, Without},
removal_detection::RemovedComponents,
schedule::{
apply_state_transition, apply_system_buffers, common_conditions::*, IntoSystemConfig,
IntoSystemConfigs, IntoSystemSet, IntoSystemSetConfig, IntoSystemSetConfigs, NextState,
OnEnter, OnExit, OnUpdate, Schedule, Schedules, State, States, SystemSet,
apply_state_transition, apply_system_buffers, common_conditions::*, Condition,
IntoSystemConfig, IntoSystemConfigs, IntoSystemSet, IntoSystemSetConfig,
IntoSystemSetConfigs, NextState, OnEnter, OnExit, OnUpdate, Schedule, Schedules, State,
States, SystemSet,
},
system::{
adapter as system_adapter,

View file

@ -1,4 +1,6 @@
use crate::system::BoxedSystem;
use std::borrow::Cow;
use crate::system::{BoxedSystem, CombinatorSystem, Combine, IntoSystem, System};
pub type BoxedCondition = BoxedSystem<(), bool>;
@ -6,7 +8,105 @@ pub type BoxedCondition = BoxedSystem<(), bool>;
///
/// Implemented for functions and closures that convert into [`System<In=(), Out=bool>`](crate::system::System)
/// with [read-only](crate::system::ReadOnlySystemParam) parameters.
pub trait Condition<Params>: sealed::Condition<Params> {}
pub trait Condition<Params>: sealed::Condition<Params> {
/// Returns a new run condition that only returns `true`
/// if both this one and the passed `and_then` return `true`.
///
/// The returned run condition is short-circuiting, meaning
/// `and_then` will only be invoked if `self` returns `true`.
///
/// # Examples
///
/// ```should_panic
/// use bevy_ecs::prelude::*;
///
/// #[derive(Resource, PartialEq)]
/// struct R(u32);
///
/// # let mut app = Schedule::new();
/// # let mut world = World::new();
/// # fn my_system() {}
/// app.add_system(
/// // The `resource_equals` run condition will panic since we don't initialize `R`,
/// // just like if we used `Res<R>` in a system.
/// my_system.run_if(resource_equals(R(0))),
/// );
/// # app.run(&mut world);
/// ```
///
/// Use `.and_then()` to avoid checking the condition.
///
/// ```
/// # use bevy_ecs::prelude::*;
/// # #[derive(Resource, PartialEq)]
/// # struct R(u32);
/// # let mut app = Schedule::new();
/// # let mut world = World::new();
/// # fn my_system() {}
/// app.add_system(
/// // `resource_equals` will only get run if the resource `R` exists.
/// my_system.run_if(resource_exists::<R>().and_then(resource_equals(R(0)))),
/// );
/// # app.run(&mut world);
/// ```
///
/// Note that in this case, it's better to just use the run condition [`resource_exists_and_equals`].
///
/// [`resource_exists_and_equals`]: common_conditions::resource_exists_and_equals
fn and_then<P, C: Condition<P>>(self, and_then: C) -> AndThen<Self::System, C::System> {
let a = IntoSystem::into_system(self);
let b = IntoSystem::into_system(and_then);
let name = format!("{} && {}", a.name(), b.name());
CombinatorSystem::new(a, b, Cow::Owned(name))
}
/// Returns a new run condition that returns `true`
/// if either this one or the passed `or_else` return `true`.
///
/// The returned run condition is short-circuiting, meaning
/// `or_else` will only be invoked if `self` returns `false`.
///
/// # Examples
///
/// ```
/// use bevy_ecs::prelude::*;
///
/// #[derive(Resource, PartialEq)]
/// struct A(u32);
///
/// #[derive(Resource, PartialEq)]
/// struct B(u32);
///
/// # let mut app = Schedule::new();
/// # let mut world = World::new();
/// # #[derive(Resource)] struct C(bool);
/// # fn my_system(mut c: ResMut<C>) { c.0 = true; }
/// app.add_system(
/// // Only run the system if either `A` or `B` exist.
/// my_system.run_if(resource_exists::<A>().or_else(resource_exists::<B>())),
/// );
/// #
/// # world.insert_resource(C(false));
/// # app.run(&mut world);
/// # assert!(!world.resource::<C>().0);
/// #
/// # world.insert_resource(A(0));
/// # app.run(&mut world);
/// # assert!(world.resource::<C>().0);
/// #
/// # world.remove_resource::<A>();
/// # world.insert_resource(B(0));
/// # world.insert_resource(C(false));
/// # app.run(&mut world);
/// # assert!(world.resource::<C>().0);
/// ```
fn or_else<P, C: Condition<P>>(self, or_else: C) -> OrElse<Self::System, C::System> {
let a = IntoSystem::into_system(self);
let b = IntoSystem::into_system(or_else);
let name = format!("{} || {}", a.name(), b.name());
CombinatorSystem::new(a, b, Cow::Owned(name))
}
}
impl<Params, F> Condition<Params> for F where F: sealed::Condition<Params> {}
@ -146,3 +246,51 @@ pub mod common_conditions {
condition.pipe(|In(val): In<bool>| !val)
}
}
/// Combines the outputs of two systems using the `&&` operator.
pub type AndThen<A, B> = CombinatorSystem<AndThenMarker, A, B>;
/// Combines the outputs of two systems using the `||` operator.
pub type OrElse<A, B> = CombinatorSystem<OrElseMarker, A, B>;
#[doc(hidden)]
pub struct AndThenMarker;
impl<In, A, B> Combine<A, B> for AndThenMarker
where
In: Copy,
A: System<In = In, Out = bool>,
B: System<In = In, Out = bool>,
{
type In = In;
type Out = bool;
fn combine(
input: Self::In,
a: impl FnOnce(<A as System>::In) -> <A as System>::Out,
b: impl FnOnce(<B as System>::In) -> <B as System>::Out,
) -> Self::Out {
a(input) && b(input)
}
}
#[doc(hidden)]
pub struct OrElseMarker;
impl<In, A, B> Combine<A, B> for OrElseMarker
where
In: Copy,
A: System<In = In, Out = bool>,
B: System<In = In, Out = bool>,
{
type In = In;
type Out = bool;
fn combine(
input: Self::In,
a: impl FnOnce(<A as System>::In) -> <A as System>::Out,
b: impl FnOnce(<B as System>::In) -> <B as System>::Out,
) -> Self::Out {
a(input) || b(input)
}
}

View file

@ -0,0 +1,234 @@
use std::{borrow::Cow, cell::UnsafeCell, marker::PhantomData};
use bevy_ptr::UnsafeCellDeref;
use crate::{
archetype::ArchetypeComponentId, component::ComponentId, prelude::World, query::Access,
};
use super::{ReadOnlySystem, System};
/// Customizes the behavior of a [`CombinatorSystem`].
///
/// # Examples
///
/// ```
/// use bevy_ecs::prelude::*;
/// use bevy_ecs::system::{CombinatorSystem, Combine};
///
/// // A system combinator that performs an exclusive-or (XOR)
/// // operation on the output of two systems.
/// pub type Xor<A, B> = CombinatorSystem<XorMarker, A, B>;
///
/// // This struct is used to customize the behavior of our combinator.
/// pub struct XorMarker;
///
/// impl<A, B> Combine<A, B> for XorMarker
/// where
/// A: System<In = (), Out = bool>,
/// B: System<In = (), Out = bool>,
/// {
/// type In = ();
/// type Out = bool;
///
/// fn combine(
/// _input: Self::In,
/// a: impl FnOnce(A::In) -> A::Out,
/// b: impl FnOnce(B::In) -> B::Out,
/// ) -> Self::Out {
/// a(()) ^ b(())
/// }
/// }
///
/// # #[derive(Resource, PartialEq, Eq)] struct A(u32);
/// # #[derive(Resource, PartialEq, Eq)] struct B(u32);
/// # #[derive(Resource, Default)] struct RanFlag(bool);
/// # let mut world = World::new();
/// # world.init_resource::<RanFlag>();
/// #
/// # let mut app = Schedule::new();
/// app.add_system(my_system.run_if(Xor::new(
/// IntoSystem::into_system(resource_equals(A(1))),
/// IntoSystem::into_system(resource_equals(B(1))),
/// // The name of the combined system.
/// std::borrow::Cow::Borrowed("a ^ b"),
/// )));
/// # fn my_system(mut flag: ResMut<RanFlag>) { flag.0 = true; }
/// #
/// # world.insert_resource(A(0));
/// # world.insert_resource(B(0));
/// # app.run(&mut world);
/// # // Neither condition passes, so the system does not run.
/// # assert!(!world.resource::<RanFlag>().0);
/// #
/// # world.insert_resource(A(1));
/// # app.run(&mut world);
/// # // Only the first condition passes, so the system runs.
/// # assert!(world.resource::<RanFlag>().0);
/// # world.resource_mut::<RanFlag>().0 = false;
/// #
/// # world.insert_resource(B(1));
/// # app.run(&mut world);
/// # // Both conditions pass, so the system does not run.
/// # assert!(!world.resource::<RanFlag>().0);
/// #
/// # world.insert_resource(A(0));
/// # app.run(&mut world);
/// # // Only the second condition passes, so the system runs.
/// # assert!(world.resource::<RanFlag>().0);
/// # world.resource_mut::<RanFlag>().0 = false;
/// ```
pub trait Combine<A: System, B: System> {
/// The [input](System::In) type for a [`CombinatorSystem`].
type In;
/// The [output](System::Out) type for a [`CombinatorSystem`].
type Out;
/// When used in a [`CombinatorSystem`], this function customizes how
/// the two composite systems are invoked and their outputs are combined.
///
/// See the trait-level docs for [`Combine`] for an example implementation.
fn combine(
input: Self::In,
a: impl FnOnce(A::In) -> A::Out,
b: impl FnOnce(B::In) -> B::Out,
) -> Self::Out;
}
/// A [`System`] defined by combining two other systems.
/// The behavior of this combinator is specified by implementing the [`Combine`] trait.
/// For a full usage example, see the docs for [`Combine`].
pub struct CombinatorSystem<Func, A, B> {
_marker: PhantomData<fn() -> Func>,
a: A,
b: B,
name: Cow<'static, str>,
component_access: Access<ComponentId>,
archetype_component_access: Access<ArchetypeComponentId>,
}
impl<Func, A, B> CombinatorSystem<Func, A, B> {
pub const fn new(a: A, b: B, name: Cow<'static, str>) -> Self {
Self {
_marker: PhantomData,
a,
b,
name,
component_access: Access::new(),
archetype_component_access: Access::new(),
}
}
}
impl<A, B, Func> System for CombinatorSystem<Func, A, B>
where
Func: Combine<A, B> + 'static,
A: System,
B: 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) -> &Access<ComponentId> {
&self.component_access
}
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
&self.archetype_component_access
}
fn is_send(&self) -> bool {
self.a.is_send() && self.b.is_send()
}
fn is_exclusive(&self) -> bool {
self.a.is_exclusive() || self.b.is_exclusive()
}
unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out {
Func::combine(
input,
// SAFETY: The world accesses for both underlying systems have been registered,
// so the caller will guarantee that no other systems will conflict with `a` or `b`.
// Since these closures are `!Send + !Synd + !'static`, they can never be called
// in parallel, so their world accesses will not conflict with each other.
|input| self.a.run_unsafe(input, world),
|input| self.b.run_unsafe(input, world),
)
}
fn run<'w>(&mut self, input: Self::In, world: &'w mut World) -> Self::Out {
// SAFETY: Converting `&mut T` -> `&UnsafeCell<T>`
// is explicitly allowed in the docs for `UnsafeCell`.
let world: &'w UnsafeCell<World> = unsafe { std::mem::transmute(world) };
Func::combine(
input,
// SAFETY: Since these closures are `!Send + !Synd + !'static`, they can never
// be called in parallel. Since mutable access to `world` only exists within
// the scope of either closure, we can be sure they will never alias one another.
|input| self.a.run(input, unsafe { world.deref_mut() }),
#[allow(clippy::undocumented_unsafe_blocks)]
|input| self.b.run(input, unsafe { world.deref_mut() }),
)
}
fn apply_buffers(&mut self, world: &mut World) {
self.a.apply_buffers(world);
self.b.apply_buffers(world);
}
fn initialize(&mut self, world: &mut World) {
self.a.initialize(world);
self.b.initialize(world);
self.component_access.extend(self.a.component_access());
self.component_access.extend(self.b.component_access());
}
fn update_archetype_component_access(&mut self, world: &World) {
self.a.update_archetype_component_access(world);
self.b.update_archetype_component_access(world);
self.archetype_component_access
.extend(self.a.archetype_component_access());
self.archetype_component_access
.extend(self.b.archetype_component_access());
}
fn check_change_tick(&mut self, change_tick: u32) {
self.a.check_change_tick(change_tick);
self.b.check_change_tick(change_tick);
}
fn get_last_change_tick(&self) -> u32 {
self.a.get_last_change_tick()
}
fn set_last_change_tick(&mut self, last_change_tick: u32) {
self.a.set_last_change_tick(last_change_tick);
self.b.set_last_change_tick(last_change_tick);
}
fn default_system_sets(&self) -> Vec<Box<dyn crate::schedule::SystemSet>> {
let mut default_sets = self.a.default_system_sets();
default_sets.append(&mut self.b.default_system_sets());
default_sets
}
}
/// SAFETY: Both systems are read-only, so any system created by combining them will only read from the world.
unsafe impl<A, B, Func> ReadOnlySystem for CombinatorSystem<Func, A, B>
where
Func: Combine<A, B> + 'static,
A: ReadOnlySystem,
B: ReadOnlySystem,
{
}

View file

@ -98,6 +98,7 @@
//! - All tuples between 1 to 16 elements where each element implements [`SystemParam`]
//! - [`()` (unit primitive type)](https://doc.rust-lang.org/stable/std/primitive.unit.html)
mod combinator;
mod commands;
mod exclusive_function_system;
mod exclusive_system_param;
@ -108,6 +109,7 @@ mod system;
mod system_param;
mod system_piping;
pub use combinator::*;
pub use commands::*;
pub use exclusive_function_system::*;
pub use exclusive_system_param::*;

View file

@ -1,13 +1,7 @@
use crate::{
archetype::ArchetypeComponentId,
component::ComponentId,
query::Access,
system::{IntoSystem, System},
world::World,
};
use std::{any::TypeId, borrow::Cow};
use crate::system::{IntoSystem, System};
use std::borrow::Cow;
use super::ReadOnlySystem;
use super::{CombinatorSystem, Combine};
/// A [`System`] created by piping the output of the first system into the input of the second.
///
@ -48,120 +42,27 @@ use super::ReadOnlySystem;
/// result.ok().filter(|&n| n < 100)
/// }
/// ```
pub struct PipeSystem<SystemA, SystemB> {
system_a: SystemA,
system_b: SystemB,
name: Cow<'static, str>,
component_access: Access<ComponentId>,
archetype_component_access: Access<ArchetypeComponentId>,
}
pub type PipeSystem<SystemA, SystemB> = CombinatorSystem<Pipe, SystemA, SystemB>;
impl<SystemA, SystemB> PipeSystem<SystemA, SystemB> {
/// Manual constructor for creating a [`PipeSystem`].
/// This should only be used when [`IntoPipeSystem::pipe`] cannot be used,
/// such as in `const` contexts.
pub const fn new(system_a: SystemA, system_b: SystemB, name: Cow<'static, str>) -> Self {
Self {
system_a,
system_b,
name,
component_access: Access::new(),
archetype_component_access: Access::new(),
}
}
}
#[doc(hidden)]
pub struct Pipe;
impl<SystemA: System, SystemB: System<In = SystemA::Out>> System for PipeSystem<SystemA, SystemB> {
type In = SystemA::In;
type Out = SystemB::Out;
fn name(&self) -> Cow<'static, str> {
self.name.clone()
}
fn type_id(&self) -> TypeId {
TypeId::of::<(SystemA, SystemB)>()
}
fn archetype_component_access(&self) -> &Access<ArchetypeComponentId> {
&self.archetype_component_access
}
fn component_access(&self) -> &Access<ComponentId> {
&self.component_access
}
fn is_send(&self) -> bool {
self.system_a.is_send() && self.system_b.is_send()
}
fn is_exclusive(&self) -> bool {
self.system_a.is_exclusive() || self.system_b.is_exclusive()
}
unsafe fn run_unsafe(&mut self, input: Self::In, world: &World) -> Self::Out {
let out = self.system_a.run_unsafe(input, world);
self.system_b.run_unsafe(out, world)
}
// needed to make exclusive systems work
fn run(&mut self, input: Self::In, world: &mut World) -> Self::Out {
let out = self.system_a.run(input, world);
self.system_b.run(out, world)
}
fn apply_buffers(&mut self, world: &mut World) {
self.system_a.apply_buffers(world);
self.system_b.apply_buffers(world);
}
fn initialize(&mut self, world: &mut World) {
self.system_a.initialize(world);
self.system_b.initialize(world);
self.component_access
.extend(self.system_a.component_access());
self.component_access
.extend(self.system_b.component_access());
}
fn update_archetype_component_access(&mut self, world: &World) {
self.system_a.update_archetype_component_access(world);
self.system_b.update_archetype_component_access(world);
self.archetype_component_access
.extend(self.system_a.archetype_component_access());
self.archetype_component_access
.extend(self.system_b.archetype_component_access());
}
fn check_change_tick(&mut self, change_tick: u32) {
self.system_a.check_change_tick(change_tick);
self.system_b.check_change_tick(change_tick);
}
fn get_last_change_tick(&self) -> u32 {
self.system_a.get_last_change_tick()
}
fn set_last_change_tick(&mut self, last_change_tick: u32) {
self.system_a.set_last_change_tick(last_change_tick);
self.system_b.set_last_change_tick(last_change_tick);
}
fn default_system_sets(&self) -> Vec<Box<dyn crate::schedule::SystemSet>> {
let mut system_sets = self.system_a.default_system_sets();
system_sets.extend_from_slice(&self.system_b.default_system_sets());
system_sets
}
}
/// SAFETY: Both systems are read-only, so piping them together will only read from the world.
unsafe impl<SystemA: System, SystemB: System<In = SystemA::Out>> ReadOnlySystem
for PipeSystem<SystemA, SystemB>
impl<A, B> Combine<A, B> for Pipe
where
SystemA: ReadOnlySystem,
SystemB: ReadOnlySystem,
A: System,
B: System<In = A::Out>,
{
type In = A::In;
type Out = B::Out;
fn combine(
input: Self::In,
a: impl FnOnce(<A as System>::In) -> <A as System>::Out,
b: impl FnOnce(<B as System>::In) -> <B as System>::Out,
) -> Self::Out {
let value = a(input);
b(value)
}
}
/// An extension trait providing the [`IntoPipeSystem::pipe`] method to pass input from one system into the next.

View file

@ -16,23 +16,25 @@ fn main() {
// The common_conditions module has a few useful run conditions
// for checking resources and states. These are included in the prelude.
.run_if(resource_exists::<InputCounter>())
// This is our custom run condition. Both this and the
// above condition must be true for the system to run.
// This is a custom run condition, defined using a system that returns
// a `bool` and which has read-only `SystemParam`s.
// Both run conditions must return `true` in order for the system to run.
// Note that this second run condition will be evaluated even if the first returns `false`.
.run_if(has_user_input),
)
.add_system(
print_input_counter
// This is also a custom run condition but this time in the form of a closure.
// This is useful for small, simple run conditions you don't need to reuse.
// All the normal rules still apply: all parameters must be read only except for local parameters.
// In this case we will only run if the input counter resource exists and has changed but not just been added.
.run_if(|res: Option<Res<InputCounter>>| {
if let Some(counter) = res {
counter.is_changed() && !counter.is_added()
} else {
false
}
}),
// `.and_then()` is a run condition combinator that only evaluates the second condition
// if the first condition returns `true`. This behavior is known as "short-circuiting",
// and is how the `&&` operator works in Rust (as well as most C-family languages).
// In this case, the short-circuiting behavior prevents the second run condition from
// panicking if the `InputCounter` resource has not been initialized.
.run_if(resource_exists::<InputCounter>().and_then(
// This is a custom run condition in the form of a closure.
// This is useful for small, simple run conditions you don't need to reuse.
// All the normal rules still apply: all parameters must be read only except for local parameters.
|counter: Res<InputCounter>| counter.is_changed() && !counter.is_added(),
)),
)
.add_system(
print_time_message