From 7d8504f30eb9504f2f841e04b2159b32fb01ade5 Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Tue, 11 Feb 2025 23:15:43 +0100 Subject: [PATCH] feat(ecs): implement fallible observer systems (#17731) This commit builds on top of the work done in #16589 and #17051, by adding support for fallible observer systems. As with the previous work, the actual results of the observer system are suppressed for now, but the intention is to provide a way to handle errors in a global way. Until then, you can use a `PipeSystem` to manually handle results. --------- Signed-off-by: Jean Mertz --- Cargo.toml | 1 + crates/bevy_ecs/src/observer/runner.rs | 76 +++++++- crates/bevy_ecs/src/system/observer_system.rs | 166 ++++++++++++++++-- examples/ecs/fallible_systems.rs | 35 +++- 4 files changed, 259 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9db0b57510..c7f5c67889 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2165,6 +2165,7 @@ name = "Fallible Systems" description = "Systems that return results to handle errors" category = "ECS (Entity Component System)" wasm = false +required-features = ["bevy_mesh_picking_backend"] [[example]] name = "startup_system" diff --git a/crates/bevy_ecs/src/observer/runner.rs b/crates/bevy_ecs/src/observer/runner.rs index 74b6519579..fdb3b4fa80 100644 --- a/crates/bevy_ecs/src/observer/runner.rs +++ b/crates/bevy_ecs/src/observer/runner.rs @@ -6,6 +6,7 @@ use crate::{ observer::{ObserverDescriptor, ObserverTrigger}, prelude::*, query::DebugCheckedUnwrap, + result::{DefaultSystemErrorHandler, SystemErrorContext}, system::{IntoObserverSystem, ObserverSystem}, world::DeferredWorld, }; @@ -272,6 +273,7 @@ pub struct Observer { system: Box, descriptor: ObserverDescriptor, hook_on_add: ComponentHook, + error_handler: Option, } impl Observer { @@ -282,6 +284,7 @@ impl Observer { system: Box::new(IntoObserverSystem::into_system(system)), descriptor: Default::default(), hook_on_add: hook_on_add::, + error_handler: None, } } @@ -316,6 +319,14 @@ impl Observer { self } + /// Set the error handler to use for this observer. + /// + /// See the [`result` module-level documentation](crate::result) for more information. + pub fn with_error_handler(mut self, error_handler: fn(Error, SystemErrorContext)) -> Self { + self.error_handler = Some(error_handler); + self + } + /// Returns the [`ObserverDescriptor`] for this [`Observer`]. pub fn descriptor(&self) -> &ObserverDescriptor { &self.descriptor @@ -363,6 +374,15 @@ fn observer_system_runner>( } state.last_trigger_id = last_trigger; + // SAFETY: Observer was triggered so must have an `Observer` component. + let error_handler = unsafe { + observer_cell + .get::() + .debug_checked_unwrap() + .error_handler + .debug_checked_unwrap() + }; + let trigger: Trigger = Trigger::new( // SAFETY: Caller ensures `ptr` is castable to `&mut T` unsafe { ptr.deref_mut() }, @@ -386,7 +406,15 @@ fn observer_system_runner>( unsafe { (*system).update_archetype_component_access(world); if (*system).validate_param_unsafe(world) { - (*system).run_unsafe(trigger, world); + if let Err(err) = (*system).run_unsafe(trigger, world) { + error_handler( + err, + SystemErrorContext { + name: (*system).name(), + last_run: (*system).get_last_run(), + }, + ); + }; (*system).queue_deferred(world.into_deferred()); } } @@ -416,10 +444,15 @@ fn hook_on_add>( ..Default::default() }; + let error_handler = world.get_resource_or_init::().0; + // Initialize System let system: *mut dyn ObserverSystem = if let Some(mut observe) = world.get_mut::(entity) { descriptor.merge(&observe.descriptor); + if observe.error_handler.is_none() { + observe.error_handler = Some(error_handler); + } let system = observe.system.downcast_mut::().unwrap(); &mut *system } else { @@ -442,3 +475,44 @@ fn hook_on_add>( } }); } + +#[cfg(test)] +mod tests { + use super::*; + use crate::{event::Event, observer::Trigger}; + + #[derive(Event)] + struct TriggerEvent; + + #[test] + #[should_panic(expected = "I failed!")] + fn test_fallible_observer() { + fn system(_: Trigger) -> Result { + Err("I failed!".into()) + } + + let mut world = World::default(); + world.add_observer(system); + Schedule::default().run(&mut world); + world.trigger(TriggerEvent); + } + + #[test] + fn test_fallible_observer_ignored_errors() { + #[derive(Resource, Default)] + struct Ran(bool); + + fn system(_: Trigger, mut ran: ResMut) -> Result { + ran.0 = true; + Err("I failed!".into()) + } + + let mut world = World::default(); + world.init_resource::(); + let observer = Observer::new(system).with_error_handler(crate::result::ignore); + world.spawn(observer); + Schedule::default().run(&mut world); + world.trigger(TriggerEvent); + assert!(world.resource::().0); + } +} diff --git a/crates/bevy_ecs/src/system/observer_system.rs b/crates/bevy_ecs/src/system/observer_system.rs index 5dca4f4497..8a8f82d99e 100644 --- a/crates/bevy_ecs/src/system/observer_system.rs +++ b/crates/bevy_ecs/src/system/observer_system.rs @@ -1,22 +1,27 @@ +use alloc::{borrow::Cow, vec::Vec}; +use core::marker::PhantomData; + use crate::{ + archetype::ArchetypeComponentId, + component::{ComponentId, Tick}, prelude::{Bundle, Trigger}, - system::System, + query::Access, + result::Result, + schedule::{Fallible, Infallible}, + system::{input::SystemIn, System}, + world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; use super::IntoSystem; /// Implemented for [`System`]s that have a [`Trigger`] as the first argument. -pub trait ObserverSystem: +pub trait ObserverSystem: System, Out = Out> + Send + 'static { } -impl< - E: 'static, - B: Bundle, - Out, - T: System, Out = Out> + Send + 'static, - > ObserverSystem for T +impl ObserverSystem for T where + T: System, Out = Out> + Send + 'static { } @@ -32,7 +37,7 @@ impl< label = "the trait `IntoObserverSystem` is not implemented", note = "for function `ObserverSystem`s, ensure the first argument is a `Trigger` and any subsequent ones are `SystemParam`" )] -pub trait IntoObserverSystem: Send + 'static { +pub trait IntoObserverSystem: Send + 'static { /// The type of [`System`] that this instance converts into. type System: ObserverSystem; @@ -40,23 +45,150 @@ pub trait IntoObserverSystem: Send + 'static fn into_system(this: Self) -> Self::System; } -impl< - S: IntoSystem, Out, M> + Send + 'static, - M, - Out, - E: 'static, - B: Bundle, - > IntoObserverSystem for S +impl IntoObserverSystem for S where + S: IntoSystem, Out, M> + Send + 'static, S::System: ObserverSystem, + E: 'static, + B: Bundle, { - type System = , Out, M>>::System; + type System = S::System; fn into_system(this: Self) -> Self::System { IntoSystem::into_system(this) } } +impl IntoObserverSystem for S +where + S: IntoSystem, (), M> + Send + 'static, + S::System: ObserverSystem, + E: Send + Sync + 'static, + B: Bundle, +{ + type System = InfallibleObserverWrapper; + + fn into_system(this: Self) -> Self::System { + InfallibleObserverWrapper::new(IntoSystem::into_system(this)) + } +} + +/// A wrapper that converts an observer system that returns `()` into one that returns `Ok(())`. +pub struct InfallibleObserverWrapper { + observer: S, + _marker: PhantomData<(E, B)>, +} + +impl InfallibleObserverWrapper { + /// Create a new `InfallibleObserverWrapper`. + pub fn new(observer: S) -> Self { + Self { + observer, + _marker: PhantomData, + } + } +} + +impl System for InfallibleObserverWrapper +where + S: ObserverSystem, + E: Send + Sync + 'static, + B: Bundle, +{ + type In = Trigger<'static, E, B>; + type Out = Result; + + #[inline] + fn name(&self) -> Cow<'static, str> { + self.observer.name() + } + + #[inline] + fn component_access(&self) -> &Access { + self.observer.component_access() + } + + #[inline] + fn archetype_component_access(&self) -> &Access { + self.observer.archetype_component_access() + } + + #[inline] + fn is_send(&self) -> bool { + self.observer.is_send() + } + + #[inline] + fn is_exclusive(&self) -> bool { + self.observer.is_exclusive() + } + + #[inline] + fn has_deferred(&self) -> bool { + self.observer.has_deferred() + } + + #[inline] + unsafe fn run_unsafe( + &mut self, + input: SystemIn<'_, Self>, + world: UnsafeWorldCell, + ) -> Self::Out { + self.observer.run_unsafe(input, world); + Ok(()) + } + + #[inline] + fn run(&mut self, input: SystemIn<'_, Self>, world: &mut World) -> Self::Out { + self.observer.run(input, world); + Ok(()) + } + + #[inline] + fn apply_deferred(&mut self, world: &mut World) { + self.observer.apply_deferred(world); + } + + #[inline] + fn queue_deferred(&mut self, world: DeferredWorld) { + self.observer.queue_deferred(world); + } + + #[inline] + unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool { + self.observer.validate_param_unsafe(world) + } + + #[inline] + fn initialize(&mut self, world: &mut World) { + self.observer.initialize(world); + } + + #[inline] + fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) { + self.observer.update_archetype_component_access(world); + } + + #[inline] + fn check_change_tick(&mut self, change_tick: Tick) { + self.observer.check_change_tick(change_tick); + } + + #[inline] + fn get_last_run(&self) -> Tick { + self.observer.get_last_run() + } + + #[inline] + fn set_last_run(&mut self, last_run: Tick) { + self.observer.set_last_run(last_run); + } + + fn default_system_sets(&self) -> Vec { + self.observer.default_system_sets() + } +} + #[cfg(test)] mod tests { use crate::{ diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs index 34b772af57..6d24cc3ca2 100644 --- a/examples/ecs/fallible_systems.rs +++ b/examples/ecs/fallible_systems.rs @@ -1,5 +1,7 @@ -//! Showcases how fallible systems can be make use of rust's powerful result handling syntax. +//! Showcases how fallible systems and observers can make use of Rust's powerful result handling +//! syntax. +use bevy::ecs::world::DeferredWorld; use bevy::math::sampling::UniformMeshSampler; use bevy::prelude::*; @@ -10,6 +12,9 @@ fn main() { app.add_plugins(DefaultPlugins); + #[cfg(feature = "bevy_mesh_picking_backend")] + app.add_plugins(MeshPickingPlugin); + // Fallible systems can be used the same way as regular systems. The only difference is they // return a `Result<(), Box>` instead of a `()` (unit) type. Bevy will handle both // types of systems the same way, except for the error handling. @@ -44,6 +49,9 @@ fn main() { }), ); + // Fallible observers are also sypported. + app.add_observer(fallible_observer); + // If we run the app, we'll see the following output at startup: // // WARN Encountered an error in system `fallible_systems::failing_system`: "Resource not initialized" @@ -53,6 +61,8 @@ fn main() { } /// An example of a system that calls several fallible functions with the question mark operator. +/// +/// See: fn setup( mut commands: Commands, mut meshes: ResMut>, @@ -118,6 +128,29 @@ fn setup( Ok(()) } +// Observer systems can also return a `Result`. +fn fallible_observer( + trigger: Trigger>, + mut world: DeferredWorld, + mut step: Local, +) -> Result { + let mut transform = world + .get_mut::(trigger.target) + .ok_or("No transform found.")?; + + *step = if transform.translation.x > 3. { + -0.1 + } else if transform.translation.x < -3. || *step == 0. { + 0.1 + } else { + *step + }; + + transform.translation.x += *step; + + Ok(()) +} + #[derive(Resource)] struct UninitializedResource;