Skip to content

Commit

Permalink
Add try_despawn methods to World/Commands (bevyengine#15480)
Browse files Browse the repository at this point in the history
# Objective

Fixes bevyengine#14511.

`despawn` allows you to remove entities from the world. However, if the
entity does not exist, it emits a warning. This may not be intended
behavior for many users who have use cases where they need to call
`despawn` regardless of if the entity actually exists (see the issue),
or don't care in general if the entity already doesn't exist.

(Also trying to gauge interest on if this feature makes sense, I'd
personally love to have it, but I could see arguments that this might be
a footgun. Just trying to help here 😄 If there's no contention I could
also implement this for `despawn_recursive` and `despawn_descendants` in
the same PR)

## Solution

Add `try_despawn`, `try_despawn_recursive` and
`try_despawn_descendants`.

Modify `World::despawn_with_caller` to also take in a `warn` boolean
argument, which is then considered when logging the warning. Set
`log_warning` to `true` in the case of `despawn`, and `false` in the
case of `try_despawn`.

## Testing

Ran `cargo run -p ci` on macOS, it seemed fine.
  • Loading branch information
rudderbucky authored and ItsDoot committed Oct 4, 2024
1 parent 785faf7 commit 4cdf6e2
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 40 deletions.
2 changes: 1 addition & 1 deletion benches/benches/bevy_ecs/world/despawn_recursive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub fn world_despawn_recursive(criterion: &mut Criterion) {
group.bench_function(format!("{}_entities", entity_count), |bencher| {
bencher.iter(|| {
ents.iter().for_each(|e| {
despawn_with_children_recursive(&mut world, *e);
despawn_with_children_recursive(&mut world, *e, true);
});
});
});
Expand Down
25 changes: 24 additions & 1 deletion crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,14 @@ impl EntityCommands<'_> {
self.queue(despawn());
}

/// Despawns the entity.
/// This will not emit a warning if the entity does not exist, essentially performing
/// the same function as [`Self::despawn`] without emitting warnings.
#[track_caller]
pub fn try_despawn(&mut self) {
self.queue(try_despawn());
}

/// Pushes an [`EntityCommand`] to the queue, which will get executed for the current [`Entity`].
///
/// # Examples
Expand Down Expand Up @@ -1697,7 +1705,22 @@ where
fn despawn() -> impl EntityCommand {
let caller = Location::caller();
move |entity: Entity, world: &mut World| {
world.despawn_with_caller(entity, caller);
world.despawn_with_caller(entity, caller, true);
}
}

/// A [`Command`] that despawns a specific entity.
/// This will not emit a warning if the entity does not exist.
///
/// # Note
///
/// This won't clean up external references to the entity (such as parent-child relationships
/// if you're using `bevy_hierarchy`), which may leave the world in an invalid state.
#[track_caller]
fn try_despawn() -> impl EntityCommand {
let caller = Location::caller();
move |entity: Entity, world: &mut World| {
world.despawn_with_caller(entity, caller, false);
}
}

Expand Down
15 changes: 13 additions & 2 deletions crates/bevy_ecs/src/world/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1504,21 +1504,32 @@ impl World {
#[track_caller]
#[inline]
pub fn despawn(&mut self, entity: Entity) -> bool {
self.despawn_with_caller(entity, Location::caller())
self.despawn_with_caller(entity, Location::caller(), true)
}

/// Performs the same function as [`Self::despawn`] but does not emit a warning if
/// the entity does not exist.
#[track_caller]
#[inline]
pub fn try_despawn(&mut self, entity: Entity) -> bool {
self.despawn_with_caller(entity, Location::caller(), false)
}

#[inline]
pub(crate) fn despawn_with_caller(
&mut self,
entity: Entity,
caller: &'static Location,
log_warning: bool,
) -> bool {
self.flush();
if let Ok(entity) = self.get_entity_mut(entity) {
entity.despawn();
true
} else {
warn!("error[B0003]: {caller}: Could not despawn entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", entity);
if log_warning {
warn!("error[B0003]: {caller}: Could not despawn entity {:?} because it doesn't exist in this World. See: https://bevyengine.org/learn/errors/b0003", entity);
}
false
}
}
Expand Down
131 changes: 96 additions & 35 deletions crates/bevy_hierarchy/src/hierarchy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,21 @@ use bevy_utils::tracing::debug;
pub struct DespawnRecursive {
/// Target entity
pub entity: Entity,
/// Whether or not this command should output a warning if the entity does not exist
pub warn: bool,
}

/// Despawns the given entity's children recursively
#[derive(Debug)]
pub struct DespawnChildrenRecursive {
/// Target entity
pub entity: Entity,
/// Whether or not this command should output a warning if the entity does not exist
pub warn: bool,
}

/// Function for despawning an entity and all its children
pub fn despawn_with_children_recursive(world: &mut World, entity: Entity) {
pub fn despawn_with_children_recursive(world: &mut World, entity: Entity, warn: bool) {
// first, make the entity's own parent forget about it
if let Some(parent) = world.get::<Parent>(entity).map(|parent| parent.0) {
if let Some(mut children) = world.get_mut::<Children>(parent) {
Expand All @@ -30,26 +34,30 @@ pub fn despawn_with_children_recursive(world: &mut World, entity: Entity) {
}

// then despawn the entity and all of its children
despawn_with_children_recursive_inner(world, entity);
despawn_with_children_recursive_inner(world, entity, warn);
}

// Should only be called by `despawn_with_children_recursive`!
fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity) {
// Should only be called by `despawn_with_children_recursive` and `try_despawn_with_children_recursive`!
fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity, warn: bool) {
if let Some(mut children) = world.get_mut::<Children>(entity) {
for e in core::mem::take(&mut children.0) {
despawn_with_children_recursive_inner(world, e);
despawn_with_children_recursive_inner(world, e, warn);
}
}

if !world.despawn(entity) {
if warn {
if !world.despawn(entity) {
debug!("Failed to despawn entity {:?}", entity);
}
} else if !world.try_despawn(entity) {
debug!("Failed to despawn entity {:?}", entity);
}
}

fn despawn_children_recursive(world: &mut World, entity: Entity) {
fn despawn_children_recursive(world: &mut World, entity: Entity, warn: bool) {
if let Some(children) = world.entity_mut(entity).take::<Children>() {
for e in children.0 {
despawn_with_children_recursive_inner(world, e);
despawn_with_children_recursive_inner(world, e, warn);
}
}
}
Expand All @@ -60,10 +68,11 @@ impl Command for DespawnRecursive {
let _span = bevy_utils::tracing::info_span!(
"command",
name = "DespawnRecursive",
entity = bevy_utils::tracing::field::debug(self.entity)
entity = bevy_utils::tracing::field::debug(self.entity),
warn = bevy_utils::tracing::field::debug(self.warn)
)
.entered();
despawn_with_children_recursive(world, self.entity);
despawn_with_children_recursive(world, self.entity, self.warn);
}
}

Expand All @@ -73,10 +82,12 @@ impl Command for DespawnChildrenRecursive {
let _span = bevy_utils::tracing::info_span!(
"command",
name = "DespawnChildrenRecursive",
entity = bevy_utils::tracing::field::debug(self.entity)
entity = bevy_utils::tracing::field::debug(self.entity),
warn = bevy_utils::tracing::field::debug(self.warn)
)
.entered();
despawn_children_recursive(world, self.entity);

despawn_children_recursive(world, self.entity, self.warn);
}
}

Expand All @@ -87,53 +98,103 @@ pub trait DespawnRecursiveExt {

/// Despawns all descendants of the given entity.
fn despawn_descendants(&mut self) -> &mut Self;

/// Similar to [`Self::despawn_recursive`] but does not emit warnings
fn try_despawn_recursive(self);

/// Similar to [`Self::despawn_descendants`] but does not emit warnings
fn try_despawn_descendants(&mut self) -> &mut Self;
}

impl DespawnRecursiveExt for EntityCommands<'_> {
/// Despawns the provided entity and its children.
/// This will emit warnings for any entity that does not exist.
fn despawn_recursive(mut self) {
let entity = self.id();
self.commands().queue(DespawnRecursive { entity });
self.commands()
.queue(DespawnRecursive { entity, warn: true });
}

fn despawn_descendants(&mut self) -> &mut Self {
let entity = self.id();
self.commands().queue(DespawnChildrenRecursive { entity });
self.commands()
.queue(DespawnChildrenRecursive { entity, warn: true });
self
}

/// Despawns the provided entity and its children.
/// This will never emit warnings.
fn try_despawn_recursive(mut self) {
let entity = self.id();
self.commands().queue(DespawnRecursive {
entity,
warn: false,
});
}

fn try_despawn_descendants(&mut self) -> &mut Self {
let entity = self.id();
self.commands().queue(DespawnChildrenRecursive {
entity,
warn: false,
});
self
}
}

fn despawn_recursive_inner(world: EntityWorldMut, warn: bool) {
let entity = world.id();

#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!(
"despawn_recursive",
entity = bevy_utils::tracing::field::debug(entity),
warn = bevy_utils::tracing::field::debug(warn)
)
.entered();

despawn_with_children_recursive(world.into_world_mut(), entity, warn);
}

fn despawn_descendants_inner<'v, 'w>(
world: &'v mut EntityWorldMut<'w>,
warn: bool,
) -> &'v mut EntityWorldMut<'w> {
let entity = world.id();

#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!(
"despawn_descendants",
entity = bevy_utils::tracing::field::debug(entity),
warn = bevy_utils::tracing::field::debug(warn)
)
.entered();

world.world_scope(|world| {
despawn_children_recursive(world, entity, warn);
});
world
}

impl<'w> DespawnRecursiveExt for EntityWorldMut<'w> {
/// Despawns the provided entity and its children.
/// This will emit warnings for any entity that does not exist.
fn despawn_recursive(self) {
let entity = self.id();

#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!(
"despawn_recursive",
entity = bevy_utils::tracing::field::debug(entity)
)
.entered();

despawn_with_children_recursive(self.into_world_mut(), entity);
despawn_recursive_inner(self, true);
}

fn despawn_descendants(&mut self) -> &mut Self {
let entity = self.id();
despawn_descendants_inner(self, true)
}

#[cfg(feature = "trace")]
let _span = bevy_utils::tracing::info_span!(
"despawn_descendants",
entity = bevy_utils::tracing::field::debug(entity)
)
.entered();
/// Despawns the provided entity and its children.
/// This will not emit warnings.
fn try_despawn_recursive(self) {
despawn_recursive_inner(self, false);
}

self.world_scope(|world| {
despawn_children_recursive(world, entity);
});
self
fn try_despawn_descendants(&mut self) -> &mut Self {
despawn_descendants_inner(self, false)
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ui/src/layout/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ mod tests {
}

// despawn the parent entity and its descendants
despawn_with_children_recursive(&mut world, ui_parent_entity);
despawn_with_children_recursive(&mut world, ui_parent_entity, true);

ui_schedule.run(&mut world);

Expand Down

0 comments on commit 4cdf6e2

Please sign in to comment.