From 09c2627364d4ab5e441aa99f10cd68e408ab9286 Mon Sep 17 00:00:00 2001
From: robtfm <50659922+robtfm@users.noreply.github.com>
Date: Mon, 28 Aug 2023 18:06:12 +0100
Subject: [PATCH] check root node for animations (#9407)

# Objective

fixes #8357

gltf animations can affect multiple "root" nodes (i.e. top level nodes
within a gltf scene).

the current loader adds an AnimationPlayer to each root node which is
affected by any animation. when a clip which affects multiple root nodes
is played on a root node player, the root node name is not checked,
leading to potentially incorrect weights being applied.
also, the `AnimationClip::compatible_with` method will never return true
for those clips, as it checks that all paths start with the root node
name - not all paths start with the same name so it can't return true.

## Solution

- check the first path node name matches the given root
- change compatible_with to return true if `any` match is found

a better alternative would probably be to attach the player to the scene
root instead of the first child, and then walk the full path from there.
this would be breaking (and would stop multiple animations that *don't*
overlap from being played concurrently), but i'm happy to modify to that
if it's preferred.

---------

Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
---
 crates/bevy_animation/src/lib.rs | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index 59eab3f92bdae..b39bfa42b7f22 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -119,7 +119,7 @@ impl AnimationClip {
 
     /// Whether this animation clip can run on entity with given [`Name`].
     pub fn compatible_with(&self, name: &Name) -> bool {
-        self.paths.keys().all(|path| &path.parts[0] == name)
+        self.paths.keys().any(|path| &path.parts[0] == name)
     }
 }
 
@@ -406,8 +406,18 @@ fn entity_from_path(
     // PERF: finding the target entity can be optimised
     let mut current_entity = root;
     path_cache.resize(path.parts.len(), None);
-    // Ignore the first name, it is the root node which we already have
-    for (idx, part) in path.parts.iter().enumerate().skip(1) {
+
+    let mut parts = path.parts.iter().enumerate();
+
+    // check the first name is the root node which we already have
+    let Some((_, root_name)) = parts.next() else {
+        return None;
+    };
+    if names.get(current_entity) != Ok(root_name) {
+        return None;
+    }
+
+    for (idx, part) in parts {
         let mut found = false;
         let children = children.get(current_entity).ok()?;
         if let Some(cached) = path_cache[idx] {
@@ -608,12 +618,14 @@ fn apply_animation(
             return;
         }
 
+        let mut any_path_found = false;
         for (path, bone_id) in &animation_clip.paths {
             let cached_path = &mut animation.path_cache[*bone_id];
             let curves = animation_clip.get_curves(*bone_id).unwrap();
             let Some(target) = entity_from_path(root, path, children, names, cached_path) else {
                 continue;
             };
+            any_path_found = true;
             // SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
             // any of their descendant Transforms.
             //
@@ -702,6 +714,10 @@ fn apply_animation(
                 }
             }
         }
+
+        if !any_path_found {
+            warn!("Animation player on {root:?} did not match any entity paths.");
+        }
     }
 }