Add ControlFlow::Continue(()) handling for systems #10874
Labels
A-ECS
Entities, components, systems, and events
C-Feature
A new feature, making something new possible
X-Controversial
There is active debate or serious implications around merging this PR
Comments
dimvoly
added
C-Feature
alice-i-cecile
added
A-ECS
|
So, your suggestion is a variant of system piping. I would really like to be able to use the ? operator here, but I don't think that as written it's possible. We need a blanket implementation over all systems: only those which both accept and return I would want to peek at the implementation again, but I suspect that using a Perhaps we could work upstream in Rust itself to support the use of the |
|
This has been discussed heavily. Bevy developers seems uninterested in adding this feature. However, I made a crate to make it possible: https://lib.rs/crates/bevy_mod_sysfail |
nicopap
added
the
X-Controversial
|
Thanks for the suggestions, cool to see a crate's been put together too. I hadn't thought about system piping that is possible too as a workaround. I'll have a play with that sysfail thing later too, it's also quite common for me to fail inside a system, log the error for the user, and then exit early. |
github-merge-queue bot
pushed a commit
that referenced
this issue
Dec 5, 2024
# Objective Error handling in bevy is hard. See for reference #11562, #10874 and #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 <#14275 (comment)>. ## 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<In = (), Out = Result>` 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<dyn Error + Send + Sync + 'static>>`, 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 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What problem does this solve or what need does it fill?
Many a time, I'd run a query and need to exit early. E.g. if some
Componentis missing or uninitialized and there's no use running the system further. The query may issue out aResult<R, E>or I may use a sub-routine that returns aControlFlow<()>.I would like to be able to use the
?operator, as described here to return early from a system.What solution would you like?
Allow for systems to have this in their signature:
What alternative(s) have you considered?
Using a macro to reduce the verbosity of returning early:
And use like so:
Additional context
I'm on bevy 0.10.1, haven't looked in the latest version to see if this is a problem.
Also I don't know the inner workings of bevy, and if this may be impossible for some internal reason.
The text was updated successfully, but these errors were encountered: