Hierarchy commandization (#4197)

## Objective
Implement absolute minimum viable product for the changes proposed in bevyengine/rfcs#53.

## Solution

 - Remove public mutative access to `Parent` (Children is already publicly read-only). This includes public construction methods like `Copy`, `Clone`, and `Default`.
 - Remove `PreviousParent`
 - Remove `parent_update_system`
 - Update all hierarchy related commands to immediately update both `Parent` and `Children` references.

## Remaining TODOs

 - [ ] Update documentation for both `Parent` and `Children`. Discourage using `EntityCommands::remove`
 - [x] Add `HierarchyEvent` to notify listeners of hierarchy updates. This is meant to replace listening on `PreviousParent`

## Followup

 - These changes should be best moved to the hooks mentioned in #3742.
 - Backing storage for both might be best moved to indexes mentioned in the same relations.
This commit is contained in:
James Liu 2022-07-10 20:29:06 +00:00
parent 518408dfda
commit 8eb0440f1e
13 changed files with 315 additions and 441 deletions

View file

@ -15,7 +15,7 @@ use bevy_ecs::{
schedule::ParallelSystemDescriptorCoercion,
system::{Query, Res},
};
use bevy_hierarchy::{Children, HierarchySystem};
use bevy_hierarchy::Children;
use bevy_math::{Quat, Vec3};
use bevy_reflect::{Reflect, TypeUuid};
use bevy_time::Time;
@ -295,9 +295,7 @@ impl Plugin for AnimationPlugin {
.register_type::<AnimationPlayer>()
.add_system_to_stage(
CoreStage::PostUpdate,
animation_player
.before(TransformSystem::TransformPropagate)
.after(HierarchySystem::ParentUpdate),
animation_player.before(TransformSystem::TransformPropagate),
);
}
}

View file

@ -1,13 +1,88 @@
use smallvec::SmallVec;
use crate::{
prelude::{Children, Parent},
HierarchyEvent,
};
use bevy_ecs::{
bundle::Bundle,
entity::Entity,
event::Events,
system::{Command, Commands, EntityCommands},
world::{EntityMut, World},
};
use smallvec::SmallVec;
use crate::prelude::{Children, Parent, PreviousParent};
fn push_events(world: &mut World, events: SmallVec<[HierarchyEvent; 8]>) {
if let Some(mut moved) = world.get_resource_mut::<Events<HierarchyEvent>>() {
for evt in events {
moved.send(evt);
}
}
}
fn push_child_unchecked(world: &mut World, parent: Entity, child: Entity) {
let mut parent = world.entity_mut(parent);
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.push(child);
} else {
parent.insert(Children(smallvec::smallvec![child]));
}
}
fn update_parent(world: &mut World, child: Entity, new_parent: Entity) -> Option<Entity> {
let mut child = world.entity_mut(child);
if let Some(mut parent) = child.get_mut::<Parent>() {
let previous = parent.0;
*parent = Parent(new_parent);
Some(previous)
} else {
child.insert(Parent(new_parent));
None
}
}
fn remove_from_children(world: &mut World, parent: Entity, child: Entity) {
let mut parent = world.entity_mut(parent);
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.retain(|x| *x != child);
if children.is_empty() {
parent.remove::<Children>();
}
}
}
fn update_old_parents(world: &mut World, parent: Entity, children: &[Entity]) {
let mut moved: SmallVec<[HierarchyEvent; 8]> = SmallVec::with_capacity(children.len());
for child in children {
if let Some(previous) = update_parent(world, *child, parent) {
debug_assert!(parent != previous);
remove_from_children(world, previous, *child);
moved.push(HierarchyEvent::ChildMoved {
child: *child,
previous_parent: previous,
new_parent: parent,
});
}
}
push_events(world, moved);
}
fn remove_children(parent: Entity, children: &[Entity], world: &mut World) {
let mut events: SmallVec<[HierarchyEvent; 8]> = SmallVec::new();
for child in children {
world.entity_mut(*child).remove::<Parent>();
events.push(HierarchyEvent::ChildRemoved {
child: *child,
parent,
});
}
push_events(world, events);
if let Some(mut parent_children) = world.get_mut::<Children>(parent) {
parent_children
.0
.retain(|parent_child| !children.contains(parent_child));
}
}
/// Command that adds a child to an entity
#[derive(Debug)]
@ -20,16 +95,32 @@ pub struct AddChild {
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);
let previous = update_parent(world, self.child, self.parent);
if let Some(previous) = previous {
if previous == self.parent {
return;
}
remove_from_children(world, previous, self.child);
if let Some(mut events) = world.get_resource_mut::<Events<HierarchyEvent>>() {
events.send(HierarchyEvent::ChildMoved {
child: self.child,
previous_parent: previous,
new_parent: self.parent,
});
}
} else if let Some(mut events) = world.get_resource_mut::<Events<HierarchyEvent>>() {
events.send(HierarchyEvent::ChildAdded {
child: self.child,
parent: self.parent,
});
}
let mut parent = world.entity_mut(self.parent);
if let Some(mut children) = parent.get_mut::<Children>() {
if !children.contains(&self.child) {
children.0.push(self.child);
}
} else {
world
.entity_mut(self.parent)
.insert(Children(smallvec::smallvec![self.child]));
parent.insert(Children(smallvec::smallvec![self.child]));
}
}
}
@ -44,20 +135,13 @@ pub struct InsertChildren {
impl Command for InsertChildren {
fn write(self, world: &mut World) {
for child in self.children.iter() {
world
.entity_mut(*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.insert_from_slice(self.index, &self.children);
} else {
world
.entity_mut(self.parent)
.insert(Children(self.children));
}
update_old_parents(world, self.parent, &self.children);
let mut parent = world.entity_mut(self.parent);
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.retain(|value| !self.children.contains(value));
children.0.insert_from_slice(self.index, &self.children);
} else {
parent.insert(Children(self.children));
}
}
}
@ -70,27 +154,14 @@ pub struct PushChildren {
}
impl Command for PushChildren {
fn write(self, world: &mut World) {
for child in self.children.iter() {
world
.entity_mut(*child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(self.parent), PreviousParent(self.parent)));
}
{
let mut added = false;
if let Some(mut children) = world.get_mut::<Children>(self.parent) {
children.0.extend(self.children.iter().cloned());
added = true;
}
// NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails
// borrow-checking
if !added {
world
.entity_mut(self.parent)
.insert(Children(self.children));
}
fn write(mut self, world: &mut World) {
update_old_parents(world, self.parent, &self.children);
let mut parent = world.entity_mut(self.parent);
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.retain(|child| !self.children.contains(child));
children.0.append(&mut self.children);
} else {
parent.insert(Children(self.children));
}
}
}
@ -101,32 +172,8 @@ pub struct RemoveChildren {
children: SmallVec<[Entity; 8]>,
}
fn remove_children(parent: Entity, children: &[Entity], world: &mut World) {
for child in children.iter() {
let mut child = world.entity_mut(*child);
let mut remove_parent = false;
if let Some(child_parent) = child.get_mut::<Parent>() {
if child_parent.0 == parent {
remove_parent = true;
}
}
if remove_parent {
if let Some(parent) = child.remove::<Parent>() {
child.insert(PreviousParent(parent.0));
}
}
}
// Remove the children from the parents.
if let Some(mut parent_children) = world.get_mut::<Children>(parent) {
parent_children
.0
.retain(|parent_child| !children.contains(parent_child));
}
}
impl Command for RemoveChildren {
fn write(self, world: &mut World) {
// Remove any matching Parent components from the children
remove_children(self.parent, &self.children, world);
}
}
@ -285,15 +332,15 @@ impl<'w> WorldChildBuilder<'w> {
.world
.spawn()
.insert_bundle(bundle)
.insert_bundle((Parent(parent_entity), PreviousParent(parent_entity)))
.insert(Parent(parent_entity))
.id();
push_child_unchecked(self.world, parent_entity, entity);
self.current_entity = Some(entity);
if let Some(mut parent) = self.world.get_entity_mut(parent_entity) {
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.push(entity);
} else {
parent.insert(Children(smallvec::smallvec![entity]));
}
if let Some(mut added) = self.world.get_resource_mut::<Events<HierarchyEvent>>() {
added.send(HierarchyEvent::ChildAdded {
child: entity,
parent: parent_entity,
});
}
self.world.entity_mut(entity)
}
@ -301,18 +348,14 @@ impl<'w> WorldChildBuilder<'w> {
/// Spawns an [`Entity`] with no components and inserts it into the children defined by the [`WorldChildBuilder`] which adds the [`Parent`] component to it.
pub fn spawn(&mut self) -> EntityMut<'_> {
let parent_entity = self.parent_entity();
let entity = self
.world
.spawn()
.insert_bundle((Parent(parent_entity), PreviousParent(parent_entity)))
.id();
let entity = self.world.spawn().insert(Parent(parent_entity)).id();
push_child_unchecked(self.world, parent_entity, entity);
self.current_entity = Some(entity);
if let Some(mut parent) = self.world.get_entity_mut(parent_entity) {
if let Some(mut children) = parent.get_mut::<Children>() {
children.0.push(entity);
} else {
parent.insert(Children(smallvec::smallvec![entity]));
}
if let Some(mut added) = self.world.get_resource_mut::<Events<HierarchyEvent>>() {
added.send(HierarchyEvent::ChildAdded {
child: entity,
parent: parent_entity,
});
}
self.world.entity_mut(entity)
}
@ -361,16 +404,14 @@ impl<'w> BuildWorldChildren for EntityMut<'w> {
{
// SAFETY: parent entity is not modified and its location is updated manually
let world = unsafe { self.world_mut() };
for child in children.iter() {
world
.entity_mut(*child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(parent), PreviousParent(parent)));
}
update_old_parents(world, parent, children);
// Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype
self.update_location();
}
if let Some(mut children_component) = self.get_mut::<Children>() {
children_component
.0
.retain(|value| !children.contains(value));
children_component.0.extend(children.iter().cloned());
} else {
self.insert(Children::with(children));
@ -383,17 +424,15 @@ impl<'w> BuildWorldChildren for EntityMut<'w> {
{
// SAFETY: parent entity is not modified and its location is updated manually
let world = unsafe { self.world_mut() };
for child in children.iter() {
world
.entity_mut(*child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(parent), PreviousParent(parent)));
}
update_old_parents(world, parent, children);
// Inserting a bundle in the children entities may change the parent entity's location if they were of the same archetype
self.update_location();
}
if let Some(mut children_component) = self.get_mut::<Children>() {
children_component
.0
.retain(|value| !children.contains(value));
children_component.0.insert_from_slice(index, children);
} else {
self.insert(Children::with(children));
@ -405,26 +444,7 @@ impl<'w> BuildWorldChildren for EntityMut<'w> {
let parent = self.id();
// SAFETY: This doesn't change the parent's location
let world = unsafe { self.world_mut() };
for child in children.iter() {
let mut child = world.entity_mut(*child);
let mut remove_parent = false;
if let Some(child_parent) = child.get_mut::<Parent>() {
if child_parent.0 == parent {
remove_parent = true;
}
}
if remove_parent {
if let Some(parent) = child.remove::<Parent>() {
child.insert(PreviousParent(parent.0));
}
}
}
// Remove the children from the parents.
if let Some(mut parent_children) = world.get_mut::<Children>(parent) {
parent_children
.0
.retain(|parent_child| !children.contains(parent_child));
}
remove_children(parent, children, world);
self
}
}
@ -450,13 +470,11 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> {
let parent = self
.current_entity
.expect("Cannot add children without a parent. Try creating an entity first.");
for child in children.iter() {
self.world
.entity_mut(*child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(parent), PreviousParent(parent)));
}
update_old_parents(self.world, parent, children);
if let Some(mut children_component) = self.world.get_mut::<Children>(parent) {
children_component
.0
.retain(|value| !children.contains(value));
children_component.0.extend(children.iter().cloned());
} else {
self.world
@ -470,14 +488,11 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> {
let parent = self
.current_entity
.expect("Cannot add children without a parent. Try creating an entity first.");
for child in children.iter() {
self.world
.entity_mut(*child)
// FIXME: don't erase the previous parent (see #1545)
.insert_bundle((Parent(parent), PreviousParent(parent)));
}
update_old_parents(self.world, parent, children);
if let Some(mut children_component) = self.world.get_mut::<Children>(parent) {
children_component
.0
.retain(|value| !children.contains(value));
children_component.0.insert_from_slice(index, children);
} else {
self.world
@ -499,6 +514,8 @@ impl<'w> BuildWorldChildren for WorldChildBuilder<'w> {
#[cfg(test)]
mod tests {
use super::{BuildChildren, BuildWorldChildren};
use crate::prelude::{Children, Parent};
use smallvec::{smallvec, SmallVec};
use bevy_ecs::{
@ -508,10 +525,6 @@ mod tests {
world::World,
};
use crate::prelude::{Children, Parent, PreviousParent};
use super::{BuildChildren, BuildWorldChildren};
#[derive(Component)]
struct C(u32);
@ -538,20 +551,13 @@ mod tests {
assert_eq!(*world.get::<Parent>(children[0]).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(children[1]).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(children[0]).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(children[1]).unwrap(),
PreviousParent(parent)
);
assert_eq!(*world.get::<Parent>(children[0]).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(children[1]).unwrap(), Parent(parent));
}
#[test]
fn push_and_insert_and_remove_children_commands() {
let mut world = World::default();
let entities = world
.spawn_batch(vec![(C(1),), (C(2),), (C(3),), (C(4),), (C(5),)])
.collect::<Vec<Entity>>();
@ -577,14 +583,8 @@ mod tests {
assert_eq!(*world.get::<Parent>(child1).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child2).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child2).unwrap(),
PreviousParent(parent)
);
assert_eq!(*world.get::<Parent>(child1).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child2).unwrap(), Parent(parent));
{
let mut commands = Commands::new(&mut queue, &world);
@ -599,14 +599,8 @@ mod tests {
);
assert_eq!(*world.get::<Parent>(child3).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child4).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(child3).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child4).unwrap(),
PreviousParent(parent)
);
assert_eq!(*world.get::<Parent>(child3).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child4).unwrap(), Parent(parent));
let remove_children = [child1, child4];
{
@ -622,20 +616,11 @@ mod tests {
);
assert!(world.get::<Parent>(child1).is_none());
assert!(world.get::<Parent>(child4).is_none());
assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child4).unwrap(),
PreviousParent(parent)
);
}
#[test]
fn push_and_insert_and_remove_children_world() {
let mut world = World::default();
let entities = world
.spawn_batch(vec![(C(1),), (C(2),), (C(3),), (C(4),), (C(5),)])
.collect::<Vec<Entity>>();
@ -656,14 +641,8 @@ mod tests {
assert_eq!(*world.get::<Parent>(child1).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child2).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child2).unwrap(),
PreviousParent(parent)
);
assert_eq!(*world.get::<Parent>(child1).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child2).unwrap(), Parent(parent));
world.entity_mut(parent).insert_children(1, &entities[3..]);
let expected_children: SmallVec<[Entity; 8]> = smallvec![child1, child3, child4, child2];
@ -673,14 +652,8 @@ mod tests {
);
assert_eq!(*world.get::<Parent>(child3).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child4).unwrap(), Parent(parent));
assert_eq!(
*world.get::<PreviousParent>(child3).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child4).unwrap(),
PreviousParent(parent)
);
assert_eq!(*world.get::<Parent>(child3).unwrap(), Parent(parent));
assert_eq!(*world.get::<Parent>(child4).unwrap(), Parent(parent));
let remove_children = [child1, child4];
world.entity_mut(parent).remove_children(&remove_children);
@ -691,14 +664,6 @@ mod tests {
);
assert!(world.get::<Parent>(child1).is_none());
assert!(world.get::<Parent>(child4).is_none());
assert_eq!(
*world.get::<PreviousParent>(child1).unwrap(),
PreviousParent(parent)
);
assert_eq!(
*world.get::<PreviousParent>(child4).unwrap(),
PreviousParent(parent)
);
}
#[test]

View file

@ -1,7 +1,9 @@
use bevy_ecs::{
component::Component,
entity::{Entity, EntityMap, MapEntities, MapEntitiesError},
prelude::FromWorld,
reflect::{ReflectComponent, ReflectMapEntities},
world::World,
};
use bevy_reflect::Reflect;
use core::slice;
@ -9,7 +11,7 @@ use smallvec::SmallVec;
use std::ops::Deref;
/// Contains references to the child entities of this entity
#[derive(Component, Default, Clone, Debug, Reflect)]
#[derive(Component, Debug, Reflect)]
#[reflect(Component, MapEntities)]
pub struct Children(pub(crate) SmallVec<[Entity; 8]>);
@ -23,6 +25,16 @@ impl MapEntities for Children {
}
}
// TODO: We need to impl either FromWorld or Default so Children can be registered as Reflect.
// This is because Reflect deserialize by creating an instance and apply a patch on top.
// However Children should only ever be set with a real user-defined entities. Its worth looking
// into better ways to handle cases like this.
impl FromWorld for Children {
fn from_world(_world: &mut World) -> Self {
Children(SmallVec::new())
}
}
impl Children {
/// Builds and returns a [`Children`] component with the given entities
pub fn with(entity: &[Entity]) -> Self {

View file

@ -2,4 +2,4 @@ mod children;
mod parent;
pub use children::Children;
pub use parent::{Parent, PreviousParent};
pub use parent::Parent;

View file

@ -5,16 +5,23 @@ use bevy_ecs::{
world::{FromWorld, World},
};
use bevy_reflect::Reflect;
use std::ops::{Deref, DerefMut};
use std::ops::Deref;
/// Holds a reference to the parent entity of this entity.
/// This component should only be present on entities that actually have a parent entity.
#[derive(Component, Debug, Copy, Clone, Eq, PartialEq, Reflect)]
#[derive(Component, Debug, Eq, PartialEq, Reflect)]
#[reflect(Component, MapEntities, PartialEq)]
pub struct Parent(pub Entity);
pub struct Parent(pub(crate) Entity);
// TODO: We need to impl either FromWorld or Default so Parent can be registered as Properties.
// This is because Properties deserialize by creating an instance and apply a patch on top.
impl Parent {
/// Gets the [`Entity`] ID of the parent.
pub fn get(&self) -> Entity {
self.0
}
}
// TODO: We need to impl either FromWorld or Default so Parent can be registered as Reflect.
// This is because Reflect deserialize by creating an instance and apply a patch on top.
// However Parent should only ever be set with a real user-defined entity. Its worth looking into
// better ways to handle cases like this.
impl FromWorld for Parent {
@ -41,32 +48,3 @@ impl Deref for Parent {
&self.0
}
}
impl DerefMut for Parent {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
}
/// Component that holds the [`Parent`] this entity had previously
#[derive(Component, Debug, Copy, Clone, Eq, PartialEq, Reflect)]
#[reflect(Component, MapEntities, PartialEq)]
pub struct PreviousParent(pub(crate) Entity);
impl MapEntities for PreviousParent {
fn map_entities(&mut self, entity_map: &EntityMap) -> Result<(), MapEntitiesError> {
// PreviousParent of an entity in the new world can be in outside world, in which
// case it should not be mapped.
if let Ok(mapped_entity) = entity_map.get(self.0) {
self.0 = mapped_entity;
}
Ok(())
}
}
// TODO: Better handle this case see `impl FromWorld for Parent`
impl FromWorld for PreviousParent {
fn from_world(_world: &mut World) -> Self {
PreviousParent(Entity::from_raw(u32::MAX))
}
}

View file

@ -0,0 +1,32 @@
use bevy_ecs::prelude::Entity;
/// A [`Event`] that is fired whenever there is a change in the world's
/// hierarchy.
///
/// [`Event`]: bevy_ecs::event::Event
#[derive(Debug, Clone)]
pub enum HierarchyEvent {
/// Fired whenever an [`Entity`] is added as a child to a new parent.
ChildAdded {
/// The child that added
child: Entity,
/// The parent the child was added to
parent: Entity,
},
/// Fired whenever an child [`Entity`] is removed from is parent.
ChildRemoved {
/// The child that removed
child: Entity,
/// The parent the child was removed from
parent: Entity,
},
/// Fired whenever an child [`Entity`] is moved to a new parent.
ChildMoved {
/// The child that moved
child: Entity,
/// The parent the child was removed from
previous_parent: Entity,
/// The parent the child was added to
new_parent: Entity,
},
}

View file

@ -13,8 +13,8 @@ pub use hierarchy::*;
mod child_builder;
pub use child_builder::*;
mod systems;
pub use systems::*;
mod events;
pub use events::*;
#[doc(hidden)]
pub mod prelude {
@ -23,31 +23,15 @@ pub mod prelude {
}
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
/// The base plugin for handling [`Parent`] and [`Children`] components
#[derive(Default)]
pub struct HierarchyPlugin;
/// Label enum for the systems relating to hierarchy upkeep
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemLabel)]
pub enum HierarchySystem {
/// Updates [`Parent`] when changes in the hierarchy occur
ParentUpdate,
}
impl Plugin for HierarchyPlugin {
fn build(&self, app: &mut App) {
app.register_type::<Children>()
.register_type::<Parent>()
.register_type::<PreviousParent>()
.add_startup_system_to_stage(
StartupStage::PostStartup,
parent_update_system.label(HierarchySystem::ParentUpdate),
)
.add_system_to_stage(
CoreStage::PostUpdate,
parent_update_system.label(HierarchySystem::ParentUpdate),
);
.add_event::<HierarchyEvent>();
}
}

View file

@ -1,72 +0,0 @@
use crate::components::*;
use bevy_ecs::{
entity::Entity,
prelude::Changed,
query::Without,
system::{Commands, Query},
};
use bevy_utils::HashMap;
use smallvec::SmallVec;
/// Updates parents when the hierarchy is changed
pub fn parent_update_system(
mut commands: Commands,
removed_parent_query: Query<(Entity, &PreviousParent), Without<Parent>>,
mut parent_query: Query<(Entity, &Parent, Option<&mut PreviousParent>), Changed<Parent>>,
mut children_query: Query<&mut Children>,
) {
// Entities with a missing `Parent` (ie. ones that have a `PreviousParent`), remove
// them from the `Children` of the `PreviousParent`.
for (entity, previous_parent) in removed_parent_query.iter() {
if let Ok(mut previous_parent_children) = children_query.get_mut(previous_parent.0) {
previous_parent_children.0.retain(|e| *e != entity);
commands.entity(entity).remove::<PreviousParent>();
}
}
// Tracks all newly created `Children` Components this frame.
let mut children_additions = HashMap::<Entity, SmallVec<[Entity; 8]>>::default();
// Entities with a changed Parent (that also have a PreviousParent, even if None)
for (entity, parent, possible_previous_parent) in parent_query.iter_mut() {
if let Some(mut previous_parent) = possible_previous_parent {
// New and previous point to the same Entity, carry on, nothing to see here.
if previous_parent.0 == parent.0 {
continue;
}
// Remove from `PreviousParent.Children`.
if let Ok(mut previous_parent_children) = children_query.get_mut(previous_parent.0) {
(*previous_parent_children).0.retain(|e| *e != entity);
}
// Set `PreviousParent = Parent`.
*previous_parent = PreviousParent(parent.0);
} else {
commands.entity(entity).insert(PreviousParent(parent.0));
};
// Add to the parent's `Children` (either the real component, or
// `children_additions`).
if let Ok(mut new_parent_children) = children_query.get_mut(parent.0) {
// This is the parent
// 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
.entry(parent.0)
.or_insert_with(Default::default)
.push(entity);
}
}
// Flush the `children_additions` to the command buffer. It is stored separate to
// collect multiple new children that point to the same parent into the same
// SmallVec, and to prevent redundant add+remove operations.
children_additions.iter().for_each(|(e, v)| {
commands.entity(*e).insert(Children::with(v));
});
}

View file

@ -14,7 +14,6 @@ pub mod prelude {
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use bevy_hierarchy::HierarchySystem;
use prelude::{GlobalTransform, Transform};
/// A [`Bundle`] of the [`Transform`] and [`GlobalTransform`]
@ -92,18 +91,14 @@ impl Plugin for TransformPlugin {
fn build(&self, app: &mut App) {
app.register_type::<Transform>()
.register_type::<GlobalTransform>()
// Adding these to startup ensures the first update is "correct"
// add transform systems to startup so the first update is "correct"
.add_startup_system_to_stage(
StartupStage::PostStartup,
systems::transform_propagate_system
.label(TransformSystem::TransformPropagate)
.after(HierarchySystem::ParentUpdate),
systems::transform_propagate_system.label(TransformSystem::TransformPropagate),
)
.add_system_to_stage(
CoreStage::PostUpdate,
systems::transform_propagate_system
.label(TransformSystem::TransformPropagate)
.after(HierarchySystem::ParentUpdate),
systems::transform_propagate_system.label(TransformSystem::TransformPropagate),
);
}
}

View file

@ -67,7 +67,7 @@ fn propagate_recursive(
transform_query.get_mut(entity).map_err(drop)?;
// Note that for parallelising, this check cannot occur here, since there is an `&mut GlobalTransform` (in global_transform)
assert_eq!(
child_parent.0, expected_parent,
child_parent.get(), expected_parent,
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
);
changed |= transform_changed;
@ -103,17 +103,14 @@ mod test {
use crate::components::{GlobalTransform, Transform};
use crate::systems::transform_propagate_system;
use crate::TransformBundle;
use bevy_hierarchy::{
parent_update_system, BuildChildren, BuildWorldChildren, Children, Parent,
};
use bevy_hierarchy::{BuildChildren, BuildWorldChildren, Children, Parent};
#[test]
fn did_propagate() {
let mut world = World::default();
let mut update_stage = SystemStage::parallel();
update_stage.add_system(parent_update_system);
update_stage.add_system(transform_propagate_system.after(parent_update_system));
update_stage.add_system(transform_propagate_system);
let mut schedule = Schedule::default();
schedule.add_stage("update", update_stage);
@ -155,10 +152,8 @@ mod test {
#[test]
fn did_propagate_command_buffer() {
let mut world = World::default();
let mut update_stage = SystemStage::parallel();
update_stage.add_system(parent_update_system);
update_stage.add_system(transform_propagate_system.after(parent_update_system));
update_stage.add_system(transform_propagate_system);
let mut schedule = Schedule::default();
schedule.add_stage("update", update_stage);
@ -200,36 +195,38 @@ mod test {
let mut world = World::default();
let mut update_stage = SystemStage::parallel();
update_stage.add_system(parent_update_system);
update_stage.add_system(transform_propagate_system.after(parent_update_system));
update_stage.add_system(transform_propagate_system);
let mut schedule = Schedule::default();
schedule.add_stage("update", update_stage);
// Add parent entities
let mut command_queue = CommandQueue::default();
let mut commands = Commands::new(&mut command_queue, &world);
let mut children = Vec::new();
let parent = commands
.spawn()
.insert(Transform::from_xyz(1.0, 0.0, 0.0))
.id();
commands.entity(parent).with_children(|parent| {
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 2.0, 0.0))
.id(),
);
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 3.0, 0.0))
.id(),
);
});
command_queue.apply(&mut world);
schedule.run(&mut world);
let parent = {
let mut command_queue = CommandQueue::default();
let mut commands = Commands::new(&mut command_queue, &world);
let parent = commands
.spawn()
.insert(Transform::from_xyz(1.0, 0.0, 0.0))
.id();
commands.entity(parent).with_children(|parent| {
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 2.0, 0.0))
.id(),
);
children.push(
parent
.spawn()
.insert(Transform::from_xyz(0.0, 3.0, 0.0))
.id(),
);
});
command_queue.apply(&mut world);
schedule.run(&mut world);
parent
};
assert_eq!(
world
@ -242,9 +239,13 @@ mod test {
);
// Parent `e1` to `e2`.
(*world.get_mut::<Parent>(children[0]).unwrap()).0 = children[1];
schedule.run(&mut world);
{
let mut command_queue = CommandQueue::default();
let mut commands = Commands::new(&mut command_queue, &world);
commands.entity(children[1]).add_child(children[0]);
command_queue.apply(&mut world);
schedule.run(&mut world);
}
assert_eq!(
world
@ -285,36 +286,28 @@ mod test {
fn correct_transforms_when_no_children() {
let mut app = App::new();
app.add_system(parent_update_system);
app.add_system(transform_propagate_system.after(parent_update_system));
app.add_system(transform_propagate_system);
let translation = vec3(1.0, 0.0, 0.0);
// These will be overwritten.
let mut child = Entity::from_raw(0);
let mut grandchild = Entity::from_raw(1);
let parent = app
.world
.spawn()
.insert(Transform::from_translation(translation))
.insert(GlobalTransform::default())
.id();
let child = app
.world
.spawn()
.insert_bundle((
Transform::identity(),
GlobalTransform::default(),
Parent(parent),
))
.id();
let grandchild = app
.world
.spawn()
.insert_bundle((
Transform::identity(),
GlobalTransform::default(),
Parent(child),
))
.with_children(|builder| {
child = builder
.spawn_bundle((Transform::identity(), GlobalTransform::default()))
.with_children(|builder| {
grandchild = builder
.spawn_bundle((Transform::identity(), GlobalTransform::default()))
.id();
})
.id();
})
.id();
app.update();
@ -336,37 +329,47 @@ mod test {
);
}
}
#[test]
#[should_panic]
fn panic_when_hierarchy_cycle() {
let mut world = World::default();
// This test is run on a single thread in order to avoid breaking the global task pool by panicking
// This fixes the flaky tests reported in https://github.com/bevyengine/bevy/issues/4996
let mut update_stage = SystemStage::single_threaded();
// We cannot directly edit Parent and Children, so we use a temp world to break
// the hierarchy's invariants.
let mut temp = World::new();
let mut app = App::new();
update_stage.add_system(parent_update_system);
update_stage.add_system(transform_propagate_system.after(parent_update_system));
app.add_system(transform_propagate_system);
let child = world
fn setup_world(world: &mut World) -> (Entity, Entity) {
let mut grandchild = Entity::from_raw(0);
let child = world
.spawn()
.insert_bundle((Transform::identity(), GlobalTransform::default()))
.with_children(|builder| {
grandchild = builder
.spawn()
.insert_bundle((Transform::identity(), GlobalTransform::default()))
.id();
})
.id();
(child, grandchild)
}
let (temp_child, temp_grandchild) = setup_world(&mut temp);
let (child, grandchild) = setup_world(&mut app.world);
assert_eq!(temp_child, child);
assert_eq!(temp_grandchild, grandchild);
app.world
.spawn()
.insert_bundle((Transform::identity(), GlobalTransform::default()))
.id();
.insert_bundle((Transform::default(), GlobalTransform::default()))
.push_children(&[child]);
std::mem::swap(
&mut *app.world.get_mut::<Parent>(child).unwrap(),
&mut *temp.get_mut::<Parent>(grandchild).unwrap(),
);
let grandchild = world
.spawn()
.insert_bundle((
Transform::identity(),
GlobalTransform::default(),
Parent(child),
))
.id();
world.spawn().insert_bundle((
Transform::default(),
GlobalTransform::default(),
Children::with(&[child]),
));
world.entity_mut(child).insert(Parent(grandchild));
update_stage.run(&mut world);
app.update();
}
}

View file

@ -279,7 +279,7 @@ pub fn flex_node_system(
new_position.x = to_logical(layout.location.x + layout.size.width / 2.0);
new_position.y = to_logical(layout.location.y + layout.size.height / 2.0);
if let Some(parent) = parent {
if let Ok(parent_layout) = flex_surface.get_layout(parent.0) {
if let Ok(parent_layout) = flex_surface.get_layout(**parent) {
new_position.x -= to_logical(parent_layout.size.width / 2.0);
new_position.y -= to_logical(parent_layout.size.height / 2.0);
}

View file

@ -52,7 +52,7 @@ fn joint_animation(
// Iter skinned mesh entity
for skinned_mesh_parent in parent_query.iter() {
// Mesh node is the parent of the skinned mesh entity.
let mesh_node_entity = skinned_mesh_parent.0;
let mesh_node_entity = skinned_mesh_parent.get();
// Get `Children` in the mesh node.
let mesh_node_children = children_query.get(mesh_node_entity).unwrap();

View file

@ -41,27 +41,6 @@ fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
// Store parent entity for next sections
.id();
// Another way to create a hierarchy is to add a Parent component to an entity,
// which would be added automatically to parents with other methods.
// Similarly, adding a Parent component will automatically add a Children component to the
// parent.
commands
.spawn_bundle(SpriteBundle {
transform: Transform {
translation: Vec3::new(-250.0, 0.0, 0.0),
scale: Vec3::splat(0.75),
..default()
},
texture: texture.clone(),
sprite: Sprite {
color: Color::RED,
..default()
},
..default()
})
// Using the entity from the previous section as the parent:
.insert(Parent(parent));
// Another way is to use the push_children function to add children after the parent
// entity has already been spawned.
let child = commands