From c82aa80b91139ffc3a94a3026c31e32abe1f1384 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 6 Jan 2025 22:40:14 -0800 Subject: [PATCH 01/11] Rework Command Error Handling --- benches/benches/bevy_ecs/world/commands.rs | 10 +- crates/bevy_ecs/Cargo.toml | 3 + .../bevy_ecs/src/reflect/entity_commands.rs | 24 +- .../bevy_ecs/src/system/commands/command.rs | 106 ++---- .../src/system/commands/entity_command.rs | 212 ++++++------ crates/bevy_ecs/src/system/commands/error.rs | 17 - .../src/system/commands/error_handler.rs | 47 ++- crates/bevy_ecs/src/system/commands/mod.rs | 319 ++++-------------- crates/bevy_ecs/src/world/command_queue.rs | 63 ++-- crates/bevy_ecs/src/world/entity_ref.rs | 41 ++- crates/bevy_hierarchy/src/child_builder.rs | 42 +-- crates/bevy_hierarchy/src/hierarchy.rs | 39 ++- crates/bevy_transform/src/commands.rs | 18 +- 13 files changed, 369 insertions(+), 572 deletions(-) delete mode 100644 crates/bevy_ecs/src/system/commands/error.rs diff --git a/benches/benches/bevy_ecs/world/commands.rs b/benches/benches/bevy_ecs/world/commands.rs index 632c9b163dfdc..6ff63b2e20416 100644 --- a/benches/benches/bevy_ecs/world/commands.rs +++ b/benches/benches/bevy_ecs/world/commands.rs @@ -2,7 +2,6 @@ use core::hint::black_box; use bevy_ecs::{ component::Component, - result::Result, system::{Command, Commands}, world::{CommandQueue, World}, }; @@ -137,18 +136,16 @@ struct FakeCommandA; struct FakeCommandB(u64); impl Command for FakeCommandA { - fn apply(self, world: &mut World) -> Result { + fn apply(self, world: &mut World) { black_box(self); black_box(world); - Ok(()) } } impl Command for FakeCommandB { - fn apply(self, world: &mut World) -> Result { + fn apply(self, world: &mut World) { black_box(self); black_box(world); - Ok(()) } } @@ -183,10 +180,9 @@ pub fn fake_commands(criterion: &mut Criterion) { struct SizedCommand(T); impl Command for SizedCommand { - fn apply(self, world: &mut World) -> Result { + fn apply(self, world: &mut World) { black_box(self); black_box(world); - Ok(()) } } diff --git a/crates/bevy_ecs/Cargo.toml b/crates/bevy_ecs/Cargo.toml index 54baa647b4db1..3f52188d1cee6 100644 --- a/crates/bevy_ecs/Cargo.toml +++ b/crates/bevy_ecs/Cargo.toml @@ -28,6 +28,9 @@ bevy_reflect = ["dep:bevy_reflect"] ## Extends reflection support to functions. reflect_functions = ["bevy_reflect", "bevy_reflect/functions"] +## Use the configurable global error handler as the default error handler +configurable_error_handler = [] + # Debugging Features ## Enables `tracing` integration, allowing spans and other metrics to be reported diff --git a/crates/bevy_ecs/src/reflect/entity_commands.rs b/crates/bevy_ecs/src/reflect/entity_commands.rs index a6346b3a69353..8586596c72368 100644 --- a/crates/bevy_ecs/src/reflect/entity_commands.rs +++ b/crates/bevy_ecs/src/reflect/entity_commands.rs @@ -169,10 +169,8 @@ pub trait ReflectCommandExt { impl ReflectCommandExt for EntityCommands<'_> { fn insert_reflect(&mut self, component: Box) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.insert_reflect(component); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.insert_reflect(component); }) } @@ -180,19 +178,15 @@ impl ReflectCommandExt for EntityCommands<'_> { &mut self, component: Box, ) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.insert_reflect_with_registry::(component); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.insert_reflect_with_registry::(component); }) } fn remove_reflect(&mut self, component_type_path: impl Into>) -> &mut Self { let component_type_path: Cow<'static, str> = component_type_path.into(); - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove_reflect(component_type_path); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.remove_reflect(component_type_path); }) } @@ -201,10 +195,8 @@ impl ReflectCommandExt for EntityCommands<'_> { component_type_path: impl Into>, ) -> &mut Self { let component_type_path: Cow<'static, str> = component_type_path.into(); - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove_reflect_with_registry::(component_type_path); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.remove_reflect_with_registry::(component_type_path); }) } } diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index eb2ba4ab77910..13b0445ae2590 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -12,9 +12,9 @@ use crate::{ entity::Entity, event::{Event, Events}, observer::TriggerTargets, - result::Result, + result::{Error, Result}, schedule::ScheduleLabel, - system::{CommandError, IntoSystem, Resource, SystemId, SystemInput}, + system::{error_handler, IntoSystem, Resource, SystemId, SystemInput}, world::{FromWorld, SpawnBatchIter, World}, }; @@ -45,91 +45,57 @@ use crate::{ /// commands.queue(AddToCounter(42)); /// } /// ``` -/// -/// # Note on Generic -/// -/// The `Marker` generic is necessary to allow multiple blanket implementations -/// of `Command` for closures, like so: -/// ```ignore (This would conflict with the real implementations) -/// impl Command for FnOnce(&mut World) -/// impl Command for FnOnce(&mut World) -> Result -/// ``` -/// Without the generic, Rust would consider the two implementations to be conflicting. -/// -/// The type used for `Marker` has no connection to anything else in the implementation. -pub trait Command: Send + 'static { +pub trait Command: Send + 'static { /// Applies this command, causing it to mutate the provided `world`. /// /// This method is used to define what a command "does" when it is ultimately applied. /// Because this method takes `self`, you can store data or settings on the type that implements this trait. /// This data is set by the system or other source of the command, and then ultimately read in this method. - fn apply(self, world: &mut World) -> Result; - - /// Applies this command and converts any resulting error into a [`CommandError`]. - /// - /// Overwriting this method allows an implementor to return a `CommandError` directly - /// and avoid erasing the error's type. - fn apply_internal(self, world: &mut World) -> Result<(), CommandError> - where - Self: Sized, - { - match self.apply(world) { - Ok(_) => Ok(()), - Err(error) => Err(CommandError::CommandFailed(error)), - } - } - - /// Returns a new [`Command`] that, when applied, will apply the original command - /// and pass any resulting error to the provided `error_handler`. - fn with_error_handling( - self, - error_handler: Option, - ) -> impl Command - where - Self: Sized, - { - move |world: &mut World| { - if let Err(error) = self.apply_internal(world) { - // TODO: Pass the error to the global error handler if `error_handler` is `None`. - let error_handler = error_handler.unwrap_or(|_, error| panic!("{error}")); - error_handler(world, error); - } - } - } + fn apply(self, world: &mut World) -> T; } -impl Command for F +impl Command for F where - F: FnOnce(&mut World) + Send + 'static, + F: FnOnce(&mut World) -> T + Send + 'static, { - fn apply(self, world: &mut World) -> Result { - self(world); - Ok(()) + fn apply(self, world: &mut World) -> T { + self(world) } } -impl Command for F -where - F: FnOnce(&mut World) -> Result + Send + 'static, -{ - fn apply(self, world: &mut World) -> Result { - self(world) +/// 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 { + /// 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(&mut World, Error)) -> 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(error_handler::default()) } } -/// Necessary to avoid erasing the type of the `CommandError` in -/// [`EntityCommand::with_entity`](crate::system::EntityCommand::with_entity). -impl Command<(Result, CommandError)> for F -where - F: FnOnce(&mut World) -> Result<(), CommandError> + Send + 'static, -{ - fn apply(self, world: &mut World) -> Result { - self(world)?; - Ok(()) +impl> HandleError for C { + fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl Command { + move |world: &mut World| match self.apply(world) { + Ok(_) => {} + Err(err) => (error_handler)(world, err), + } } +} - fn apply_internal(self, world: &mut World) -> Result<(), CommandError> { - self(world) +/// Takes a [`Command`] that returns a [`Result`] with an error that can be converted into the [`Error`] type +/// and returns a [`Command`] that internally converts that error to [`Error`] (if it occurs). +pub fn map_command_err>( + command: impl Command>, +) -> impl Command> { + move |world: &mut World| match command.apply(world) { + Ok(result) => Ok(result), + Err(err) => Err(err.into()), } } diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 5794fcb45631b..fcda65f225d02 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -15,11 +15,11 @@ use crate::{ component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityCloneBuilder}, event::Event, - result::Result, - system::{Command, CommandError, IntoObserverSystem}, - world::{EntityWorldMut, FromWorld, World}, + system::{error_handler, Command, IntoObserverSystem}, + world::{error::EntityFetchError, EntityWorldMut, FromWorld, World}, }; use bevy_ptr::OwningPtr; +use thiserror::Error; /// A [`Command`] which gets executed for a given [`Entity`]. /// @@ -41,14 +41,16 @@ use bevy_ptr::OwningPtr; /// struct Counter(i64); /// /// /// A `Command` which names an entity based on a global counter. -/// fn count_name(entity: Entity, world: &mut World) { +/// fn count_name(mut entity: EntityWorldMut) { /// // Get the current value of the counter, and increment it for next time. -/// let mut counter = world.resource_mut::(); -/// let i = counter.0; -/// counter.0 += 1; -/// +/// let i = { +/// let mut counter = entity.resource_mut::(); +/// let i = counter.0; +/// counter.0 += 1; +/// i +/// }; /// // Name the entity after the value of the counter. -/// world.entity_mut(entity).insert(Name::new(format!("Entity #{i}"))); +/// entity.insert(Name::new(format!("Entity #{i}"))); /// } /// /// // App creation boilerplate omitted... @@ -74,96 +76,117 @@ use bevy_ptr::OwningPtr; /// assert_eq!(names, HashSet::from_iter(["Entity #0", "Entity #1"])); /// } /// ``` -/// -/// # Note on Generic -/// -/// The `Marker` generic is necessary to allow multiple blanket implementations -/// of `EntityCommand` for closures, like so: -/// ```ignore (This would conflict with the real implementations) -/// impl EntityCommand for FnOnce(Entity, &mut World) -/// impl EntityCommand for FnOnce(EntityWorldMut) -/// impl EntityCommand for FnOnce(Entity, &mut World) -> Result -/// impl EntityCommand<(World, Result)> for FnOnce(EntityWorldMut) -> Result -/// ``` -/// Without the generic, Rust would consider the implementations to be conflicting. -/// -/// The type used for `Marker` has no connection to anything else in the implementation. -pub trait EntityCommand: Send + 'static { +pub trait EntityCommand: Send + 'static { /// Executes this command for the given [`Entity`] and /// returns a [`Result`] for error handling. - fn apply(self, entity: Entity, world: &mut World) -> Result; + fn apply(self, entity: EntityWorldMut) -> T; +} +/// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that +/// internally runs the [`EntityCommand`] on that entity. +pub trait CommandWithEntity { + /// 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; +} - /// Returns a [`Command`] which executes this [`EntityCommand`] for the given [`Entity`]. - /// - /// This method is called when adding an [`EntityCommand`] to a command queue via [`Commands`](crate::system::Commands). - /// You can override the provided implementation if you can return a `Command` with a smaller memory - /// footprint than `(Entity, Self)`. - /// In most cases the provided implementation is sufficient. - #[must_use = "commands do nothing unless applied to a `World`"] - fn with_entity(self, entity: Entity) -> impl Command<(Result, CommandError)> - where - Self: Sized, - { - move |world: &mut World| -> Result<(), CommandError> { - if world.entities().contains(entity) { - match self.apply(entity, world) { - Ok(_) => Ok(()), - Err(error) => Err(CommandError::CommandFailed(error)), - } - } else { - Err(CommandError::NoSuchEntity( - entity, - world - .entities() - .entity_does_not_exist_error_details_message(entity), - )) - } +impl> CommandWithEntity> for C { + fn with_entity(self, entity: Entity) -> impl Command> { + move |world: &mut World| { + let entity = world.get_entity_mut(entity)?; + self.apply(entity); + Ok(()) } } } -impl EntityCommand for F -where - F: FnOnce(Entity, &mut World) + Send + 'static, +impl>, T, Err> CommandWithEntity>> + for C { - fn apply(self, id: Entity, world: &mut World) -> Result { - self(id, world); - Ok(()) + fn with_entity(self, entity: Entity) -> impl Command>> { + move |world: &mut World| { + let entity = world.get_entity_mut(entity)?; + match self.apply(entity) { + Ok(result) => Ok(result), + Err(err) => Err(EntityCommandError::Error(err)), + } + } } } -impl EntityCommand for F -where - F: FnOnce(Entity, &mut World) -> Result + Send + 'static, -{ - fn apply(self, id: Entity, world: &mut World) -> Result { - self(id, world) +/// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into +/// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. +pub trait HandleEntityError { + /// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into + /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. + fn handle_error_with( + self, + error_handler: fn(&mut World, crate::result::Error), + ) -> impl EntityCommand; + /// Takes a [`EntityCommand`] that returns a Result and uses the default error handler function to convert it into + /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. + fn handle_error(self) -> impl EntityCommand + where + Self: Sized, + { + self.handle_error_with(error_handler::default()) } } -impl EntityCommand for F -where - F: FnOnce(EntityWorldMut) + Send + 'static, -{ - fn apply(self, id: Entity, world: &mut World) -> Result { - self(world.entity_mut(id)); - Ok(()) +impl> HandleEntityError for C { + fn handle_error_with( + self, + error_handler: fn(&mut World, crate::result::Error), + ) -> impl EntityCommand { + move |entity: EntityWorldMut| { + let location = entity.location(); + let id = entity.id(); + // This is broken up into parts so we can pass in the world to the error handler + // after EntityWorldMut is consumed + let world = entity.into_world_mut(); + // SAFETY: location has not changed and entity is valid + match self.apply(unsafe { EntityWorldMut::new(world, id, location) }) { + Ok(_) => {} + Err(err) => (error_handler)(world, err), + } + } } } -impl EntityCommand<(World, Result)> for F +/// Takes a [`EntityCommand`] that returns a [`Result`] with an error that can be converted into the [`Error`] type +/// and returns a [`EntityCommand`] that internally converts that error to [`Error`] (if it occurs). +pub fn map_entity_command_err>( + command: impl EntityCommand>, +) -> impl EntityCommand> { + move |entity: EntityWorldMut| match command.apply(entity) { + Ok(result) => Ok(result), + Err(err) => Err(err.into()), + } +} + +/// An error that occurs when running an [`EntityCommand`] on a specific entity. +#[derive(Error, Debug)] +pub enum EntityCommandError { + /// The entity this [`EntityCommand`] tried to run on could not be fetched. + #[error(transparent)] + EntityFetchError(#[from] EntityFetchError), + /// An error that occurred while running the [`EntityCommand`]. + #[error("{0}")] + Error(E), +} + +impl EntityCommand for F where - F: FnOnce(EntityWorldMut) -> Result + Send + 'static, + F: FnOnce(EntityWorldMut) -> T + Send + 'static, { - fn apply(self, id: Entity, world: &mut World) -> Result { - self(world.entity_mut(id)) + fn apply(self, entity: EntityWorldMut) -> T { + self(entity) } } /// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity, /// replacing any that were already present. #[track_caller] -pub fn insert(bundle: impl Bundle) -> impl EntityCommand { +pub fn insert(bundle: impl Bundle) -> impl EntityCommand { #[cfg(feature = "track_location")] let caller = Location::caller(); move |mut entity: EntityWorldMut| { @@ -179,7 +202,7 @@ pub fn insert(bundle: impl Bundle) -> impl EntityCommand { /// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity, /// except for any that were already present. #[track_caller] -pub fn insert_if_new(bundle: impl Bundle) -> impl EntityCommand { +pub fn insert_if_new(bundle: impl Bundle) -> impl EntityCommand { #[cfg(feature = "track_location")] let caller = Location::caller(); move |mut entity: EntityWorldMut| { @@ -194,10 +217,7 @@ pub fn insert_if_new(bundle: impl Bundle) -> impl EntityCommand { /// An [`EntityCommand`] that adds a dynamic component to an entity. #[track_caller] -pub fn insert_by_id( - component_id: ComponentId, - value: T, -) -> impl EntityCommand { +pub fn insert_by_id(component_id: ComponentId, value: T) -> impl EntityCommand { move |mut entity: EntityWorldMut| { // SAFETY: // - `component_id` safety is ensured by the caller @@ -214,9 +234,8 @@ pub fn insert_by_id( pub fn insert_from_world(mode: InsertMode) -> impl EntityCommand { #[cfg(feature = "track_location")] let caller = Location::caller(); - move |entity: Entity, world: &mut World| { - let value = T::from_world(world); - let mut entity = world.entity_mut(entity); + move |mut entity: EntityWorldMut| { + let value = entity.world_scope(|world| T::from_world(world)); entity.insert_with_caller( value, mode, @@ -227,7 +246,7 @@ pub fn insert_from_world(mode: InsertMode) -> impl Ent } /// An [`EntityCommand`] that removes the components in a [`Bundle`] from an entity. -pub fn remove() -> impl EntityCommand { +pub fn remove() -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.remove::(); } @@ -235,21 +254,21 @@ pub fn remove() -> impl EntityCommand { /// An [`EntityCommand`] that removes the components in a [`Bundle`] from an entity, /// as well as the required components for each component removed. -pub fn remove_with_requires() -> impl EntityCommand { +pub fn remove_with_requires() -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.remove_with_requires::(); } } /// An [`EntityCommand`] that removes a dynamic component from an entity. -pub fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { +pub fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.remove_by_id(component_id); } } /// An [`EntityCommand`] that removes all components from an entity. -pub fn clear() -> impl EntityCommand { +pub fn clear() -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.clear(); } @@ -257,7 +276,7 @@ pub fn clear() -> impl EntityCommand { /// An [`EntityCommand`] that removes all components from an entity, /// except for those in the given [`Bundle`]. -pub fn retain() -> impl EntityCommand { +pub fn retain() -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.retain::(); } @@ -269,7 +288,7 @@ pub fn retain() -> impl EntityCommand { /// /// This won't clean up external references to the entity (such as parent-child relationships /// if you're using `bevy_hierarchy`), which may leave the world in an invalid state. -pub fn despawn() -> impl EntityCommand { +pub fn despawn() -> impl EntityCommand { #[cfg(feature = "track_location")] let caller = Location::caller(); move |entity: EntityWorldMut| { @@ -284,7 +303,7 @@ pub fn despawn() -> impl EntityCommand { /// listening for events of type `E` targeting an entity pub fn observe( observer: impl IntoObserverSystem, -) -> impl EntityCommand { +) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.observe(observer); } @@ -295,7 +314,7 @@ pub fn observe( pub fn clone_with( target: Entity, config: impl FnOnce(&mut EntityCloneBuilder) + Send + Sync + 'static, -) -> impl EntityCommand { +) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.clone_with(target, config); } @@ -303,7 +322,7 @@ pub fn clone_with( /// An [`EntityCommand`] that clones the specified components of an entity /// and inserts them into another entity. -pub fn clone_components(target: Entity) -> impl EntityCommand { +pub fn clone_components(target: Entity) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.clone_components::(target); } @@ -311,7 +330,7 @@ pub fn clone_components(target: Entity) -> impl EntityCommand /// An [`EntityCommand`] that clones the specified components of an entity /// and inserts them into another entity, then removes them from the original entity. -pub fn move_components(target: Entity) -> impl EntityCommand { +pub fn move_components(target: Entity) -> impl EntityCommand { move |mut entity: EntityWorldMut| { entity.move_components::(target); } @@ -319,11 +338,12 @@ pub fn move_components(target: Entity) -> impl EntityCommand { /// An [`EntityCommand`] that logs the components of an entity. pub fn log_components() -> impl EntityCommand { - move |entity: Entity, world: &mut World| { - let debug_infos: Vec<_> = world - .inspect_entity(entity) + move |entity: EntityWorldMut| { + let debug_infos: Vec<_> = entity + .world() + .inspect_entity(entity.id()) .map(ComponentInfo::name) .collect(); - info!("Entity {entity}: {debug_infos:?}"); + info!("Entity {}: {debug_infos:?}", entity.id()); } } diff --git a/crates/bevy_ecs/src/system/commands/error.rs b/crates/bevy_ecs/src/system/commands/error.rs deleted file mode 100644 index d805742cbda6a..0000000000000 --- a/crates/bevy_ecs/src/system/commands/error.rs +++ /dev/null @@ -1,17 +0,0 @@ -//! This module contains the error type used by commands. - -use alloc::boxed::Box; -use thiserror::Error; - -use crate::entity::{Entity, EntityDoesNotExistDetails}; - -/// An error that occurs when executing a command. -#[derive(Error, Debug)] -pub enum CommandError { - /// The entity with the given ID does not exist. - #[error("Command failed because the entity with ID {0} {1}")] - NoSuchEntity(Entity, EntityDoesNotExistDetails), - /// The command returned an error. - #[error("Command returned an error: {0}")] - CommandFailed(Box), -} diff --git a/crates/bevy_ecs/src/system/commands/error_handler.rs b/crates/bevy_ecs/src/system/commands/error_handler.rs index 2b90dffc67a8e..7f4230fe05f3e 100644 --- a/crates/bevy_ecs/src/system/commands/error_handler.rs +++ b/crates/bevy_ecs/src/system/commands/error_handler.rs @@ -1,32 +1,55 @@ //! This module contains convenience functions that return simple error handlers -//! for use with the following methods: -//! - [`Commands::queue_fallible_with`](super::Commands::queue_fallible_with) -//! - [`Commands::override_error_handler`](super::Commands::override_error_handler) -//! - [`EntityCommands::queue_with`](super::EntityCommands::queue_with) -//! - [`EntityCommands::override_error_handler`](super::EntityCommands::override_error_handler) -//! - [`EntityEntryCommands::override_error_handler`](super::EntityEntryCommands::override_error_handler) +//! for use with [`HandleError::handle_error_with`](super::HandleError::handle_error_with) on instances of +//! [`Command`](super::Command) and [`EntityCommand`](super::EntityCommand) that return a Result with an +//! [`Error`]. +use crate::{result::Error, world::World}; use log::{error, warn}; -use crate::{system::CommandError, world::World}; - /// An error handler that does nothing. -pub fn silent() -> fn(&mut World, CommandError) { +pub fn silent() -> fn(&mut World, Error) { |_, _| {} } /// An error handler that accepts an error and logs it with [`warn!`]. -pub fn warn() -> fn(&mut World, CommandError) { +pub fn warn() -> fn(&mut World, Error) { |_, error| warn!("{error}") } /// An error handler that accepts an error and logs it with [`error!`]. -pub fn error() -> fn(&mut World, CommandError) { +pub fn error() -> fn(&mut World, Error) { |_, error| error!("{error}") } /// An error handler that accepts an error and panics with the error in /// the panic message. -pub fn panic() -> fn(&mut World, CommandError) { +pub fn panic() -> fn(&mut World, Error) { |_, error| panic!("{error}") } + +/// The default error handler. This defaults to [`panic`]. If the +/// `configurable_error_handler` cargo feature is enabled, then +/// `GLOBAL_ERROR_HANDLER` will be used instead, enabling error handler customization. +#[cfg(not(feature = "configurable_error_handler"))] +#[inline] +pub fn default() -> fn(&mut World, Error) { + panic() +} + +/// A global error handler. This can be set at startup, as long as it is set before +/// any uses. This should generally be configured _before_ initializing the app. +/// +/// If the `configurable_error_handler` cargo feature is enabled, this will be used +/// by default. +#[cfg(feature = "configurable_error_handler")] +pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = + std::sync::OnceLock::new(); + +/// The default error handler. This defaults to [`panic`]. If the +/// `configurable_error_handler` cargo feature is enabled, then +/// [`GLOBAL_ERROR_HANDLER`] will be used instead, enabling error handler customization. +#[cfg(feature = "configurable_error_handler")] +#[inline] +pub fn default() -> fn(&mut World, Error) { + *GLOBAL_ERROR_HANDLER.get_or_init(|| panic()) +} diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index fb3c8df5db2d6..b2f7a3c8f0658 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,14 +1,12 @@ pub mod command; pub mod entity_command; -pub mod error; pub mod error_handler; #[cfg(feature = "std")] mod parallel_scope; -pub use command::Command; -pub use entity_command::EntityCommand; -pub use error::CommandError; +pub use command::{Command, HandleError}; +pub use entity_command::{CommandWithEntity, EntityCommand}; #[cfg(feature = "std")] pub use parallel_scope::*; @@ -28,10 +26,11 @@ use crate::{ entity::{Entities, Entity, EntityCloneBuilder}, event::Event, observer::{Observer, TriggerTargets}, + result::{Error, Result}, schedule::ScheduleLabel, system::{ - input::SystemInput, Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, - SystemId, + command::map_command_err, entity_command::map_entity_command_err, input::SystemInput, + Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, SystemId, }, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue, @@ -92,24 +91,18 @@ use crate::{ /// /// # Error handling /// -/// Commands return a [`Result`](crate::result::Result), which gets passed to +/// Commands can return a [`Result`](crate::result::Result), which can be passed to /// an error handler. Error handlers are functions/closures of the form /// `fn(&mut World, CommandError)`. /// -/// By default, if a command returns an error, it will be passed to the -/// global error handler. Currently, the global error handler just panics; -/// in the future, this will be configurable. +/// Fallible commands should generally be passed to [`Commands::queue_fallible`], which will +/// use the default error handler. /// -/// [`Commands::override_error_handler`] allows you to override a [`Commands`] -/// instance's default error handler. This method takes an error handler -/// which will be used by all subsequent commands queued through either -/// [`Commands::queue_fallible`] or built-in commands' dedicated methods. +/// The default error handler panics. It can be configured by enabling the `configurable_error_handler` +/// cargo feature, then setting the `GLOBAL_ERROR_HANDLER`. /// -/// [`Commands::queue_fallible_with`] allows you to provide an error handler -/// directly to a command. This is unaffected by any default or override. -/// If you would like to use this method with built-in commands, -/// the [`command`] module provides unqueued forms of built-in commands -/// that you can queue manually. +/// Alternatively, you can customize the error handler for a specific command by calling [`HandleError::handle_error_with`] +/// on it, then passing the result to [`Commands::queue`]. /// /// The [`error_handler`] module provides some simple error handlers for convenience. /// @@ -117,12 +110,6 @@ use crate::{ pub struct Commands<'w, 's> { queue: InternalQueue<'s>, entities: &'w Entities, - /// This can be set using [`Commands::override_error_handler`] to override - /// the global error handler for all subsequent commands, which would be - /// more convenient than using [`Commands::queue_fallible_with`] to override - /// each command individually if you wanted to use the same error handler for - /// all of them. - error_handler_override: Option, } // SAFETY: All commands [`Command`] implement [`Send`] @@ -218,7 +205,6 @@ const _: () = { Commands { queue: InternalQueue::CommandQueue(f0), entities: f1, - error_handler_override: None, } } } @@ -255,7 +241,6 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::CommandQueue(Deferred(queue)), entities, - error_handler_override: None, } } @@ -273,7 +258,6 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::RawCommandQueue(queue), entities, - error_handler_override: None, } } @@ -304,7 +288,6 @@ impl<'w, 's> Commands<'w, 's> { } }, entities: self.entities, - error_handler_override: self.error_handler_override, } } @@ -573,12 +556,10 @@ impl<'w, 's> Commands<'w, 's> { /// - A custom struct that implements [`Command`]. /// - A closure or function that matches one of the following signatures: /// - [`(&mut World)`](World) - /// - [`(&mut World)`](World)`->`[`Result`](crate::result::Result) /// - A built-in command from the [`command`] module. /// - /// Commands can return a [`Result`], but this method will ignore them. - /// If you want to queue a command with error handling, - /// use [`Commands::queue_fallible`] or [`Commands::queue_fallible_with`]. + /// If you want to queue a fallible command with error handling, + /// use [`Commands::queue_fallible`]. /// /// # Example /// @@ -590,10 +571,9 @@ impl<'w, 's> Commands<'w, 's> { /// struct AddToCounter(u64); /// /// impl Command for AddToCounter { - /// fn apply(self, world: &mut World) -> Result { + /// fn apply(self, world: &mut World) { /// let mut counter = world.get_resource_or_insert_with(Counter::default); /// counter.0 += self.0; - /// Ok(()) /// } /// } /// @@ -609,7 +589,7 @@ impl<'w, 's> Commands<'w, 's> { /// # bevy_ecs::system::assert_is_system(add_three_to_counter_system); /// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system); /// ``` - pub fn queue, M: 'static>(&mut self, command: C) { + pub fn queue(&mut self, command: C) { match &mut self.queue { InternalQueue::CommandQueue(queue) => { queue.push(command); @@ -631,66 +611,9 @@ impl<'w, 's> Commands<'w, 's> { /// - A closure or function that has the signature [`(&mut World)`](World)`->`[`Result`](crate::result::Result). /// - A built-in command from the [`command`] module. /// - /// If the command returns an error, it will panic by default. You can use - /// [`Commands::queue_fallible_with`] to override an individual command's error handler, - /// or you can use [`Commands::override_error_handler`] to override the default - /// error handler for all subsequent commands queued by this [`Commands`] instance. - pub fn queue_fallible, M: 'static>(&mut self, command: C) { - self.queue(command.with_error_handling(self.error_handler_override)); - } - - /// Pushes a generic [`Command`] to the command queue with a particular error handler. - /// - /// The command can be: - /// - A custom struct that implements [`Command`]. - /// - A closure or function that has the signature [`(&mut World)`](World)`->`[`Result`](crate::result::Result). - /// - A built-in command from the [`command`] module. - /// - /// If the command returns an error, it will be passed to `error_handler`. - /// - /// See the [`command`] module for built-in fallible commands that can be - /// queued manually, as well as the [`error_handler`] module for simple - /// error handlers. - pub fn queue_fallible_with, M: 'static>( - &mut self, - command: C, - error_handler: fn(&mut World, CommandError), - ) { - self.queue(command.with_error_handling(Some(error_handler))); - } - - /// Pushes a generic [`Command`] to the command queue with error handling. - /// - /// If the command returns an error, it will be passed to the [`Commands`] instance's - /// error handler override if set, or `error_handler` otherwise. - // TODO: This is only useful for commands that fail differently (non-panic) by default, but - // still want to be overridden by the Commands instance's setting. It can be removed once - // all commands obey the global error handler by default. - fn queue_fallible_with_default, M: 'static>( - &mut self, - command: C, - error_handler: fn(&mut World, CommandError), - ) { - let error_handler = self.error_handler_override.unwrap_or(error_handler); - self.queue(command.with_error_handling(Some(error_handler))); - } - - /// Sets the [`Commands`] instance to use a custom error handler when encountering an error. - /// - /// This will apply to all subsequent commands. You can use [`Self::reset_error_handler`] to undo this. - /// - /// `fn()` can be a closure if it doesn't capture its environment. - pub fn override_error_handler(&mut self, error_handler: fn(&mut World, CommandError)) { - self.error_handler_override = Some(error_handler); - } - - /// Resets the [`Commands`] instance's error handler, allowing commands - /// to respond to errors in their default manner. - /// - /// This is only useful if the instance's error handler was previously overridden - /// by [`Self::override_error_handler`]. - pub fn reset_error_handler(&mut self) { - self.error_handler_override = None; + /// This will use the default error handler. See [`Command`] for details on error handling. + pub fn queue_fallible>(&mut self, command: impl Command>) { + self.queue(map_command_err(command).handle_error()); } /// Pushes a [`Command`] to the queue for creating entities, if needed, @@ -934,7 +857,7 @@ impl<'w, 's> Commands<'w, 's> { /// execution of the system happens later. To get the output of a system, use /// [`World::run_system`] or [`World::run_system_with`] instead of running the system as a command. pub fn run_system(&mut self, id: SystemId) { - self.queue_fallible_with_default(command::run_system(id), error_handler::warn()); + self.queue(command::run_system(id).handle_error_with(error_handler::warn())); } /// Runs the system corresponding to the given [`SystemId`]. @@ -950,10 +873,7 @@ impl<'w, 's> Commands<'w, 's> { where I: SystemInput: Send> + 'static, { - self.queue_fallible_with_default( - command::run_system_with(id, input), - error_handler::warn(), - ); + self.queue(command::run_system_with(id, input).handle_error_with(error_handler::warn())); } /// Registers a system and returns a [`SystemId`] so it can later be called by [`World::run_system`]. @@ -1027,10 +947,7 @@ impl<'w, 's> Commands<'w, 's> { I: SystemInput + Send + 'static, O: Send + 'static, { - self.queue_fallible_with_default( - command::unregister_system(system_id), - error_handler::warn(), - ); + self.queue(command::unregister_system(system_id).handle_error_with(error_handler::warn())); } /// Removes a system previously registered with [`World::register_system_cached`]. @@ -1045,9 +962,8 @@ impl<'w, 's> Commands<'w, 's> { &mut self, system: S, ) { - self.queue_fallible_with_default( - command::unregister_system_cached(system), - error_handler::warn(), + self.queue( + command::unregister_system_cached(system).handle_error_with(error_handler::warn()), ); } @@ -1059,7 +975,7 @@ impl<'w, 's> Commands<'w, 's> { &mut self, system: S, ) { - self.queue_fallible_with_default(command::run_system_cached(system), error_handler::warn()); + self.queue(command::run_system_cached(system).handle_error_with(error_handler::warn())); } /// Similar to [`Self::run_system_with`], but caching the [`SystemId`] in a @@ -1072,9 +988,8 @@ impl<'w, 's> Commands<'w, 's> { M: 'static, S: IntoSystem + Send + 'static, { - self.queue_fallible_with_default( - command::run_system_cached_with(system, input), - error_handler::warn(), + self.queue( + command::run_system_cached_with(system, input).handle_error_with(error_handler::warn()), ); } @@ -1167,7 +1082,7 @@ impl<'w, 's> Commands<'w, 's> { /// # assert_eq!(world.resource::().0, 1); /// ``` pub fn run_schedule(&mut self, label: impl ScheduleLabel) { - self.queue_fallible_with_default(command::run_schedule(label), error_handler::warn()); + self.queue(command::run_schedule(label).handle_error_with(error_handler::warn())); } } @@ -1189,24 +1104,18 @@ impl<'w, 's> Commands<'w, 's> { /// /// # Error handling /// -/// Entity commands return a [`Result`](crate::result::Result), which gets passed to +/// [`EntityCommands`] can return a [`Result`](crate::result::Result), which can be passed to /// an error handler. Error handlers are functions/closures of the form /// `fn(&mut World, CommandError)`. /// -/// By default, if a command returns an error, it will be passed to the -/// global error handler. Currently, the global error handler just panics; -/// in the future, this will be configurable. +/// Fallible commands should generally be passed to [`EntityCommands::queue_fallible`], which will +/// use the default error handler. /// -/// [`EntityCommands::override_error_handler`] allows you to override an [`EntityCommands`] -/// instance's default error handler. This method takes an error handler -/// which will be used by all subsequent commands queued through either -/// [`EntityCommands::queue`] or built-in commands' dedicated methods. +/// The default error handler panics. It can be configured by enabling the `configurable_error_handler` +/// cargo feature, then setting the `GLOBAL_ERROR_HANDLER`. /// -/// [`EntityCommands::queue_with`] allows you to provide an error handler -/// directly to a command. This is unaffected by any default or override. -/// If you would like to use this method with built-in commands, -/// the [`entity_command`] module provides unqueued forms of built-in entity commands -/// that you can queue manually. +/// Alternatively, you can customize the error handler for a specific command by calling [`HandleError::handle_error_with`] +/// on it, then passing the result to [`EntityCommands::queue`]. /// /// The [`error_handler`] module provides some simple error handlers for convenience. pub struct EntityCommands<'a> { @@ -1454,10 +1363,7 @@ impl<'a> EntityCommands<'a> { component_id: ComponentId, value: T, ) -> &mut Self { - self.queue_with_default( - entity_command::insert_by_id(component_id, value), - error_handler::silent(), - ) + self.queue(entity_command::insert_by_id(component_id, value)) } /// Tries to add a [`Bundle`] of components to the entity. @@ -1510,7 +1416,7 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue_with_default(entity_command::insert(bundle), error_handler::silent()) + self.queue(entity_command::insert(bundle)) } /// Similar to [`Self::try_insert`] but will only try to insert if the predicate returns true. @@ -1609,10 +1515,7 @@ impl<'a> EntityCommands<'a> { /// Unlike [`Self::insert_if_new`], this will not panic if the associated entity does not exist. #[track_caller] pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue_with_default( - entity_command::insert_if_new(bundle), - error_handler::silent(), - ) + self.queue(entity_command::insert_if_new(bundle)) } /// Removes a [`Bundle`] of components from the entity. @@ -1654,7 +1557,7 @@ impl<'a> EntityCommands<'a> { where T: Bundle, { - self.queue_with_default(entity_command::remove::(), error_handler::silent()) + self.queue(entity_command::remove::()) } /// Removes all components in the [`Bundle`] components and remove all required components for each component in the [`Bundle`] from entity. @@ -1682,10 +1585,7 @@ impl<'a> EntityCommands<'a> { /// # bevy_ecs::system::assert_is_system(remove_with_requires_system); /// ``` pub fn remove_with_requires(&mut self) -> &mut Self { - self.queue_with_default( - entity_command::remove_with_requires::(), - error_handler::silent(), - ) + self.queue(entity_command::remove_with_requires::()) } /// Removes a dynamic [`Component`] from the entity if it exists. @@ -1694,15 +1594,12 @@ impl<'a> EntityCommands<'a> { /// /// Panics if the provided [`ComponentId`] does not exist in the [`World`]. pub fn remove_by_id(&mut self, component_id: ComponentId) -> &mut Self { - self.queue_with_default( - entity_command::remove_by_id(component_id), - error_handler::silent(), - ) + self.queue(entity_command::remove_by_id(component_id)) } /// Removes all components associated with the entity. pub fn clear(&mut self) -> &mut Self { - self.queue_with_default(entity_command::clear(), error_handler::silent()) + self.queue(entity_command::clear()) } /// Despawns the entity. @@ -1735,7 +1632,7 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn despawn(&mut self) { - self.queue_with_default(entity_command::despawn(), error_handler::warn()); + self.queue(entity_command::despawn()); } /// Despawns the entity. @@ -1744,29 +1641,17 @@ impl<'a> EntityCommands<'a> { /// the same function as [`Self::despawn`] without emitting warnings. #[track_caller] pub fn try_despawn(&mut self) { - self.queue_with_default(entity_command::despawn(), error_handler::silent()); + self.queue(entity_command::despawn()); } /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. /// /// The command can be: /// - A custom struct that implements [`EntityCommand`]. - /// - A closure or function that matches one of the following signatures: - /// - `(Entity, &mut World)` - /// - `(Entity, &mut World) ->`[`Result`](crate::result::Result) + /// - A closure or function that matches the following signature: /// - [`(EntityWorldMut)`](EntityWorldMut) - /// - [`(EntityWorldMut)`](EntityWorldMut)`->`[`Result`](crate::result::Result) /// - A built-in command from the [`entity_command`] module. /// - /// All entity commands are fallible, because they must return an error if the entity - /// doesn't exist when the command is executed. Therefore, all entity commands are - /// queued with error handling. - /// - /// If the command returns an error, it will panic by default. You can use - /// [`EntityCommands::queue_with`] to override an individual command's error handler, - /// or you can use [`EntityCommands::override_error_handler`] to override the default - /// error handler for all subsequent commands queued by this [`EntityCommands`] instance. - /// /// # Examples /// /// ``` @@ -1781,83 +1666,25 @@ impl<'a> EntityCommands<'a> { /// # } /// # bevy_ecs::system::assert_is_system(my_system); /// ``` - pub fn queue(&mut self, command: impl EntityCommand) -> &mut Self { + pub fn queue(&mut self, command: impl EntityCommand) -> &mut Self { self.commands .queue_fallible(command.with_entity(self.entity)); self } - /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. + /// Pushes a fallible [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. /// /// The command can be: /// - A custom struct that implements [`EntityCommand`]. - /// - A closure or function that matches one of the following signatures: - /// - `(Entity, &mut World)` - /// - `(Entity, &mut World) ->`[`Result`](crate::result::Result) - /// - [`(EntityWorldMut)`](EntityWorldMut) - /// - [`(EntityWorldMut)`](EntityWorldMut)`->`[`Result`](crate::result::Result) + /// - A closure or function that matches the following signature: + /// - [`(EntityWorldMut)`](EntityWorldMut)`->` [`Result<(), E>`] /// - A built-in command from the [`entity_command`] module. - /// - /// All entity commands are fallible, because they must return an error if the entity - /// doesn't exist when the command is executed. Therefore, all entity commands are - /// queued with error handling. - /// - /// If the command returns an error, it will be passed to `error_handler`. - /// - /// See the [`entity_command`] module for built-in entity commands that can be - /// queued manually, as well as the [`error_handler`] module for simple - /// error handlers. - pub fn queue_with( + pub fn queue_fallible>( &mut self, - command: impl EntityCommand, - error_handler: fn(&mut World, CommandError), + command: impl EntityCommand>, ) -> &mut Self { self.commands - .queue_fallible_with(command.with_entity(self.entity), error_handler); - self - } - - /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. - /// - /// If the command returns an error, it will be passed to the [`EntityCommands`] instance's - /// error handler override if set, or `default_error_handler` otherwise. - // TODO: This is only useful for commands that fail differently (non-panic) by default, but - // still want to be overridden by the EntityCommands instance's setting. It can be removed once - // all commands obey the global error handler by default. - fn queue_with_default( - &mut self, - command: impl EntityCommand, - default_error_handler: fn(&mut World, CommandError), - ) -> &mut Self { - let error_handler = self - .commands - .error_handler_override - .unwrap_or(default_error_handler); - self.commands - .queue_fallible_with(command.with_entity(self.entity), error_handler); - self - } - - /// Sets the [`EntityCommands`] instance to use a custom error handler when encountering an error. - /// - /// This will apply to all subsequent commands. You can use [`Self::reset_error_handler`] to undo this. - /// - /// `fn()` can be a closure if it doesn't capture its environment. - pub fn override_error_handler( - &mut self, - error_handler: fn(&mut World, CommandError), - ) -> &mut Self { - self.commands.override_error_handler(error_handler); - self - } - - /// Resets the [`EntityCommands`] instance's error handler, allowing commands - /// to respond to errors in their default manner. - /// - /// This is only useful if the instance's error handler was previously overridden - /// by [`Self::override_error_handler`]. - pub fn reset_error_handler(&mut self) -> &mut Self { - self.commands.reset_error_handler(); + .queue_fallible(map_entity_command_err(command).with_entity(self.entity)); self } @@ -1902,7 +1729,7 @@ impl<'a> EntityCommands<'a> { where T: Bundle, { - self.queue_with_default(entity_command::retain::(), error_handler::panic()) + self.queue(entity_command::retain::()) } /// Logs the components of the entity at the info level. @@ -1938,7 +1765,7 @@ impl<'a> EntityCommands<'a> { &mut self, observer: impl IntoObserverSystem, ) -> &mut Self { - self.queue_with_default(entity_command::observe(observer), error_handler::silent()) + self.queue(entity_command::observe(observer)) } /// Clones parts of an entity (components, observers, etc.) onto another entity, @@ -1986,10 +1813,7 @@ impl<'a> EntityCommands<'a> { target: Entity, config: impl FnOnce(&mut EntityCloneBuilder) + Send + Sync + 'static, ) -> &mut Self { - self.queue_with_default( - entity_command::clone_with(target, config), - error_handler::silent(), - ) + self.queue(entity_command::clone_with(target, config)) } /// Spawns a clone of this entity and returns the [`EntityCommands`] of the clone. @@ -2085,10 +1909,7 @@ impl<'a> EntityCommands<'a> { /// /// The command will panic when applied if the target entity does not exist. pub fn clone_components(&mut self, target: Entity) -> &mut Self { - self.queue_with_default( - entity_command::clone_components::(target), - error_handler::silent(), - ) + self.queue(entity_command::clone_components::(target)) } /// Clones the specified components of this entity and inserts them into another entity, @@ -2101,10 +1922,7 @@ impl<'a> EntityCommands<'a> { /// /// The command will panic when applied if the target entity does not exist. pub fn move_components(&mut self, target: Entity) -> &mut Self { - self.queue_with_default( - entity_command::move_components::(target), - error_handler::silent(), - ) + self.queue(entity_command::move_components::(target)) } } @@ -2128,29 +1946,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { } impl<'a, T: Component> EntityEntryCommands<'a, T> { - /// Sets the [`EntityEntryCommands`] instance to use a custom error handler when encountering an error. - /// - /// This will apply to all subsequent commands. You can use [`Self::reset_error_handler`] to undo this. - /// - /// `fn()` can be a closure if it doesn't capture its environment. - pub fn override_error_handler( - &mut self, - error_handler: fn(&mut World, CommandError), - ) -> &mut Self { - self.entity_commands.override_error_handler(error_handler); - self - } - - /// Resets the [`EntityEntryCommands`] instance's error handler, allowing commands - /// to respond to errors in their default manner. - /// - /// This is only useful if the instance's error handler was previously overridden - /// by [`Self::override_error_handler`]. - pub fn reset_error_handler(&mut self) -> &mut Self { - self.entity_commands.reset_error_handler(); - self - } - /// [Insert](EntityCommands::insert) `default` into this entity, if `T` is not already present. /// /// See also [`or_insert_with`](Self::or_insert_with). diff --git a/crates/bevy_ecs/src/world/command_queue.rs b/crates/bevy_ecs/src/world/command_queue.rs index 63c6bcf2d180e..76b9b6eddb9ec 100644 --- a/crates/bevy_ecs/src/world/command_queue.rs +++ b/crates/bevy_ecs/src/world/command_queue.rs @@ -1,21 +1,17 @@ -use crate::system::{SystemBuffer, SystemMeta}; - +use crate::{ + system::{Command, SystemBuffer, SystemMeta}, + world::{DeferredWorld, World}, +}; +use alloc::{boxed::Box, vec::Vec}; +use bevy_ptr::{OwningPtr, Unaligned}; use core::{ fmt::Debug, - marker::PhantomData, mem::{size_of, MaybeUninit}, panic::AssertUnwindSafe, ptr::{addr_of_mut, NonNull}, }; - -use alloc::{boxed::Box, vec::Vec}; -use bevy_ptr::{OwningPtr, Unaligned}; use log::warn; -use crate::{system::Command, world::World}; - -use super::DeferredWorld; - struct CommandMeta { /// SAFETY: The `value` must point to a value of type `T: Command`, /// where `T` is some specific type that was used to produce this metadata. @@ -76,11 +72,7 @@ unsafe impl Sync for CommandQueue {} impl CommandQueue { /// Push a [`Command`] onto the queue. #[inline] - pub fn push(&mut self, command: C) - where - C: Command, - M: 'static, - { + pub fn push(&mut self, command: impl Command) { // SAFETY: self is guaranteed to live for the lifetime of this method unsafe { self.get_raw().push(command); @@ -156,23 +148,14 @@ impl RawCommandQueue { /// /// * Caller ensures that `self` has not outlived the underlying queue #[inline] - pub unsafe fn push(&mut self, command: C) - where - C: Command, - M: 'static, - { + pub unsafe fn push(&mut self, command: C) { // Stores a command alongside its metadata. // `repr(C)` prevents the compiler from reordering the fields, // while `repr(packed)` prevents the compiler from inserting padding bytes. #[repr(C, packed)] - struct Packed - where - C: Command, - M: 'static, - { + struct Packed { meta: CommandMeta, command: C, - phantom: PhantomData, } let meta = CommandMeta { @@ -187,7 +170,7 @@ impl RawCommandQueue { Some(mut world) => { // SAFETY: Caller ensures pointer is not null let world = unsafe { world.as_mut() }; - _ = command.apply(world); + command.apply(world); // The command may have queued up world commands, which we flush here to ensure they are also picked up. // If the current command queue already the World Command queue, this will still behave appropriately because the global cursor // is still at the current `stop`, ensuring only the newly queued Commands will be applied. @@ -205,7 +188,7 @@ impl RawCommandQueue { let old_len = bytes.len(); // Reserve enough bytes for both the metadata and the command itself. - bytes.reserve(size_of::>()); + bytes.reserve(size_of::>()); // Pointer to the bytes at the end of the buffer. // SAFETY: We know it is within bounds of the allocation, due to the call to `.reserve()`. @@ -217,18 +200,15 @@ impl RawCommandQueue { // The call to `reserve()` ensures that the buffer has enough space to fit a value of type `C`, // and it is valid to write any bit pattern since the underlying buffer is of type `MaybeUninit`. unsafe { - ptr.cast::>().write_unaligned(Packed { - meta, - command, - phantom: PhantomData, - }); + ptr.cast::>() + .write_unaligned(Packed { meta, command }); } // Extend the length of the buffer to include the data we just wrote. // SAFETY: The new length is guaranteed to fit in the vector's capacity, // due to the call to `.reserve()` above. unsafe { - bytes.set_len(old_len + size_of::>()); + bytes.set_len(old_len + size_of::>()); } } @@ -355,7 +335,7 @@ impl SystemBuffer for CommandQueue { #[cfg(test)] mod test { use super::*; - use crate::{self as bevy_ecs, result::Result, system::Resource}; + use crate::{self as bevy_ecs, system::Resource}; use alloc::{borrow::ToOwned, string::String, sync::Arc}; use core::{ panic::AssertUnwindSafe, @@ -381,9 +361,7 @@ mod test { } impl Command for DropCheck { - fn apply(self, _: &mut World) -> Result { - Ok(()) - } + fn apply(self, _: &mut World) {} } #[test] @@ -430,9 +408,8 @@ mod test { struct SpawnCommand; impl Command for SpawnCommand { - fn apply(self, world: &mut World) -> Result { + fn apply(self, world: &mut World) { world.spawn_empty(); - Ok(()) } } @@ -460,7 +437,7 @@ mod test { #[allow(dead_code)] struct PanicCommand(String); impl Command for PanicCommand { - fn apply(self, _: &mut World) -> Result { + fn apply(self, _: &mut World) { panic!("command is panicking"); } } @@ -536,9 +513,7 @@ mod test { #[allow(dead_code)] struct CommandWithPadding(u8, u16); impl Command for CommandWithPadding { - fn apply(self, _: &mut World) -> Result { - Ok(()) - } + fn apply(self, _: &mut World) {} } #[cfg(miri)] diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index abcabc3891e81..507448a96fecd 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -11,7 +11,7 @@ use crate::{ query::{Access, ReadOnlyQueryData}, removal_detection::RemovedComponentEvents, storage::Storages, - system::IntoObserverSystem, + system::{IntoObserverSystem, Resource}, world::{error::EntityComponentError, DeferredWorld, Mut, World}, }; use alloc::vec::Vec; @@ -1322,6 +1322,45 @@ impl<'w> EntityWorldMut<'w> { unsafe { self.into_unsafe_entity_cell().get_mut() } } + /// Gets a reference to the resource of the given type + /// + /// # Panics + /// + /// Panics if the resource does not exist. + /// Use [`get_resource`](EntityWorldMut::get_resource) instead if you want to handle this case. + #[inline] + #[track_caller] + pub fn resource(&self) -> &R { + self.world.resource::() + } + + /// Gets a mutable reference to the resource of the given type + /// + /// # Panics + /// + /// Panics if the resource does not exist. + /// Use [`get_resource_mut`](World::get_resource_mut) instead if you want to handle this case. + /// + /// If you want to instead insert a value if the resource does not exist, + /// use [`get_resource_or_insert_with`](World::get_resource_or_insert_with). + #[inline] + #[track_caller] + pub fn resource_mut(&mut self) -> Mut<'_, R> { + self.world.resource_mut::() + } + + /// Gets a reference to the resource of the given type if it exists + #[inline] + pub fn get_resource(&self) -> Option<&R> { + self.world.get_resource() + } + + /// Gets a mutable reference to the resource of the given type if it exists + #[inline] + pub fn get_resource_mut(&mut self) -> Option> { + self.world.get_resource_mut() + } + /// Retrieves the change ticks for the given component. This can be useful for implementing change /// detection in custom runtimes. /// diff --git a/crates/bevy_hierarchy/src/child_builder.rs b/crates/bevy_hierarchy/src/child_builder.rs index 9684148bdb384..a0fe9549ebd5c 100644 --- a/crates/bevy_hierarchy/src/child_builder.rs +++ b/crates/bevy_hierarchy/src/child_builder.rs @@ -351,15 +351,15 @@ impl BuildChildren for EntityCommands<'_> { if children.contains(&parent) { panic!("Entity cannot be a child of itself."); } - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).add_children(&children); + self.queue(move |mut entity: EntityWorldMut| { + entity.add_children(&children); }) } fn with_child(&mut self, bundle: B) -> &mut Self { let child = self.commands().spawn(bundle).id(); - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).add_child(child); + self.queue(move |mut entity: EntityWorldMut| { + entity.add_child(child); }) } @@ -369,8 +369,8 @@ impl BuildChildren for EntityCommands<'_> { panic!("Cannot add entity as a child of itself."); } let children = SmallVec::<[Entity; 8]>::from_slice(children); - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).add_children(&children); + self.queue(move |mut entity: EntityWorldMut| { + entity.add_children(&children); }) } @@ -380,15 +380,15 @@ impl BuildChildren for EntityCommands<'_> { panic!("Cannot insert entity as a child of itself."); } let children = SmallVec::<[Entity; 8]>::from_slice(children); - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).insert_children(index, &children); + self.queue(move |mut entity: EntityWorldMut| { + entity.insert_children(index, &children); }) } fn remove_children(&mut self, children: &[Entity]) -> &mut Self { let children = SmallVec::<[Entity; 8]>::from_slice(children); - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).remove_children(&children); + self.queue(move |mut entity: EntityWorldMut| { + entity.remove_children(&children); }) } @@ -397,14 +397,14 @@ impl BuildChildren for EntityCommands<'_> { if child == parent { panic!("Cannot add entity as a child of itself."); } - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).add_child(child); + self.queue(move |mut entity: EntityWorldMut| { + entity.add_child(child); }) } fn clear_children(&mut self) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).clear_children(); + self.queue(move |mut entity: EntityWorldMut| { + entity.clear_children(); }) } @@ -414,8 +414,8 @@ impl BuildChildren for EntityCommands<'_> { panic!("Cannot replace entity as a child of itself."); } let children = SmallVec::<[Entity; 8]>::from_slice(children); - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).replace_children(&children); + self.queue(move |mut entity: EntityWorldMut| { + entity.replace_children(&children); }) } @@ -424,14 +424,16 @@ impl BuildChildren for EntityCommands<'_> { if child == parent { panic!("Cannot set parent to itself"); } - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(parent).add_child(entity); + self.queue(move |mut entity: EntityWorldMut| { + entity.world_scope(|world| { + world.entity_mut(parent).add_child(child); + }); }) } fn remove_parent(&mut self) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - world.entity_mut(entity).remove_parent(); + self.queue(move |mut entity: EntityWorldMut| { + entity.remove_parent(); }) } } diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index a71b7def8ad82..247eef48fc06b 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -68,31 +68,37 @@ impl DespawnRecursiveExt for EntityCommands<'_> { /// This will emit warnings for any entity that does not exist. fn despawn_recursive(mut self) { let warn = true; - self.queue(move |entity: Entity, world: &mut World| { + self.queue(move |mut entity: EntityWorldMut| { + let id = entity.id(); #[cfg(feature = "trace")] let _span = tracing::info_span!( "command", name = "DespawnRecursive", - entity = tracing::field::debug(entity), + entity = tracing::field::debug(id), warn = tracing::field::debug(warn) ) .entered(); - despawn_with_children_recursive(world, entity, warn); + entity.world_scope(|world| { + despawn_with_children_recursive(world, id, warn); + }); }); } fn despawn_descendants(&mut self) -> &mut Self { let warn = true; - self.queue(move |entity: Entity, world: &mut World| { + self.queue(move |mut entity: EntityWorldMut| { + let id = entity.id(); #[cfg(feature = "trace")] let _span = tracing::info_span!( "command", name = "DespawnChildrenRecursive", - entity = tracing::field::debug(entity), + entity = tracing::field::debug(id), warn = tracing::field::debug(warn) ) .entered(); - despawn_children_recursive(world, entity, warn); + entity.world_scope(|world| { + despawn_children_recursive(world, id, warn); + }); }); self } @@ -101,31 +107,37 @@ impl DespawnRecursiveExt for EntityCommands<'_> { /// This will never emit warnings. fn try_despawn_recursive(mut self) { let warn = false; - self.queue(move |entity: Entity, world: &mut World| { + self.queue(move |mut entity: EntityWorldMut| { + let id = entity.id(); #[cfg(feature = "trace")] let _span = tracing::info_span!( "command", name = "TryDespawnRecursive", - entity = tracing::field::debug(entity), + entity = tracing::field::debug(id), warn = tracing::field::debug(warn) ) .entered(); - despawn_with_children_recursive(world, entity, warn); + entity.world_scope(|world| { + despawn_with_children_recursive(world, id, warn); + }); }); } fn try_despawn_descendants(&mut self) -> &mut Self { let warn = false; - self.queue(move |entity: Entity, world: &mut World| { + self.queue(move |mut entity: EntityWorldMut| { + let id = entity.id(); #[cfg(feature = "trace")] let _span = tracing::info_span!( "command", name = "TryDespawnChildrenRecursive", - entity = tracing::field::debug(entity), + entity = tracing::field::debug(id), warn = tracing::field::debug(warn) ) .entered(); - despawn_children_recursive(world, entity, warn); + entity.world_scope(|world| { + despawn_children_recursive(world, id, warn); + }); }); self } @@ -250,7 +262,7 @@ mod tests { use alloc::{borrow::ToOwned, string::String, vec, vec::Vec}; use bevy_ecs::{ component::Component, - system::{error_handler, Commands}, + system::Commands, world::{CommandQueue, World}, }; @@ -310,7 +322,6 @@ mod tests { { let mut commands = Commands::new(&mut queue, &world); - commands.override_error_handler(error_handler::silent()); commands.entity(parent_entity).despawn_recursive(); // despawning the same entity twice should not panic commands.entity(parent_entity).despawn_recursive(); diff --git a/crates/bevy_transform/src/commands.rs b/crates/bevy_transform/src/commands.rs index 4ea8f0d1f737a..dab624566910f 100644 --- a/crates/bevy_transform/src/commands.rs +++ b/crates/bevy_transform/src/commands.rs @@ -2,11 +2,7 @@ //! while preserving [`GlobalTransform`]. use crate::prelude::{GlobalTransform, Transform}; -use bevy_ecs::{ - entity::Entity, - system::EntityCommands, - world::{EntityWorldMut, World}, -}; +use bevy_ecs::{entity::Entity, system::EntityCommands, world::EntityWorldMut}; use bevy_hierarchy::BuildChildren; /// Collection of methods similar to [`BuildChildren`], but preserving each @@ -35,18 +31,14 @@ pub trait BuildChildrenTransformExt { impl BuildChildrenTransformExt for EntityCommands<'_> { fn set_parent_in_place(&mut self, parent: Entity) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.set_parent_in_place(parent); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.set_parent_in_place(parent); }) } fn remove_parent_in_place(&mut self) -> &mut Self { - self.queue(move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove_parent_in_place(); - } + self.queue(move |mut entity: EntityWorldMut| { + entity.remove_parent_in_place(); }) } } From c0b827b4b6b5534ef69ade10b2b4c3529aa6a39c Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 7 Jan 2025 17:44:46 -0800 Subject: [PATCH 02/11] Merge queue and queue_fallible --- .../bevy_ecs/src/system/commands/command.rs | 25 ++++---- .../src/system/commands/entity_command.rs | 58 +++++++++++++------ crates/bevy_ecs/src/system/commands/mod.rs | 40 +++---------- 3 files changed, 60 insertions(+), 63 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 13b0445ae2590..8cb70f73c5e76 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -65,7 +65,7 @@ where /// 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 { +pub trait HandleError { /// 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(&mut World, Error)) -> impl Command; @@ -79,23 +79,26 @@ pub trait HandleError { } } -impl> HandleError for C { +impl>, T, E: Into> HandleError> for C { fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl Command { move |world: &mut World| match self.apply(world) { Ok(_) => {} - Err(err) => (error_handler)(world, err), + Err(err) => (error_handler)(world, err.into()), } } } -/// Takes a [`Command`] that returns a [`Result`] with an error that can be converted into the [`Error`] type -/// and returns a [`Command`] that internally converts that error to [`Error`] (if it occurs). -pub fn map_command_err>( - command: impl Command>, -) -> impl Command> { - move |world: &mut World| match command.apply(world) { - Ok(result) => Ok(result), - Err(err) => Err(err.into()), +impl HandleError for C { + #[inline] + fn handle_error_with(self, _error_handler: fn(&mut World, Error)) -> impl Command { + self + } + #[inline] + fn handle_error(self) -> impl Command + where + Self: Sized, + { + self } } diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index fcda65f225d02..ea93dcd22ebd2 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -15,7 +15,7 @@ use crate::{ component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityCloneBuilder}, event::Event, - system::{error_handler, Command, IntoObserverSystem}, + system::{error_handler, Command, HandleError, IntoObserverSystem}, world::{error::EntityFetchError, EntityWorldMut, FromWorld, World}, }; use bevy_ptr::OwningPtr; @@ -86,12 +86,16 @@ pub trait EntityCommand: Send + 'static { pub trait CommandWithEntity { /// 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; + fn with_entity(self, entity: Entity) -> impl Command + HandleError; } -impl> CommandWithEntity> for C { - fn with_entity(self, entity: Entity) -> impl Command> { - move |world: &mut World| { +impl CommandWithEntity> for C { + fn with_entity( + self, + entity: Entity, + ) -> impl Command> + HandleError> + { + move |world: &mut World| -> Result<(), EntityFetchError> { let entity = world.get_entity_mut(entity)?; self.apply(entity); Ok(()) @@ -99,10 +103,17 @@ impl> CommandWithEntity> for C } } -impl>, T, Err> CommandWithEntity>> - for C +impl< + C: EntityCommand>, + T, + Err: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static, + > CommandWithEntity>> for C { - fn with_entity(self, entity: Entity) -> impl Command>> { + fn with_entity( + self, + entity: Entity, + ) -> impl Command>> + HandleError>> + { move |world: &mut World| { let entity = world.get_entity_mut(entity)?; match self.apply(entity) { @@ -115,7 +126,7 @@ impl>, T, Err> CommandWithEntity { /// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. fn handle_error_with( @@ -132,7 +143,9 @@ pub trait HandleEntityError { } } -impl> HandleEntityError for C { +impl>, T, E: Into> + HandleEntityError> for C +{ fn handle_error_with( self, error_handler: fn(&mut World, crate::result::Error), @@ -146,20 +159,27 @@ impl> HandleEntityError for C { // SAFETY: location has not changed and entity is valid match self.apply(unsafe { EntityWorldMut::new(world, id, location) }) { Ok(_) => {} - Err(err) => (error_handler)(world, err), + Err(err) => (error_handler)(world, err.into()), } } } } -/// Takes a [`EntityCommand`] that returns a [`Result`] with an error that can be converted into the [`Error`] type -/// and returns a [`EntityCommand`] that internally converts that error to [`Error`] (if it occurs). -pub fn map_entity_command_err>( - command: impl EntityCommand>, -) -> impl EntityCommand> { - move |entity: EntityWorldMut| match command.apply(entity) { - Ok(result) => Ok(result), - Err(err) => Err(err.into()), +impl HandleEntityError for C { + #[inline] + fn handle_error_with( + self, + _error_handler: fn(&mut World, crate::result::Error), + ) -> impl EntityCommand { + self + } + + #[inline] + fn handle_error(self) -> impl EntityCommand + where + Self: Sized, + { + self } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index b2f7a3c8f0658..e6f564938f084 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -26,11 +26,10 @@ use crate::{ entity::{Entities, Entity, EntityCloneBuilder}, event::Event, observer::{Observer, TriggerTargets}, - result::{Error, Result}, schedule::ScheduleLabel, system::{ - command::map_command_err, entity_command::map_entity_command_err, input::SystemInput, - Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, SystemId, + input::SystemInput, Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, + SystemId, }, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue, @@ -589,7 +588,8 @@ impl<'w, 's> Commands<'w, 's> { /// # bevy_ecs::system::assert_is_system(add_three_to_counter_system); /// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system); /// ``` - pub fn queue(&mut self, command: C) { + pub fn queue + HandleError, T>(&mut self, command: C) { + let command = command.handle_error(); match &mut self.queue { InternalQueue::CommandQueue(queue) => { queue.push(command); @@ -604,18 +604,6 @@ impl<'w, 's> Commands<'w, 's> { } } - /// Pushes a generic [`Command`] to the command queue with error handling. - /// - /// The command can be: - /// - A custom struct that implements [`Command`]. - /// - A closure or function that has the signature [`(&mut World)`](World)`->`[`Result`](crate::result::Result). - /// - A built-in command from the [`command`] module. - /// - /// This will use the default error handler. See [`Command`] for details on error handling. - pub fn queue_fallible>(&mut self, command: impl Command>) { - self.queue(map_command_err(command).handle_error()); - } - /// Pushes a [`Command`] to the queue for creating entities, if needed, /// and for adding a bundle to each entity. /// @@ -1666,25 +1654,11 @@ impl<'a> EntityCommands<'a> { /// # } /// # bevy_ecs::system::assert_is_system(my_system); /// ``` - pub fn queue(&mut self, command: impl EntityCommand) -> &mut Self { - self.commands - .queue_fallible(command.with_entity(self.entity)); - self - } - - /// Pushes a fallible [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. - /// - /// The command can be: - /// - A custom struct that implements [`EntityCommand`]. - /// - A closure or function that matches the following signature: - /// - [`(EntityWorldMut)`](EntityWorldMut)`->` [`Result<(), E>`] - /// - A built-in command from the [`entity_command`] module. - pub fn queue_fallible>( + pub fn queue + CommandWithEntity, T, M>( &mut self, - command: impl EntityCommand>, + command: C, ) -> &mut Self { - self.commands - .queue_fallible(map_entity_command_err(command).with_entity(self.entity)); + self.commands.queue(command.with_entity(self.entity)); self } From dd985e188fe2a99a6acd94b5307660e5e7edfb7d Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Tue, 7 Jan 2025 18:01:46 -0800 Subject: [PATCH 03/11] map_err --- crates/bevy_ecs/src/system/commands/entity_command.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index ea93dcd22ebd2..6c787af81d7a0 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -116,10 +116,7 @@ impl< { move |world: &mut World| { let entity = world.get_entity_mut(entity)?; - match self.apply(entity) { - Ok(result) => Ok(result), - Err(err) => Err(EntityCommandError::Error(err)), - } + self.apply(entity).map_err(EntityCommandError::Error) } } } From 423779a6aca5d45022fa6254aede7a66b817e433 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 8 Jan 2025 15:51:52 -0800 Subject: [PATCH 04/11] nicer imports --- .../src/system/commands/entity_command.rs | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 6c787af81d7a0..369da3bba7473 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -15,11 +15,11 @@ use crate::{ component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityCloneBuilder}, event::Event, + result::{Error, Result}, system::{error_handler, Command, HandleError, IntoObserverSystem}, world::{error::EntityFetchError, EntityWorldMut, FromWorld, World}, }; use bevy_ptr::OwningPtr; -use thiserror::Error; /// A [`Command`] which gets executed for a given [`Entity`]. /// @@ -126,10 +126,7 @@ impl< pub trait HandleEntityError { /// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. - fn handle_error_with( - self, - error_handler: fn(&mut World, crate::result::Error), - ) -> impl EntityCommand; + fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl EntityCommand; /// Takes a [`EntityCommand`] that returns a Result and uses the default error handler function to convert it into /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. fn handle_error(self) -> impl EntityCommand @@ -140,13 +137,8 @@ pub trait HandleEntityError { } } -impl>, T, E: Into> - HandleEntityError> for C -{ - fn handle_error_with( - self, - error_handler: fn(&mut World, crate::result::Error), - ) -> impl EntityCommand { +impl>, T, E: Into> HandleEntityError> for C { + fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl EntityCommand { move |entity: EntityWorldMut| { let location = entity.location(); let id = entity.id(); @@ -164,10 +156,7 @@ impl>, T, E: Into HandleEntityError for C { #[inline] - fn handle_error_with( - self, - _error_handler: fn(&mut World, crate::result::Error), - ) -> impl EntityCommand { + fn handle_error_with(self, _error_handler: fn(&mut World, Error)) -> impl EntityCommand { self } @@ -181,7 +170,7 @@ impl HandleEntityError for C { } /// An error that occurs when running an [`EntityCommand`] on a specific entity. -#[derive(Error, Debug)] +#[derive(thiserror::Error, Debug)] pub enum EntityCommandError { /// The entity this [`EntityCommand`] tried to run on could not be fetched. #[error(transparent)] From 1c95064ff8225006dda3fe9ec72a18e78d11472f Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Wed, 8 Jan 2025 16:27:55 -0800 Subject: [PATCH 05/11] Remove HandleEntityError --- .../src/system/commands/entity_command.rs | 52 +------- crates/bevy_ecs/src/system/commands/mod.rs | 125 ++++++++++++++++-- 2 files changed, 115 insertions(+), 62 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index 369da3bba7473..a6e831541d5ef 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -15,8 +15,8 @@ use crate::{ component::{Component, ComponentId, ComponentInfo}, entity::{Entity, EntityCloneBuilder}, event::Event, - result::{Error, Result}, - system::{error_handler, Command, HandleError, IntoObserverSystem}, + result::Result, + system::{command::HandleError, Command, IntoObserverSystem}, world::{error::EntityFetchError, EntityWorldMut, FromWorld, World}, }; use bevy_ptr::OwningPtr; @@ -121,54 +121,6 @@ impl< } } -/// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into -/// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. -pub trait HandleEntityError { - /// Takes a [`EntityCommand`] that returns a Result and uses a given error handler function to convert it into - /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. - fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl EntityCommand; - /// Takes a [`EntityCommand`] that returns a Result and uses the default error handler function to convert it into - /// a [`EntityCommand`] that internally handles an error if it occurs and returns `()`. - fn handle_error(self) -> impl EntityCommand - where - Self: Sized, - { - self.handle_error_with(error_handler::default()) - } -} - -impl>, T, E: Into> HandleEntityError> for C { - fn handle_error_with(self, error_handler: fn(&mut World, Error)) -> impl EntityCommand { - move |entity: EntityWorldMut| { - let location = entity.location(); - let id = entity.id(); - // This is broken up into parts so we can pass in the world to the error handler - // after EntityWorldMut is consumed - let world = entity.into_world_mut(); - // SAFETY: location has not changed and entity is valid - match self.apply(unsafe { EntityWorldMut::new(world, id, location) }) { - Ok(_) => {} - Err(err) => (error_handler)(world, err.into()), - } - } - } -} - -impl HandleEntityError for C { - #[inline] - fn handle_error_with(self, _error_handler: fn(&mut World, Error)) -> impl EntityCommand { - self - } - - #[inline] - fn handle_error(self) -> impl EntityCommand - where - Self: Sized, - { - self - } -} - /// An error that occurs when running an [`EntityCommand`] on a specific entity. #[derive(thiserror::Error, Debug)] pub enum EntityCommandError { diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index e6f564938f084..d2e6542883fc1 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -5,8 +5,8 @@ pub mod error_handler; #[cfg(feature = "std")] mod parallel_scope; -pub use command::{Command, HandleError}; -pub use entity_command::{CommandWithEntity, EntityCommand}; +pub use command::Command; +pub use entity_command::EntityCommand; #[cfg(feature = "std")] pub use parallel_scope::*; @@ -26,10 +26,11 @@ use crate::{ entity::{Entities, Entity, EntityCloneBuilder}, event::Event, observer::{Observer, TriggerTargets}, + result::Error, schedule::ScheduleLabel, system::{ - input::SystemInput, Deferred, IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, - SystemId, + command::HandleError, entity_command::CommandWithEntity, input::SystemInput, Deferred, + IntoObserverSystem, IntoSystem, RegisteredSystem, Resource, SystemId, }, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, CommandQueue, @@ -551,15 +552,16 @@ impl<'w, 's> Commands<'w, 's> { /// Pushes a generic [`Command`] to the command queue. /// + /// If the [`Command`] returns a [`Result`], it will be handled using the [default error handler](error_handler::default). + /// + /// To use a custom error handler, see [`Commands::queue_handled`]. + /// /// The command can be: /// - A custom struct that implements [`Command`]. /// - A closure or function that matches one of the following signatures: /// - [`(&mut World)`](World) /// - A built-in command from the [`command`] module. /// - /// If you want to queue a fallible command with error handling, - /// use [`Commands::queue_fallible`]. - /// /// # Example /// /// ``` @@ -567,17 +569,18 @@ impl<'w, 's> Commands<'w, 's> { /// #[derive(Resource, Default)] /// struct Counter(u64); /// - /// struct AddToCounter(u64); + /// struct AddToCounter(String); /// /// impl Command for AddToCounter { - /// fn apply(self, world: &mut World) { + /// fn apply(self, world: &mut World) -> Result { /// let mut counter = world.get_resource_or_insert_with(Counter::default); - /// counter.0 += self.0; + /// let amount: usize = self.0.parse()?; + /// counter.0 += amount; /// } /// } /// /// fn add_three_to_counter_system(mut commands: Commands) { - /// commands.queue(AddToCounter(3)); + /// commands.queue(AddToCounter("3".to_string())); /// } /// fn add_twenty_five_to_counter_system(mut commands: Commands) { /// commands.queue(|world: &mut World| { @@ -589,7 +592,58 @@ impl<'w, 's> Commands<'w, 's> { /// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system); /// ``` pub fn queue + HandleError, T>(&mut self, command: C) { - let command = command.handle_error(); + self.queue_internal(command.handle_error()); + } + /// Pushes a generic [`Command`] to the command queue. If the command returns a [`Result`] the given + /// `error_handler` will be used to handle error cases. + /// + /// To implicitly use the default error handler, see [`Commands::queue`]. + /// + /// The command can be: + /// - A custom struct that implements [`Command`]. + /// - A closure or function that matches one of the following signatures: + /// - [`(&mut World)`](World) + /// - [`(&mut World)`](World) `->` [`Result`] + /// - A built-in command from the [`command`] module. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// #[derive(Resource, Default)] + /// struct Counter(u64); + /// + /// struct AddToCounter(String); + /// + /// impl Command for AddToCounter { + /// fn apply(self, world: &mut World) -> Result { + /// let mut counter = world.get_resource_or_insert_with(Counter::default); + /// let amount: usize = self.0.parse()?; + /// counter.0 += amount; + /// } + /// } + /// + /// fn add_three_to_counter_system(mut commands: Commands) { + /// commands.queue_handled(AddToCounter("3".to_string()), error_handler::warn()); + /// } + /// fn add_twenty_five_to_counter_system(mut commands: Commands) { + /// commands.queue(|world: &mut World| { + /// let mut counter = world.get_resource_or_insert_with(Counter::default); + /// counter.0 += 25; + /// }); + /// } + /// # bevy_ecs::system::assert_is_system(add_three_to_counter_system); + /// # bevy_ecs::system::assert_is_system(add_twenty_five_to_counter_system); + /// ``` + pub fn queue_handled + HandleError, T>( + &mut self, + command: C, + error_handler: fn(&mut World, Error), + ) { + self.queue_internal(command.handle_error_with(error_handler)); + } + + fn queue_internal(&mut self, command: impl Command) { match &mut self.queue { InternalQueue::CommandQueue(queue) => { queue.push(command); @@ -1634,10 +1688,15 @@ impl<'a> EntityCommands<'a> { /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. /// + /// If the [`EntityCommand`] returns a [`Result`], it will be handled using the [default error handler](error_handler::default). + /// + /// To use a custom error handler, see [`EntityCommands::queue_handled`]. + /// /// The command can be: /// - A custom struct that implements [`EntityCommand`]. /// - A closure or function that matches the following signature: /// - [`(EntityWorldMut)`](EntityWorldMut) + /// - [`(EntityWorldMut)`](EntityWorldMut) `->` [`Result`] /// - A built-in command from the [`entity_command`] module. /// /// # Examples @@ -1662,6 +1721,48 @@ impl<'a> EntityCommands<'a> { self } + /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. + /// If the command returns a [`Result`] the given `error_handler` will be used to handle error cases. + /// + /// To implicitly use the default error handler, see [`EntityCommands::queue`]. + /// + /// The command can be: + /// - A custom struct that implements [`EntityCommand`]. + /// - A closure or function that matches the following signature: + /// - [`(EntityWorldMut)`](EntityWorldMut) + /// - [`(EntityWorldMut)`](EntityWorldMut) `->` [`Result`] + /// - A built-in command from the [`entity_command`] module. + /// + /// # Examples + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::system::error_handler; + /// # fn my_system(mut commands: Commands) { + /// commands + /// .spawn_empty() + /// // Closures with this signature implement `EntityCommand`. + /// .queue_handled( + /// |entity: EntityWorldMut| -> Result { + /// let value: usize = "100".parse()?; + /// println!("Successfully parsed the value {} for entity {}", value, entity.id()); + /// Ok(()) + /// }, + /// error_handler::warn() + /// ); + /// # } + /// # bevy_ecs::system::assert_is_system(my_system); + /// ``` + pub fn queue_handled + CommandWithEntity, T, M>( + &mut self, + command: C, + error_handler: fn(&mut World, Error), + ) -> &mut Self { + self.commands + .queue_handled(command.with_entity(self.entity), error_handler); + self + } + /// Removes all components except the given [`Bundle`] from the entity. /// /// This can also be used to remove all the components from the entity by passing it an empty Bundle. From 2374d6ed605e58000bfe612a8bf4dcd8bf55b5de Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 12:51:50 -0800 Subject: [PATCH 06/11] Handle try_ variants correctly (and despawn) --- .../bevy_ecs/src/system/commands/command.rs | 3 +-- crates/bevy_ecs/src/system/commands/mod.rs | 25 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index 8cb70f73c5e76..d2828e8cba37e 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -34,10 +34,9 @@ use crate::{ /// struct AddToCounter(u64); /// /// impl Command for AddToCounter { -/// fn apply(self, world: &mut World) -> Result { +/// fn apply(self, world: &mut World) { /// let mut counter = world.get_resource_or_insert_with(Counter::default); /// counter.0 += self.0; -/// Ok(()) /// } /// } /// diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d2e6542883fc1..e384ba3936afb 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -571,11 +571,12 @@ impl<'w, 's> Commands<'w, 's> { /// /// struct AddToCounter(String); /// - /// impl Command for AddToCounter { + /// impl Command for AddToCounter { /// fn apply(self, world: &mut World) -> Result { /// let mut counter = world.get_resource_or_insert_with(Counter::default); - /// let amount: usize = self.0.parse()?; + /// let amount: u64 = self.0.parse()?; /// counter.0 += amount; + /// Ok(()) /// } /// } /// @@ -610,16 +611,18 @@ impl<'w, 's> Commands<'w, 's> { /// /// ``` /// # use bevy_ecs::prelude::*; + /// # use bevy_ecs::system::error_handler; /// #[derive(Resource, Default)] /// struct Counter(u64); /// /// struct AddToCounter(String); /// - /// impl Command for AddToCounter { + /// impl Command for AddToCounter { /// fn apply(self, world: &mut World) -> Result { /// let mut counter = world.get_resource_or_insert_with(Counter::default); - /// let amount: usize = self.0.parse()?; + /// let amount: u64 = self.0.parse()?; /// counter.0 += amount; + /// Ok(()) /// } /// } /// @@ -1405,7 +1408,10 @@ impl<'a> EntityCommands<'a> { component_id: ComponentId, value: T, ) -> &mut Self { - self.queue(entity_command::insert_by_id(component_id, value)) + self.queue_handled( + entity_command::insert_by_id(component_id, value), + error_handler::warn(), + ) } /// Tries to add a [`Bundle`] of components to the entity. @@ -1458,7 +1464,7 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue(entity_command::insert(bundle)) + self.queue_handled(entity_command::insert(bundle), error_handler::warn()) } /// Similar to [`Self::try_insert`] but will only try to insert if the predicate returns true. @@ -1557,7 +1563,7 @@ impl<'a> EntityCommands<'a> { /// Unlike [`Self::insert_if_new`], this will not panic if the associated entity does not exist. #[track_caller] pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue(entity_command::insert_if_new(bundle)) + self.queue_handled(entity_command::insert_if_new(bundle), error_handler::warn()) } /// Removes a [`Bundle`] of components from the entity. @@ -1674,16 +1680,15 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn despawn(&mut self) { - self.queue(entity_command::despawn()); + self.queue_handled(entity_command::despawn(), error_handler::warn()); } /// Despawns the entity. /// /// This will not emit a warning if the entity does not exist, essentially performing /// the same function as [`Self::despawn`] without emitting warnings. - #[track_caller] pub fn try_despawn(&mut self) { - self.queue(entity_command::despawn()); + self.queue_handled(entity_command::despawn(), error_handler::silent()); } /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. From 4ff4d94a9f5a48b596c5862b6ffefc4389f13174 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 13:17:56 -0800 Subject: [PATCH 07/11] handle recursive despawn correctly --- crates/bevy_hierarchy/src/hierarchy.rs | 126 ++++++++++++++----------- 1 file changed, 69 insertions(+), 57 deletions(-) diff --git a/crates/bevy_hierarchy/src/hierarchy.rs b/crates/bevy_hierarchy/src/hierarchy.rs index 247eef48fc06b..5b89149154c0b 100644 --- a/crates/bevy_hierarchy/src/hierarchy.rs +++ b/crates/bevy_hierarchy/src/hierarchy.rs @@ -5,7 +5,7 @@ use crate::{ use bevy_ecs::{ component::ComponentCloneHandler, entity::{ComponentCloneCtx, Entity, EntityCloneBuilder}, - system::EntityCommands, + system::{error_handler, EntityCommands}, world::{DeferredWorld, EntityWorldMut, World}, }; use log::debug; @@ -68,38 +68,44 @@ impl DespawnRecursiveExt for EntityCommands<'_> { /// This will emit warnings for any entity that does not exist. fn despawn_recursive(mut self) { let warn = true; - self.queue(move |mut entity: EntityWorldMut| { - let id = entity.id(); - #[cfg(feature = "trace")] - let _span = tracing::info_span!( - "command", - name = "DespawnRecursive", - entity = tracing::field::debug(id), - warn = tracing::field::debug(warn) - ) - .entered(); - entity.world_scope(|world| { - despawn_with_children_recursive(world, id, warn); - }); - }); + self.queue_handled( + move |mut entity: EntityWorldMut| { + let id = entity.id(); + #[cfg(feature = "trace")] + let _span = tracing::info_span!( + "command", + name = "DespawnRecursive", + entity = tracing::field::debug(id), + warn = tracing::field::debug(warn) + ) + .entered(); + entity.world_scope(|world| { + despawn_with_children_recursive(world, id, warn); + }); + }, + error_handler::warn(), + ); } fn despawn_descendants(&mut self) -> &mut Self { let warn = true; - self.queue(move |mut entity: EntityWorldMut| { - let id = entity.id(); - #[cfg(feature = "trace")] - let _span = tracing::info_span!( - "command", - name = "DespawnChildrenRecursive", - entity = tracing::field::debug(id), - warn = tracing::field::debug(warn) - ) - .entered(); - entity.world_scope(|world| { - despawn_children_recursive(world, id, warn); - }); - }); + self.queue_handled( + move |mut entity: EntityWorldMut| { + let id = entity.id(); + #[cfg(feature = "trace")] + let _span = tracing::info_span!( + "command", + name = "DespawnChildrenRecursive", + entity = tracing::field::debug(id), + warn = tracing::field::debug(warn) + ) + .entered(); + entity.world_scope(|world| { + despawn_children_recursive(world, id, warn); + }); + }, + error_handler::warn(), + ); self } @@ -107,38 +113,44 @@ impl DespawnRecursiveExt for EntityCommands<'_> { /// This will never emit warnings. fn try_despawn_recursive(mut self) { let warn = false; - self.queue(move |mut entity: EntityWorldMut| { - let id = entity.id(); - #[cfg(feature = "trace")] - let _span = tracing::info_span!( - "command", - name = "TryDespawnRecursive", - entity = tracing::field::debug(id), - warn = tracing::field::debug(warn) - ) - .entered(); - entity.world_scope(|world| { - despawn_with_children_recursive(world, id, warn); - }); - }); + self.queue_handled( + move |mut entity: EntityWorldMut| { + let id = entity.id(); + #[cfg(feature = "trace")] + let _span = tracing::info_span!( + "command", + name = "TryDespawnRecursive", + entity = tracing::field::debug(id), + warn = tracing::field::debug(warn) + ) + .entered(); + entity.world_scope(|world| { + despawn_with_children_recursive(world, id, warn); + }); + }, + error_handler::silent(), + ); } fn try_despawn_descendants(&mut self) -> &mut Self { let warn = false; - self.queue(move |mut entity: EntityWorldMut| { - let id = entity.id(); - #[cfg(feature = "trace")] - let _span = tracing::info_span!( - "command", - name = "TryDespawnChildrenRecursive", - entity = tracing::field::debug(id), - warn = tracing::field::debug(warn) - ) - .entered(); - entity.world_scope(|world| { - despawn_children_recursive(world, id, warn); - }); - }); + self.queue_handled( + move |mut entity: EntityWorldMut| { + let id = entity.id(); + #[cfg(feature = "trace")] + let _span = tracing::info_span!( + "command", + name = "TryDespawnChildrenRecursive", + entity = tracing::field::debug(id), + warn = tracing::field::debug(warn) + ) + .entered(); + entity.world_scope(|world| { + despawn_children_recursive(world, id, warn); + }); + }, + error_handler::silent(), + ); self } } From 601447d1606a184059e098b86241c92635ef4bbc Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 14:58:35 -0800 Subject: [PATCH 08/11] Update docs --- crates/bevy_ecs/src/system/commands/error_handler.rs | 8 +++----- crates/bevy_ecs/src/system/commands/mod.rs | 12 ++---------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/error_handler.rs b/crates/bevy_ecs/src/system/commands/error_handler.rs index 7f4230fe05f3e..1aac7270d5b3f 100644 --- a/crates/bevy_ecs/src/system/commands/error_handler.rs +++ b/crates/bevy_ecs/src/system/commands/error_handler.rs @@ -1,7 +1,5 @@ //! This module contains convenience functions that return simple error handlers -//! for use with [`HandleError::handle_error_with`](super::HandleError::handle_error_with) on instances of -//! [`Command`](super::Command) and [`EntityCommand`](super::EntityCommand) that return a Result with an -//! [`Error`]. +//! for use with [`Commands::queue_handled`](super::Commands::queue_handled) and [`EntityCommands::queue_handled`](super::EntityCommands::queue_handled). use crate::{result::Error, world::World}; use log::{error, warn}; @@ -27,7 +25,7 @@ pub fn panic() -> fn(&mut World, Error) { |_, error| panic!("{error}") } -/// The default error handler. This defaults to [`panic`]. If the +/// The default error handler. This defaults to [`panic()`]. If the /// `configurable_error_handler` cargo feature is enabled, then /// `GLOBAL_ERROR_HANDLER` will be used instead, enabling error handler customization. #[cfg(not(feature = "configurable_error_handler"))] @@ -45,7 +43,7 @@ pub fn default() -> fn(&mut World, Error) { pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = std::sync::OnceLock::new(); -/// The default error handler. This defaults to [`panic`]. If the +/// The default error handler. This defaults to [`panic()`]. If the /// `configurable_error_handler` cargo feature is enabled, then /// [`GLOBAL_ERROR_HANDLER`] will be used instead, enabling error handler customization. #[cfg(feature = "configurable_error_handler")] diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index e384ba3936afb..a17ce662a2e68 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -95,14 +95,10 @@ use crate::{ /// an error handler. Error handlers are functions/closures of the form /// `fn(&mut World, CommandError)`. /// -/// Fallible commands should generally be passed to [`Commands::queue_fallible`], which will -/// use the default error handler. -/// /// The default error handler panics. It can be configured by enabling the `configurable_error_handler` /// cargo feature, then setting the `GLOBAL_ERROR_HANDLER`. /// -/// Alternatively, you can customize the error handler for a specific command by calling [`HandleError::handle_error_with`] -/// on it, then passing the result to [`Commands::queue`]. +/// Alternatively, you can customize the error handler for a specific command by calling [`Commands::queue_handled`]. /// /// The [`error_handler`] module provides some simple error handlers for convenience. /// @@ -1153,14 +1149,10 @@ impl<'w, 's> Commands<'w, 's> { /// an error handler. Error handlers are functions/closures of the form /// `fn(&mut World, CommandError)`. /// -/// Fallible commands should generally be passed to [`EntityCommands::queue_fallible`], which will -/// use the default error handler. -/// /// The default error handler panics. It can be configured by enabling the `configurable_error_handler` /// cargo feature, then setting the `GLOBAL_ERROR_HANDLER`. /// -/// Alternatively, you can customize the error handler for a specific command by calling [`HandleError::handle_error_with`] -/// on it, then passing the result to [`EntityCommands::queue`]. +/// Alternatively, you can customize the error handler for a specific command by calling [`EntityCommands::queue_handled`]. /// /// The [`error_handler`] module provides some simple error handlers for convenience. pub struct EntityCommands<'a> { From 88d96f74d02b69a64fcaa15c3df28268ae8f82f5 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 17:57:18 -0800 Subject: [PATCH 09/11] Resolve comments --- .../bevy_ecs/src/system/commands/command.rs | 14 +++++---- .../src/system/commands/entity_command.rs | 30 ++++++++++++------- .../src/system/commands/error_handler.rs | 8 +++++ 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/command.rs b/crates/bevy_ecs/src/system/commands/command.rs index d2828e8cba37e..bbff4240684d4 100644 --- a/crates/bevy_ecs/src/system/commands/command.rs +++ b/crates/bevy_ecs/src/system/commands/command.rs @@ -22,6 +22,8 @@ use crate::{ /// /// Should be used with [`Commands::queue`](crate::system::Commands::queue). /// +/// The `Out` generic parameter is the returned "output" of the command. +/// /// # Usage /// /// ``` @@ -44,27 +46,27 @@ use crate::{ /// commands.queue(AddToCounter(42)); /// } /// ``` -pub trait Command: Send + 'static { +pub trait Command: Send + 'static { /// Applies this command, causing it to mutate the provided `world`. /// /// This method is used to define what a command "does" when it is ultimately applied. /// Because this method takes `self`, you can store data or settings on the type that implements this trait. /// This data is set by the system or other source of the command, and then ultimately read in this method. - fn apply(self, world: &mut World) -> T; + fn apply(self, world: &mut World) -> Out; } -impl Command for F +impl Command for F where - F: FnOnce(&mut World) -> T + Send + 'static, + F: FnOnce(&mut World) -> Out + Send + 'static, { - fn apply(self, world: &mut World) -> T { + fn apply(self, world: &mut World) -> Out { self(world) } } /// 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 { +pub trait HandleError { /// 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(&mut World, Error)) -> impl Command; diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index a6e831541d5ef..c4098d3ad26c8 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -21,7 +21,11 @@ use crate::{ }; use bevy_ptr::OwningPtr; -/// A [`Command`] which gets executed for a given [`Entity`]. +/// A command which gets executed for a given [`Entity`]. + +/// Should be used with [`EntityCommands::queue`](crate::system::EntityCommands::queue). +/// +/// The `Out` generic parameter is the returned "output" of the command. /// /// # Examples /// @@ -76,17 +80,22 @@ use bevy_ptr::OwningPtr; /// assert_eq!(names, HashSet::from_iter(["Entity #0", "Entity #1"])); /// } /// ``` -pub trait EntityCommand: Send + 'static { +pub trait EntityCommand: Send + 'static { /// Executes this command for the given [`Entity`] and /// returns a [`Result`] for error handling. - fn apply(self, entity: EntityWorldMut) -> T; + fn apply(self, entity: EntityWorldMut) -> Out; } /// Passes in a specific entity to an [`EntityCommand`], resulting in a [`Command`] that /// internally runs the [`EntityCommand`] on that entity. -pub trait CommandWithEntity { +/// +// 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 { /// 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 + HandleError; + fn with_entity(self, entity: Entity) -> impl Command + HandleError; } impl CommandWithEntity> for C { @@ -116,7 +125,8 @@ impl< { move |world: &mut World| { let entity = world.get_entity_mut(entity)?; - self.apply(entity).map_err(EntityCommandError::Error) + self.apply(entity) + .map_err(EntityCommandError::CommandFailed) } } } @@ -129,14 +139,14 @@ pub enum EntityCommandError { EntityFetchError(#[from] EntityFetchError), /// An error that occurred while running the [`EntityCommand`]. #[error("{0}")] - Error(E), + CommandFailed(E), } -impl EntityCommand for F +impl EntityCommand for F where - F: FnOnce(EntityWorldMut) -> T + Send + 'static, + F: FnOnce(EntityWorldMut) -> Out + Send + 'static, { - fn apply(self, entity: EntityWorldMut) -> T { + fn apply(self, entity: EntityWorldMut) -> Out { self(entity) } } diff --git a/crates/bevy_ecs/src/system/commands/error_handler.rs b/crates/bevy_ecs/src/system/commands/error_handler.rs index 1aac7270d5b3f..231df9ec7387e 100644 --- a/crates/bevy_ecs/src/system/commands/error_handler.rs +++ b/crates/bevy_ecs/src/system/commands/error_handler.rs @@ -39,6 +39,14 @@ pub fn default() -> fn(&mut World, Error) { /// /// If the `configurable_error_handler` cargo feature is enabled, this will be used /// by default. +/// +/// This should be set in the following way: +/// +/// ``` +/// # use bevy_ecs::system::error_handler::{GLOBAL_ERROR_HANDLER, warn}; +/// GLOBAL_ERROR_HANDLER.set(warn()); +/// // initialize Bevy App here +/// ``` #[cfg(feature = "configurable_error_handler")] pub static GLOBAL_ERROR_HANDLER: std::sync::OnceLock = std::sync::OnceLock::new(); From 0ba534bbe4d6585be00a9bbf3d2594657b2fee0a Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 18:53:53 -0800 Subject: [PATCH 10/11] Missing `///` --- crates/bevy_ecs/src/system/commands/entity_command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/commands/entity_command.rs b/crates/bevy_ecs/src/system/commands/entity_command.rs index c4098d3ad26c8..c81f657084ec0 100644 --- a/crates/bevy_ecs/src/system/commands/entity_command.rs +++ b/crates/bevy_ecs/src/system/commands/entity_command.rs @@ -22,7 +22,7 @@ use crate::{ use bevy_ptr::OwningPtr; /// A command which gets executed for a given [`Entity`]. - +/// /// Should be used with [`EntityCommands::queue`](crate::system::EntityCommands::queue). /// /// The `Out` generic parameter is the returned "output" of the command. From aaf327072cfdaab0810e810c66d569844aa0fe03 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Thu, 9 Jan 2025 19:58:29 -0800 Subject: [PATCH 11/11] try_remove --- crates/bevy_ecs/src/system/commands/mod.rs | 57 ++++++++++++++++++++-- crates/bevy_pbr/src/render/light.rs | 2 +- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index a17ce662a2e68..ffff2cb0944c2 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1402,7 +1402,7 @@ impl<'a> EntityCommands<'a> { ) -> &mut Self { self.queue_handled( entity_command::insert_by_id(component_id, value), - error_handler::warn(), + error_handler::silent(), ) } @@ -1456,7 +1456,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), error_handler::warn()) + self.queue_handled(entity_command::insert(bundle), error_handler::silent()) } /// Similar to [`Self::try_insert`] but will only try to insert if the predicate returns true. @@ -1555,7 +1555,10 @@ impl<'a> EntityCommands<'a> { /// Unlike [`Self::insert_if_new`], this will not panic if the associated entity does not exist. #[track_caller] pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue_handled(entity_command::insert_if_new(bundle), error_handler::warn()) + self.queue_handled( + entity_command::insert_if_new(bundle), + error_handler::silent(), + ) } /// Removes a [`Bundle`] of components from the entity. @@ -1597,7 +1600,53 @@ impl<'a> EntityCommands<'a> { where T: Bundle, { - self.queue(entity_command::remove::()) + self.queue_handled(entity_command::remove::(), error_handler::warn()) + } + + /// Removes a [`Bundle`] of components from the entity. + /// + /// # Note + /// + /// Unlike [`Self::remove`], this will not panic if the associated entity does not exist. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # + /// # #[derive(Resource)] + /// # struct PlayerEntity { entity: Entity } + /// #[derive(Component)] + /// struct Health(u32); + /// #[derive(Component)] + /// struct Strength(u32); + /// #[derive(Component)] + /// struct Defense(u32); + /// + /// #[derive(Bundle)] + /// struct CombatBundle { + /// health: Health, + /// strength: Strength, + /// } + /// + /// fn remove_combat_stats_system(mut commands: Commands, player: Res) { + /// commands + /// .entity(player.entity) + /// // You can remove individual components: + /// .try_remove::() + /// // You can also remove pre-defined Bundles of components: + /// .try_remove::() + /// // You can also remove tuples of components and bundles. + /// // This is equivalent to the calls above: + /// .try_remove::<(Defense, CombatBundle)>(); + /// } + /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); + /// ``` + pub fn try_remove(&mut self) -> &mut Self + where + T: Bundle, + { + self.queue_handled(entity_command::remove::(), error_handler::silent()) } /// Removes all components in the [`Bundle`] components and remove all required components for each component in the [`Bundle`] from entity. diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 7f7434a9f7113..97023b0ab6bf9 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -515,7 +515,7 @@ pub(crate) fn extracted_light_removed( mut commands: Commands, ) { if let Some(mut v) = commands.get_entity(trigger.target()) { - v.remove::(); + v.try_remove::(); } }