diff --git a/.gitignore b/.gitignore index 81f0d6684..104328886 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/FixscriptableobjecPlan.md b/FixscriptableobjecPlan.md new file mode 100644 index 000000000..8275fabf6 --- /dev/null +++ b/FixscriptableobjecPlan.md @@ -0,0 +1,206 @@ +## Fixing ScriptableObject creation/config in UnityMCP (design + plan) + +### Goal +Make ScriptableObject (SO) creation + configuration **reliable** when driven by an MCP client, specifically for: +- Correct folder/name placement +- Setting `[SerializeField] private` and inherited serialized fields +- UnityEngine.Object references (GUID/path-based) +- Arrays/lists and nested structs (e.g. `timeline.Array.data[0].eventDef`) + +### Non-goals +- A reflection-based “set any C# value” system (we should use Unity serialization paths) +- Runtime (player) support; this is editor tooling +- A full schema/introspection UI (we can add minimal discovery later) + +--- + +### Observed failure modes (what users see) +- Wrong placement: assets named “New …” landing in the wrong folder/category even though type is correct. +- Partial setup: asset creates, but setting references/lists/nested fields fails. +- Type lookup flakiness: `CreateInstance` fails during compilation/domain reload, or type resolution is too narrow. +- Field visibility mismatch: reflection misses `[SerializeField] private` and inherited serialized fields. + +--- + +## Core principle +All writes should go through `SerializedObject` / `SerializedProperty` using Unity **property paths**, not reflection. + +This is the robust way to support: +- private `[SerializeField]` fields +- inherited serialized fields +- arrays/lists (resize + assign) +- nested structs/classes +- object reference assignment + +--- + +## Proposed new UnityMCP tools (Unity-side bridge) + +### Tool A: `create_scriptable_object_asset` +Create an SO asset, optionally applying an initial patch set. + +#### Request (JSON) +- `typeName` (string): namespace-qualified type name (accept assembly-qualified too) +- `folderPath` (string): `Assets/...` folder +- `assetName` (string): file name without extension +- `overwrite` (bool, optional; default false): if false, use `AssetDatabase.GenerateUniqueAssetPath` +- `patches` (array, optional): same format as Tool B + +Example: +```json +{ + "typeName": "Game.Interactions.InteractionDefinition", + "folderPath": "Assets/Scriptable Objects/Interactions", + "assetName": "Int_Coffee_IntoCup", + "patches": [ + { "propertyPath": "displayName", "op": "set", "value": "Coffee Into Cup" }, + { "propertyPath": "requiredTags.Array.size", "op": "array_resize", "value": 2 }, + { "propertyPath": "requiredTags.Array.data[0]", "op": "set", "ref": { "guid": "..." } }, + { "propertyPath": "requiredTags.Array.data[1]", "op": "set", "ref": { "path": "Assets/Tags/Tag_ReceivesLiquid.asset" } } + ] +} +``` + +#### Response (JSON) +- `guid` (string) +- `path` (string) +- `typeNameResolved` (string) +- `patchResults` (array): per-patch result entries +- `warnings` (array of string) + +#### Errors (structured) +- `compiling_or_reloading` (retryable) +- `type_not_found` +- `invalid_folder_path` +- `asset_create_failed` + +#### Unity-side implementation notes +- Normalize/validate `folderPath`: + - forward slashes + - ensure prefix `Assets/` + - create folders recursively (`AssetDatabase.IsValidFolder` + `AssetDatabase.CreateFolder`) +- Block during compilation/domain reload: + - if `EditorApplication.isCompiling` or `EditorApplication.isUpdating` return `compiling_or_reloading` +- Resolve type: + - try `Type.GetType(typeName)` + - else scan `AppDomain.CurrentDomain.GetAssemblies()` and match `FullName` +- Create + place: + - `ScriptableObject.CreateInstance(resolvedType)` + - `AssetDatabase.GenerateUniqueAssetPath` unless `overwrite==true` + - `AssetDatabase.CreateAsset(instance, path)` +- Apply patches using Tool B logic via `SerializedObject`. +- Persist: + - `EditorUtility.SetDirty(asset)` + - `AssetDatabase.SaveAssets()` + +--- + +### Tool B: `set_serialized_properties` +Patch serialized fields on an existing target using Unity property paths. + +#### Request (JSON) +- `target` (object): exactly one of: + - `guid` (string) + - `path` (string) +- `patches` (array): patch objects + +#### Patch object schema +- `propertyPath` (string): Unity serialized path (examples below) +- `op` (string): + - `set` (default) + - `array_resize` +- `value` (any, optional): primitives/numbers/strings +- `ref` (object, optional): for `UnityEngine.Object` references: + - `guid` (string, optional) + - `path` (string, optional) + - `typeName` (string, optional; validation only) +- `expectType` (string, optional): used only for better error messages + +Example property paths: +- `displayName` +- `requiredTags.Array.size` +- `requiredTags.Array.data[0]` +- `timeline.Array.data[0].eventDef` + +#### Response (JSON) +- `targetGuid`, `targetPath`, `targetTypeName` +- `results` (array): + - `propertyPath` + - `op` + - `ok` (bool) + - `message` (string, optional) + - `resolvedPropertyType` (string, optional) + +#### Unity-side implementation notes +- Resolve the target by GUID/path. +- Use `SerializedObject`: + - `var so = new SerializedObject(target);` + - `var prop = so.FindProperty(propertyPath);` +- Apply per-op: + - `array_resize`: set `prop.intValue` (for `.Array.size`) or set `prop.arraySize` if targeting the array property + - `set`: + - primitives: set the correct `SerializedProperty` field (`stringValue`, `intValue`, `floatValue`, `boolValue`) + - enums: set `enumValueIndex` + - object refs: load via GUID/path and set `objectReferenceValue` +- Prefer batching `so.ApplyModifiedProperties()` once, but allow per-patch apply if array resizing requires it. + +--- + +## Shared infrastructure (recommended) + +### Path normalization +- Normalize to `Assets/...` and forward slashes. +- Ensure folder exists (recursive creation). +- Use `AssetDatabase.GenerateUniqueAssetPath` to avoid collisions. + +### Type resolution +- Differentiate clearly between: + - “type not found” + - “compiling/reloading” + +### Error reporting contract +Return structured errors with: +- `code` (stable) +- `message` (human-readable) +- `details` (input + resolved context: path, typeName, propertyPath) + +--- + +## Acceptance tests (definition of done) +These should be runnable as EditMode tests in a small Unity test project using the MCP endpoints. + +1. Create asset in nested folder + - Pass folder that doesn’t exist + - Verify folder is created, asset placed correctly + - Verify response includes `guid` and `path` + +2. Set private serialized field + - SO has `[SerializeField] private string displayName;` + - Patch `displayName`, verify it persists + +3. Set inherited serialized field + - Base class has a serialized field + - Patch via property path, verify persistence + +4. Set object reference by GUID and by path + - Patch reference field using GUID + - Patch reference field using path + +5. Resize list + assign elements + - Set `.Array.size`, then set `.Array.data[i]` + +6. Nested struct field + - Patch `timeline.Array.data[0].eventDef` + +7. Compilation-safe behavior + - When compiling/reloading, return `compiling_or_reloading` (retryable), not a generic failure + +--- + +## Open question (to map to the exact fix) +When you say “so many fails”, which are you seeing most often? +- A) failing to create the SO at all +- B) create succeeds but patching refs/lists fails +- C) create + patch succeed but it ends up in the wrong folder/name + +If you paste one representative failure payload/error, we can map it directly to a specific missing op/type/path rule. \ No newline at end of file diff --git a/MCPForUnity/Editor/Tools/ManageAsset.cs b/MCPForUnity/Editor/Tools/ManageAsset.cs index 7217da7c7..eecbcac71 100644 --- a/MCPForUnity/Editor/Tools/ManageAsset.cs +++ b/MCPForUnity/Editor/Tools/ManageAsset.cs @@ -229,31 +229,6 @@ private static object CreateAsset(JObject @params) 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 @@ private static object CreateAsset(JObject @params) // 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 @@ prop.Value is JObject componentProperties // 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 000000000..0ac467f1c --- /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 000000000..fa17b473e --- /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/Server/src/services/tools/manage_asset.py b/Server/src/services/tools/manage_asset.py index e7264ea67..3a4f65549 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 000000000..38249ae39 --- /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 000000000..a40e4e317 --- /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 000000000..c35ade8a1 --- /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 000000000..afa7b05b5 --- /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/Helpers/CodexConfigHelperTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs index a186e115b..7665e5301 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs @@ -32,6 +32,7 @@ public MockPlatformService(bool isWindows, string systemRoot = "C:\\Windows") private string _originalGitOverride; private bool _hadHttpTransport; private bool _originalHttpTransport; + private IPlatformService _originalPlatformService; [OneTimeSetUp] public void OneTimeSetUp() @@ -40,6 +41,7 @@ public void OneTimeSetUp() _originalGitOverride = EditorPrefs.GetString(EditorPrefKeys.GitUrlOverride, string.Empty); _hadHttpTransport = EditorPrefs.HasKey(EditorPrefKeys.UseHttpTransport); _originalHttpTransport = EditorPrefs.GetBool(EditorPrefKeys.UseHttpTransport, true); + _originalPlatformService = MCPServiceLocator.Platform; } [SetUp] @@ -54,8 +56,16 @@ public void SetUp() [TearDown] public void TearDown() { - // Reset service locator after each test - MCPServiceLocator.Reset(); + // IMPORTANT: + // These tests can be executed while an MCP session is active (e.g., when running tests via MCP). + // MCPServiceLocator.Reset() disposes the bridge + transport manager, which can kill the MCP connection + // mid-run. Instead, restore only what this fixture mutates. + // To avoid leaking global state to other tests/fixtures, restore the original platform service + // instance captured before this fixture started running. + if (_originalPlatformService != null) + { + MCPServiceLocator.Register(_originalPlatformService); + } } [OneTimeTearDown] 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 000000000..92fa77aa7 --- /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 000000000..bb7a3bffa --- /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 000000000..d51794a18 --- /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 000000000..aae9224b7 --- /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 000000000..8ed6e2ef5 --- /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 000000000..2c3cdaf2d --- /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 000000000..70b99e897 --- /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 000000000..5e97f8393 --- /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