Allow piping run conditions (bevyengine#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 bevyengine#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`.

Author: joseph-gio

crates/bevy_ecs/src/schedule/condition.rs

@@ -11,15 +11,14 @@ pub trait Condition<Params>: sealed::Condition<Params> {}
 impl<Params, F> Condition<Params> for F where F: sealed::Condition<Params> {}
 
 mod sealed {
-    use crate::system::{IntoSystem, IsFunctionSystem, ReadOnlySystemParam, SystemParamFunction};
+    use crate::system::{IntoSystem, ReadOnlySystem};
 
     pub trait Condition<Params>: IntoSystem<(), bool, Params> {}
 
-    impl<Params, Marker, F> Condition<(IsFunctionSystem, Params, Marker)> for F
+    impl<Params, F> Condition<Params> for F
     where
-        F: SystemParamFunction<(), bool, Params, Marker> + Send + Sync + 'static,
-        Params: ReadOnlySystemParam + 'static,
-        Marker: 'static,
+        F: IntoSystem<(), bool, Params>,
+        F::System: ReadOnlySystem,
     {
     }
 }

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<In, Out, Param, Marker, F> ReadOnlySystem for FunctionSystem<In, Out, Param, Marker, F>
+where
+    In: 'static,
+    Out: 'static,
+    Param: ReadOnlySystemParam + 'static,
+    Marker: 'static,
+    F: SystemParamFunction<In, Out, Param, Marker> + 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,

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<In, Out, Params, S: IntoSystem<In, Out, Params>>(sys: S) {
     if false {
         // Check it can be converted into a system
@@ -130,6 +130,22 @@ pub fn assert_is_system<In, Out, Params, S: IntoSystem<In, Out, Params>>(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<In, Out, Params, S: IntoSystem<In, Out, Params>>(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;

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<In = (), Out = ()> = Box<dyn System<In = In, Out = Out>>;
 

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<SystemA: System, SystemB: System<In = SystemA::Out>> System for PipeSystem<
     }
 }
 
+/// 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>
+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>) -> bool {
+            !val
+        }
+
         assert_is_system(returning::<Result<u32, std::io::Error>>.pipe(unwrap));
         assert_is_system(returning::<Option<()>>.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::<bool>.pipe(exclusive_in_out::<bool, ()>));
+
+        returning::<()>.run_if(returning::<bool>.pipe(not));
     }
 }