Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove child from parent when it is despawned #386

Merged
merged 4 commits into from
Sep 1, 2020
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 45 additions & 12 deletions crates/bevy_transform/src/hierarchy/hierarchy.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::components::Children;
use crate::components::{Children, Parent};
use bevy_ecs::{Commands, Entity, Query, World, WorldWriter};

pub fn run_on_hierarchy<T, S>(
Expand Down Expand Up @@ -44,6 +44,18 @@ pub struct DespawnRecursive {
}

fn despawn_with_children_recursive(world: &mut World, entity: Entity) {
// first, make the entity's own parent forget about it
if let Ok(parent) = world.get::<Parent>(entity) {
if let Ok(mut children) = world.get_mut::<Children>(parent.0) {
children.0.retain(|c| *c != entity);
CleanCut marked this conversation as resolved.
Show resolved Hide resolved
}
}
// then despawn the entity and all of its children
despawn_with_children_recursive_inner(world, entity);
}

// Should only be called by `despawn_with_children_recursive`!
fn despawn_with_children_recursive_inner(world: &mut World, entity: Entity) {
if let Some(children) = world
.get::<Children>(entity)
.ok()
Expand Down Expand Up @@ -78,32 +90,41 @@ impl DespawnRecursiveExt for Commands {
#[cfg(test)]
mod tests {
use super::DespawnRecursiveExt;
use crate::hierarchy::BuildChildren;
use bevy_ecs::{Commands, Entity, Resources, World};
use crate::{components::Children, hierarchy::BuildChildren};
use bevy_ecs::{Commands, Resources, World};

#[test]
fn despawn_recursive() {
let mut world = World::default();
let mut resources = Resources::default();
let mut command_buffer = Commands::default();
let parent_entity = Entity::new();

command_buffer.spawn((0u32, 0u64)).with_children(|parent| {
parent.spawn((0u32, 0u64));
});

command_buffer
.spawn_as_entity(parent_entity, (1u32, 2u64))
.with_children(|parent| {
parent.spawn((1u32, 2u64)).with_children(|parent| {
parent.spawn((1u32, 2u64));
// Create a grandparent entity which will _not_ be deleted
command_buffer.spawn((1u32, 1u64));
let grandparent_entity = command_buffer.current_entity().unwrap();

command_buffer.with_children(|parent| {
// Add a child to the grandparent (the "parent"), which will get deleted
parent.spawn((2u32, 2u64));
// All descendents of the "parent" should also be deleted.
parent.with_children(|parent| {
parent.spawn((3u32, 3u64)).with_children(|parent| {
// child
parent.spawn((4u32, 4u64));
});
parent.spawn((1u32, 2u64));
parent.spawn((5u32, 5u64));
});
});

command_buffer.spawn((0u32, 0u64));
command_buffer.apply(&mut world, &mut resources);

let parent_entity = world.get::<Children>(grandparent_entity).unwrap()[0];

command_buffer.despawn_recursive(parent_entity);
command_buffer.apply(&mut world, &mut resources);

Expand All @@ -113,8 +134,20 @@ mod tests {
.map(|(a, b)| (*a, *b))
.collect::<Vec<_>>();

{
let children = world.get::<Children>(grandparent_entity).unwrap();
assert_eq!(
children.iter().any(|&i| i == parent_entity),
false,
"grandparent should no longer know about its child which has been removed"
);
}

// parent_entity and its children should be deleted,
// the (0, 0) tuples remaining.
assert_eq!(results, vec![(0u32, 0u64), (0u32, 0u64), (0u32, 0u64)]);
// the grandparent tuple (1, 1) and (0, 0) tuples remaining.
assert_eq!(
results,
vec![(0u32, 0u64), (0u32, 0u64), (0u32, 0u64), (1u32, 1u64)]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is the order of entities stable? If not, then we should probably alter the grandparent to also be (0u32, 0u64) to avoid a flaky test.

CleanCut marked this conversation as resolved.
Show resolved Hide resolved
);
}
}