From 5e2bbba5dfce0770bc2dad09d410bdde225398d0 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Tue, 15 Oct 2024 08:23:21 -0500 Subject: [PATCH 01/29] working prototype --- crates/bevy_ecs/src/system/commands/mod.rs | 78 ++++++++++++++++++++-- 1 file changed, 71 insertions(+), 7 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 15079b32a97ec..2a242a5f9be4e 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -308,6 +308,7 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), + failure_mode: FailureMode::Panic, } } @@ -334,6 +335,7 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), + failure_mode: FailureMode::Panic, } } @@ -485,6 +487,7 @@ impl<'w, 's> Commands<'w, 's> { self.entities.contains(entity).then_some(EntityCommands { entity, commands: self.reborrow(), + failure_mode: FailureMode::Panic, }) } @@ -1018,7 +1021,7 @@ impl<'w, 's> Commands<'w, 's> { /// ``` pub trait EntityCommand: Send + 'static { /// Executes this command for the given [`Entity`]. - fn apply(self, entity: Entity, world: &mut World); + fn apply(self, entity: Entity, world: &mut World, failure_mode: FailureMode); /// Returns a [`Command`] which executes this [`EntityCommand`] for the given [`Entity`]. /// @@ -1027,21 +1030,50 @@ pub trait EntityCommand: Send + 'static { /// 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 + fn with_entity(self, entity: Entity, failure_mode: FailureMode) -> impl Command where Self: Sized, { - move |world: &mut World| self.apply(entity, world) + move |world: &mut World| self.apply(entity, world, failure_mode) } } +#[derive(Clone, Copy)] +pub enum FailureMode { + Ignore, + Log, + Warn, + Panic, +} + /// A list of commands that will be run to modify an [entity](crate::entity). pub struct EntityCommands<'a> { pub(crate) entity: Entity, pub(crate) commands: Commands<'a, 'a>, + pub(crate) failure_mode: FailureMode, } impl<'a> EntityCommands<'a> { + pub fn ignore_if_missing(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Ignore; + self + } + + pub fn log_if_missing(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Log; + self + } + + pub fn warn_if_missing(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Warn; + self + } + + pub fn panic_if_missing(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Panic; + self + } + /// Returns the [`Entity`] id of the entity. /// /// # Example @@ -1066,6 +1098,7 @@ impl<'a> EntityCommands<'a> { EntityCommands { entity: self.entity, commands: self.commands.reborrow(), + failure_mode: FailureMode::Panic, } } @@ -1573,7 +1606,7 @@ 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(command.with_entity(self.entity)); + self.commands.queue(command.with_entity(self.entity, self.failure_mode)); self } @@ -1774,7 +1807,7 @@ impl EntityCommand for F where F: FnOnce(EntityWorldMut) + Send + 'static, { - fn apply(self, id: Entity, world: &mut World) { + fn apply(self, id: Entity, world: &mut World, _failure_mode: FailureMode) { self(world.entity_mut(id)); } } @@ -1783,8 +1816,17 @@ impl EntityCommand for F where F: FnOnce(Entity, &mut World) + Send + 'static, { - fn apply(self, id: Entity, world: &mut World) { - self(id, world); + fn apply(self, id: Entity, world: &mut World, failure_mode: FailureMode) { + if world.entities.contains(id) { + self(id, world); + } else { + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => (), + FailureMode::Warn => println!("WEEWOO"), + FailureMode::Panic => panic!("WEEWOOWEEWOO"), + } + } } } @@ -2214,6 +2256,28 @@ mod tests { assert_eq!("*****", &world.get::>(entity).unwrap().0); } + #[test] + fn entity_commands_failure() { + #[derive(Component)] + struct X; + + let mut world = World::default(); + + let mut queue_a = CommandQueue::default(); + let mut queue_b = CommandQueue::default(); + + let mut commands_a = Commands::new(&mut queue_a, &world); + let mut commands_b = Commands::new(&mut queue_b, &world); + + let entity = commands_a.spawn_empty().id(); + + commands_a.entity(entity).despawn(); + commands_b.entity(entity).warn_if_missing().insert(X); + + queue_a.apply(&mut world); + queue_b.apply(&mut world); + } + #[test] fn commands() { let mut world = World::default(); From 9096ee3646c3e46cf8c14058d2d0d4bcc8ede8ca Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Tue, 15 Oct 2024 19:13:57 -0500 Subject: [PATCH 02/29] add actual errors --- crates/bevy_ecs/src/system/commands/mod.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 2a242a5f9be4e..b2727e2f7eba0 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -20,7 +20,7 @@ use crate::{ }, }; use bevy_ptr::OwningPtr; -use bevy_utils::tracing::{error, info}; +use bevy_utils::tracing::{error, info, warn}; pub use parallel_scope::*; /// A [`Command`] queue to perform structural changes to the [`World`]. @@ -1820,11 +1820,12 @@ where if world.entities.contains(id) { self(id, world); } else { + let message = format!("Could not execute EntityCommand because its entity {id:?} was missing"); match failure_mode { FailureMode::Ignore => (), - FailureMode::Log => (), - FailureMode::Warn => println!("WEEWOO"), - FailureMode::Panic => panic!("WEEWOOWEEWOO"), + FailureMode::Log => info!("{}", message), + FailureMode::Warn => warn!("{}", message), + FailureMode::Panic => panic!("{}", message), } } } From ab35a4f3bee38ba75291d883c2572e1babdfbcea Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Tue, 15 Oct 2024 19:49:22 -0500 Subject: [PATCH 03/29] remove checks that can no longer trigger --- crates/bevy_ecs/src/system/commands/mod.rs | 95 ++++++++++------------ 1 file changed, 43 insertions(+), 52 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index b2727e2f7eba0..deedab143f6fe 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1606,7 +1606,8 @@ 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(command.with_entity(self.entity, self.failure_mode)); + self.commands + .queue(command.with_entity(self.entity, self.failure_mode)); self } @@ -1820,12 +1821,13 @@ where if world.entities.contains(id) { self(id, world); } else { - let message = format!("Could not execute EntityCommand because its entity {id:?} was missing"); + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); match failure_mode { FailureMode::Ignore => (), - FailureMode::Log => info!("{}", message), - FailureMode::Warn => warn!("{}", message), - FailureMode::Panic => panic!("{}", message), + FailureMode::Log => info!("{}", message), + FailureMode::Warn => warn!("{}", message), + FailureMode::Panic => panic!("{}", message), } } } @@ -2000,37 +2002,33 @@ fn try_despawn() -> impl EntityCommand { /// An [`EntityCommand`] that adds the components in a [`Bundle`] to an entity. #[track_caller] fn insert(bundle: T, mode: InsertMode) -> impl EntityCommand { + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.insert_with_caller( - bundle, - mode, - #[cfg(feature = "track_change_detection")] - caller, - ); - } else { - panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity); - } + let mut entity = world.entity_mut(entity); + entity.insert_with_caller( + bundle, + mode, + #[cfg(feature = "track_change_detection")] + caller, + ); } } /// An [`EntityCommand`] that adds the component using its `FromWorld` implementation. #[track_caller] fn insert_from_world(mode: InsertMode) -> impl EntityCommand { + #[cfg(feature = "track_change_detection")] let caller = Location::caller(); move |entity: Entity, world: &mut World| { let value = T::from_world(world); - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.insert_with_caller( - value, - mode, - #[cfg(feature = "track_change_detection")] - caller, - ); - } else { - panic!("error[B0003]: {caller}: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity); - } + let mut entity = world.entity_mut(entity); + entity.insert_with_caller( + value, + mode, + #[cfg(feature = "track_change_detection")] + caller, + ); } } @@ -2041,14 +2039,13 @@ fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand { #[cfg(feature = "track_change_detection")] let caller = Location::caller(); move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.insert_with_caller( - bundle, - mode, - #[cfg(feature = "track_change_detection")] - caller, - ); - } + let mut entity = world.entity_mut(entity); + entity.insert_with_caller( + bundle, + mode, + #[cfg(feature = "track_change_detection")] + caller, + ); } } @@ -2082,9 +2079,8 @@ unsafe fn insert_by_id( /// For a [`Bundle`] type `T`, this will remove any components in the bundle. /// Any components in the bundle that aren't found on the entity will be ignored. fn remove(entity: Entity, world: &mut World) { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove::(); - } + let mut entity = world.entity_mut(entity); + entity.remove::(); } /// An [`EntityCommand`] that removes components with a provided [`ComponentId`] from an entity. @@ -2093,25 +2089,22 @@ fn remove(entity: Entity, world: &mut World) { /// Panics if the provided [`ComponentId`] does not exist in the [`World`]. fn remove_by_id(component_id: ComponentId) -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove_by_id(component_id); - } + let mut entity = world.entity_mut(entity); + entity.remove_by_id(component_id); } } /// An [`EntityCommand`] that remove all components in the bundle and remove all required components for each component in the bundle. fn remove_with_requires(entity: Entity, world: &mut World) { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.remove_with_requires::(); - } + let mut entity = world.entity_mut(entity); + entity.remove_with_requires::(); } /// An [`EntityCommand`] that removes all components associated with a provided entity. fn clear() -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.clear(); - } + let mut entity = world.entity_mut(entity); + entity.clear(); } } @@ -2120,9 +2113,8 @@ fn clear() -> impl EntityCommand { /// For a [`Bundle`] type `T`, this will remove all components except those in the bundle. /// Any components in the bundle that aren't found on the entity will be ignored. fn retain(entity: Entity, world: &mut World) { - if let Ok(mut entity_mut) = world.get_entity_mut(entity) { - entity_mut.retain::(); - } + let mut entity = world.entity_mut(entity); + entity.retain::(); } /// A [`Command`] that inserts a [`Resource`] into the world using a value @@ -2165,9 +2157,8 @@ fn observe( observer: impl IntoObserverSystem, ) -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - entity.observe(observer); - } + let mut entity = world.entity_mut(entity); + entity.observe(observer); } } @@ -2271,7 +2262,7 @@ mod tests { let mut commands_b = Commands::new(&mut queue_b, &world); let entity = commands_a.spawn_empty().id(); - + commands_a.entity(entity).despawn(); commands_b.entity(entity).warn_if_missing().insert(X); From f36780d1d2a7ed6a2390c1b5ce4009e6379c0783 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Tue, 15 Oct 2024 21:30:41 -0500 Subject: [PATCH 04/29] implement for niche variant of EntityCommand --- crates/bevy_ecs/src/system/commands/mod.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index deedab143f6fe..1b589dbedd305 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1808,8 +1808,19 @@ impl EntityCommand for F where F: FnOnce(EntityWorldMut) + Send + 'static, { - fn apply(self, id: Entity, world: &mut World, _failure_mode: FailureMode) { - self(world.entity_mut(id)); + fn apply(self, id: Entity, world: &mut World, failure_mode: FailureMode) { + if world.entities.contains(id) { + self(world.entity_mut(id)); + } else { + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => info!("{}", message), + FailureMode::Warn => warn!("{}", message), + FailureMode::Panic => panic!("{}", message), + } + } } } From d54ff94ff865d74edc9bec243d37066bb905af2b Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 15:49:25 -0500 Subject: [PATCH 05/29] derive default, wire up `EntityEntryCommands` --- crates/bevy_ecs/src/system/commands/mod.rs | 29 +++++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 1b589dbedd305..deefb7d464601 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -308,7 +308,7 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), - failure_mode: FailureMode::Panic, + failure_mode: FailureMode::default(), } } @@ -335,7 +335,7 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), - failure_mode: FailureMode::Panic, + failure_mode: FailureMode::default(), } } @@ -487,7 +487,7 @@ impl<'w, 's> Commands<'w, 's> { self.entities.contains(entity).then_some(EntityCommands { entity, commands: self.reborrow(), - failure_mode: FailureMode::Panic, + failure_mode: FailureMode::default(), }) } @@ -1038,11 +1038,12 @@ pub trait EntityCommand: Send + 'static { } } -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Default)] pub enum FailureMode { Ignore, Log, Warn, + #[default] Panic, } @@ -1699,6 +1700,26 @@ pub struct EntityEntryCommands<'a, T> { } impl<'a, T: Component> EntityEntryCommands<'a, T> { + pub fn ignore_if_missing(&mut self) -> &mut Self { + self.entity_commands.ignore_if_missing(); + self + } + + pub fn log_if_missing(&mut self) -> &mut Self { + self.entity_commands.log_if_missing(); + self + } + + pub fn warn_if_missing(&mut self) -> &mut Self { + self.entity_commands.warn_if_missing(); + self + } + + pub fn panic_if_missing(&mut self) -> &mut Self { + self.entity_commands.panic_if_missing(); + self + } + /// Modify the component `T` if it exists, using the the function `modify`. pub fn and_modify(&mut self, modify: impl FnOnce(Mut) + Send + Sync + 'static) -> &mut Self { self.entity_commands From 60e570332eea0b2ea053d9d35c3a13b02552320d Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 17:39:34 -0500 Subject: [PATCH 06/29] move `failure_mode` to `Commands`, implement for fallible --- crates/bevy_ecs/src/system/commands/mod.rs | 83 ++++++++++++++-------- crates/bevy_ecs/src/world/mod.rs | 12 ++++ 2 files changed, 66 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index deefb7d464601..e28616ac057fd 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -16,7 +16,7 @@ use crate::{ system::{input::SystemInput, RunSystemWithInput, SystemId}, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, Command, CommandQueue, - EntityWorldMut, FromWorld, SpawnBatchIter, World, + EntityWorldMut, FailureMode, FromWorld, SpawnBatchIter, World, }, }; use bevy_ptr::OwningPtr; @@ -77,6 +77,7 @@ pub use parallel_scope::*; pub struct Commands<'w, 's> { queue: InternalQueue<'s>, entities: &'w Entities, + pub(crate) failure_mode: FailureMode, } // SAFETY: All commands [`Command`] implement [`Send`] @@ -172,6 +173,7 @@ const _: () = { Commands { queue: InternalQueue::CommandQueue(f0), entities: f1, + failure_mode: FailureMode::default(), } } } @@ -190,6 +192,26 @@ enum InternalQueue<'s> { } impl<'w, 's> Commands<'w, 's> { + pub fn ignore_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Ignore; + self + } + + pub fn log_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Log; + self + } + + pub fn warn_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Warn; + self + } + + pub fn panic_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Panic; + self + } + /// Returns a new `Commands` instance from a [`CommandQueue`] and a [`World`]. /// /// It is not required to call this constructor when using `Commands` as a [system parameter]. @@ -208,6 +230,7 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::CommandQueue(Deferred(queue)), entities, + failure_mode: FailureMode::default(), } } @@ -225,6 +248,7 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::RawCommandQueue(queue), entities, + failure_mode: FailureMode::default(), } } @@ -255,6 +279,7 @@ impl<'w, 's> Commands<'w, 's> { } }, entities: self.entities, + failure_mode: FailureMode::default(), } } @@ -308,7 +333,6 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), - failure_mode: FailureMode::default(), } } @@ -335,7 +359,6 @@ impl<'w, 's> Commands<'w, 's> { EntityCommands { entity, commands: self.reborrow(), - failure_mode: FailureMode::default(), } } @@ -441,7 +464,7 @@ impl<'w, 's> Commands<'w, 's> { #[track_caller] fn panic_no_entity(entity: Entity) -> ! { panic!( - "Attempting to create an EntityCommands for entity {entity:?}, which doesn't exist.", + "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist.", ); } @@ -484,11 +507,22 @@ impl<'w, 's> Commands<'w, 's> { #[inline] #[track_caller] pub fn get_entity(&mut self, entity: Entity) -> Option { - self.entities.contains(entity).then_some(EntityCommands { - entity, - commands: self.reborrow(), - failure_mode: FailureMode::default(), - }) + if self.entities.contains(entity) { + Some(EntityCommands { + entity, + commands: self.reborrow(), + }) + } else { + let message = + format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist."); + match self.failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => info!("{}", message), + FailureMode::Warn => warn!("{}", message), + FailureMode::Panic => panic!("{}", message), + }; + None + } } /// Pushes a [`Command`] to the queue for creating entities with a particular [`Bundle`] type. @@ -639,7 +673,7 @@ impl<'w, 's> Commands<'w, 's> { I: IntoIterator + Send + Sync + 'static, B: Bundle, { - self.queue(insert_batch(batch)); + self.queue(insert_batch(batch, self.failure_mode)); } /// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity). @@ -666,7 +700,7 @@ impl<'w, 's> Commands<'w, 's> { I: IntoIterator + Send + Sync + 'static, B: Bundle, { - self.queue(insert_batch_if_new(batch)); + self.queue(insert_batch_if_new(batch, self.failure_mode)); } /// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity). @@ -1038,40 +1072,30 @@ pub trait EntityCommand: Send + 'static { } } -#[derive(Clone, Copy, Default)] -pub enum FailureMode { - Ignore, - Log, - Warn, - #[default] - Panic, -} - /// A list of commands that will be run to modify an [entity](crate::entity). pub struct EntityCommands<'a> { pub(crate) entity: Entity, pub(crate) commands: Commands<'a, 'a>, - pub(crate) failure_mode: FailureMode, } impl<'a> EntityCommands<'a> { pub fn ignore_if_missing(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Ignore; + self.commands.ignore_on_error(); self } pub fn log_if_missing(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Log; + self.commands.log_on_error(); self } pub fn warn_if_missing(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Warn; + self.commands.warn_on_error(); self } pub fn panic_if_missing(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Panic; + self.commands.panic_on_error(); self } @@ -1099,7 +1123,6 @@ impl<'a> EntityCommands<'a> { EntityCommands { entity: self.entity, commands: self.commands.reborrow(), - failure_mode: FailureMode::Panic, } } @@ -1608,7 +1631,7 @@ impl<'a> EntityCommands<'a> { /// ``` pub fn queue(&mut self, command: impl EntityCommand) -> &mut Self { self.commands - .queue(command.with_entity(self.entity, self.failure_mode)); + .queue(command.with_entity(self.entity, self.commands.failure_mode)); self } @@ -1918,7 +1941,7 @@ where /// /// This is more efficient than inserting the bundles individually. #[track_caller] -fn insert_batch(batch: I) -> impl Command +fn insert_batch(batch: I, failure_mode: FailureMode) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, @@ -1929,6 +1952,7 @@ where world.insert_batch_with_caller( batch, InsertMode::Replace, + failure_mode, #[cfg(feature = "track_change_detection")] caller, ); @@ -1940,7 +1964,7 @@ where /// /// This is more efficient than inserting the bundles individually. #[track_caller] -fn insert_batch_if_new(batch: I) -> impl Command +fn insert_batch_if_new(batch: I, failure_mode: FailureMode) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, @@ -1951,6 +1975,7 @@ where world.insert_batch_with_caller( batch, InsertMode::Keep, + failure_mode, #[cfg(feature = "track_change_detection")] caller, ); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 013b72dc7dea5..f58df1f0da53c 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2491,6 +2491,7 @@ impl World { self.insert_batch_with_caller( batch, InsertMode::Replace, + FailureMode::Panic, #[cfg(feature = "track_change_detection")] Location::caller(), ); @@ -2521,6 +2522,7 @@ impl World { self.insert_batch_with_caller( batch, InsertMode::Keep, + FailureMode::Panic, #[cfg(feature = "track_change_detection")] Location::caller(), ); @@ -2538,6 +2540,7 @@ impl World { &mut self, iter: I, insert_mode: InsertMode, + failure_mode: FailureMode, #[cfg(feature = "track_change_detection")] caller: &'static Location, ) where I: IntoIterator, @@ -3786,6 +3789,15 @@ impl FromWorld for T { } } +#[derive(Default, Clone, Copy, PartialEq, Eq)] +pub enum FailureMode { + Ignore, + #[default] + Log, + Warn, + Panic, +} + #[cfg(test)] mod tests { use super::{FromWorld, World}; From 586da9535811f273437383b278dc47b3378b9a5d Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 17:45:55 -0500 Subject: [PATCH 07/29] maintain `failure_mode` when reborrowing --- 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 e28616ac057fd..19e9d36fdb75c 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -279,7 +279,7 @@ impl<'w, 's> Commands<'w, 's> { } }, entities: self.entities, - failure_mode: FailureMode::default(), + failure_mode: self.failure_mode, } } From 36503477931b7f32ab6e045c63586196fa9f2142 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 18:18:10 -0500 Subject: [PATCH 08/29] allow `Commands::entity` to return invalid `EntityCommands` --- crates/bevy_ecs/src/system/commands/mod.rs | 25 +++++++++++----------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 19e9d36fdb75c..374f0c118be74 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -459,18 +459,19 @@ impl<'w, 's> Commands<'w, 's> { #[inline] #[track_caller] pub fn entity(&mut self, entity: Entity) -> EntityCommands { - #[inline(never)] - #[cold] - #[track_caller] - fn panic_no_entity(entity: Entity) -> ! { - panic!( - "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist.", - ); + if !self.entities.contains(entity) { + let message = + format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist"); + match self.failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => info!("{}; returned invalid EntityCommands", message), + FailureMode::Warn => warn!("{}; returned invalid EntityCommands", message), + FailureMode::Panic => panic!("{}", message), + }; } - - match self.get_entity(entity) { - Some(entity) => entity, - None => panic_no_entity(entity), + EntityCommands { + entity, + commands: self.reborrow(), } } @@ -514,7 +515,7 @@ impl<'w, 's> Commands<'w, 's> { }) } else { let message = - format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist."); + format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist"); match self.failure_mode { FailureMode::Ignore => (), FailureMode::Log => info!("{}", message), From 40b6b03d265d6b5c0519848efe7071278637ef2c Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 18:45:44 -0500 Subject: [PATCH 09/29] added convenience method also handled failure properly in `insert_batch_with_caller` --- crates/bevy_ecs/src/system/commands/mod.rs | 41 ++++++++-------------- crates/bevy_ecs/src/world/mod.rs | 27 ++++++++++++-- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 374f0c118be74..f3ce5e46985f0 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -460,8 +460,9 @@ impl<'w, 's> Commands<'w, 's> { #[track_caller] pub fn entity(&mut self, entity: Entity) -> EntityCommands { if !self.entities.contains(entity) { - let message = - format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist"); + let message = format!( + "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist" + ); match self.failure_mode { FailureMode::Ignore => (), FailureMode::Log => info!("{}; returned invalid EntityCommands", message), @@ -514,14 +515,10 @@ impl<'w, 's> Commands<'w, 's> { commands: self.reborrow(), }) } else { - let message = - format!("Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist"); - match self.failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => info!("{}", message), - FailureMode::Warn => warn!("{}", message), - FailureMode::Panic => panic!("{}", message), - }; + let message = format!( + "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist" + ); + self.failure_mode.fail(message); None } } @@ -1857,14 +1854,10 @@ where if world.entities.contains(id) { self(world.entity_mut(id)); } else { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => info!("{}", message), - FailureMode::Warn => warn!("{}", message), - FailureMode::Panic => panic!("{}", message), - } + let message = format!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ); + failure_mode.fail(message); } } } @@ -1877,14 +1870,10 @@ where if world.entities.contains(id) { self(id, world); } else { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => info!("{}", message), - FailureMode::Warn => warn!("{}", message), - FailureMode::Panic => panic!("{}", message), - } + let message = format!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ); + failure_mode.fail(message); } } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index f58df1f0da53c..d929e405a7e1b 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -51,7 +51,7 @@ use crate::{ }, }; use bevy_ptr::{OwningPtr, Ptr}; -use bevy_utils::tracing::warn; +use bevy_utils::tracing::{info, warn}; use core::{ any::TypeId, fmt, @@ -2616,11 +2616,21 @@ impl World { ) }; } else { - panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity); + let message = format!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + core::any::type_name::(), + entity, + ); + failure_mode.fail(message); } } } else { - panic!("error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), first_entity); + let message = format!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + core::any::type_name::(), + first_entity, + ); + failure_mode.fail(message); } } } @@ -3798,6 +3808,17 @@ pub enum FailureMode { Panic, } +impl FailureMode { + pub fn fail(&self, message: String) { + match self { + Self::Ignore => (), + Self::Log => info!("{}", message), + Self::Warn => warn!("{}", message), + Self::Panic => panic!("{}", message), + }; + } +} + #[cfg(test)] mod tests { use super::{FromWorld, World}; From 3ac66ecd3156b0210f20b831d7cacba2392d7fa5 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 22:28:50 -0500 Subject: [PATCH 10/29] add docs and update existing docs --- crates/bevy_ecs/src/system/commands/mod.rs | 312 +++++++++++---------- crates/bevy_ecs/src/world/mod.rs | 8 + 2 files changed, 176 insertions(+), 144 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index f3ce5e46985f0..230b2325fb4ff 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -192,26 +192,6 @@ enum InternalQueue<'s> { } impl<'w, 's> Commands<'w, 's> { - pub fn ignore_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Ignore; - self - } - - pub fn log_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Log; - self - } - - pub fn warn_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Warn; - self - } - - pub fn panic_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Panic; - self - } - /// Returns a new `Commands` instance from a [`CommandQueue`] and a [`World`]. /// /// It is not required to call this constructor when using `Commands` as a [system parameter]. @@ -294,6 +274,58 @@ impl<'w, 's> Commands<'w, 's> { } } + /// Sets the [`Commands`] instance to ignore errors. + /// + /// Any subsequent commands that can fail will do so silently. + /// + /// # See also: + /// - [`log_on_error`](Self::log_on_error) to log errors. + /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. + /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + pub fn ignore_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Ignore; + self + } + + /// Sets the [`Commands`] instance to log errors. + /// + /// Any subsequent commands that can fail will log each failure. + /// + /// # See also: + /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. + /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. + /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + pub fn log_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Log; + self + } + + /// Sets the [`Commands`] instance to send warnings upon encountering errors. + /// + /// Any subsequent commands that can fail will warn for each failure. + /// + /// # See also: + /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. + /// - [`log_on_error`](Self::log_on_error) to log errors. + /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + pub fn warn_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Warn; + self + } + + /// Sets the [`Commands`] instance to panic upon encountering an error. + /// + /// Any subsequent commands that can fail will panic if they do. + /// + /// # See also: + /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. + /// - [`log_on_error`](Self::log_on_error) to log errors. + /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. + pub fn panic_on_error(&mut self) -> &mut Self { + self.failure_mode = FailureMode::Panic; + self + } + /// Reserves a new empty [`Entity`] to be spawned, and returns its corresponding [`EntityCommands`]. /// /// See [`World::spawn_empty`] for more details. @@ -424,9 +456,8 @@ impl<'w, 's> Commands<'w, 's> { /// Returns the [`EntityCommands`] for the requested [`Entity`]. /// - /// # Panics - /// - /// This method panics if the requested entity does not exist. + /// This method does not guarantee that `EntityCommands` will be successfully applied, + /// since another command in the queue may delete the entity before them. /// /// # Example /// @@ -505,7 +536,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// # See also /// - /// - [`entity`](Self::entity) for the panicking version. + /// - [`entity`](Self::entity) for the unwrapped version. #[inline] #[track_caller] pub fn get_entity(&mut self, entity: Entity) -> Option { @@ -659,12 +690,6 @@ impl<'w, 's> Commands<'w, 's> { /// calling [`entity`](Self::entity) for each pair, /// and passing the bundle to [`insert`](EntityCommands::insert), /// but it is faster due to memory pre-allocation. - /// - /// # Panics - /// - /// This command panics if any of the given entities do not exist. - /// - /// For the non-panicking version, see [`try_insert_batch`](Self::try_insert_batch). #[track_caller] pub fn insert_batch(&mut self, batch: I) where @@ -686,12 +711,6 @@ impl<'w, 's> Commands<'w, 's> { /// calling [`entity`](Self::entity) for each pair, /// and passing the bundle to [`insert_if_new`](EntityCommands::insert_if_new), /// but it is faster due to memory pre-allocation. - /// - /// # Panics - /// - /// This command panics if any of the given entities do not exist. - /// - /// For the non-panicking version, see [`try_insert_batch_if_new`](Self::try_insert_batch_if_new). #[track_caller] pub fn insert_batch_if_new(&mut self, batch: I) where @@ -716,7 +735,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// This command silently fails by ignoring any entities that do not exist. /// - /// For the panicking version, see [`insert_batch`](Self::insert_batch). + /// For the customizable version, see [`insert_batch`](Self::insert_batch). #[track_caller] pub fn try_insert_batch(&mut self, batch: I) where @@ -741,7 +760,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// This command silently fails by ignoring any entities that do not exist. /// - /// For the panicking version, see [`insert_batch_if_new`](Self::insert_batch_if_new). + /// For the customizable version, see [`insert_batch_if_new`](Self::insert_batch_if_new). #[track_caller] pub fn try_insert_batch_if_new(&mut self, batch: I) where @@ -1070,33 +1089,27 @@ pub trait EntityCommand: Send + 'static { } } -/// A list of commands that will be run to modify an [entity](crate::entity). +/// A list of commands that will be run to modify an [`Entity`]. +/// +/// Most [`Commands`] (and thereby [`EntityCommands`]) are deferred: when you call the command, +/// if it requires mutable access to the [`World`] (that is, if it removes, adds, or changes something), +/// it's not executed immediately. Instead, the command is added to a "command queue." +/// The command queue is applied between [`Schedules`](bevy_ecs::schedule::Schedule), one by one, +/// so that each command can have exclusive access to the World. +/// +/// Due to their deferred nature, an entity you're trying change with an `EntityCommand` can be +/// despawned by the time the command is executed. Use the following commands to set how you +/// would like the command to response if the entity is missing: +/// - [`ignore_if_missing`](Self::ignore_if_missing) +/// - [`log_if_missing`](Self::log_if_missing) (default) +/// - [`warn_if_missing`](Self::warn_if_missing) +/// - [`panic_if_missing`](Self::panic_if_missing) pub struct EntityCommands<'a> { pub(crate) entity: Entity, pub(crate) commands: Commands<'a, 'a>, } impl<'a> EntityCommands<'a> { - pub fn ignore_if_missing(&mut self) -> &mut Self { - self.commands.ignore_on_error(); - self - } - - pub fn log_if_missing(&mut self) -> &mut Self { - self.commands.log_on_error(); - self - } - - pub fn warn_if_missing(&mut self) -> &mut Self { - self.commands.warn_on_error(); - self - } - - pub fn panic_if_missing(&mut self) -> &mut Self { - self.commands.panic_on_error(); - self - } - /// Returns the [`Entity`] id of the entity. /// /// # Example @@ -1124,6 +1137,52 @@ impl<'a> EntityCommands<'a> { } } + /// Sets the [`EntityCommands`] instance to ignore commands if the entity doesn't exist. + /// + /// # See also: + /// - [`log_if_missing`](Self::log_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) + pub fn ignore_if_missing(&mut self) -> &mut Self { + self.commands.ignore_on_error(); + self + } + + /// Sets the [`EntityCommands`] instance to log if the entity doesn't exist when a command is executed. + /// + /// This is the default setting. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) + pub fn log_if_missing(&mut self) -> &mut Self { + self.commands.log_on_error(); + self + } + + /// Sets the [`EntityCommands`] instance to warn if the entity doesn't exist when a command is executed. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) + pub fn warn_if_missing(&mut self) -> &mut Self { + self.commands.warn_on_error(); + self + } + + /// Sets the [`EntityCommands`] instance to panic if the entity doesn't exist when a command is executed. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) + pub fn panic_if_missing(&mut self) -> &mut Self { + self.commands.panic_on_error(); + self + } + /// Get an [`EntityEntryCommands`] for the [`Component`] `T`, /// allowing you to modify it or insert it if it isn't already present. /// @@ -1161,12 +1220,6 @@ impl<'a> EntityCommands<'a> { /// This will overwrite any previous value(s) of the same component type. /// See [`EntityCommands::insert_if_new`] to keep the old value instead. /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not exist. - /// - /// To avoid a panic in this case, use the command [`Self::try_insert`] instead. - /// /// # Example /// /// ``` @@ -1216,12 +1269,6 @@ impl<'a> EntityCommands<'a> { /// Similar to [`Self::insert`] but will only insert if the predicate returns true. /// This is useful for chaining method calls. /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not exist. - /// - /// To avoid a panic in this case, use the command [`Self::try_insert_if`] instead. - /// /// # Example /// /// ``` @@ -1262,12 +1309,6 @@ impl<'a> EntityCommands<'a> { /// /// See also [`entry`](Self::entry), which lets you modify a [`Component`] if it's present, /// as well as initialize it with a default value. - /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not exist. - /// - /// To avoid a panic in this case, use the command [`Self::try_insert_if_new`] instead. pub fn insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { self.queue(insert(bundle, InsertMode::Keep)) } @@ -1278,14 +1319,6 @@ impl<'a> EntityCommands<'a> { /// This is the same as [`EntityCommands::insert_if`], but in case of duplicate /// components will leave the old values instead of replacing them with new /// ones. - /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not - /// exist. - /// - /// To avoid a panic in this case, use the command [`Self::try_insert_if_new`] - /// instead. pub fn insert_if_new_and(&mut self, bundle: impl Bundle, condition: F) -> &mut Self where F: FnOnce() -> bool, @@ -1301,12 +1334,6 @@ impl<'a> EntityCommands<'a> { /// /// See [`EntityWorldMut::insert_by_id`] for more information. /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not exist. - /// - /// To avoid a panic in this case, use the command [`Self::try_insert_by_id`] instead. - /// /// # Safety /// /// - [`ComponentId`] must be from the same world as `self`. @@ -1317,11 +1344,8 @@ impl<'a> EntityCommands<'a> { component_id: ComponentId, value: T, ) -> &mut Self { - let caller = Location::caller(); // SAFETY: same invariants as parent call - self.queue(unsafe {insert_by_id(component_id, value, move |entity| { - panic!("error[B0003]: {caller}: Could not insert a component {component_id:?} (with type {}) for entity {entity:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::()); - })}) + self.queue(unsafe { insert_by_id(component_id, value) }) } /// Attempts to add a dynamic component to an entity. @@ -1338,7 +1362,7 @@ impl<'a> EntityCommands<'a> { value: T, ) -> &mut Self { // SAFETY: same invariants as parent call - self.queue(unsafe { insert_by_id(component_id, value, |_| {}) }) + self.queue(unsafe { insert_by_id(component_id, value) }) } /// Tries to add a [`Bundle`] of components to the entity. @@ -1347,7 +1371,8 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// Unlike [`Self::insert`], this will not panic if the associated entity does not exist. + /// [`Self::insert`] used to panic if the entity was missing, and this was the non-panicking version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`insert`] /// /// # Example /// @@ -1441,8 +1466,8 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// Unlike [`Self::insert_if_new_and`], this will not panic if the associated entity does - /// not exist. + /// [`Self::insert_if_new_and`] used to panic if the entity was missing, and this was the non-panicking version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new_and`] /// /// # Example /// @@ -1486,7 +1511,8 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// Unlike [`Self::insert_if_new`], this will not panic if the associated entity does not exist. + /// [`Self::insert_if_new`] used to panic if the entity was missing, and this was the non-panicking version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new`] pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { self.queue(try_insert(bundle, InsertMode::Keep)) } @@ -1572,7 +1598,6 @@ impl<'a> EntityCommands<'a> { } /// Despawns the entity. - /// This will emit a warning if the entity does not exist. /// /// See [`World::despawn`] for more details. /// @@ -1604,8 +1629,9 @@ impl<'a> EntityCommands<'a> { } /// 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. + /// + /// [`Self::despawn`] used to warn if the entity was missing, and this was the silent version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::despawn`] #[track_caller] pub fn try_despawn(&mut self) { self.queue(try_despawn()); @@ -1678,10 +1704,6 @@ impl<'a> EntityCommands<'a> { } /// Logs the components of the entity at the info level. - /// - /// # Panics - /// - /// The command will panic when applied if the associated entity does not exist. pub fn log_components(&mut self) -> &mut Self { self.queue(log_components) } @@ -1721,21 +1743,45 @@ pub struct EntityEntryCommands<'a, T> { } impl<'a, T: Component> EntityEntryCommands<'a, T> { + /// Sets the [`EntityEntryCommands`] instance to ignore commands if the entity doesn't exist. + /// + /// # See also: + /// - [`log_if_missing`](Self::log_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) pub fn ignore_if_missing(&mut self) -> &mut Self { self.entity_commands.ignore_if_missing(); self } + /// Sets the [`EntityEntryCommands`] instance to log if the entity doesn't exist when a command is executed. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) pub fn log_if_missing(&mut self) -> &mut Self { self.entity_commands.log_if_missing(); self } + /// Sets the [`EntityEntryCommands`] instance to warn if the entity doesn't exist when a command is executed. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) pub fn warn_if_missing(&mut self) -> &mut Self { self.entity_commands.warn_if_missing(); self } + /// Sets the [`EntityEntryCommands`] instance to panic if the entity doesn't exist when a command is executed. + /// + /// # See also: + /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.entity_commands.panic_if_missing(); self @@ -1755,11 +1801,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `default` into this entity, if `T` is not already present. /// /// See also [`or_insert_with`](Self::or_insert_with). - /// - /// # Panics - /// - /// Panics if the entity does not exist. - /// See [`or_try_insert`](Self::or_try_insert) for a non-panicking version. #[track_caller] pub fn or_insert(&mut self, default: T) -> &mut Self { self.entity_commands @@ -1769,7 +1810,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `default` into this entity, if `T` is not already present. /// - /// Unlike [`or_insert`](Self::or_insert), this will not panic if the entity does not exist. + /// [`Self::or_insert`] used to panic if the entity was missing, and this was the non-panicking version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert`] /// /// See also [`or_insert_with`](Self::or_insert_with). #[track_caller] @@ -1782,11 +1824,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) the value returned from `default` into this entity, if `T` is not already present. /// /// See also [`or_insert`](Self::or_insert) and [`or_try_insert`](Self::or_try_insert). - /// - /// # Panics - /// - /// Panics if the entity does not exist. - /// See [`or_try_insert_with`](Self::or_try_insert_with) for a non-panicking version. #[track_caller] pub fn or_insert_with(&mut self, default: impl Fn() -> T) -> &mut Self { self.or_insert(default()) @@ -1794,7 +1831,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) the value returned from `default` into this entity, if `T` is not already present. /// - /// Unlike [`or_insert_with`](Self::or_insert_with), this will not panic if the entity does not exist. + /// [`Self::or_insert_with`] used to panic if the entity was missing, and this was the non-panicking version. + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert_with`] /// /// See also [`or_insert`](Self::or_insert) and [`or_try_insert`](Self::or_try_insert). #[track_caller] @@ -1805,10 +1843,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `T::default` into this entity, if `T` is not already present. /// /// See also [`or_insert`](Self::or_insert) and [`or_from_world`](Self::or_from_world). - /// - /// # Panics - /// - /// Panics if the entity does not exist. #[track_caller] pub fn or_default(&mut self) -> &mut Self where @@ -1822,10 +1856,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `T::from_world` into this entity, if `T` is not already present. /// /// See also [`or_insert`](Self::or_insert) and [`or_default`](Self::or_default). - /// - /// # Panics - /// - /// Panics if the entity does not exist. #[track_caller] pub fn or_from_world(&mut self) -> &mut Self where @@ -1854,9 +1884,8 @@ where if world.entities.contains(id) { self(world.entity_mut(id)); } else { - let message = format!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ); + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); failure_mode.fail(message); } } @@ -1870,9 +1899,8 @@ where if world.entities.contains(id) { self(id, world); } else { - let message = format!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ); + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); failure_mode.fail(message); } } @@ -2105,19 +2133,15 @@ fn try_insert(bundle: impl Bundle, mode: InsertMode) -> impl EntityCommand { unsafe fn insert_by_id( component_id: ComponentId, value: T, - on_none_entity: impl FnOnce(Entity) + Send + 'static, ) -> impl EntityCommand { move |entity: Entity, world: &mut World| { - if let Ok(mut entity) = world.get_entity_mut(entity) { - // SAFETY: - // - `component_id` safety is ensured by the caller - // - `ptr` is valid within the `make` block; - OwningPtr::make(value, |ptr| unsafe { - entity.insert_by_id(component_id, ptr); - }); - } else { - on_none_entity(entity); - } + let mut entity = world.entity_mut(entity); + // SAFETY: + // - `component_id` safety is ensured by the caller + // - `ptr` is valid within the `make` block; + OwningPtr::make(value, |ptr| unsafe { + entity.insert_by_id(component_id, ptr); + }); } } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index d929e405a7e1b..0a749d8629866 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3799,16 +3799,24 @@ impl FromWorld for T { } } +/// How to respond if a function fails #[derive(Default, Clone, Copy, PartialEq, Eq)] pub enum FailureMode { + /// Do nothing Ignore, + /// Send a benign message to the log #[default] Log, + /// Send a more serious message to the log Warn, + /// Stop the application Panic, } impl FailureMode { + /// Convenience method to send a message upon failure. + /// If you want to send different messages for different modes, + /// match the enum manually. pub fn fail(&self, message: String) { match self { Self::Ignore => (), From 2f4f8a100b71e2506f2ecd39592c9846c1610dda Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 22:42:03 -0500 Subject: [PATCH 11/29] doc tidbits --- crates/bevy_ecs/src/system/commands/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 230b2325fb4ff..bdfb804b9b791 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -291,6 +291,8 @@ impl<'w, 's> Commands<'w, 's> { /// /// Any subsequent commands that can fail will log each failure. /// + /// This is the default setting. + /// /// # See also: /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. @@ -1099,7 +1101,7 @@ pub trait EntityCommand: Send + 'static { /// /// Due to their deferred nature, an entity you're trying change with an `EntityCommand` can be /// despawned by the time the command is executed. Use the following commands to set how you -/// would like the command to response if the entity is missing: +/// would like subsequent commands to response if the entity is missing: /// - [`ignore_if_missing`](Self::ignore_if_missing) /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) @@ -1165,7 +1167,7 @@ impl<'a> EntityCommands<'a> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn warn_if_missing(&mut self) -> &mut Self { self.commands.warn_on_error(); @@ -1176,8 +1178,8 @@ impl<'a> EntityCommands<'a> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.commands.panic_on_error(); self @@ -1372,7 +1374,7 @@ impl<'a> EntityCommands<'a> { /// # Note /// /// [`Self::insert`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`insert`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert`] /// /// # Example /// @@ -1756,6 +1758,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Sets the [`EntityEntryCommands`] instance to log if the entity doesn't exist when a command is executed. /// + /// This is the default setting. + /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) @@ -1769,7 +1773,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`warn_if_missing`](Self::warn_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn warn_if_missing(&mut self) -> &mut Self { self.entity_commands.warn_if_missing(); @@ -1780,8 +1784,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.entity_commands.panic_if_missing(); self From ba826f27b4f8dd65f7cb4991e9f0cdec2a60497a Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 23:38:29 -0500 Subject: [PATCH 12/29] more docs and fix get_entity --- crates/bevy_ecs/src/system/commands/mod.rs | 46 ++++++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index bdfb804b9b791..6298bbded1c4a 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -461,6 +461,11 @@ impl<'w, 's> Commands<'w, 's> { /// This method does not guarantee that `EntityCommands` will be successfully applied, /// since another command in the queue may delete the entity before them. /// + /// # Fallible + /// This command can fail if the entity does not exist. If the [`Commands`] instance + /// is not set to [`panic`] on failure, this command will return an invalid `EntityCommands`, + /// which will do nothing except [`warn`]/[`log`]/[`ignore`] according to the `Commands` current setting. + /// /// # Example /// /// ``` @@ -488,7 +493,12 @@ impl<'w, 's> Commands<'w, 's> { /// /// # See also /// - /// - [`get_entity`](Self::get_entity) for the fallible version. + /// - [`get_entity`](Self::get_entity) for the [`Option`] version. + /// + /// [`panic`]: Self::panic_on_error + /// [`warn`]: Self::warn_on_error + /// [`log`]: Self::log_on_error + /// [`ignore`]: Self::ignore_on_error #[inline] #[track_caller] pub fn entity(&mut self, entity: Entity) -> EntityCommands { @@ -542,18 +552,10 @@ impl<'w, 's> Commands<'w, 's> { #[inline] #[track_caller] pub fn get_entity(&mut self, entity: Entity) -> Option { - if self.entities.contains(entity) { - Some(EntityCommands { - entity, - commands: self.reborrow(), - }) - } else { - let message = format!( - "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist" - ); - self.failure_mode.fail(message); - None - } + self.entities.contains(entity).then_some(EntityCommands { + entity, + commands: self.reborrow(), + }) } /// Pushes a [`Command`] to the queue for creating entities with a particular [`Bundle`] type. @@ -692,6 +694,15 @@ impl<'w, 's> Commands<'w, 's> { /// calling [`entity`](Self::entity) for each pair, /// and passing the bundle to [`insert`](EntityCommands::insert), /// but it is faster due to memory pre-allocation. + /// + /// # Fallible + /// This command can fail if any of the entities do not exist. It will [`panic`]/[`warn`]/[`log`]/[`ignore`] + /// according the `Commands` instance's current setting. + /// + /// [`panic`]: Self::panic_on_error + /// [`warn`]: Self::warn_on_error + /// [`log`]: Self::log_on_error + /// [`ignore`]: Self::ignore_on_error #[track_caller] pub fn insert_batch(&mut self, batch: I) where @@ -713,6 +724,15 @@ impl<'w, 's> Commands<'w, 's> { /// calling [`entity`](Self::entity) for each pair, /// and passing the bundle to [`insert_if_new`](EntityCommands::insert_if_new), /// but it is faster due to memory pre-allocation. + /// + /// # Fallible + /// This command can fail if any of the entities do not exist. It will [`panic`]/[`warn`]/[`log`]/[`ignore`] + /// according the `Commands` instance's current setting. + /// + /// [`panic`]: Self::panic_on_error + /// [`warn`]: Self::warn_on_error + /// [`log`]: Self::log_on_error + /// [`ignore`]: Self::ignore_on_error #[track_caller] pub fn insert_batch_if_new(&mut self, batch: I) where From dd4d4e9dd1a6a65c240698e116e5f0bea87c4ab2 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 16 Oct 2024 23:51:19 -0500 Subject: [PATCH 13/29] minor spelling mistake --- 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 6298bbded1c4a..7d3c4015c2fd9 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1121,7 +1121,7 @@ pub trait EntityCommand: Send + 'static { /// /// Due to their deferred nature, an entity you're trying change with an `EntityCommand` can be /// despawned by the time the command is executed. Use the following commands to set how you -/// would like subsequent commands to response if the entity is missing: +/// would like subsequent commands to respond if the entity is missing: /// - [`ignore_if_missing`](Self::ignore_if_missing) /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) From e64db6d195cbd8f994951347ebeaadc9b65a8620 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Thu, 17 Oct 2024 00:17:43 -0500 Subject: [PATCH 14/29] a smidgen of docs --- crates/bevy_ecs/src/system/commands/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 7d3c4015c2fd9..124e0dd78320e 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1979,7 +1979,7 @@ where } /// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities. -/// If any entities do not exist in the world, this command will panic. +/// If any entities do not exist in the world, this command will fail according to `failure_mode`. /// /// This is more efficient than inserting the bundles individually. #[track_caller] @@ -2002,7 +2002,7 @@ where } /// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities. -/// If any entities do not exist in the world, this command will panic. +/// If any entities do not exist in the world, this command will fail according to `failure_mode`. /// /// This is more efficient than inserting the bundles individually. #[track_caller] @@ -2069,7 +2069,6 @@ where } /// A [`Command`] that despawns a specific entity. -/// This will emit a warning if the entity does not exist. /// /// # Note /// @@ -2084,7 +2083,6 @@ fn despawn() -> impl EntityCommand { } /// A [`Command`] that despawns a specific entity. -/// This will not emit a warning if the entity does not exist. /// /// # Note /// From c1840147e943529b6cfc76449ee88b7aac80317c Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Thu, 17 Oct 2024 18:19:12 -0500 Subject: [PATCH 15/29] a little more docs --- crates/bevy_ecs/src/system/commands/mod.rs | 64 ++++++++++++++-------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 124e0dd78320e..5a5769bc10bb3 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -279,9 +279,9 @@ impl<'w, 's> Commands<'w, 's> { /// Any subsequent commands that can fail will do so silently. /// /// # See also: - /// - [`log_on_error`](Self::log_on_error) to log errors. - /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. - /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + /// - [`log_on_error`](Self::log_on_error) (default) + /// - [`warn_on_error`](Self::warn_on_error) + /// - [`panic_on_error`](Self::panic_on_error) pub fn ignore_on_error(&mut self) -> &mut Self { self.failure_mode = FailureMode::Ignore; self @@ -294,9 +294,9 @@ impl<'w, 's> Commands<'w, 's> { /// This is the default setting. /// /// # See also: - /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. - /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. - /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + /// - [`ignore_on_error`](Self::ignore_on_error) + /// - [`warn_on_error`](Self::warn_on_error) + /// - [`panic_on_error`](Self::panic_on_error) pub fn log_on_error(&mut self) -> &mut Self { self.failure_mode = FailureMode::Log; self @@ -307,9 +307,9 @@ impl<'w, 's> Commands<'w, 's> { /// Any subsequent commands that can fail will warn for each failure. /// /// # See also: - /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. - /// - [`log_on_error`](Self::log_on_error) to log errors. - /// - [`panic_on_error`](Self::panic_on_error) to panic upon failure. + /// - [`ignore_on_error`](Self::ignore_on_error) + /// - [`log_on_error`](Self::log_on_error) (default) + /// - [`panic_on_error`](Self::panic_on_error) pub fn warn_on_error(&mut self) -> &mut Self { self.failure_mode = FailureMode::Warn; self @@ -320,9 +320,9 @@ impl<'w, 's> Commands<'w, 's> { /// Any subsequent commands that can fail will panic if they do. /// /// # See also: - /// - [`ignore_on_error`](Self::ignore_on_error) to ignore errors. - /// - [`log_on_error`](Self::log_on_error) to log errors. - /// - [`warn_on_error`](Self::warn_on_error) to send warnings for errors. + /// - [`ignore_on_error`](Self::ignore_on_error) + /// - [`log_on_error`](Self::log_on_error) (default) + /// - [`warn_on_error`](Self::warn_on_error) pub fn panic_on_error(&mut self) -> &mut Self { self.failure_mode = FailureMode::Panic; self @@ -462,9 +462,13 @@ impl<'w, 's> Commands<'w, 's> { /// since another command in the queue may delete the entity before them. /// /// # Fallible + /// /// This command can fail if the entity does not exist. If the [`Commands`] instance /// is not set to [`panic`] on failure, this command will return an invalid `EntityCommands`, - /// which will do nothing except [`warn`]/[`log`]/[`ignore`] according to the `Commands` current setting. + /// which will do nothing except [`warn`]/[`log`]/[`ignore`] the entity's non-existence + /// according to the instance's current setting. + /// + /// To get an [`Option`] instead, see [`get_entity`](Self::get_entity). /// /// # Example /// @@ -491,10 +495,6 @@ impl<'w, 's> Commands<'w, 's> { /// # bevy_ecs::system::assert_is_system(example_system); /// ``` /// - /// # See also - /// - /// - [`get_entity`](Self::get_entity) for the [`Option`] version. - /// /// [`panic`]: Self::panic_on_error /// [`warn`]: Self::warn_on_error /// [`log`]: Self::log_on_error @@ -1092,6 +1092,24 @@ impl<'w, 's> Commands<'w, 's> { /// assert_eq!(names, HashSet::from_iter(["Entity #0", "Entity #1"])); /// } /// ``` +/// +/// # Implementing +/// +/// This trait is blanket-implemented for any function or closure that takes the parameters +/// `(Entity, &mut World)`. This blanket implementation automatically checks if the entity +/// exists in the `apply` method as follows: +/// +/// ```ignore +/// if world.entities.contains(entity) { +/// // Execute command +/// } else { +/// // Handle failure according to internal `failure_mode` +/// } +/// ``` +/// +/// If you directly implement this trait on a struct (not recommended), you will have to +/// manually implement the `apply` method yourself. If you do not check that the entity +/// exists as above, the command could fail in less predictable ways. pub trait EntityCommand: Send + 'static { /// Executes this command for the given [`Entity`]. fn apply(self, entity: Entity, world: &mut World, failure_mode: FailureMode); @@ -1162,7 +1180,7 @@ impl<'a> EntityCommands<'a> { /// Sets the [`EntityCommands`] instance to ignore commands if the entity doesn't exist. /// /// # See also: - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn ignore_if_missing(&mut self) -> &mut Self { @@ -1187,7 +1205,7 @@ impl<'a> EntityCommands<'a> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn warn_if_missing(&mut self) -> &mut Self { self.commands.warn_on_error(); @@ -1198,7 +1216,7 @@ impl<'a> EntityCommands<'a> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.commands.panic_on_error(); @@ -1768,7 +1786,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Sets the [`EntityEntryCommands`] instance to ignore commands if the entity doesn't exist. /// /// # See also: - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn ignore_if_missing(&mut self) -> &mut Self { @@ -1793,7 +1811,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`panic_if_missing`](Self::panic_if_missing) pub fn warn_if_missing(&mut self) -> &mut Self { self.entity_commands.warn_if_missing(); @@ -1804,7 +1822,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) (default) /// - [`warn_if_missing`](Self::warn_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.entity_commands.panic_if_missing(); From f600a17ae30917cd1baf4696d4d4f08b0e43def4 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Fri, 18 Oct 2024 14:59:57 -0500 Subject: [PATCH 16/29] fix ignore performance --- crates/bevy_ecs/src/system/commands/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 5a5769bc10bb3..eb36ccd9cfa99 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1926,9 +1926,11 @@ where if world.entities.contains(id) { self(world.entity_mut(id)); } else { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - failure_mode.fail(message); + if failure_mode != FailureMode::Ignore { + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); + failure_mode.fail(message); + } } } } @@ -1941,9 +1943,11 @@ where if world.entities.contains(id) { self(id, world); } else { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - failure_mode.fail(message); + if failure_mode != FailureMode::Ignore { + let message = + format!("Could not execute EntityCommand because its entity {id:?} was missing"); + failure_mode.fail(message); + } } } } From eea892f95ea4033a4dc3bbc4d5e0555e4e88ba00 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Fri, 18 Oct 2024 15:33:03 -0500 Subject: [PATCH 17/29] fix warn performance --- crates/bevy_ecs/src/system/commands/mod.rs | 30 +++++++++++++++++----- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index eb36ccd9cfa99..235646ca0fa84 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1927,9 +1927,18 @@ where self(world.entity_mut(id)); } else { if failure_mode != FailureMode::Ignore { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - failure_mode.fail(message); + match failure_mode { + FailureMode::Ignore => unreachable!(), + FailureMode::Log => info!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + FailureMode::Warn => warn!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + FailureMode::Panic => panic!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + }; } } } @@ -1944,9 +1953,18 @@ where self(id, world); } else { if failure_mode != FailureMode::Ignore { - let message = - format!("Could not execute EntityCommand because its entity {id:?} was missing"); - failure_mode.fail(message); + match failure_mode { + FailureMode::Ignore => unreachable!(), + FailureMode::Log => info!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + FailureMode::Warn => warn!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + FailureMode::Panic => panic!( + "Could not execute EntityCommand because its entity {id:?} was missing" + ), + }; } } } From 5fb2be95b0baeea8da0e82db606a9ffc377606a2 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Fri, 18 Oct 2024 15:53:08 -0500 Subject: [PATCH 18/29] ci knows best --- crates/bevy_ecs/src/system/commands/mod.rs | 52 ++++++++++------------ 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 235646ca0fa84..a718e97b2ed50 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1926,20 +1926,18 @@ where if world.entities.contains(id) { self(world.entity_mut(id)); } else { - if failure_mode != FailureMode::Ignore { - match failure_mode { - FailureMode::Ignore => unreachable!(), - FailureMode::Log => info!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - FailureMode::Warn => warn!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - FailureMode::Panic => panic!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - }; - } + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => { + info!("Could not execute EntityCommand because its entity {id:?} was missing") + } + FailureMode::Warn => { + warn!("Could not execute EntityCommand because its entity {id:?} was missing") + } + FailureMode::Panic => { + panic!("Could not execute EntityCommand because its entity {id:?} was missing") + } + }; } } } @@ -1952,20 +1950,18 @@ where if world.entities.contains(id) { self(id, world); } else { - if failure_mode != FailureMode::Ignore { - match failure_mode { - FailureMode::Ignore => unreachable!(), - FailureMode::Log => info!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - FailureMode::Warn => warn!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - FailureMode::Panic => panic!( - "Could not execute EntityCommand because its entity {id:?} was missing" - ), - }; - } + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => { + info!("Could not execute EntityCommand because its entity {id:?} was missing") + } + FailureMode::Warn => { + warn!("Could not execute EntityCommand because its entity {id:?} was missing") + } + FailureMode::Panic => { + panic!("Could not execute EntityCommand because its entity {id:?} was missing") + } + }; } } } From b364e099117742dfbc583b189c9ed1bb0654d687 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Fri, 18 Oct 2024 15:57:42 -0500 Subject: [PATCH 19/29] semicolons --- crates/bevy_ecs/src/system/commands/mod.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index a718e97b2ed50..0fbbe1fac58c3 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1929,13 +1929,13 @@ where match failure_mode { FailureMode::Ignore => (), FailureMode::Log => { - info!("Could not execute EntityCommand because its entity {id:?} was missing") + info!("Could not execute EntityCommand because its entity {id:?} was missing"); } FailureMode::Warn => { - warn!("Could not execute EntityCommand because its entity {id:?} was missing") + warn!("Could not execute EntityCommand because its entity {id:?} was missing"); } FailureMode::Panic => { - panic!("Could not execute EntityCommand because its entity {id:?} was missing") + panic!("Could not execute EntityCommand because its entity {id:?} was missing"); } }; } @@ -1953,13 +1953,13 @@ where match failure_mode { FailureMode::Ignore => (), FailureMode::Log => { - info!("Could not execute EntityCommand because its entity {id:?} was missing") + info!("Could not execute EntityCommand because its entity {id:?} was missing"); } FailureMode::Warn => { - warn!("Could not execute EntityCommand because its entity {id:?} was missing") + warn!("Could not execute EntityCommand because its entity {id:?} was missing"); } FailureMode::Panic => { - panic!("Could not execute EntityCommand because its entity {id:?} was missing") + panic!("Could not execute EntityCommand because its entity {id:?} was missing"); } }; } From 581e54b68bd37c5099c0a6305d89fd91260fc0e6 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Sat, 19 Oct 2024 16:54:00 -0500 Subject: [PATCH 20/29] remove bad convenience method --- crates/bevy_ecs/src/system/commands/mod.rs | 12 +++--- crates/bevy_ecs/src/world/mod.rs | 49 ++++++++++------------ 2 files changed, 29 insertions(+), 32 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 0fbbe1fac58c3..5d24478df1d12 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1929,13 +1929,13 @@ where match failure_mode { FailureMode::Ignore => (), FailureMode::Log => { - info!("Could not execute EntityCommand because its entity {id:?} was missing"); + info!("Could not execute EntityCommand because its Entity {id:?} was missing"); } FailureMode::Warn => { - warn!("Could not execute EntityCommand because its entity {id:?} was missing"); + warn!("Could not execute EntityCommand because its Entity {id:?} was missing"); } FailureMode::Panic => { - panic!("Could not execute EntityCommand because its entity {id:?} was missing"); + panic!("Could not execute EntityCommand because its Entity {id:?} was missing"); } }; } @@ -1953,13 +1953,13 @@ where match failure_mode { FailureMode::Ignore => (), FailureMode::Log => { - info!("Could not execute EntityCommand because its entity {id:?} was missing"); + info!("Could not execute EntityCommand because its Entity {id:?} was missing"); } FailureMode::Warn => { - warn!("Could not execute EntityCommand because its entity {id:?} was missing"); + warn!("Could not execute EntityCommand because its Entity {id:?} was missing"); } FailureMode::Panic => { - panic!("Could not execute EntityCommand because its entity {id:?} was missing"); + panic!("Could not execute EntityCommand because its Entity {id:?} was missing"); } }; } diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 0a749d8629866..69ae221d17091 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2560,6 +2560,27 @@ impl World { archetype_id: ArchetypeId, } + let insert_batch_fail = |entity: Entity| { + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => info!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + core::any::type_name::(), + entity, + ), + FailureMode::Warn => warn!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + core::any::type_name::(), + entity, + ), + FailureMode::Panic => panic!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + core::any::type_name::(), + entity, + ), + }; + }; + let mut batch = iter.into_iter(); if let Some((first_entity, first_bundle)) = batch.next() { @@ -2616,21 +2637,11 @@ impl World { ) }; } else { - let message = format!( - "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", - core::any::type_name::(), - entity, - ); - failure_mode.fail(message); + insert_batch_fail(entity); } } } else { - let message = format!( - "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", - core::any::type_name::(), - first_entity, - ); - failure_mode.fail(message); + insert_batch_fail(first_entity); } } } @@ -3813,20 +3824,6 @@ pub enum FailureMode { Panic, } -impl FailureMode { - /// Convenience method to send a message upon failure. - /// If you want to send different messages for different modes, - /// match the enum manually. - pub fn fail(&self, message: String) { - match self { - Self::Ignore => (), - Self::Log => info!("{}", message), - Self::Warn => warn!("{}", message), - Self::Panic => panic!("{}", message), - }; - } -} - #[cfg(test)] mod tests { use super::{FromWorld, World}; From 1f4e7d00f77207cf50f6b9763eb7db0cbca3fa3c Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Sun, 20 Oct 2024 17:13:35 -0500 Subject: [PATCH 21/29] move check from `apply` to `with_entity` also more docs and resolve conflict --- crates/bevy_ecs/src/system/commands/mod.rs | 140 +++++++++++++-------- 1 file changed, 85 insertions(+), 55 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 5d24478df1d12..809e6dd595fba 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -503,14 +503,11 @@ impl<'w, 's> Commands<'w, 's> { #[track_caller] pub fn entity(&mut self, entity: Entity) -> EntityCommands { if !self.entities.contains(entity) { - let message = format!( - "Attempted to create an EntityCommands for entity {entity:?}, which doesn't exist" - ); match self.failure_mode { FailureMode::Ignore => (), - FailureMode::Log => info!("{}; returned invalid EntityCommands", message), - FailureMode::Warn => warn!("{}; returned invalid EntityCommands", message), - FailureMode::Panic => panic!("{}", message), + FailureMode::Log => info!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist; returned invalid EntityCommands"), + FailureMode::Warn => warn!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist; returned invalid EntityCommands"), + FailureMode::Panic => panic!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist"), }; } EntityCommands { @@ -1041,7 +1038,7 @@ impl<'w, 's> Commands<'w, 's> { /// A [`Command`] which gets executed for a given [`Entity`]. /// -/// # Examples +/// # Example /// /// ``` /// # use std::collections::HashSet; @@ -1093,26 +1090,70 @@ impl<'w, 's> Commands<'w, 's> { /// } /// ``` /// -/// # Implementing +/// # Custom commands +/// +/// There are three ways to create a new `EntityCommand`: +/// +/// ## Function +/// +/// As seen above, a function can be used as an `EntityCommand` if it takes the parameters +/// `(Entity, &mut World)` and returns nothing: +/// +/// ```ignore +/// fn new_command(entity: Entity, world: &mut World) { +/// // Command code +/// } +/// commands.entity(entity).queue(new_command); +/// ``` +/// +/// Note that this approach does not allow the command to take additional parameters +/// (see below if that's what you need). /// -/// This trait is blanket-implemented for any function or closure that takes the parameters -/// `(Entity, &mut World)`. This blanket implementation automatically checks if the entity -/// exists in the `apply` method as follows: +/// ## Closure +/// +/// A closure can be used as an `EntityCommand` if it takes the parameters +/// `(Entity, &mut World)` and returns nothing. +/// +/// The most versatile form of this (and the way most built-in `EntityCommand`s are implemented) +/// is a function that returns such a closure. This allows you to take in parameters for the command: /// /// ```ignore -/// if world.entities.contains(entity) { -/// // Execute command -/// } else { -/// // Handle failure according to internal `failure_mode` +/// fn new_command(whatever_parameters: i32) -> impl EntityCommand { +/// move |entity: Entity, world: &mut World| { +/// // Command code (can access parameters here) +/// } /// } +/// commands.entity(entity).queue(new_command(5)); +/// ``` +/// +/// You can also queue a closure directly if so desired: +/// +/// ```ignore +/// commands.entity(entity).queue(|entity: Entity, world: &mut World| { +/// // Command code +/// }); /// ``` /// -/// If you directly implement this trait on a struct (not recommended), you will have to -/// manually implement the `apply` method yourself. If you do not check that the entity -/// exists as above, the command could fail in less predictable ways. +/// ## Struct +/// +/// A struct (or enum) can be used as an `EntityCommand` if it implements the trait +/// and its `apply` method: +/// +/// ```ignore +/// struct NewCommand { +/// // Fields act as parameters +/// whatever_parameters: i32, +/// } +/// impl EntityCommand for NewCommand { +/// fn apply(self, entity: Entity, world: &mut World) { +/// // Command code +/// } +/// } +/// commands.entity(entity).queue(NewCommand { whatever_parameters: 5 }); +/// ``` pub trait EntityCommand: Send + 'static { /// Executes this command for the given [`Entity`]. - fn apply(self, entity: Entity, world: &mut World, failure_mode: FailureMode); + fn apply(self, entity: Entity, world: &mut World); /// Returns a [`Command`] which executes this [`EntityCommand`] for the given [`Entity`]. /// @@ -1125,7 +1166,24 @@ pub trait EntityCommand: Send + 'static { where Self: Sized, { - move |world: &mut World| self.apply(entity, world, failure_mode) + move |world: &mut World| { + if world.entities.contains(entity) { + self.apply(entity, world); + } else { + match failure_mode { + FailureMode::Ignore => (), + FailureMode::Log => { + info!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + } + FailureMode::Warn => { + warn!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + } + FailureMode::Panic => { + panic!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + } + }; + } + } } } @@ -1137,6 +1195,8 @@ pub trait EntityCommand: Send + 'static { /// The command queue is applied between [`Schedules`](bevy_ecs::schedule::Schedule), one by one, /// so that each command can have exclusive access to the World. /// +/// # Fallible +/// /// Due to their deferred nature, an entity you're trying change with an `EntityCommand` can be /// despawned by the time the command is executed. Use the following commands to set how you /// would like subsequent commands to respond if the entity is missing: @@ -1829,7 +1889,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { self } - /// Modify the component `T` if it exists, using the the function `modify`. + /// Modify the component `T` if it exists, using the function `modify`. pub fn and_modify(&mut self, modify: impl FnOnce(Mut) + Send + Sync + 'static) -> &mut Self { self.entity_commands .queue(move |mut entity: EntityWorldMut| { @@ -1922,23 +1982,8 @@ impl EntityCommand for F where F: FnOnce(EntityWorldMut) + Send + 'static, { - fn apply(self, id: Entity, world: &mut World, failure_mode: FailureMode) { - if world.entities.contains(id) { - self(world.entity_mut(id)); - } else { - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => { - info!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - FailureMode::Warn => { - warn!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - FailureMode::Panic => { - panic!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - }; - } + fn apply(self, id: Entity, world: &mut World) { + self(world.entity_mut(id)); } } @@ -1946,23 +1991,8 @@ impl EntityCommand for F where F: FnOnce(Entity, &mut World) + Send + 'static, { - fn apply(self, id: Entity, world: &mut World, failure_mode: FailureMode) { - if world.entities.contains(id) { - self(id, world); - } else { - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => { - info!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - FailureMode::Warn => { - warn!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - FailureMode::Panic => { - panic!("Could not execute EntityCommand because its Entity {id:?} was missing"); - } - }; - } + fn apply(self, id: Entity, world: &mut World) { + self(id, world); } } From 001634c85083b08a9c406c43a966900541eff023 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Sun, 20 Oct 2024 17:18:05 -0500 Subject: [PATCH 22/29] conflict resolve left a space RIP --- crates/bevy_ecs/src/system/commands/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 085d1a3077142..a4d716fd17161 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1889,7 +1889,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { self } - /// Modify the component `T` if it exists, using the function `modify`. pub fn and_modify(&mut self, modify: impl FnOnce(Mut) + Send + Sync + 'static) -> &mut Self { self.entity_commands From 1916616cdc5cd2fad1e8111a092ffabec1504b00 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Thu, 24 Oct 2024 10:54:29 -0500 Subject: [PATCH 23/29] allergic to "to"?? --- crates/bevy_ecs/src/system/commands/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index a4d716fd17161..53d1e62e0fba4 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -694,7 +694,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// # Fallible /// This command can fail if any of the entities do not exist. It will [`panic`]/[`warn`]/[`log`]/[`ignore`] - /// according the `Commands` instance's current setting. + /// according to the `Commands` instance's current setting. /// /// [`panic`]: Self::panic_on_error /// [`warn`]: Self::warn_on_error @@ -724,7 +724,7 @@ impl<'w, 's> Commands<'w, 's> { /// /// # Fallible /// This command can fail if any of the entities do not exist. It will [`panic`]/[`warn`]/[`log`]/[`ignore`] - /// according the `Commands` instance's current setting. + /// according to the `Commands` instance's current setting. /// /// [`panic`]: Self::panic_on_error /// [`warn`]: Self::warn_on_error @@ -1197,7 +1197,7 @@ pub trait EntityCommand: Send + 'static { /// /// # Fallible /// -/// Due to their deferred nature, an entity you're trying change with an `EntityCommand` can be +/// Due to their deferred nature, an entity you're trying to change with an `EntityCommand` can be /// despawned by the time the command is executed. Use the following commands to set how you /// would like subsequent commands to respond if the entity is missing: /// - [`ignore_if_missing`](Self::ignore_if_missing) From b129f1ff4189d7e24312a5cbfd7ac2209a4b8adf Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Sun, 27 Oct 2024 23:16:51 -0500 Subject: [PATCH 24/29] address comments `failure_mode` and `FailureMode` are now `failure_handling_mode` and `FailureHandlingMode` --- crates/bevy_ecs/src/system/commands/mod.rs | 112 +++++++++++---------- crates/bevy_ecs/src/world/mod.rs | 24 ++--- 2 files changed, 73 insertions(+), 63 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 53d1e62e0fba4..e861545070701 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -16,7 +16,7 @@ use crate::{ system::{input::SystemInput, RunSystemWithInput, SystemId}, world::{ command_queue::RawCommandQueue, unsafe_world_cell::UnsafeWorldCell, Command, CommandQueue, - EntityWorldMut, FailureMode, FromWorld, SpawnBatchIter, World, + EntityWorldMut, FailureHandlingMode, FromWorld, SpawnBatchIter, World, }, }; use bevy_ptr::OwningPtr; @@ -77,7 +77,7 @@ pub use parallel_scope::*; pub struct Commands<'w, 's> { queue: InternalQueue<'s>, entities: &'w Entities, - pub(crate) failure_mode: FailureMode, + pub(crate) failure_handling_mode: FailureHandlingMode, } // SAFETY: All commands [`Command`] implement [`Send`] @@ -173,7 +173,7 @@ const _: () = { Commands { queue: InternalQueue::CommandQueue(f0), entities: f1, - failure_mode: FailureMode::default(), + failure_handling_mode: FailureHandlingMode::default(), } } } @@ -210,7 +210,7 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::CommandQueue(Deferred(queue)), entities, - failure_mode: FailureMode::default(), + failure_handling_mode: FailureHandlingMode::default(), } } @@ -228,7 +228,7 @@ impl<'w, 's> Commands<'w, 's> { Self { queue: InternalQueue::RawCommandQueue(queue), entities, - failure_mode: FailureMode::default(), + failure_handling_mode: FailureHandlingMode::default(), } } @@ -259,7 +259,7 @@ impl<'w, 's> Commands<'w, 's> { } }, entities: self.entities, - failure_mode: self.failure_mode, + failure_handling_mode: self.failure_handling_mode, } } @@ -283,7 +283,7 @@ impl<'w, 's> Commands<'w, 's> { /// - [`warn_on_error`](Self::warn_on_error) /// - [`panic_on_error`](Self::panic_on_error) pub fn ignore_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Ignore; + self.failure_handling_mode = FailureHandlingMode::Ignore; self } @@ -298,7 +298,7 @@ impl<'w, 's> Commands<'w, 's> { /// - [`warn_on_error`](Self::warn_on_error) /// - [`panic_on_error`](Self::panic_on_error) pub fn log_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Log; + self.failure_handling_mode = FailureHandlingMode::Log; self } @@ -311,7 +311,7 @@ impl<'w, 's> Commands<'w, 's> { /// - [`log_on_error`](Self::log_on_error) (default) /// - [`panic_on_error`](Self::panic_on_error) pub fn warn_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Warn; + self.failure_handling_mode = FailureHandlingMode::Warn; self } @@ -324,7 +324,7 @@ impl<'w, 's> Commands<'w, 's> { /// - [`log_on_error`](Self::log_on_error) (default) /// - [`warn_on_error`](Self::warn_on_error) pub fn panic_on_error(&mut self) -> &mut Self { - self.failure_mode = FailureMode::Panic; + self.failure_handling_mode = FailureHandlingMode::Panic; self } @@ -458,8 +458,8 @@ impl<'w, 's> Commands<'w, 's> { /// Returns the [`EntityCommands`] for the requested [`Entity`]. /// - /// This method does not guarantee that `EntityCommands` will be successfully applied, - /// since another command in the queue may delete the entity before them. + /// This method does not guarantee that commands queued by the `EntityCommands` + /// will be successful, since the entity could be despawned before they are executed. /// /// # Fallible /// @@ -503,11 +503,11 @@ impl<'w, 's> Commands<'w, 's> { #[track_caller] pub fn entity(&mut self, entity: Entity) -> EntityCommands { if !self.entities.contains(entity) { - match self.failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => info!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist; returned invalid EntityCommands"), - FailureMode::Warn => warn!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist; returned invalid EntityCommands"), - FailureMode::Panic => panic!("Attempted to create an EntityCommands for Entity {entity:?}, which doesn't exist"), + match self.failure_handling_mode { + FailureHandlingMode::Ignore => (), + FailureHandlingMode::Log => info!("Attempted to create an EntityCommands for entity {entity}, which doesn't exist; returned invalid EntityCommands"), + FailureHandlingMode::Warn => warn!("Attempted to create an EntityCommands for entity {entity}, which doesn't exist; returned invalid EntityCommands"), + FailureHandlingMode::Panic => panic!("Attempted to create an EntityCommands for entity {entity}, which doesn't exist"), }; } EntityCommands { @@ -520,8 +520,8 @@ impl<'w, 's> Commands<'w, 's> { /// /// Returns `None` if the entity does not exist. /// - /// This method does not guarantee that `EntityCommands` will be successfully applied, - /// since another command in the queue may delete the entity before them. + /// This method does not guarantee that commands queued by the `EntityCommands` + /// will be successful, since the entity could be despawned before they are executed. /// /// # Example /// @@ -706,7 +706,7 @@ impl<'w, 's> Commands<'w, 's> { I: IntoIterator + Send + Sync + 'static, B: Bundle, { - self.queue(insert_batch(batch, self.failure_mode)); + self.queue(insert_batch(batch, self.failure_handling_mode)); } /// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity). @@ -736,7 +736,7 @@ impl<'w, 's> Commands<'w, 's> { I: IntoIterator + Send + Sync + 'static, B: Bundle, { - self.queue(insert_batch_if_new(batch, self.failure_mode)); + self.queue(insert_batch_if_new(batch, self.failure_handling_mode)); } /// Pushes a [`Command`] to the queue for adding a [`Bundle`] type to a batch of [`Entities`](Entity). @@ -1162,7 +1162,7 @@ pub trait EntityCommand: Send + 'static { /// 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, failure_mode: FailureMode) -> impl Command + fn with_entity(self, entity: Entity, failure_handling_mode: FailureHandlingMode) -> impl Command where Self: Sized, { @@ -1170,16 +1170,16 @@ pub trait EntityCommand: Send + 'static { if world.entities.contains(entity) { self.apply(entity, world); } else { - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => { - info!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + match failure_handling_mode { + FailureHandlingMode::Ignore => (), + FailureHandlingMode::Log => { + info!("Could not execute EntityCommand because its entity {entity} was missing"); } - FailureMode::Warn => { - warn!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + FailureHandlingMode::Warn => { + warn!("Could not execute EntityCommand because its entity {entity} was missing"); } - FailureMode::Panic => { - panic!("Could not execute EntityCommand because its Entity {entity:?} was missing"); + FailureHandlingMode::Panic => { + panic!("Could not execute EntityCommand because its entity {entity} was missing"); } }; } @@ -1237,7 +1237,8 @@ impl<'a> EntityCommands<'a> { } } - /// Sets the [`EntityCommands`] instance to ignore commands if the entity doesn't exist. + /// Sets the [`EntityCommands`] instance to ignore commands if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`log_if_missing`](Self::log_if_missing) (default) @@ -1248,7 +1249,8 @@ impl<'a> EntityCommands<'a> { self } - /// Sets the [`EntityCommands`] instance to log if the entity doesn't exist when a command is executed. + /// Sets the [`EntityCommands`] instance to log if the entity doesn't exist + /// when a command is executed. /// /// This is the default setting. /// @@ -1261,7 +1263,8 @@ impl<'a> EntityCommands<'a> { self } - /// Sets the [`EntityCommands`] instance to warn if the entity doesn't exist when a command is executed. + /// Sets the [`EntityCommands`] instance to warn if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) @@ -1272,7 +1275,8 @@ impl<'a> EntityCommands<'a> { self } - /// Sets the [`EntityCommands`] instance to panic if the entity doesn't exist when a command is executed. + /// Sets the [`EntityCommands`] instance to panic if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) @@ -1472,7 +1476,7 @@ impl<'a> EntityCommands<'a> { /// # Note /// /// [`Self::insert`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert`]. /// /// # Example /// @@ -1567,7 +1571,7 @@ impl<'a> EntityCommands<'a> { /// # Note /// /// [`Self::insert_if_new_and`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new_and`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new_and`]. /// /// # Example /// @@ -1612,7 +1616,7 @@ impl<'a> EntityCommands<'a> { /// # Note /// /// [`Self::insert_if_new`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new`]. pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { self.queue(try_insert(bundle, InsertMode::Keep)) } @@ -1731,7 +1735,7 @@ impl<'a> EntityCommands<'a> { /// Despawns the entity. /// /// [`Self::despawn`] used to warn if the entity was missing, and this was the silent version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::despawn`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::despawn`]. #[track_caller] pub fn try_despawn(&mut self) { self.queue(try_despawn()); @@ -1755,7 +1759,7 @@ impl<'a> EntityCommands<'a> { /// ``` pub fn queue(&mut self, command: impl EntityCommand) -> &mut Self { self.commands - .queue(command.with_entity(self.entity, self.commands.failure_mode)); + .queue(command.with_entity(self.entity, self.commands.failure_handling_mode)); self } @@ -1843,7 +1847,8 @@ pub struct EntityEntryCommands<'a, T> { } impl<'a, T: Component> EntityEntryCommands<'a, T> { - /// Sets the [`EntityEntryCommands`] instance to ignore commands if the entity doesn't exist. + /// Sets the [`EntityEntryCommands`] instance to ignore commands if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`log_if_missing`](Self::log_if_missing) (default) @@ -1854,7 +1859,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { self } - /// Sets the [`EntityEntryCommands`] instance to log if the entity doesn't exist when a command is executed. + /// Sets the [`EntityEntryCommands`] instance to log if the entity doesn't exist + /// when a command is executed. /// /// This is the default setting. /// @@ -1867,7 +1873,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { self } - /// Sets the [`EntityEntryCommands`] instance to warn if the entity doesn't exist when a command is executed. + /// Sets the [`EntityEntryCommands`] instance to warn if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) @@ -1878,7 +1885,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { self } - /// Sets the [`EntityEntryCommands`] instance to panic if the entity doesn't exist when a command is executed. + /// Sets the [`EntityEntryCommands`] instance to panic if the entity doesn't exist + /// when a command is executed. /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) @@ -1913,7 +1921,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `default` into this entity, if `T` is not already present. /// /// [`Self::or_insert`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert`]. /// /// See also [`or_insert_with`](Self::or_insert_with). #[track_caller] @@ -1934,7 +1942,7 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) the value returned from `default` into this entity, if `T` is not already present. /// /// [`Self::or_insert_with`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert_with`] + /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert_with`]. /// /// See also [`or_insert`](Self::or_insert) and [`or_try_insert`](Self::or_try_insert). #[track_caller] @@ -2045,11 +2053,12 @@ where } /// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities. -/// If any entities do not exist in the world, this command will fail according to `failure_mode`. +/// If any entities do not exist in the world, this command will fail according to +/// `failure_handling_mode`. /// /// This is more efficient than inserting the bundles individually. #[track_caller] -fn insert_batch(batch: I, failure_mode: FailureMode) -> impl Command +fn insert_batch(batch: I, failure_handling_mode: FailureHandlingMode) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, @@ -2060,7 +2069,7 @@ where world.insert_batch_with_caller( batch, InsertMode::Replace, - failure_mode, + failure_handling_mode, #[cfg(feature = "track_change_detection")] caller, ); @@ -2068,11 +2077,12 @@ where } /// A [`Command`] that consumes an iterator to add a series of [`Bundles`](Bundle) to a set of entities. -/// If any entities do not exist in the world, this command will fail according to `failure_mode`. +/// If any entities do not exist in the world, this command will fail according to +/// `failure_handling_mode`. /// /// This is more efficient than inserting the bundles individually. #[track_caller] -fn insert_batch_if_new(batch: I, failure_mode: FailureMode) -> impl Command +fn insert_batch_if_new(batch: I, failure_handling_mode: FailureHandlingMode) -> impl Command where I: IntoIterator + Send + Sync + 'static, B: Bundle, @@ -2083,7 +2093,7 @@ where world.insert_batch_with_caller( batch, InsertMode::Keep, - failure_mode, + failure_handling_mode, #[cfg(feature = "track_change_detection")] caller, ); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 73f0271cea833..8c35278bd667f 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -2491,7 +2491,7 @@ impl World { self.insert_batch_with_caller( batch, InsertMode::Replace, - FailureMode::Panic, + FailureHandlingMode::Panic, #[cfg(feature = "track_change_detection")] Location::caller(), ); @@ -2522,7 +2522,7 @@ impl World { self.insert_batch_with_caller( batch, InsertMode::Keep, - FailureMode::Panic, + FailureHandlingMode::Panic, #[cfg(feature = "track_change_detection")] Location::caller(), ); @@ -2540,7 +2540,7 @@ impl World { &mut self, iter: I, insert_mode: InsertMode, - failure_mode: FailureMode, + failure_handling_mode: FailureHandlingMode, #[cfg(feature = "track_change_detection")] caller: &'static Location, ) where I: IntoIterator, @@ -2561,20 +2561,20 @@ impl World { } let insert_batch_fail = |entity: Entity| { - match failure_mode { - FailureMode::Ignore => (), - FailureMode::Log => info!( - "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + match failure_handling_mode { + FailureHandlingMode::Ignore => (), + FailureHandlingMode::Log => info!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity, ), - FailureMode::Warn => warn!( - "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + FailureHandlingMode::Warn => warn!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity, ), - FailureMode::Panic => panic!( - "error[B0003]: Could not insert a bundle (of type `{}`) for entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", + FailureHandlingMode::Panic => panic!( + "error[B0003]: Could not insert a bundle (of type `{}`) for entity {} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", core::any::type_name::(), entity, ), @@ -3812,7 +3812,7 @@ impl FromWorld for T { /// How to respond if a function fails #[derive(Default, Clone, Copy, PartialEq, Eq)] -pub enum FailureMode { +pub enum FailureHandlingMode { /// Do nothing Ignore, /// Send a benign message to the log From 791a1f04cf31630f83714e44b36d3d85f9fdf40e Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Mon, 2 Dec 2024 15:04:47 -0600 Subject: [PATCH 25/29] change default from log to panic, remove unused pub(crate) --- crates/bevy_ecs/src/system/commands/mod.rs | 50 +++++++++++----------- crates/bevy_ecs/src/world/mod.rs | 2 +- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index e861545070701..6df8826df4f7d 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -77,7 +77,7 @@ pub use parallel_scope::*; pub struct Commands<'w, 's> { queue: InternalQueue<'s>, entities: &'w Entities, - pub(crate) failure_handling_mode: FailureHandlingMode, + failure_handling_mode: FailureHandlingMode, } // SAFETY: All commands [`Command`] implement [`Send`] @@ -279,9 +279,9 @@ impl<'w, 's> Commands<'w, 's> { /// Any subsequent commands that can fail will do so silently. /// /// # See also: - /// - [`log_on_error`](Self::log_on_error) (default) + /// - [`log_on_error`](Self::log_on_error) /// - [`warn_on_error`](Self::warn_on_error) - /// - [`panic_on_error`](Self::panic_on_error) + /// - [`panic_on_error`](Self::panic_on_error) (default) pub fn ignore_on_error(&mut self) -> &mut Self { self.failure_handling_mode = FailureHandlingMode::Ignore; self @@ -291,12 +291,10 @@ impl<'w, 's> Commands<'w, 's> { /// /// Any subsequent commands that can fail will log each failure. /// - /// This is the default setting. - /// /// # See also: /// - [`ignore_on_error`](Self::ignore_on_error) /// - [`warn_on_error`](Self::warn_on_error) - /// - [`panic_on_error`](Self::panic_on_error) + /// - [`panic_on_error`](Self::panic_on_error) (default) pub fn log_on_error(&mut self) -> &mut Self { self.failure_handling_mode = FailureHandlingMode::Log; self @@ -308,8 +306,8 @@ impl<'w, 's> Commands<'w, 's> { /// /// # See also: /// - [`ignore_on_error`](Self::ignore_on_error) - /// - [`log_on_error`](Self::log_on_error) (default) - /// - [`panic_on_error`](Self::panic_on_error) + /// - [`log_on_error`](Self::log_on_error) + /// - [`panic_on_error`](Self::panic_on_error) (default) pub fn warn_on_error(&mut self) -> &mut Self { self.failure_handling_mode = FailureHandlingMode::Warn; self @@ -319,9 +317,11 @@ impl<'w, 's> Commands<'w, 's> { /// /// Any subsequent commands that can fail will panic if they do. /// + /// This is the default setting. + /// /// # See also: /// - [`ignore_on_error`](Self::ignore_on_error) - /// - [`log_on_error`](Self::log_on_error) (default) + /// - [`log_on_error`](Self::log_on_error) /// - [`warn_on_error`](Self::warn_on_error) pub fn panic_on_error(&mut self) -> &mut Self { self.failure_handling_mode = FailureHandlingMode::Panic; @@ -1241,9 +1241,9 @@ impl<'a> EntityCommands<'a> { /// when a command is executed. /// /// # See also: - /// - [`log_if_missing`](Self::log_if_missing) (default) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn ignore_if_missing(&mut self) -> &mut Self { self.commands.ignore_on_error(); self @@ -1252,12 +1252,10 @@ impl<'a> EntityCommands<'a> { /// Sets the [`EntityCommands`] instance to log if the entity doesn't exist /// when a command is executed. /// - /// This is the default setting. - /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn log_if_missing(&mut self) -> &mut Self { self.commands.log_on_error(); self @@ -1268,8 +1266,8 @@ impl<'a> EntityCommands<'a> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) (default) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn warn_if_missing(&mut self) -> &mut Self { self.commands.warn_on_error(); self @@ -1278,9 +1276,11 @@ impl<'a> EntityCommands<'a> { /// Sets the [`EntityCommands`] instance to panic if the entity doesn't exist /// when a command is executed. /// + /// This is the default setting. + /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) (default) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.commands.panic_on_error(); @@ -1851,9 +1851,9 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// when a command is executed. /// /// # See also: - /// - [`log_if_missing`](Self::log_if_missing) (default) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn ignore_if_missing(&mut self) -> &mut Self { self.entity_commands.ignore_if_missing(); self @@ -1862,12 +1862,10 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Sets the [`EntityEntryCommands`] instance to log if the entity doesn't exist /// when a command is executed. /// - /// This is the default setting. - /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn log_if_missing(&mut self) -> &mut Self { self.entity_commands.log_if_missing(); self @@ -1878,8 +1876,8 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) (default) - /// - [`panic_if_missing`](Self::panic_if_missing) + /// - [`log_if_missing`](Self::log_if_missing) + /// - [`panic_if_missing`](Self::panic_if_missing) (default) pub fn warn_if_missing(&mut self) -> &mut Self { self.entity_commands.warn_if_missing(); self @@ -1888,9 +1886,11 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Sets the [`EntityEntryCommands`] instance to panic if the entity doesn't exist /// when a command is executed. /// + /// This is the default setting. + /// /// # See also: /// - [`ignore_if_missing`](Self::ignore_if_missing) - /// - [`log_if_missing`](Self::log_if_missing) (default) + /// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) pub fn panic_if_missing(&mut self) -> &mut Self { self.entity_commands.panic_if_missing(); diff --git a/crates/bevy_ecs/src/world/mod.rs b/crates/bevy_ecs/src/world/mod.rs index 8c35278bd667f..3db7cb98caafc 100644 --- a/crates/bevy_ecs/src/world/mod.rs +++ b/crates/bevy_ecs/src/world/mod.rs @@ -3816,11 +3816,11 @@ pub enum FailureHandlingMode { /// Do nothing Ignore, /// Send a benign message to the log - #[default] Log, /// Send a more serious message to the log Warn, /// Stop the application + #[default] Panic, } From 374aad5035fc72cdaa371886e69a1b77aaf80d04 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Mon, 2 Dec 2024 17:09:23 -0600 Subject: [PATCH 26/29] add method to allow `try_` variants and `despawn` to fail uniquely --- crates/bevy_ecs/src/system/commands/mod.rs | 66 ++++++++++++++++------ 1 file changed, 50 insertions(+), 16 deletions(-) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 2c578c859ae59..6afbe56a52de5 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1477,8 +1477,11 @@ impl<'a> EntityCommands<'a> { component_id: ComponentId, value: T, ) -> &mut Self { - // SAFETY: same invariants as parent call - self.queue(unsafe { insert_by_id(component_id, value) }) + self.queue_with_different_failure_handling_mode( + // SAFETY: same invariants as parent call + unsafe { insert_by_id(component_id, value) }, + FailureHandlingMode::Ignore, + ) } /// Tries to add a [`Bundle`] of components to the entity. @@ -1532,7 +1535,10 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue(try_insert(bundle, InsertMode::Replace)) + self.queue_with_different_failure_handling_mode( + try_insert(bundle, InsertMode::Replace), + FailureHandlingMode::Ignore, + ) } /// Similar to [`Self::try_insert`] but will only try to insert if the predicate returns true. @@ -1567,7 +1573,10 @@ impl<'a> EntityCommands<'a> { F: FnOnce() -> bool, { if condition() { - self.queue(try_insert(bundle, InsertMode::Replace)) + self.queue_with_different_failure_handling_mode( + try_insert(bundle, InsertMode::Replace), + FailureHandlingMode::Ignore, + ) } else { self } @@ -1630,7 +1639,10 @@ impl<'a> EntityCommands<'a> { /// [`Self::insert_if_new`] used to panic if the entity was missing, and this was the non-panicking version. /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new`]. pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { - self.queue(try_insert(bundle, InsertMode::Keep)) + self.queue_with_different_failure_handling_mode( + try_insert(bundle, InsertMode::Keep), + FailureHandlingMode::Ignore, + ) } /// Removes a [`Bundle`] of components from the entity. @@ -1714,6 +1726,7 @@ impl<'a> EntityCommands<'a> { } /// Despawns the entity. + /// This will emit a warning if the entity does not exist. /// /// See [`World::despawn`] for more details. /// @@ -1741,16 +1754,15 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn despawn(&mut self) { - self.queue(despawn()); + self.queue_with_different_failure_handling_mode(despawn(), FailureHandlingMode::Warn); } /// Despawns the entity. - /// - /// [`Self::despawn`] used to warn if the entity was missing, and this was the silent version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::despawn`]. + /// 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(try_despawn()); + self.queue_with_different_failure_handling_mode(try_despawn(), FailureHandlingMode::Ignore); } /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. @@ -1775,6 +1787,22 @@ impl<'a> EntityCommands<'a> { self } + /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. + /// + /// Allows commands to use a specific failure handling mode instead of the instance's internal setting. + /// Used to avoid breaking `try_` variants and [`Self::despawn`]. + /// + /// TODO: deprecate alongside `try_` variants + pub(crate) fn queue_with_different_failure_handling_mode( + &mut self, + command: impl EntityCommand, + failure_handling_mode: FailureHandlingMode, + ) -> &mut Self { + self.commands + .queue(command.with_entity(self.entity, failure_handling_mode)); + 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. @@ -1912,11 +1940,14 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Modify the component `T` if it exists, using the function `modify`. pub fn and_modify(&mut self, modify: impl FnOnce(Mut) + Send + Sync + 'static) -> &mut Self { self.entity_commands - .queue(move |mut entity: EntityWorldMut| { - if let Some(value) = entity.get_mut() { - modify(value); - } - }); + .queue_with_different_failure_handling_mode( + move |mut entity: EntityWorldMut| { + if let Some(value) = entity.get_mut() { + modify(value); + } + }, + FailureHandlingMode::Ignore, + ); self } @@ -1939,7 +1970,10 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { #[track_caller] pub fn or_try_insert(&mut self, default: T) -> &mut Self { self.entity_commands - .queue(try_insert(default, InsertMode::Keep)); + .queue_with_different_failure_handling_mode( + try_insert(default, InsertMode::Keep), + FailureHandlingMode::Ignore, + ); self } From c8e2bae9864656d8744719945cc82b9dae4ad36c Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Mon, 2 Dec 2024 19:57:01 -0600 Subject: [PATCH 27/29] set internal commands to old behavior to avoid breaking stuff also undo `despawn` change --- crates/bevy_app/src/app.rs | 2 +- crates/bevy_audio/src/audio_output.rs | 4 ++-- .../src/contrast_adaptive_sharpening/mod.rs | 5 +++- crates/bevy_core_pipeline/src/dof/mod.rs | 2 +- .../bevy_core_pipeline/src/msaa_writeback.rs | 1 + crates/bevy_core_pipeline/src/taa/mod.rs | 2 +- crates/bevy_ecs/examples/change_detection.rs | 2 +- .../bevy_ecs/src/observer/entity_observer.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 4 ++-- crates/bevy_input/src/gamepad.rs | 2 +- crates/bevy_pbr/src/cluster/assign.rs | 1 + crates/bevy_pbr/src/cluster/mod.rs | 4 +++- crates/bevy_pbr/src/light_probe/mod.rs | 1 + crates/bevy_pbr/src/prepass/mod.rs | 2 +- crates/bevy_pbr/src/render/light.rs | 5 ++-- crates/bevy_pbr/src/ssao/mod.rs | 4 +++- crates/bevy_pbr/src/volumetric_fog/render.rs | 13 +++++++---- crates/bevy_pbr/src/wireframe.rs | 5 +++- crates/bevy_render/src/camera/camera.rs | 23 +++++++++++-------- crates/bevy_render/src/extract_component.rs | 10 ++++++-- crates/bevy_sprite/src/mesh2d/wireframe2d.rs | 5 +++- crates/bevy_ui/src/render/mod.rs | 1 + crates/bevy_ui/src/update.rs | 10 ++++++-- crates/bevy_window/src/system.rs | 2 +- crates/bevy_winit/src/system.rs | 2 +- 25 files changed, 76 insertions(+), 38 deletions(-) diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs index 70a2f7456d950..2d0ddb7645afc 100644 --- a/crates/bevy_app/src/app.rs +++ b/crates/bevy_app/src/app.rs @@ -1604,7 +1604,7 @@ mod tests { fn despawn_one_foo(mut commands: Commands, foos: Query>) { if let Some(e) = foos.iter().next() { - commands.entity(e).despawn(); + commands.entity(e).warn_if_missing().despawn(); }; } fn check_despawns(mut removed_foos: RemovedComponents) { diff --git a/crates/bevy_audio/src/audio_output.rs b/crates/bevy_audio/src/audio_output.rs index bbb9c5b682133..6e4efed251b6d 100644 --- a/crates/bevy_audio/src/audio_output.rs +++ b/crates/bevy_audio/src/audio_output.rs @@ -263,7 +263,7 @@ pub(crate) fn cleanup_finished_audio( } for (entity, sink) in &query_nonspatial_remove { if sink.sink.empty() { - commands.entity(entity).remove::<( + commands.entity(entity).ignore_if_missing().remove::<( AudioPlayer, AudioSink, PlaybackSettings, @@ -273,7 +273,7 @@ pub(crate) fn cleanup_finished_audio( } for (entity, sink) in &query_spatial_remove { if sink.sink.empty() { - commands.entity(entity).remove::<( + commands.entity(entity).ignore_if_missing().remove::<( AudioPlayer, SpatialAudioSink, PlaybackSettings, diff --git a/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs b/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs index fbc3ecfec3f75..d90231711af3d 100644 --- a/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs +++ b/crates/bevy_core_pipeline/src/contrast_adaptive_sharpening/mod.rs @@ -250,7 +250,10 @@ fn prepare_cas_pipelines( mut removals: RemovedComponents, ) { for entity in removals.read() { - commands.entity(entity).remove::(); + commands + .entity(entity) + .ignore_if_missing() + .remove::(); } for (entity, view, denoise_cas) in &views { diff --git a/crates/bevy_core_pipeline/src/dof/mod.rs b/crates/bevy_core_pipeline/src/dof/mod.rs index 06cbbe3e9d312..614b324216943 100644 --- a/crates/bevy_core_pipeline/src/dof/mod.rs +++ b/crates/bevy_core_pipeline/src/dof/mod.rs @@ -832,7 +832,7 @@ fn extract_depth_of_field_settings( // Depth of field is nonsensical without a perspective projection. let Projection::Perspective(ref perspective_projection) = *projection else { // TODO: needs better strategy for cleaning up - entity_commands.remove::<( + entity_commands.ignore_if_missing().remove::<( DepthOfField, DepthOfFieldUniform, // components added in prepare systems (because `DepthOfFieldNode` does not query extracted components) diff --git a/crates/bevy_core_pipeline/src/msaa_writeback.rs b/crates/bevy_core_pipeline/src/msaa_writeback.rs index f9c543aeff03c..4783bbd411c7f 100644 --- a/crates/bevy_core_pipeline/src/msaa_writeback.rs +++ b/crates/bevy_core_pipeline/src/msaa_writeback.rs @@ -146,6 +146,7 @@ fn prepare_msaa_writeback_pipelines( // want this to silently break commands .entity(entity) + .ignore_if_missing() .remove::(); } } diff --git a/crates/bevy_core_pipeline/src/taa/mod.rs b/crates/bevy_core_pipeline/src/taa/mod.rs index e9f8f1ac3d6a2..307264fbc377b 100644 --- a/crates/bevy_core_pipeline/src/taa/mod.rs +++ b/crates/bevy_core_pipeline/src/taa/mod.rs @@ -386,7 +386,7 @@ fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut) for (entity, age) in &entities { if age.frames > 2 { println!(" despawning {entity:?} due to age > 2"); - commands.entity(entity).despawn(); + commands.entity(entity).warn_if_missing().despawn(); } } } diff --git a/crates/bevy_ecs/src/observer/entity_observer.rs b/crates/bevy_ecs/src/observer/entity_observer.rs index a5f332e6f3b8c..ca4fa1eabdee9 100644 --- a/crates/bevy_ecs/src/observer/entity_observer.rs +++ b/crates/bevy_ecs/src/observer/entity_observer.rs @@ -34,7 +34,7 @@ impl Component for ObservedBy { // Despawn Observer if it has no more active sources. if total_entities == despawned_watched_entities { - world.commands().entity(e).despawn(); + world.commands().entity(e).warn_if_missing().despawn(); } } }); diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 6afbe56a52de5..d225f3393590d 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1754,7 +1754,7 @@ impl<'a> EntityCommands<'a> { /// ``` #[track_caller] pub fn despawn(&mut self) { - self.queue_with_different_failure_handling_mode(despawn(), FailureHandlingMode::Warn); + self.queue(despawn()); } /// Despawns the entity. @@ -2504,7 +2504,7 @@ mod tests { { let mut commands = Commands::new(&mut command_queue, &world); commands.entity(entity).despawn(); - commands.entity(entity).despawn(); // double despawn shouldn't panic + commands.entity(entity).warn_if_missing().despawn(); // shouldn't panic } command_queue.apply(&mut world); let results2 = world diff --git a/crates/bevy_input/src/gamepad.rs b/crates/bevy_input/src/gamepad.rs index 5493103a61805..a11cabb282ca6 100644 --- a/crates/bevy_input/src/gamepad.rs +++ b/crates/bevy_input/src/gamepad.rs @@ -1366,7 +1366,7 @@ pub fn gamepad_connection_system( // Gamepad entities are left alive to preserve their state (e.g. [`GamepadSettings`]). // Instead of despawning, we remove Gamepad components that don't need to preserve state // and re-add them if they ever reconnect. - gamepad.remove::(); + gamepad.ignore_if_missing().remove::(); info!("Gamepad {:} disconnected.", id); } } diff --git a/crates/bevy_pbr/src/cluster/assign.rs b/crates/bevy_pbr/src/cluster/assign.rs index 4c3e22febed65..55c08797f87b1 100644 --- a/crates/bevy_pbr/src/cluster/assign.rs +++ b/crates/bevy_pbr/src/cluster/assign.rs @@ -212,6 +212,7 @@ pub(crate) fn assign_objects_to_clusters( if visible_clusterable_objects.is_some() { commands .entity(view_entity) + .ignore_if_missing() .remove::(); } clusters.clear(); diff --git a/crates/bevy_pbr/src/cluster/mod.rs b/crates/bevy_pbr/src/cluster/mod.rs index f30dc0f432d24..ac839d6a7a60c 100644 --- a/crates/bevy_pbr/src/cluster/mod.rs +++ b/crates/bevy_pbr/src/cluster/mod.rs @@ -539,7 +539,9 @@ pub fn extract_clusters( .get_entity(entity) .expect("Clusters entity wasn't synced."); if !camera.is_active { - entity_commands.remove::<(ExtractedClusterableObjects, ExtractedClusterConfig)>(); + entity_commands + .ignore_if_missing() + .remove::<(ExtractedClusterableObjects, ExtractedClusterConfig)>(); continue; } diff --git a/crates/bevy_pbr/src/light_probe/mod.rs b/crates/bevy_pbr/src/light_probe/mod.rs index cab462c2801bf..6a2a490427e1c 100644 --- a/crates/bevy_pbr/src/light_probe/mod.rs +++ b/crates/bevy_pbr/src/light_probe/mod.rs @@ -443,6 +443,7 @@ fn gather_light_probes( commands .get_entity(view_entity) .expect("View entity wasn't synced.") + .ignore_if_missing() .remove::>(); } else { commands diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 8bd10f9678984..d74360029df34 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -597,7 +597,7 @@ pub fn extract_camera_previous_view_data( entity.insert(previous_view_data.clone()); } } else { - entity.remove::(); + entity.ignore_if_missing().remove::(); } } } diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index b307429c8ee92..4a2889adcc1f2 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -394,6 +394,7 @@ pub fn extract_lights( commands .get_entity(entity) .expect("Light entity wasn't synced.") + .ignore_if_missing() .remove::<(ExtractedDirectionalLight, RenderCascadesVisibleEntities)>(); continue; } @@ -498,7 +499,7 @@ pub(crate) fn extracted_light_removed( mut commands: Commands, ) { if let Some(mut v) = commands.get_entity(trigger.entity()) { - v.remove::(); + v.ignore_if_missing().remove::(); } } @@ -511,7 +512,7 @@ pub(crate) fn remove_light_view_entities( for v in entities.0.values() { for e in v.iter().copied() { if let Some(mut v) = commands.get_entity(e) { - v.despawn(); + v.warn_if_missing().despawn(); } } } diff --git a/crates/bevy_pbr/src/ssao/mod.rs b/crates/bevy_pbr/src/ssao/mod.rs index 366ac91e22acd..8209b6b2dce22 100644 --- a/crates/bevy_pbr/src/ssao/mod.rs +++ b/crates/bevy_pbr/src/ssao/mod.rs @@ -543,7 +543,9 @@ fn extract_ssao_settings( if camera.is_active { entity_commands.insert(ssao_settings.clone()); } else { - entity_commands.remove::(); + entity_commands + .ignore_if_missing() + .remove::(); } } } diff --git a/crates/bevy_pbr/src/volumetric_fog/render.rs b/crates/bevy_pbr/src/volumetric_fog/render.rs index 76039bbbe448b..286db59e7221e 100644 --- a/crates/bevy_pbr/src/volumetric_fog/render.rs +++ b/crates/bevy_pbr/src/volumetric_fog/render.rs @@ -279,12 +279,17 @@ pub fn extract_volumetric_fog( if volumetric_lights.is_empty() { // TODO: needs better way to handle clean up in render world for (entity, ..) in view_targets.iter() { - commands - .entity(entity) - .remove::<(VolumetricFog, ViewVolumetricFogPipelines, ViewVolumetricFog)>(); + commands.entity(entity).ignore_if_missing().remove::<( + VolumetricFog, + ViewVolumetricFogPipelines, + ViewVolumetricFog, + )>(); } for (entity, ..) in fog_volumes.iter() { - commands.entity(entity).remove::(); + commands + .entity(entity) + .ignore_if_missing() + .remove::(); } return; } diff --git a/crates/bevy_pbr/src/wireframe.rs b/crates/bevy_pbr/src/wireframe.rs index 413933135c85a..4402eaa2a4c2e 100644 --- a/crates/bevy_pbr/src/wireframe.rs +++ b/crates/bevy_pbr/src/wireframe.rs @@ -157,7 +157,9 @@ fn apply_wireframe_material( ) { for e in removed_wireframes.read().chain(no_wireframes.iter()) { if let Some(mut commands) = commands.get_entity(e) { - commands.remove::>(); + commands + .ignore_if_missing() + .remove::>(); } } @@ -199,6 +201,7 @@ fn apply_global_wireframe_material( for e in &meshes_with_global_material { commands .entity(e) + .ignore_if_missing() .remove::>(); } } diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 689734c4fba4e..28ae196c941b1 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -1056,16 +1056,19 @@ pub fn extract_cameras( ) in query.iter() { if !camera.is_active { - commands.entity(render_entity).remove::<( - ExtractedCamera, - ExtractedView, - RenderVisibleEntities, - TemporalJitter, - RenderLayers, - Projection, - GpuCulling, - ViewUniformOffset, - )>(); + commands + .entity(render_entity) + .ignore_if_missing() + .remove::<( + ExtractedCamera, + ExtractedView, + RenderVisibleEntities, + TemporalJitter, + RenderLayers, + Projection, + GpuCulling, + ViewUniformOffset, + )>(); continue; } diff --git a/crates/bevy_render/src/extract_component.rs b/crates/bevy_render/src/extract_component.rs index 64e744775ffaf..6879979efea50 100644 --- a/crates/bevy_render/src/extract_component.rs +++ b/crates/bevy_render/src/extract_component.rs @@ -207,7 +207,10 @@ fn extract_components( if let Some(component) = C::extract_component(query_item) { values.push((entity, component)); } else { - commands.entity(entity).remove::(); + commands + .entity(entity) + .ignore_if_missing() + .remove::(); } } *previous_len = values.len(); @@ -226,7 +229,10 @@ fn extract_visible_components( if let Some(component) = C::extract_component(query_item) { values.push((entity, component)); } else { - commands.entity(entity).remove::(); + commands + .entity(entity) + .ignore_if_missing() + .remove::(); } } } diff --git a/crates/bevy_sprite/src/mesh2d/wireframe2d.rs b/crates/bevy_sprite/src/mesh2d/wireframe2d.rs index 6f2659fbaaf91..cd80532a44d8b 100644 --- a/crates/bevy_sprite/src/mesh2d/wireframe2d.rs +++ b/crates/bevy_sprite/src/mesh2d/wireframe2d.rs @@ -163,7 +163,9 @@ fn apply_wireframe_material( ) { for e in removed_wireframes.read().chain(no_wireframes.iter()) { if let Some(mut commands) = commands.get_entity(e) { - commands.remove::>(); + commands + .ignore_if_missing() + .remove::>(); } } @@ -213,6 +215,7 @@ fn apply_global_wireframe_material( for e in &meshes_with_global_material { commands .entity(e) + .ignore_if_missing() .remove::>(); } } diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 8badc687714a6..72d308d867c1d 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -552,6 +552,7 @@ pub fn extract_default_ui_camera_view( commands .get_entity(entity) .expect("Camera entity wasn't synced.") + .ignore_if_missing() .remove::<(DefaultCameraView, UiAntiAlias, UiBoxShadowSamples)>(); continue; } diff --git a/crates/bevy_ui/src/update.rs b/crates/bevy_ui/src/update.rs index 049c797ea103c..ee70e6a22861a 100644 --- a/crates/bevy_ui/src/update.rs +++ b/crates/bevy_ui/src/update.rs @@ -73,7 +73,10 @@ fn update_clipping( } } else { // No inherited clipping rect, remove the component - commands.entity(entity).remove::(); + commands + .entity(entity) + .ignore_if_missing() + .remove::(); } } else if let Some(inherited_clip) = maybe_inherited_clip { // No previous calculated clip, add a new CalculatedClip component with the inherited clipping rect @@ -201,7 +204,10 @@ fn update_children_target_camera( commands.entity(child).try_insert(camera.clone()); } None => { - commands.entity(child).remove::(); + commands + .entity(child) + .ignore_if_missing() + .remove::(); } } updated_entities.insert(child); diff --git a/crates/bevy_window/src/system.rs b/crates/bevy_window/src/system.rs index d507e9e84139e..0ae701c513e43 100644 --- a/crates/bevy_window/src/system.rs +++ b/crates/bevy_window/src/system.rs @@ -46,7 +46,7 @@ pub fn close_when_requested( ) { // This was inserted by us on the last frame so now we can despawn the window for window in closing.iter() { - commands.entity(window).despawn(); + commands.entity(window).warn_if_missing().despawn(); } // Mark the window as closing so we can despawn it on the next frame for event in closed.read() { diff --git a/crates/bevy_winit/src/system.rs b/crates/bevy_winit/src/system.rs index 113e5d4a44540..3aac3344ff4ea 100644 --- a/crates/bevy_winit/src/system.rs +++ b/crates/bevy_winit/src/system.rs @@ -210,7 +210,7 @@ pub fn create_monitors( true } else { info!("Monitor removed {:?}", entity); - commands.entity(*entity).despawn(); + commands.entity(*entity).warn_if_missing().despawn(); idx += 1; false } From 0248dd1df2adcb33c9ea73bcfd7b7a35d2a4bd1f Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 4 Dec 2024 12:50:43 -0600 Subject: [PATCH 28/29] tidy up docs --- crates/bevy_ecs/examples/change_detection.rs | 2 +- crates/bevy_ecs/src/system/commands/mod.rs | 97 ++++++++++++++------ 2 files changed, 70 insertions(+), 29 deletions(-) diff --git a/crates/bevy_ecs/examples/change_detection.rs b/crates/bevy_ecs/examples/change_detection.rs index 4bf74b9206843..41300653ba6fb 100644 --- a/crates/bevy_ecs/examples/change_detection.rs +++ b/crates/bevy_ecs/examples/change_detection.rs @@ -99,7 +99,7 @@ fn remove_old_entities(mut commands: Commands, entities: Query<(Entity, &Age)>) for (entity, age) in &entities { if age.frames > 2 { println!(" despawning {entity:?} due to age > 2"); - commands.entity(entity).warn_if_missing().despawn(); + commands.entity(entity).despawn(); } } } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index d225f3393590d..8785ba392f7c2 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -1213,9 +1213,9 @@ pub trait EntityCommand: Send + 'static { /// despawned by the time the command is executed. Use the following commands to set how you /// would like subsequent commands to respond if the entity is missing: /// - [`ignore_if_missing`](Self::ignore_if_missing) -/// - [`log_if_missing`](Self::log_if_missing) (default) +/// - [`log_if_missing`](Self::log_if_missing) /// - [`warn_if_missing`](Self::warn_if_missing) -/// - [`panic_if_missing`](Self::panic_if_missing) +/// - [`panic_if_missing`](Self::panic_if_missing) (default) pub struct EntityCommands<'a> { pub(crate) entity: Entity, pub(crate) commands: Commands<'a, 'a>, @@ -1472,6 +1472,14 @@ impl<'a> EntityCommands<'a> { /// /// - [`ComponentId`] must be from the same world as `self`. /// - `T` must have the same layout as the one passed during `component_id` creation. + /// + /// # Note + /// + /// [`EntityCommands::insert_by_id`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityCommands` instance. pub unsafe fn try_insert_by_id( &mut self, component_id: ComponentId, @@ -1490,8 +1498,11 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// [`Self::insert`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert`]. + /// [`EntityCommands::insert`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityCommands` instance. /// /// # Example /// @@ -1544,6 +1555,14 @@ impl<'a> EntityCommands<'a> { /// Similar to [`Self::try_insert`] but will only try to insert if the predicate returns true. /// This is useful for chaining method calls. /// + /// # Note + /// + /// [`EntityCommands::insert_if`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityCommands` instance. + /// /// # Example /// /// ``` @@ -1591,8 +1610,11 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// [`Self::insert_if_new_and`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new_and`]. + /// [`EntityCommands::insert_if_new_and`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityCommands` instance. /// /// # Example /// @@ -1636,8 +1658,11 @@ impl<'a> EntityCommands<'a> { /// /// # Note /// - /// [`Self::insert_if_new`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::insert_if_new`]. + /// [`EntityCommands::insert_if_new`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityCommands` instance. pub fn try_insert_if_new(&mut self, bundle: impl Bundle) -> &mut Self { self.queue_with_different_failure_handling_mode( try_insert(bundle, InsertMode::Keep), @@ -1726,7 +1751,6 @@ impl<'a> EntityCommands<'a> { } /// Despawns the entity. - /// This will emit a warning if the entity does not exist. /// /// See [`World::despawn`] for more details. /// @@ -1758,8 +1782,18 @@ impl<'a> EntityCommands<'a> { } /// 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. + /// + /// This will ignore the command if the entity does not exist, + /// essentially performing the same function as [`Self::despawn`] + /// without respecting the instance's failure handling mode. + /// + /// # Note + /// + /// [`EntityCommands::despawn`] used to warn if the entity was missing, + /// and this was the silent version. + /// [`EntityCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityCommands::ignore_if_missing`] (or another variant) + /// to change the failure response for all commands from that `EntityCommands` instance. #[track_caller] pub fn try_despawn(&mut self) { self.queue_with_different_failure_handling_mode(try_despawn(), FailureHandlingMode::Ignore); @@ -1790,10 +1824,10 @@ impl<'a> EntityCommands<'a> { /// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`]. /// /// Allows commands to use a specific failure handling mode instead of the instance's internal setting. - /// Used to avoid breaking `try_` variants and [`Self::despawn`]. + /// Used to avoid breaking `try_` variants before their deprecation. /// - /// TODO: deprecate alongside `try_` variants - pub(crate) fn queue_with_different_failure_handling_mode( + /// TODO: remove alongside `try_` variants + fn queue_with_different_failure_handling_mode( &mut self, command: impl EntityCommand, failure_handling_mode: FailureHandlingMode, @@ -1940,14 +1974,11 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// Modify the component `T` if it exists, using the function `modify`. pub fn and_modify(&mut self, modify: impl FnOnce(Mut) + Send + Sync + 'static) -> &mut Self { self.entity_commands - .queue_with_different_failure_handling_mode( - move |mut entity: EntityWorldMut| { - if let Some(value) = entity.get_mut() { - modify(value); - } - }, - FailureHandlingMode::Ignore, - ); + .queue(move |mut entity: EntityWorldMut| { + if let Some(value) = entity.get_mut() { + modify(value); + } + }); self } @@ -1963,10 +1994,15 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) `default` into this entity, if `T` is not already present. /// - /// [`Self::or_insert`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert`]. - /// /// See also [`or_insert_with`](Self::or_insert_with). + /// + /// # Note + /// + /// [`EntityEntryCommands::or_insert`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityEntryCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityEntryCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityEntryCommands` instance. #[track_caller] pub fn or_try_insert(&mut self, default: T) -> &mut Self { self.entity_commands @@ -1987,10 +2023,15 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { /// [Insert](EntityCommands::insert) the value returned from `default` into this entity, if `T` is not already present. /// - /// [`Self::or_insert_with`] used to panic if the entity was missing, and this was the non-panicking version. - /// `EntityCommands` no longer need to handle missing entities individually, so just use [`Self::or_insert_with`]. + /// See also [`or_insert`](Self::or_insert). /// - /// See also [`or_insert`](Self::or_insert) and [`or_try_insert`](Self::or_try_insert). + /// # Note + /// + /// [`EntityEntryCommands::or_insert_with`] used to panic if the entity was missing, + /// and this was the non-panicking version. + /// [`EntityEntryCommands`] no longer need to handle missing entities individually; + /// you can now use [`EntityEntryCommands::ignore_if_missing`] (or another variant) + /// to disable panicking for all commands from that `EntityEntryCommands` instance. #[track_caller] pub fn or_try_insert_with(&mut self, default: impl Fn() -> T) -> &mut Self { self.or_try_insert(default()) From 771d6ff48cc232e9e15f87cb4080f72e3535a256 Mon Sep 17 00:00:00 2001 From: JaySpruce Date: Wed, 4 Dec 2024 12:59:59 -0600 Subject: [PATCH 29/29] oopsie --- crates/bevy_ecs/src/system/commands/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 1ae38ff523732..ca9d91aba4033 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -328,6 +328,7 @@ impl<'w, 's> Commands<'w, 's> { pub fn panic_on_error(&mut self) -> &mut Self { self.failure_handling_mode = FailureHandlingMode::Panic; self + } /// Clones an entity and allows configuring cloning behavior using [`EntityCloneBuilder`], returning [`EntityCommands`] of the cloned entity. ///