Extend animator controller editing capabilities#1206
Conversation
📝 WalkthroughWalkthroughThe pull request extends the animation controller tooling with three major feature areas: lossless string-based instance ID serialization for cross-version object tracking, clip root motion trajectory sampling, and new blend tree and state property endpoints. A new ChangesAnimation Controller and Clip Property Endpoints
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Thaina
force-pushed
the
animation-node-graph-position-MCP
branch
from
4d8cf5d to
95b9c2f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs`:
- Around line 434-443: The GetStatePositions method currently collects and
returns all nodes in a single response without pagination, which violates coding
guidelines for endpoints that could return large datasets. Modify the method to
accept page_size and cursor parameters in the method signature, implement
pagination logic to slice the nodes list collected by CollectPositions based on
these parameters, and update the return object to include a next_cursor field
that indicates the position for the next page of results (or null if this is the
final page). This ensures large AnimatorControllers do not create oversized
payloads.
- Around line 474-481: The foreach loop iterating through positions assumes
every token item is a JObject that supports property indexing, but malformed
entries (nulls, primitives, or incompatible types) will throw exceptions. Before
accessing token["name"], token["x"], and token["y"], add a guard clause to
verify that token is a JObject instance; if not, skip that iteration using
continue or collect the error appropriately. This prevents crashes on malformed
position data while maintaining the existing logic for valid entries.
- Around line 473-503: The SetStatePositions method currently matches states by
name only, which is ambiguous since duplicate names are valid in Animator graphs
and could cause unintended updates across layers. Replace the name-only key in
the want dictionary with a unique state identifier that combines the layer
index, state-machine path, and state name (for example
"layer0/RootStateMachine/StateName"). Update the contract to expect this unique
identifier format instead of just "name", and modify the ApplyPositions method
(and any other matching logic around lines 507-521) to construct and match
against this same unique identifier when traversing the state machine hierarchy,
so that only the intended state is updated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10199cd6-3ddb-40ef-aa2e-d236839cdc09
📒 Files selected for processing (3)
MCPForUnity/Editor/Tools/Animation/ControllerCreate.csMCPForUnity/Editor/Tools/Animation/ManageAnimation.csServer/src/services/tools/manage_animation.py
There was a problem hiding this comment.
♻️ Duplicate comments (2)
MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs (2)
499-506:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
positionsitems before property indexing to avoid runtime exceptions.On Line 501–506, the loop indexes
token["..."]without verifying object shape. Non-object entries inpositionscan throw and convert client input errors into internal failures. Add anif (token is not JObject item) continue/collect-errorguard before field access.Minimal fix
foreach (var token in positions) { - string name = token["name"]?.ToString(); + if (token is not JObject item) + continue; + + string name = item["name"]?.ToString(); if (string.IsNullOrEmpty(name)) continue; - float x = token["x"]?.ToObject<float>() ?? 0f; - float y = token["y"]?.ToObject<float>() ?? 0f; - int? layer = token["layer"]?.ToObject<int>() ?? defaultLayer; + float x = item["x"]?.ToObject<float>() ?? 0f; + float y = item["y"]?.ToObject<float>() ?? 0f; + int? layer = item["layer"]?.ToObject<int>() ?? defaultLayer; want[$"{(layer.HasValue ? layer.Value.ToString() : "*")}:{name}"] = new Vector2(x, y); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs` around lines 499 - 506, In the foreach loop that iterates over positions, add a guard clause at the beginning to verify that each token is a JObject before attempting to access its properties using the indexer (e.g., token["name"]). If the token is not a JObject, use continue to skip it or handle the error appropriately. This prevents runtime exceptions when non-object entries exist in the positions collection and converts potential client input errors into graceful handling rather than internal failures.
497-507:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftState key is still ambiguous for duplicate names within the same layer.
On Line 497 and Line 541, matching is keyed by
{layer}:{name}(or wildcard), which is not unique across sub-state-machines in that layer. A single request can still move multiple unintended nodes when names repeat. The read/write contract needs a unique identifier (e.g., layer + state-machine path + state name) in bothGetStatePositionsoutput andSetStatePositionsmatching.Suggested direction
- // Returns [{ name, x, y, layer }] + // Returns [{ id, name, x, y, layer, path }] - want[$"{(layer.HasValue ? layer.Value.ToString() : "*")}:{name}"] = new Vector2(x, y); + // Expect `id` from GetStatePositions; use that as the only write key. + want[id] = new Vector2(x, y); - string scopedKey = $"{layer}:{name}"; - string anyKey = $"*:{name}"; + string scopedKey = BuildStateId(layer, stateMachinePath, name);Also applies to: 539-545
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs` around lines 497 - 507, The state positioning key format using only layer and name is ambiguous when duplicate state names exist in different sub-state-machines within the same layer. Update the key structure to include the state-machine path in addition to layer and name to ensure uniqueness. In the section around line 497-507 where the want dictionary is built, modify the key format from {layer}:{name} to include the full state-machine path (e.g., {layer}:{stateMachinePath}:{name}). Apply the same key format change in the matching logic around lines 539-545 to ensure consistency between how positions are stored and retrieved. This requires coordinating the GetStatePositions output format with the SetStatePositions matching logic to use the same unique identifier scheme.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs`:
- Around line 499-506: In the foreach loop that iterates over positions, add a
guard clause at the beginning to verify that each token is a JObject before
attempting to access its properties using the indexer (e.g., token["name"]). If
the token is not a JObject, use continue to skip it or handle the error
appropriately. This prevents runtime exceptions when non-object entries exist in
the positions collection and converts potential client input errors into
graceful handling rather than internal failures.
- Around line 497-507: The state positioning key format using only layer and
name is ambiguous when duplicate state names exist in different
sub-state-machines within the same layer. Update the key structure to include
the state-machine path in addition to layer and name to ensure uniqueness. In
the section around line 497-507 where the want dictionary is built, modify the
key format from {layer}:{name} to include the full state-machine path (e.g.,
{layer}:{stateMachinePath}:{name}). Apply the same key format change in the
matching logic around lines 539-545 to ensure consistency between how positions
are stored and retrieved. This requires coordinating the GetStatePositions
output format with the SetStatePositions matching logic to use the same unique
identifier scheme.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a2f12fc9-d9c0-495a-9b3f-ef909f5fbfe1
📒 Files selected for processing (2)
MCPForUnity/Editor/Tools/Animation/ControllerCreate.csServer/tests/test_manage_animation.py
Add long entityID for lossless convertion
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Server/tests/test_manage_animation.py (1)
75-82: ⚡ Quick winInclude
controller_remove_transitionin the expected controller action contract test.
CONTROLLER_ACTIONSnow includescontroller_remove_transition, but this expected-set assertion doesn’t check it, so allowlist regressions for that action can slip through.Suggested test update
def test_expected_controller_actions_present(self): expected = {"controller_create", "controller_add_state", "controller_set_state_properties", "controller_get_state_properties", - "controller_add_transition", + "controller_add_transition", "controller_remove_transition", "controller_add_parameter", "controller_get_info", "controller_assign", "controller_add_layer", "controller_remove_layer", "controller_set_layer_weight", "controller_create_blend_tree_1d", "controller_create_blend_tree_2d", "controller_add_blend_tree_child"} assert expected.issubset(set(CONTROLLER_ACTIONS))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Server/tests/test_manage_animation.py` around lines 75 - 82, The test_expected_controller_actions_present method has an expected set of controller actions but is missing controller_remove_transition, which is now part of CONTROLLER_ACTIONS. Add controller_remove_transition to the expected set in the assertion so that the test will catch if this action is accidentally removed from CONTROLLER_ACTIONS in the future. Include it in the set of expected action strings being checked against CONTROLLER_ACTIONS.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Server/tests/test_manage_animation.py`:
- Around line 75-82: The test_expected_controller_actions_present method has an
expected set of controller actions but is missing controller_remove_transition,
which is now part of CONTROLLER_ACTIONS. Add controller_remove_transition to the
expected set in the assertion so that the test will catch if this action is
accidentally removed from CONTROLLER_ACTIONS in the future. Include it in the
set of expected action strings being checked against CONTROLLER_ACTIONS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b812efa-0ba7-47b8-81bb-8bc31580c634
📒 Files selected for processing (4)
MCPForUnity/Editor/Tools/Animation/ControllerCreate.csMCPForUnity/Editor/Tools/Animation/ManageAnimation.csServer/src/services/tools/manage_animation.pyServer/tests/test_manage_animation.py
Thaina
force-pushed
the
animation-node-graph-position-MCP
branch
from
2f1fbdd to
cb7c478
Compare
add remove_transition for editing
Thaina
force-pushed
the
animation-node-graph-position-MCP
branch
from
cb7c478 to
3787ea3
Compare
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Thaina
changed the title
Read and edit existing BlendTree child motions (position, threshold, timeScale, cycleOffset, mirror) on an AnimatorController state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs`:
- Around line 444-495: The nested BlendTree created via CreateBlendTreeChild is
not being registered as a sub-asset in the controller, causing it to not persist
on reload. Currently the code only calls SetDirty(child) but lacks the
AssetDatabase.AddObjectToAsset(child, controller) call that is used in other
blend tree creation paths. To fix this, retrieve the controller reference from
the `@params` (using controllerPath and AssetDatabase.LoadAssetAtPath if
necessary), and then call AssetDatabase.AddObjectToAsset(child, controller)
before or after the existing SetDirty(child) call to ensure the nested blend
tree is properly registered as a sub-asset of the controller.
In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs`:
- Around line 188-192: The code currently ignores invalid interruptionSource
enum values when Enum.TryParse fails, making configuration errors silent and
undetectable. Add validation by checking the boolean return value of
Enum.TryParse when parsing the interruptionSource parameter. When the parse
fails (returns false), return a validation error instead of silently proceeding
with the default value. This ensures invalid enum values are properly reported
rather than ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04710fb1-147b-4f5f-badf-a7085c05fec3
📒 Files selected for processing (8)
MCPForUnity/Editor/Tools/Animation/ClipCreate.csMCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.csMCPForUnity/Editor/Tools/Animation/ControllerCreate.csMCPForUnity/Editor/Tools/Animation/ManageAnimation.csMCPForUnity/Runtime/Helpers/UnityObjectIdCompat.csServer/pyproject.tomlServer/src/services/tools/manage_animation.pyServer/tests/test_manage_animation.py
✅ Files skipped from review due to trivial changes (1)
- Server/pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (2)
- Server/src/services/tools/manage_animation.py
- Server/tests/test_manage_animation.py
| if (@params["addChildTree"] is JObject ct) | ||
| { | ||
| BlendTree child; | ||
| if (blendTree.blendType == BlendTreeType.Simple1D) | ||
| { | ||
| float thr = ct["threshold"]?.ToObject<float>() ?? 0f; | ||
| child = blendTree.CreateBlendTreeChild(thr); | ||
| } | ||
| else | ||
| { | ||
| if (!(ct["position"] is JArray cpos) || cpos.Count < 2) | ||
| return new { success = false, message = "addChildTree.position [x,y] is required when parent is a 2D tree" }; | ||
| child = blendTree.CreateBlendTreeChild(new Vector2(cpos[0].ToObject<float>(), cpos[1].ToObject<float>())); | ||
| } | ||
|
|
||
| child.name = ct["name"]?.ToString() ?? "Look Blend Tree"; | ||
| child.hideFlags = HideFlags.HideInHierarchy; | ||
| if (ct["blendType"] != null && Enum.TryParse<BlendTreeType>(ct["blendType"].ToString(), true, out var cbt)) | ||
| child.blendType = cbt; | ||
| if (ct["blendParameter"] != null) child.blendParameter = ct["blendParameter"].ToString(); | ||
| if (ct["blendParameterY"] != null) child.blendParameterY = ct["blendParameterY"].ToString(); | ||
| if (ct["useAutomaticThresholds"] != null) child.useAutomaticThresholds = ct["useAutomaticThresholds"].ToObject<bool>(); | ||
|
|
||
| int nestedAdded = 0; | ||
| if (ct["children"] is JArray nestedKids) | ||
| { | ||
| foreach (var kt in nestedKids) | ||
| { | ||
| if (!(kt is JObject kid)) continue; | ||
| AnimationClip clip = null; | ||
| string cInst = kid["clipInstanceId"]?.ToString(); | ||
| string cPath = kid["clipPath"]?.ToString(); | ||
| if (!string.IsNullOrEmpty(cInst)) | ||
| clip = UnityObjectIdCompat.InstanceIDFromString(cInst) as AnimationClip; | ||
| else if (!string.IsNullOrEmpty(cPath)) | ||
| clip = AssetDatabase.LoadAssetAtPath<AnimationClip>(AssetPathUtility.SanitizeAssetPath(cPath)); | ||
| if (clip == null) | ||
| return new { success = false, message = $"addChildTree child clip not resolved (instanceId '{cInst}', path '{cPath}')" }; | ||
|
|
||
| if (child.blendType == BlendTreeType.Simple1D) | ||
| child.AddChild(clip, kid["threshold"]?.ToObject<float>() ?? 0f); | ||
| else if (kid["position"] is JArray kpos && kpos.Count >= 2) | ||
| child.AddChild(clip, new Vector2(kpos[0].ToObject<float>(), kpos[1].ToObject<float>())); | ||
| else | ||
| child.AddChild(clip); | ||
| nestedAdded++; | ||
| } | ||
| } | ||
|
|
||
| EditorUtility.SetDirty(child); | ||
| nestedInfo = new { name = child.name, blendType = child.blendType.ToString(), blendParameter = child.blendParameter, childCount = child.children.Length, added = nestedAdded }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical
🧩 Analysis chain
🌐 Web query:
Unity BlendTree.CreateBlendTreeChild AddObjectToAsset persist sub-asset AnimatorController
💡 Result:
To persist a BlendTree and its children as sub-assets of an AnimatorController, you must explicitly use AssetDatabase.AddObjectToAsset [1][2]. When you create a BlendTree or a child BlendTree via script, it exists only in memory [1]. To ensure it is saved alongside the AnimatorController asset, follow these guidelines: 1. Use AnimatorController.CreateBlendTreeInController to initialize the primary BlendTree [3][4]. This method automatically adds the new BlendTree as a sub-asset to the AnimatorController [3]. 2. When manually creating child BlendTrees (e.g., using BlendTree.CreateBlendTreeChild), these are generally added as sub-assets by the internal implementation [5][6]. However, if you are manually creating and adding Motion objects or nested BlendTrees, you must call AssetDatabase.AddObjectToAsset(childObject, animatorController) to ensure they are serialized into the controller file [1][7][2]. 3. Always call AssetDatabase.SaveAssets after modifying the asset structure [1][8]. Failure to do so may result in the loss of in-memory changes [1]. Example pattern: using UnityEditor; using UnityEditor.Animations; using UnityEngine; // 1. Create or load your controller AnimatorController controller = AnimatorController.CreateAnimatorControllerAtPath("Assets/MyController.controller"); // 2. Create the main BlendTree within the controller controller.CreateBlendTreeInController("MyBlendTree", out BlendTree rootTree); // 3. Add a child BlendTree BlendTree childTree = rootTree.CreateBlendTreeChild(0.5f); // 4. If creating other sub-assets manually, add them to the controller // AssetDatabase.AddObjectToAsset(myNewObject, controller); // 5. Persist changes AssetDatabase.SaveAssets; Note that BlendTrees added as sub-assets are typically hidden from the Project window by default (using HideFlags) [6][2]. This is standard behavior for Unity animation sub-assets [2]. [5][3]
Citations:
- 1: http://unity3d.com/support/documentation/ScriptReference/AssetDatabase.AddObjectToAsset.html
- 2: https://discussions.unity.com/t/how-to-save-the-animation-blend-tree-created-by-script/669361
- 3: https://docs.unity3d.com/ScriptReference/Animations.AnimatorController.CreateBlendTreeInController.html
- 4: https://github.com/Unity-Technologies/UnityCsReference/blob/master/Editor/Mono/Animation/AnimatorController.cs/
- 5: https://docs.unity3d.com/ScriptReference/Animations.BlendTree.CreateBlendTreeChild.html
- 6: https://github.com/Unity-Technologies/UnityCsReference/blob/master/Editor/Mono/Animation/BlendTree.cs
- 7: https://docs.unity3d.com/ScriptReference/Motion.html
- 8: https://docs.unity3d.com/6000.5/Documentation/ScriptReference/AssetDatabase.AddObjectToAsset.html
🏁 Script executed:
ast-grep outline MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.csRepository: CoplayDev/unity-mcp
Length of output: 230
🏁 Script executed:
ls -la MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.csRepository: CoplayDev/unity-mcp
Length of output: 266
🏁 Script executed:
wc -l MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs && cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '400,550p'Repository: CoplayDev/unity-mcp
Length of output: 7081
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '1,150p' | grep -A5 -B5 "CreateBlendTree"Repository: CoplayDev/unity-mcp
Length of output: 1090
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '13,130p'Repository: CoplayDev/unity-mcp
Length of output: 6262
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '200,350p'Repository: CoplayDev/unity-mcp
Length of output: 7668
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '349,395p'Repository: CoplayDev/unity-mcp
Length of output: 3495
🏁 Script executed:
rg "CreateBlendTreeChild" MCPForUnity/ -A 5 -B 2Repository: CoplayDev/unity-mcp
Length of output: 1812
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '272,310p'Repository: CoplayDev/unity-mcp
Length of output: 2437
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '349,360p'Repository: CoplayDev/unity-mcp
Length of output: 767
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '349,510p' | head -80Repository: CoplayDev/unity-mcp
Length of output: 5580
🏁 Script executed:
cat -n MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs | sed -n '349,507p'Repository: CoplayDev/unity-mcp
Length of output: 10613
Add explicit AssetDatabase.AddObjectToAsset(child, controller) to persist the nested blend tree.
The nested BlendTree created via CreateBlendTreeChild lacks explicit sub-asset registration. Unlike CreateBlendTree1D/2D (which call AssetDatabase.AddObjectToAsset(blendTree, controller) at line 53 and 124), this code calls only SetDirty(child) without registering the child into the controller asset. The controller reference is also unavailable in EditBlendTree scope since ResolveBlendTree only returns the blendTree, not the controller.
Retrieve the controller (e.g., via AssetDatabase.LoadAssetAtPath using controllerPath from @params) and call AssetDatabase.AddObjectToAsset(child, controller) before or after SetDirty(child) to ensure the nested tree persists on reload.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@MCPForUnity/Editor/Tools/Animation/ControllerBlendTrees.cs` around lines 444
- 495, The nested BlendTree created via CreateBlendTreeChild is not being
registered as a sub-asset in the controller, causing it to not persist on
reload. Currently the code only calls SetDirty(child) but lacks the
AssetDatabase.AddObjectToAsset(child, controller) call that is used in other
blend tree creation paths. To fix this, retrieve the controller reference from
the `@params` (using controllerPath and AssetDatabase.LoadAssetAtPath if
necessary), and then call AssetDatabase.AddObjectToAsset(child, controller)
before or after the existing SetDirty(child) call to ensure the nested blend
tree is properly registered as a sub-asset of the controller.
| if (@params["interruptionSource"] != null) | ||
| { | ||
| if (Enum.TryParse<TransitionInterruptionSource>(@params["interruptionSource"].ToString(), true, out var src)) | ||
| transition.interruptionSource = src; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return a validation error for invalid interruptionSource values.
Invalid enum text is currently ignored, so the API reports success while keeping Unity defaults. This makes transition configuration failures hard to detect.
Suggested fix
if (`@params`["interruptionSource"] != null)
{
- if (Enum.TryParse<TransitionInterruptionSource>(`@params`["interruptionSource"].ToString(), true, out var src))
- transition.interruptionSource = src;
+ if (!Enum.TryParse<TransitionInterruptionSource>(`@params`["interruptionSource"].ToString(), true, out var src))
+ return new
+ {
+ success = false,
+ message = $"Invalid 'interruptionSource'. Valid values: {string.Join(", ", Enum.GetNames(typeof(TransitionInterruptionSource)))}"
+ };
+ transition.interruptionSource = src;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@MCPForUnity/Editor/Tools/Animation/ControllerCreate.cs` around lines 188 -
192, The code currently ignores invalid interruptionSource enum values when
Enum.TryParse fails, making configuration errors silent and undetectable. Add
validation by checking the boolean return value of Enum.TryParse when parsing
the interruptionSource parameter. When the parse fails (returns false), return a
validation error instead of silently proceeding with the default value. This
ensures invalid enum values are properly reported rather than ignored.
1 participant
Description
adding
get_state_propertiesandset_state_propertiesto the mcp managed animation controller so that AI can modified the node graph position in addition to create and set/remove its transition (RemoveTransition added too)Type of Change
Changes Made
Compatibility / Package Source
Packages/packages-lock.json: 4d8cf5dTesting/Screenshots/Recordings
cd Server && uv run pytest tests/ -v)Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)Related Issues
Additional Notes
Summary by CodeRabbit
Summary