From f8df323103d56926962afd09cdd26dd246a45a83 Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Sat, 4 Nov 2023 21:48:22 -0700 Subject: [PATCH 1/6] Add 'World::run_system_with_input' function --- crates/bevy_ecs/src/system/system_registry.rs | 174 +++++++++++++++--- 1 file changed, 147 insertions(+), 27 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 05a01719fcd2c..bd87aa64b190c 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -7,18 +7,18 @@ use thiserror::Error; /// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized. #[derive(Component)] -struct RegisteredSystem { +struct RegisteredSystem { initialized: bool, - system: BoxedSystem, + system: BoxedSystem, } /// A system that has been removed from the registry. /// It contains the system and whether or not it has been initialized. /// /// This struct is returned by [`World::remove_system`]. -pub struct RemovedSystem { +pub struct RemovedSystem { initialized: bool, - system: BoxedSystem, + system: BoxedSystem, } impl RemovedSystem { @@ -38,8 +38,35 @@ impl RemovedSystem { /// /// These are opaque identifiers, keyed to a specific [`World`], /// and are created via [`World::register_system`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct SystemId(Entity); +#[derive(Eq)] +pub struct SystemId(Entity, std::marker::PhantomData); + +// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. +impl Copy for SystemId {} +// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. +impl Clone for SystemId { + fn clone(&self) -> Self { + *self + } +} +// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. +impl PartialEq for SystemId { + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 && self.1 == other.1 + } +} +// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. +impl std::hash::Hash for SystemId { + fn hash(&self, state: &mut H) { + self.0.hash(state); + } +} +impl std::fmt::Debug for SystemId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // The PhantomData field is omitted for simplicity. + f.debug_tuple("SystemId").field(&self.0).finish() + } +} impl World { /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. @@ -51,10 +78,10 @@ impl World { /// This allows for running systems in a pushed-based fashion. /// Using a [`Schedule`](crate::schedule::Schedule) is still preferred for most cases /// due to its better performance and abillity to run non-conflicting systems simultaneously. - pub fn register_system + 'static>( + pub fn register_system + 'static>( &mut self, system: S, - ) -> SystemId { + ) -> SystemId { self.register_boxed_system(Box::new(IntoSystem::into_system(system))) } @@ -62,13 +89,14 @@ impl World { /// /// This is useful if the [`IntoSystem`] implementor has already been turned into a /// [`System`](crate::system::System) trait object and put in a [`Box`]. - pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemId { + pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemId { SystemId( self.spawn(RegisteredSystem { initialized: false, system, }) .id(), + std::marker::PhantomData, ) } @@ -78,11 +106,14 @@ impl World { /// /// If no system corresponds to the given [`SystemId`], this method returns an error. /// Systems are also not allowed to remove themselves, this returns an error too. - pub fn remove_system(&mut self, id: SystemId) -> Result { + pub fn remove_system( + &mut self, + id: SystemId, + ) -> Result, RegisteredSystemError> { match self.get_entity_mut(id.0) { Some(mut entity) => { let registered_system = entity - .take::() + .take::>() .ok_or(RegisteredSystemError::SelfRemove(id))?; entity.despawn(); Ok(RemovedSystem { @@ -100,9 +131,10 @@ impl World { /// This is different from [`RunSystemOnce::run_system_once`](crate::system::RunSystemOnce::run_system_once), /// because it keeps local state between calls and change detection works correctly. /// + /// In order to run a chained system with an input, use [`World::run_system_with_input`] instead. + /// /// # Limitations /// - /// - Stored systems cannot be chained: they can neither have an [`In`](crate::system::In) nor return any values. /// - Stored systems cannot be recursive, they cannot call themselves through [`Commands::run_system`](crate::system::Commands). /// - Exclusive systems cannot be used. /// @@ -150,6 +182,44 @@ impl World { /// let _ = world.run_system(detector); // -> Something happened! /// ``` pub fn run_system(&mut self, id: SystemId) -> Result<(), RegisteredSystemError> { + self.run_system_with_input(id, ()) + } + + /// Run a stored chained system by its [`SystemId`], providing an input value. + /// Before running a system, it must first be registered. + /// The method [`World::register_system`] stores a given system and returns a [`SystemId`]. + /// + /// # Limitations + /// + /// - Stored systems cannot be recursive, they cannot call themselves through [`Commands::run_system`](crate::system::Commands). + /// - Exclusive systems cannot be used. + /// + /// # Examples + /// + /// ```rust + /// # use bevy_ecs::prelude::*; + /// #[derive(Resource, Default)] + /// struct Counter(u8); + /// + /// fn increment(In(increment_by): In, mut counter: Local) { + /// counter.0 += increment_by; + /// println!("{}", counter.0); + /// } + /// + /// let mut world = World::default(); + /// let counter_one = world.register_system(increment); + /// let counter_two = world.register_system(increment); + /// world.run_system_with_input(counter_one, 1); // -> 1 + /// world.run_system_with_input(counter_one, 20); // -> 21 + /// world.run_system_with_input(counter_two, 30); // -> 51 + /// ``` + /// + /// See [`World::run_system`] for more examples. + pub fn run_system_with_input( + &mut self, + id: SystemId, + input: I, + ) -> Result<(), RegisteredSystemError> { // lookup let mut entity = self .get_entity_mut(id.0) @@ -160,7 +230,7 @@ impl World { mut initialized, mut system, } = entity - .take::() + .take::>() .ok_or(RegisteredSystemError::Recursive(id))?; // run the system @@ -168,12 +238,12 @@ impl World { system.initialize(self); initialized = true; } - system.run((), self); + system.run(input, self); system.apply_deferred(self); // return ownership of system trait object (if entity still exists) if let Some(mut entity) = self.get_entity_mut(id.0) { - entity.insert::(RegisteredSystem { + entity.insert::>(RegisteredSystem { initialized, system, }); @@ -206,19 +276,31 @@ impl Command for RunSystem { } /// An operation with stored systems failed. -#[derive(Debug, Error)] -pub enum RegisteredSystemError { +#[derive(Error)] +pub enum RegisteredSystemError { /// A system was run by id, but no system with that id was found. /// /// Did you forget to register it? #[error("System {0:?} was not registered")] - SystemIdNotRegistered(SystemId), + SystemIdNotRegistered(SystemId), /// A system tried to run itself recursively. #[error("System {0:?} tried to run itself recursively")] - Recursive(SystemId), + Recursive(SystemId), /// A system tried to remove itself. #[error("System {0:?} tried to remove itself")] - SelfRemove(SystemId), + SelfRemove(SystemId), +} + +impl std::fmt::Debug for RegisteredSystemError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::SystemIdNotRegistered(arg0) => { + f.debug_tuple("SystemIdNotRegistered").field(arg0).finish() + } + Self::Recursive(arg0) => f.debug_tuple("Recursive").field(arg0).finish(), + Self::SelfRemove(arg0) => f.debug_tuple("SelfRemove").field(arg0).finish(), + } + } } mod tests { @@ -248,14 +330,14 @@ mod tests { assert_eq!(*world.resource::(), Counter(0)); // Resources are changed when they are first added. let id = world.register_system(count_up_iff_changed); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); // Nothing changed - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); // Making a change world.resource_mut::().set_changed(); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); } @@ -271,16 +353,54 @@ mod tests { world.insert_resource(Counter(1)); assert_eq!(*world.resource::(), Counter(1)); let id = world.register_system(doubling); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(1)); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(2)); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(4)); - let _ = world.run_system(id); + world.run_system(id).expect("system runs successfully"); assert_eq!(*world.resource::(), Counter(8)); } + #[test] + fn input_values() { + // Verify that a non-Copy, non-Clone type can be passed in. + struct NonCopy(u8); + + fn increment_sys(In(NonCopy(increment_by)): In, mut counter: ResMut) { + counter.0 += increment_by; + } + + let mut world = World::new(); + + let id = world.register_system(increment_sys); + + // Insert the resource after registering the system. + world.insert_resource(Counter(1)); + assert_eq!(*world.resource::(), Counter(1)); + + world + .run_system_with_input(id, NonCopy(1)) + .expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(2)); + + world + .run_system_with_input(id, NonCopy(1)) + .expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(3)); + + world + .run_system_with_input(id, NonCopy(20)) + .expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(23)); + + world + .run_system_with_input(id, NonCopy(1)) + .expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(24)); + } + #[test] fn nested_systems() { use crate::system::SystemId; From 6346ccdfc0c7bb8ef1283184683843d53f25c15e Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Sun, 5 Nov 2023 22:38:55 -0800 Subject: [PATCH 2/6] allow 'run_system' to get system output --- crates/bevy_ecs/src/system/system_registry.rs | 139 +++++++++++++----- 1 file changed, 104 insertions(+), 35 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index bd87aa64b190c..fb2fa9a875eed 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -7,18 +7,18 @@ use thiserror::Error; /// A small wrapper for [`BoxedSystem`] that also keeps track whether or not the system has been initialized. #[derive(Component)] -struct RegisteredSystem { +struct RegisteredSystem { initialized: bool, - system: BoxedSystem, + system: BoxedSystem, } /// A system that has been removed from the registry. /// It contains the system and whether or not it has been initialized. /// /// This struct is returned by [`World::remove_system`]. -pub struct RemovedSystem { +pub struct RemovedSystem { initialized: bool, - system: BoxedSystem, + system: BoxedSystem, } impl RemovedSystem { @@ -39,29 +39,29 @@ impl RemovedSystem { /// These are opaque identifiers, keyed to a specific [`World`], /// and are created via [`World::register_system`]. #[derive(Eq)] -pub struct SystemId(Entity, std::marker::PhantomData); +pub struct SystemId(Entity, std::marker::PhantomData O>); -// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. -impl Copy for SystemId {} -// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. -impl Clone for SystemId { +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl Copy for SystemId {} +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl Clone for SystemId { fn clone(&self) -> Self { *self } } -// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. -impl PartialEq for SystemId { +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl PartialEq for SystemId { fn eq(&self, other: &Self) -> bool { self.0 == other.0 && self.1 == other.1 } } -// A manual impl is used because the trait bounds should ignore the `I` phantom parameter. -impl std::hash::Hash for SystemId { +// A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. +impl std::hash::Hash for SystemId { fn hash(&self, state: &mut H) { self.0.hash(state); } } -impl std::fmt::Debug for SystemId { +impl std::fmt::Debug for SystemId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { // The PhantomData field is omitted for simplicity. f.debug_tuple("SystemId").field(&self.0).finish() @@ -78,10 +78,10 @@ impl World { /// This allows for running systems in a pushed-based fashion. /// Using a [`Schedule`](crate::schedule::Schedule) is still preferred for most cases /// due to its better performance and abillity to run non-conflicting systems simultaneously. - pub fn register_system + 'static>( + pub fn register_system + 'static>( &mut self, system: S, - ) -> SystemId { + ) -> SystemId { self.register_boxed_system(Box::new(IntoSystem::into_system(system))) } @@ -89,7 +89,10 @@ impl World { /// /// This is useful if the [`IntoSystem`] implementor has already been turned into a /// [`System`](crate::system::System) trait object and put in a [`Box`]. - pub fn register_boxed_system(&mut self, system: BoxedSystem) -> SystemId { + pub fn register_boxed_system( + &mut self, + system: BoxedSystem, + ) -> SystemId { SystemId( self.spawn(RegisteredSystem { initialized: false, @@ -106,14 +109,14 @@ impl World { /// /// If no system corresponds to the given [`SystemId`], this method returns an error. /// Systems are also not allowed to remove themselves, this returns an error too. - pub fn remove_system( + pub fn remove_system( &mut self, - id: SystemId, - ) -> Result, RegisteredSystemError> { + id: SystemId, + ) -> Result, RegisteredSystemError> { match self.get_entity_mut(id.0) { Some(mut entity) => { let registered_system = entity - .take::>() + .take::>() .ok_or(RegisteredSystemError::SelfRemove(id))?; entity.despawn(); Ok(RemovedSystem { @@ -140,6 +143,8 @@ impl World { /// /// # Examples /// + /// ## Running a system + /// /// ```rust /// # use bevy_ecs::prelude::*; /// #[derive(Resource, Default)] @@ -158,7 +163,7 @@ impl World { /// world.run_system(counter_two); // -> 1 /// ``` /// - /// Change detection: + /// ## Change detection /// /// ```rust /// # use bevy_ecs::prelude::*; @@ -181,7 +186,43 @@ impl World { /// world.resource_mut::().set_changed(); /// let _ = world.run_system(detector); // -> Something happened! /// ``` - pub fn run_system(&mut self, id: SystemId) -> Result<(), RegisteredSystemError> { + /// + /// ## Getting system output + /// + /// ```rust + /// # use bevy_ecs::prelude::*; + /// + /// #[derive(Resource)] + /// struct PlayerScore(i32); + /// + /// #[derive(Resource)] + /// struct OpponentScore(i32); + /// + /// fn get_player_score(player_score: Res) -> i32 { + /// player_score.0 + /// } + /// + /// fn get_opponent_score(opponent_score: Res) -> i32 { + /// opponent_score.0 + /// } + /// + /// let mut world = World::default(); + /// world.insert_resource(PlayerScore(3)); + /// world.insert_resource(OpponentScore(2)); + /// + /// let scoring_systems = [ + /// ("player", world.register_system(get_player_score)), + /// ("opponent", world.register_system(get_opponent_score)), + /// ]; + /// + /// for (label, scoring_system) in scoring_systems { + /// println!("{label} has score {}", world.run_system(scoring_system).expect("system succeeded")); + /// } + /// ``` + pub fn run_system( + &mut self, + id: SystemId<(), O>, + ) -> Result> { self.run_system_with_input(id, ()) } @@ -215,11 +256,11 @@ impl World { /// ``` /// /// See [`World::run_system`] for more examples. - pub fn run_system_with_input( + pub fn run_system_with_input( &mut self, - id: SystemId, + id: SystemId, input: I, - ) -> Result<(), RegisteredSystemError> { + ) -> Result> { // lookup let mut entity = self .get_entity_mut(id.0) @@ -230,7 +271,7 @@ impl World { mut initialized, mut system, } = entity - .take::>() + .take::>() .ok_or(RegisteredSystemError::Recursive(id))?; // run the system @@ -238,17 +279,17 @@ impl World { system.initialize(self); initialized = true; } - system.run(input, self); + let result = system.run(input, self); system.apply_deferred(self); // return ownership of system trait object (if entity still exists) if let Some(mut entity) = self.get_entity_mut(id.0) { - entity.insert::>(RegisteredSystem { + entity.insert::>(RegisteredSystem { initialized, system, }); } - Ok(()) + Ok(result) } } @@ -277,21 +318,21 @@ impl Command for RunSystem { /// An operation with stored systems failed. #[derive(Error)] -pub enum RegisteredSystemError { +pub enum RegisteredSystemError { /// A system was run by id, but no system with that id was found. /// /// Did you forget to register it? #[error("System {0:?} was not registered")] - SystemIdNotRegistered(SystemId), + SystemIdNotRegistered(SystemId), /// A system tried to run itself recursively. #[error("System {0:?} tried to run itself recursively")] - Recursive(SystemId), + Recursive(SystemId), /// A system tried to remove itself. #[error("System {0:?} tried to remove itself")] - SelfRemove(SystemId), + SelfRemove(SystemId), } -impl std::fmt::Debug for RegisteredSystemError { +impl std::fmt::Debug for RegisteredSystemError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::SystemIdNotRegistered(arg0) => { @@ -401,6 +442,34 @@ mod tests { assert_eq!(*world.resource::(), Counter(24)); } + #[test] + fn output_values() { + // Verify that a non-Copy, non-Clone type can be returned. + #[derive(Eq, PartialEq, Debug)] + struct NonCopy(u8); + + fn increment_sys(mut counter: ResMut) -> NonCopy { + counter.0 += 1; + NonCopy(counter.0) + } + + let mut world = World::new(); + + let id = world.register_system(increment_sys); + + // Insert the resource after registering the system. + world.insert_resource(Counter(1)); + assert_eq!(*world.resource::(), Counter(1)); + + let output = world.run_system(id).expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(2)); + assert_eq!(output, NonCopy(2)); + + let output = world.run_system(id).expect("system runs successfully"); + assert_eq!(*world.resource::(), Counter(3)); + assert_eq!(output, NonCopy(3)); + } + #[test] fn nested_systems() { use crate::system::SystemId; From 7f68e79bceec1077090a33dec4cbbf54842b1477 Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Sun, 12 Nov 2023 14:40:12 -0800 Subject: [PATCH 3/6] include phantom data in SystemId debug --- crates/bevy_ecs/src/system/system_registry.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index fb2fa9a875eed..99262271e9e17 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -43,28 +43,34 @@ pub struct SystemId(Entity, std::marker::PhantomData O> // A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. impl Copy for SystemId {} + // A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. impl Clone for SystemId { fn clone(&self) -> Self { *self } } + // A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. impl PartialEq for SystemId { fn eq(&self, other: &Self) -> bool { self.0 == other.0 && self.1 == other.1 } } + // A manual impl is used because the trait bounds should ignore the `I` and `O` phantom parameters. impl std::hash::Hash for SystemId { fn hash(&self, state: &mut H) { self.0.hash(state); } } + impl std::fmt::Debug for SystemId { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // The PhantomData field is omitted for simplicity. - f.debug_tuple("SystemId").field(&self.0).finish() + f.debug_tuple("SystemId") + .field(&self.0) + .field(&self.1) + .finish() } } From 90eaaa93b29614a2e76f5a542ec7429ec79b3e0b Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Sun, 12 Nov 2023 15:04:23 -0800 Subject: [PATCH 4/6] support the run_system_with_input Command --- crates/bevy_ecs/src/system/commands/mod.rs | 14 +++- crates/bevy_ecs/src/system/system_registry.rs | 69 +++++++++++++++++-- 2 files changed, 75 insertions(+), 8 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 89c8493dc667b..304ccf3bbe2e0 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -5,7 +5,7 @@ use crate::{ self as bevy_ecs, bundle::Bundle, entity::{Entities, Entity}, - system::{RunSystem, SystemId}, + system::{RunSystemWithInput, SystemId}, world::{EntityWorldMut, FromWorld, World}, }; use bevy_ecs_macros::SystemParam; @@ -524,7 +524,17 @@ impl<'w, 's> Commands<'w, 's> { /// /// Calls [`World::run_system`](crate::system::World::run_system). pub fn run_system(&mut self, id: SystemId) { - self.queue.push(RunSystem::new(id)); + self.run_system_with_input(id, ()) + } + + /// Runs the system corresponding to the given [`SystemId`]. + /// Systems are ran in an exclusive and single threaded way. + /// Running slow systems can become a bottleneck. + /// + /// Calls [`World::run_system_with_input`](crate::system::World::run_system_with_input). + pub fn run_system_with_input(&mut self, id: SystemId, input: I) { + self.queue + .push(RunSystemWithInput::new_with_input(id, input)); } /// Pushes a generic [`Command`] to the command queue. diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 99262271e9e17..92a8708f138ce 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -299,26 +299,55 @@ impl World { } } -/// The [`Command`] type for [`World::run_system`]. +/// The [`Command`] type for [`World::run_system`] or [`World::run_system_with_input`]. /// /// This command runs systems in an exclusive and single threaded way. /// Running slow systems can become a bottleneck. +/// +/// If the system needs an [`In<_>`](crate::system::In) input value to run, it must +/// be provided as part of the command. +/// +/// There is no way to get the output of a system when run as a command, because the +/// execution of the system happens later. To get the output of a system, use +/// [World::run_system] or [World::run_system_with_input] instead of running the system as a command. #[derive(Debug, Clone)] -pub struct RunSystem { - system_id: SystemId, +pub struct RunSystemWithInput { + system_id: SystemId, + input: I, } +/// The [`Command`] type for [`World::run_system`]. +/// +/// This command runs systems in an exclusive and single threaded way. +/// Running slow systems can become a bottleneck. +/// +/// If the system needs an [`In<_>`](crate::system::In) input value to run, use the +/// [crate::system::RunSystemWithInput] type instead. +/// +/// There is no way to get the output of a system when run as a command, because the +/// execution of the system happens later. To get the output of a system, use +/// [World::run_system] or [World::run_system_with_input] instead of running the system as a command. +pub type RunSystem = RunSystemWithInput<()>; + impl RunSystem { /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands) pub fn new(system_id: SystemId) -> Self { - Self { system_id } + Self::new_with_input(system_id, ()) + } +} + +impl RunSystemWithInput { + /// Creates a new [`Command`] struct, which can be added to [`Commands`](crate::system::Commands) + /// in order to run the specified system with the provided [`In<_>`](crate::system::In) input value. + pub fn new_with_input(system_id: SystemId, input: I) -> Self { + Self { system_id, input } } } -impl Command for RunSystem { +impl Command for RunSystemWithInput { #[inline] fn apply(self, world: &mut World) { - let _ = world.run_system(self.system_id); + let _ = world.run_system_with_input(self.system_id, self.input); } } @@ -505,4 +534,32 @@ mod tests { let _ = world.run_system(nested_id); assert_eq!(*world.resource::(), Counter(5)); } + + #[test] + fn nested_systems_with_inputs() { + use crate::system::SystemId; + + #[derive(Component)] + struct Callback(SystemId, u8); + + fn nested(query: Query<&Callback>, mut commands: Commands) { + for callback in query.iter() { + commands.run_system_with_input(callback.0, callback.1); + } + } + + let mut world = World::new(); + world.insert_resource(Counter(0)); + + let increment_by = + world.register_system(|In(amt): In, mut counter: ResMut| { + counter.0 += amt; + }); + let nested_id = world.register_system(nested); + + world.spawn(Callback(increment_by, 2)); + world.spawn(Callback(increment_by, 3)); + let _ = world.run_system(nested_id); + assert_eq!(*world.resource::(), Counter(5)); + } } From dc4b380f91c71f3503099fc09369dedfb86e1295 Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Sun, 12 Nov 2023 15:11:15 -0800 Subject: [PATCH 5/6] fix up docs --- crates/bevy_ecs/src/system/commands/mod.rs | 10 +++++++++- crates/bevy_ecs/src/system/system_registry.rs | 4 ++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 304ccf3bbe2e0..15dcafead34c2 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -523,8 +523,12 @@ impl<'w, 's> Commands<'w, 's> { /// Running slow systems can become a bottleneck. /// /// Calls [`World::run_system`](crate::system::World::run_system). + /// + /// There is no way to get the output of a system when run as a command, because the + /// execution of the system happens later. To get the output of a system, use + /// [`World::run_system`] or [`World::run_system_with_input`] instead of running the system as a command. pub fn run_system(&mut self, id: SystemId) { - self.run_system_with_input(id, ()) + self.run_system_with_input(id, ()); } /// Runs the system corresponding to the given [`SystemId`]. @@ -532,6 +536,10 @@ impl<'w, 's> Commands<'w, 's> { /// Running slow systems can become a bottleneck. /// /// Calls [`World::run_system_with_input`](crate::system::World::run_system_with_input). + /// + /// There is no way to get the output of a system when run as a command, because the + /// execution of the system happens later. To get the output of a system, use + /// [`World::run_system`] or [`World::run_system_with_input`] instead of running the system as a command. pub fn run_system_with_input(&mut self, id: SystemId, input: I) { self.queue .push(RunSystemWithInput::new_with_input(id, input)); diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 92a8708f138ce..5510abbb7a815 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -309,7 +309,7 @@ impl World { /// /// There is no way to get the output of a system when run as a command, because the /// execution of the system happens later. To get the output of a system, use -/// [World::run_system] or [World::run_system_with_input] instead of running the system as a command. +/// [`World::run_system`] or [`World::run_system_with_input`] instead of running the system as a command. #[derive(Debug, Clone)] pub struct RunSystemWithInput { system_id: SystemId, @@ -326,7 +326,7 @@ pub struct RunSystemWithInput { /// /// There is no way to get the output of a system when run as a command, because the /// execution of the system happens later. To get the output of a system, use -/// [World::run_system] or [World::run_system_with_input] instead of running the system as a command. +/// [`World::run_system`] or [`World::run_system_with_input`] instead of running the system as a command. pub type RunSystem = RunSystemWithInput<()>; impl RunSystem { From 53ff88d7ca722f1221c160a0fcb23902cb520b8e Mon Sep 17 00:00:00 2001 From: Nathan Fenner Date: Tue, 14 Nov 2023 18:58:25 -0800 Subject: [PATCH 6/6] fix doc comment --- crates/bevy_ecs/src/system/system_registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 5510abbb7a815..21a9f46a84d28 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -322,7 +322,7 @@ pub struct RunSystemWithInput { /// Running slow systems can become a bottleneck. /// /// If the system needs an [`In<_>`](crate::system::In) input value to run, use the -/// [crate::system::RunSystemWithInput] type instead. +/// [`crate::system::RunSystemWithInput`] type instead. /// /// There is no way to get the output of a system when run as a command, because the /// execution of the system happens later. To get the output of a system, use