Harden manage_scriptable_object Tool
#522
Conversation
…auto-resize, bulk mapping Phase 1: Path Syntax & Auto-Resizing - Add NormalizePropertyPath() to convert field[index] to Array.data format - Add EnsureArrayCapacity() to auto-grow arrays when targeting out-of-bounds indices Phase 2: Consolidation - Replace duplicate TryGet* helpers with ParamCoercion/VectorParsing shared utilities - Add Vector4 parsing support to VectorParsing.cs Phase 3: Bulk Data Mapping - Handle JArray values for list/array properties (recursive element setting) - Handle JObject values for nested struct/class properties Phase 4: Enhanced Reference Resolution - Support plain 32-char GUID strings for ObjectReference fields Phase 5: Validation & Dry-Run - Add ValidatePatches() for pre-validation of all patches - Add dry_run parameter to validate without mutating Includes comprehensive stress test suite covering: - Big Bang (large nested arrays), Out of Bounds, Friendly Path Syntax - Deep Nesting, Mixed References, Rapid Fire, Type Mismatch - Bulk Array Mapping, GUID Shorthand, Dry Run validation
…object tool
- Implement TrySetAnimationCurve() supporting both {'keys': [...]} and direct [...] formats
* Support keyframe properties: time, value, inSlope, outSlope, weightedMode, inWeight, outWeight
* Gracefully default missing optional fields to 0
* Clear error messages for malformed structures
- Implement TrySetQuaternion() with 4 input formats:
* Euler array [x, y, z] - 3 elements interpreted as degrees
* Raw array [x, y, z, w] - 4 components
* Object format {x, y, z, w} - explicit components
* Explicit euler {euler: [x, y, z]} - labeled format
- Improve error handling:
* Null values: AnimationCurve→empty, Quaternion→identity
* Invalid inputs rejected with specific, actionable error messages
* Validate keyframe objects and array sizes
- Add comprehensive test coverage in ManageScriptableObjectStressTests.cs:
* AnimationCurve with keyframe array format
* AnimationCurve with direct array (no wrapper)
* Quaternion via Euler angles
* Quaternion via raw components
* Quaternion via object format
* Quaternion via explicit euler property
- Fix test file compilation issues:
* Replace undefined TestFolder with _runRoot
* Add System.IO using statement
- Add TestUtilities.cs with shared helpers: - ToJObject() - consolidates 11 duplicates across test files - EnsureFolder() - consolidates 2 duplicates - WaitForUnityReady() - consolidates 2 duplicates - FindFallbackShader() - consolidates shader chain duplicates - SafeDeleteAsset() - helper for asset cleanup - CleanupEmptyParentFolders() - standardizes TearDown cleanup - Update 11 test files to use shared TestUtilities via 'using static' - Standardize TearDown cleanup patterns across all test files - Net reduction of ~40 lines while improving maintainability
Add AnimationCurve and Quaternion fields required by Phase 6 stress tests.
Reviewer's GuideHardens the ManageScriptableObject Unity editor tool with path normalization, auto-growing arrays, extended SerializedProperty type support (including AnimationCurve and Quaternion), and a dry-run validation mode, while consolidating shared Unity test helpers and adding a large stress-test suite plus fixtures for ScriptableObject operations. Sequence diagram for manage_scriptable_object modify with dry-run validationsequenceDiagram
actor Caller
participant PythonTool as manage_scriptable_object_py
participant UnityBridge as UnityMCPServer
participant ManageTool as ManageScriptableObject
participant SO as ScriptableObject
Caller->>PythonTool: manage_scriptable_object(action="modify", target, patches, dry_run)
PythonTool->>PythonTool: coerce_bool(dry_run)
PythonTool->>UnityBridge: send request { action: modify, target, patches, dryRun }
UnityBridge->>ManageTool: HandleModify(params)
ManageTool->>ManageTool: TryResolveTarget(target)
ManageTool->>SO: new SerializedObject(SO)
ManageTool->>ManageTool: parse patches JArray
ManageTool->>ManageTool: dryRun = params["dryRun"]
alt dryRun == true
ManageTool->>ManageTool: ValidatePatches(SO, patches)
loop each patch
ManageTool->>ManageTool: NormalizePropertyPath(propertyPath)
alt op == array_resize
ManageTool->>SO: FindProperty(arrayPath)
ManageTool-->>ManageTool: validationResult(ok, message, currentSize)
else op == set
ManageTool->>SO: FindProperty(normalizedPath)
alt property found
ManageTool-->>ManageTool: validationResult(ok=true, propertyType)
else auto-growable array
ManageTool-->>ManageTool: validationResult(ok=true, will auto-grow)
else not found
ManageTool-->>ManageTool: validationResult(ok=false, message)
end
end
end
ManageTool-->>UnityBridge: SuccessResponse { dryRun:true, valid, validationResults }
else dryRun == false
ManageTool->>ManageTool: ApplyPatches(SO, patches)
loop each patch
ManageTool->>ManageTool: ApplyPatch(SerializedObject, propertyPath, op, patch)
ManageTool->>ManageTool: NormalizePropertyPath(propertyPath)
alt op == array_resize
ManageTool->>ManageTool: ApplyArrayResize
else op == set
ManageTool->>ManageTool: ApplySet (EnsureArrayCapacity, TrySetValueRecursive)
end
end
ManageTool-->>UnityBridge: SuccessResponse { results, warnings }
end
UnityBridge-->>PythonTool: JSON response
PythonTool-->>Caller: parsed response
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis pull request introduces dry-run validation mode for the ManageScriptableObject tool, adds ParseVector4 support to vector parsing, implements path normalization for property access, enables automatic array capacity handling, creates new specialized setters for AnimationCurve and Quaternion types, and establishes centralized test utilities with comprehensive stress tests covering nested structures, array operations, and multiple data types. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom Pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs:
- Around line 113-130: The catch block around
Directory.GetDirectories/Directory.GetFiles/AssetDatabase.DeleteAsset silently
swallows exceptions; update the catch in the cleanup loop to at least log the
caught exception (e.g., using UnityEngine.Debug.LogException or Debug.LogWarning
with context like the current parent path) and then break/return as appropriate
so failures aren't hidden—locate the try/catch that references parent,
Directory.GetDirectories, Directory.GetFiles and AssetDatabase.DeleteAsset and
replace the empty catch with logging of the exception and a clear break.
🧹 Nitpick comments (5)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.cs (1)
17-17: Consider initializing thescoreslist.
List<float> scoreswill benullby default. If stress tests access this field before initialization, it could causeNullReferenceException. Unity's serialization will handle this during asset creation, but explicit initialization can make the fixture more robust.🔎 Optional: Initialize scores list
public class ComplexSubClass { public string name; public int level; - public List<float> scores; + public List<float> scores = new List<float>(); }TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ArrayStressSO.cs (1)
1-7: Consider adding a namespace declaration.While this test fixture follows the same pattern as other stress test ScriptableObjects (DeepStressSO, ComplexStressSO), it lacks a namespace declaration. Adding a namespace like
MCPForUnityTests.Editor.Tools.Fixtures.StressTestSOswould follow C# best practices and reduce the risk of type name collisions.🔎 Proposed refactor
using UnityEngine; + +namespace MCPForUnityTests.Editor.Tools.Fixtures.StressTestSOs +{ + [CreateAssetMenu(fileName = "ArrayStressSO", menuName = "StressTests/ArrayStressSO")] + public class ArrayStressSO : ScriptableObject + { + public float[] floatArray = new float[3]; + } +} - -[CreateAssetMenu(fileName = "ArrayStressSO", menuName = "StressTests/ArrayStressSO")] -public class ArrayStressSO : ScriptableObject -{ - public float[] floatArray = new float[3]; -}TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.cs (1)
2-2: Unused import.
System.Collections.Genericis not used in this file.🔎 Proposed fix
using UnityEngine; -using System.Collections.Generic;MCPForUnity/Editor/Tools/ManageScriptableObject.cs (1)
230-248: Using reflection to check anonymous type property is fragile.Line 244 uses reflection (
GetProperty("ok")?.GetValue(r)) to access theokproperty of anonymous result objects. This works but is fragile—property name typos or refactoring could silently break validation. Consider using a named result type or LINQ with pattern matching.🔎 Proposed fix
- valid = validationResults.All(r => (bool)r.GetType().GetProperty("ok")?.GetValue(r)), + valid = validationResults.All(r => r is { } obj && + obj.GetType().GetProperty("ok")?.GetValue(obj) is bool ok && ok),Or define a simple record/class for validation results to avoid reflection entirely.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs (1)
1082-1095: Test does not exercise any code paths.This test immediately calls
Assert.Pass()without actually verifying unsupported type behavior. Consider either implementing a real test or removing this placeholder.🔎 Proposed fix
Either implement a proper test using a fixture with an unsupported type, or convert to a code comment and remove the empty test:
- /// <summary> - /// Test: Unsupported type returns a helpful error message. - /// </summary> - [UnityTest] - public IEnumerator UnsupportedType_ReturnsHelpfulError() - { - yield return WaitForUnityReady(); - - // This test verifies that the improved error message is returned - // We can't easily test an actual unsupported type without creating a custom SO, - // so we just verify the error message format by checking the code path exists. - // The actual unsupported type behavior is implicitly tested if we ever add - // a field that hits the default case. - - Debug.Log("[UnsupportedType] Error message improvement verified in code review"); - Assert.Pass("Error message improvement verified in code"); - } + // NOTE: Unsupported type error messaging is verified via code review. + // A dedicated test would require a custom SO with a field of an unsupported SerializedPropertyType.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
MCPForUnity/Editor/Helpers/VectorParsing.csMCPForUnity/Editor/Tools/ManageScriptableObject.csServer/src/services/tools/manage_scriptable_object.pyServer/tests/integration/test_manage_scriptable_object_tool.pyTestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ArrayStressSO.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ArrayStressSO.cs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.cs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.cs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs.metaTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.cs
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-29T15:23:11.613Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:11.613Z
Learning: In MCPForUnity, prefer relying on the established testing process to catch UI initialization issues instead of adding defensive null checks for UI elements in editor windows. This means during reviews, verify that tests cover UI initialization paths and that code avoids repetitive null-check boilerplate in Editor Windows. If a UI element can be null, ensure there is a well-tested fallback or that its initialization is guaranteed by design, rather than sprinkling null checks throughout editor code.
Applied to files:
MCPForUnity/Editor/Helpers/VectorParsing.csMCPForUnity/Editor/Tools/ManageScriptableObject.cs
📚 Learning: 2025-09-03T16:00:55.839Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 0
File: :0-0
Timestamp: 2025-09-03T16:00:55.839Z
Learning: ComponentResolver in UnityMcpBridge/Editor/Tools/ManageGameObject.cs is a nested static class within ManageGameObject, not a sibling type. The `using static MCPForUnity.Editor.Tools.ManageGameObject;` import is required to access ComponentResolver methods directly without the outer class qualifier.
Applied to files:
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.csMCPForUnity/Editor/Tools/ManageScriptableObject.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs
📚 Learning: 2025-12-29T15:23:17.871Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 491
File: MCPForUnity/Editor/Windows/EditorPrefs/EditorPrefsWindow.cs:78-115
Timestamp: 2025-12-29T15:23:17.871Z
Learning: In the MCPForUnity project, the team prefers to rely on their testing process to catch UI initialization issues rather than adding defensive null checks for UI elements in editor windows.
Applied to files:
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.csMCPForUnity/Editor/Tools/ManageScriptableObject.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.csTestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs
🧬 Code graph analysis (9)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ArrayStressSO.cs (2)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.cs (1)
CreateAssetMenu(27-52)TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.cs (1)
CreateAssetMenu(25-30)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.cs (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (2)
TestUtilities(15-138)CleanupEmptyParentFolders(103-137)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (2)
TestUtilities(15-138)CleanupEmptyParentFolders(103-137)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.cs (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (1)
TestUtilities(15-138)
Server/tests/integration/test_manage_scriptable_object_tool.py (2)
Server/tests/integration/test_helpers.py (2)
DummyContext(16-55)set_state(49-51)Server/src/services/tools/manage_scriptable_object.py (1)
manage_scriptable_object(32-80)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (2)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
AssetPathUtility(15-212)SanitizeAssetPath(31-45)MCPForUnity/Editor/Tools/ManageAsset.cs (2)
CreateFolder(279-332)DeleteAsset(495-523)
Server/src/services/tools/manage_scriptable_object.py (1)
Server/src/services/tools/utils.py (1)
coerce_bool(11-24)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs (1)
ManageScriptableObjectTestDefinition(14-23)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs (1)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (1)
TestUtilities(15-138)
🪛 Ruff (0.14.10)
Server/tests/integration/test_manage_scriptable_object_tool.py
75-75: Unused function argument: kwargs
(ARG001)
106-106: Unused function argument: kwargs
(ARG001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (46)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs.meta (1)
1-8: LGTM!This is a standard Unity folder metadata file. The GUID is valid, the folderAsset flag is correctly set, and the structure conforms to Unity conventions for asset metadata.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs.meta (1)
1-11: Meta file structure is correct.The Unity meta file follows standard conventions with a valid GUID and proper MonoImporter configuration for the TestUtilities.cs script.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs.meta (1)
1-11: Meta file structure is correct.The Unity meta file follows standard conventions with a valid GUID and proper MonoImporter configuration for the stress test script.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.cs.meta (1)
1-11: Meta file structure is correct.The Unity meta file follows standard conventions with a valid GUID and proper MonoImporter configuration for the fixture script.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs (1)
12-12: LGTM! Clean refactor to shared test utility.The switch from a local
ToJObjecthelper to the sharedTestUtilities.ToJObjectvia static import reduces duplication and aligns with the test infrastructure consolidation across the PR.Server/src/services/tools/manage_scriptable_object.py (2)
44-45: LGTM! Clean addition of dry_run parameter.The parameter follows the established pattern (matching
overwrite), accepts both bool and string for LLM flexibility, and usescoerce_boolconsistently. The "(modify only)" constraint in the docstring matches the PR objectives.
67-67: Consistent forwarding to Unity.The
dryRunkey is correctly forwarded withcoerce_boolcoercion and will be filtered out whenNone(line 71), keeping the Unity handler interface clean.Server/tests/integration/test_manage_scriptable_object_tool.py (3)
72-100: Good test coverage for dry_run forwarding.The test properly verifies that
dry_run=Trueis forwarded asdryRun=Truein the Unity command payload, along with correct handling oftargetandpatches.
102-128: Thorough string coercion test.Validates that string
"true"is correctly coerced to booleanTrue, which is important for LLM-generated inputs that may stringify boolean values.
75-78: The**kwargsis intentional for signature compatibility.Ruff flags unused
kwargs(ARG001), but this is a false positive. The**kwargsis required to match the signature ofasync_send_command_with_retrybeing monkeypatched. This pattern is consistent with the existing tests at lines 10-13 and 46-49.Also applies to: 106-109
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.cs (1)
1-52: Well-structured stress test fixture.The
ComplexStressSOprovides comprehensive field coverage for testing the hardened ManageScriptableObject tool:
- Basic types (int, float, string, bool, Vector3, Color, enum)
- Collections (arrays and lists)
- Nested types (struct and class)
- Extended types (AnimationCurve, Quaternion)
This aligns well with the PR's goals of testing path normalization, auto-resizing arrays, and extended type support.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ComplexStressSO.cs.meta (1)
1-11: Standard Unity meta file.The metadata is correctly structured with a unique GUID and default MonoImporter settings.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/ArrayStressSO.cs.meta (1)
1-11: Standard Unity meta file.The metadata is correctly structured for the ArrayStressSO test fixture.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ReadConsoleTests.cs (1)
6-6: LGTM! Consistent refactor to shared test utilities.The static import of
TestUtilitiesaligns with the PR's consolidation of test helpers across 11 test files, reducing code duplication.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManagePrefabsTests.cs (1)
8-8: LGTM! Clean refactoring to centralized test utilities.The static import and shared cleanup logic eliminate code duplication across the test suite. The
CleanupEmptyParentFolderscall correctly handles empty parent directories after asset deletion.Also applies to: 34-35
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs (1)
7-7: LGTM! Consistent refactoring to shared test utilities.The changes align with the test infrastructure improvements across the suite, reducing duplication and improving maintainability.
Also applies to: 40-41
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs (1)
8-8: LGTM! Test utility consolidation.Consistent with the refactoring pattern applied across the test suite. The centralized utilities improve maintainability and reduce duplication.
Also applies to: 55-56
MCPForUnity/Editor/Helpers/VectorParsing.cs (1)
99-140: LGTM! ParseVector4 follows established patterns.The implementation is consistent with
ParseVector2andParseVector3, supporting both array[x,y,z,w]and object{x,y,z,w}formats. Error handling and documentation align with existing methods in this class.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs (1)
7-7: LGTM! Consolidation to centralized test utilities.The refactoring reduces duplication and aligns with the test infrastructure improvements across the PR.
Also applies to: 38-39
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs (1)
10-10: LGTM! Test utilities consolidation improves maintainability.The refactor to use shared
TestUtilities(FindFallbackShader, CleanupEmptyParentFolders) reduces duplication and centralizes common test helpers. The static import enables clean, unqualified method calls throughout the test file.Also applies to: 44-44, 75-76
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialDirectPropertiesTests.cs (1)
8-8: LGTM! Consistent test utilities refactor.The addition of the static
TestUtilitiesimport and the switch toCleanupEmptyParentFoldersaligns with the broader test suite refactor, reducing duplication and improving maintainability.Also applies to: 61-62
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.cs (1)
8-8: LGTM! Consistent test utilities refactor.The changes mirror the pattern used across the test suite—consolidating cleanup and helper methods into shared
TestUtilitiesfor better maintainability.Also applies to: 46-47
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.cs (1)
8-8: LGTM! Consistent test utilities refactor.The refactor to use shared
TestUtilitiesand centralized cleanup logic is consistent with the other test files in this PR, improving code reuse and reducing duplication.Also applies to: 66-67
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/StressTestSOs/DeepStressSO.cs (1)
4-30: Well-structured test fixture for deep nesting scenarios.The nested
Level1 → Level2 → Level3hierarchy with mixed types (string, Vector3, Color) provides good coverage for the path traversal and deep nesting stress tests.TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs (3)
21-25: LGTM!Clean null-safe conversion with appropriate fallback to
JObject.FromObjectfor non-JObject results.
32-52: LGTM!Properly delegates path sanitization to
AssetPathUtility.SanitizeAssetPathand iteratively creates parent folders. This aligns with the pattern used inManageAsset.cs.
60-71: LGTM!Appropriate timeout mechanism with
Assert.Failfor test failures when Unity is stuck compiling/updating.MCPForUnity/Editor/Tools/ManageScriptableObject.cs (12)
265-405: Dry-run validation logic is well-structured.The
ValidatePatchesmethod correctly:
- Normalizes paths before validation
- Checks array existence and type for
array_resize- Detects auto-growable array paths
- Returns detailed validation results with property type info
This provides useful feedback without mutating assets.
485-521: Path normalization regex handles edge cases correctly.The logic to avoid double-conversion (checking if already in
.Array.data[n]format) is sound. The bounds check at line 509 (matchStart >= 7) correctly ensures we can safely look back 7 characters for.Array..
523-566: Auto-resize array capacity handling is robust.
EnsureArrayCapacitycorrectly:
- Parses the array path and target index
- Validates the array property exists and is actually an array
- Grows the array and applies modifications immediately
The immediate
ApplyModifiedProperties()+Update()pattern ensures subsequent property lookups succeed.
662-702: LGTM!Good error messaging when array auto-grow fails—provides specific context about whether the array itself wasn't found or isn't actually an array.
706-765: Enhanced reference resolution with multiple formats.Good addition of GUID shorthand (32-char hex string) and path shorthand detection. The
resolveMethodtracking in the response message helps with debugging reference resolution issues.
790-846: Bulk array mapping with recursion depth guard.The
MaxRecursionDepth = 20guard prevents stack overflow on circular references. The partial success handling (returning true ifsuccessCount > 0) is reasonable for bulk operations.
848-886: Bulk object mapping for nested structs.Recursively setting fields via
JObjectkey-value pairs with proper child path construction (prop.propertyPath + "." + kvp.Key) is clean. Partial success semantics match the array handling above.
892-942: Improved type coercion using shared helpers.Using
ParamCoercion.CoerceInt/Float/Boolprovides consistent parsing across the codebase. The additional validation for boolean (checking for recognized string values) is a good safeguard.
1000-1004: Extended type support for AnimationCurve and Quaternion.Good addition of these commonly-needed Unity types to the switch statement, delegating to specialized setters.
1058-1143: Flexible AnimationCurve input formats.Accepting both
{ "keys": [...] }and direct array[...]provides good DX. Support for optional weighted tangent properties (weightedMode,inWeight,outWeight) covers advanced use cases.
1145-1240: Comprehensive Quaternion input format support.Four input formats covered:
[x, y, z]as Euler (degrees)[x, y, z, w]as raw components{ x, y, z, w }as object{ euler: [x, y, z] }for explicit EulerClear error messages guide users to valid formats.
1019-1023: Helpful error message for unsupported types.The extended error message with actionable suggestions ("Consider editing the .asset file directly...") is user-friendly.
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs (7)
34-61: Well-structured test setup with proper isolation.Using
Guid.NewGuid()for unique run directories ensures test isolation. Creating test assets (Material, Texture2D) for reference tests is appropriate setup.
63-84: Thorough teardown with asset cleanup.Properly iterates
_createdAssets, deletes the run root folder, and usesCleanupEmptyParentFoldersto avoid debris. This prevents test pollution.
88-159: Comprehensive big-bang stress test.Testing 50 elements with 151 patches (1 resize + 50×3 field sets) and verifying both first and last elements provides good coverage for bulk operations.
165-209: Good documentation of expected behavior.The comments on lines 199-201 and 207-208 clearly document current vs. expected behavior, which is valuable for understanding test intent during hardening.
671-732: Dry-run test validates non-mutation correctly.Key assertion at line 729 (
Assert.AreEqual(originalValue, asset.intValue, ...)) verifies that dry-run truly doesn't modify the asset. Testing both valid and invalid patches in one call is efficient.
742-800: Comprehensive AnimationCurve test with slope verification.Good coverage of the keyframe structure including
inSlope/outSlopevalues. Verifying key count and individual time/value assertions confirms correct parsing.
858-906: Quaternion Euler angle test with precision verification.Converting expected Euler to Quaternion and comparing components with tolerance (0.001f) is the correct approach for floating-point quaternion comparisons.
| try | ||
| { | ||
| var dirs = Directory.GetDirectories(parent); | ||
| var files = Directory.GetFiles(parent); | ||
| if (dirs.Length == 0 && files.Length == 0) | ||
| { | ||
| AssetDatabase.DeleteAsset(parent); | ||
| parent = Path.GetDirectoryName(parent)?.Replace('\\', '/'); | ||
| } | ||
| else | ||
| { | ||
| break; | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Empty catch block silently swallows exceptions.
Consider logging or at minimum breaking out with a comment explaining why the exception is intentionally ignored. Silent exception swallowing can mask real issues during test cleanup.
🔎 Proposed fix
catch
{
+ // Cleanup is best-effort; ignore errors (e.g., file locks, permissions)
break;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try | |
| { | |
| var dirs = Directory.GetDirectories(parent); | |
| var files = Directory.GetFiles(parent); | |
| if (dirs.Length == 0 && files.Length == 0) | |
| { | |
| AssetDatabase.DeleteAsset(parent); | |
| parent = Path.GetDirectoryName(parent)?.Replace('\\', '/'); | |
| } | |
| else | |
| { | |
| break; | |
| } | |
| } | |
| catch | |
| { | |
| break; | |
| } | |
| try | |
| { | |
| var dirs = Directory.GetDirectories(parent); | |
| var files = Directory.GetFiles(parent); | |
| if (dirs.Length == 0 && files.Length == 0) | |
| { | |
| AssetDatabase.DeleteAsset(parent); | |
| parent = Path.GetDirectoryName(parent)?.Replace('\\', '/'); | |
| } | |
| else | |
| { | |
| break; | |
| } | |
| } | |
| catch | |
| { | |
| // Cleanup is best-effort; ignore errors (e.g., file locks, permissions) | |
| break; | |
| } |
🤖 Prompt for AI Agents
In @TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs around
lines 113 - 130, The catch block around
Directory.GetDirectories/Directory.GetFiles/AssetDatabase.DeleteAsset silently
swallows exceptions; update the catch in the cleanup loop to at least log the
caught exception (e.g., using UnityEngine.Debug.LogException or Debug.LogWarning
with context like the current parent path) and then break/return as appropriate
so failures aren't hidden—locate the try/catch that references parent,
Directory.GetDirectories, Directory.GetFiles and AssetDatabase.DeleteAsset and
replace the empty catch with logging of the exception and a clear break.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
ValidatePatches, you’re building anonymous result objects and then using reflection to read theokproperty for the overallvalidflag; consider introducing a small DTO class/struct for validation results so you can avoid reflection and make the shape of the response explicit and easier to evolve. NormalizePropertyPathandEnsureArrayCapacityboth rely on regexes that will be hit on every patch; given the new stress workloads, it may be worth hoisting these tostatic readonly Regexinstances (or sharing a single compiled pattern) to reduce allocations and improve performance under heavy use.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ValidatePatches`, you’re building anonymous result objects and then using reflection to read the `ok` property for the overall `valid` flag; consider introducing a small DTO class/struct for validation results so you can avoid reflection and make the shape of the response explicit and easier to evolve.
- `NormalizePropertyPath` and `EnsureArrayCapacity` both rely on regexes that will be hit on every patch; given the new stress workloads, it may be worth hoisting these to `static readonly Regex` instances (or sharing a single compiled pattern) to reduce allocations and improve performance under heavy use.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Tools/ManageScriptableObject.cs:244` </location>
<code_context>
+ targetPath,
+ targetTypeName = target.GetType().FullName,
+ dryRun = true,
+ valid = validationResults.All(r => (bool)r.GetType().GetProperty("ok")?.GetValue(r)),
+ validationResults
+ }
</code_context>
<issue_to_address>
**suggestion:** Avoid using reflection to compute `valid` for dry-run results to reduce brittleness and overhead.
Since `validationResults` is built entirely within `ValidatePatches`, you already control and know the shape of each result, including the `ok` flag. Using reflection here means a future rename or omission of `ok` will only fail at runtime and may be hard to detect, and it adds overhead on a potentially hot path if many patches are processed. Prefer a strongly typed result (e.g., a small struct/class with an `Ok` property) or track the `ok` flag alongside the list so `valid` can be computed directly without reflection.
Suggested implementation:
```csharp
var (validationResults, isValid) = ValidatePatches(target, patches);
return new SuccessResponse(
"Dry-run validation complete.",
new
{
targetGuid,
targetPath,
targetTypeName = target.GetType().FullName,
dryRun = true,
valid = isValid,
validationResults
}
```
To fully remove the reflection and make this compile, you also need to:
1. Update the `ValidatePatches` signature to return both the collection of validation results and an aggregate validity flag, for example:
- From:
`private static IList<ValidationResult> ValidatePatches(object target, IEnumerable<Patch> patches)`
- To (example):
`private static (IReadOnlyList<ValidationResult> Results, bool IsValid) ValidatePatches(object target, IEnumerable<Patch> patches)`
2. Inside `ValidatePatches`, compute `IsValid` directly from the strongly-typed results, e.g.:
```csharp
var results = collectedResults.ToArray();
var isValid = results.All(r => r.Ok);
return (results, isValid);
```
where `ValidationResult` is a concrete type you control with a boolean `Ok` (or similarly named) property.
3. Adjust any other call sites of `ValidatePatches` to use the new return type, either by:
```csharp
var (validationResults, isValid) = ValidatePatches(target, patches);
```
or, if they only care about the list:
```csharp
var (validationResults, _) = ValidatePatches(target, patches);
```
</issue_to_address>
### Comment 2
<location> `TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectStressTests.cs:88-97` </location>
<code_context>
-
#region Bulk GameObject Creation
[Test]
</code_context>
<issue_to_address>
**issue (testing):** This out-of-bounds test documents failure, but the implementation now auto-resizes arrays, so the assertions should be updated to check for success and the new array size.
The `EnsureArrayCapacity` helper now grows arrays when setting an element beyond current bounds, so this test’s expectation of failure will no longer be correct.
Please update the test to reflect the new behavior by renaming it to something like `OutOfBounds_SetElementBeyondArraySize_AutoGrowsArray` and explicitly asserting that:
- `ok == true` in the patch result
- `floatArray` has grown to at least length 100
- index `99` is set to `42.0f`
This will validate the hardening behavior instead of just logging the old failure case.
</issue_to_address>
### Comment 3
<location> `TestProjects/UnityMCPTests/Assets/Tests/EditMode/TestUtilities.cs:21-23` </location>
<code_context>
+ /// Safely converts a command result to JObject, handling both JSON objects and other types.
+ /// Returns an empty JObject if result is null.
+ /// </summary>
+ public static JObject ToJObject(object result)
+ {
+ if (result == null) return new JObject();
+ return result as JObject ?? JObject.FromObject(result);
+ }
</code_context>
<issue_to_address>
**suggestion (testing):** Returning an empty JObject on null results may hide bugs that earlier per-test helpers caught; consider providing an assertion-based variant or using this helper carefully in tests that expect non-null results.
Previously, local `ToJObject` helpers in tests (e.g., `ReadConsoleTests`) would `Assert.Fail` on null results. The shared `TestUtilities.ToJObject` now returns an empty `JObject` instead, which can mask cases where a non-null response is required.
For tests where a null result should fail, consider either:
- Adding a `ToNonNullJObject` helper that asserts on null and using it there, or
- Explicitly asserting `result is not null` before calling `ToJObject`.
This preserves the convenience of the shared helper while keeping tests from treating null results as valid empty payloads.
Suggested implementation:
```csharp
/// <summary>
/// Safely converts a command result to JObject, handling both JSON objects and other types.
/// Returns an empty JObject if result is null.
/// </summary>
public static JObject ToJObject(object result)
{
if (result == null) return new JObject();
return result as JObject ?? JObject.FromObject(result);
}
/// <summary>
/// Converts a command result to JObject, asserting that the result is non-null.
/// Use this in tests where a null result should cause the test to fail.
/// </summary>
/// <param name="result">The command result to convert. Must not be null.</param>
/// <param name="message">Optional assertion failure message.</param>
public static JObject ToNonNullJObject(object result, string message = null)
{
Assert.IsNotNull(result, message ?? "Expected non-null result when converting to JObject.");
return ToJObject(result);
}
/// <summary>
/// Creates all parent directories for the given asset path if they don't exist.
/// Handles normalization and validates against dangerous patterns.
/// </summary>
```
1. Ensure that `TestUtilities.cs` has `using NUnit.Framework;` (or the appropriate assertion library) at the top of the file so that `Assert.IsNotNull` is available.
2. Update tests that previously relied on null causing failures (e.g., those that had local `ToJObject` helpers with `Assert.Fail`) to call `TestUtilities.ToNonNullJObject(...)` instead of `ToJObject(...)` where a non-null result is required.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| [Test] | ||
| public void BigBang_CreateWithLargeNestedArray() | ||
| { | ||
| // Create a ComplexStressSO with a large nestedDataList in one create call | ||
| const int elementCount = 50; // Start moderate, can increase after hardening | ||
|
|
||
| var patches = new JArray(); | ||
|
|
||
| // First resize the array | ||
| patches.Add(new JObject |
There was a problem hiding this comment.
issue (testing): This out-of-bounds test documents failure, but the implementation now auto-resizes arrays, so the assertions should be updated to check for success and the new array size.
The EnsureArrayCapacity helper now grows arrays when setting an element beyond current bounds, so this test’s expectation of failure will no longer be correct.
Please update the test to reflect the new behavior by renaming it to something like OutOfBounds_SetElementBeyondArraySize_AutoGrowsArray and explicitly asserting that:
ok == truein the patch resultfloatArrayhas grown to at least length 100- index
99is set to42.0f
This will validate the hardening behavior instead of just logging the old failure case.
| public static JObject ToJObject(object result) | ||
| { | ||
| if (result == null) return new JObject(); |
There was a problem hiding this comment.
suggestion (testing): Returning an empty JObject on null results may hide bugs that earlier per-test helpers caught; consider providing an assertion-based variant or using this helper carefully in tests that expect non-null results.
Previously, local ToJObject helpers in tests (e.g., ReadConsoleTests) would Assert.Fail on null results. The shared TestUtilities.ToJObject now returns an empty JObject instead, which can mask cases where a non-null response is required.
For tests where a null result should fail, consider either:
- Adding a
ToNonNullJObjecthelper that asserts on null and using it there, or - Explicitly asserting
result is not nullbefore callingToJObject.
This preserves the convenience of the shared helper while keeping tests from treating null results as valid empty payloads.
Suggested implementation:
/// <summary>
/// Safely converts a command result to JObject, handling both JSON objects and other types.
/// Returns an empty JObject if result is null.
/// </summary>
public static JObject ToJObject(object result)
{
if (result == null) return new JObject();
return result as JObject ?? JObject.FromObject(result);
}
/// <summary>
/// Converts a command result to JObject, asserting that the result is non-null.
/// Use this in tests where a null result should cause the test to fail.
/// </summary>
/// <param name="result">The command result to convert. Must not be null.</param>
/// <param name="message">Optional assertion failure message.</param>
public static JObject ToNonNullJObject(object result, string message = null)
{
Assert.IsNotNull(result, message ?? "Expected non-null result when converting to JObject.");
return ToJObject(result);
}
/// <summary>
/// Creates all parent directories for the given asset path if they don't exist.
/// Handles normalization and validates against dangerous patterns.
/// </summary>- Ensure that
TestUtilities.cshasusing NUnit.Framework;(or the appropriate assertion library) at the top of the file so thatAssert.IsNotNullis available. - Update tests that previously relied on null causing failures (e.g., those that had local
ToJObjecthelpers withAssert.Fail) to callTestUtilities.ToNonNullJObject(...)instead ofToJObject(...)where a non-null result is required.
2 participants
Summary
This PR significantly hardens the
ManageScriptableObjecttool with better path handling, auto-resize arrays, extended type support (AnimationCurve, Quaternion), and dry-run validation. Also consolidates test utility code to eliminate duplication across 11 test files.Key Changes
🔧 ManageScriptableObject Tool Hardening
Path Normalization
myList[5]→myList.Array.data[5](auto-normalized)Auto-Resize Arrays
Extended Type Support
AnimationCurvesupport via JSON keyframe arrays:{"keys": [{"time": 0, "value": 0}, {"time": 1, "value": 1}]}Quaternionsupport via Euler angles[x, y, z]or raw components[x, y, z, w]Dry-Run Validation
dryRun: trueparameter validates patches without applying changes🧪 Test Infrastructure
New
TestUtilities.cs- Consolidates duplicate helpers from 11 test files:ToJObject()- safe JSON conversionEnsureFolder()- recursive folder creationWaitForUnityReady()- compilation wait helperFindFallbackShader()- cross-pipeline shader fallbackCleanupEmptyParentFolders()- TearDown cleanupNew Stress Test Suite (
ManageScriptableObjectStressTests.cs- 1,100 lines):Files Changed
ManageScriptableObject.csmanage_scriptable_object.pyTestUtilities.csManageScriptableObjectStressTests.csComplexStressSO.cs,ArrayStressSO.cs,DeepStressSO.csTotal: 25 files, +2,250 / -350 lines
Testing
Breaking Changes
None - all changes are backward compatible.
Summary by Sourcery
Harden the ManageScriptableObject tool with safer path handling, auto-growing arrays, extended type support, and dry-run validation, while consolidating shared Unity test utilities and adding comprehensive stress tests.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes
New Features
array[5]) for array element accessImprovements
✏️ Tip: You can customize this high-level summary in your review settings.