From 6506b09ba7e2088194de5721e3260e74dd6d7a00 Mon Sep 17 00:00:00 2001 From: Ethereumdegen Date: Wed, 20 Sep 2023 15:34:30 -0400 Subject: [PATCH] add try_insert to entity commands (#9844) # Objective - I spoke with some users in the ECS channel of bevy discord today and they suggested that I implement a fallible form of .insert for components. - In my opinion, it would be nice to have a fallible .insert like .try_insert (or to just make insert be fallible!) because it was causing a lot of panics in my game. In my game, I am spawning terrain chunks and despawning them in the Update loop. However, this was causing bevy_xpbd to panic because it was trying to .insert some physics components on my chunks and a race condition meant that its check to see if the entity exists would pass but then the next execution step it would not exist and would do an .insert and then panic. This means that there is no way to avoid a panic with conditionals. Luckily, bevy_xpbd does not care about inserting these components if the entity is being deleted and so if there were a .try_insert, like this PR provides it could use that instead in order to NOT panic. ( My interim solution for my own game has been to run the entity despawn events in the Last schedule but really this is just a hack and I should not be expected to manage the scheduling of despawns like this - it should just be easy and simple. IF it just so happened that bevy_xpbd ran .inserts in the Last schedule also, this would be an untenable soln overall ) ## Solution - Describe the solution used to achieve the objective above. Add a new command named TryInsert (entitycommands.try_insert) which functions exactly like .insert except if the entity does not exist it will not panic. Instead, it will log to info. This way, crates that are attaching components in ways which they do not mind that the entity no longer exists can just use try_insert instead of insert. --- ## Changelog ## Additional Thoughts In my opinion, NOT panicing should really be the default and having an .insert that does panic should be the odd edgecase but removing the panic! from .insert seems a bit above my paygrade -- although i would love to see it. My other thought is it would be good for .insert to return an Option AND not panic but it seems it uses an event bus right now so that seems to be impossible w the current architecture. --- crates/bevy_ecs/src/system/commands/mod.rs | 77 ++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 512dadff88017..bf9945c347e23 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -680,6 +680,8 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { /// /// 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 /// /// ``` @@ -729,6 +731,62 @@ impl<'w, 's, 'a> EntityCommands<'w, 's, 'a> { self } + /// Tries to add a [`Bundle`] of components to the entity. + /// + /// This will overwrite any previous value(s) of the same component type. + /// + /// # Note + /// + /// Unlike [`Self::insert`], this will not panic if the associated entity does not exist. + /// + /// # Example + /// + /// ``` + /// # use bevy_ecs::prelude::*; + /// # #[derive(Resource)] + /// # struct PlayerEntity { entity: Entity } + /// #[derive(Component)] + /// struct Health(u32); + /// #[derive(Component)] + /// struct Strength(u32); + /// #[derive(Component)] + /// struct Defense(u32); + /// + /// #[derive(Bundle)] + /// struct CombatBundle { + /// health: Health, + /// strength: Strength, + /// } + /// + /// fn add_combat_stats_system(mut commands: Commands, player: Res) { + /// commands.entity(player.entity) + /// // You can try_insert individual components: + /// .try_insert(Defense(10)) + /// + /// // You can also insert tuples of components: + /// .try_insert(CombatBundle { + /// health: Health(100), + /// strength: Strength(40), + /// }); + /// + /// // Suppose this occurs in a parallel adjacent system or process + /// commands.entity(player.entity) + /// .despawn(); + /// + /// commands.entity(player.entity) + /// // This will not panic nor will it add the component + /// .try_insert(Defense(5)); + /// } + /// # bevy_ecs::system::assert_is_system(add_combat_stats_system); + /// ``` + pub fn try_insert(&mut self, bundle: impl Bundle) -> &mut Self { + self.commands.add(TryInsert { + entity: self.entity, + bundle, + }); + self + } + /// Removes a [`Bundle`] of components from the entity. /// /// See [`EntityWorldMut::remove`](crate::world::EntityWorldMut::remove) for more @@ -966,6 +1024,25 @@ where } } +/// A [`Command`] that attempts to add the components in a [`Bundle`] to an entity. +pub struct TryInsert { + /// The entity to which the components will be added. + pub entity: Entity, + /// The [`Bundle`] containing the components that will be added to the entity. + pub bundle: T, +} + +impl Command for TryInsert +where + T: Bundle + 'static, +{ + fn apply(self, world: &mut World) { + if let Some(mut entity) = world.get_entity_mut(self.entity) { + entity.insert(self.bundle); + } + } +} + /// A [`Command`] that removes components from an entity. /// 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.