Don't create errors for ignored failed commands (#19718)
# Objective 1. Reduce overhead from error handling for ECS commands that intentionally ignore errors, such as `try_despawn`. These commands currently allocate error objects and pass them to a no-op handler (`ignore`), which can impact performance when many operations fail. 2. Fix a hang when removing `ChildOf` components during entity despawning. Excessive logging of these failures can cause significant hangs (I'm noticing around 100ms). - Fixes https://github.com/bevyengine/bevy/issues/19777 - Fixes https://github.com/bevyengine/bevy/issues/19753 <img width="1387" alt="image" src="https://github.com/user-attachments/assets/5c67ab77-97bb-46e5-b287-2c502bef9358" /> ## Solution * Added a `ignore_error` method to the `HandleError` trait to use instead of `handle_error_with(ignore)`. It swallows errors and does not create error objects. * Replaced `remove::<ChildOf>` with `try_remove::<ChildOf>` to suppress expected (?) errors and reduce log noise. ## Testing - I ran these changes on a local project.
This commit is contained in:
parent
85448b767e
commit
795e273a9a
@ -20,6 +20,8 @@ pub trait HandleError<Out = ()>: Send + 'static {
|
||||
/// 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;
|
||||
/// Takes a [`Command`] that returns a Result and ignores any error that occurs.
|
||||
fn ignore_error(self) -> impl Command;
|
||||
}
|
||||
|
||||
impl<C, T, E> HandleError<Result<T, E>> for C
|
||||
@ -50,6 +52,12 @@ where
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
fn ignore_error(self) -> impl Command {
|
||||
move |world: &mut World| {
|
||||
let _ = self.apply(world);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<C> HandleError<Never> for C
|
||||
@ -68,6 +76,13 @@ where
|
||||
self.apply(world);
|
||||
}
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn ignore_error(self) -> impl Command {
|
||||
move |world: &mut World| {
|
||||
self.apply(world);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<C> HandleError for C
|
||||
@ -82,6 +97,10 @@ where
|
||||
fn handle_error(self) -> impl Command {
|
||||
self
|
||||
}
|
||||
#[inline]
|
||||
fn ignore_error(self) -> impl Command {
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
/// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that
|
||||
|
@ -14,7 +14,7 @@ pub use relationship_source_collection::*;
|
||||
use crate::{
|
||||
component::{Component, Mutable},
|
||||
entity::{ComponentCloneCtx, Entity, SourceComponent},
|
||||
error::{ignore, CommandWithEntity, HandleError},
|
||||
error::CommandWithEntity,
|
||||
lifecycle::HookContext,
|
||||
world::{DeferredWorld, EntityWorldMut},
|
||||
};
|
||||
@ -187,7 +187,7 @@ pub trait Relationship: Component + Sized {
|
||||
|
||||
world
|
||||
.commands()
|
||||
.queue(command.with_entity(target_entity).handle_error_with(ignore));
|
||||
.queue_silenced(command.with_entity(target_entity));
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -244,7 +244,7 @@ pub trait RelationshipTarget: Component<Mutability = Mutable> + Sized {
|
||||
for source_entity in relationship_target.iter() {
|
||||
commands
|
||||
.entity(source_entity)
|
||||
.remove::<Self::Relationship>();
|
||||
.try_remove::<Self::Relationship>();
|
||||
}
|
||||
}
|
||||
|
||||
@ -255,7 +255,7 @@ pub trait RelationshipTarget: Component<Mutability = Mutable> + Sized {
|
||||
let (entities, mut commands) = world.entities_and_commands();
|
||||
let relationship_target = entities.get(entity).unwrap().get::<Self>().unwrap();
|
||||
for source_entity in relationship_target.iter() {
|
||||
commands.entity(source_entity).despawn();
|
||||
commands.entity(source_entity).try_despawn();
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -19,7 +19,7 @@ use crate::{
|
||||
change_detection::{MaybeLocation, Mut},
|
||||
component::{Component, ComponentId, Mutable},
|
||||
entity::{Entities, Entity, EntityClonerBuilder, EntityDoesNotExistError, OptIn, OptOut},
|
||||
error::{ignore, warn, BevyError, CommandWithEntity, ErrorContext, HandleError},
|
||||
error::{warn, BevyError, CommandWithEntity, ErrorContext, HandleError},
|
||||
event::{BufferedEvent, EntityEvent, Event},
|
||||
observer::{Observer, TriggerTargets},
|
||||
resource::Resource,
|
||||
@ -641,6 +641,11 @@ impl<'w, 's> Commands<'w, 's> {
|
||||
self.queue_internal(command.handle_error_with(error_handler));
|
||||
}
|
||||
|
||||
/// Pushes a generic [`Command`] to the queue like [`Commands::queue_handled`], but instead silently ignores any errors.
|
||||
pub fn queue_silenced<C: Command<T> + HandleError<T>, T>(&mut self, command: C) {
|
||||
self.queue_internal(command.ignore_error());
|
||||
}
|
||||
|
||||
fn queue_internal(&mut self, command: impl Command) {
|
||||
match &mut self.queue {
|
||||
InternalQueue::CommandQueue(queue) => {
|
||||
@ -1466,12 +1471,11 @@ impl<'a> EntityCommands<'a> {
|
||||
component_id: ComponentId,
|
||||
value: T,
|
||||
) -> &mut Self {
|
||||
self.queue_handled(
|
||||
self.queue_silenced(
|
||||
// SAFETY:
|
||||
// - `ComponentId` safety is ensured by the caller.
|
||||
// - `T` safety is ensured by the caller.
|
||||
unsafe { entity_command::insert_by_id(component_id, value, InsertMode::Replace) },
|
||||
ignore,
|
||||
)
|
||||
}
|
||||
|
||||
@ -1523,7 +1527,7 @@ impl<'a> EntityCommands<'a> {
|
||||
/// ```
|
||||
#[track_caller]
|
||||
pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self {
|
||||
self.queue_handled(entity_command::insert(bundle, InsertMode::Replace), ignore)
|
||||
self.queue_silenced(entity_command::insert(bundle, InsertMode::Replace))
|
||||
}
|
||||
|
||||
/// Adds a [`Bundle`] of components to the entity if the predicate returns true.
|
||||
@ -1579,7 +1583,7 @@ impl<'a> EntityCommands<'a> {
|
||||
/// the resulting error will be ignored.
|
||||
#[track_caller]
|
||||
pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self {
|
||||
self.queue_handled(entity_command::insert(bundle, InsertMode::Keep), ignore)
|
||||
self.queue_silenced(entity_command::insert(bundle, InsertMode::Keep))
|
||||
}
|
||||
|
||||
/// Removes a [`Bundle`] of components from the entity.
|
||||
@ -1724,7 +1728,7 @@ impl<'a> EntityCommands<'a> {
|
||||
/// # bevy_ecs::system::assert_is_system(remove_combat_stats_system);
|
||||
/// ```
|
||||
pub fn try_remove<B: Bundle>(&mut self) -> &mut Self {
|
||||
self.queue_handled(entity_command::remove::<B>(), ignore)
|
||||
self.queue_silenced(entity_command::remove::<B>())
|
||||
}
|
||||
|
||||
/// Removes a [`Bundle`] of components from the entity,
|
||||
@ -1818,7 +1822,7 @@ impl<'a> EntityCommands<'a> {
|
||||
///
|
||||
/// For example, this will recursively despawn [`Children`](crate::hierarchy::Children).
|
||||
pub fn try_despawn(&mut self) {
|
||||
self.queue_handled(entity_command::despawn(), ignore);
|
||||
self.queue_silenced(entity_command::despawn());
|
||||
}
|
||||
|
||||
/// Pushes an [`EntityCommand`] to the queue,
|
||||
@ -1907,6 +1911,18 @@ impl<'a> EntityCommands<'a> {
|
||||
self
|
||||
}
|
||||
|
||||
/// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`].
|
||||
///
|
||||
/// Unlike [`EntityCommands::queue_handled`], this will completely ignore any errors that occur.
|
||||
pub fn queue_silenced<C: EntityCommand<T> + CommandWithEntity<M>, T, M>(
|
||||
&mut self,
|
||||
command: C,
|
||||
) -> &mut Self {
|
||||
self.commands
|
||||
.queue_silenced(command.with_entity(self.entity));
|
||||
self
|
||||
}
|
||||
|
||||
/// Removes all components except the given [`Bundle`] from the entity.
|
||||
///
|
||||
/// # Example
|
||||
|
Loading…
Reference in New Issue
Block a user