From 28f60b42b0ea03cfcf74e0d8e069ec5d61226cf0 Mon Sep 17 00:00:00 2001 From: dsarno Date: Sun, 28 Dec 2025 20:15:50 -0800 Subject: [PATCH] feature/Add new manage_scriptable_object tool (#489) * Fix test teardown to avoid dropping MCP bridge CodexConfigHelperTests was calling MCPServiceLocator.Reset() in TearDown, which disposes the active bridge/transport during MCP-driven test runs. Replace with restoring only the mutated service (IPlatformService). * Avoid leaking PlatformService in CodexConfigHelperTests Capture the original IPlatformService before this fixture runs and restore it in TearDown. This preserves the MCP connection safety fix (no MCPServiceLocator.Reset()) while avoiding global state leakage to subsequent tests. * Fix SO MCP tooling: validate folder roots, normalize paths, expand tests; remove vestigial SO tools * Remove UnityMCPTests stress artifacts and ignore Assets/Temp * Ignore UnityMCPTests Assets/Temp only * Clarify array_resize fallback logic comments * Refactor: simplify action set and reuse slash sanitization * Enhance: preserve GUID on overwrite & support Vector/Color types in ScriptableObject tools * Fix: ensure asset name matches filename to suppress Unity warnings * Fix: resolve Unity warnings by ensuring asset name match and removing redundant import * Refactor: Validate assetName, strict object parsing for vectors, remove broken SO logic from ManageAsset * Hardening: reject Windows drive paths; clarify supported asset types * Delete FixscriptableobjecPlan.md * docs: Add manage_scriptable_object tool description to README --- .gitignore | 3 + MCPForUnity/Editor/Tools/ManageAsset.cs | 34 +- .../Editor/Tools/ManageScriptableObject.cs | 931 ++++++++++++++++++ .../Tools/ManageScriptableObject.cs.meta | 14 + README.md | 1 + Server/src/services/tools/manage_asset.py | 2 +- .../tools/manage_scriptable_object.py | 75 ++ .../test_manage_scriptable_object_tool.py | 72 ++ .../UnityMCPTests/Assets/Packages.meta | 8 + TestProjects/UnityMCPTests/Assets/Temp.meta | 8 + .../Assets/Tests/EditMode/Tools/Fixtures.meta | 8 + .../ManageScriptableObjectTestDefinition.cs | 27 + ...nageScriptableObjectTestDefinition.cs.meta | 11 + ...anageScriptableObjectTestDefinitionBase.cs | 14 + ...ScriptableObjectTestDefinitionBase.cs.meta | 11 + .../Tools/ManageScriptableObjectTests.cs | 310 ++++++ .../Tools/ManageScriptableObjectTests.cs.meta | 11 + .../SceneTemplateSettings.json | 121 +++ 18 files changed, 1631 insertions(+), 30 deletions(-) create mode 100644 MCPForUnity/Editor/Tools/ManageScriptableObject.cs create mode 100644 MCPForUnity/Editor/Tools/ManageScriptableObject.cs.meta create mode 100644 Server/src/services/tools/manage_scriptable_object.py create mode 100644 Server/tests/integration/test_manage_scriptable_object_tool.py create mode 100644 TestProjects/UnityMCPTests/Assets/Packages.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Temp.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs.meta create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs create mode 100644 TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs.meta create mode 100644 TestProjects/UnityMCPTests/ProjectSettings/SceneTemplateSettings.json diff --git a/.gitignore b/.gitignore index 81f0d66..1043288 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,9 @@ CONTRIBUTING.md.meta # Unity test project lock files TestProjects/UnityMCPTests/Packages/packages-lock.json +# UnityMCPTests stress-run artifacts (these are created by tests/tools and should never be committed) +TestProjects/UnityMCPTests/Assets/Temp/ + # Backup artifacts *.backup *.backup.meta diff --git a/MCPForUnity/Editor/Tools/ManageAsset.cs b/MCPForUnity/Editor/Tools/ManageAsset.cs index 7217da7..eecbcac 100644 --- a/MCPForUnity/Editor/Tools/ManageAsset.cs +++ b/MCPForUnity/Editor/Tools/ManageAsset.cs @@ -229,31 +229,6 @@ namespace MCPForUnity.Editor.Tools AssetDatabase.CreateAsset(pmat, fullPath); newAsset = pmat; } - else if (lowerAssetType == "scriptableobject") - { - string scriptClassName = properties?["scriptClass"]?.ToString(); - if (string.IsNullOrEmpty(scriptClassName)) - return new ErrorResponse( - "'scriptClass' property required when creating ScriptableObject asset." - ); - - Type scriptType = ComponentResolver.TryResolve(scriptClassName, out var resolvedType, out var error) ? resolvedType : null; - if ( - scriptType == null - || !typeof(ScriptableObject).IsAssignableFrom(scriptType) - ) - { - var reason = scriptType == null - ? (string.IsNullOrEmpty(error) ? "Type not found." : error) - : "Type found but does not inherit from ScriptableObject."; - return new ErrorResponse($"Script class '{scriptClassName}' invalid: {reason}"); - } - - ScriptableObject so = ScriptableObject.CreateInstance(scriptType); - // TODO: Apply properties from JObject to the ScriptableObject instance? - AssetDatabase.CreateAsset(so, fullPath); - newAsset = so; - } else if (lowerAssetType == "prefab") { // Creating prefabs usually involves saving an existing GameObject hierarchy. @@ -274,7 +249,7 @@ namespace MCPForUnity.Editor.Tools // AssetDatabase.ImportAsset(fullPath); // Let Unity try to import it // newAsset = AssetDatabase.LoadAssetAtPath(fullPath); return new ErrorResponse( - $"Creation for asset type '{assetType}' is not explicitly supported yet. Supported: Folder, Material, ScriptableObject." + $"Creation for asset type '{assetType}' is not explicitly supported yet. Supported: Folder, Material, PhysicsMaterial." ); } @@ -445,11 +420,12 @@ namespace MCPForUnity.Editor.Tools // Use |= in case the asset was already marked modified by previous logic (though unlikely here) modified |= MaterialOps.ApplyProperties(material, properties, ManageGameObject.InputSerializer); } - // Example: Modifying a ScriptableObject + // Example: Modifying a ScriptableObject (Use manage_scriptable_object instead!) else if (asset is ScriptableObject so) { - // Apply properties directly to the ScriptableObject. - modified |= ApplyObjectProperties(so, properties); // General helper + // Deprecated: Prefer manage_scriptable_object for robust patching. + // Kept for simple property setting fallback on existing assets if manage_scriptable_object isn't used. + modified |= ApplyObjectProperties(so, properties); } // Example: Modifying TextureImporter settings else if (asset is Texture) diff --git a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs new file mode 100644 index 0000000..0ac467f --- /dev/null +++ b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs @@ -0,0 +1,931 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.IO; +using System.Linq; +using System.Reflection; +using System.Text.RegularExpressions; +using MCPForUnity.Editor.Helpers; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; + +namespace MCPForUnity.Editor.Tools +{ + /// + /// Single tool for ScriptableObject workflows: + /// - action=create: create a ScriptableObject asset (and optionally apply patches) + /// - action=modify: apply serialized property patches to an existing asset + /// + /// Patching is performed via SerializedObject/SerializedProperty paths (Unity-native), not reflection. + /// + [McpForUnityTool("manage_scriptable_object", AutoRegister = false)] + public static class ManageScriptableObject + { + private const string CodeCompilingOrReloading = "compiling_or_reloading"; + private const string CodeInvalidParams = "invalid_params"; + private const string CodeTypeNotFound = "type_not_found"; + private const string CodeInvalidFolderPath = "invalid_folder_path"; + private const string CodeTargetNotFound = "target_not_found"; + private const string CodeAssetCreateFailed = "asset_create_failed"; + + private static readonly HashSet ValidActions = new(StringComparer.OrdinalIgnoreCase) + { + // NOTE: Action strings are normalized by NormalizeAction() (lowercased, '_'/'-' removed), + // so we only need the canonical normalized forms here. + "create", + "createso", + "modify", + "modifyso", + }; + + public static object HandleCommand(JObject @params) + { + if (@params == null) + { + return new ErrorResponse(CodeInvalidParams); + } + + if (EditorApplication.isCompiling || EditorApplication.isUpdating) + { + // Unity is transient; treat as retryable on the client side. + return new ErrorResponse(CodeCompilingOrReloading, new { hint = "retry" }); + } + + // Allow JSON-string parameters for objects/arrays. + JsonUtil.CoerceJsonStringParameter(@params, "target"); + CoerceJsonStringArrayParameter(@params, "patches"); + + string actionRaw = @params["action"]?.ToString(); + if (string.IsNullOrWhiteSpace(actionRaw)) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'action' is required.", validActions = ValidActions.ToArray() }); + } + + string action = NormalizeAction(actionRaw); + if (!ValidActions.Contains(action)) + { + return new ErrorResponse(CodeInvalidParams, new { message = $"Unknown action: '{actionRaw}'.", validActions = ValidActions.ToArray() }); + } + + if (IsCreateAction(action)) + { + return HandleCreate(@params); + } + + return HandleModify(@params); + } + + private static object HandleCreate(JObject @params) + { + string typeName = @params["typeName"]?.ToString() ?? @params["type_name"]?.ToString(); + string folderPath = @params["folderPath"]?.ToString() ?? @params["folder_path"]?.ToString(); + string assetName = @params["assetName"]?.ToString() ?? @params["asset_name"]?.ToString(); + bool overwrite = @params["overwrite"]?.ToObject() ?? false; + + if (string.IsNullOrWhiteSpace(typeName)) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'typeName' is required." }); + } + + if (string.IsNullOrWhiteSpace(folderPath)) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'folderPath' is required." }); + } + + if (string.IsNullOrWhiteSpace(assetName)) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'assetName' is required." }); + } + + if (assetName.Contains("/") || assetName.Contains("\\")) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'assetName' must not contain path separators." }); + } + + if (!TryNormalizeFolderPath(folderPath, out var normalizedFolder, out var folderNormalizeError)) + { + return new ErrorResponse(CodeInvalidFolderPath, new { message = folderNormalizeError, folderPath }); + } + + if (!EnsureFolderExists(normalizedFolder, out var folderError)) + { + return new ErrorResponse(CodeInvalidFolderPath, new { message = folderError, folderPath = normalizedFolder }); + } + + var resolvedType = ResolveType(typeName); + if (resolvedType == null || !typeof(ScriptableObject).IsAssignableFrom(resolvedType)) + { + return new ErrorResponse(CodeTypeNotFound, new { message = $"ScriptableObject type not found: '{typeName}'", typeName }); + } + + string fileName = assetName.EndsWith(".asset", StringComparison.OrdinalIgnoreCase) + ? assetName + : assetName + ".asset"; + string desiredPath = $"{normalizedFolder.TrimEnd('/')}/{fileName}"; + string finalPath = overwrite ? desiredPath : AssetDatabase.GenerateUniqueAssetPath(desiredPath); + + ScriptableObject instance; + try + { + instance = ScriptableObject.CreateInstance(resolvedType); + if (instance == null) + { + return new ErrorResponse(CodeAssetCreateFailed, new { message = "CreateInstance returned null.", typeName = resolvedType.FullName }); + } + } + catch (Exception ex) + { + return new ErrorResponse(CodeAssetCreateFailed, new { message = ex.Message, typeName = resolvedType.FullName }); + } + + // GUID-preserving overwrite logic + bool isNewAsset = true; + try + { + if (overwrite) + { + var existingAsset = AssetDatabase.LoadAssetAtPath(finalPath); + if (existingAsset != null && existingAsset.GetType() == resolvedType) + { + // Preserve GUID by overwriting existing asset data in-place + EditorUtility.CopySerialized(instance, existingAsset); + + // Fix for "Main Object Name does not match filename" warning: + // CopySerialized overwrites the name with the (empty) name of the new instance. + // We must restore the correct name to match the filename. + existingAsset.name = Path.GetFileNameWithoutExtension(finalPath); + + UnityEngine.Object.DestroyImmediate(instance); // Destroy temporary instance + instance = existingAsset; // Proceed with patching the existing asset + isNewAsset = false; + + // Mark dirty to ensure changes are picked up + EditorUtility.SetDirty(instance); + } + else if (existingAsset != null) + { + // Type mismatch or not a ScriptableObject - must delete and recreate to change type, losing GUID + // (Or we could warn, but overwrite usually implies replacing) + AssetDatabase.DeleteAsset(finalPath); + } + } + + if (isNewAsset) + { + // Ensure the new instance has the correct name before creating asset to avoid warnings + instance.name = Path.GetFileNameWithoutExtension(finalPath); + AssetDatabase.CreateAsset(instance, finalPath); + } + } + catch (Exception ex) + { + return new ErrorResponse(CodeAssetCreateFailed, new { message = ex.Message, path = finalPath }); + } + + string guid = AssetDatabase.AssetPathToGUID(finalPath); + var patchesToken = @params["patches"]; + object patchResults = null; + var warnings = new List(); + + if (patchesToken is JArray patches && patches.Count > 0) + { + var patchApply = ApplyPatches(instance, patches); + patchResults = patchApply.results; + warnings.AddRange(patchApply.warnings); + } + + EditorUtility.SetDirty(instance); + AssetDatabase.SaveAssets(); + + return new SuccessResponse( + "ScriptableObject created.", + new + { + guid, + path = finalPath, + typeNameResolved = resolvedType.FullName, + patchResults, + warnings = warnings.Count > 0 ? warnings : null + } + ); + } + + private static object HandleModify(JObject @params) + { + if (!TryResolveTarget(@params["target"], out var target, out var targetPath, out var targetGuid, out var err)) + { + return err; + } + + var patchesToken = @params["patches"]; + if (patchesToken == null || patchesToken.Type == JTokenType.Null) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'patches' is required.", targetPath, targetGuid }); + } + + if (patchesToken is not JArray patches) + { + return new ErrorResponse(CodeInvalidParams, new { message = "'patches' must be an array.", targetPath, targetGuid }); + } + + var (results, warnings) = ApplyPatches(target, patches); + + return new SuccessResponse( + "Serialized properties patched.", + new + { + targetGuid, + targetPath, + targetTypeName = target.GetType().FullName, + results, + warnings = warnings.Count > 0 ? warnings : null + } + ); + } + + private static (List results, List warnings) ApplyPatches(UnityEngine.Object target, JArray patches) + { + var warnings = new List(); + var results = new List(patches.Count); + bool anyChanged = false; + + var so = new SerializedObject(target); + so.Update(); + + for (int i = 0; i < patches.Count; i++) + { + if (patches[i] is not JObject patchObj) + { + results.Add(new { propertyPath = "", op = "", ok = false, message = $"Patch at index {i} must be an object." }); + continue; + } + + string propertyPath = patchObj["propertyPath"]?.ToString() + ?? patchObj["property_path"]?.ToString() + ?? patchObj["path"]?.ToString(); + string op = (patchObj["op"]?.ToString() ?? "set").Trim(); + if (string.IsNullOrWhiteSpace(propertyPath)) + { + results.Add(new { propertyPath = propertyPath ?? "", op, ok = false, message = "Missing required field: propertyPath" }); + continue; + } + + if (string.IsNullOrWhiteSpace(op)) + { + op = "set"; + } + + var patchResult = ApplyPatch(so, propertyPath, op, patchObj, out bool changed); + anyChanged |= changed; + results.Add(patchResult); + + // Array resize should be applied immediately so later paths resolve. + if (string.Equals(op, "array_resize", StringComparison.OrdinalIgnoreCase) && changed) + { + so.ApplyModifiedProperties(); + so.Update(); + } + } + + if (anyChanged) + { + so.ApplyModifiedProperties(); + EditorUtility.SetDirty(target); + AssetDatabase.SaveAssets(); + } + + return (results, warnings); + } + + private static object ApplyPatch(SerializedObject so, string propertyPath, string op, JObject patchObj, out bool changed) + { + changed = false; + try + { + string normalizedOp = op.Trim().ToLowerInvariant(); + + switch (normalizedOp) + { + case "array_resize": + return ApplyArrayResize(so, propertyPath, patchObj, out changed); + case "set": + default: + return ApplySet(so, propertyPath, patchObj, out changed); + } + } + catch (Exception ex) + { + return new { propertyPath, op, ok = false, message = ex.Message }; + } + } + + private static object ApplyArrayResize(SerializedObject so, string propertyPath, JObject patchObj, out bool changed) + { + changed = false; + if (!TryGetInt(patchObj["value"], out int newSize)) + { + return new { propertyPath, op = "array_resize", ok = false, message = "array_resize requires integer 'value'." }; + } + + newSize = Math.Max(0, newSize); + + // Unity supports resizing either: + // - the array/list property itself (prop.isArray -> prop.arraySize) + // - the synthetic leaf property ".Array.size" (prop.intValue) + // + // Different Unity versions/serialization edge cases can fail to resolve the synthetic leaf via FindProperty + // (or can return different property types), so we keep a "best-effort" fallback: + // - Prefer acting on the requested path if it resolves. + // - If the requested path doesn't resolve, try to resolve the *array property* and set arraySize directly. + SerializedProperty prop = so.FindProperty(propertyPath); + SerializedProperty arrayProp = null; + if (propertyPath.EndsWith(".Array.size", StringComparison.Ordinal)) + { + // Caller explicitly targeted the synthetic leaf. Resolve the parent array property as a fallback + // (Unity sometimes fails to resolve the synthetic leaf in certain serialization contexts). + var arrayPath = propertyPath.Substring(0, propertyPath.Length - ".Array.size".Length); + arrayProp = so.FindProperty(arrayPath); + } + else + { + // Caller targeted either the array property itself (e.g., "items") or some other property. + // If it's already an array, we can resize it directly. Otherwise, we attempt to resolve + // a synthetic ".Array.size" leaf as a convenience, which some clients may pass. + arrayProp = prop != null && prop.isArray ? prop : so.FindProperty(propertyPath + ".Array.size"); + } + + if (prop == null) + { + // If we failed to find the direct property but we *can* find the array property, use that. + if (arrayProp != null && arrayProp.isArray) + { + if (arrayProp.arraySize != newSize) + { + arrayProp.arraySize = newSize; + changed = true; + } + return new + { + propertyPath, + op = "array_resize", + ok = true, + resolvedPropertyType = "Array", + message = $"Set array size to {newSize}." + }; + } + + return new { propertyPath, op = "array_resize", ok = false, message = $"Property not found: {propertyPath}" }; + } + + // Unity may represent ".Array.size" as either Integer or ArraySize depending on version. + if ((prop.propertyType == SerializedPropertyType.Integer || prop.propertyType == SerializedPropertyType.ArraySize) + && propertyPath.EndsWith(".Array.size", StringComparison.Ordinal)) + { + // We successfully resolved the synthetic leaf; write the size through its intValue. + if (prop.intValue != newSize) + { + prop.intValue = newSize; + changed = true; + } + return new { propertyPath, op = "array_resize", ok = true, resolvedPropertyType = prop.propertyType.ToString(), message = $"Set array size to {newSize}." }; + } + + if (prop.isArray) + { + // We resolved the array property itself; write through arraySize. + if (prop.arraySize != newSize) + { + prop.arraySize = newSize; + changed = true; + } + return new { propertyPath, op = "array_resize", ok = true, resolvedPropertyType = "Array", message = $"Set array size to {newSize}." }; + } + + return new { propertyPath, op = "array_resize", ok = false, resolvedPropertyType = prop.propertyType.ToString(), message = $"Property is not an array or array-size field: {propertyPath}" }; + } + + private static object ApplySet(SerializedObject so, string propertyPath, JObject patchObj, out bool changed) + { + changed = false; + var prop = so.FindProperty(propertyPath); + if (prop == null) + { + return new { propertyPath, op = "set", ok = false, message = $"Property not found: {propertyPath}" }; + } + + if (prop.propertyType == SerializedPropertyType.ObjectReference) + { + var refObj = patchObj["ref"] as JObject; + UnityEngine.Object newRef = null; + string refGuid = refObj?["guid"]?.ToString(); + string refPath = refObj?["path"]?.ToString(); + + if (refObj == null && patchObj["value"]?.Type == JTokenType.Null) + { + newRef = null; + } + else if (!string.IsNullOrEmpty(refGuid) || !string.IsNullOrEmpty(refPath)) + { + string resolvedPath = !string.IsNullOrEmpty(refGuid) + ? AssetDatabase.GUIDToAssetPath(refGuid) + : AssetPathUtility.SanitizeAssetPath(refPath); + + if (!string.IsNullOrEmpty(resolvedPath)) + { + newRef = AssetDatabase.LoadAssetAtPath(resolvedPath); + } + } + + if (prop.objectReferenceValue != newRef) + { + prop.objectReferenceValue = newRef; + changed = true; + } + + return new { propertyPath, op = "set", ok = true, resolvedPropertyType = prop.propertyType.ToString(), message = newRef == null ? "Cleared reference." : "Set reference." }; + } + + var valueToken = patchObj["value"]; + if (valueToken == null) + { + return new { propertyPath, op = "set", ok = false, resolvedPropertyType = prop.propertyType.ToString(), message = "Missing required field: value" }; + } + + bool ok = TrySetValue(prop, valueToken, out string message); + changed = ok; + return new { propertyPath, op = "set", ok, resolvedPropertyType = prop.propertyType.ToString(), message }; + } + + private static bool TrySetValue(SerializedProperty prop, JToken valueToken, out string message) + { + message = null; + try + { + // Supported Types: Integer, Boolean, Float, String, Enum, Vector2, Vector3, Vector4, Color + switch (prop.propertyType) + { + case SerializedPropertyType.Integer: + if (!TryGetInt(valueToken, out var intVal)) { message = "Expected integer value."; return false; } + prop.intValue = intVal; message = "Set int."; return true; + + case SerializedPropertyType.Boolean: + if (!TryGetBool(valueToken, out var boolVal)) { message = "Expected boolean value."; return false; } + prop.boolValue = boolVal; message = "Set bool."; return true; + + case SerializedPropertyType.Float: + if (!TryGetFloat(valueToken, out var floatVal)) { message = "Expected float value."; return false; } + prop.floatValue = floatVal; message = "Set float."; return true; + + case SerializedPropertyType.String: + prop.stringValue = valueToken.Type == JTokenType.Null ? null : valueToken.ToString(); + message = "Set string."; return true; + + case SerializedPropertyType.Enum: + return TrySetEnum(prop, valueToken, out message); + + case SerializedPropertyType.Vector2: + if (!TryGetVector2(valueToken, out var v2)) { message = "Expected Vector2 (array or object)."; return false; } + prop.vector2Value = v2; message = "Set Vector2."; return true; + + case SerializedPropertyType.Vector3: + if (!TryGetVector3(valueToken, out var v3)) { message = "Expected Vector3 (array or object)."; return false; } + prop.vector3Value = v3; message = "Set Vector3."; return true; + + case SerializedPropertyType.Vector4: + if (!TryGetVector4(valueToken, out var v4)) { message = "Expected Vector4 (array or object)."; return false; } + prop.vector4Value = v4; message = "Set Vector4."; return true; + + case SerializedPropertyType.Color: + if (!TryGetColor(valueToken, out var col)) { message = "Expected Color (array or object)."; return false; } + prop.colorValue = col; message = "Set Color."; return true; + + default: + message = $"Unsupported SerializedPropertyType: {prop.propertyType}"; + return false; + } + } + catch (Exception ex) + { + message = ex.Message; + return false; + } + } + + private static bool TrySetEnum(SerializedProperty prop, JToken valueToken, out string message) + { + message = null; + var names = prop.enumNames; + if (names == null || names.Length == 0) { message = "Enum has no names."; return false; } + + if (valueToken.Type == JTokenType.Integer) + { + int idx = valueToken.Value(); + if (idx < 0 || idx >= names.Length) { message = $"Enum index out of range: {idx}"; return false; } + prop.enumValueIndex = idx; message = "Set enum."; return true; + } + + string s = valueToken.ToString(); + for (int i = 0; i < names.Length; i++) + { + if (string.Equals(names[i], s, StringComparison.OrdinalIgnoreCase)) + { + prop.enumValueIndex = i; message = "Set enum."; return true; + } + } + message = $"Unknown enum name '{s}'."; + return false; + } + + private static bool TryResolveTarget(JToken targetToken, out UnityEngine.Object target, out string targetPath, out string targetGuid, out object error) + { + target = null; + targetPath = null; + targetGuid = null; + error = null; + + if (targetToken is not JObject targetObj) + { + error = new ErrorResponse(CodeInvalidParams, new { message = "'target' must be an object with {guid|path}." }); + return false; + } + + string guid = targetObj["guid"]?.ToString(); + string path = targetObj["path"]?.ToString(); + + if (string.IsNullOrWhiteSpace(guid) && string.IsNullOrWhiteSpace(path)) + { + error = new ErrorResponse(CodeInvalidParams, new { message = "'target' must include 'guid' or 'path'." }); + return false; + } + + string resolvedPath = !string.IsNullOrWhiteSpace(guid) + ? AssetDatabase.GUIDToAssetPath(guid) + : AssetPathUtility.SanitizeAssetPath(path); + + if (string.IsNullOrWhiteSpace(resolvedPath)) + { + error = new ErrorResponse(CodeTargetNotFound, new { message = "Could not resolve target path.", guid, path }); + return false; + } + + var obj = AssetDatabase.LoadAssetAtPath(resolvedPath); + if (obj == null) + { + error = new ErrorResponse(CodeTargetNotFound, new { message = "Target asset not found.", targetPath = resolvedPath, targetGuid = guid }); + return false; + } + + target = obj; + targetPath = resolvedPath; + targetGuid = string.IsNullOrWhiteSpace(guid) ? AssetDatabase.AssetPathToGUID(resolvedPath) : guid; + return true; + } + + private static void CoerceJsonStringArrayParameter(JObject @params, string paramName) + { + var token = @params?[paramName]; + if (token != null && token.Type == JTokenType.String) + { + try + { + var parsed = JToken.Parse(token.ToString()); + if (parsed is JArray arr) + { + @params[paramName] = arr; + } + } + catch (Exception e) + { + Debug.LogWarning($"[MCP] Could not parse '{paramName}' JSON string: {e.Message}"); + } + } + } + + private static bool EnsureFolderExists(string folderPath, out string error) + { + error = null; + if (string.IsNullOrWhiteSpace(folderPath)) + { + error = "Folder path is empty."; + return false; + } + + // Expect normalized input here (Assets/... or Assets). + string sanitized = SanitizeSlashes(folderPath); + + if (!sanitized.StartsWith("Assets/", StringComparison.OrdinalIgnoreCase) + && !string.Equals(sanitized, "Assets", StringComparison.OrdinalIgnoreCase)) + { + error = "Folder path must be under Assets/."; + return false; + } + + if (string.Equals(sanitized, "Assets", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + sanitized = sanitized.TrimEnd('/'); + if (AssetDatabase.IsValidFolder(sanitized)) + { + return true; + } + + // Create recursively from Assets/ + var parts = sanitized.Split(new[] { '/' }, StringSplitOptions.RemoveEmptyEntries); + if (parts.Length == 0 || !string.Equals(parts[0], "Assets", StringComparison.OrdinalIgnoreCase)) + { + error = "Folder path must start with Assets/"; + return false; + } + + string current = "Assets"; + for (int i = 1; i < parts.Length; i++) + { + string next = current + "/" + parts[i]; + if (!AssetDatabase.IsValidFolder(next)) + { + string guid = AssetDatabase.CreateFolder(current, parts[i]); + if (string.IsNullOrEmpty(guid)) + { + error = $"Failed to create folder: {next}"; + return false; + } + } + current = next; + } + + return AssetDatabase.IsValidFolder(sanitized); + } + + private static string SanitizeSlashes(string path) + { + if (string.IsNullOrWhiteSpace(path)) + { + return path; + } + + var s = path.Replace('\\', '/'); + while (s.IndexOf("//", StringComparison.Ordinal) >= 0) + { + s = s.Replace("//", "/", StringComparison.Ordinal); + } + return s; + } + + private static bool TryNormalizeFolderPath(string folderPath, out string normalized, out string error) + { + normalized = null; + error = null; + + if (string.IsNullOrWhiteSpace(folderPath)) + { + error = "Folder path is empty."; + return false; + } + + var s = SanitizeSlashes(folderPath.Trim()); + + // Reject obvious non-project/invalid roots. We only support Assets/ (and relative paths that will be rooted under Assets/). + if (s.StartsWith("/", StringComparison.Ordinal) + || s.StartsWith("file:", StringComparison.OrdinalIgnoreCase) + || Regex.IsMatch(s, @"^[a-zA-Z]:")) + { + error = "Folder path must be a project-relative path under Assets/."; + return false; + } + + if (s.StartsWith("Packages/", StringComparison.OrdinalIgnoreCase) + || s.StartsWith("ProjectSettings/", StringComparison.OrdinalIgnoreCase) + || s.StartsWith("Library/", StringComparison.OrdinalIgnoreCase)) + { + error = "Folder path must be under Assets/."; + return false; + } + + if (string.Equals(s, "Assets", StringComparison.OrdinalIgnoreCase)) + { + normalized = "Assets"; + return true; + } + + if (s.StartsWith("Assets/", StringComparison.OrdinalIgnoreCase)) + { + normalized = s.TrimEnd('/'); + return true; + } + + // Allow relative paths like "Temp/MyFolder" and root them under Assets/. + normalized = ("Assets/" + s.TrimStart('/')).TrimEnd('/'); + return true; + } + + private static bool TryGetInt(JToken token, out int value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + try + { + if (token.Type == JTokenType.Integer) { value = token.Value(); return true; } + if (token.Type == JTokenType.Float) { value = Convert.ToInt32(token.Value()); return true; } + var s = token.ToString().Trim(); + return int.TryParse(s, NumberStyles.Integer, CultureInfo.InvariantCulture, out value); + } + catch { return false; } + } + + private static bool TryGetFloat(JToken token, out float value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + try + { + if (token.Type == JTokenType.Float || token.Type == JTokenType.Integer) { value = token.Value(); return true; } + var s = token.ToString().Trim(); + return float.TryParse(s, NumberStyles.Float, CultureInfo.InvariantCulture, out value); + } + catch { return false; } + } + + private static bool TryGetBool(JToken token, out bool value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + try + { + if (token.Type == JTokenType.Boolean) { value = token.Value(); return true; } + var s = token.ToString().Trim(); + return bool.TryParse(s, out value); + } + catch { return false; } + } + + // --- Vector/Color Parsing Helpers --- + + private static bool TryGetVector2(JToken token, out Vector2 value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + + // Handle [x, y] + if (token is JArray arr && arr.Count >= 2) + { + if (TryGetFloat(arr[0], out float x) && TryGetFloat(arr[1], out float y)) + { + value = new Vector2(x, y); + return true; + } + } + // Handle { "x": ..., "y": ... } + if (token is JObject obj) + { + if (TryGetFloat(obj["x"], out float x) && TryGetFloat(obj["y"], out float y)) + { + value = new Vector2(x, y); + return true; + } + } + return false; + } + + private static bool TryGetVector3(JToken token, out Vector3 value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + + // Handle [x, y, z] + if (token is JArray arr && arr.Count >= 3) + { + if (TryGetFloat(arr[0], out float x) && TryGetFloat(arr[1], out float y) && TryGetFloat(arr[2], out float z)) + { + value = new Vector3(x, y, z); + return true; + } + } + // Handle { "x": ..., "y": ..., "z": ... } + if (token is JObject obj) + { + if (TryGetFloat(obj["x"], out float x) && TryGetFloat(obj["y"], out float y) && TryGetFloat(obj["z"], out float z)) + { + value = new Vector3(x, y, z); + return true; + } + } + return false; + } + + private static bool TryGetVector4(JToken token, out Vector4 value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + + // Handle [x, y, z, w] + if (token is JArray arr && arr.Count >= 4) + { + if (TryGetFloat(arr[0], out float x) && TryGetFloat(arr[1], out float y) + && TryGetFloat(arr[2], out float z) && TryGetFloat(arr[3], out float w)) + { + value = new Vector4(x, y, z, w); + return true; + } + } + // Handle { "x": ..., "y": ..., "z": ..., "w": ... } + if (token is JObject obj) + { + if (TryGetFloat(obj["x"], out float x) && TryGetFloat(obj["y"], out float y) + && TryGetFloat(obj["z"], out float z) && TryGetFloat(obj["w"], out float w)) + { + value = new Vector4(x, y, z, w); + return true; + } + } + return false; + } + + private static bool TryGetColor(JToken token, out Color value) + { + value = default; + if (token == null || token.Type == JTokenType.Null) return false; + + // Handle [r, g, b, a] + if (token is JArray arr && arr.Count >= 3) + { + float r = 0, g = 0, b = 0, a = 1; + bool ok = TryGetFloat(arr[0], out r) && TryGetFloat(arr[1], out g) && TryGetFloat(arr[2], out b); + if (arr.Count > 3) TryGetFloat(arr[3], out a); + if (ok) + { + value = new Color(r, g, b, a); + return true; + } + } + // Handle { "r": ..., "g": ..., "b": ..., "a": ... } + if (token is JObject obj) + { + if (TryGetFloat(obj["r"], out float r) && TryGetFloat(obj["g"], out float g) && TryGetFloat(obj["b"], out float b)) + { + // Alpha is optional, defaults to 1.0 + float a = 1.0f; + TryGetFloat(obj["a"], out a); + value = new Color(r, g, b, a); + return true; + } + } + return false; + } + + private static string NormalizeAction(string raw) + { + var s = raw.Trim(); + s = s.Replace("-", "").Replace("_", ""); + return s.ToLowerInvariant(); + } + + private static bool IsCreateAction(string normalized) + { + return normalized == "create" || normalized == "createso"; + } + + private static Type ResolveType(string typeName) + { + if (string.IsNullOrWhiteSpace(typeName)) return null; + + var type = Type.GetType(typeName, throwOnError: false); + if (type != null) return type; + + foreach (var asm in AppDomain.CurrentDomain.GetAssemblies().Where(a => a != null && !a.IsDynamic)) + { + try + { + type = asm.GetType(typeName, throwOnError: false); + if (type != null) return type; + } + catch + { + // ignore + } + } + + // fallback: scan types by FullName match (covers cases where GetType lookup fails) + foreach (var asm in AppDomain.CurrentDomain.GetAssemblies().Where(a => a != null && !a.IsDynamic)) + { + Type[] types; + try { types = asm.GetTypes(); } + catch (ReflectionTypeLoadException e) { types = e.Types.Where(t => t != null).ToArray(); } + catch { continue; } + + foreach (var t in types) + { + if (t == null) continue; + if (string.Equals(t.FullName, typeName, StringComparison.Ordinal)) + { + return t; + } + } + } + + return null; + } + } +} diff --git a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs.meta b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs.meta new file mode 100644 index 0000000..fa17b47 --- /dev/null +++ b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs.meta @@ -0,0 +1,14 @@ +fileFormatVersion: 2 +guid: 9e0bb5a8c1b24b7ea8bce09ce0a1f234 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: + + + diff --git a/README.md b/README.md index 6fee5ba..62ab224 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ MCP for Unity acts as a bridge, allowing AI assistants (like Claude, Cursor) to * `manage_prefabs`: Performs prefab operations (create, modify, delete, etc.). * `manage_scene`: Manages scenes (load, save, create, get hierarchy, etc.). * `manage_script`: Compatibility router for legacy script operations (create, read, delete). Prefer `apply_text_edits` or `script_apply_edits` for edits. +* `manage_scriptable_object`: Creates and modifies ScriptableObject assets using Unity SerializedObject property paths. * `manage_shader`: Performs shader CRUD operations (create, read, modify, delete). * `read_console`: Gets messages from or clears the console. * `run_tests`: Runs tests in the Unity Editor. diff --git a/Server/src/services/tools/manage_asset.py b/Server/src/services/tools/manage_asset.py index e7264ea..3a4f655 100644 --- a/Server/src/services/tools/manage_asset.py +++ b/Server/src/services/tools/manage_asset.py @@ -22,7 +22,7 @@ async def manage_asset( action: Annotated[Literal["import", "create", "modify", "delete", "duplicate", "move", "rename", "search", "get_info", "create_folder", "get_components"], "Perform CRUD operations on assets."], path: Annotated[str, "Asset path (e.g., 'Materials/MyMaterial.mat') or search scope."], asset_type: Annotated[str, - "Asset type (e.g., 'Material', 'Folder') - required for 'create'."] | None = None, + "Asset type (e.g., 'Material', 'Folder') - required for 'create'. Note: For ScriptableObjects, use manage_scriptable_object."] | None = None, properties: Annotated[dict[str, Any] | str, "Dictionary (or JSON string) of properties for 'create'/'modify'."] | None = None, destination: Annotated[str, diff --git a/Server/src/services/tools/manage_scriptable_object.py b/Server/src/services/tools/manage_scriptable_object.py new file mode 100644 index 0000000..38249ae --- /dev/null +++ b/Server/src/services/tools/manage_scriptable_object.py @@ -0,0 +1,75 @@ +""" +Tool wrapper for managing ScriptableObject assets via Unity MCP. + +Unity-side handler: MCPForUnity.Editor.Tools.ManageScriptableObject +Command name: "manage_scriptable_object" +Actions: + - create: create an SO asset (optionally with patches) + - modify: apply serialized property patches to an existing SO asset +""" + +from __future__ import annotations + +from typing import Annotated, Any, Literal + +from fastmcp import Context + +from services.registry import mcp_for_unity_tool +from services.tools import get_unity_instance_from_context +from services.tools.utils import coerce_bool, parse_json_payload +from transport.unity_transport import send_with_unity_instance +from transport.legacy.unity_connection import async_send_command_with_retry + + +@mcp_for_unity_tool( + description="Creates and modifies ScriptableObject assets using Unity SerializedObject property paths." +) +async def manage_scriptable_object( + ctx: Context, + action: Annotated[Literal["create", "modify"], "Action to perform: create or modify."], + # --- create params --- + type_name: Annotated[str | None, "Namespace-qualified ScriptableObject type name (for create)."] = None, + folder_path: Annotated[str | None, "Target folder under Assets/... (for create)."] = None, + asset_name: Annotated[str | None, "Asset file name without extension (for create)."] = None, + overwrite: Annotated[bool | str | None, "If true, overwrite existing asset at same path (for create)."] = None, + # --- modify params --- + target: Annotated[dict[str, Any] | str | None, "Target asset reference {guid|path} (for modify)."] = None, + # --- shared --- + patches: Annotated[list[dict[str, Any]] | str | None, "Patch list (or JSON string) to apply."] = None, +) -> dict[str, Any]: + unity_instance = get_unity_instance_from_context(ctx) + + # Tolerate JSON-string payloads (LLMs sometimes stringify complex objects) + parsed_target = parse_json_payload(target) + parsed_patches = parse_json_payload(patches) + + if parsed_target is not None and not isinstance(parsed_target, dict): + return {"success": False, "message": "manage_scriptable_object: 'target' must be an object {guid|path} (or JSON string of such)."} + + if parsed_patches is not None and not isinstance(parsed_patches, list): + return {"success": False, "message": "manage_scriptable_object: 'patches' must be a list (or JSON string of a list)."} + + params: dict[str, Any] = { + "action": action, + "typeName": type_name, + "folderPath": folder_path, + "assetName": asset_name, + "overwrite": coerce_bool(overwrite, default=None), + "target": parsed_target, + "patches": parsed_patches, + } + + # Remove None values to keep Unity handler simpler + params = {k: v for k, v in params.items() if v is not None} + + response = await send_with_unity_instance( + async_send_command_with_retry, + unity_instance, + "manage_scriptable_object", + params, + ) + await ctx.info(f"Response {response}") + return response if isinstance(response, dict) else {"success": False, "message": "Unexpected response from Unity."} + + + diff --git a/Server/tests/integration/test_manage_scriptable_object_tool.py b/Server/tests/integration/test_manage_scriptable_object_tool.py new file mode 100644 index 0000000..a40e4e3 --- /dev/null +++ b/Server/tests/integration/test_manage_scriptable_object_tool.py @@ -0,0 +1,72 @@ +import asyncio + +from .test_helpers import DummyContext +import services.tools.manage_scriptable_object as mod + + +def test_manage_scriptable_object_forwards_create_params(monkeypatch): + captured = {} + + async def fake_async_send(cmd, params, **kwargs): + captured["cmd"] = cmd + captured["params"] = params + return {"success": True, "data": {"ok": True}} + + monkeypatch.setattr(mod, "async_send_command_with_retry", fake_async_send) + + ctx = DummyContext() + ctx.set_state("unity_instance", "UnityMCPTests@dummy") + + result = asyncio.run( + mod.manage_scriptable_object( + ctx=ctx, + action="create", + type_name="My.Namespace.TestDefinition", + folder_path="Assets/Temp/Foo", + asset_name="Bar", + overwrite="true", + patches='[{"propertyPath":"displayName","op":"set","value":"Hello"}]', + ) + ) + + assert result["success"] is True + assert captured["cmd"] == "manage_scriptable_object" + assert captured["params"]["action"] == "create" + assert captured["params"]["typeName"] == "My.Namespace.TestDefinition" + assert captured["params"]["folderPath"] == "Assets/Temp/Foo" + assert captured["params"]["assetName"] == "Bar" + assert captured["params"]["overwrite"] is True + assert isinstance(captured["params"]["patches"], list) + assert captured["params"]["patches"][0]["propertyPath"] == "displayName" + + +def test_manage_scriptable_object_forwards_modify_params(monkeypatch): + captured = {} + + async def fake_async_send(cmd, params, **kwargs): + captured["cmd"] = cmd + captured["params"] = params + return {"success": True, "data": {"ok": True}} + + monkeypatch.setattr(mod, "async_send_command_with_retry", fake_async_send) + + ctx = DummyContext() + ctx.set_state("unity_instance", "UnityMCPTests@dummy") + + result = asyncio.run( + mod.manage_scriptable_object( + ctx=ctx, + action="modify", + target='{"guid":"abc"}', + patches=[{"propertyPath": "materials.Array.size", "op": "array_resize", "value": 2}], + ) + ) + + assert result["success"] is True + assert captured["cmd"] == "manage_scriptable_object" + assert captured["params"]["action"] == "modify" + assert captured["params"]["target"] == {"guid": "abc"} + assert captured["params"]["patches"][0]["op"] == "array_resize" + + + diff --git a/TestProjects/UnityMCPTests/Assets/Packages.meta b/TestProjects/UnityMCPTests/Assets/Packages.meta new file mode 100644 index 0000000..c35ade8 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Packages.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 28581c55743854f40bc0f3f4f52ae1f1 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Temp.meta b/TestProjects/UnityMCPTests/Assets/Temp.meta new file mode 100644 index 0000000..afa7b05 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Temp.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 20332651bb6f64cadb92cf3c6d68bed5 +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures.meta new file mode 100644 index 0000000..92fa77a --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: fbad1c3ddb00a48918a2b59a2a1714cb +folderAsset: yes +DefaultImporter: + externalObjects: {} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs new file mode 100644 index 0000000..bb7a3bf --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Generic; +using UnityEngine; + +namespace MCPForUnityTests.Editor.Tools.Fixtures +{ + [Serializable] + public struct ManageScriptableObjectNestedData + { + public string note; + } + + // NOTE: File name matches class name so Unity can resolve a MonoScript asset for this ScriptableObject type. + public class ManageScriptableObjectTestDefinition : ManageScriptableObjectTestDefinitionBase + { + [SerializeField] private string displayName; + [SerializeField] private List materials = new(); + [SerializeField] private ManageScriptableObjectNestedData nested; + + public string DisplayName => displayName; + public IReadOnlyList Materials => materials; + public string NestedNote => nested.note; + } +} + + + diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs.meta new file mode 100644 index 0000000..d51794a --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinition.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ba28697c0d65145a1ad753ef73c53185 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs new file mode 100644 index 0000000..aae9224 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs @@ -0,0 +1,14 @@ +using UnityEngine; + +namespace MCPForUnityTests.Editor.Tools.Fixtures +{ + // NOTE: File name matches class name so Unity can resolve a MonoScript asset for this ScriptableObject type. + public class ManageScriptableObjectTestDefinitionBase : ScriptableObject + { + [SerializeField] private int baseNumber = 1; + public int BaseNumber => baseNumber; + } +} + + + diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs.meta new file mode 100644 index 0000000..8ed6e2e --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/Fixtures/ManageScriptableObjectTestDefinitionBase.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a757068d676ee47dba1045bfb8b8fb12 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs new file mode 100644 index 0000000..2c3cdaf --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs @@ -0,0 +1,310 @@ +using System; +using Newtonsoft.Json.Linq; +using NUnit.Framework; +using UnityEditor; +using UnityEngine; +using MCPForUnity.Editor.Helpers; +using MCPForUnity.Editor.Tools; +using MCPForUnityTests.Editor.Tools.Fixtures; + +namespace MCPForUnityTests.Editor.Tools +{ + public class ManageScriptableObjectTests + { + private const string TempRoot = "Assets/Temp/ManageScriptableObjectTests"; + private const string NestedFolder = TempRoot + "/Nested/Deeper"; + + private string _createdAssetPath; + private string _createdGuid; + private string _matAPath; + private string _matBPath; + + [SetUp] + public void SetUp() + { + EnsureFolder("Assets/Temp"); + // Start from a clean slate every time (prevents intermittent setup failures). + if (AssetDatabase.IsValidFolder(TempRoot)) + { + AssetDatabase.DeleteAsset(TempRoot); + } + EnsureFolder(TempRoot); + + _createdAssetPath = null; + _createdGuid = null; + + // Create two Materials we can reference by guid/path. + _matAPath = $"{TempRoot}/MatA_{Guid.NewGuid():N}.mat"; + _matBPath = $"{TempRoot}/MatB_{Guid.NewGuid():N}.mat"; + var shader = Shader.Find("Universal Render Pipeline/Lit") + ?? Shader.Find("HDRP/Lit") + ?? Shader.Find("Standard") + ?? Shader.Find("Unlit/Color"); + Assert.IsNotNull(shader, "A fallback shader must be available for creating Material assets in tests."); + AssetDatabase.CreateAsset(new Material(shader), _matAPath); + AssetDatabase.CreateAsset(new Material(shader), _matBPath); + AssetDatabase.SaveAssets(); + AssetDatabase.Refresh(); + } + + [TearDown] + public void TearDown() + { + // Best-effort cleanup + if (!string.IsNullOrEmpty(_createdAssetPath) && AssetDatabase.LoadAssetAtPath(_createdAssetPath) != null) + { + AssetDatabase.DeleteAsset(_createdAssetPath); + } + if (!string.IsNullOrEmpty(_matAPath) && AssetDatabase.LoadAssetAtPath(_matAPath) != null) + { + AssetDatabase.DeleteAsset(_matAPath); + } + if (!string.IsNullOrEmpty(_matBPath) && AssetDatabase.LoadAssetAtPath(_matBPath) != null) + { + AssetDatabase.DeleteAsset(_matBPath); + } + + if (AssetDatabase.IsValidFolder(TempRoot)) + { + AssetDatabase.DeleteAsset(TempRoot); + } + + // Clean up parent Temp folder if empty + if (AssetDatabase.IsValidFolder("Assets/Temp")) + { + var remainingDirs = System.IO.Directory.GetDirectories("Assets/Temp"); + var remainingFiles = System.IO.Directory.GetFiles("Assets/Temp"); + if (remainingDirs.Length == 0 && remainingFiles.Length == 0) + { + AssetDatabase.DeleteAsset("Assets/Temp"); + } + } + + AssetDatabase.Refresh(); + } + + [Test] + public void Create_CreatesNestedFolders_PlacesAssetCorrectly_AndAppliesPatches() + { + var create = new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = NestedFolder, + ["assetName"] = "My_Test_Def", + ["overwrite"] = true, + ["patches"] = new JArray + { + new JObject { ["propertyPath"] = "displayName", ["op"] = "set", ["value"] = "Hello" }, + new JObject { ["propertyPath"] = "baseNumber", ["op"] = "set", ["value"] = 42 }, + new JObject { ["propertyPath"] = "nested.note", ["op"] = "set", ["value"] = "note!" } + } + }; + + var raw = ManageScriptableObject.HandleCommand(create); + var result = raw as JObject ?? JObject.FromObject(raw); + + Assert.IsTrue(result.Value("success"), result.ToString()); + var data = result["data"] as JObject; + Assert.IsNotNull(data, "Expected data payload"); + + _createdGuid = data!["guid"]?.ToString(); + _createdAssetPath = data["path"]?.ToString(); + + Assert.IsTrue(AssetDatabase.IsValidFolder(NestedFolder), "Nested folder should be created."); + Assert.IsTrue(_createdAssetPath!.StartsWith(NestedFolder, StringComparison.Ordinal), $"Asset should be created under {NestedFolder}: {_createdAssetPath}"); + Assert.IsTrue(_createdAssetPath.EndsWith(".asset", StringComparison.OrdinalIgnoreCase), "Asset should have .asset extension."); + Assert.IsFalse(string.IsNullOrWhiteSpace(_createdGuid), "Expected guid in response."); + + var asset = AssetDatabase.LoadAssetAtPath(_createdAssetPath); + Assert.IsNotNull(asset, "Created asset should load as TestDefinition."); + Assert.AreEqual("Hello", asset!.DisplayName, "Private [SerializeField] string should be set via SerializedProperty."); + Assert.AreEqual(42, asset.BaseNumber, "Inherited serialized field should be set via SerializedProperty."); + Assert.AreEqual("note!", asset.NestedNote, "Nested struct field should be set via SerializedProperty path."); + } + + [Test] + public void Modify_ArrayResize_ThenAssignObjectRefs_ByGuidAndByPath() + { + // Create base asset first with no patches. + var create = new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = TempRoot, + ["assetName"] = "Modify_Target", + ["overwrite"] = true + }; + var createRes = ToJObject(ManageScriptableObject.HandleCommand(create)); + Assert.IsTrue(createRes.Value("success"), createRes.ToString()); + _createdGuid = createRes["data"]?["guid"]?.ToString(); + _createdAssetPath = createRes["data"]?["path"]?.ToString(); + + var matAGuid = AssetDatabase.AssetPathToGUID(_matAPath); + + var modify = new JObject + { + ["action"] = "modify", + ["target"] = new JObject { ["guid"] = _createdGuid }, + ["patches"] = new JArray + { + // Resize list to 2 + new JObject { ["propertyPath"] = "materials.Array.size", ["op"] = "array_resize", ["value"] = 2 }, + // Assign element 0 by guid + new JObject + { + ["propertyPath"] = "materials.Array.data[0]", + ["op"] = "set", + ["ref"] = new JObject { ["guid"] = matAGuid } + }, + // Assign element 1 by path + new JObject + { + ["propertyPath"] = "materials.Array.data[1]", + ["op"] = "set", + ["ref"] = new JObject { ["path"] = _matBPath } + } + } + }; + + var modRes = ToJObject(ManageScriptableObject.HandleCommand(modify)); + Assert.IsTrue(modRes.Value("success"), modRes.ToString()); + + // Assert patch results are ok so failures are visible even if the tool returns success. + var results = modRes["data"]?["results"] as JArray; + Assert.IsNotNull(results, "Expected per-patch results in response."); + foreach (var r in results!) + { + Assert.IsTrue(r.Value("ok"), $"Patch failed: {r}"); + } + + var asset = AssetDatabase.LoadAssetAtPath(_createdAssetPath); + Assert.IsNotNull(asset); + Assert.AreEqual(2, asset!.Materials.Count, "List should be resized to 2."); + + var matA = AssetDatabase.LoadAssetAtPath(_matAPath); + var matB = AssetDatabase.LoadAssetAtPath(_matBPath); + Assert.AreEqual(matA, asset.Materials[0], "Element 0 should be set by GUID ref."); + Assert.AreEqual(matB, asset.Materials[1], "Element 1 should be set by path ref."); + } + + [Test] + public void Errors_InvalidAction_TypeNotFound_TargetNotFound() + { + // invalid action + var badAction = ToJObject(ManageScriptableObject.HandleCommand(new JObject { ["action"] = "nope" })); + Assert.IsFalse(badAction.Value("success")); + Assert.AreEqual("invalid_params", badAction.Value("error")); + + // type not found + var badType = ToJObject(ManageScriptableObject.HandleCommand(new JObject + { + ["action"] = "create", + ["typeName"] = "Nope.MissingType", + ["folderPath"] = TempRoot, + ["assetName"] = "X", + })); + Assert.IsFalse(badType.Value("success")); + Assert.AreEqual("type_not_found", badType.Value("error")); + + // target not found + var badTarget = ToJObject(ManageScriptableObject.HandleCommand(new JObject + { + ["action"] = "modify", + ["target"] = new JObject { ["guid"] = "00000000000000000000000000000000" }, + ["patches"] = new JArray(), + })); + Assert.IsFalse(badTarget.Value("success")); + Assert.AreEqual("target_not_found", badTarget.Value("error")); + } + + [Test] + public void Create_RejectsNonAssetsRootFolders() + { + var badPackages = ToJObject(ManageScriptableObject.HandleCommand(new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = "Packages/NotAllowed", + ["assetName"] = "BadFolder", + ["overwrite"] = true, + })); + Assert.IsFalse(badPackages.Value("success")); + Assert.AreEqual("invalid_folder_path", badPackages.Value("error")); + + var badAbsolute = ToJObject(ManageScriptableObject.HandleCommand(new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = "/tmp/not_allowed", + ["assetName"] = "BadFolder2", + ["overwrite"] = true, + })); + Assert.IsFalse(badAbsolute.Value("success")); + Assert.AreEqual("invalid_folder_path", badAbsolute.Value("error")); + + var badFileUri = ToJObject(ManageScriptableObject.HandleCommand(new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = "file:///tmp/not_allowed", + ["assetName"] = "BadFolder3", + ["overwrite"] = true, + })); + Assert.IsFalse(badFileUri.Value("success")); + Assert.AreEqual("invalid_folder_path", badFileUri.Value("error")); + } + + [Test] + public void Create_NormalizesRelativeAndBackslashPaths_AndAvoidsDoubleSlashesInResult() + { + var create = new JObject + { + ["action"] = "create", + ["typeName"] = typeof(ManageScriptableObjectTestDefinition).FullName, + ["folderPath"] = @"Temp\ManageScriptableObjectTests\SlashProbe\\Deep", + ["assetName"] = "SlashProbe", + ["overwrite"] = true, + }; + + var res = ToJObject(ManageScriptableObject.HandleCommand(create)); + Assert.IsTrue(res.Value("success"), res.ToString()); + + var path = res["data"]?["path"]?.ToString(); + Assert.IsNotNull(path, "Expected path in response."); + Assert.IsTrue(path!.StartsWith("Assets/Temp/ManageScriptableObjectTests/SlashProbe/Deep", StringComparison.Ordinal), + $"Expected sanitized Assets-rooted path, got: {path}"); + Assert.IsFalse(path.Contains("//", StringComparison.Ordinal), $"Path should not contain double slashes: {path}"); + } + + private static void EnsureFolder(string folderPath) + { + if (AssetDatabase.IsValidFolder(folderPath)) + return; + + // Only used for Assets/... paths in tests. + var sanitized = AssetPathUtility.SanitizeAssetPath(folderPath); + if (string.Equals(sanitized, "Assets", StringComparison.OrdinalIgnoreCase)) + return; + + var parts = sanitized.Split('/'); + string current = "Assets"; + for (int i = 1; i < parts.Length; i++) + { + var next = current + "/" + parts[i]; + if (!AssetDatabase.IsValidFolder(next)) + { + AssetDatabase.CreateFolder(current, parts[i]); + } + current = next; + } + } + + private static JObject ToJObject(object result) + { + return result as JObject ?? JObject.FromObject(result); + } + } +} + + diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs.meta new file mode 100644 index 0000000..70b99e8 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageScriptableObjectTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 1aac13ba83f134fc2ae264408d048c9b +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/ProjectSettings/SceneTemplateSettings.json b/TestProjects/UnityMCPTests/ProjectSettings/SceneTemplateSettings.json new file mode 100644 index 0000000..5e97f83 --- /dev/null +++ b/TestProjects/UnityMCPTests/ProjectSettings/SceneTemplateSettings.json @@ -0,0 +1,121 @@ +{ + "templatePinStates": [], + "dependencyTypeInfos": [ + { + "userAdded": false, + "type": "UnityEngine.AnimationClip", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEditor.Animations.AnimatorController", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.AnimatorOverrideController", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEditor.Audio.AudioMixerController", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.ComputeShader", + "defaultInstantiationMode": 1 + }, + { + "userAdded": false, + "type": "UnityEngine.Cubemap", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.GameObject", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEditor.LightingDataAsset", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.LightingSettings", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Material", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEditor.MonoScript", + "defaultInstantiationMode": 1 + }, + { + "userAdded": false, + "type": "UnityEngine.PhysicMaterial", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.PhysicsMaterial2D", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Rendering.PostProcessing.PostProcessProfile", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Rendering.PostProcessing.PostProcessResources", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Rendering.VolumeProfile", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEditor.SceneAsset", + "defaultInstantiationMode": 1 + }, + { + "userAdded": false, + "type": "UnityEngine.Shader", + "defaultInstantiationMode": 1 + }, + { + "userAdded": false, + "type": "UnityEngine.ShaderVariantCollection", + "defaultInstantiationMode": 1 + }, + { + "userAdded": false, + "type": "UnityEngine.Texture", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Texture2D", + "defaultInstantiationMode": 0 + }, + { + "userAdded": false, + "type": "UnityEngine.Timeline.TimelineAsset", + "defaultInstantiationMode": 0 + } + ], + "defaultDependencyTypeInfo": { + "userAdded": false, + "type": "", + "defaultInstantiationMode": 1 + }, + "newSceneOverride": 0 +} \ No newline at end of file