Fallible systems

Cargo.toml

@@ -2020,6 +2020,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"

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;
@@ -52,6 +55,7 @@ pub mod prelude {
         observer::{Observer, Trigger},
         query::{Added, AnyOf, Changed, Has, Or, QueryBuilder, QueryState, With, Without},
         removal_detection::RemovedComponents,
+        result::{Error, Result},
         schedule::{
             apply_deferred, common_conditions::*, Condition, IntoSystemConfigs, IntoSystemSet,
             IntoSystemSetConfigs, Schedule, Schedules, SystemSet,

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<dyn core::error::Error + Send + Sync + 'static>;
+
+/// A result type for use in fallible systems.
+pub type Result<T = (), E = Error> = core::result::Result<T, E>;

crates/bevy_ecs/src/schedule/config.rs

@@ -1,13 +1,14 @@
 use bevy_utils::all_tuples;
 
 use crate::{
+    result::Result,
     schedule::{
         condition::{BoxedCondition, Condition},
         graph_utils::{Ambiguity, Dependency, DependencyKind, GraphInfo},
         set::{InternedSystemSet, IntoSystemSet, SystemSet},
         Chain,
     },
-    system::{BoxedSystem, IntoSystem, System},
+    system::{BoxedSystem, IntoSystem, ScheduleSystem, System},
 };
 
 fn new_condition<M>(condition: impl Condition<M>) -> BoxedCondition {
@@ -47,7 +48,7 @@ pub struct NodeConfig<T> {
 }
 
 /// Stores configuration for a single system.
-pub type SystemConfig = NodeConfig<BoxedSystem>;
+pub type SystemConfig = NodeConfig<ScheduleSystem>;
 
 /// A collections of generic [`NodeConfig`]s.
 pub enum NodeConfigs<T> {
@@ -65,10 +66,10 @@ pub enum NodeConfigs<T> {
 }
 
 /// A collection of [`SystemConfig`].
-pub type SystemConfigs = NodeConfigs<BoxedSystem>;
+pub type SystemConfigs = NodeConfigs<ScheduleSystem>;
 
 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<Marker, F> IntoSystemConfigs<Marker> for F
+/// A marker type for an infallible system. Used internally.
+pub struct Infallible;
+
+impl<F, Marker> 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))
+    }
+}
+
+/// A marker type for a fallible system. Used internally.
+pub struct Fallible;
+
+impl<F, Marker> 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))
     }
 }
 

crates/bevy_ecs/src/schedule/executor/mod.rs

@@ -12,7 +12,7 @@ use fixedbitset::FixedBitSet;
 
 use crate::{
     schedule::{BoxedCondition, NodeId},
-    system::BoxedSystem,
+    system::ScheduleSystem,
     world::World,
 };
 
@@ -60,7 +60,7 @@ pub struct SystemSchedule {
     /// List of system node ids.
     pub(super) system_ids: Vec<NodeId>,
     /// Indexed by system node id.
-    pub(super) systems: Vec<BoxedSystem>,
+    pub(super) systems: Vec<ScheduleSystem>,
     /// Indexed by system node id.
     pub(super) system_conditions: Vec<Vec<BoxedCondition>>,
     /// Indexed by system node id.
@@ -122,10 +122,10 @@ impl SystemSchedule {
 pub fn apply_deferred(world: &mut World) {}
 
 /// Returns `true` if the [`System`](crate::system::System) is an instance of [`apply_deferred`].
-pub(super) fn is_apply_deferred(system: &BoxedSystem) -> bool {
-    use crate::system::IntoSystem;
+pub(super) fn is_apply_deferred(system: &ScheduleSystem) -> bool {
+    use crate::system::{IntoSystem, System};
     // deref to use `System::type_id` instead of `Any::type_id`
-    system.as_ref().type_id() == apply_deferred.system_type_id()
+    system.type_id() == apply_deferred.system_type_id()
 }
 
 /// These functions hide the bottom of the callstack from `RUST_BACKTRACE=1` (assuming the default panic handler is used).
@@ -140,19 +140,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<In = (), Out = ()>,
-        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
@@ -166,9 +165,10 @@ mod __rust_begin_short_backtrace {
     }
 
     #[inline(never)]
-    pub(super) fn run(system: &mut dyn System<In = (), Out = ()>, 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)]

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<BoxedSystem>],
+    systems: &'sys [SyncUnsafeCell<ScheduleSystem>],
     conditions: SyncUnsafeCell<Conditions<'sys>>,
     world_cell: UnsafeWorldCell<'env>,
 }
@@ -269,7 +269,7 @@ impl<'scope, 'env: 'scope, 'sys> Context<'scope, 'env, 'sys> {
         &self,
         system_index: usize,
         res: Result<(), Box<dyn Any + Send>>,
-        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<BoxedSystem>],
+    systems: &[SyncUnsafeCell<ScheduleSystem>],
     world: &mut World,
 ) -> Result<(), Box<dyn Any + Send>> {
     for system_index in unapplied_systems.ones() {

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(),

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 {