From ba8c9b07eb6431b6ecd3d492ec742a8cd6d81853 Mon Sep 17 00:00:00 2001 From: JoJoJet <21144246+JoJoJet@users.noreply.github.com> Date: Tue, 7 Feb 2023 22:22:16 +0000 Subject: [PATCH] Allow piping run conditions (#7547) # Objective Run conditions are a special type of system that do not modify the world, and which return a bool. Due to the way they are currently implemented, you can *only* use bare function systems as a run condition. Among other things, this prevents the use of system piping with run conditions. This make very basic constructs impossible, such as `my_system.run_if(my_condition.pipe(not))`. Unblocks a basic solution for #7202. ## Solution Add the trait `ReadOnlySystem`, which is implemented for any system whose parameters all implement `ReadOnlySystemParam`. Allow any `-> bool` system implementing this trait to be used as a run condition. --- ## Changelog + Added the trait `ReadOnlySystem`, which is implemented for any `System` type whose parameters all implement `ReadOnlySystemParam`. + Added the function `bevy::ecs::system::assert_is_read_only_system`. --- crates/bevy_ecs/src/schedule/condition.rs | 9 ++++----- crates/bevy_ecs/src/system/function_system.rs | 13 ++++++++++++ crates/bevy_ecs/src/system/mod.rs | 20 +++++++++++++++++-- crates/bevy_ecs/src/system/system.rs | 10 ++++++++++ crates/bevy_ecs/src/system/system_piping.rs | 17 ++++++++++++++++ 5 files changed, 62 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/schedule/condition.rs b/crates/bevy_ecs/src/schedule/condition.rs index 7ee6c1723213d7..541ed202957b6c 100644 --- a/crates/bevy_ecs/src/schedule/condition.rs +++ b/crates/bevy_ecs/src/schedule/condition.rs @@ -11,15 +11,14 @@ pub trait Condition: sealed::Condition {} impl Condition for F where F: sealed::Condition {} mod sealed { - use crate::system::{IntoSystem, IsFunctionSystem, ReadOnlySystemParam, SystemParamFunction}; + use crate::system::{IntoSystem, ReadOnlySystem}; pub trait Condition: IntoSystem<(), bool, Params> {} - impl Condition<(IsFunctionSystem, Params, Marker)> for F + impl Condition for F where - F: SystemParamFunction<(), bool, Params, Marker> + Send + Sync + 'static, - Params: ReadOnlySystemParam + 'static, - Marker: 'static, + F: IntoSystem<(), bool, Params>, + F::System: ReadOnlySystem, { } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index b1e4c73e863325..a95a108bdc0666 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -10,6 +10,8 @@ use crate::{ use bevy_ecs_macros::all_tuples; use std::{any::TypeId, borrow::Cow, marker::PhantomData}; +use super::ReadOnlySystem; + /// The metadata of a [`System`]. #[derive(Clone)] pub struct SystemMeta { @@ -528,6 +530,17 @@ where } } +/// SAFETY: `F`'s param is `ReadOnlySystemParam`, so this system will only read from the world. +unsafe impl ReadOnlySystem for FunctionSystem +where + In: 'static, + Out: 'static, + Param: ReadOnlySystemParam + 'static, + Marker: 'static, + F: SystemParamFunction + Send + Sync + 'static, +{ +} + /// 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, diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 7b8922e79c72ad..dab5dd654d825c 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -117,11 +117,11 @@ pub use system::*; pub use system_param::*; pub use system_piping::*; -/// Ensure that a given function is a system +/// Ensure that a given function is a [system](System). /// /// This should be used when writing doc examples, /// to confirm that systems used in an example are -/// valid systems +/// valid systems. pub fn assert_is_system>(sys: S) { if false { // Check it can be converted into a system @@ -130,6 +130,22 @@ pub fn assert_is_system>(sys: S) } } +/// Ensure that a given function is a [read-only system](ReadOnlySystem). +/// +/// This should be used when writing doc examples, +/// to confirm that systems used in an example are +/// valid systems. +pub fn assert_is_read_only_system>(sys: S) +where + S::System: ReadOnlySystem, +{ + 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); + } +} + #[cfg(test)] mod tests { use std::any::TypeId; diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index dbbe58156834f1..78980e9063b812 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -78,6 +78,16 @@ pub trait System: Send + Sync + 'static { fn set_last_change_tick(&mut self, last_change_tick: u32); } +/// [`System`] types that do not modify the [`World`] when run. +/// This is implemented for any systems whose parameters all implement [`ReadOnlySystemParam`]. +/// +/// [`ReadOnlySystemParam`]: crate::system::ReadOnlySystemParam +/// +/// # Safety +/// +/// This must only be implemented for system types which do not mutate the `World`. +pub unsafe trait ReadOnlySystem: System {} + /// A convenience type alias for a boxed [`System`] trait object. pub type BoxedSystem = Box>; diff --git a/crates/bevy_ecs/src/system/system_piping.rs b/crates/bevy_ecs/src/system/system_piping.rs index 62eb057defc944..4787fccd469d06 100644 --- a/crates/bevy_ecs/src/system/system_piping.rs +++ b/crates/bevy_ecs/src/system/system_piping.rs @@ -7,6 +7,8 @@ use crate::{ }; use std::{any::TypeId, borrow::Cow}; +use super::ReadOnlySystem; + /// A [`System`] created by piping the output of the first system into the input of the second. /// /// This can be repeated indefinitely, but system pipes cannot branch: the output is consumed by the receiving system. @@ -153,6 +155,15 @@ impl> System for PipeSystem< } } +/// SAFETY: Both systems are read-only, so piping them together will only read from the world. +unsafe impl> ReadOnlySystem + for PipeSystem +where + SystemA: ReadOnlySystem, + SystemB: ReadOnlySystem, +{ +} + /// An extension trait providing the [`IntoPipeSystem::pipe`] method to pass input from one system into the next. /// /// The first system must have return type `T` @@ -412,10 +423,16 @@ pub mod adapter { unimplemented!() } + fn not(In(val): In) -> bool { + !val + } + assert_is_system(returning::>.pipe(unwrap)); assert_is_system(returning::>.pipe(ignore)); assert_is_system(returning::<&str>.pipe(new(u64::from_str)).pipe(unwrap)); assert_is_system(exclusive_in_out::<(), Result<(), std::io::Error>>.pipe(error)); assert_is_system(returning::.pipe(exclusive_in_out::)); + + returning::<()>.run_if(returning::.pipe(not)); } }