From d1c6fbea570be64f8e24e943ee5a3cef221794e0 Mon Sep 17 00:00:00 2001 From: Alejandro Pascual Date: Tue, 17 Jun 2025 21:48:37 +0200 Subject: [PATCH] Support fallible one-shot systems (#19678) Closes #19677. I don't think that the output type needs to be `Send`. I've done some test at it seems to work fine without it, which in IMO makes sense, but please correct me if that is not the case. --- .../bevy_ecs/src/system/commands/command.rs | 15 ++++-- crates/bevy_ecs/src/system/commands/mod.rs | 16 +++--- crates/bevy_ecs/src/system/system_registry.rs | 54 +++++++++++++++++-- 3 files changed, 70 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 84a2fdf4e9..f3fc677e47 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -144,10 +144,11 @@ where /// A [`Command`] that runs the given system, /// caching its [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) resource. -pub fn run_system_cached(system: S) -> impl Command +pub fn run_system_cached(system: S) -> impl Command where + O: 'static, M: 'static, - S: IntoSystem<(), (), M> + Send + 'static, + S: IntoSystem<(), O, M> + Send + 'static, { move |world: &mut World| -> Result { world.run_system_cached(system)?; @@ -157,11 +158,15 @@ where /// A [`Command`] that runs the given system with the given input value, /// caching its [`SystemId`] in a [`CachedSystemId`](crate::system::CachedSystemId) resource. -pub fn run_system_cached_with(system: S, input: I::Inner<'static>) -> impl Command +pub fn run_system_cached_with( + system: S, + input: I::Inner<'static>, +) -> impl Command where I: SystemInput: Send> + Send + 'static, + O: 'static, M: 'static, - S: IntoSystem + Send + 'static, + S: IntoSystem + Send + 'static, { move |world: &mut World| -> Result { world.run_system_cached_with(system, input)?; @@ -175,7 +180,7 @@ where pub fn unregister_system(system_id: SystemId) -> impl Command where I: SystemInput + Send + 'static, - O: Send + 'static, + O: 'static, { move |world: &mut World| -> Result { world.unregister_system(system_id)?; diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d36588d377..84f9784228 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -872,7 +872,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// It will internally return a [`RegisteredSystemError`](crate::system::system_registry::RegisteredSystemError), /// which will be handled by [logging the error at the `warn` level](warn). - pub fn run_system(&mut self, id: SystemId) { + pub fn run_system(&mut self, id: SystemId<(), O>) { self.queue(command::run_system(id).handle_error_with(warn)); } @@ -965,7 +965,7 @@ impl<'w, 's> Commands<'w, 's> { ) -> SystemId where I: SystemInput + Send + 'static, - O: Send + 'static, + O: 'static, { let entity = self.spawn_empty().id(); let system = RegisteredSystem::::new(Box::new(IntoSystem::into_system(system))); @@ -990,7 +990,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn unregister_system(&mut self, system_id: SystemId) where I: SystemInput + Send + 'static, - O: Send + 'static, + O: 'static, { self.queue(command::unregister_system(system_id).handle_error_with(warn)); } @@ -1039,10 +1039,11 @@ impl<'w, 's> Commands<'w, 's> { /// consider passing them in as inputs via [`Commands::run_system_cached_with`]. /// /// If that's not an option, consider [`Commands::register_system`] instead. - pub fn run_system_cached(&mut self, system: S) + pub fn run_system_cached(&mut self, system: S) where + O: 'static, M: 'static, - S: IntoSystem<(), (), M> + Send + 'static, + S: IntoSystem<(), O, M> + Send + 'static, { self.queue(command::run_system_cached(system).handle_error_with(warn)); } @@ -1069,11 +1070,12 @@ impl<'w, 's> Commands<'w, 's> { /// consider passing them in as inputs. /// /// If that's not an option, consider [`Commands::register_system`] instead. - pub fn run_system_cached_with(&mut self, system: S, input: I::Inner<'static>) + pub fn run_system_cached_with(&mut self, system: S, input: I::Inner<'static>) where I: SystemInput: Send> + Send + 'static, + O: 'static, M: 'static, - S: IntoSystem + Send + 'static, + S: IntoSystem + Send + 'static, { self.queue(command::run_system_cached_with(system, input).handle_error_with(warn)); } diff --git a/crates/bevy_ecs/src/system/system_registry.rs b/crates/bevy_ecs/src/system/system_registry.rs index 272cc85d0d..6889a8f4cd 100644 --- a/crates/bevy_ecs/src/system/system_registry.rs +++ b/crates/bevy_ecs/src/system/system_registry.rs @@ -651,6 +651,19 @@ mod tests { assert_eq!(output, NonCopy(3)); } + #[test] + fn fallible_system() { + fn sys() -> Result<()> { + Err("error")?; + Ok(()) + } + + let mut world = World::new(); + let fallible_system_id = world.register_system(sys); + let output = world.run_system(fallible_system_id); + assert!(matches!(output, Ok(Err(_)))); + } + #[test] fn exclusive_system() { let mut world = World::new(); @@ -751,19 +764,54 @@ mod tests { assert!(matches!(output, Ok(x) if x == four())); } + #[test] + fn cached_fallible_system() { + fn sys() -> Result<()> { + Err("error")?; + Ok(()) + } + + let mut world = World::new(); + let fallible_system_id = world.register_system_cached(sys); + let output = world.run_system(fallible_system_id); + assert!(matches!(output, Ok(Err(_)))); + let output = world.run_system_cached(sys); + assert!(matches!(output, Ok(Err(_)))); + let output = world.run_system_cached_with(sys, ()); + assert!(matches!(output, Ok(Err(_)))); + } + #[test] fn cached_system_commands() { fn sys(mut counter: ResMut) { - counter.0 = 1; + counter.0 += 1; } let mut world = World::new(); world.insert_resource(Counter(0)); - world.commands().run_system_cached(sys); world.flush_commands(); - assert_eq!(world.resource::().0, 1); + world.commands().run_system_cached_with(sys, ()); + world.flush_commands(); + assert_eq!(world.resource::().0, 2); + } + + #[test] + fn cached_fallible_system_commands() { + fn sys(mut counter: ResMut) -> Result { + counter.0 += 1; + Ok(()) + } + + let mut world = World::new(); + world.insert_resource(Counter(0)); + world.commands().run_system_cached(sys); + world.flush_commands(); + assert_eq!(world.resource::().0, 1); + world.commands().run_system_cached_with(sys, ()); + world.flush_commands(); + assert_eq!(world.resource::().0, 2); } #[test]