From 0070514f5468909ffa50845bca2a665766edfe9d Mon Sep 17 00:00:00 2001 From: Miles Silberling-Cook Date: Thu, 5 Dec 2024 17:29:06 -0500 Subject: [PATCH] Fallible systems (#16589) # Objective Error handling in bevy is hard. See for reference https://github.com/bevyengine/bevy/issues/11562, https://github.com/bevyengine/bevy/issues/10874 and https://github.com/bevyengine/bevy/issues/12660. The goal of this PR is to make it better, by allowing users to optionally return `Result` from systems as outlined by Cart in . ## Solution This PR introduces a new `ScheuleSystem` type to represent systems that can be added to schedules. Instances of this type contain either an infallible `BoxedSystem<(), ()>` or a fallible `BoxedSystem<(), Result>`. `ScheuleSystem` implements `System` and replaces all uses of `BoxedSystem` in schedules. The async executor now receives a result after executing a system, which for infallible systems is always `Ok(())`. Currently it ignores this result, but more useful error handling could also be implemented. Aliases for `Error` and `Result` have been added to the `bevy_ecs` prelude, as well as const `OK` which new users may find more friendly than `Ok(())`. ## Testing - Currently there are not actual semantics changes that really require new tests, but I added a basic one just to make sure we don't break stuff in the future. - The behavior of existing systems is totally unchanged, including logging. - All of the existing systems tests pass, and I have not noticed anything strange while playing with the examples ## Showcase The following minimal example prints "hello world" once, then completes. ```rust use bevy::prelude::*; fn main() { App::new().add_systems(Update, hello_world_system).run(); } fn hello_world_system() -> Result { println!("hello world"); Err("string")?; println!("goodbye world"); OK } ``` ## Migration Guide This change should be pretty much non-breaking, except for users who have implemented their own custom executors. Those users should use `ScheduleSystem` in place of `BoxedSystem<(), ()>` and import the `System` trait where needed. They can choose to do whatever they wish with the result. ## Current Work + [x] Fix tests & doc comments + [x] Write more tests + [x] Add examples + [X] Draft release notes ## Draft Release Notes As of this release, systems can now return results. First a bit of background: Bevy has hisotrically expected systems to return the empty type `()`. While this makes sense in the context of the ecs, it's at odds with how error handling is typically done in rust: returning `Result::Error` to indicate failure, and using the short-circuiting `?` operator to propagate that error up the call stack to where it can be properly handled. Users of functional languages will tell you this is called "monadic error handling". Not being able to return `Results` from systems left bevy users with a quandry. They could add custom error handling logic to every system, or manually pipe every system into an error handler, or perhaps sidestep the issue with some combination of fallible assignents, logging, macros, and early returns. Often, users would just litter their systems with unwraps and possible panics. While any one of these approaches might be fine for a particular user, each of them has their own drawbacks, and none makes good use of the language. Serious issues could also arrise when two different crates used by the same project made different choices about error handling. Now, by returning results, systems can defer error handling to the application itself. It looks like this: ```rust // Previous, handling internally app.add_systems(my_system) fn my_system(window: Query<&Window>) { let Ok(window) = query.get_single() else { return; }; // ... do something to the window here } // Previous, handling externally app.add_systems(my_system.pipe(my_error_handler)) fn my_system(window: Query<&Window>) -> Result<(), impl Error> { let window = query.get_single()?; // ... do something to the window here Ok(()) } // Previous, panicking app.add_systems(my_system) fn my_system(window: Query<&Window>) { let window = query.single(); // ... do something to the window here } // Now app.add_systems(my_system) fn my_system(window: Query<&Window>) -> Result { let window = query.get_single()?; // ... do something to the window here Ok(()) } ``` There are currently some limitations. Systems must either return `()` or `Result<(), Box>`, with no in-between. Results are also ignored by default, and though implementing a custom handler is possible, it involves writing your own custom ecs executor (which is *not* recomended). Systems should return errors when they cannot perform their normal behavior. In turn, errors returned to the executor while running the schedule will (eventually) be treated as unexpected. Users and library authors should prefer to return errors for anything that disrupts the normal expected behavior of a system, and should only handle expected cases internally. We have big plans for improving error handling further: + Allowing users to change the error handling logic of the default executors. + Adding source tracking and optional backtraces to errors. + Possibly adding tracing-levels (Error/Warn/Info/Debug/Trace) to errors. + Generally making the default error logging more helpful and inteligent. + Adding monadic system combininators for fallible systems. + Possibly removing all panicking variants from our api. --------- Co-authored-by: Zachary Harrold --- Cargo.toml | 11 ++ crates/bevy_ecs/src/lib.rs | 4 + crates/bevy_ecs/src/result.rs | 7 + crates/bevy_ecs/src/schedule/config.rs | 38 +++- crates/bevy_ecs/src/schedule/executor/mod.rs | 25 ++- .../src/schedule/executor/multi_threaded.rs | 20 +- .../bevy_ecs/src/schedule/executor/simple.rs | 6 +- .../src/schedule/executor/single_threaded.rs | 9 +- crates/bevy_ecs/src/schedule/schedule.rs | 34 ++-- crates/bevy_ecs/src/schedule/stepping.rs | 2 +- crates/bevy_ecs/src/system/mod.rs | 22 ++- crates/bevy_ecs/src/system/schedule_system.rs | 183 ++++++++++++++++++ examples/README.md | 1 + examples/ecs/fallible_systems.rs | 79 ++++++++ 14 files changed, 389 insertions(+), 52 deletions(-) create mode 100644 crates/bevy_ecs/src/result.rs create mode 100644 crates/bevy_ecs/src/system/schedule_system.rs create mode 100644 examples/ecs/fallible_systems.rs diff --git a/Cargo.toml b/Cargo.toml index 8a43ae266f..4a4a005e2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2031,6 +2031,17 @@ description = "Systems are skipped if their parameters cannot be acquired" category = "ECS (Entity Component System)" wasm = false +[[example]] +name = "fallible_systems" +path = "examples/ecs/fallible_systems.rs" +doc-scrape-examples = true + +[package.metadata.example.fallible_systems] +name = "Fallible Systems" +description = "Systems that return results to handle errors" +category = "ECS (Entity Component System)" +wasm = false + [[example]] name = "startup_system" path = "examples/ecs/startup_system.rs" diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 3d33e7f367..20f40f7036 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1,5 +1,7 @@ // FIXME(11590): remove this once the lint is fixed #![allow(unsafe_op_in_unsafe_fn)] +// TODO: remove once Edition 2024 is released +#![allow(dependency_on_unit_never_type_fallback)] #![doc = include_str!("../README.md")] // `rustdoc_internals` is needed for `#[doc(fake_variadics)]` #![allow(internal_features)] @@ -30,6 +32,7 @@ pub mod query; #[cfg(feature = "bevy_reflect")] pub mod reflect; pub mod removal_detection; +pub mod result; pub mod schedule; pub mod storage; pub mod system; @@ -53,6 +56,7 @@ pub mod prelude { observer::{CloneEntityWithObserversExt, Observer, Trigger}, query::{Added, AnyOf, Changed, Has, Or, QueryBuilder, QueryState, With, Without}, removal_detection::RemovedComponents, + result::{Error, Result}, schedule::{ apply_deferred, common_conditions::*, ApplyDeferred, Condition, IntoSystemConfigs, IntoSystemSet, IntoSystemSetConfigs, Schedule, Schedules, SystemSet, diff --git a/crates/bevy_ecs/src/result.rs b/crates/bevy_ecs/src/result.rs new file mode 100644 index 0000000000..4127e2ec91 --- /dev/null +++ b/crates/bevy_ecs/src/result.rs @@ -0,0 +1,7 @@ +//! Contains error and result helpers for use in fallible systems. + +/// A dynamic error type for use in fallible systems. +pub type Error = Box; + +/// A result type for use in fallible systems. +pub type Result = core::result::Result; diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 4c80d421d2..88bf0ffc8c 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -1,13 +1,14 @@ use variadics_please::all_tuples; use crate::{ + result::Result, schedule::{ condition::{BoxedCondition, Condition}, graph::{Ambiguity, Dependency, DependencyKind, GraphInfo}, set::{InternedSystemSet, IntoSystemSet, SystemSet}, Chain, }, - system::{BoxedSystem, IntoSystem, System}, + system::{BoxedSystem, IntoSystem, ScheduleSystem, System}, }; fn new_condition(condition: impl Condition) -> BoxedCondition { @@ -47,7 +48,7 @@ pub struct NodeConfig { } /// Stores configuration for a single system. -pub type SystemConfig = NodeConfig; +pub type SystemConfig = NodeConfig; /// A collections of generic [`NodeConfig`]s. pub enum NodeConfigs { @@ -65,10 +66,10 @@ pub enum NodeConfigs { } /// A collection of [`SystemConfig`]. -pub type SystemConfigs = NodeConfigs; +pub type SystemConfigs = NodeConfigs; impl SystemConfigs { - fn new_system(system: BoxedSystem) -> Self { + fn new_system(system: ScheduleSystem) -> Self { // include system in its default sets let sets = system.default_system_sets().into_iter().collect(); Self::NodeConfig(SystemConfig { @@ -517,18 +518,41 @@ impl IntoSystemConfigs<()> for SystemConfigs { } } -impl IntoSystemConfigs for F +#[doc(hidden)] +pub struct Infallible; + +impl IntoSystemConfigs<(Infallible, Marker)> for F where F: IntoSystem<(), (), Marker>, { fn into_configs(self) -> SystemConfigs { - SystemConfigs::new_system(Box::new(IntoSystem::into_system(self))) + let boxed_system = Box::new(IntoSystem::into_system(self)); + SystemConfigs::new_system(ScheduleSystem::Infallible(boxed_system)) } } impl IntoSystemConfigs<()> for BoxedSystem<(), ()> { fn into_configs(self) -> SystemConfigs { - SystemConfigs::new_system(self) + SystemConfigs::new_system(ScheduleSystem::Infallible(self)) + } +} + +#[doc(hidden)] +pub struct Fallible; + +impl IntoSystemConfigs<(Fallible, Marker)> for F +where + F: IntoSystem<(), Result, Marker>, +{ + fn into_configs(self) -> SystemConfigs { + let boxed_system = Box::new(IntoSystem::into_system(self)); + SystemConfigs::new_system(ScheduleSystem::Fallible(boxed_system)) + } +} + +impl IntoSystemConfigs<()> for BoxedSystem<(), Result> { + fn into_configs(self) -> SystemConfigs { + SystemConfigs::new_system(ScheduleSystem::Fallible(self)) } } diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 09ae3c84cf..d0231bb2f0 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -19,7 +19,7 @@ use crate::{ prelude::{IntoSystemSet, SystemSet}, query::Access, schedule::{BoxedCondition, InternedSystemSet, NodeId, SystemTypeSet}, - system::{BoxedSystem, System, SystemIn}, + system::{ScheduleSystem, System, SystemIn}, world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, }; @@ -67,7 +67,7 @@ pub struct SystemSchedule { /// List of system node ids. pub(super) system_ids: Vec, /// Indexed by system node id. - pub(super) systems: Vec, + pub(super) systems: Vec, /// Indexed by system node id. pub(super) system_conditions: Vec>, /// Indexed by system node id. @@ -140,9 +140,8 @@ pub const apply_deferred: ApplyDeferred = ApplyDeferred; pub struct ApplyDeferred; /// Returns `true` if the [`System`] is an instance of [`ApplyDeferred`]. -pub(super) fn is_apply_deferred(system: &BoxedSystem) -> bool { - // deref to use `System::type_id` instead of `Any::type_id` - system.as_ref().type_id() == TypeId::of::() +pub(super) fn is_apply_deferred(system: &ScheduleSystem) -> bool { + system.type_id() == TypeId::of::() } impl System for ApplyDeferred { @@ -247,19 +246,18 @@ mod __rust_begin_short_backtrace { use core::hint::black_box; use crate::{ - system::{ReadOnlySystem, System}, + result::Result, + system::{ReadOnlySystem, ScheduleSystem, System}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; /// # Safety /// See `System::run_unsafe`. #[inline(never)] - pub(super) unsafe fn run_unsafe( - system: &mut dyn System, - world: UnsafeWorldCell, - ) { - system.run_unsafe((), world); + pub(super) unsafe fn run_unsafe(system: &mut ScheduleSystem, world: UnsafeWorldCell) -> Result { + let result = system.run_unsafe((), world); black_box(()); + result } /// # Safety @@ -273,9 +271,10 @@ mod __rust_begin_short_backtrace { } #[inline(never)] - pub(super) fn run(system: &mut dyn System, world: &mut World) { - system.run((), world); + pub(super) fn run(system: &mut ScheduleSystem, world: &mut World) -> Result { + let result = system.run((), world); black_box(()); + result } #[inline(never)] diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 53453240dc..3585d74e0e 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -18,7 +18,7 @@ use crate::{ prelude::Resource, query::Access, schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, - system::BoxedSystem, + system::{ScheduleSystem, System}, world::{unsafe_world_cell::UnsafeWorldCell, World}, }; @@ -29,7 +29,7 @@ use super::__rust_begin_short_backtrace; /// Borrowed data used by the [`MultiThreadedExecutor`]. struct Environment<'env, 'sys> { executor: &'env MultiThreadedExecutor, - systems: &'sys [SyncUnsafeCell], + systems: &'sys [SyncUnsafeCell], conditions: SyncUnsafeCell>, world_cell: UnsafeWorldCell<'env>, } @@ -269,7 +269,7 @@ impl<'scope, 'env: 'scope, 'sys> Context<'scope, 'env, 'sys> { &self, system_index: usize, res: Result<(), Box>, - system: &BoxedSystem, + system: &ScheduleSystem, ) { // tell the executor that the system finished self.environment @@ -459,7 +459,7 @@ impl ExecutorState { fn can_run( &mut self, system_index: usize, - system: &mut BoxedSystem, + system: &mut ScheduleSystem, conditions: &mut Conditions, world: UnsafeWorldCell, ) -> bool { @@ -523,7 +523,7 @@ impl ExecutorState { unsafe fn should_run( &mut self, system_index: usize, - system: &mut BoxedSystem, + system: &mut ScheduleSystem, conditions: &mut Conditions, world: UnsafeWorldCell, ) -> bool { @@ -603,8 +603,9 @@ impl ExecutorState { // access the world data used by the system. // - `update_archetype_component_access` has been called. unsafe { - __rust_begin_short_backtrace::run_unsafe( - &mut **system, + // TODO: implement an error-handling API instead of suppressing a possible failure. + let _ = __rust_begin_short_backtrace::run_unsafe( + system, context.environment.world_cell, ); }; @@ -650,7 +651,8 @@ impl ExecutorState { // that no other systems currently have access to the world. let world = unsafe { context.environment.world_cell.world_mut() }; let res = std::panic::catch_unwind(AssertUnwindSafe(|| { - __rust_begin_short_backtrace::run(&mut **system, world); + // TODO: implement an error-handling API instead of suppressing a possible failure. + let _ = __rust_begin_short_backtrace::run(system, world); })); context.system_completed(system_index, res, system); }; @@ -710,7 +712,7 @@ impl ExecutorState { fn apply_deferred( unapplied_systems: &FixedBitSet, - systems: &[SyncUnsafeCell], + systems: &[SyncUnsafeCell], world: &mut World, ) -> Result<(), Box> { for system_index in unapplied_systems.ones() { diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 9940bacced..fa3ccffb0b 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -7,6 +7,7 @@ use crate::{ schedule::{ executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule, }, + system::System, world::World, }; @@ -100,7 +101,8 @@ impl SystemExecutor for SimpleExecutor { } let res = std::panic::catch_unwind(AssertUnwindSafe(|| { - __rust_begin_short_backtrace::run(&mut **system, world); + // TODO: implement an error-handling API instead of suppressing a possible failure. + let _ = __rust_begin_short_backtrace::run(system, world); })); if let Err(payload) = res { eprintln!("Encountered a panic in system `{}`!", &*system.name()); @@ -119,7 +121,7 @@ impl SystemExecutor for SimpleExecutor { impl SimpleExecutor { /// Creates a new simple executor for use in a [`Schedule`](crate::schedule::Schedule). - /// This calls each system in order and immediately calls [`System::apply_deferred`](crate::system::System::apply_deferred). + /// This calls each system in order and immediately calls [`System::apply_deferred`]. pub const fn new() -> Self { Self { evaluated_sets: FixedBitSet::new(), diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index 9cc6199ce6..cd44c4e66e 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -5,6 +5,7 @@ use fixedbitset::FixedBitSet; use crate::{ schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule}, + system::System, world::World, }; @@ -108,14 +109,18 @@ impl SystemExecutor for SingleThreadedExecutor { let res = std::panic::catch_unwind(AssertUnwindSafe(|| { if system.is_exclusive() { - __rust_begin_short_backtrace::run(&mut **system, world); + // TODO: implement an error-handling API instead of suppressing a possible failure. + let _ = __rust_begin_short_backtrace::run(system, world); } else { // Use run_unsafe to avoid immediately applying deferred buffers let world = world.as_unsafe_world_cell(); system.update_archetype_component_access(world); // SAFETY: We have exclusive, single-threaded access to the world and // update_archetype_component_access is being called immediately before this. - unsafe { __rust_begin_short_backtrace::run_unsafe(&mut **system, world) }; + unsafe { + // TODO: implement an error-handling API instead of suppressing a possible failure. + let _ = __rust_begin_short_backtrace::run_unsafe(system, world); + }; } })); if let Err(payload) = res { diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index 85e89a0422..a68edd1adb 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -17,8 +17,9 @@ use crate::{ self as bevy_ecs, component::{ComponentId, Components, Tick}, prelude::Component, + result::Result, schedule::*, - system::{BoxedSystem, IntoSystem, Resource, System}, + system::{IntoSystem, Resource, ScheduleSystem, System}, world::World, }; @@ -485,7 +486,8 @@ impl Schedule { /// schedule has never been initialized or run. pub fn systems( &self, - ) -> Result + Sized, ScheduleNotInitialized> { + ) -> Result + Sized, ScheduleNotInitialized> + { if !self.executor_initialized { return Err(ScheduleNotInitialized); } @@ -563,23 +565,23 @@ impl SystemSetNode { } } -/// A [`BoxedSystem`] with metadata, stored in a [`ScheduleGraph`]. +/// A [`ScheduleSystem`] stored in a [`ScheduleGraph`]. struct SystemNode { - inner: Option, + inner: Option, } impl SystemNode { - pub fn new(system: BoxedSystem) -> Self { + pub fn new(system: ScheduleSystem) -> Self { Self { inner: Some(system), } } - pub fn get(&self) -> Option<&BoxedSystem> { + pub fn get(&self) -> Option<&ScheduleSystem> { self.inner.as_ref() } - pub fn get_mut(&mut self) -> Option<&mut BoxedSystem> { + pub fn get_mut(&mut self) -> Option<&mut ScheduleSystem> { self.inner.as_mut() } } @@ -642,13 +644,13 @@ impl ScheduleGraph { } /// Returns the system at the given [`NodeId`], if it exists. - pub fn get_system_at(&self, id: NodeId) -> Option<&dyn System> { + pub fn get_system_at(&self, id: NodeId) -> Option<&ScheduleSystem> { if !id.is_system() { return None; } self.systems .get(id.index()) - .and_then(|system| system.inner.as_deref()) + .and_then(|system| system.inner.as_ref()) } /// Returns `true` if the given system set is part of the graph. Otherwise, returns `false`. @@ -660,7 +662,7 @@ impl ScheduleGraph { /// /// Panics if it doesn't exist. #[track_caller] - pub fn system_at(&self, id: NodeId) -> &dyn System { + pub fn system_at(&self, id: NodeId) -> &ScheduleSystem { self.get_system_at(id) .ok_or_else(|| format!("system with id {id:?} does not exist in this Schedule")) .unwrap() @@ -685,15 +687,13 @@ impl ScheduleGraph { } /// Returns an iterator over all systems in this schedule, along with the conditions for each system. - pub fn systems( - &self, - ) -> impl Iterator, &[BoxedCondition])> { + pub fn systems(&self) -> impl Iterator { self.systems .iter() .zip(self.system_conditions.iter()) .enumerate() .filter_map(|(i, (system_node, condition))| { - let system = system_node.inner.as_deref()?; + let system = system_node.inner.as_ref()?; Some((NodeId::System(i), system, condition.as_slice())) }) } @@ -1196,8 +1196,8 @@ impl ScheduleGraph { let id = NodeId::System(self.systems.len()); self.systems - .push(SystemNode::new(Box::new(IntoSystem::into_system( - ApplyDeferred, + .push(SystemNode::new(ScheduleSystem::Infallible(Box::new( + IntoSystem::into_system(ApplyDeferred), )))); self.system_conditions.push(Vec::new()); @@ -1554,7 +1554,7 @@ trait ProcessNodeConfig: Sized { fn process_config(schedule_graph: &mut ScheduleGraph, config: NodeConfig) -> NodeId; } -impl ProcessNodeConfig for BoxedSystem { +impl ProcessNodeConfig for ScheduleSystem { fn process_config(schedule_graph: &mut ScheduleGraph, config: NodeConfig) -> NodeId { schedule_graph.add_system_inner(config).unwrap() } diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs index 61cfc3ca24..c5a964a2cb 100644 --- a/crates/bevy_ecs/src/schedule/stepping.rs +++ b/crates/bevy_ecs/src/schedule/stepping.rs @@ -4,7 +4,7 @@ use std::collections::HashMap; use crate::{ schedule::{InternedScheduleLabel, NodeId, Schedule, ScheduleLabel}, - system::{IntoSystem, ResMut, Resource}, + system::{IntoSystem, ResMut, Resource, System}, }; use bevy_utils::{ tracing::{info, warn}, diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index b230535c0b..58f5800ede 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -79,6 +79,12 @@ //! } //! ``` //! +//! # System return type +//! +//! Systems added to a schedule through [`add_systems`](crate::schedule::Schedule) may either return +//! empty `()` or a [`Result`](crate::result::Result). Other contexts (like one shot systems) allow +//! systems to return arbitrary values. +//! //! # System parameter list //! Following is the complete list of accepted types as system parameters: //! @@ -121,6 +127,7 @@ mod function_system; mod input; mod observer_system; mod query; +mod schedule_system; #[allow(clippy::module_inception)] mod system; mod system_name; @@ -139,6 +146,7 @@ pub use function_system::*; pub use input::*; pub use observer_system::*; pub use query::*; +pub use schedule_system::*; pub use system::*; pub use system_name::*; pub use system_param::*; @@ -322,6 +330,7 @@ mod tests { prelude::{AnyOf, EntityRef}, query::{Added, Changed, Or, With, Without}, removal_detection::RemovedComponents, + result::Result, schedule::{ common_conditions::resource_exists, ApplyDeferred, Condition, IntoSystemConfigs, Schedule, @@ -371,7 +380,7 @@ mod tests { system.run((), &mut world); } - fn run_system>(world: &mut World, system: S) { + fn run_system>(world: &mut World, system: S) { let mut schedule = Schedule::default(); schedule.add_systems(system); schedule.run(world); @@ -1749,4 +1758,15 @@ mod tests { sched.run(&mut world); assert_eq!(world.get_resource(), Some(&C(3))); } + + #[test] + fn simple_fallible_system() { + fn sys() -> Result { + Err("error")?; + Ok(()) + } + + let mut world = World::new(); + run_system(&mut world, sys); + } } diff --git a/crates/bevy_ecs/src/system/schedule_system.rs b/crates/bevy_ecs/src/system/schedule_system.rs new file mode 100644 index 0000000000..c88db47f04 --- /dev/null +++ b/crates/bevy_ecs/src/system/schedule_system.rs @@ -0,0 +1,183 @@ +use alloc::borrow::Cow; +use core::any::TypeId; + +use crate::{ + archetype::ArchetypeComponentId, + component::{ComponentId, Tick}, + query::Access, + result::Result, + schedule::InternedSystemSet, + system::{input::SystemIn, BoxedSystem, System}, + world::{unsafe_world_cell::UnsafeWorldCell, DeferredWorld, World}, +}; + +/// A type which wraps and unifies the different sorts of systems that can be added to a schedule. +pub enum ScheduleSystem { + /// A system that does not return a result. + Infallible(BoxedSystem<(), ()>), + /// A system that does return a result. + Fallible(BoxedSystem<(), Result>), +} + +impl System for ScheduleSystem { + type In = (); + type Out = Result; + + #[inline(always)] + fn name(&self) -> Cow<'static, str> { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.name(), + ScheduleSystem::Fallible(inner_system) => inner_system.name(), + } + } + + #[inline(always)] + fn type_id(&self) -> TypeId { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.type_id(), + ScheduleSystem::Fallible(inner_system) => inner_system.type_id(), + } + } + + #[inline(always)] + fn component_access(&self) -> &Access { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.component_access(), + ScheduleSystem::Fallible(inner_system) => inner_system.component_access(), + } + } + + #[inline(always)] + fn archetype_component_access(&self) -> &Access { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.archetype_component_access(), + ScheduleSystem::Fallible(inner_system) => inner_system.archetype_component_access(), + } + } + + #[inline(always)] + fn is_exclusive(&self) -> bool { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.is_exclusive(), + ScheduleSystem::Fallible(inner_system) => inner_system.is_exclusive(), + } + } + + #[inline(always)] + fn has_deferred(&self) -> bool { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.has_deferred(), + ScheduleSystem::Fallible(inner_system) => inner_system.has_deferred(), + } + } + + #[inline(always)] + unsafe fn run_unsafe( + &mut self, + input: SystemIn<'_, Self>, + world: UnsafeWorldCell, + ) -> Self::Out { + match self { + ScheduleSystem::Infallible(inner_system) => { + inner_system.run_unsafe(input, world); + Ok(()) + } + ScheduleSystem::Fallible(inner_system) => inner_system.run_unsafe(input, world), + } + } + + #[inline(always)] + fn run(&mut self, input: SystemIn<'_, Self>, world: &mut World) -> Self::Out { + match self { + ScheduleSystem::Infallible(inner_system) => { + inner_system.run(input, world); + Ok(()) + } + ScheduleSystem::Fallible(inner_system) => inner_system.run(input, world), + } + } + + #[inline(always)] + fn apply_deferred(&mut self, world: &mut World) { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.apply_deferred(world), + ScheduleSystem::Fallible(inner_system) => inner_system.apply_deferred(world), + } + } + + #[inline(always)] + fn queue_deferred(&mut self, world: DeferredWorld) { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.queue_deferred(world), + ScheduleSystem::Fallible(inner_system) => inner_system.queue_deferred(world), + } + } + + #[inline(always)] + fn is_send(&self) -> bool { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.is_send(), + ScheduleSystem::Fallible(inner_system) => inner_system.is_send(), + } + } + + #[inline(always)] + unsafe fn validate_param_unsafe(&mut self, world: UnsafeWorldCell) -> bool { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.validate_param_unsafe(world), + ScheduleSystem::Fallible(inner_system) => inner_system.validate_param_unsafe(world), + } + } + + #[inline(always)] + fn initialize(&mut self, world: &mut World) { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.initialize(world), + ScheduleSystem::Fallible(inner_system) => inner_system.initialize(world), + } + } + + #[inline(always)] + fn update_archetype_component_access(&mut self, world: UnsafeWorldCell) { + match self { + ScheduleSystem::Infallible(inner_system) => { + inner_system.update_archetype_component_access(world); + } + ScheduleSystem::Fallible(inner_system) => { + inner_system.update_archetype_component_access(world); + } + } + } + + #[inline(always)] + fn check_change_tick(&mut self, change_tick: Tick) { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.check_change_tick(change_tick), + ScheduleSystem::Fallible(inner_system) => inner_system.check_change_tick(change_tick), + } + } + + #[inline(always)] + fn default_system_sets(&self) -> Vec { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.default_system_sets(), + ScheduleSystem::Fallible(inner_system) => inner_system.default_system_sets(), + } + } + + #[inline(always)] + fn get_last_run(&self) -> Tick { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.get_last_run(), + ScheduleSystem::Fallible(inner_system) => inner_system.get_last_run(), + } + } + + #[inline(always)] + fn set_last_run(&mut self, last_run: Tick) { + match self { + ScheduleSystem::Infallible(inner_system) => inner_system.set_last_run(last_run), + ScheduleSystem::Fallible(inner_system) => inner_system.set_last_run(last_run), + } + } +} diff --git a/examples/README.md b/examples/README.md index 2e48c40886..1ddb014c97 100644 --- a/examples/README.md +++ b/examples/README.md @@ -299,6 +299,7 @@ Example | Description [ECS Guide](../examples/ecs/ecs_guide.rs) | Full guide to Bevy's ECS [Event](../examples/ecs/event.rs) | Illustrates event creation, activation, and reception [Fallible System Parameters](../examples/ecs/fallible_params.rs) | Systems are skipped if their parameters cannot be acquired +[Fallible Systems](../examples/ecs/fallible_systems.rs) | Systems that return results to handle errors [Fixed Timestep](../examples/ecs/fixed_timestep.rs) | Shows how to create systems that run every fixed timestep, rather than every tick [Generic System](../examples/ecs/generic_system.rs) | Shows how to create systems that can be reused with different types [Hierarchy](../examples/ecs/hierarchy.rs) | Creates a hierarchy of parents and children entities diff --git a/examples/ecs/fallible_systems.rs b/examples/ecs/fallible_systems.rs new file mode 100644 index 0000000000..740976dadf --- /dev/null +++ b/examples/ecs/fallible_systems.rs @@ -0,0 +1,79 @@ +//! Showcases how fallible systems can be make use of rust's powerful result handling syntax. + +use bevy::math::sampling::UniformMeshSampler; +use bevy::prelude::*; + +use rand::distributions::Distribution; + +fn main() { + App::new() + .add_plugins(DefaultPlugins) + .add_systems(Startup, setup) + .run(); +} + +/// An example of a system that calls several fallible functions with the questionmark operator. +fn setup( + mut commands: Commands, + mut meshes: ResMut>, + mut materials: ResMut>, +) -> Result { + let mut rng = rand::thread_rng(); + + // Make a plane for establishing space. + commands.spawn(( + Mesh3d(meshes.add(Plane3d::default().mesh().size(12.0, 12.0))), + MeshMaterial3d(materials.add(Color::srgb(0.3, 0.5, 0.3))), + Transform::from_xyz(0.0, -2.5, 0.0), + )); + + // Spawn a light: + commands.spawn(( + PointLight { + shadows_enabled: true, + ..default() + }, + Transform::from_xyz(4.0, 8.0, 4.0), + )); + + // Spawn a camera: + commands.spawn(( + Camera3d::default(), + Transform::from_xyz(-2.0, 3.0, 5.0).looking_at(Vec3::ZERO, Vec3::Y), + )); + + // Create a new sphere mesh: + let mut sphere_mesh = Sphere::new(1.0).mesh().ico(7)?; + sphere_mesh.generate_tangents()?; + + // Spawn the mesh into the scene: + let mut sphere = commands.spawn(( + Mesh3d(meshes.add(sphere_mesh.clone())), + MeshMaterial3d(materials.add(StandardMaterial::default())), + Transform::from_xyz(-1.0, 1.0, 0.0), + )); + + // Generate random sample points: + let triangles = sphere_mesh.triangles()?; + let distribution = UniformMeshSampler::try_new(triangles)?; + + // Setup sample points: + let point_mesh = meshes.add(Sphere::new(0.01).mesh().ico(3)?); + let point_material = materials.add(StandardMaterial { + base_color: Srgba::RED.into(), + emissive: LinearRgba::rgb(1.0, 0.0, 0.0), + ..default() + }); + + // Add sample points as children of the sphere: + for point in distribution.sample_iter(&mut rng).take(10000) { + sphere.with_child(( + Mesh3d(point_mesh.clone()), + MeshMaterial3d(point_material.clone()), + Transform::from_translation(point), + )); + } + + // Indicate the system completed successfully: + Ok(()) +}