From 54cba7abd99229b6643f92562b7e71d0c29a1840 Mon Sep 17 00:00:00 2001
From: Matthew Pomes <the10thwiz@gmail.com>
Date: Wed, 14 Sep 2022 10:11:05 -0500
Subject: [PATCH] Move EntityPath and NameLookup to core

---
 crates/bevy_animation/src/lib.rs | 129 +++----------------------------
 crates/bevy_core/Cargo.toml      |   1 +
 crates/bevy_core/src/name.rs     |  98 ++++++++++++++++++++++-
 crates/bevy_gltf/src/loader.rs   |   2 +-
 4 files changed, 110 insertions(+), 120 deletions(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index f4c9b5a5b0e06..f001fb913ed97 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -6,15 +6,15 @@ use std::ops::Deref;
 
 use bevy_app::{App, CoreStage, Plugin};
 use bevy_asset::{AddAsset, Assets, Handle};
-use bevy_core::Name;
+use bevy_core::{EntityPath, Name, NameLookup};
 use bevy_ecs::{
     change_detection::DetectChanges,
     entity::Entity,
     prelude::Component,
+    query::QueryEntityError,
     reflect::ReflectComponent,
     schedule::ParallelSystemDescriptorCoercion,
     system::{Query, Res, SystemParam},
-    query::QueryEntityError,
 };
 use bevy_hierarchy::Children;
 use bevy_math::{Quat, Vec3};
@@ -26,94 +26,9 @@ use bevy_utils::{tracing::warn, HashMap};
 #[allow(missing_docs)]
 pub mod prelude {
     #[doc(hidden)]
-    pub use crate::{
-        AnimationClip, AnimationPlayer, AnimationPlugin, EntityPath, Keyframes, VariableCurve,
-    };
-}
-
-/// System param to enable entity lookup of an entity via EntityPath
-#[derive(SystemParam)]
-pub struct NameLookup<'w, 's> {
-    named: Query<'w, 's, (Entity, &'static Name)>,
-    children: Query<'w, 's, &'static Children>,
-}
-
-/// Errors when looking up an entity by name
-pub enum LookupError {
-    /// An entity could not be found, this either means the entity has been
-    /// despawned, or the entity doesn't have the required components
-    Query(QueryEntityError),
-    /// The root node does not have the corrent name
-    // TODO: add expected / found name
-    RootNotFound,
-    /// A child was not found
-    // TODO: add expected name
-    ChildNotFound,
-    /// The name does not uniquely identify an entity
-    // TODO: add name
-    NameNotUnique,
-}
-
-impl From<QueryEntityError> for LookupError {
-    fn from(q: QueryEntityError) -> Self {
-        Self::Query(q)
-    }
-}
-
-impl<'w, 's> NameLookup<'w, 's> {
-    /// Find an entity by entity path, may return an error if the root name isn't unique
-    pub fn lookup_any(&self, path: &EntityPath) -> Result<Entity, LookupError> {
-        let mut path = path.parts.iter();
-        let root_name = path.next().unwrap();
-        let mut root = None;
-        for (entity, name) in self.named.iter() {
-            if root_name == name {
-                if root.is_some() {
-                    return Err(LookupError::NameNotUnique);
-                }
-                root = Some(entity);
-            }
-        }
-        let mut current_node = root.ok_or(LookupError::RootNotFound)?;
-        for part in path {
-            current_node = self.find_child(current_node, part)?;
-        }
-        Ok(current_node)
-    }
-
-    /// Find an entity by the root & entity path
-    pub fn lookup(&self, root: Entity, path: &EntityPath) -> Result<Entity, LookupError> {
-        let mut path = path.parts.iter();
-        let (_, root_name) = self.named.get(root)?;
-        if root_name != path.next().unwrap() {
-            return Err(LookupError::RootNotFound);
-        }
-        let mut current_node = root;
-        for part in path {
-            current_node = self.find_child(current_node, part)?;
-        }
-        Ok(current_node)
-    }
-
-    /// Internal function to get the child of `current_node` that has the name `part`
-    fn find_child(&self, current_node: Entity, part: &Name) -> Result<Entity, LookupError> {
-        let children = self.children.get(current_node)?;
-        let mut ret = Err(LookupError::ChildNotFound);
-        for child in children {
-            if let Ok((_, name)) = self.named.get(*child) {
-                if name == part {
-                    if ret.is_ok() {
-                        return Err(LookupError::NameNotUnique);
-                    }
-                    ret = Ok(*child);
-                }
-            }
-        }
-        ret
-    }
+    pub use crate::{AnimationClip, AnimationPlayer, AnimationPlugin, Keyframes, VariableCurve};
 }
 
-
 /// List of keyframes for one of the attribute of a [`Transform`].
 #[derive(Clone, Debug)]
 pub enum Keyframes {
@@ -136,13 +51,6 @@ pub struct VariableCurve {
     pub keyframes: Keyframes,
 }
 
-/// Path to an entity, with [`Name`]s. Each entity in a path must have a name.
-#[derive(Clone, Debug, Hash, PartialEq, Eq, Default)]
-pub struct EntityPath {
-    /// Parts of the path
-    pub parts: Vec<Name>,
-}
-
 /// A list of [`VariableCurve`], and the [`EntityPath`] to which they apply.
 #[derive(Clone, TypeUuid, Debug, Default)]
 #[uuid = "d81b7179-0448-4eb0-89fe-c067222725bf"]
@@ -263,9 +171,8 @@ pub fn animation_player(
     time: Res<Time>,
     animations: Res<Assets<AnimationClip>>,
     mut animation_players: Query<(Entity, &mut AnimationPlayer)>,
-    names: Query<&Name>,
     mut transforms: Query<&mut Transform>,
-    children: Query<&Children>,
+    lookup: NameLookup,
 ) {
     for (entity, mut player) in &mut animation_players {
         if let Some(animation_clip) = animations.get(&player.animation_clip) {
@@ -284,29 +191,15 @@ pub fn animation_player(
             if elapsed < 0.0 {
                 elapsed += animation_clip.duration;
             }
-            'entity: for (path, curves) in &animation_clip.curves {
+            for (path, curves) in &animation_clip.curves {
                 // PERF: finding the target entity can be optimised
-                let mut current_entity = entity;
-                // Ignore the first name, it is the root node which we already have
-                for part in path.parts.iter().skip(1) {
-                    let mut found = false;
-                    if let Ok(children) = children.get(current_entity) {
-                        for child in children.deref() {
-                            if let Ok(name) = names.get(*child) {
-                                if name == part {
-                                    // Found a children with the right name, continue to the next part
-                                    current_entity = *child;
-                                    found = true;
-                                    break;
-                                }
-                            }
-                        }
+                let current_entity = match lookup.lookup(entity, path) {
+                    Ok(e) => e,
+                    Err(e) => {
+                        warn!("Entity for path {path:?} was not found");
+                        continue;
                     }
-                    if !found {
-                        warn!("Entity not found for path {:?} on part {:?}", path, part);
-                        continue 'entity;
-                    }
-                }
+                };
                 if let Ok(mut transform) = transforms.get_mut(current_entity) {
                     for curve in curves {
                         // Some curves have only one keyframe used to set a transform
diff --git a/crates/bevy_core/Cargo.toml b/crates/bevy_core/Cargo.toml
index 08bd7987c36f7..bf591ab18b63d 100644
--- a/crates/bevy_core/Cargo.toml
+++ b/crates/bevy_core/Cargo.toml
@@ -17,6 +17,7 @@ bevy_math = { path = "../bevy_math", version = "0.9.0-dev" }
 bevy_reflect = { path = "../bevy_reflect", version = "0.9.0-dev", features = ["bevy"] }
 bevy_tasks = { path = "../bevy_tasks", version = "0.9.0-dev" }
 bevy_utils = { path = "../bevy_utils", version = "0.9.0-dev" }
+bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.9.0-dev" }
 
 # other
 bytemuck = "1.5"
diff --git a/crates/bevy_core/src/name.rs b/crates/bevy_core/src/name.rs
index fe320300fd03d..577b6dcd7d7ff 100644
--- a/crates/bevy_core/src/name.rs
+++ b/crates/bevy_core/src/name.rs
@@ -1,4 +1,11 @@
-use bevy_ecs::{component::Component, reflect::ReflectComponent};
+use bevy_ecs::{
+    component::Component,
+    entity::Entity,
+    query::QueryEntityError,
+    reflect::ReflectComponent,
+    system::{Query, SystemParam},
+};
+use bevy_hierarchy::Children;
 use bevy_reflect::std_traits::ReflectDefault;
 use bevy_reflect::Reflect;
 use bevy_utils::AHasher;
@@ -150,3 +157,92 @@ impl Deref for Name {
         self.name.as_ref()
     }
 }
+
+/// Path to an entity, with [`Name`]s. Each entity in a path must have a name.
+#[derive(Clone, Debug, Hash, PartialEq, Eq, Default)]
+pub struct EntityPath {
+    /// Parts of the path
+    pub parts: Vec<Name>,
+}
+
+/// System param to enable entity lookup of an entity via EntityPath
+#[derive(SystemParam)]
+pub struct NameLookup<'w, 's> {
+    named: Query<'w, 's, (Entity, &'static Name)>,
+    children: Query<'w, 's, &'static Children>,
+}
+
+/// Errors when looking up an entity by name
+pub enum LookupError {
+    /// An entity could not be found, this either means the entity has been
+    /// despawned, or the entity doesn't have the required components
+    Query(QueryEntityError),
+    /// The root node does not have the corrent name
+    // TODO: add expected / found name
+    RootNotFound,
+    /// A child was not found
+    // TODO: add expected name
+    ChildNotFound,
+    /// The name does not uniquely identify an entity
+    // TODO: add name
+    NameNotUnique,
+}
+
+impl From<QueryEntityError> for LookupError {
+    fn from(q: QueryEntityError) -> Self {
+        Self::Query(q)
+    }
+}
+
+impl<'w, 's> NameLookup<'w, 's> {
+    /// Find an entity by entity path, may return an error if the root name isn't unique
+    pub fn lookup_any(&self, path: &EntityPath) -> Result<Entity, LookupError> {
+        let mut path = path.parts.iter();
+        let root_name = path.next().unwrap();
+        let mut root = None;
+        for (entity, name) in self.named.iter() {
+            if root_name == name {
+                if root.is_some() {
+                    return Err(LookupError::NameNotUnique);
+                }
+                root = Some(entity);
+            }
+        }
+        let mut current_node = root.ok_or(LookupError::RootNotFound)?;
+        for part in path {
+            current_node = self.find_child(current_node, part)?;
+        }
+        Ok(current_node)
+    }
+
+    /// Find an entity by the root & entity path
+    pub fn lookup(&self, root: Entity, path: &EntityPath) -> Result<Entity, LookupError> {
+        let mut path = path.parts.iter();
+        let (_, root_name) = self.named.get(root)?;
+        if root_name != path.next().unwrap() {
+            return Err(LookupError::RootNotFound);
+        }
+        let mut current_node = root;
+        for part in path {
+            current_node = self.find_child(current_node, part)?;
+        }
+        Ok(current_node)
+    }
+
+    /// Internal function to get the child of `current_node` that has the name `part`
+    fn find_child(&self, current_node: Entity, part: &Name) -> Result<Entity, LookupError> {
+        let children = self.children.get(current_node)?;
+        let mut ret = Err(LookupError::ChildNotFound);
+        for child in children {
+            if let Ok((_, name)) = self.named.get(*child) {
+                if name == part {
+                    if ret.is_ok() {
+                        return Err(LookupError::NameNotUnique);
+                    }
+                    ret = Ok(*child);
+                }
+            }
+        }
+        ret
+    }
+}
diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs
index 2db5126b7743f..f04f1e9b4516a 100644
--- a/crates/bevy_gltf/src/loader.rs
+++ b/crates/bevy_gltf/src/loader.rs
@@ -204,7 +204,7 @@ async fn load_gltf<'a, 'b>(
                 if let Some((root_index, path)) = paths.get(&node.index()) {
                     animation_roots.insert(root_index);
                     animation_clip.add_curve_to_path(
-                        bevy_animation::EntityPath {
+                        bevy_core::EntityPath {
                             parts: path.clone(),
                         },
                         bevy_animation::VariableCurve {