Skip to content

Commit

Permalink
add try_insert to entity commands (bevyengine#9844)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
ethereumdegen authored and Ray Redondo committed Jan 9, 2024
1 parent 071d564 commit 6506b09
Showing 1 changed file with 77 additions and 0 deletions.
77 changes: 77 additions & 0 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
///
/// ```
Expand Down Expand Up @@ -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<PlayerEntity>) {
/// 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
Expand Down Expand Up @@ -966,6 +1024,25 @@ where
}
}

/// A [`Command`] that attempts to add the components in a [`Bundle`] to an entity.
pub struct TryInsert<T> {
/// 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<T> Command for TryInsert<T>
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.
Expand Down

0 comments on commit 6506b09

Please sign in to comment.