Fix duplicated chilren in Scene spawn (#904)

Fix duplicated chilren in Scene spawn

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(pub(crate) SmallVec<[Entity; 8]>);
 
 impl MapEntities for Children {
     fn map_entities(
@@ -23,18 +23,17 @@ 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 {
-    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[..]
     }
 }

crates/bevy_transform/src/components/parent.rs

@@ -25,9 +25,6 @@ impl MapEntities for Parent {
     }
 }
 
-#[derive(Debug, Copy, Clone, Eq, PartialEq)]
-pub struct PreviousParent(pub Entity);
-
 impl Deref for Parent {
     type Target = Entity;
 
@@ -41,3 +38,23 @@ impl DerefMut for Parent {
         &mut self.0
     }
 }
+
+#[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 see `impl FromResources for Parent`
+impl FromResources for PreviousParent {
+    fn from_resources(_resources: &bevy_ecs::Resources) -> Self {
+        PreviousParent(Entity::new(u32::MAX))
+    }
+}

crates/bevy_transform/src/hierarchy/child_builder.rs

@@ -19,7 +19,7 @@ impl Command for InsertChildren {
         {
             let mut added = false;
             if let Ok(mut children) = world.get_mut::<Children>(self.parent) {
-                children.insert_from_slice(self.index, &self.children);
+                children.0.insert_from_slice(self.index, &self.children);
                 added = true;
             }
 
@@ -54,7 +54,7 @@ impl Command for PushChildren {
         {
             let mut added = false;
             if let Ok(mut children) = world.get_mut::<Children>(self.parent) {
-                children.extend(self.children.iter().cloned());
+                children.0.extend(self.children.iter().cloned());
                 added = true;
             }
 

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);
         }
     }
 

crates/bevy_transform/src/hierarchy/hierarchy_maintenance_system.rs

@@ -47,6 +47,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);
         } else {
             // The parent doesn't have a children entity, lets add it

crates/bevy_transform/src/hierarchy/world_child_builder.rs

@@ -22,7 +22,7 @@ impl<'a, 'b> WorldChildBuilder<'a, 'b> {
             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);
+                children.0.push(entity);
                 added = true;
             }
 

crates/bevy_transform/src/lib.rs

@@ -8,7 +8,7 @@ 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;
@@ -17,6 +17,7 @@ impl Plugin for TransformPlugin {
     fn build(&self, app: &mut AppBuilder) {
         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"

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));
         }
@@ -110,10 +110,7 @@ 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.pop().unwrap();
+            let child = children.last().copied().unwrap();
             commands.despawn(child);
         }