From 1cf6ebd57422693c0d614a633e4795a9d7ae050e Mon Sep 17 00:00:00 2001 From: NathanW Date: Thu, 20 May 2021 09:12:13 -0600 Subject: [PATCH 01/15] [ecs] command error handling --- Cargo.toml | 4 + crates/bevy_ecs/src/lib.rs | 5 +- crates/bevy_ecs/src/system/commands/config.rs | 201 ++++++++ .../system/{commands.rs => commands/mod.rs} | 451 +++++++++++++++--- examples/README.md | 1 + examples/ecs/command_error_handling.rs | 77 +++ 6 files changed, 682 insertions(+), 57 deletions(-) create mode 100644 crates/bevy_ecs/src/system/commands/config.rs rename crates/bevy_ecs/src/system/{commands.rs => commands/mod.rs} (51%) create mode 100644 examples/ecs/command_error_handling.rs diff --git a/Cargo.toml b/Cargo.toml index 9e8551ffc3daf..b81785e5aeabf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -262,6 +262,10 @@ path = "examples/ecs/ecs_guide.rs" name = "change_detection" path = "examples/ecs/change_detection.rs" +[[example]] +name = "command_error_handling" +path = "examples/ecs/command_error_handling.rs" + [[example]] name = "event" path = "examples/ecs/event.rs" diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index a6fdbced0894d..78051a0bd9e8b 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -29,8 +29,9 @@ pub mod prelude { Schedule, Stage, StageLabel, State, SystemLabel, SystemSet, SystemStage, }, system::{ - Commands, In, IntoChainSystem, IntoExclusiveSystem, IntoSystem, Local, NonSend, - NonSendMut, Query, QuerySet, RemovedComponents, Res, ResMut, System, + CommandErrorHandler, Commands, FallibleCommand, In, IntoChainSystem, + IntoExclusiveSystem, IntoSystem, Local, NonSend, NonSendMut, Query, QuerySet, + RemovedComponents, Res, ResMut, System, }, world::{FromWorld, Mut, World}, }; diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs new file mode 100644 index 0000000000000..a73c7b9a506b1 --- /dev/null +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -0,0 +1,201 @@ +use crate::{ + prelude::{FallibleCommand, World}, + system::Command, +}; +use bevy_utils::tracing::error; +use std::{ + fmt::Debug, + ops::{Deref, DerefMut}, +}; + +#[doc(hidden)] +pub trait AddCommand { + fn add_command(&mut self, command: impl Command); +} + +/// Provides configuration mechanisms in case a command errors. +/// You can specify a custom handler via [`FallibleCommandConfig::on_failure`] or +/// use one of the provided implementations. +/// +/// ## Note +/// The default error handler logs the error (via [`error!`]), but does not panic. +pub struct FallibleCommandConfig<'a, C, T> +where + C: FallibleCommand, + T: AddCommand, +{ + command: Option, + inner: &'a mut T, +} + +impl<'a, C, T> Deref for FallibleCommandConfig<'a, C, T> +where + C: FallibleCommand, + T: AddCommand, +{ + type Target = T; + + #[inline] + fn deref(&self) -> &Self::Target { + self.inner + } +} + +impl<'a, C, T> DerefMut for FallibleCommandConfig<'a, C, T> +where + C: FallibleCommand, + T: AddCommand, +{ + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + self.inner + } +} + +/// Builtin command error handlers. +pub struct CommandErrorHandler; + +impl CommandErrorHandler { + /// If the command failed, log the error. + /// + /// ## Note + /// This is the default behavior if no error handler is specified. + pub fn log(error: E, _ctx: CommandContext) { + error!("Commands failed with error: {:?}", error) + } + + /// If the command failed, [`panic!`] with the error. + pub fn panic(error: E, _ctx: CommandContext) { + panic!("Commands failed with error: {:?}", error) + } + + /// If the command failed, ignore the error and silently succeed. + pub fn ignore(_error: E, _ctx: CommandContext) {} +} + +pub(crate) struct HandledErrorCommand +where + C: FallibleCommand, + F: FnOnce(C::Error, CommandContext) + Send + Sync + 'static, +{ + pub(crate) command: C, + pub(crate) error_handler: F, +} + +impl Command for HandledErrorCommand +where + C: FallibleCommand, + F: FnOnce(C::Error, CommandContext) + Send + Sync + 'static, +{ + fn write(self: Box, world: &mut World) { + let HandledErrorCommand { + command, + error_handler, + } = *self; + + if let Err(error) = command.try_write(world) { + error_handler(error, CommandContext { world }); + } + } +} + +#[non_exhaustive] +pub struct CommandContext<'a> { + pub world: &'a mut World, +} + +/// Similar to [`FallibleCommandConfig`] however does not +/// implement [`DerefMut`] nor return `&mut T` of the underlying +/// Commands type. +pub struct FinalFallibleCommandConfig<'a, C, T> +where + C: FallibleCommand, + T: AddCommand, +{ + command: Option, + inner: &'a mut T, +} + +macro_rules! impl_fallible_commands { + ($name:ident, $returnty:ty, $returnfunc:ident) => { + impl<'a, C, T> $name<'a, C, T> + where + C: FallibleCommand, + C::Error: Debug, + T: AddCommand, + { + #[inline] + pub(crate) fn new(command: C, inner: &'a mut T) -> Self { + Self { + command: Some(command), + inner, + } + } + + #[inline] + #[allow(dead_code)] + fn return_inner(&mut self) -> &mut T { + self.inner + } + + #[inline] + #[allow(dead_code)] + fn return_unit(&self) {} + } + + impl<'a, C, T> $name<'a, C, T> + where + C: FallibleCommand, + C::Error: Debug, + T: AddCommand, + { + /// If the command failed, run the provided `error_handler`. + /// + /// ## Note + /// This is normally used in conjunction with [`CommandErrorHandler`]. + /// However, this can also be used with custom error handlers (e.g. closures). + /// + /// # Examples + /// ``` + /// use bevy_ecs::prelude::*; + /// + /// fn system(mut commands: Commands) { + /// // built-in error handler + /// commands.spawn().insert(42).on_err(CommandErrorHandler::ignore); + /// + /// // custom error handler + /// commands.spawn().insert(42).on_err(|error, ctx| {}); + /// } + /// ``` + pub fn on_err( + &mut self, + error_handler: impl FnOnce(C::Error, CommandContext) + Send + Sync + 'static, + ) -> $returnty { + let command = self + .command + .take() + .expect("Cannot call `on_err` multiple times for a command error handler."); + self.inner.add_command(HandledErrorCommand { + command, + error_handler, + }); + self.$returnfunc() + } + } + + impl<'a, C, T> Drop for $name<'a, C, T> + where + C: FallibleCommand, + T: AddCommand, + { + fn drop(&mut self) { + if self.command.is_some() { + self.on_err(CommandErrorHandler::log); + } + } + } + }; +} + +impl_fallible_commands!(FinalFallibleCommandConfig, (), return_unit); +impl_fallible_commands!(FallibleCommandConfig, &mut T, return_inner); diff --git a/crates/bevy_ecs/src/system/commands.rs b/crates/bevy_ecs/src/system/commands/mod.rs similarity index 51% rename from crates/bevy_ecs/src/system/commands.rs rename to crates/bevy_ecs/src/system/commands/mod.rs index 90adfb8dc510c..373e2b867b5d5 100644 --- a/crates/bevy_ecs/src/system/commands.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,17 +1,28 @@ +mod config; +pub use config::*; + use crate::{ bundle::Bundle, component::Component, entity::{Entities, Entity}, world::World, }; -use bevy_utils::tracing::debug; -use std::marker::PhantomData; +use std::{fmt::Debug, marker::PhantomData}; /// A [`World`] mutation. +/// If this could potentially fail, use [`FallibleCommand`]. pub trait Command: Send + Sync + 'static { fn write(self: Box, world: &mut World); } +/// A [`World`] mutation that can potentially fail. +/// For an infallible variant, use [`Command`]. +pub trait FallibleCommand: Send + Sync + 'static { + type Error: Debug; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error>; +} + /// A queue of [`Command`]s. #[derive(Default)] pub struct CommandQueue { @@ -76,7 +87,7 @@ impl<'a> Commands<'a> { /// } /// # example_system.system(); /// ``` - pub fn spawn(&mut self) -> EntityCommands<'a, '_> { + pub fn spawn<'this>(&'this mut self) -> EntityCommands<'a, 'this> { let entity = self.entities.reserve_entity(); EntityCommands { entity, @@ -124,7 +135,7 @@ impl<'a> Commands<'a> { /// } /// # example_system.system(); /// ``` - pub fn spawn_bundle<'b, T: Bundle>(&'b mut self, bundle: T) -> EntityCommands<'a, 'b> { + pub fn spawn_bundle<'this, T: Bundle>(&'this mut self, bundle: T) -> EntityCommands<'a, 'this> { let mut e = self.spawn(); e.insert_bundle(bundle); e @@ -149,7 +160,7 @@ impl<'a> Commands<'a> { /// } /// # example_system.system(); /// ``` - pub fn entity(&mut self, entity: Entity) -> EntityCommands<'a, '_> { + pub fn entity<'this>(&'this mut self, entity: Entity) -> EntityCommands<'a, 'this> { EntityCommands { entity, commands: self, @@ -172,16 +183,35 @@ impl<'a> Commands<'a> { } /// Queue a resource removal. - pub fn remove_resource(&mut self) { - self.queue.push(RemoveResource:: { - phantom: PhantomData, - }); + pub fn remove_resource( + &mut self, + ) -> FinalFallibleCommandConfig<'_, RemoveResource, Self> { + FinalFallibleCommandConfig::new( + RemoveResource { + phantom: PhantomData, + }, + self, + ) } /// Adds a command directly to the command list. pub fn add(&mut self, command: C) { self.queue.push(command); } + + /// Adds a fallible command to the command list. + pub fn add_fallible(&mut self, command: C) -> FinalFallibleCommandConfig<'_, C, Self> + where + C: FallibleCommand, + { + FinalFallibleCommandConfig::new(command, self) + } +} + +impl<'a> AddCommand for Commands<'a> { + fn add_command(&mut self, command: impl Command) { + self.add(command); + } } /// A list of commands that will be run to modify an [`Entity`]. @@ -198,12 +228,17 @@ impl<'a, 'b> EntityCommands<'a, 'b> { } /// Adds a [`Bundle`] of components to the current entity. - pub fn insert_bundle(&mut self, bundle: impl Bundle) -> &mut Self { - self.commands.add(InsertBundle { - entity: self.entity, - bundle, - }); - self + pub fn insert_bundle( + &mut self, + bundle: T, + ) -> FallibleCommandConfig<'_, InsertBundle, Self> { + FallibleCommandConfig::new( + InsertBundle { + entity: self.entity, + bundle, + }, + self, + ) } /// Adds a single [`Component`] to the current entity. @@ -238,43 +273,55 @@ impl<'a, 'b> EntityCommands<'a, 'b> { /// } /// # example_system.system(); /// ``` - pub fn insert(&mut self, component: impl Component) -> &mut Self { - self.commands.add(Insert { - entity: self.entity, - component, - }); - self + pub fn insert( + &mut self, + component: T, + ) -> FallibleCommandConfig<'_, Insert, Self> { + FallibleCommandConfig::new( + Insert { + entity: self.entity, + component, + }, + self, + ) } /// See [`EntityMut::remove_bundle`](crate::world::EntityMut::remove_bundle). - pub fn remove_bundle(&mut self) -> &mut Self + pub fn remove_bundle(&mut self) -> FallibleCommandConfig<'_, RemoveBundle, Self> where T: Bundle, { - self.commands.add(RemoveBundle:: { - entity: self.entity, - phantom: PhantomData, - }); - self + FallibleCommandConfig::new( + RemoveBundle { + entity: self.entity, + phantom: PhantomData, + }, + self, + ) } /// See [`EntityMut::remove`](crate::world::EntityMut::remove). - pub fn remove(&mut self) -> &mut Self + pub fn remove(&mut self) -> FallibleCommandConfig<'_, Remove, Self> where T: Component, { - self.commands.add(Remove:: { - entity: self.entity, - phantom: PhantomData, - }); - self + FallibleCommandConfig::new( + Remove { + entity: self.entity, + phantom: PhantomData, + }, + self, + ) } /// Despawns only the specified entity, not including its children. - pub fn despawn(&mut self) { - self.commands.add(Despawn { - entity: self.entity, - }) + pub fn despawn(&mut self) -> FinalFallibleCommandConfig<'_, Despawn, Self> { + FinalFallibleCommandConfig::new( + Despawn { + entity: self.entity, + }, + self, + ) } /// Returns the underlying `[Commands]`. @@ -283,6 +330,12 @@ impl<'a, 'b> EntityCommands<'a, 'b> { } } +impl<'a, 'b> AddCommand for EntityCommands<'a, 'b> { + fn add_command(&mut self, command: impl Command) { + self.commands.add_command(command); + } +} + #[derive(Debug)] pub struct Spawn { pub bundle: T, @@ -320,10 +373,22 @@ pub struct Despawn { pub entity: Entity, } -impl Command for Despawn { - fn write(self: Box, world: &mut World) { - if !world.despawn(self.entity) { - debug!("Failed to despawn non-existent entity {:?}", self.entity); +/// The error resulting from [`EntityCommands::despawn`] +#[derive(Debug)] +pub struct DespawnError { + pub entity: Entity, +} + +impl FallibleCommand for Despawn { + type Error = DespawnError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { + if world.despawn(self.entity) { + Ok(()) + } else { + Err(DespawnError { + entity: self.entity, + }) } } } @@ -333,12 +398,38 @@ pub struct InsertBundle { pub bundle: T, } -impl Command for InsertBundle +/// The error resulting from [`EntityCommands::insert_bundle`] +/// Contains both the failed to insert bundle and the relative entity. +pub struct InsertBundleError { + pub entity: Entity, + pub bundle: T, +} + +impl Debug for InsertBundleError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InsertBundleError") + .field("entity", &self.entity) + .field("bundle_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for InsertBundle where T: Bundle + 'static, { - fn write(self: Box, world: &mut World) { - world.entity_mut(self.entity).insert_bundle(self.bundle); + type Error = InsertBundleError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { + if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { + entity_mut.insert_bundle(self.bundle); + Ok(()) + } else { + Err(InsertBundleError { + entity: self.entity, + bundle: self.bundle, + }) + } } } @@ -348,12 +439,39 @@ pub struct Insert { pub component: T, } -impl Command for Insert +/// The error resulting from [`EntityCommands::insert`] +/// Contains both the failed to insert component and the relative entity. +pub struct InsertError { + pub entity: Entity, + pub component: T, +} + +impl Debug for InsertError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InsertError") + .field("entity", &self.entity) + .field("component_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for Insert where T: Component, { - fn write(self: Box, world: &mut World) { - world.entity_mut(self.entity).insert(self.component); + type Error = InsertError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { + match world.get_entity_mut(self.entity) { + Some(mut entity) => { + entity.insert(self.component); + Ok(()) + } + None => Err(InsertError { + entity: self.entity, + component: self.component, + }), + } } } @@ -363,13 +481,36 @@ pub struct Remove { phantom: PhantomData, } -impl Command for Remove +/// The error resulting from [`EntityCommands::remove`] +pub struct RemoveError { + pub entity: Entity, + phantom: PhantomData, +} + +impl Debug for RemoveError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveError") + .field("entity", &self.entity) + .field("component_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for Remove where T: Component, { - fn write(self: Box, world: &mut World) { + type Error = RemoveError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { entity_mut.remove::(); + Ok(()) + } else { + Err(RemoveError { + entity: self.entity, + phantom: PhantomData, + }) } } } @@ -380,15 +521,38 @@ pub struct RemoveBundle { pub phantom: PhantomData, } -impl Command for RemoveBundle +/// The error resulting from [`EntityCommands::remove_bundle`] +pub struct RemoveBundleError { + pub entity: Entity, + phantom: PhantomData, +} + +impl Debug for RemoveBundleError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveBundleError") + .field("entity", &self.entity) + .field("bundle_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for RemoveBundle where T: Bundle, { - fn write(self: Box, world: &mut World) { + type Error = RemoveBundleError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { // remove intersection to gracefully handle components that were removed before running // this command entity_mut.remove_bundle_intersection::(); + Ok(()) + } else { + Err(RemoveBundleError { + entity: self.entity, + phantom: PhantomData, + }) } } } @@ -407,18 +571,42 @@ pub struct RemoveResource { pub phantom: PhantomData, } -impl Command for RemoveResource { - fn write(self: Box, world: &mut World) { - world.remove_resource::(); +/// The error resulting from [`Commands::remove_resource`] +pub struct RemoveResourceError { + phantom: PhantomData, +} + +impl Debug for RemoveResourceError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveResourceError") + .field("resource_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for RemoveResource { + type Error = RemoveResourceError; + + fn try_write(self, world: &mut World) -> Result<(), Self::Error> { + if world.remove_resource::().is_some() { + Ok(()) + } else { + Err(RemoveResourceError { + phantom: PhantomData, + }) + } } } #[cfg(test)] #[allow(clippy::float_cmp, clippy::approx_constant)] mod tests { + use crate as bevy_ecs; use crate::{ + bundle::Bundle, component::{ComponentDescriptor, StorageType}, - system::{CommandQueue, Commands}, + entity::Entity, + system::{CommandErrorHandler, CommandQueue, Commands, FallibleCommand}, world::World, }; use std::sync::{ @@ -546,4 +734,157 @@ mod tests { assert!(!world.contains_resource::()); assert!(world.contains_resource::()); } + + struct FailingCommand; + impl FallibleCommand for FailingCommand { + type Error = (); + + fn try_write(self, _: &mut World) -> Result<(), Self::Error> { + Err(()) + } + } + + struct SuccessfulCommand; + impl FallibleCommand for SuccessfulCommand { + type Error = (); + + fn try_write(self, _: &mut World) -> Result<(), Self::Error> { + Ok(()) + } + } + + #[test] + fn test_commands_error_handler() { + let invoked = Arc::new(AtomicUsize::new(0)); + let mut world = World::default(); + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + + commands.insert_resource(42u32); + let invoked_clone = invoked.clone(); + // should succeed + commands.remove_resource::().on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should fail + commands.remove_resource::().on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should fail + commands.add_fallible(FailingCommand).on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should succeed + commands + .add_fallible(SuccessfulCommand) + .on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + } + queue.apply(&mut world); + + assert_eq!(invoked.load(Ordering::Relaxed), 2); + } + + #[test] + fn test_entity_commands_error_handler() { + #[derive(Bundle)] + struct TestBundle { + value: u32, + } + + let invoked = Arc::new(AtomicUsize::new(0)); + + let mut world = World::default(); + + let valid_entity = world.spawn().id(); + let invalid_entity = Entity::new(42); + + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + + // EntityCommands::despawn + let mut try_despawn = |e| { + let invoked_clone = invoked.clone(); + commands.entity(e).despawn().on_err(move |error, _| { + assert_eq!(error.entity, e); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + }; + + try_despawn(invalid_entity); + try_despawn(valid_entity); + + // EntityCommands::insert + let invoked_clone = invoked.clone(); + commands + .entity(invalid_entity) + .insert(42) + .on_err(move |error, _| { + assert_eq!(error.entity, invalid_entity); + assert_eq!(error.component, 42); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + // EntityCommands::insert_bundle + let invoked_clone = invoked.clone(); + commands + .entity(invalid_entity) + .insert_bundle(TestBundle { value: 42 }) + .on_err(move |error, _| { + assert_eq!(error.entity, invalid_entity); + assert_eq!(error.bundle.value, 42); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + // EntityCommands::remove + let invoked_clone = invoked.clone(); + commands + .entity(invalid_entity) + .remove::() + .on_err(move |error, _| { + assert_eq!(error.entity, invalid_entity); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + // EntityCommands::remove_resource + let invoked_clone = invoked.clone(); + commands + .entity(invalid_entity) + .remove_bundle::() + .on_err(move |error, _| { + assert_eq!(error.entity, invalid_entity); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + } + queue.apply(&mut world); + + assert_eq!(invoked.load(Ordering::Relaxed), 5); + } + + #[test] + #[should_panic] + fn test_panicking_error_handler() { + std::panic::set_hook(Box::new(|_| {})); // prevents printing of stack trace. + + let mut world = World::default(); + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + let invalid_entity = Entity::new(42); + commands + .entity(invalid_entity) + .despawn() + .on_err(CommandErrorHandler::panic); + } + queue.apply(&mut world); + } } diff --git a/examples/README.md b/examples/README.md index e3b8878c136f0..3d0bb113807c2 100644 --- a/examples/README.md +++ b/examples/README.md @@ -157,6 +157,7 @@ Example | File | Description --- | --- | --- `ecs_guide` | [`ecs/ecs_guide.rs`](./ecs/ecs_guide.rs) | Full guide to Bevy's ECS `change_detection` | [`ecs/change_detection.rs`](./ecs/change_detection.rs) | Change detection on components +`command_error_handling` | [`ecs/command_error_handling.rs`](./ecs/command_error_handling.rs) | Error handling fallible commands `event` | [`ecs/event.rs`](./ecs/event.rs) | Illustrates event creation, activation, and reception `fixed_timestep` | [`ecs/fixed_timestep.rs`](./ecs/fixed_timestep.rs) | Shows how to create systems that run every fixed timestep, rather than every tick `hierarchy` | [`ecs/hierarchy.rs`](./ecs/hierarchy.rs) | Creates a hierarchy of parents and children entities diff --git a/examples/ecs/command_error_handling.rs b/examples/ecs/command_error_handling.rs new file mode 100644 index 0000000000000..5459379b158e0 --- /dev/null +++ b/examples/ecs/command_error_handling.rs @@ -0,0 +1,77 @@ +use bevy::{core::FixedTimestep, prelude::*}; + +fn main() { + App::build() + .insert_resource(FailedDespawnAttempts(0)) + .add_startup_system(setup.system()) + .add_system( + remove_components + .system() + .with_run_criteria(FixedTimestep::step(0.5)), + ) + .add_system_set( + SystemSet::new() + .with_run_criteria(FixedTimestep::step(2.0)) + .with_system(despawn_all_entities.system()) + .with_system(log_failed_despawn_attempts.system()), + ) + .run(); +} + +struct A(usize); + +#[derive(Bundle, Default)] +struct B { + value: usize, +} + +struct FailedDespawnAttempts(usize); + +fn setup(mut commands: Commands) { + for i in 0..3 { + // Note that `insert` and `insert_bundle` are fallible functions. + // If no error handler is specified, the default behavior is to log the error, and continue. + // However, these calls to `insert` and `insert_bundle` will not fail, since the entity is valid. + commands.spawn().insert(A(i)).insert_bundle(B::default()); + } +} + +fn log_failed_despawn_attempts(attempts: Res) { + info!("There have been {} failed despawn attempts!", attempts.0); +} + +fn despawn_all_entities(mut commands: Commands, query: Query) { + for e in query.iter() { + // `on_err` also allows you to provide a custom error handler! + commands.entity(e).despawn().on_err(|error, ctx| { + // You'll notice that the `error` will also give you back the entity + // you tried to despawn. + let entity = error.entity; + + warn!("Sadly our entity '{:?}' didn't despawn :(", entity); + + // error handlers have mutable access to `World` + if let Some(mut failed_despawns) = ctx.world.get_resource_mut::() + { + failed_despawns.0 += 1; + } + }); + } +} + +fn remove_components(mut commands: Commands, query: Query) { + for e in query.iter() { + // Some nice things: + // - You can still chain commands! + // - There are a slew of built-in error handlers + commands + .entity(e) + .remove::() + // `CommandErrorHandler::ignore` will neither log nor panic the error + .on_err(CommandErrorHandler::ignore) + .remove_bundle::() + // `CommandErrorHandler::log` is the default behavior, and will log the error. + // `CommandErrorHandler::panic` is another alternative which will panic on the error. + .on_err(CommandErrorHandler::log); + } +} From 64546c471d16ea98cd20139e9de82ca1fece125b Mon Sep 17 00:00:00 2001 From: NathanSWard Date: Mon, 28 Jun 2021 22:14:41 -0500 Subject: [PATCH 02/15] condense impls --- crates/bevy_ecs/src/system/commands/config.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs index a73c7b9a506b1..c3180d623eef8 100644 --- a/crates/bevy_ecs/src/system/commands/config.rs +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -141,14 +141,7 @@ macro_rules! impl_fallible_commands { #[inline] #[allow(dead_code)] fn return_unit(&self) {} - } - impl<'a, C, T> $name<'a, C, T> - where - C: FallibleCommand, - C::Error: Debug, - T: AddCommand, - { /// If the command failed, run the provided `error_handler`. /// /// ## Note @@ -167,6 +160,7 @@ macro_rules! impl_fallible_commands { /// commands.spawn().insert(42).on_err(|error, ctx| {}); /// } /// ``` + #[inline] pub fn on_err( &mut self, error_handler: impl FnOnce(C::Error, CommandContext) + Send + Sync + 'static, @@ -188,9 +182,13 @@ macro_rules! impl_fallible_commands { C: FallibleCommand, T: AddCommand, { + #[inline] fn drop(&mut self) { - if self.command.is_some() { - self.on_err(CommandErrorHandler::log); + if let Some(command) = self.command.take() { + self.inner.add_command(HandledErrorCommand { + command, + error_handler: CommandErrorHandler::log, + }); } } } From c8ac67eff28e21b75eb7ca7b5a8552516426bc6d Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 22:43:57 +0200 Subject: [PATCH 03/15] Working first version of failable commands --- Cargo.toml | 8 + crates/bevy_ecs/src/lib.rs | 1 + crates/bevy_ecs/src/system/commands/config.rs | 15 +- crates/bevy_ecs/src/system/commands/mod.rs | 382 ++++++++++++++++-- crates/bevy_ecs/src/world/mod.rs | 1 - examples/ecs/command_error_handling.rs | 42 +- 6 files changed, 387 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index e5f397e42be26..5ad9346bc0ed1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -303,6 +303,8 @@ bytemuck = "1.7" futures-lite = "2.0.1" crossbeam-channel = "0.5.0" argh = "0.1.12" +# Needed for command handler example +tracing-subscriber = "0.3.18" [[example]] name = "hello_world" @@ -1309,6 +1311,12 @@ wasm = false name = "command_error_handling" path = "examples/ecs/command_error_handling.rs" +[package.metadata.example.command_error_handling] +name = "Command Error Handling" +description = "Command error handling" +category = "ECS (Entity Component System)" +wasm = false + [[example]] name = "event" path = "examples/ecs/event.rs" diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 4ffd78a29b3f1..96a7851dd3262 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -45,6 +45,7 @@ pub mod prelude { system::{ Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands, ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction, + CommandErrorHandler, FallibleCommand, }, world::{EntityMut, EntityRef, EntityWorldMut, FromWorld, World}, }; diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs index c3180d623eef8..20f0ae44cf1da 100644 --- a/crates/bevy_ecs/src/system/commands/config.rs +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -61,7 +61,7 @@ impl CommandErrorHandler { /// ## Note /// This is the default behavior if no error handler is specified. pub fn log(error: E, _ctx: CommandContext) { - error!("Commands failed with error: {:?}", error) + error!("Commands failed with error: {:?}", error); } /// If the command failed, [`panic!`] with the error. @@ -87,13 +87,13 @@ where C: FallibleCommand, F: FnOnce(C::Error, CommandContext) + Send + Sync + 'static, { - fn write(self: Box, world: &mut World) { + fn apply(self: Self, world: &mut World) { let HandledErrorCommand { command, error_handler, - } = *self; + } = self; - if let Err(error) = command.try_write(world) { + if let Err(error) = command.try_apply(world) { error_handler(error, CommandContext { world }); } } @@ -152,12 +152,15 @@ macro_rules! impl_fallible_commands { /// ``` /// use bevy_ecs::prelude::*; /// + /// #[derive(Component, Resource)] + /// struct TestComponent(pub u32); + /// /// fn system(mut commands: Commands) { /// // built-in error handler - /// commands.spawn().insert(42).on_err(CommandErrorHandler::ignore); + /// commands.spawn_empty().insert(TestComponent(42)).on_err(CommandErrorHandler::ignore); /// /// // custom error handler - /// commands.spawn().insert(42).on_err(|error, ctx| {}); + /// commands.spawn_empty().insert(TestComponent(42)).on_err(|error, ctx| {}); /// } /// ``` #[inline] diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 80de73f9f15ab..8a801afe3b8b9 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,3 +1,4 @@ +mod config; mod command_queue; mod parallel_scope; @@ -12,7 +13,8 @@ use bevy_ecs_macros::SystemParam; use bevy_utils::tracing::{error, info}; pub use command_queue::CommandQueue; pub use parallel_scope::*; -use std::marker::PhantomData; +pub use config::*; +use std::{fmt::Debug, marker::PhantomData}; use super::{Deferred, Resource, SystemBuffer, SystemMeta}; @@ -52,6 +54,23 @@ pub trait Command: Send + 'static { fn apply(self, world: &mut World); } +/// A [`World`] mutation that can potentially fail. +/// For an infallible variant, use [`Command`]. +pub trait FallibleCommand: Send + Sync + 'static { + /// The type of error that this command may return. + type Error: Debug; + + /// Like Command::apply it applies this command causing it to mutate the provided `world` but + /// it won't panic if an error occurs. + /// + /// 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 try_apply(self, world: &mut World) -> Result<(), Self::Error>; +} + + + /// A [`Command`] queue to perform structural changes to the [`World`]. /// /// Since each command requires exclusive access to the `World`, @@ -519,8 +538,14 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn remove_resource(&mut self) { - self.queue.push(RemoveResource::::new()); + pub fn remove_resource(&mut self) -> FinalFallibleCommandConfig<'_, RemoveResource, Self> { + // self.queue.push(RemoveResource::::new()); + FinalFallibleCommandConfig::new( + RemoveResource { + _marker: PhantomData, + }, + self, + ) } /// Runs the system corresponding to the given [`SystemId`]. @@ -586,8 +611,23 @@ impl<'w, 's> Commands<'w, 's> { pub fn add(&mut self, command: C) { self.queue.push(command); } + + /// Adds a fallible command to the command list. + pub fn add_fallible(&mut self, command: C) -> FinalFallibleCommandConfig<'_, C, Self> + where + C: FallibleCommand, + { + FinalFallibleCommandConfig::new(command, self) + } +} + +impl<'a, 'w> AddCommand for Commands<'a, 'w> { + fn add_command(&mut self, command: impl Command) { + self.add(command); + } } + /// A [`Command`] which gets executed for a given [`Entity`]. /// /// # Examples @@ -746,12 +786,17 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(add_combat_stats_system); /// ``` - pub fn insert(&mut self, bundle: impl Bundle) -> &mut Self { - self.commands.add(Insert { - entity: self.entity, - bundle, - }); - self + pub fn insert( + &mut self, + bundle: T, + ) -> FallibleCommandConfig<'_, Insert, Self> { + FallibleCommandConfig::new( + Insert { + entity: self.entity, + bundle, + }, + self, + ) } /// Tries to add a [`Bundle`] of components to the entity. @@ -810,6 +855,7 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { self } + /// Removes a [`Bundle`] of components from the entity. /// /// # Example @@ -845,12 +891,17 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` - pub fn remove(&mut self) -> &mut Self + pub fn remove(&mut self) -> FallibleCommandConfig<'_, Remove, Self> where T: Bundle, { - self.commands.add(Remove::::new(self.entity)); - self + FallibleCommandConfig::new( + Remove { + entity: self.entity, + _marker: PhantomData, + }, + self, + ) } /// Despawns the entity. @@ -883,10 +934,13 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(remove_character_system); /// ``` - pub fn despawn(&mut self) { - self.commands.add(Despawn { - entity: self.entity, - }); + pub fn despawn(&mut self) -> FinalFallibleCommandConfig<'_, Despawn, Self> { + FinalFallibleCommandConfig::new( + Despawn { + entity: self.entity, + }, + self, + ) } /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. @@ -947,12 +1001,17 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(remove_combat_stats_system); /// ``` - pub fn retain(&mut self) -> &mut Self + pub fn retain(&mut self) -> FallibleCommandConfig<'_, Retain, Self> where T: Bundle, { - self.commands.add(Retain::::new(self.entity)); - self + FallibleCommandConfig::new( + Retain { + entity: self.entity, + _marker: PhantomData + }, + self, + ) } /// Logs the components of the entity at the info level. @@ -972,6 +1031,12 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { } } +impl<'w, 's, 'a> AddCommand for EntityCommands<'w, 's, 'a> { + fn add_command(&mut self, command: impl Command) { + self.commands.add_command(command); + } +} + impl Command for F where F: FnOnce(&mut World) + Send + 'static, @@ -1072,9 +1137,24 @@ pub struct Despawn { pub entity: Entity, } -impl Command for Despawn { - fn apply(self, world: &mut World) { - world.despawn(self.entity); +/// The error resulting from [`EntityCommands::despawn`] +#[derive(Debug)] +pub struct DespawnError { + /// The entity that was supposed to get despawned. + pub entity: Entity, +} + +impl FallibleCommand for Despawn { + type Error = DespawnError; + + fn try_apply(self, world: &mut World) -> Result<(), Self::Error> { + if world.despawn(self.entity) { + Ok(()) + } else { + Err(DespawnError { + entity: self.entity, + }) + } } } @@ -1086,15 +1166,39 @@ pub struct Insert { pub bundle: T, } -impl Command for Insert +/// The error resulting from [`EntityCommands::insert`] +/// Contains both the failed to insert component and the relative entity. +pub struct InsertError { + /// The entity that was supposed to get the bundle inserted. + pub entity: Entity, + /// The bundle that was supposed to get inserted. + pub bundle: T, +} + +impl Debug for InsertError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("InsertError") + .field("entity", &self.entity) + .field("bundle_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for Insert where T: Bundle + 'static, { - fn apply(self, world: &mut World) { - if let Some(mut entity) = world.get_entity_mut(self.entity) { - entity.insert(self.bundle); + type Error = InsertError; + + fn try_apply(self, world: &mut World) -> Result<(), Self::Error> { + if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { + entity_mut.insert(self.bundle); + Ok(()) } else { - panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World.", std::any::type_name::(), self.entity); + Err(InsertError { + entity: self.entity, + bundle: self.bundle, + }) } } } @@ -1128,13 +1232,37 @@ pub struct Remove { _marker: PhantomData, } -impl Command for Remove +/// The error resulting from [`EntityCommands::remove`] +pub struct RemoveError { + /// The entity that was supposed to get the bundle removed. + pub entity: Entity, + phantom: PhantomData, +} + +impl Debug for RemoveError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveError") + .field("entity", &self.entity) + .field("component_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for Remove where T: Bundle, { - fn apply(self, world: &mut World) { + type Error = RemoveError; + + fn try_apply(self, world: &mut World) -> Result<(), Self::Error> { if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { entity_mut.remove::(); + Ok(()) + } else { + Err(RemoveError { + entity: self.entity, + phantom: PhantomData, + }) } } } @@ -1159,13 +1287,38 @@ pub struct Retain { _marker: PhantomData, } -impl Command for Retain +/// The error resulting from [`Commands::retain`] +pub struct RemoveRetainError { + /// The entity that was supposed to get the bundle retained. + pub entity: Entity, + _marker: PhantomData +} + +impl Debug for RemoveRetainError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveRetainError") + .field("entity", &self.entity) + .field("component_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for Retain where T: Bundle, { - fn apply(self, world: &mut World) { - if let Some(mut entity_mut) = world.get_entity_mut(self.entity) { - entity_mut.retain::(); + type Error = RemoveRetainError; + + fn try_apply(self, world: &mut World) -> Result<(), Self::Error> { + match world.get_entity_mut(self.entity) { + Some(mut entity_mut) => { + entity_mut.retain::(); + Ok(()) + }, + None => Err(RemoveRetainError { + entity: self.entity, + _marker: PhantomData, + }) } } } @@ -1218,9 +1371,30 @@ pub struct RemoveResource { _marker: PhantomData, } -impl Command for RemoveResource { - fn apply(self, world: &mut World) { - world.remove_resource::(); +/// The error resulting from [`Commands::remove_resource`] +pub struct RemoveResourceError { + phantom: PhantomData, +} + +impl Debug for RemoveResourceError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("RemoveResourceError") + .field("resource_type", &std::any::type_name::()) + .finish() + } +} + +impl FallibleCommand for RemoveResource { + type Error = RemoveResourceError; + + fn try_apply(self, world: &mut World) -> Result<(), Self::Error> { + if world.remove_resource::().is_some() { + Ok(()) + } else { + Err(RemoveResourceError { + phantom: PhantomData, + }) + } } } @@ -1255,7 +1429,8 @@ mod tests { use crate::{ self as bevy_ecs, component::Component, - system::{CommandQueue, Commands, Resource}, + entity::Entity, + system::{CommandErrorHandler, CommandQueue, Commands, Resource, FallibleCommand}, world::World, }; use std::sync::{ @@ -1428,4 +1603,137 @@ mod tests { assert!(world.contains_resource::>()); assert!(world.contains_resource::>()); } + + struct FailingCommand; + impl FallibleCommand for FailingCommand { + type Error = (); + + fn try_apply(self, _: &mut World) -> Result<(), Self::Error> { + Err(()) + } + } + + struct SuccessfulCommand; + impl FallibleCommand for SuccessfulCommand { + type Error = (); + + fn try_apply(self, _: &mut World) -> Result<(), Self::Error> { + Ok(()) + } + } + + #[test] + fn test_commands_error_handler() { + let invoked = Arc::new(AtomicUsize::new(0)); + let mut world = World::default(); + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + + commands.insert_resource(W(42u32)); + let invoked_clone = invoked.clone(); + // should succeed + commands.remove_resource::>().on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should fail + commands.remove_resource::>().on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should fail + commands.add_fallible(FailingCommand).on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + let invoked_clone = invoked.clone(); + // should succeed + commands + .add_fallible(SuccessfulCommand) + .on_err(move |_, _| { + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + } + queue.apply(&mut world); + + assert_eq!(invoked.load(Ordering::Relaxed), 2); + } + + #[test] + fn test_entity_commands_error_handler() { + #[derive(Component)] + struct TestComponent { + value: u32, + } + + let invoked = Arc::new(AtomicUsize::new(0)); + + let mut world = World::default(); + + let valid_entity = world.spawn_empty().id(); + let entity_for_checking_despawning = world.spawn_empty().id(); + + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + + // EntityCommands::despawn + let mut try_despawn = |e| { + let invoked_clone = invoked.clone(); + commands.entity(e).despawn().on_err(move |error, _| { + assert_eq!(error.entity, e); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + }; + + try_despawn(entity_for_checking_despawning); + try_despawn(entity_for_checking_despawning); + try_despawn(valid_entity); + + // EntityCommands::insert + let invoked_clone = invoked.clone(); + commands + .entity(valid_entity) + .insert(TestComponent { value: 42 }) + .on_err(move |error, _| { + assert_eq!(error.entity, valid_entity); + assert_eq!(error.bundle.value, 42); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + + // EntityCommands::remove_resource + let invoked_clone = invoked.clone(); + commands + .entity(valid_entity) + .remove::() + .on_err(move |error, _| { + assert_eq!(error.entity, valid_entity); + invoked_clone.fetch_add(1, Ordering::Relaxed); + }); + } + queue.apply(&mut world); + + assert_eq!(invoked.load(Ordering::Relaxed), 3); + } + + #[test] + #[should_panic] + fn test_panicking_error_handler() { + std::panic::set_hook(Box::new(|_| {})); // prevents printing of stack trace. + + let mut world = World::default(); + let mut queue = CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + let invalid_entity = Entity::new(42, 0); + commands + .entity(invalid_entity) + .despawn() + .on_err(CommandErrorHandler::panic); + } + queue.apply(&mut world); + } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index fe6533e8310ae..8000e45cd99cc 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -879,7 +879,6 @@ impl World { entity.despawn(); true } else { - warn!("error[B0003]: Could not despawn entity {:?} because it doesn't exist in this World.", entity); false } } diff --git a/examples/ecs/command_error_handling.rs b/examples/ecs/command_error_handling.rs index 5459379b158e0..607ae24efef56 100644 --- a/examples/ecs/command_error_handling.rs +++ b/examples/ecs/command_error_handling.rs @@ -1,38 +1,36 @@ -use bevy::{core::FixedTimestep, prelude::*}; +use bevy::prelude::*; fn main() { - App::build() + tracing_subscriber::fmt::init(); + + App::new() .insert_resource(FailedDespawnAttempts(0)) - .add_startup_system(setup.system()) - .add_system( - remove_components - .system() - .with_run_criteria(FixedTimestep::step(0.5)), - ) - .add_system_set( - SystemSet::new() - .with_run_criteria(FixedTimestep::step(2.0)) - .with_system(despawn_all_entities.system()) - .with_system(log_failed_despawn_attempts.system()), - ) + .add_systems(Startup, setup) + .add_systems(Update, ( + despawn_all_entities, + remove_components, + log_failed_despawn_attempts.after(despawn_all_entities), + )) .run(); } +#[derive(Component)] struct A(usize); -#[derive(Bundle, Default)] +#[derive(Component, Default)] struct B { value: usize, } +#[derive(Resource)] struct FailedDespawnAttempts(usize); fn setup(mut commands: Commands) { for i in 0..3 { - // Note that `insert` and `insert_bundle` are fallible functions. + // Note that `insert` is a fallible function. // If no error handler is specified, the default behavior is to log the error, and continue. // However, these calls to `insert` and `insert_bundle` will not fail, since the entity is valid. - commands.spawn().insert(A(i)).insert_bundle(B::default()); + commands.spawn_empty().insert(A(i)).insert(B::default()); } } @@ -41,6 +39,14 @@ fn log_failed_despawn_attempts(attempts: Res) { } fn despawn_all_entities(mut commands: Commands, query: Query) { + info!("Despawning entities..."); + for e in query.iter() { + // `on_err` also allows you to provide a custom error handler! + commands.entity(e).despawn().on_err(CommandErrorHandler::ignore); + } + + + info!("Trying to despawn again..."); for e in query.iter() { // `on_err` also allows you to provide a custom error handler! commands.entity(e).despawn().on_err(|error, ctx| { @@ -69,7 +75,7 @@ fn remove_components(mut commands: Commands, query: Query) { .remove::() // `CommandErrorHandler::ignore` will neither log nor panic the error .on_err(CommandErrorHandler::ignore) - .remove_bundle::() + .remove::() // `CommandErrorHandler::log` is the default behavior, and will log the error. // `CommandErrorHandler::panic` is another alternative which will panic on the error. .on_err(CommandErrorHandler::log); From 24fbbbcd41178864919cf1ee16960aadf92f0902 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 23:08:52 +0200 Subject: [PATCH 04/15] Added documentation for config.rs types --- crates/bevy_ecs/src/system/commands/config.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs index 20f0ae44cf1da..8e1c6810b6b26 100644 --- a/crates/bevy_ecs/src/system/commands/config.rs +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -100,7 +100,9 @@ where } #[non_exhaustive] +/// Context passed to [`CommandErrorHandler`]. pub struct CommandContext<'a> { + /// The [`World`] the command was applied to. pub world: &'a mut World, } From 919418ee91c9564e96c289b7587501eea6af2d13 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 23:12:42 +0200 Subject: [PATCH 05/15] Formatted the code --- crates/bevy_ecs/src/lib.rs | 6 ++--- crates/bevy_ecs/src/system/commands/mod.rs | 27 +++++++++------------- examples/ecs/command_error_handling.rs | 19 +++++++++------ 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 96a7851dd3262..cf821fedd33bf 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -43,9 +43,9 @@ pub mod prelude { OnTransition, Schedule, Schedules, State, States, SystemSet, }, system::{ - Commands, Deferred, In, IntoSystem, Local, NonSend, NonSendMut, ParallelCommands, - ParamSet, Query, ReadOnlySystem, Res, ResMut, Resource, System, SystemParamFunction, - CommandErrorHandler, FallibleCommand, + CommandErrorHandler, Commands, Deferred, FallibleCommand, In, IntoSystem, Local, + NonSend, NonSendMut, ParallelCommands, ParamSet, Query, ReadOnlySystem, Res, ResMut, + Resource, System, SystemParamFunction, }, world::{EntityMut, EntityRef, EntityWorldMut, FromWorld, World}, }; diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 8a801afe3b8b9..98efe51b8072a 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1,5 +1,5 @@ -mod config; mod command_queue; +mod config; mod parallel_scope; use crate::{ @@ -12,8 +12,8 @@ use crate::{ use bevy_ecs_macros::SystemParam; use bevy_utils::tracing::{error, info}; pub use command_queue::CommandQueue; -pub use parallel_scope::*; pub use config::*; +pub use parallel_scope::*; use std::{fmt::Debug, marker::PhantomData}; use super::{Deferred, Resource, SystemBuffer, SystemMeta}; @@ -69,8 +69,6 @@ pub trait FallibleCommand: Send + Sync + 'static { fn try_apply(self, world: &mut World) -> Result<(), Self::Error>; } - - /// A [`Command`] queue to perform structural changes to the [`World`]. /// /// Since each command requires exclusive access to the `World`, @@ -538,7 +536,9 @@ impl<'w, 's> Commands<'w, 's> { /// # } /// # bevy_ecs::system::assert_is_system(system); /// ``` - pub fn remove_resource(&mut self) -> FinalFallibleCommandConfig<'_, RemoveResource, Self> { + pub fn remove_resource( + &mut self, + ) -> FinalFallibleCommandConfig<'_, RemoveResource, Self> { // self.queue.push(RemoveResource::::new()); FinalFallibleCommandConfig::new( RemoveResource { @@ -627,7 +627,6 @@ impl<'a, 'w> AddCommand for Commands<'a, 'w> { } } - /// A [`Command`] which gets executed for a given [`Entity`]. /// /// # Examples @@ -786,10 +785,7 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// } /// # bevy_ecs::system::assert_is_system(add_combat_stats_system); /// ``` - pub fn insert( - &mut self, - bundle: T, - ) -> FallibleCommandConfig<'_, Insert, Self> { + pub fn insert(&mut self, bundle: T) -> FallibleCommandConfig<'_, Insert, Self> { FallibleCommandConfig::new( Insert { entity: self.entity, @@ -855,7 +851,6 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { self } - /// Removes a [`Bundle`] of components from the entity. /// /// # Example @@ -1008,7 +1003,7 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { FallibleCommandConfig::new( Retain { entity: self.entity, - _marker: PhantomData + _marker: PhantomData, }, self, ) @@ -1291,7 +1286,7 @@ pub struct Retain { pub struct RemoveRetainError { /// The entity that was supposed to get the bundle retained. pub entity: Entity, - _marker: PhantomData + _marker: PhantomData, } impl Debug for RemoveRetainError { @@ -1314,11 +1309,11 @@ where Some(mut entity_mut) => { entity_mut.retain::(); Ok(()) - }, + } None => Err(RemoveRetainError { entity: self.entity, _marker: PhantomData, - }) + }), } } } @@ -1430,7 +1425,7 @@ mod tests { self as bevy_ecs, component::Component, entity::Entity, - system::{CommandErrorHandler, CommandQueue, Commands, Resource, FallibleCommand}, + system::{CommandErrorHandler, CommandQueue, Commands, FallibleCommand, Resource}, world::World, }; use std::sync::{ diff --git a/examples/ecs/command_error_handling.rs b/examples/ecs/command_error_handling.rs index 607ae24efef56..44adf37eca588 100644 --- a/examples/ecs/command_error_handling.rs +++ b/examples/ecs/command_error_handling.rs @@ -6,11 +6,14 @@ fn main() { App::new() .insert_resource(FailedDespawnAttempts(0)) .add_systems(Startup, setup) - .add_systems(Update, ( - despawn_all_entities, - remove_components, - log_failed_despawn_attempts.after(despawn_all_entities), - )) + .add_systems( + Update, + ( + despawn_all_entities, + remove_components, + log_failed_despawn_attempts.after(despawn_all_entities), + ), + ) .run(); } @@ -42,10 +45,12 @@ fn despawn_all_entities(mut commands: Commands, query: Query) { info!("Despawning entities..."); for e in query.iter() { // `on_err` also allows you to provide a custom error handler! - commands.entity(e).despawn().on_err(CommandErrorHandler::ignore); + commands + .entity(e) + .despawn() + .on_err(CommandErrorHandler::ignore); } - info!("Trying to despawn again..."); for e in query.iter() { // `on_err` also allows you to provide a custom error handler! From e88e06b95140113d4a32ff130af942be04fa9a96 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 23:14:44 +0200 Subject: [PATCH 06/15] Fixed missing examples in README --- examples/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/README.md b/examples/README.md index db5ce7f96cfd9..149abf14b2f10 100644 --- a/examples/README.md +++ b/examples/README.md @@ -218,6 +218,7 @@ Example | Description Example | Description --- | --- [Apply System Buffers](../examples/ecs/apply_deferred.rs) | Show how to use `apply_deferred` system +[Command Error Handling](../examples/ecs/command_error_handling.rs) | Command error handling [Component Change Detection](../examples/ecs/component_change_detection.rs) | Change detection on components [Custom Query Parameters](../examples/ecs/custom_query_param.rs) | Groups commonly used compound queries and query filters into a single type [ECS Guide](../examples/ecs/ecs_guide.rs) | Full guide to Bevy's ECS From e9bc96333e9d96d2c891ae9cdd7fe35a1968a57c Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 23:22:47 +0200 Subject: [PATCH 07/15] Fixed cliipy errors --- crates/bevy_ecs/src/system/commands/config.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- examples/ecs/command_error_handling.rs | 4 +--- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs index 8e1c6810b6b26..2cce90870b52f 100644 --- a/crates/bevy_ecs/src/system/commands/config.rs +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -87,7 +87,7 @@ where C: FallibleCommand, F: FnOnce(C::Error, CommandContext) + Send + Sync + 'static, { - fn apply(self: Self, world: &mut World) { + fn apply(self, world: &mut World) { let HandledErrorCommand { command, error_handler, diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 98efe51b8072a..b18846ef09719 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -60,7 +60,7 @@ pub trait FallibleCommand: Send + Sync + 'static { /// The type of error that this command may return. type Error: Debug; - /// Like Command::apply it applies this command causing it to mutate the provided `world` but + /// Like `[Command::apply]` it applies this command causing it to mutate the provided `world` but /// it won't panic if an error occurs. /// /// This method is used to define what a command "does" when it is ultimately applied. diff --git a/examples/ecs/command_error_handling.rs b/examples/ecs/command_error_handling.rs index 44adf37eca588..c73dd191046c3 100644 --- a/examples/ecs/command_error_handling.rs +++ b/examples/ecs/command_error_handling.rs @@ -21,9 +21,7 @@ fn main() { struct A(usize); #[derive(Component, Default)] -struct B { - value: usize, -} +struct B(u32); #[derive(Resource)] struct FailedDespawnAttempts(usize); From 08c544d91189ae516727bd6e83fc5240dee0a09c Mon Sep 17 00:00:00 2001 From: EngoDev Date: Tue, 2 Jan 2024 23:45:53 +0200 Subject: [PATCH 08/15] Fixed document refernce error --- crates/bevy_ecs/src/system/commands/config.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/config.rs b/crates/bevy_ecs/src/system/commands/config.rs index 2cce90870b52f..5a5fc3df0b15a 100644 --- a/crates/bevy_ecs/src/system/commands/config.rs +++ b/crates/bevy_ecs/src/system/commands/config.rs @@ -14,7 +14,7 @@ pub trait AddCommand { } /// Provides configuration mechanisms in case a command errors. -/// You can specify a custom handler via [`FallibleCommandConfig::on_failure`] or +/// You can specify a custom handler via [`CommandErrorHandler`] or /// use one of the provided implementations. /// /// ## Note diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index b18846ef09719..b878a69d58941 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1282,7 +1282,7 @@ pub struct Retain { _marker: PhantomData, } -/// The error resulting from [`Commands::retain`] +/// The error resulting from [`EntityCommands::retain`] pub struct RemoveRetainError { /// The entity that was supposed to get the bundle retained. pub entity: Entity, From 64db7520c11413a9d6c20af5d6ebbfd01f7d7baa Mon Sep 17 00:00:00 2001 From: EngoDev Date: Sat, 20 Jan 2024 19:01:24 +0200 Subject: [PATCH 09/15] Fixed merge errors --- crates/bevy_ecs/src/system/commands/mod.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 7b0b18d57416c..bccec835adea7 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1023,6 +1023,15 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { } } +fn try_insert(bundle: impl Bundle) -> impl EntityCommand { + move |entity, world: &mut World| { + if let Some(mut entity) = world.get_entity_mut(entity) { + entity.insert(bundle); + } + } +} + + impl<'w, 's, 'a> AddCommand for EntityCommands<'w, 's, 'a> { fn add_command(&mut self, command: impl Command) { self.commands.add_command(command); @@ -1166,7 +1175,7 @@ where }) } } - } +} /// A [`Command`] that removes components from an entity. /// For a [`Bundle`] type `T`, this will remove any components in the bundle. @@ -1315,6 +1324,7 @@ impl FallibleCommand for RemoveResource { }) } } +} /// A [`Command`] that inserts a [`Resource`] into the world. From 420ebf873b0f056f1e4acf532d76bd7379399f31 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Sat, 20 Jan 2024 19:22:29 +0200 Subject: [PATCH 10/15] Fixed CI errors --- crates/bevy_ecs/src/system/commands/mod.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index bccec835adea7..999aa16f2ded2 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1031,7 +1031,6 @@ fn try_insert(bundle: impl Bundle) -> impl EntityCommand { } } - impl<'w, 's, 'a> AddCommand for EntityCommands<'w, 's, 'a> { fn add_command(&mut self, command: impl Command) { self.commands.add_command(command); @@ -1326,7 +1325,6 @@ impl FallibleCommand for RemoveResource { } } - /// A [`Command`] that inserts a [`Resource`] into the world. fn insert_resource(resource: R) -> impl Command { move |world: &mut World| { @@ -1649,7 +1647,8 @@ mod tests { let mut queue = CommandQueue::default(); { let mut commands = Commands::new(&mut queue, &world); - let invalid_entity = Entity::new(42, 0); + let invalid_entity = + Entity::from_raw_and_generation(42, std::num::NonZeroU32::new(0).unwrap()); commands .entity(invalid_entity) .despawn() From 1a2c6ea79536cd58cd50a93c034c65e82e04c867 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Sat, 20 Jan 2024 19:24:09 +0200 Subject: [PATCH 11/15] Fixed readme --- examples/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/README.md b/examples/README.md index 64c81e526e494..217a2263f5849 100644 --- a/examples/README.md +++ b/examples/README.md @@ -223,7 +223,6 @@ Example | Description Example | Description --- | --- -[Apply System Buffers](../examples/ecs/apply_deferred.rs) | Show how to use `apply_deferred` system [Command Error Handling](../examples/ecs/command_error_handling.rs) | Command error handling [Component Change Detection](../examples/ecs/component_change_detection.rs) | Change detection on components [Custom Query Parameters](../examples/ecs/custom_query_param.rs) | Groups commonly used compound queries and query filters into a single type From ddb9ad4fc168d8fbbaed329661adde74f4ab3989 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Mon, 22 Jan 2024 22:05:45 +0200 Subject: [PATCH 12/15] Added modified on_err benches --- benches/benches/bevy_ecs/world/commands.rs | 45 +++++++++++++++++++++- benches/benches/bevy_ecs/world/mod.rs | 1 + 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/benches/benches/bevy_ecs/world/commands.rs b/benches/benches/bevy_ecs/world/commands.rs index 70cf1351acfcb..d350b60ee2a66 100644 --- a/benches/benches/bevy_ecs/world/commands.rs +++ b/benches/benches/bevy_ecs/world/commands.rs @@ -1,7 +1,7 @@ use bevy_ecs::{ component::Component, entity::Entity, - system::{Command, CommandQueue, Commands}, + system::{Command, CommandQueue, Commands, CommandErrorHandler}, world::World, }; use criterion::{black_box, Criterion}; @@ -69,6 +69,49 @@ pub fn spawn_commands(criterion: &mut Criterion) { group.finish(); } +pub fn spawn_commands_error_handler(criterion: &mut Criterion) { + let mut group = criterion.benchmark_group("spawn_commands_error_handler"); + group.warm_up_time(std::time::Duration::from_millis(500)); + group.measurement_time(std::time::Duration::from_secs(4)); + + for entity_count in (1..5).map(|i| i * 2 * 1000) { + group.bench_function(format!("{}_entities", entity_count), |bencher| { + let mut world = World::default(); + let mut command_queue = CommandQueue::default(); + + bencher.iter(|| { + let mut commands = Commands::new(&mut command_queue, &world); + for i in 0..entity_count { + let mut entity = commands.spawn_empty(); + + if black_box(i % 2 == 0) { + entity.insert(A).on_err(CommandErrorHandler::log); + } + + if black_box(i % 3 == 0) { + entity.insert(B).on_err(CommandErrorHandler::ignore); + } + + if black_box(i % 4 == 0) { + entity.insert(C).on_err(|err, _ctx| { + println!("Error: {:?}", err); + }); + } + + if black_box(i % 5 == 0) { + entity.despawn().on_err(CommandErrorHandler::log); + } + } + command_queue.apply(&mut world); + }); + }); + } + + group.finish(); +} + + + #[derive(Default, Component)] struct Matrix([[f32; 4]; 4]); diff --git a/benches/benches/bevy_ecs/world/mod.rs b/benches/benches/bevy_ecs/world/mod.rs index f594d082d7fb0..4318a3765d21c 100644 --- a/benches/benches/bevy_ecs/world/mod.rs +++ b/benches/benches/bevy_ecs/world/mod.rs @@ -12,6 +12,7 @@ criterion_group!( world_benches, empty_commands, spawn_commands, + spawn_commands_error_handler, insert_commands, fake_commands, zero_sized_commands, From 349bee25be6276b7250b3ae1384e138ad401bee6 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Mon, 22 Jan 2024 22:17:27 +0200 Subject: [PATCH 13/15] Adding changed file after running ci locally --- .../tests/deref_mut_derive/missing_deref.fail.stderr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr b/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr index 3e11d49532e94..d4cfc71d7a920 100644 --- a/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr +++ b/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr @@ -6,6 +6,9 @@ error[E0277]: the trait bound `TupleStruct: Deref` is not satisfied | note: required by a bound in `DerefMut` --> $RUST/core/src/ops/deref.rs + | + | pub trait DerefMut: Deref { + | ^^^^^ required by this bound in `DerefMut` error[E0277]: the trait bound `Struct: Deref` is not satisfied --> tests/deref_mut_derive/missing_deref.fail.rs:7:8 @@ -15,6 +18,9 @@ error[E0277]: the trait bound `Struct: Deref` is not satisfied | note: required by a bound in `DerefMut` --> $RUST/core/src/ops/deref.rs + | + | pub trait DerefMut: Deref { + | ^^^^^ required by this bound in `DerefMut` error[E0277]: the trait bound `TupleStruct: Deref` is not satisfied --> tests/deref_mut_derive/missing_deref.fail.rs:3:10 From a60e87e50b136decf38366e86e859fd6f4f3e437 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Mon, 22 Jan 2024 22:29:19 +0200 Subject: [PATCH 14/15] Fixed error --- crates/bevy_ecs/src/system/commands/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index aa4435e5f5953..9dd5816ce162c 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1065,7 +1065,7 @@ fn try_insert(bundle: impl Bundle) -> impl EntityCommand { } } -impl<'w, 's, 'a> AddCommand for EntityCommands<'w, 's, 'a> { +impl<'a> AddCommand for EntityCommands<'a> { fn add_command(&mut self, command: impl Command) { self.commands.add_command(command); } From 93ad8ce501128d1a5709f340606dffed3af63063 Mon Sep 17 00:00:00 2001 From: EngoDev Date: Mon, 22 Jan 2024 23:19:49 +0200 Subject: [PATCH 15/15] Reverted change --- .../tests/deref_mut_derive/missing_deref.fail.stderr | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr b/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr index d4cfc71d7a920..3e11d49532e94 100644 --- a/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr +++ b/crates/bevy_macros_compile_fail_tests/tests/deref_mut_derive/missing_deref.fail.stderr @@ -6,9 +6,6 @@ error[E0277]: the trait bound `TupleStruct: Deref` is not satisfied | note: required by a bound in `DerefMut` --> $RUST/core/src/ops/deref.rs - | - | pub trait DerefMut: Deref { - | ^^^^^ required by this bound in `DerefMut` error[E0277]: the trait bound `Struct: Deref` is not satisfied --> tests/deref_mut_derive/missing_deref.fail.rs:7:8 @@ -18,9 +15,6 @@ error[E0277]: the trait bound `Struct: Deref` is not satisfied | note: required by a bound in `DerefMut` --> $RUST/core/src/ops/deref.rs - | - | pub trait DerefMut: Deref { - | ^^^^^ required by this bound in `DerefMut` error[E0277]: the trait bound `TupleStruct: Deref` is not satisfied --> tests/deref_mut_derive/missing_deref.fail.rs:3:10