-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Unify and simplify command and system error handling #18351
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
Merged
alice-i-cecile
merged 35 commits into
bevyengine:main
from
alice-i-cecile:from-the-other-direction
Mar 18, 2025
Merged
Changes from all commits
Commits
Show all changes
35 commits
Select commit
Hold shift + click to select a range
0ab0788
Remove App / Subapp helper methods
alice-i-cecile 38baf8f
Remove per-schedule error handling
alice-i-cecile 155ab34
Rename fallible_systems example to error_handling
alice-i-cecile 0894a05
Remove feature flag
alice-i-cecile 45f23b7
Rename error_handler_default for easier grepping
alice-i-cecile 4926938
Move and rename command error-handlers
alice-i-cecile bc9a808
Move command error handling traits
alice-i-cecile 72b501f
Swap to a generic error context
alice-i-cecile d3243a1
Unify error handlers and standardize on fn(BevyError, ErrorContext)
alice-i-cecile 0b8bf3b
Standardize on GlobalErrorHandler
alice-i-cecile 66396f2
Small improvements for module docs
alice-i-cecile 38a4915
Import OnceLock for code quality reasons
alice-i-cecile e5f7832
Helpful traits for ErrorContext
alice-i-cecile 39bec0d
Demonstrate error handling for commands
alice-i-cecile 87f586e
No bevy_ecs imports. Bad!
alice-i-cecile d52b347
clippy::match_same_arms
alice-i-cecile 09f32d7
OnceLock from bevy_platform_support
alice-i-cecile a437f80
Missing whitespace in doc comment
alice-i-cecile 2bb2e9c
Yeet default_error_handler
alice-i-cecile df1f552
Merge branch 'main' into from-the-other-direction
alice-i-cecile f224aa3
Fix example metadata heading
alice-i-cecile 31b53a9
Regenerate examples README
alice-i-cecile 309aa62
Small fixes for CI
alice-i-cecile 924f365
Merge branch 'main' into from-the-other-direction
alice-i-cecile e12b996
More doc test fixes
alice-i-cecile 8f030d6
Merge remote-tracking branch 'alice-i-cecile/from-the-other-direction…
alice-i-cecile b20dc8e
Remove stray mentions of default_error_handler in docs
alice-i-cecile b987d07
Re-add default error handler helper
alice-i-cecile 83f040a
Re-add and beef up the feature flag
alice-i-cecile 19b90c2
cfg gate the OnceLock import
alice-i-cecile 6e00af7
Ignore doc test that relies on cfg gate :(
alice-i-cecile fb3ebda
Ensure comments in Cargo.toml are consistent with neighbors
alice-i-cecile 4947b99
Regenerate cargo_features.md
alice-i-cecile 2e7f9d2
Broken doc links
alice-i-cecile e59acca
Spaces not tabs in doc comments :upside_down:
alice-i-cecile File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
use core::{any::type_name, fmt}; | ||
|
||
use crate::{ | ||
entity::Entity, | ||
system::{entity_command::EntityCommandError, Command, EntityCommand}, | ||
world::{error::EntityMutableFetchError, World}, | ||
}; | ||
|
||
use super::{default_error_handler, BevyError, ErrorContext}; | ||
|
||
/// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into | ||
/// a [`Command`] that internally handles an error if it occurs and returns `()`. | ||
pub trait HandleError<Out = ()> { | ||
/// Takes a [`Command`] that returns a Result and uses a given error handler function to convert it into | ||
/// a [`Command`] that internally handles an error if it occurs and returns `()`. | ||
fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command; | ||
/// Takes a [`Command`] that returns a Result and uses the default error handler function to convert it into | ||
/// a [`Command`] that internally handles an error if it occurs and returns `()`. | ||
fn handle_error(self) -> impl Command | ||
where | ||
Self: Sized, | ||
{ | ||
self.handle_error_with(default_error_handler()) | ||
} | ||
} | ||
|
||
impl<C, T, E> HandleError<Result<T, E>> for C | ||
where | ||
C: Command<Result<T, E>>, | ||
E: Into<BevyError>, | ||
{ | ||
fn handle_error_with(self, error_handler: fn(BevyError, ErrorContext)) -> impl Command { | ||
move |world: &mut World| match self.apply(world) { | ||
Ok(_) => {} | ||
Err(err) => (error_handler)( | ||
err.into(), | ||
ErrorContext::Command { | ||
name: type_name::<C>().into(), | ||
}, | ||
), | ||
} | ||
} | ||
} | ||
|
||
impl<C> HandleError for C | ||
where | ||
C: Command, | ||
{ | ||
#[inline] | ||
fn handle_error_with(self, _error_handler: fn(BevyError, ErrorContext)) -> impl Command { | ||
self | ||
} | ||
#[inline] | ||
fn handle_error(self) -> impl Command | ||
where | ||
Self: Sized, | ||
{ | ||
self | ||
} | ||
} | ||
|
||
/// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that | ||
/// internally runs the [`EntityCommand`] on that entity. | ||
/// | ||
// NOTE: This is a separate trait from `EntityCommand` because "result-returning entity commands" and | ||
// "non-result returning entity commands" require different implementations, so they cannot be automatically | ||
// implemented. And this isn't the type of implementation that we want to thrust on people implementing | ||
// EntityCommand. | ||
pub trait CommandWithEntity<Out> { | ||
/// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that | ||
/// internally runs the [`EntityCommand`] on that entity. | ||
fn with_entity(self, entity: Entity) -> impl Command<Out> + HandleError<Out>; | ||
} | ||
|
||
impl<C> CommandWithEntity<Result<(), EntityMutableFetchError>> for C | ||
where | ||
C: EntityCommand, | ||
{ | ||
fn with_entity( | ||
self, | ||
entity: Entity, | ||
) -> impl Command<Result<(), EntityMutableFetchError>> | ||
+ HandleError<Result<(), EntityMutableFetchError>> { | ||
move |world: &mut World| -> Result<(), EntityMutableFetchError> { | ||
let entity = world.get_entity_mut(entity)?; | ||
self.apply(entity); | ||
Ok(()) | ||
} | ||
} | ||
} | ||
|
||
impl<C, T, Err> CommandWithEntity<Result<T, EntityCommandError<Err>>> for C | ||
where | ||
C: EntityCommand<Result<T, Err>>, | ||
Err: fmt::Debug + fmt::Display + Send + Sync + 'static, | ||
{ | ||
fn with_entity( | ||
self, | ||
entity: Entity, | ||
) -> impl Command<Result<T, EntityCommandError<Err>>> + HandleError<Result<T, EntityCommandError<Err>>> | ||
{ | ||
move |world: &mut World| { | ||
let entity = world.get_entity_mut(entity)?; | ||
self.apply(entity) | ||
.map_err(EntityCommandError::CommandFailed) | ||
} | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sort of confused why this is a generic with an empty default. The docs don't make it clear to me either. Can you help me out here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was moved unchanged in this PR. It was introduced in https://github.com/bevyengine/bevy/pull/17215/files#diff-7f622096463e35e3ef38c2c59a68fa4c5e1f0f367632eaf1e9c4779fcc72d74eR69
I would like this to be clearer / better documented, but I'm not convinced this PR is the place for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes more sense to me now that I've read the rest of the code.