Skip to content

Commit

Permalink
fix parenting of scenes (#2410)
Browse files Browse the repository at this point in the history
# Objective

Fix #2406 

Scene parenting was not done completely, leaving the hierarchy maintenance to the standard system. As scene spawning happens in stage `PreUpdate` and hierarchy maintenance in stage `PostUpdate`, this left the scene in an invalid state parent wise for part of a frame

## Solution

Also add/update the `Children` component when spawning the scene.

I kept the `Children` component as a `SmallVec`, it could be moved to an `HashSet` to guarantee uniqueness


Co-authored-by: François <8672791+mockersf@users.noreply.github.com>
  • Loading branch information
mockersf and mockersf committed Dec 23, 2021
1 parent 79d36e7 commit 1103834
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 9 deletions.
21 changes: 17 additions & 4 deletions crates/bevy_scene/src/scene_spawner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ use bevy_asset::{AssetEvent, Assets, Handle};
use bevy_ecs::{
entity::{Entity, EntityMap},
reflect::{ReflectComponent, ReflectMapEntities},
system::Command,
world::{Mut, World},
};
use bevy_reflect::TypeRegistryArc;
use bevy_transform::prelude::Parent;
use bevy_transform::{hierarchy::AddChild, prelude::Parent};
use bevy_utils::{tracing::error, HashMap};
use thiserror::Error;
use uuid::Uuid;
Expand Down Expand Up @@ -268,10 +269,22 @@ impl SceneSpawner {
for (instance_id, parent) in scenes_with_parent {
if let Some(instance) = self.spawned_instances.get(&instance_id) {
for entity in instance.entity_map.values() {
if let Some(mut entity_mut) = world.get_entity_mut(entity) {
if !entity_mut.contains::<Parent>() {
entity_mut.insert(Parent(parent));
// Add the `Parent` component to the scene root, and update the `Children` component of
// the scene parent
if !world
.get_entity(entity)
// This will filter only the scene root entity, as all other from the
// scene have a parent
.map(|entity| entity.contains::<Parent>())
// Default is true so that it won't run on an entity that wouldn't exist anymore
// this case shouldn't happen anyway
.unwrap_or(true)
{
AddChild {
parent,
child: entity,
}
.write(world);
}
}
} else {
Expand Down
29 changes: 29 additions & 0 deletions crates/bevy_transform/src/hierarchy/child_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,28 @@ use bevy_ecs::{
};
use smallvec::SmallVec;

#[derive(Debug)]
pub struct AddChild {
pub parent: Entity,
pub child: Entity,
}

impl Command for AddChild {
fn write(self, world: &mut World) {
world
.entity_mut(self.child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(self.parent), PreviousParent(self.parent)));
if let Some(mut children) = world.get_mut::<Children>(self.parent) {
children.0.push(self.child);
} else {
world
.entity_mut(self.parent)
.insert(Children(smallvec::smallvec![self.child]));
}
}
}

#[derive(Debug)]
pub struct InsertChildren {
parent: Entity,
Expand Down Expand Up @@ -134,6 +156,7 @@ pub trait BuildChildren {
fn push_children(&mut self, children: &[Entity]) -> &mut Self;
fn insert_children(&mut self, index: usize, children: &[Entity]) -> &mut Self;
fn remove_children(&mut self, children: &[Entity]) -> &mut Self;
fn add_child(&mut self, child: Entity) -> &mut Self;
}

impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> {
Expand Down Expand Up @@ -182,6 +205,12 @@ impl<'w, 's, 'a> BuildChildren for EntityCommands<'w, 's, 'a> {
});
self
}

fn add_child(&mut self, child: Entity) -> &mut Self {
let parent = self.id();
self.commands().add(AddChild { child, parent });
self
}
}

#[derive(Debug)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,10 @@ pub fn parent_update_system(
// `children_additions`).
if let Ok(mut new_parent_children) = children_query.get_mut(parent.0) {
// This is the parent
debug_assert!(
!(*new_parent_children).0.contains(&entity),
"children already added"
);
(*new_parent_children).0.push(entity);
// PERF: Ideally we shouldn't need to check for duplicates
if !(*new_parent_children).0.contains(&entity) {
(*new_parent_children).0.push(entity);
}
} else {
// The parent doesn't have a children entity, lets add it
children_additions
Expand Down

0 comments on commit 1103834

Please sign in to comment.