Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallible systems #16589

Merged
merged 25 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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)]
NthTensor marked this conversation as resolved.
Show resolved Hide resolved
NthTensor marked this conversation as resolved.
Show resolved Hide resolved
#![doc = include_str!("../README.md")]
// `rustdoc_internals` is needed for `#[doc(fake_variadics)]`
#![allow(internal_features)]
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions crates/bevy_ecs/src/result.rs
Original file line number Diff line number Diff line change
@@ -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>;
NthTensor marked this conversation as resolved.
Show resolved Hide resolved
38 changes: 31 additions & 7 deletions crates/bevy_ecs/src/schedule/config.rs
Original file line number Diff line number Diff line change
@@ -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<M>(condition: impl Condition<M>) -> BoxedCondition {
Expand Down Expand Up @@ -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> {
Expand All @@ -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 {
Expand Down Expand Up @@ -517,18 +518,41 @@ impl IntoSystemConfigs<()> for SystemConfigs {
}
}

impl<Marker, F> IntoSystemConfigs<Marker> for F
#[doc(hidden)]
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))
}
}

#[doc(hidden)]
pub struct Fallible;
NthTensor marked this conversation as resolved.
Show resolved Hide resolved

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))
}
}

Expand Down
25 changes: 12 additions & 13 deletions crates/bevy_ecs/src/schedule/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand Down Expand Up @@ -67,7 +67,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.
Expand Down Expand Up @@ -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::<ApplyDeferred>()
pub(super) fn is_apply_deferred(system: &ScheduleSystem) -> bool {
system.type_id() == TypeId::of::<ApplyDeferred>()
}

impl System for ApplyDeferred {
Expand Down Expand Up @@ -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<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
Expand All @@ -273,9 +271,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)]
Expand Down
20 changes: 11 additions & 9 deletions crates/bevy_ecs/src/schedule/executor/multi_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
};

Expand All @@ -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>,
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
);
};
Expand Down Expand Up @@ -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);
};
Expand Down Expand Up @@ -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() {
Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/simple.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::{
schedule::{
executor::is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule,
},
system::System,
world::World,
};

Expand Down Expand Up @@ -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());
Expand All @@ -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(),
Expand Down
9 changes: 7 additions & 2 deletions crates/bevy_ecs/src/schedule/executor/single_threaded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use fixedbitset::FixedBitSet;

use crate::{
schedule::{is_apply_deferred, BoxedCondition, ExecutorKind, SystemExecutor, SystemSchedule},
system::System,
world::World,
};

Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading