From 7976d0f70951bf107f493cee52e1546eaaaa12d6 Mon Sep 17 00:00:00 2001 From: Burak Erdemci Date: Fri, 21 Aug 2026 14:15:56 +0300 Subject: [PATCH 1/2] fix(animation): resolve the Animator on child objects, not just the exact target Imported models keep their Animator on the model root, which is normally a child of the GameObject a caller names. The animator_* actions looked the component up with GetComponent() on the target alone, so reads and controls both failed with "No Animator component" on the most common rig setup. Route the seven read/control lookups through a shared AnimatorResolver.Find that falls back to a descendant search. Inactive descendants are included because a disabled rig is still readable. Ambiguity is reported, never guessed. Unity's descendant search is depth-first, so a wrapper holding several rigs returns the first branch however deep - not the nearest rig, and not what a caller would predict. When more than one descendant carries an Animator the call fails and names the candidates instead of silently mutating one of them. Every successful response names the object that actually changed, including the CLI's own success line for animator_play, which echoed the requested target and so contradicted the result it had just printed. This matters most for animator_set_parameter, which in Edit mode writes the shared AnimatorController asset, so the caller has to be able to see which Animator answered. Actions that ADD an Animator (controller_assign) deliberately keep the exact-target lookup: making them search descendants would silently retarget the component they create. A test locks that boundary in. animator_get_info additionally reports animatorGameObject. The existing gameObject field keeps naming the resolved target. --- .../Editor/Tools/Animation/AnimatorControl.cs | 46 +-- .../Editor/Tools/Animation/AnimatorRead.cs | 9 +- .../Tools/Animation/AnimatorResolver.cs | 82 +++++ .../Tools/Animation/AnimatorResolver.cs.meta | 11 + Server/src/cli/commands/animation.py | 5 +- Server/tests/test_manage_animation.py | 26 ++ .../EditMode/Tools/AnimatorResolverTests.cs | 164 +++++++++ .../Tools/AnimatorResolverTests.cs.meta | 11 + .../EditMode/Tools/ManageAnimationTests.cs | 340 ++++++++++++++++++ 9 files changed, 666 insertions(+), 28 deletions(-) create mode 100644 MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs create mode 100644 MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs.meta diff --git a/MCPForUnity/Editor/Tools/Animation/AnimatorControl.cs b/MCPForUnity/Editor/Tools/Animation/AnimatorControl.cs index 7ca43e38e..bd8f9a347 100644 --- a/MCPForUnity/Editor/Tools/Animation/AnimatorControl.cs +++ b/MCPForUnity/Editor/Tools/Animation/AnimatorControl.cs @@ -15,9 +15,9 @@ public static object Play(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); string stateName = @params["stateName"]?.ToString(); if (string.IsNullOrEmpty(stateName)) @@ -28,7 +28,7 @@ public static object Play(JObject @params) Undo.RecordObject(animator, "Play Animation State"); animator.Play(stateName, layer); - return new { success = true, message = $"Playing state '{stateName}' on '{go.name}'" }; + return new { success = true, message = $"Playing state '{stateName}' on {AnimatorResolver.Describe(go, animator)}" }; } public static object Crossfade(JObject @params) @@ -37,9 +37,9 @@ public static object Crossfade(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); string stateName = @params["stateName"]?.ToString(); if (string.IsNullOrEmpty(stateName)) @@ -51,7 +51,7 @@ public static object Crossfade(JObject @params) Undo.RecordObject(animator, "Crossfade Animation State"); animator.CrossFade(stateName, duration, layer); - return new { success = true, message = $"Crossfading to '{stateName}' over {duration}s on '{go.name}'" }; + return new { success = true, message = $"Crossfading to '{stateName}' over {duration}s on {AnimatorResolver.Describe(go, animator)}" }; } public static object SetParameter(JObject @params) @@ -60,9 +60,9 @@ public static object SetParameter(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); string paramName = @params["parameterName"]?.ToString(); if (string.IsNullOrEmpty(paramName)) @@ -103,23 +103,23 @@ public static object SetParameter(JObject @params) case "float": float fVal = valueToken?.ToObject() ?? 0f; animator.SetFloat(paramName, fVal); - return new { success = true, message = $"Set float '{paramName}' = {fVal}" }; + return new { success = true, message = $"Set float '{paramName}' = {fVal}" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "int": case "integer": int iVal = valueToken?.ToObject() ?? 0; animator.SetInteger(paramName, iVal); - return new { success = true, message = $"Set int '{paramName}' = {iVal}" }; + return new { success = true, message = $"Set int '{paramName}' = {iVal}" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "bool": case "boolean": bool bVal = valueToken?.ToObject() ?? false; animator.SetBool(paramName, bVal); - return new { success = true, message = $"Set bool '{paramName}' = {bVal}" }; + return new { success = true, message = $"Set bool '{paramName}' = {bVal}" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "trigger": animator.SetTrigger(paramName); - return new { success = true, message = $"Set trigger '{paramName}'" }; + return new { success = true, message = $"Set trigger '{paramName}'" + AnimatorResolver.ResolvedSuffix(go, animator) }; default: return new { success = false, message = $"Unknown parameter type: {paramType}. Valid: float, int, bool, trigger" }; @@ -130,7 +130,7 @@ public static object SetParameter(JObject @params) // Edit mode: modify the AnimatorController asset's default parameter values var controller = animator.runtimeAnimatorController as AnimatorController; if (controller == null) - return new { success = false, message = $"No AnimatorController assigned to Animator on '{go.name}'. Cannot set parameter defaults in Edit mode." }; + return new { success = false, message = $"No AnimatorController assigned to the Animator on {AnimatorResolver.Describe(go, animator)}. Cannot set parameter defaults in Edit mode." }; var allParams = controller.parameters; int paramIndex = -1; @@ -156,7 +156,7 @@ public static object SetParameter(JObject @params) controller.parameters = allParams; EditorUtility.SetDirty(controller); AssetDatabase.SaveAssets(); - return new { success = true, message = $"Set float '{paramName}' = {fVal} (default value, Edit mode)" }; + return new { success = true, message = $"Set float '{paramName}' = {fVal} (default value, Edit mode)" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "int": case "integer": @@ -165,7 +165,7 @@ public static object SetParameter(JObject @params) controller.parameters = allParams; EditorUtility.SetDirty(controller); AssetDatabase.SaveAssets(); - return new { success = true, message = $"Set int '{paramName}' = {iVal} (default value, Edit mode)" }; + return new { success = true, message = $"Set int '{paramName}' = {iVal} (default value, Edit mode)" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "bool": case "boolean": @@ -174,10 +174,10 @@ public static object SetParameter(JObject @params) controller.parameters = allParams; EditorUtility.SetDirty(controller); AssetDatabase.SaveAssets(); - return new { success = true, message = $"Set bool '{paramName}' = {bVal} (default value, Edit mode)" }; + return new { success = true, message = $"Set bool '{paramName}' = {bVal} (default value, Edit mode)" + AnimatorResolver.ResolvedSuffix(go, animator) }; case "trigger": - return new { success = true, message = $"Trigger '{paramName}' noted (triggers are runtime-only, no default to set)" }; + return new { success = true, message = $"Trigger '{paramName}' noted (triggers are runtime-only, no default to set)" + AnimatorResolver.ResolvedSuffix(go, animator) }; default: return new { success = false, message = $"Unknown parameter type: {paramType}. Valid: float, int, bool, trigger" }; @@ -191,16 +191,16 @@ public static object SetSpeed(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); float speed = @params["speed"]?.ToObject() ?? 1f; Undo.RecordObject(animator, "Set Animator Speed"); animator.speed = speed; - return new { success = true, message = $"Set animator speed to {speed} on '{go.name}'" }; + return new { success = true, message = $"Set animator speed to {speed} on {AnimatorResolver.Describe(go, animator)}" }; } public static object SetEnabled(JObject @params) @@ -209,16 +209,16 @@ public static object SetEnabled(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); bool enabled = @params["enabled"]?.ToObject() ?? true; Undo.RecordObject(animator, "Set Animator Enabled"); animator.enabled = enabled; - return new { success = true, message = $"Animator {(enabled ? "enabled" : "disabled")} on '{go.name}'" }; + return new { success = true, message = $"Animator {(enabled ? "enabled" : "disabled")} on {AnimatorResolver.Describe(go, animator)}" }; } } } diff --git a/MCPForUnity/Editor/Tools/Animation/AnimatorRead.cs b/MCPForUnity/Editor/Tools/Animation/AnimatorRead.cs index 722280ae0..718a7d153 100644 --- a/MCPForUnity/Editor/Tools/Animation/AnimatorRead.cs +++ b/MCPForUnity/Editor/Tools/Animation/AnimatorRead.cs @@ -14,9 +14,9 @@ public static object GetInfo(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); var parameters = new List(); for (int i = 0; i < animator.parameterCount; i++) @@ -73,6 +73,7 @@ public static object GetInfo(JObject @params) data = new { gameObject = go.name, + animatorGameObject = animator.gameObject.name, enabled = animator.enabled, speed = animator.speed, hasController = animator.runtimeAnimatorController != null, @@ -95,9 +96,9 @@ public static object GetParameter(JObject @params) if (go == null) return new { success = false, message = "Target GameObject not found" }; - var animator = go.GetComponent(); + var animator = AnimatorResolver.Find(go, out var animatorCandidates); if (animator == null) - return new { success = false, message = $"No Animator component on '{go.name}'" }; + return AnimatorResolver.NotResolvedError(go, animatorCandidates); string paramName = @params["parameterName"]?.ToString(); if (string.IsNullOrEmpty(paramName)) diff --git a/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs b/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs new file mode 100644 index 000000000..e2b1abd1f --- /dev/null +++ b/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs @@ -0,0 +1,82 @@ +using System.Linq; +using UnityEngine; + +namespace MCPForUnity.Editor.Tools.Animation +{ + internal static class AnimatorResolver + { + /// + /// Resolves the Animator that read and control operations should act on: the one on + /// itself, or the single Animator among its descendants. + /// + /// + /// Imported models keep their Animator on the model root, which is normally a child of + /// the GameObject a caller names, so an exact-match lookup fails on the most common rig + /// setup. Inactive descendants are included because a disabled rig is still readable. + /// Operations that ADD an Animator must not use this - they need the exact target. + /// + /// + /// The descendant Animators found when the target carried none. Unity's descendant search + /// is depth-first, so with several rigs under one wrapper it returns the first branch + /// however deep, which is not the nearest rig and not what a caller would predict. More + /// than one candidate therefore resolves to null and is reported, never guessed. + /// + /// The resolved Animator, or null when there is none or the choice is ambiguous. + public static Animator Find(GameObject go, out Animator[] candidates) + { + candidates = System.Array.Empty(); + if (go == null) + return null; + + var own = go.GetComponent(); + if (own != null) + return own; + + // go carries none, so every hit here is a descendant. + candidates = go.GetComponentsInChildren(true); + return candidates.Length == 1 ? candidates[0] : null; + } + + /// + /// The error for a target whose Animator could not be resolved - missing, or ambiguous + /// because several descendants carry one. + /// + public static object NotResolvedError(GameObject go, Animator[] candidates) + { + if (candidates != null && candidates.Length > 1) + { + string names = string.Join(", ", candidates.Select(a => $"'{a.gameObject.name}'")); + return new + { + success = false, + message = $"'{go.name}' has no Animator and {candidates.Length} of its children do " + + $"({names}). Target one of them directly." + }; + } + + return new { success = false, message = $"No Animator component on '{go.name}' or its children" }; + } + + /// + /// Names the object a response should report as changed. When resolution retargeted to a + /// descendant, the caller is told so - otherwise the response claims the wrapper changed. + /// + /// + /// A suffix disclosing that resolution retargeted to a descendant; empty when the target + /// carried the Animator itself, so responses that already read correctly are untouched. + /// + public static string ResolvedSuffix(GameObject target, Animator resolved) + { + return resolved.gameObject == target + ? string.Empty + : $" (on '{resolved.gameObject.name}', resolved from '{target.name}')"; + } + + public static string Describe(GameObject target, Animator resolved) + { + return resolved.gameObject == target + ? $"'{target.name}'" + : $"'{resolved.gameObject.name}' (resolved from '{target.name}')"; + } + } +} diff --git a/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs.meta b/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs.meta new file mode 100644 index 000000000..86c93340a --- /dev/null +++ b/MCPForUnity/Editor/Tools/Animation/AnimatorResolver.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: c010b345a1c8474e8423ff919f94c5f5 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/Server/src/cli/commands/animation.py b/Server/src/cli/commands/animation.py index afb7f36f6..a9e6f566c 100644 --- a/Server/src/cli/commands/animation.py +++ b/Server/src/cli/commands/animation.py @@ -96,7 +96,10 @@ def animator_play(target: str, state_name: str, layer: int, search_method: Optio result = run_command("manage_animation", _normalize_params(params), config) click.echo(format_output(result, config.format)) if result.get("success"): - print_success(f"Playing state '{state_name}' on {target}") + # Prefer the tool's own message: the Animator may live on a descendant of + # `target`, and only the tool knows which object actually played. The fallback + # names no object for the same reason - here the tool told us nothing. + print_success(result.get("message") or f"Playing state '{state_name}'") @animator.command("crossfade") diff --git a/Server/tests/test_manage_animation.py b/Server/tests/test_manage_animation.py index 85c4f9651..4abf7241c 100644 --- a/Server/tests/test_manage_animation.py +++ b/Server/tests/test_manage_animation.py @@ -186,6 +186,32 @@ def test_animator_play_builds_correct_params(self, runner, mock_config, mock_suc # stateName goes into properties (non-top-level key) assert params["properties"]["stateName"] == "Walk" + def test_animator_play_reports_the_object_the_tool_resolved(self, runner, mock_config): + """The Animator may live on a descendant of the named target, and only the tool + knows which object played. Echoing the CLI's own `target` back would contradict + the result it just printed.""" + resolved = { + "success": True, + "message": "Playing state 'Walk' on 'Rig' (resolved from 'Wrapper')", + "data": {}, + } + with patch("cli.commands.animation.get_config", return_value=mock_config): + with patch("cli.commands.animation.run_command", return_value=resolved): + result = runner.invoke(animation, ["animator", "play", "Wrapper", "Walk"]) + + assert "Rig" in result.output + assert "Playing state 'Walk' on Wrapper" not in result.output + + def test_animator_play_fallback_names_no_object(self, runner, mock_config): + """With no message the CLI cannot know whether the target or a descendant played, + so the fallback must not claim one.""" + with patch("cli.commands.animation.get_config", return_value=mock_config): + with patch("cli.commands.animation.run_command", return_value={"success": True, "data": {}}): + result = runner.invoke(animation, ["animator", "play", "Wrapper", "Walk"]) + + assert "Playing state 'Walk'" in result.output + assert "on Wrapper" not in result.output + def test_animator_play_with_layer(self, runner, mock_config, mock_success): with patch("cli.commands.animation.get_config", return_value=mock_config): with patch("cli.commands.animation.run_command", return_value=mock_success) as mock_run: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs new file mode 100644 index 000000000..7ed461607 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs @@ -0,0 +1,164 @@ +using NUnit.Framework; +using UnityEngine; +using MCPForUnity.Editor.Tools.Animation; +using static MCPForUnityTests.Editor.TestUtilities; + +namespace MCPForUnityTests.Editor.Tools +{ + /// + /// Direct coverage of the resolver every animator read and control response depends on, + /// including the Play-mode parameter branches that an EditMode test cannot drive. + /// + public class AnimatorResolverTests + { + private GameObject _root; + + [TearDown] + public void TearDown() + { + if (_root != null) + UnityEngine.Object.DestroyImmediate(_root); + } + + private GameObject Child(string name) + { + var go = new GameObject(name); + go.transform.SetParent(_root.transform); + return go; + } + + [Test] + public void Find_NullTarget_ReturnsNullAndEmptyCandidates() + { + var resolved = AnimatorResolver.Find(null, out var candidates); + Assert.IsNull(resolved); + Assert.IsNotNull(candidates, "candidates must never be null - callers pass it straight on"); + Assert.AreEqual(0, candidates.Length); + } + + [Test] + public void Find_NoAnimatorAnywhere_ReturnsNullAndEmptyCandidates() + { + _root = new GameObject("AnimResTest_Empty"); + Child("AnimResTest_EmptyChild"); + + var resolved = AnimatorResolver.Find(_root, out var candidates); + Assert.IsNull(resolved); + Assert.AreEqual(0, candidates.Length); + } + + [Test] + public void Find_AnimatorOnTarget_PrefersItOverDescendants() + { + _root = new GameObject("AnimResTest_Priority"); + var own = _root.AddComponent(); + Child("AnimResTest_PriorityChild").AddComponent(); + + var resolved = AnimatorResolver.Find(_root, out var candidates); + Assert.AreSame(own, resolved, "An Animator on the target wins over any descendant"); + Assert.AreEqual(0, candidates.Length, "The exact-target path reports no candidates"); + } + + [Test] + public void Find_SingleDescendantAnimator_ResolvesIt() + { + _root = new GameObject("AnimResTest_Single"); + var rig = Child("AnimResTest_SingleRig").AddComponent(); + + Assert.AreSame(rig, AnimatorResolver.Find(_root, out _)); + } + + [Test] + public void Find_InactiveDescendantAnimator_ResolvesIt() + { + _root = new GameObject("AnimResTest_Inactive"); + var child = Child("AnimResTest_InactiveRig"); + var rig = child.AddComponent(); + child.SetActive(false); + + Assert.AreSame(rig, AnimatorResolver.Find(_root, out _), + "A disabled rig is still readable"); + } + + [Test] + public void Find_SeveralDescendantAnimators_ReturnsNullAndReportsThem() + { + _root = new GameObject("AnimResTest_Ambiguous"); + Child("AnimResTest_RigA").AddComponent(); + Child("AnimResTest_RigB").AddComponent(); + + var resolved = AnimatorResolver.Find(_root, out var candidates); + Assert.IsNull(resolved, "An ambiguous request must not pick one silently"); + Assert.AreEqual(2, candidates.Length); + } + + [Test] + public void NotResolvedError_Ambiguous_NamesEveryCandidate() + { + _root = new GameObject("AnimResTest_ErrAmbiguous"); + Child("AnimResTest_ErrRigA").AddComponent(); + Child("AnimResTest_ErrRigB").AddComponent(); + + AnimatorResolver.Find(_root, out var candidates); + var error = ToJObject(AnimatorResolver.NotResolvedError(_root, candidates)); + + Assert.IsFalse(error.Value("success")); + string message = error["message"].ToString(); + StringAssert.Contains("AnimResTest_ErrRigA", message); + StringAssert.Contains("AnimResTest_ErrRigB", message); + } + + [Test] + public void NotResolvedError_NoCandidates_ReportsTargetAndChildren() + { + _root = new GameObject("AnimResTest_ErrEmpty"); + + AnimatorResolver.Find(_root, out var candidates); + var error = ToJObject(AnimatorResolver.NotResolvedError(_root, candidates)); + + Assert.IsFalse(error.Value("success")); + StringAssert.Contains("or its children", error["message"].ToString()); + } + + [Test] + public void ResolvedSuffix_TargetCarriesTheAnimator_IsEmpty() + { + _root = new GameObject("AnimResTest_SuffixSame"); + var own = _root.AddComponent(); + + Assert.AreEqual(string.Empty, AnimatorResolver.ResolvedSuffix(_root, own), + "A response that did not retarget must read exactly as before"); + } + + [Test] + public void ResolvedSuffix_Retargeted_NamesBothObjects() + { + _root = new GameObject("AnimResTest_SuffixRoot"); + var rig = Child("AnimResTest_SuffixRig").AddComponent(); + + string suffix = AnimatorResolver.ResolvedSuffix(_root, rig); + StringAssert.Contains("AnimResTest_SuffixRig", suffix); + StringAssert.Contains("AnimResTest_SuffixRoot", suffix); + } + + [Test] + public void Describe_TargetCarriesTheAnimator_NamesItOnce() + { + _root = new GameObject("AnimResTest_DescSame"); + var own = _root.AddComponent(); + + Assert.AreEqual("'AnimResTest_DescSame'", AnimatorResolver.Describe(_root, own)); + } + + [Test] + public void Describe_Retargeted_NamesBothObjects() + { + _root = new GameObject("AnimResTest_DescRoot"); + var rig = Child("AnimResTest_DescRig").AddComponent(); + + string described = AnimatorResolver.Describe(_root, rig); + StringAssert.Contains("AnimResTest_DescRig", described); + StringAssert.Contains("AnimResTest_DescRoot", described); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs.meta new file mode 100644 index 000000000..ad2df4c67 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AnimatorResolverTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: b2a73258659e4bb1a68221c941de6932 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageAnimationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageAnimationTests.cs index 277aabaeb..683d676b4 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageAnimationTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageAnimationTests.cs @@ -139,6 +139,346 @@ public void AnimatorGetInfo_WithAnimator_ReturnsData() } } + // ============================================================================= + // Animator: Child Resolution + // ============================================================================= + + [Test] + public void AnimatorGetInfo_AnimatorOnChild_ResolvesFromChild() + { + var root = new GameObject("AnimTest_ChildRoot"); + var model = new GameObject("AnimTest_ChildModel"); + model.transform.SetParent(root.transform); + model.AddComponent(); + try + { + Assert.IsNull(root.GetComponent()); + + var paramsObj = new JObject + { + ["action"] = "animator_get_info", + ["target"] = "AnimTest_ChildRoot" + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + + var data = result["data"] as JObject; + Assert.IsNotNull(data); + Assert.AreEqual("AnimTest_ChildRoot", data["gameObject"].ToString()); + Assert.AreEqual("AnimTest_ChildModel", data["animatorGameObject"].ToString(), + "Response must name the GameObject that actually carries the Animator"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorGetInfo_InactiveChildAnimator_ResolvesFromChild() + { + var root = new GameObject("AnimTest_InactiveRoot"); + var model = new GameObject("AnimTest_InactiveModel"); + model.transform.SetParent(root.transform); + model.AddComponent(); + model.SetActive(false); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_get_info", + ["target"] = "AnimTest_InactiveRoot" + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("AnimTest_InactiveModel", result["data"]["animatorGameObject"].ToString()); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorSetSpeed_AnimatorOnChild_AppliesToChildAnimator() + { + var root = new GameObject("AnimTest_ChildSpeedRoot"); + var model = new GameObject("AnimTest_ChildSpeedModel"); + model.transform.SetParent(root.transform); + var animator = model.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_set_speed", + ["target"] = "AnimTest_ChildSpeedRoot", + ["speed"] = 2.5f + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual(2.5f, animator.speed, 0.001f); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorSetSpeed_MultipleDescendantAnimators_RefusesAndNamesThem() + { + var root = new GameObject("AnimTest_AmbigRoot"); + var a = new GameObject("AnimTest_AmbigA"); + var b = new GameObject("AnimTest_AmbigB"); + a.transform.SetParent(root.transform); + b.transform.SetParent(root.transform); + var animA = a.AddComponent(); + var animB = b.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_set_speed", + ["target"] = "AnimTest_AmbigRoot", + ["speed"] = 2.5f + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsFalse(result.Value("success"), result.ToString()); + + var message = result["message"].ToString(); + Assert.That(message, Does.Contain("AnimTest_AmbigA")); + Assert.That(message, Does.Contain("AnimTest_AmbigB")); + Assert.AreEqual(1f, animA.speed, 0.001f, "An ambiguous request must not mutate a rig"); + Assert.AreEqual(1f, animB.speed, 0.001f, "An ambiguous request must not mutate a rig"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorSetSpeed_AnimatorOnChild_ResponseNamesTheChild() + { + var root = new GameObject("AnimTest_AttribRoot"); + var model = new GameObject("AnimTest_AttribModel"); + model.transform.SetParent(root.transform); + model.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_set_speed", + ["target"] = "AnimTest_AttribRoot", + ["speed"] = 2f + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.That(result["message"].ToString(), Does.Contain("AnimTest_AttribModel"), + "A retargeted control response must name the object it actually changed"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorGetInfo_TargetAndDescendantsHaveAnimators_UsesTheTarget() + { + var root = new GameObject("AnimTest_PriorityRoot"); + var child = new GameObject("AnimTest_PriorityChild"); + child.transform.SetParent(root.transform); + root.AddComponent(); + child.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_get_info", + ["target"] = "AnimTest_PriorityRoot" + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.AreEqual("AnimTest_PriorityRoot", result["data"]["animatorGameObject"].ToString(), + "An Animator on the exact target wins over any descendant"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorGetInfo_MultipleDescendantAnimators_RefusesAndNamesThem() + { + var root = new GameObject("AnimTest_AmbigReadRoot"); + var a = new GameObject("AnimTest_AmbigReadA"); + var b = new GameObject("AnimTest_AmbigReadB"); + a.transform.SetParent(root.transform); + b.transform.SetParent(root.transform); + a.AddComponent(); + b.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_get_info", + ["target"] = "AnimTest_AmbigReadRoot" + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsFalse(result.Value("success"), result.ToString()); + + var message = result["message"].ToString(); + Assert.That(message, Does.Contain("AnimTest_AmbigReadA")); + Assert.That(message, Does.Contain("AnimTest_AmbigReadB")); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + // Every resolver consumer, not just the two the other tests happen to call: a call site + // that drops the shared resolver must fail here rather than ship green. + [TestCase("animator_get_info")] + [TestCase("animator_get_parameter")] + [TestCase("animator_play")] + [TestCase("animator_crossfade")] + [TestCase("animator_set_parameter")] + [TestCase("animator_set_speed")] + [TestCase("animator_set_enabled")] + public void AnimatorActions_MultipleDescendantAnimators_AllRefuse(string action) + { + var root = new GameObject("AnimTest_AllRefuseRoot"); + var a = new GameObject("AnimTest_AllRefuseA"); + var b = new GameObject("AnimTest_AllRefuseB"); + a.transform.SetParent(root.transform); + b.transform.SetParent(root.transform); + var animA = a.AddComponent(); + var animB = b.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = action, + ["target"] = "AnimTest_AllRefuseRoot", + ["stateName"] = "Walk", + ["parameterName"] = "Speed", + ["value"] = 1f, + ["speed"] = 2f, + ["enabled"] = false + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsFalse(result.Value("success"), $"{action} must refuse an ambiguous target: {result}"); + + var message = result["message"].ToString(); + Assert.That(message, Does.Contain("AnimTest_AllRefuseA"), action); + Assert.That(message, Does.Contain("AnimTest_AllRefuseB"), action); + Assert.AreEqual(1f, animA.speed, 0.001f, $"{action} mutated a rig it should not have picked"); + Assert.AreEqual(1f, animB.speed, 0.001f, $"{action} mutated a rig it should not have picked"); + Assert.IsTrue(animA.enabled && animB.enabled, $"{action} mutated a rig it should not have picked"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorSetEnabled_AnimatorOnChild_ResponseNamesTheChild() + { + var root = new GameObject("AnimTest_EnabledRoot"); + var model = new GameObject("AnimTest_EnabledModel"); + model.transform.SetParent(root.transform); + var animator = model.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "animator_set_enabled", + ["target"] = "AnimTest_EnabledRoot", + ["enabled"] = false + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.IsFalse(animator.enabled); + Assert.That(result["message"].ToString(), Does.Contain("AnimTest_EnabledModel"), + "A retargeted control response must name the object it actually changed"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void AnimatorSetParameter_AnimatorOnChild_EditModeResponseNamesTheChild() + { + string controllerPath = $"{TempRoot}/ChildParam_{Guid.NewGuid():N}.controller"; + var controller = AnimatorController.CreateAnimatorControllerAtPath(controllerPath); + controller.AddParameter("Speed", AnimatorControllerParameterType.Float); + AssetDatabase.SaveAssets(); + + var root = new GameObject("AnimTest_ParamRoot"); + var model = new GameObject("AnimTest_ParamModel"); + model.transform.SetParent(root.transform); + var animator = model.AddComponent(); + animator.runtimeAnimatorController = controller; + try + { + var paramsObj = new JObject + { + ["action"] = "animator_set_parameter", + ["target"] = "AnimTest_ParamRoot", + ["parameterName"] = "Speed", + ["parameterType"] = "float", + ["value"] = 3f + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + Assert.That(result["message"].ToString(), Does.Contain("AnimTest_ParamModel"), + "A retargeted parameter write must disclose the Animator it resolved"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + + [Test] + public void ControllerAssign_AnimatorOnChild_AddsAnimatorToTarget() + { + string controllerPath = $"{TempRoot}/ChildAssign_{Guid.NewGuid():N}.controller"; + AnimatorController.CreateAnimatorControllerAtPath(controllerPath); + AssetDatabase.SaveAssets(); + + var root = new GameObject("AnimTest_AssignRoot"); + var model = new GameObject("AnimTest_AssignModel"); + model.transform.SetParent(root.transform); + var childAnimator = model.AddComponent(); + try + { + var paramsObj = new JObject + { + ["action"] = "controller_assign", + ["target"] = "AnimTest_AssignRoot", + ["controllerPath"] = controllerPath + }; + var result = ToJObject(ManageAnimation.HandleCommand(paramsObj)); + Assert.IsTrue(result.Value("success"), result.ToString()); + + Assert.IsNotNull(root.GetComponent(), + "Assign must add an Animator to the named target, not reuse a descendant's"); + Assert.IsNull(childAnimator.runtimeAnimatorController, + "The child Animator must be left untouched"); + } + finally + { + UnityEngine.Object.DestroyImmediate(root); + } + } + // ============================================================================= // Animator: Set Speed / Set Enabled // ============================================================================= From bb0294da88691e0342ca639d34147114ecc36950 Mon Sep 17 00:00:00 2001 From: Burak Erdemci Date: Fri, 21 Aug 2026 14:15:56 +0300 Subject: [PATCH 2/2] docs(cli): the animation commands no longer require the Animator on the target The usage guide still stated the old precondition, so a user reading it would retarget a command to the model child that the resolver now handles on its own. --- Server/src/cli/CLI_USAGE_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Server/src/cli/CLI_USAGE_GUIDE.md b/Server/src/cli/CLI_USAGE_GUIDE.md index a276bd15a..d3d57f341 100644 --- a/Server/src/cli/CLI_USAGE_GUIDE.md +++ b/Server/src/cli/CLI_USAGE_GUIDE.md @@ -606,7 +606,7 @@ unity-mcp audio volume "MusicPlayer" 0.5 ### Animation Commands ```bash -# Control Animator (target must have Animator component) +# Control Animator (target or one of its children must have an Animator component) unity-mcp animation play "Character" "Walk" unity-mcp animation set-parameter "Character" "Speed" 1.5 --type float unity-mcp animation set-parameter "Character" "IsRunning" true --type bool