From 87c0971c4f6135b491d2b7948852404e213eff67 Mon Sep 17 00:00:00 2001
From: lassade <lassade@localhost.localdomain>
Date: Thu, 19 Nov 2020 01:57:59 -0300
Subject: [PATCH 1/4] fixed dup children

---
 .../bevy_transform/src/components/children.rs | 20 ++++---------------
 .../src/hierarchy/child_builder.rs            | 12 ++---------
 .../bevy_transform/src/hierarchy/hierarchy.rs |  2 +-
 .../hierarchy/hierarchy_maintenance_system.rs |  8 ++++++--
 .../src/hierarchy/world_child_builder.rs      |  6 +-----
 crates/bevy_transform/src/lib.rs              |  2 +-
 examples/ecs/hierarchy.rs                     |  6 +++---
 7 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/crates/bevy_transform/src/components/children.rs b/crates/bevy_transform/src/components/children.rs
index 5b08a41b48f6d..c8c274a18d66f 100644
--- a/crates/bevy_transform/src/components/children.rs
+++ b/crates/bevy_transform/src/components/children.rs
@@ -1,10 +1,10 @@
 use bevy_ecs::{Entity, MapEntities};
 use bevy_property::Properties;
 use smallvec::SmallVec;
-use std::ops::{Deref, DerefMut};
+use std::ops::Deref;
 
 #[derive(Default, Clone, Properties, Debug)]
-pub struct Children(pub SmallVec<[Entity; 8]>);
+pub struct Children(#[property(ignore)] pub(crate) SmallVec<[Entity; 8]>);
 
 impl MapEntities for Children {
     fn map_entities(
@@ -19,22 +19,10 @@ impl MapEntities for Children {
     }
 }
 
-impl Children {
-    pub fn with(entity: &[Entity]) -> Self {
-        Self(SmallVec::from_slice(entity))
-    }
-}
-
 impl Deref for Children {
-    type Target = SmallVec<[Entity; 8]>;
+    type Target = [Entity];
 
     fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl DerefMut for Children {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
+        &self.0[..]
     }
 }
diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs
index b42772cfb8bc7..abb0d8e78c711 100644
--- a/crates/bevy_transform/src/hierarchy/child_builder.rs
+++ b/crates/bevy_transform/src/hierarchy/child_builder.rs
@@ -17,11 +17,7 @@ impl Command for InsertChildren {
                 .unwrap();
         }
         {
-            let mut added = false;
-            if let Ok(mut children) = world.get_mut::<Children>(self.parent) {
-                children.insert_from_slice(self.index, &self.children);
-                added = true;
-            }
+            let added = world.get_mut::<Children>(self.parent).is_ok();
 
             // NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails borrow-checking
             if !added {
@@ -52,11 +48,7 @@ impl Command for PushChildren {
                 .unwrap();
         }
         {
-            let mut added = false;
-            if let Ok(mut children) = world.get_mut::<Children>(self.parent) {
-                children.extend(self.children.iter().cloned());
-                added = true;
-            }
+            let added = world.get_mut::<Children>(self.parent).is_ok();
 
             // NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails borrow-checking
             if !added {
diff --git a/crates/bevy_transform/src/hierarchy/hierarchy.rs b/crates/bevy_transform/src/hierarchy/hierarchy.rs
index 6509fa0ff7a0d..e9ec1070bc80b 100644
--- a/crates/bevy_transform/src/hierarchy/hierarchy.rs
+++ b/crates/bevy_transform/src/hierarchy/hierarchy.rs
@@ -42,7 +42,7 @@ 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).map(|parent| parent.0) {
         if let Ok(mut children) = world.get_mut::<Children>(parent) {
-            children.retain(|c| *c != entity);
+            children.0.retain(|c| *c != entity);
         }
     }
 
diff --git a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
index 09f67b68e8b05..f8f4d2a642827 100644
--- a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
+++ b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
@@ -46,8 +46,12 @@ pub fn parent_update_system(
         // 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) {
+            let children_vec = &mut (*new_parent_children).0;
+            // Duplicate entities inside the Children component will lead to a state explosion
+            // that will kill the game frame rate
+            debug_assert!(!children_vec.contains(&entity), "duplicate children");
             // This is the parent
-            (*new_parent_children).0.push(entity);
+            children_vec.push(entity);
         } else {
             // The parent doesn't have a children entity, lets add it
             children_additions
@@ -61,7 +65,7 @@ pub fn parent_update_system(
     // 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(|(k, v)| {
-        commands.insert_one(*k, Children::with(v));
+        commands.insert_one(*k, Children(SmallVec::from_slice(v)));
     });
 }
 #[cfg(test)]
diff --git a/crates/bevy_transform/src/hierarchy/world_child_builder.rs b/crates/bevy_transform/src/hierarchy/world_child_builder.rs
index a658956c06e69..e36734f3b4e63 100644
--- a/crates/bevy_transform/src/hierarchy/world_child_builder.rs
+++ b/crates/bevy_transform/src/hierarchy/world_child_builder.rs
@@ -20,11 +20,7 @@ impl<'a, 'b> WorldChildBuilder<'a, 'b> {
         let entity = self.world_builder.current_entity.unwrap();
         {
             let world = &mut self.world_builder.world;
-            let mut added = false;
-            if let Ok(mut children) = world.get_mut::<Children>(parent_entity) {
-                children.push(entity);
-                added = true;
-            }
+            let added = world.get_mut::<Children>(parent_entity).is_ok();
 
             // NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails borrow-checking
             if !added {
diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs
index fe8dc696c996e..b44114a5eb386 100644
--- a/crates/bevy_transform/src/lib.rs
+++ b/crates/bevy_transform/src/lib.rs
@@ -15,7 +15,7 @@ pub struct TransformPlugin;
 
 impl Plugin for TransformPlugin {
     fn build(&self, app: &mut AppBuilder) {
-        app.register_component_with::<Children>(|reg| reg.map_entities())
+        app.register_component::<Children>()
             .register_component_with::<Parent>(|reg| reg.map_entities())
             .register_component::<Transform>()
             .register_component::<GlobalTransform>()
diff --git a/examples/ecs/hierarchy.rs b/examples/ecs/hierarchy.rs
index 9dbbccf19c07c..82e56760486b6 100644
--- a/examples/ecs/hierarchy.rs
+++ b/examples/ecs/hierarchy.rs
@@ -91,11 +91,11 @@ fn setup(
 fn rotate(
     commands: &mut Commands,
     time: Res<Time>,
-    mut parents_query: Query<(Entity, &mut Children), With<Sprite>>,
+    mut parents_query: Query<(Entity, &Children), With<Sprite>>,
     mut transform_query: Query<&mut Transform, With<Sprite>>,
 ) {
     let angle = std::f32::consts::PI / 2.0;
-    for (parent, mut children) in parents_query.iter_mut() {
+    for (parent, children) in parents_query.iter_mut() {
         if let Ok(mut transform) = transform_query.get_mut(parent) {
             transform.rotate(Quat::from_rotation_z(-angle * time.delta_seconds));
         }
@@ -113,7 +113,7 @@ fn rotate(
             // Using .despawn() on an entity does not remove it from its parent's list of children!
             // It must be done manually if using .despawn()
             // NOTE: This is a bug. Eventually Bevy will update the children list automatically
-            let child = children.pop().unwrap();
+            let child = children.last().copied().unwrap();
             commands.despawn(child);
         }
 

From 47aa7e128ac2ddf047931bf274cdb43a1a3b3827 Mon Sep 17 00:00:00 2001
From: lassade <lassade@localhost.localdomain>
Date: Sat, 21 Nov 2020 05:31:38 -0300
Subject: [PATCH 2/4] fix children already added (best short term)

---
 .../bevy_transform/src/components/children.rs |  8 ++++++-
 .../bevy_transform/src/components/parent.rs   | 21 +++++++++++++++++--
 .../src/hierarchy/child_builder.rs            | 12 +++++++++--
 .../hierarchy/hierarchy_maintenance_system.rs | 12 +++++------
 .../src/hierarchy/world_child_builder.rs      |  6 +++++-
 crates/bevy_transform/src/lib.rs              |  5 +++--
 6 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/crates/bevy_transform/src/components/children.rs b/crates/bevy_transform/src/components/children.rs
index c8c274a18d66f..a38611fda1b69 100644
--- a/crates/bevy_transform/src/components/children.rs
+++ b/crates/bevy_transform/src/components/children.rs
@@ -4,7 +4,7 @@ use smallvec::SmallVec;
 use std::ops::Deref;
 
 #[derive(Default, Clone, Properties, Debug)]
-pub struct Children(#[property(ignore)] pub(crate) SmallVec<[Entity; 8]>);
+pub struct Children(pub(crate) SmallVec<[Entity; 8]>);
 
 impl MapEntities for Children {
     fn map_entities(
@@ -19,6 +19,12 @@ impl MapEntities for Children {
     }
 }
 
+impl Children {
+    pub fn with(entity: &[Entity]) -> Self {
+        Self(SmallVec::from_slice(entity))
+    }
+}
+
 impl Deref for Children {
     type Target = [Entity];
 
diff --git a/crates/bevy_transform/src/components/parent.rs b/crates/bevy_transform/src/components/parent.rs
index bf9bdab9a67b2..1246d2d256f07 100644
--- a/crates/bevy_transform/src/components/parent.rs
+++ b/crates/bevy_transform/src/components/parent.rs
@@ -25,8 +25,25 @@ impl MapEntities for Parent {
     }
 }
 
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct PreviousParent(pub Entity);
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Properties)]
+pub struct PreviousParent(pub(crate) Entity);
+
+impl MapEntities for PreviousParent {
+    fn map_entities(
+        &mut self,
+        entity_map: &bevy_ecs::EntityMap,
+    ) -> Result<(), bevy_ecs::MapEntitiesError> {
+        self.0 = entity_map.get(self.0)?;
+        Ok(())
+    }
+}
+
+// TODO: Better handle this case
+impl FromResources for PreviousParent {
+    fn from_resources(_resources: &bevy_ecs::Resources) -> Self {
+        PreviousParent(Entity::new(u32::MAX))
+    }
+}
 
 impl Deref for Parent {
     type Target = Entity;
diff --git a/crates/bevy_transform/src/hierarchy/child_builder.rs b/crates/bevy_transform/src/hierarchy/child_builder.rs
index abb0d8e78c711..572f2a100c9b4 100644
--- a/crates/bevy_transform/src/hierarchy/child_builder.rs
+++ b/crates/bevy_transform/src/hierarchy/child_builder.rs
@@ -17,7 +17,11 @@ impl Command for InsertChildren {
                 .unwrap();
         }
         {
-            let added = world.get_mut::<Children>(self.parent).is_ok();
+            let mut added = false;
+            if let Ok(mut children) = world.get_mut::<Children>(self.parent) {
+                children.0.insert_from_slice(self.index, &self.children);
+                added = true;
+            }
 
             // NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails borrow-checking
             if !added {
@@ -48,7 +52,11 @@ impl Command for PushChildren {
                 .unwrap();
         }
         {
-            let added = world.get_mut::<Children>(self.parent).is_ok();
+            let mut added = false;
+            if let Ok(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 {
diff --git a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
index f8f4d2a642827..6073300001af7 100644
--- a/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
+++ b/crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs
@@ -46,12 +46,12 @@ pub fn parent_update_system(
         // 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) {
-            let children_vec = &mut (*new_parent_children).0;
-            // Duplicate entities inside the Children component will lead to a state explosion
-            // that will kill the game frame rate
-            debug_assert!(!children_vec.contains(&entity), "duplicate children");
             // This is the parent
-            children_vec.push(entity);
+            debug_assert!(
+                !(*new_parent_children).0.contains(&entity),
+                "children already added"
+            );
+            (*new_parent_children).0.push(entity);
         } else {
             // The parent doesn't have a children entity, lets add it
             children_additions
@@ -65,7 +65,7 @@ pub fn parent_update_system(
     // 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(|(k, v)| {
-        commands.insert_one(*k, Children(SmallVec::from_slice(v)));
+        commands.insert_one(*k, Children::with(v));
     });
 }
 #[cfg(test)]
diff --git a/crates/bevy_transform/src/hierarchy/world_child_builder.rs b/crates/bevy_transform/src/hierarchy/world_child_builder.rs
index e36734f3b4e63..8f1dc84528fe6 100644
--- a/crates/bevy_transform/src/hierarchy/world_child_builder.rs
+++ b/crates/bevy_transform/src/hierarchy/world_child_builder.rs
@@ -20,7 +20,11 @@ impl<'a, 'b> WorldChildBuilder<'a, 'b> {
         let entity = self.world_builder.current_entity.unwrap();
         {
             let world = &mut self.world_builder.world;
-            let added = world.get_mut::<Children>(parent_entity).is_ok();
+            let mut added = false;
+            if let Ok(mut children) = world.get_mut::<Children>(parent_entity) {
+                children.0.push(entity);
+                added = true;
+            }
 
             // NOTE: ideally this is just an else statement, but currently that _incorrectly_ fails borrow-checking
             if !added {
diff --git a/crates/bevy_transform/src/lib.rs b/crates/bevy_transform/src/lib.rs
index b44114a5eb386..eece2199a1e4a 100644
--- a/crates/bevy_transform/src/lib.rs
+++ b/crates/bevy_transform/src/lib.rs
@@ -8,15 +8,16 @@ pub mod prelude {
 
 use bevy_app::prelude::*;
 use bevy_type_registry::RegisterType;
-use prelude::{parent_update_system, Children, GlobalTransform, Parent, Transform};
+use prelude::{parent_update_system, Children, GlobalTransform, Parent, PreviousParent, Transform};
 
 #[derive(Default)]
 pub struct TransformPlugin;
 
 impl Plugin for TransformPlugin {
     fn build(&self, app: &mut AppBuilder) {
-        app.register_component::<Children>()
+        app.register_component_with::<Children>(|reg| reg.map_entities())
             .register_component_with::<Parent>(|reg| reg.map_entities())
+            .register_component_with::<PreviousParent>(|reg| reg.map_entities())
             .register_component::<Transform>()
             .register_component::<GlobalTransform>()
             // add transform systems to startup so the first update is "correct"

From 0ee990ac34a64ce0b3d62039311c103beb0e08b9 Mon Sep 17 00:00:00 2001
From: lassade <lassade@localhost.localdomain>
Date: Sat, 21 Nov 2020 06:33:44 -0300
Subject: [PATCH 3/4] reorder impls

---
 .../bevy_transform/src/components/parent.rs   | 30 +++++++++----------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/crates/bevy_transform/src/components/parent.rs b/crates/bevy_transform/src/components/parent.rs
index 1246d2d256f07..9eddcdaff94f5 100644
--- a/crates/bevy_transform/src/components/parent.rs
+++ b/crates/bevy_transform/src/components/parent.rs
@@ -25,6 +25,20 @@ impl MapEntities for Parent {
     }
 }
 
+impl Deref for Parent {
+    type Target = Entity;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl DerefMut for Parent {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
 #[derive(Debug, Copy, Clone, Eq, PartialEq, Properties)]
 pub struct PreviousParent(pub(crate) Entity);
 
@@ -38,23 +52,9 @@ impl MapEntities for PreviousParent {
     }
 }
 
-// TODO: Better handle this case
+// TODO: Better handle this case see `impl FromResources for Parent`
 impl FromResources for PreviousParent {
     fn from_resources(_resources: &bevy_ecs::Resources) -> Self {
         PreviousParent(Entity::new(u32::MAX))
     }
 }
-
-impl Deref for Parent {
-    type Target = Entity;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl DerefMut for Parent {
-    fn deref_mut(&mut self) -> &mut Self::Target {
-        &mut self.0
-    }
-}

From 7413e32288126153b43203a5366bf8346b074be3 Mon Sep 17 00:00:00 2001
From: Carter Anderson <mcanders1@gmail.com>
Date: Sat, 21 Nov 2020 17:26:39 -0800
Subject: [PATCH 4/4] add swap method and update comments

---
 crates/bevy_transform/src/components/children.rs | 5 +++++
 examples/ecs/hierarchy.rs                        | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/crates/bevy_transform/src/components/children.rs b/crates/bevy_transform/src/components/children.rs
index a38611fda1b69..e4205c9ebeaae 100644
--- a/crates/bevy_transform/src/components/children.rs
+++ b/crates/bevy_transform/src/components/children.rs
@@ -23,6 +23,11 @@ impl Children {
     pub fn with(entity: &[Entity]) -> Self {
         Self(SmallVec::from_slice(entity))
     }
+
+    /// Swaps the child at `a_index` with the child at `b_index`
+    pub fn swap(&mut self, a_index: usize, b_index: usize) {
+        self.0.swap(a_index, b_index);
+    }
 }
 
 impl Deref for Children {
diff --git a/examples/ecs/hierarchy.rs b/examples/ecs/hierarchy.rs
index 82e56760486b6..03fe54830f170 100644
--- a/examples/ecs/hierarchy.rs
+++ b/examples/ecs/hierarchy.rs
@@ -110,9 +110,6 @@ fn rotate(
 
         // To demonstrate removing children, we'll start to remove the children after a couple of seconds
         if time.seconds_since_startup >= 2.0 && children.len() == 3 {
-            // Using .despawn() on an entity does not remove it from its parent's list of children!
-            // It must be done manually if using .despawn()
-            // NOTE: This is a bug. Eventually Bevy will update the children list automatically
             let child = children.last().copied().unwrap();
             commands.despawn(child);
         }