diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 40fdc9eeeffe80..ed4a19c5301ed9 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,16 +78,17 @@ 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 { SystemId( self.spawn(RegisteredSystem { initialized: false, system: Box::new(IntoSystem::into_system(system)), }) .id(), + std::marker::PhantomData, ) } @@ -70,11 +98,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 { @@ -92,9 +123,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. /// @@ -142,6 +174,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) @@ -152,7 +222,7 @@ impl World { mut initialized, mut system, } = entity - .take::() + .take::>() .ok_or(RegisteredSystemError::Recursive(id))?; // run the system @@ -160,12 +230,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, }); @@ -198,19 +268,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 { @@ -240,14 +322,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)); } @@ -263,16 +345,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;