diff --git a/.claude/prompts/nl-gameobject-suite.md b/.claude/prompts/nl-gameobject-suite.md index d3a1f50..1ff16de 100644 --- a/.claude/prompts/nl-gameobject-suite.md +++ b/.claude/prompts/nl-gameobject-suite.md @@ -115,13 +115,12 @@ AllowedTools: Write,mcp__UnityMCP__manage_editor,mcp__UnityMCP__manage_gameobjec - Clean up: `mcp__UnityMCP__manage_gameobject(action="delete", target="GO_Test_Object")` - **Pass criteria**: Pagination works (cursor present when more results available) -### GO-10. Deprecation Warnings -**Goal**: Verify legacy actions log deprecation warnings +### GO-10. Removed Actions Return Error +**Goal**: Verify legacy actions (find, get_components, etc.) return clear errors directing to new tools **Actions**: -- Call legacy action: `mcp__UnityMCP__manage_gameobject(action="find", search_term="Camera", search_method="by_component")` -- Read console using `mcp__UnityMCP__read_console` for deprecation warning -- Verify warning mentions "find_gameobjects" as replacement -- **Pass criteria**: Deprecation warning logged +- Call removed action: `mcp__UnityMCP__manage_gameobject(action="find", search_term="Camera", search_method="by_component")` +- Verify response contains error indicating action is unknown/removed +- **Pass criteria**: Error response received (legacy actions were removed, not deprecated) --- diff --git a/.github/workflows/claude-gameobject-suite.yml b/.github/workflows/claude-gameobject-suite.yml index 43b35dd..e8a56aa 100644 --- a/.github/workflows/claude-gameobject-suite.yml +++ b/.github/workflows/claude-gameobject-suite.yml @@ -473,8 +473,8 @@ jobs: if localname(froot.tag) == 'testcase': suite.append(froot) added += 1 - except Exception: - pass + except Exception as e: + print(f"Warning: Could not parse fragment {frag}: {e}") if added: for tc in list(suite.findall('.//testcase')): diff --git a/.github/workflows/claude-nl-suite.yml b/.github/workflows/claude-nl-suite.yml index 7b63628..4bae2e6 100644 --- a/.github/workflows/claude-nl-suite.yml +++ b/.github/workflows/claude-nl-suite.yml @@ -1235,7 +1235,12 @@ jobs: return (0, 999) if n.startswith('T-') and len(n) > 2: return (1, ord(n[2])) - return (2, n) + if n.startswith('GO-'): + try: + return (2, int(n.split('-')[1])) + except: + return (2, 999) + return (3, n) MAX_CHARS = 2000 seen = set() diff --git a/MCPForUnity/Editor/Helpers/AssetPathUtility.cs b/MCPForUnity/Editor/Helpers/AssetPathUtility.cs index a310c6e..832d44a 100644 --- a/MCPForUnity/Editor/Helpers/AssetPathUtility.cs +++ b/MCPForUnity/Editor/Helpers/AssetPathUtility.cs @@ -14,6 +14,17 @@ namespace MCPForUnity.Editor.Helpers /// public static class AssetPathUtility { + /// + /// Normalizes path separators to forward slashes without modifying the path structure. + /// Use this for non-asset paths (e.g., file system paths, relative directories). + /// + public static string NormalizeSeparators(string path) + { + if (string.IsNullOrEmpty(path)) + return path; + return path.Replace('\\', '/'); + } + /// /// Normalizes a Unity asset path by ensuring forward slashes are used and that it is rooted under "Assets/". /// @@ -24,7 +35,7 @@ namespace MCPForUnity.Editor.Helpers return path; } - path = path.Replace('\\', '/'); + path = NormalizeSeparators(path); if (!path.StartsWith("Assets/", StringComparison.OrdinalIgnoreCase)) { return "Assets/" + path.TrimStart('/'); diff --git a/MCPForUnity/Editor/Helpers/ComponentOps.cs b/MCPForUnity/Editor/Helpers/ComponentOps.cs new file mode 100644 index 0000000..5d5c4b7 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ComponentOps.cs @@ -0,0 +1,308 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Low-level component operations extracted from ManageGameObject and ManageComponents. + /// Provides pure C# operations without JSON parsing or response formatting. + /// + public static class ComponentOps + { + /// + /// Adds a component to a GameObject with Undo support. + /// + /// The target GameObject + /// The type of component to add + /// Error message if operation fails + /// The added component, or null if failed + public static Component AddComponent(GameObject target, Type componentType, out string error) + { + error = null; + + if (target == null) + { + error = "Target GameObject is null."; + return null; + } + + if (componentType == null || !typeof(Component).IsAssignableFrom(componentType)) + { + error = $"Type '{componentType?.Name ?? "null"}' is not a valid Component type."; + return null; + } + + // Prevent adding duplicate Transform + if (componentType == typeof(Transform)) + { + error = "Cannot add another Transform component."; + return null; + } + + // Check for 2D/3D physics conflicts + string conflictError = CheckPhysicsConflict(target, componentType); + if (conflictError != null) + { + error = conflictError; + return null; + } + + try + { + Component newComponent = Undo.AddComponent(target, componentType); + if (newComponent == null) + { + error = $"Failed to add component '{componentType.Name}' to '{target.name}'. It might be disallowed."; + return null; + } + + // Apply default values for specific component types + ApplyDefaultValues(newComponent); + + return newComponent; + } + catch (Exception ex) + { + error = $"Error adding component '{componentType.Name}': {ex.Message}"; + return null; + } + } + + /// + /// Removes a component from a GameObject with Undo support. + /// + /// The target GameObject + /// The type of component to remove + /// Error message if operation fails + /// True if component was removed successfully + public static bool RemoveComponent(GameObject target, Type componentType, out string error) + { + error = null; + + if (target == null) + { + error = "Target GameObject is null."; + return false; + } + + if (componentType == null) + { + error = "Component type is null."; + return false; + } + + // Prevent removing Transform + if (componentType == typeof(Transform)) + { + error = "Cannot remove Transform component."; + return false; + } + + Component component = target.GetComponent(componentType); + if (component == null) + { + error = $"Component '{componentType.Name}' not found on '{target.name}'."; + return false; + } + + try + { + Undo.DestroyObjectImmediate(component); + return true; + } + catch (Exception ex) + { + error = $"Error removing component '{componentType.Name}': {ex.Message}"; + return false; + } + } + + /// + /// Sets a property value on a component using reflection. + /// + /// The target component + /// The property or field name + /// The value to set (JToken) + /// Error message if operation fails + /// True if property was set successfully + public static bool SetProperty(Component component, string propertyName, JToken value, out string error) + { + error = null; + + if (component == null) + { + error = "Component is null."; + return false; + } + + if (string.IsNullOrEmpty(propertyName)) + { + error = "Property name is null or empty."; + return false; + } + + Type type = component.GetType(); + BindingFlags flags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase; + string normalizedName = ParamCoercion.NormalizePropertyName(propertyName); + + // Try property first - check both original and normalized names for backwards compatibility + PropertyInfo propInfo = type.GetProperty(propertyName, flags) + ?? type.GetProperty(normalizedName, flags); + if (propInfo != null && propInfo.CanWrite) + { + try + { + object convertedValue = PropertyConversion.ConvertToType(value, propInfo.PropertyType); + // Detect conversion failure: null result when input wasn't null + if (convertedValue == null && value.Type != JTokenType.Null) + { + error = $"Failed to convert value for property '{propertyName}' to type '{propInfo.PropertyType.Name}'."; + return false; + } + propInfo.SetValue(component, convertedValue); + return true; + } + catch (Exception ex) + { + error = $"Failed to set property '{propertyName}': {ex.Message}"; + return false; + } + } + + // Try field - check both original and normalized names for backwards compatibility + FieldInfo fieldInfo = type.GetField(propertyName, flags) + ?? type.GetField(normalizedName, flags); + if (fieldInfo != null && !fieldInfo.IsInitOnly) + { + try + { + object convertedValue = PropertyConversion.ConvertToType(value, fieldInfo.FieldType); + // Detect conversion failure: null result when input wasn't null + if (convertedValue == null && value.Type != JTokenType.Null) + { + error = $"Failed to convert value for field '{propertyName}' to type '{fieldInfo.FieldType.Name}'."; + return false; + } + fieldInfo.SetValue(component, convertedValue); + return true; + } + catch (Exception ex) + { + error = $"Failed to set field '{propertyName}': {ex.Message}"; + return false; + } + } + + // Try non-public serialized fields - check both original and normalized names + BindingFlags privateFlags = BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase; + fieldInfo = type.GetField(propertyName, privateFlags) + ?? type.GetField(normalizedName, privateFlags); + if (fieldInfo != null && fieldInfo.GetCustomAttribute() != null) + { + try + { + object convertedValue = PropertyConversion.ConvertToType(value, fieldInfo.FieldType); + // Detect conversion failure: null result when input wasn't null + if (convertedValue == null && value.Type != JTokenType.Null) + { + error = $"Failed to convert value for serialized field '{propertyName}' to type '{fieldInfo.FieldType.Name}'."; + return false; + } + fieldInfo.SetValue(component, convertedValue); + return true; + } + catch (Exception ex) + { + error = $"Failed to set serialized field '{propertyName}': {ex.Message}"; + return false; + } + } + + error = $"Property or field '{propertyName}' not found on component '{type.Name}'."; + return false; + } + + /// + /// Gets all public properties and fields from a component type. + /// + public static List GetAccessibleMembers(Type componentType) + { + var members = new List(); + if (componentType == null) return members; + + BindingFlags flags = BindingFlags.Public | BindingFlags.Instance; + + foreach (var prop in componentType.GetProperties(flags)) + { + if (prop.CanWrite && prop.GetSetMethod() != null) + { + members.Add(prop.Name); + } + } + + foreach (var field in componentType.GetFields(flags)) + { + if (!field.IsInitOnly) + { + members.Add(field.Name); + } + } + + // Include private [SerializeField] fields + foreach (var field in componentType.GetFields(BindingFlags.NonPublic | BindingFlags.Instance)) + { + if (field.GetCustomAttribute() != null) + { + members.Add(field.Name); + } + } + + members.Sort(); + return members; + } + + // --- Private Helpers --- + + private static string CheckPhysicsConflict(GameObject target, Type componentType) + { + bool isAdding2DPhysics = + typeof(Rigidbody2D).IsAssignableFrom(componentType) || + typeof(Collider2D).IsAssignableFrom(componentType); + + bool isAdding3DPhysics = + typeof(Rigidbody).IsAssignableFrom(componentType) || + typeof(Collider).IsAssignableFrom(componentType); + + if (isAdding2DPhysics) + { + if (target.GetComponent() != null || target.GetComponent() != null) + { + return $"Cannot add 2D physics component '{componentType.Name}' because the GameObject '{target.name}' already has a 3D Rigidbody or Collider."; + } + } + else if (isAdding3DPhysics) + { + if (target.GetComponent() != null || target.GetComponent() != null) + { + return $"Cannot add 3D physics component '{componentType.Name}' because the GameObject '{target.name}' already has a 2D Rigidbody or Collider."; + } + } + + return null; + } + + private static void ApplyDefaultValues(Component component) + { + // Default newly added Lights to Directional + if (component is Light light) + { + light.type = LightType.Directional; + } + } + } +} + diff --git a/MCPForUnity/Editor/Helpers/ComponentOps.cs.meta b/MCPForUnity/Editor/Helpers/ComponentOps.cs.meta new file mode 100644 index 0000000..e887250 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ComponentOps.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 13dead161bc4540eeb771961df437779 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Helpers/GameObjectLookup.cs b/MCPForUnity/Editor/Helpers/GameObjectLookup.cs index ce3ec2a..bce2a82 100644 --- a/MCPForUnity/Editor/Helpers/GameObjectLookup.cs +++ b/MCPForUnity/Editor/Helpers/GameObjectLookup.cs @@ -282,41 +282,12 @@ namespace MCPForUnity.Editor.Helpers /// /// Finds a component type by name, searching loaded assemblies. /// + /// + /// Delegates to UnityTypeResolver.ResolveComponent() for unified type resolution. + /// public static Type FindComponentType(string typeName) { - // Try direct type lookup first - var type = Type.GetType(typeName); - if (type != null && typeof(Component).IsAssignableFrom(type)) - return type; - - // Search in UnityEngine - type = typeof(Component).Assembly.GetType($"UnityEngine.{typeName}"); - if (type != null && typeof(Component).IsAssignableFrom(type)) - return type; - - // Search all loaded assemblies - foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies()) - { - try - { - // Try exact match - type = assembly.GetType(typeName); - if (type != null && typeof(Component).IsAssignableFrom(type)) - return type; - - // Try with UnityEngine prefix - type = assembly.GetType($"UnityEngine.{typeName}"); - if (type != null && typeof(Component).IsAssignableFrom(type)) - return type; - } - catch (Exception) - { - // Skip assemblies that can't be searched (e.g., dynamic, reflection-only) - // This is expected for some assemblies in Unity's domain - } - } - - return null; + return UnityTypeResolver.ResolveComponent(typeName); } /// diff --git a/MCPForUnity/Editor/Helpers/ObjectResolver.cs b/MCPForUnity/Editor/Helpers/ObjectResolver.cs new file mode 100644 index 0000000..6fc6065 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ObjectResolver.cs @@ -0,0 +1,201 @@ +using System; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Resolves Unity Objects by instruction (handles GameObjects, Components, Assets). + /// Extracted from ManageGameObject to eliminate cross-tool dependencies. + /// + public static class ObjectResolver + { + /// + /// Resolves any Unity Object by instruction. + /// + /// The type of Unity Object to resolve + /// JObject with "find" (required), "method" (optional), "component" (optional) + /// The resolved object, or null if not found + public static T Resolve(JObject instruction) where T : UnityEngine.Object + { + return Resolve(instruction, typeof(T)) as T; + } + + /// + /// Resolves any Unity Object by instruction. + /// + /// JObject with "find" (required), "method" (optional), "component" (optional) + /// The type of Unity Object to resolve + /// The resolved object, or null if not found + public static UnityEngine.Object Resolve(JObject instruction, Type targetType) + { + if (instruction == null) + return null; + + string findTerm = instruction["find"]?.ToString(); + string method = instruction["method"]?.ToString()?.ToLower(); + string componentName = instruction["component"]?.ToString(); + + if (string.IsNullOrEmpty(findTerm)) + { + Debug.LogWarning("[ObjectResolver] Find instruction missing 'find' term."); + return null; + } + + // Use a flexible default search method if none provided + string searchMethodToUse = string.IsNullOrEmpty(method) ? "by_id_or_name_or_path" : method; + + // --- Asset Search --- + // Normalize path separators before checking asset paths + string normalizedPath = AssetPathUtility.NormalizeSeparators(findTerm); + + // If the target is an asset type, try AssetDatabase first + if (IsAssetType(targetType) || + (typeof(GameObject).IsAssignableFrom(targetType) && normalizedPath.StartsWith("Assets/"))) + { + UnityEngine.Object asset = TryLoadAsset(normalizedPath, targetType); + if (asset != null) + return asset; + // If still not found, fall through to scene search + } + + // --- Scene Object Search --- + GameObject foundGo = GameObjectLookup.FindByTarget(new JValue(findTerm), searchMethodToUse, includeInactive: false); + + if (foundGo == null) + { + return null; + } + + // Get the target object/component from the found GameObject + if (targetType == typeof(GameObject)) + { + return foundGo; + } + else if (typeof(Component).IsAssignableFrom(targetType)) + { + Type componentToGetType = targetType; + if (!string.IsNullOrEmpty(componentName)) + { + Type specificCompType = GameObjectLookup.FindComponentType(componentName); + if (specificCompType != null && typeof(Component).IsAssignableFrom(specificCompType)) + { + componentToGetType = specificCompType; + } + else + { + Debug.LogWarning($"[ObjectResolver] Could not find component type '{componentName}'. Falling back to target type '{targetType.Name}'."); + } + } + + Component foundComp = foundGo.GetComponent(componentToGetType); + if (foundComp == null) + { + Debug.LogWarning($"[ObjectResolver] Found GameObject '{foundGo.name}' but could not find component of type '{componentToGetType.Name}'."); + } + return foundComp; + } + else + { + Debug.LogWarning($"[ObjectResolver] Find instruction handling not implemented for target type: {targetType.Name}"); + return null; + } + } + + /// + /// Convenience method to resolve a GameObject. + /// + public static GameObject ResolveGameObject(JToken target, string searchMethod = null) + { + if (target == null) + return null; + + // If target is a simple value, use GameObjectLookup directly + if (target.Type != JTokenType.Object) + { + return GameObjectLookup.FindByTarget(target, searchMethod ?? "by_id_or_name_or_path"); + } + + // If target is an instruction object + var instruction = target as JObject; + if (instruction != null) + { + return Resolve(instruction); + } + + return null; + } + + /// + /// Convenience method to resolve a Material. + /// + public static Material ResolveMaterial(string pathOrName) + { + if (string.IsNullOrEmpty(pathOrName)) + return null; + + var instruction = new JObject { ["find"] = pathOrName }; + return Resolve(instruction); + } + + /// + /// Convenience method to resolve a Texture. + /// + public static Texture ResolveTexture(string pathOrName) + { + if (string.IsNullOrEmpty(pathOrName)) + return null; + + var instruction = new JObject { ["find"] = pathOrName }; + return Resolve(instruction); + } + + // --- Private Helpers --- + + private static bool IsAssetType(Type type) + { + return typeof(Material).IsAssignableFrom(type) || + typeof(Texture).IsAssignableFrom(type) || + typeof(ScriptableObject).IsAssignableFrom(type) || + type.FullName?.StartsWith("UnityEngine.U2D") == true || + typeof(AudioClip).IsAssignableFrom(type) || + typeof(AnimationClip).IsAssignableFrom(type) || + typeof(Font).IsAssignableFrom(type) || + typeof(Shader).IsAssignableFrom(type) || + typeof(ComputeShader).IsAssignableFrom(type); + } + + private static UnityEngine.Object TryLoadAsset(string findTerm, Type targetType) + { + // Try loading directly by path first + UnityEngine.Object asset = AssetDatabase.LoadAssetAtPath(findTerm, targetType); + if (asset != null) + return asset; + + // Try generic load if type-specific failed + asset = AssetDatabase.LoadAssetAtPath(findTerm); + if (asset != null && targetType.IsAssignableFrom(asset.GetType())) + return asset; + + // Try finding by name/type using FindAssets + string searchFilter = $"t:{targetType.Name} {System.IO.Path.GetFileNameWithoutExtension(findTerm)}"; + string[] guids = AssetDatabase.FindAssets(searchFilter); + + if (guids.Length == 1) + { + asset = AssetDatabase.LoadAssetAtPath(AssetDatabase.GUIDToAssetPath(guids[0]), targetType); + if (asset != null) + return asset; + } + else if (guids.Length > 1) + { + Debug.LogWarning($"[ObjectResolver] Ambiguous asset find: Found {guids.Length} assets matching filter '{searchFilter}'. Provide a full path or unique name."); + return null; + } + + return null; + } + } +} + diff --git a/MCPForUnity/Editor/Helpers/ObjectResolver.cs.meta b/MCPForUnity/Editor/Helpers/ObjectResolver.cs.meta new file mode 100644 index 0000000..610a8f6 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/ObjectResolver.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: ad678f7b0a2e6458bbdb38a15d857acf +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Helpers/Pagination.cs b/MCPForUnity/Editor/Helpers/Pagination.cs new file mode 100644 index 0000000..e1d1387 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/Pagination.cs @@ -0,0 +1,149 @@ +using System.Collections.Generic; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Standard pagination request for all paginated tool operations. + /// Provides consistent handling of page_size/pageSize and cursor/page_number parameters. + /// + public class PaginationRequest + { + /// + /// Number of items per page. Default is 50. + /// + public int PageSize { get; set; } = 50; + + /// + /// 0-based cursor position for the current page. + /// + public int Cursor { get; set; } = 0; + + /// + /// Creates a PaginationRequest from JObject parameters. + /// Accepts both snake_case and camelCase parameter names for flexibility. + /// Converts 1-based page_number to 0-based cursor if needed. + /// + public static PaginationRequest FromParams(JObject @params, int defaultPageSize = 50) + { + if (@params == null) + return new PaginationRequest { PageSize = defaultPageSize }; + + // Accept both page_size and pageSize + int pageSize = ParamCoercion.CoerceInt( + @params["page_size"] ?? @params["pageSize"], + defaultPageSize + ); + + // Accept both cursor (0-based) and page_number (convert 1-based to 0-based) + var cursorToken = @params["cursor"]; + var pageNumberToken = @params["page_number"] ?? @params["pageNumber"]; + + int cursor; + if (cursorToken != null) + { + cursor = ParamCoercion.CoerceInt(cursorToken, 0); + } + else if (pageNumberToken != null) + { + // Convert 1-based page_number to 0-based cursor + int pageNumber = ParamCoercion.CoerceInt(pageNumberToken, 1); + cursor = (pageNumber - 1) * pageSize; + if (cursor < 0) cursor = 0; + } + else + { + cursor = 0; + } + + return new PaginationRequest + { + PageSize = pageSize > 0 ? pageSize : defaultPageSize, + Cursor = cursor + }; + } + } + + /// + /// Standard pagination response for all paginated tool operations. + /// Provides consistent response structure across all tools. + /// + /// The type of items in the paginated list + public class PaginationResponse + { + /// + /// The items on the current page. + /// + [JsonProperty("items")] + public List Items { get; set; } = new List(); + + /// + /// The cursor position for the current page (0-based). + /// + [JsonProperty("cursor")] + public int Cursor { get; set; } + + /// + /// The cursor for the next page, or null if this is the last page. + /// + [JsonProperty("nextCursor")] + public int? NextCursor { get; set; } + + /// + /// Total number of items across all pages. + /// + [JsonProperty("totalCount")] + public int TotalCount { get; set; } + + /// + /// Number of items per page. + /// + [JsonProperty("pageSize")] + public int PageSize { get; set; } + + /// + /// Whether there are more items after this page. + /// + [JsonProperty("hasMore")] + public bool HasMore => NextCursor.HasValue; + + /// + /// Creates a PaginationResponse from a full list of items and pagination parameters. + /// + /// The full list of items to paginate + /// The pagination request parameters + /// A paginated response with the appropriate slice of items + public static PaginationResponse Create(IList allItems, PaginationRequest request) + { + int totalCount = allItems.Count; + int cursor = request.Cursor; + int pageSize = request.PageSize; + + // Clamp cursor to valid range + if (cursor < 0) cursor = 0; + if (cursor > totalCount) cursor = totalCount; + + // Get the page of items + var items = new List(); + int endIndex = System.Math.Min(cursor + pageSize, totalCount); + for (int i = cursor; i < endIndex; i++) + { + items.Add(allItems[i]); + } + + // Calculate next cursor + int? nextCursor = endIndex < totalCount ? endIndex : (int?)null; + + return new PaginationResponse + { + Items = items, + Cursor = cursor, + NextCursor = nextCursor, + TotalCount = totalCount, + PageSize = pageSize + }; + } + } +} + diff --git a/MCPForUnity/Editor/Helpers/Pagination.cs.meta b/MCPForUnity/Editor/Helpers/Pagination.cs.meta new file mode 100644 index 0000000..8d0479b --- /dev/null +++ b/MCPForUnity/Editor/Helpers/Pagination.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 745564d5894d74c0ca24db39c77bab2c +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Helpers/ParamCoercion.cs b/MCPForUnity/Editor/Helpers/ParamCoercion.cs index 68f1fba..1b4b67c 100644 --- a/MCPForUnity/Editor/Helpers/ParamCoercion.cs +++ b/MCPForUnity/Editor/Helpers/ParamCoercion.cs @@ -155,6 +155,48 @@ namespace MCPForUnity.Editor.Helpers return defaultValue; } + + /// + /// Normalizes a property name by removing separators and converting to camelCase. + /// Handles common naming variations from LLMs and humans. + /// Examples: + /// "Use Gravity" → "useGravity" + /// "is_kinematic" → "isKinematic" + /// "max-angular-velocity" → "maxAngularVelocity" + /// "Angular Drag" → "angularDrag" + /// + /// The property name to normalize + /// The normalized camelCase property name + public static string NormalizePropertyName(string input) + { + if (string.IsNullOrEmpty(input)) + return input; + + // Split on common separators: space, underscore, dash + var parts = input.Split(new[] { ' ', '_', '-' }, StringSplitOptions.RemoveEmptyEntries); + if (parts.Length == 0) + return input; + + // First word is lowercase, subsequent words are Title case (camelCase) + var sb = new System.Text.StringBuilder(); + for (int i = 0; i < parts.Length; i++) + { + string part = parts[i]; + if (i == 0) + { + // First word: all lowercase + sb.Append(part.ToLowerInvariant()); + } + else + { + // Subsequent words: capitalize first letter, lowercase rest + sb.Append(char.ToUpperInvariant(part[0])); + if (part.Length > 1) + sb.Append(part.Substring(1).ToLowerInvariant()); + } + } + return sb.ToString(); + } } } diff --git a/MCPForUnity/Editor/Helpers/PropertyConversion.cs b/MCPForUnity/Editor/Helpers/PropertyConversion.cs new file mode 100644 index 0000000..a80ec27 --- /dev/null +++ b/MCPForUnity/Editor/Helpers/PropertyConversion.cs @@ -0,0 +1,92 @@ +using System; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using UnityEditor; +using UnityEngine; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Unified property conversion from JSON to Unity types. + /// Uses UnityJsonSerializer for consistent type handling. + /// + public static class PropertyConversion + { + /// + /// Converts a JToken to the specified target type using Unity type converters. + /// + /// The JSON token to convert + /// The target type to convert to + /// The converted object, or null if conversion fails + public static object ConvertToType(JToken token, Type targetType) + { + if (token == null || token.Type == JTokenType.Null) + { + if (targetType.IsValueType && Nullable.GetUnderlyingType(targetType) == null) + { + Debug.LogWarning($"[PropertyConversion] Cannot assign null to non-nullable value type {targetType.Name}. Returning default value."); + return Activator.CreateInstance(targetType); + } + return null; + } + + try + { + // Use the shared Unity serializer with custom converters + return token.ToObject(targetType, UnityJsonSerializer.Instance); + } + catch (Exception ex) + { + Debug.LogError($"Error converting token to {targetType.FullName}: {ex.Message}\nToken: {token.ToString(Formatting.None)}"); + throw; + } + } + + /// + /// Tries to convert a JToken to the specified target type. + /// Returns null and logs warning on failure (does not throw). + /// + public static object TryConvertToType(JToken token, Type targetType) + { + try + { + return ConvertToType(token, targetType); + } + catch + { + return null; + } + } + + /// + /// Generic version of ConvertToType. + /// + public static T ConvertTo(JToken token) + { + return (T)ConvertToType(token, typeof(T)); + } + + /// + /// Converts a JToken to a Unity asset by loading from path. + /// + /// JToken containing asset path + /// Expected asset type + /// The loaded asset, or null if not found + public static UnityEngine.Object LoadAssetFromToken(JToken token, Type targetType) + { + if (token == null || token.Type != JTokenType.String) + return null; + + string assetPath = AssetPathUtility.SanitizeAssetPath(token.ToString()); + UnityEngine.Object loadedAsset = AssetDatabase.LoadAssetAtPath(assetPath, targetType); + + if (loadedAsset == null) + { + Debug.LogWarning($"[PropertyConversion] Could not load asset of type {targetType.Name} from path: {assetPath}"); + } + + return loadedAsset; + } + } +} + diff --git a/MCPForUnity/Editor/Helpers/PropertyConversion.cs.meta b/MCPForUnity/Editor/Helpers/PropertyConversion.cs.meta new file mode 100644 index 0000000..098528c --- /dev/null +++ b/MCPForUnity/Editor/Helpers/PropertyConversion.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 4b4187d5b338a453fbe0baceaeea6bcd +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs b/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs new file mode 100644 index 0000000..83b650f --- /dev/null +++ b/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs @@ -0,0 +1,33 @@ +using System.Collections.Generic; +using Newtonsoft.Json; +using MCPForUnity.Runtime.Serialization; + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Shared JsonSerializer with Unity type converters. + /// Extracted from ManageGameObject to eliminate cross-tool dependencies. + /// + public static class UnityJsonSerializer + { + /// + /// Shared JsonSerializer instance with converters for Unity types. + /// Use this for all JToken-to-Unity-type conversions. + /// + public static readonly JsonSerializer Instance = JsonSerializer.Create(new JsonSerializerSettings + { + Converters = new List + { + new Vector2Converter(), + new Vector3Converter(), + new Vector4Converter(), + new QuaternionConverter(), + new ColorConverter(), + new RectConverter(), + new BoundsConverter(), + new UnityEngineObjectConverter() + } + }); + } +} + diff --git a/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs.meta b/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs.meta new file mode 100644 index 0000000..5c4391a --- /dev/null +++ b/MCPForUnity/Editor/Helpers/UnityJsonSerializer.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 24d94c9c030bd4ff1ab208c748f26b01 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs new file mode 100644 index 0000000..feb0b5c --- /dev/null +++ b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs @@ -0,0 +1,217 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using UnityEngine; +#if UNITY_EDITOR +using UnityEditor; +using UnityEditor.Compilation; +#endif + +namespace MCPForUnity.Editor.Helpers +{ + /// + /// Unified type resolution for Unity types (Components, ScriptableObjects, etc.). + /// Extracted from ComponentResolver in ManageGameObject and ResolveType in ManageScriptableObject. + /// Features: caching, prioritizes Player assemblies over Editor assemblies, uses TypeCache. + /// + public static class UnityTypeResolver + { + private static readonly Dictionary CacheByFqn = new(StringComparer.Ordinal); + private static readonly Dictionary CacheByName = new(StringComparer.Ordinal); + + /// + /// Resolves a type by name, with optional base type constraint. + /// Caches results for performance. Prefers runtime assemblies over Editor assemblies. + /// + /// The short name or fully-qualified name of the type + /// The resolved type, or null if not found + /// Error message if resolution failed + /// Optional base type constraint (e.g., typeof(Component)) + /// True if type was resolved successfully + public static bool TryResolve(string typeName, out Type type, out string error, Type requiredBaseType = null) + { + error = string.Empty; + type = null; + + if (string.IsNullOrWhiteSpace(typeName)) + { + error = "Type name cannot be null or empty"; + return false; + } + + // Check caches + if (CacheByFqn.TryGetValue(typeName, out type) && PassesConstraint(type, requiredBaseType)) + return true; + if (!typeName.Contains(".") && CacheByName.TryGetValue(typeName, out type) && PassesConstraint(type, requiredBaseType)) + return true; + + // Try direct Type.GetType + type = Type.GetType(typeName, throwOnError: false); + if (type != null && PassesConstraint(type, requiredBaseType)) + { + Cache(type); + return true; + } + + // Search loaded assemblies (prefer Player assemblies) + var candidates = FindCandidates(typeName, requiredBaseType); + if (candidates.Count == 1) + { + type = candidates[0]; + Cache(type); + return true; + } + if (candidates.Count > 1) + { + error = FormatAmbiguityError(typeName, candidates); + type = null; + return false; + } + +#if UNITY_EDITOR + // Last resort: TypeCache (fast index) + if (requiredBaseType != null) + { + var tc = TypeCache.GetTypesDerivedFrom(requiredBaseType) + .Where(t => NamesMatch(t, typeName)); + candidates = PreferPlayer(tc).ToList(); + if (candidates.Count == 1) + { + type = candidates[0]; + Cache(type); + return true; + } + if (candidates.Count > 1) + { + error = FormatAmbiguityError(typeName, candidates); + type = null; + return false; + } + } +#endif + + error = $"Type '{typeName}' not found in loaded runtime assemblies. " + + "Use a fully-qualified name (Namespace.TypeName) and ensure the script compiled."; + type = null; + return false; + } + + /// + /// Convenience method to resolve a Component type. + /// + public static Type ResolveComponent(string typeName) + { + if (TryResolve(typeName, out Type type, out _, typeof(Component))) + return type; + return null; + } + + /// + /// Convenience method to resolve a ScriptableObject type. + /// + public static Type ResolveScriptableObject(string typeName) + { + if (TryResolve(typeName, out Type type, out _, typeof(ScriptableObject))) + return type; + return null; + } + + /// + /// Convenience method to resolve any type without constraints. + /// + public static Type ResolveAny(string typeName) + { + if (TryResolve(typeName, out Type type, out _, null)) + return type; + return null; + } + + // --- Private Helpers --- + + private static bool PassesConstraint(Type type, Type requiredBaseType) + { + if (type == null) return false; + if (requiredBaseType == null) return true; + return requiredBaseType.IsAssignableFrom(type); + } + + private static bool NamesMatch(Type t, string query) => + t.Name.Equals(query, StringComparison.Ordinal) || + (t.FullName?.Equals(query, StringComparison.Ordinal) ?? false); + + private static void Cache(Type t) + { + if (t == null) return; + if (t.FullName != null) CacheByFqn[t.FullName] = t; + CacheByName[t.Name] = t; + } + + private static List FindCandidates(string query, Type requiredBaseType) + { + bool isShort = !query.Contains('.'); + var loaded = AppDomain.CurrentDomain.GetAssemblies(); + +#if UNITY_EDITOR + // Names of Player (runtime) script assemblies + var playerAsmNames = new HashSet( + CompilationPipeline.GetAssemblies(AssembliesType.Player).Select(a => a.name), + StringComparer.Ordinal); + + var playerAsms = loaded.Where(a => playerAsmNames.Contains(a.GetName().Name)); + var editorAsms = loaded.Except(playerAsms); +#else + var playerAsms = loaded; + var editorAsms = Array.Empty(); +#endif + + Func match = isShort + ? (t => t.Name.Equals(query, StringComparison.Ordinal)) + : (t => t.FullName?.Equals(query, StringComparison.Ordinal) ?? false); + + var fromPlayer = playerAsms.SelectMany(SafeGetTypes) + .Where(t => PassesConstraint(t, requiredBaseType)) + .Where(match); + var fromEditor = editorAsms.SelectMany(SafeGetTypes) + .Where(t => PassesConstraint(t, requiredBaseType)) + .Where(match); + + // Prefer Player over Editor + var candidates = fromPlayer.ToList(); + if (candidates.Count == 0) + candidates = fromEditor.ToList(); + + return candidates; + } + + private static IEnumerable SafeGetTypes(System.Reflection.Assembly assembly) + { + try { return assembly.GetTypes(); } + catch (ReflectionTypeLoadException rtle) { return rtle.Types.Where(t => t != null); } + catch { return Enumerable.Empty(); } + } + + private static IEnumerable PreferPlayer(IEnumerable types) + { +#if UNITY_EDITOR + var playerAsmNames = new HashSet( + CompilationPipeline.GetAssemblies(AssembliesType.Player).Select(a => a.name), + StringComparer.Ordinal); + + var list = types.ToList(); + var fromPlayer = list.Where(t => playerAsmNames.Contains(t.Assembly.GetName().Name)).ToList(); + return fromPlayer.Count > 0 ? fromPlayer : list; +#else + return types; +#endif + } + + private static string FormatAmbiguityError(string query, List candidates) + { + var names = string.Join(", ", candidates.Take(5).Select(t => t.FullName)); + if (candidates.Count > 5) names += $" ... ({candidates.Count - 5} more)"; + return $"Ambiguous type reference '{query}'. Found {candidates.Count} matches: [{names}]. Use a fully-qualified name."; + } + } +} + diff --git a/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs.meta b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs.meta new file mode 100644 index 0000000..c0eed2d --- /dev/null +++ b/MCPForUnity/Editor/Helpers/UnityTypeResolver.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 2cdf06f869b124741af31f27b25742db +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/MCPForUnity/Editor/Services/ServerManagementService.cs b/MCPForUnity/Editor/Services/ServerManagementService.cs index 0b9a0e9..b40788c 100644 --- a/MCPForUnity/Editor/Services/ServerManagementService.cs +++ b/MCPForUnity/Editor/Services/ServerManagementService.cs @@ -945,19 +945,32 @@ namespace MCPForUnity.Editor.Services if (Application.platform == RuntimePlatform.WindowsEditor) { - // netstat -ano | findstr : - success = ExecPath.TryRun("cmd.exe", $"/c netstat -ano | findstr :{port}", Application.dataPath, out stdout, out stderr); - if (success && !string.IsNullOrEmpty(stdout)) + // Run netstat -ano directly (without findstr) and filter in C#. + // Using findstr in a pipe causes the entire command to return exit code 1 when no matches are found, + // which ExecPath.TryRun interprets as failure. Running netstat alone gives us exit code 0 on success. + success = ExecPath.TryRun("netstat.exe", "-ano", Application.dataPath, out stdout, out stderr); + + // Process stdout regardless of success flag - netstat might still produce valid output + if (!string.IsNullOrEmpty(stdout)) { + string portSuffix = $":{port}"; var lines = stdout.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries); foreach (var line in lines) { - if (line.Contains("LISTENING")) + // Windows netstat format: Proto Local Address Foreign Address State PID + // Example: TCP 0.0.0.0:8080 0.0.0.0:0 LISTENING 12345 + if (line.Contains("LISTENING") && line.Contains(portSuffix)) { var parts = line.Split(new[] { ' ' }, StringSplitOptions.RemoveEmptyEntries); - if (parts.Length > 0 && int.TryParse(parts[parts.Length - 1], out int pid)) + // Verify the local address column actually ends with :{port} + // parts[0] = Proto (TCP), parts[1] = Local Address, parts[2] = Foreign Address, parts[3] = State, parts[4] = PID + if (parts.Length >= 5) { - results.Add(pid); + string localAddr = parts[1]; + if (localAddr.EndsWith(portSuffix) && int.TryParse(parts[parts.Length - 1], out int pid)) + { + results.Add(pid); + } } } } @@ -1002,16 +1015,44 @@ namespace MCPForUnity.Editor.Services { try { - bool debugLogs = false; - try { debugLogs = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false); } catch { } - - // Windows best-effort: tasklist /FI "PID eq X" + // Windows best-effort: First check process name with tasklist, then try to get command line with wmic if (Application.platform == RuntimePlatform.WindowsEditor) { - ExecPath.TryRun("cmd.exe", $"/c tasklist /FI \"PID eq {pid}\"", Application.dataPath, out var stdout, out var stderr, 5000); - string combined = ((stdout ?? string.Empty) + "\n" + (stderr ?? string.Empty)).ToLowerInvariant(); - // Common process names: python.exe, uv.exe, uvx.exe - return combined.Contains("python") || combined.Contains("uvx") || combined.Contains("uv.exe") || combined.Contains("uvx.exe"); + // Step 1: Check if process name matches known server executables + ExecPath.TryRun("cmd.exe", $"/c tasklist /FI \"PID eq {pid}\"", Application.dataPath, out var tasklistOut, out var tasklistErr, 5000); + string tasklistCombined = ((tasklistOut ?? string.Empty) + "\n" + (tasklistErr ?? string.Empty)).ToLowerInvariant(); + + // Check for common process names + bool isPythonOrUv = tasklistCombined.Contains("python") || tasklistCombined.Contains("uvx") || tasklistCombined.Contains("uv.exe"); + if (!isPythonOrUv) + { + return false; + } + + // Step 2: Try to get command line with wmic for better validation + ExecPath.TryRun("cmd.exe", $"/c wmic process where \"ProcessId={pid}\" get CommandLine /value", Application.dataPath, out var wmicOut, out var wmicErr, 5000); + string wmicCombined = ((wmicOut ?? string.Empty) + "\n" + (wmicErr ?? string.Empty)).ToLowerInvariant(); + string wmicCompact = NormalizeForMatch(wmicOut ?? string.Empty); + + // If we can see the command line, validate it's our server + if (!string.IsNullOrEmpty(wmicCombined) && wmicCombined.Contains("commandline=")) + { + bool mentionsMcp = wmicCompact.Contains("mcp-for-unity") + || wmicCompact.Contains("mcp_for_unity") + || wmicCompact.Contains("mcpforunity") + || wmicCompact.Contains("mcpforunityserver"); + bool mentionsTransport = wmicCompact.Contains("--transporthttp") || (wmicCompact.Contains("--transport") && wmicCompact.Contains("http")); + bool mentionsUvicorn = wmicCombined.Contains("uvicorn"); + + if (mentionsMcp || mentionsTransport || mentionsUvicorn) + { + return true; + } + } + + // Fall back to just checking for python/uv processes if wmic didn't give us details + // This is less precise but necessary for cases where wmic access is restricted + return isPythonOrUv; } // macOS/Linux: ps -p pid -ww -o comm= -o args= @@ -1027,7 +1068,6 @@ namespace MCPForUnity.Editor.Services string sCompact = NormalizeForMatch(raw); if (!string.IsNullOrEmpty(s)) { - bool mentionsMcp = sCompact.Contains("mcp-for-unity") || sCompact.Contains("mcp_for_unity") || sCompact.Contains("mcpforunity"); @@ -1058,15 +1098,6 @@ namespace MCPForUnity.Editor.Services { return true; } - - if (debugLogs) - { - LogStopDiagnosticsOnce(pid, $"ps='{TrimForLog(s)}' uvx={mentionsUvx} uv={mentionsUv} py={mentionsPython} uvicorn={mentionsUvicorn} mcp={mentionsMcp} transportHttp={mentionsTransport}"); - } - } - else if (debugLogs) - { - LogStopDiagnosticsOnce(pid, "ps output was empty (could not classify process)."); } } catch { } diff --git a/MCPForUnity/Editor/Tools/FindGameObjects.cs b/MCPForUnity/Editor/Tools/FindGameObjects.cs index b1c9498..3a59706 100644 --- a/MCPForUnity/Editor/Tools/FindGameObjects.cs +++ b/MCPForUnity/Editor/Tools/FindGameObjects.cs @@ -37,41 +37,30 @@ namespace MCPForUnity.Editor.Tools return new ErrorResponse("'searchTerm' or 'target' parameter is required."); } - // Pagination parameters - int pageSize = ParamCoercion.CoerceInt(@params["pageSize"] ?? @params["page_size"], 50); - int cursor = ParamCoercion.CoerceInt(@params["cursor"], 0); + // Pagination parameters using standard PaginationRequest + var pagination = PaginationRequest.FromParams(@params, defaultPageSize: 50); + pagination.PageSize = Mathf.Clamp(pagination.PageSize, 1, 500); // Search options bool includeInactive = ParamCoercion.CoerceBool(@params["includeInactive"] ?? @params["searchInactive"] ?? @params["include_inactive"], false); - // Validate pageSize bounds - pageSize = Mathf.Clamp(pageSize, 1, 500); - try { // Get all matching instance IDs var allIds = GameObjectLookup.SearchGameObjects(searchMethod, searchTerm, includeInactive, 0); - int totalCount = allIds.Count; + + // Use standard pagination response + var paginatedResult = PaginationResponse.Create(allIds, pagination); - // Apply pagination - var pagedIds = allIds.Skip(cursor).Take(pageSize).ToList(); - - // Calculate next cursor - int? nextCursor = cursor + pagedIds.Count < totalCount ? cursor + pagedIds.Count : (int?)null; - - return new + return new SuccessResponse("Found GameObjects", new { - success = true, - data = new - { - instanceIDs = pagedIds, - pageSize = pageSize, - cursor = cursor, - nextCursor = nextCursor, - totalCount = totalCount, - hasMore = nextCursor.HasValue - } - }; + instanceIDs = paginatedResult.Items, + pageSize = paginatedResult.PageSize, + cursor = paginatedResult.Cursor, + nextCursor = paginatedResult.NextCursor, + totalCount = paginatedResult.TotalCount, + hasMore = paginatedResult.HasMore + }); } catch (System.Exception ex) { diff --git a/MCPForUnity/Editor/Tools/ManageAsset.cs b/MCPForUnity/Editor/Tools/ManageAsset.cs index a4d1119..30daf5d 100644 --- a/MCPForUnity/Editor/Tools/ManageAsset.cs +++ b/MCPForUnity/Editor/Tools/ManageAsset.cs @@ -215,7 +215,7 @@ namespace MCPForUnity.Editor.Tools if (propertiesForApply.HasValues) { - MaterialOps.ApplyProperties(mat, propertiesForApply, ManageGameObject.InputSerializer); + MaterialOps.ApplyProperties(mat, propertiesForApply, UnityJsonSerializer.Instance); } } AssetDatabase.CreateAsset(mat, fullPath); @@ -418,7 +418,7 @@ namespace MCPForUnity.Editor.Tools { // Apply properties directly to the material. If this modifies, it sets modified=true. // Use |= in case the asset was already marked modified by previous logic (though unlikely here) - modified |= MaterialOps.ApplyProperties(material, properties, ManageGameObject.InputSerializer); + modified |= MaterialOps.ApplyProperties(material, properties, UnityJsonSerializer.Instance); } // Example: Modifying a ScriptableObject (Use manage_scriptable_object instead!) else if (asset is ScriptableObject so) @@ -989,7 +989,7 @@ namespace MCPForUnity.Editor.Tools System.Reflection.PropertyInfo propInfo = type.GetProperty(memberName, flags); if (propInfo != null && propInfo.CanWrite) { - object convertedValue = ConvertJTokenToType(value, propInfo.PropertyType); + object convertedValue = Helpers.PropertyConversion.TryConvertToType(value, propInfo.PropertyType); if ( convertedValue != null && !object.Equals(propInfo.GetValue(target), convertedValue) @@ -1004,7 +1004,7 @@ namespace MCPForUnity.Editor.Tools System.Reflection.FieldInfo fieldInfo = type.GetField(memberName, flags); if (fieldInfo != null) { - object convertedValue = ConvertJTokenToType(value, fieldInfo.FieldType); + object convertedValue = Helpers.PropertyConversion.TryConvertToType(value, fieldInfo.FieldType); if ( convertedValue != null && !object.Equals(fieldInfo.GetValue(target), convertedValue) @@ -1025,89 +1025,6 @@ namespace MCPForUnity.Editor.Tools return false; } - /// - /// Simple JToken to Type conversion for common Unity types and primitives. - /// - private static object ConvertJTokenToType(JToken token, Type targetType) - { - try - { - if (token == null || token.Type == JTokenType.Null) - return null; - - if (targetType == typeof(string)) - return token.ToObject(); - if (targetType == typeof(int)) - return token.ToObject(); - if (targetType == typeof(float)) - return token.ToObject(); - if (targetType == typeof(bool)) - return token.ToObject(); - if (targetType == typeof(Vector2) && token is JArray arrV2 && arrV2.Count == 2) - return new Vector2(arrV2[0].ToObject(), arrV2[1].ToObject()); - if (targetType == typeof(Vector3) && token is JArray arrV3 && arrV3.Count == 3) - return new Vector3( - arrV3[0].ToObject(), - arrV3[1].ToObject(), - arrV3[2].ToObject() - ); - if (targetType == typeof(Vector4) && token is JArray arrV4 && arrV4.Count == 4) - return new Vector4( - arrV4[0].ToObject(), - arrV4[1].ToObject(), - arrV4[2].ToObject(), - arrV4[3].ToObject() - ); - if (targetType == typeof(Quaternion) && token is JArray arrQ && arrQ.Count == 4) - return new Quaternion( - arrQ[0].ToObject(), - arrQ[1].ToObject(), - arrQ[2].ToObject(), - arrQ[3].ToObject() - ); - if (targetType == typeof(Color) && token is JArray arrC && arrC.Count >= 3) // Allow RGB or RGBA - return new Color( - arrC[0].ToObject(), - arrC[1].ToObject(), - arrC[2].ToObject(), - arrC.Count > 3 ? arrC[3].ToObject() : 1.0f - ); - if (targetType.IsEnum) - return Enum.Parse(targetType, token.ToString(), true); // Case-insensitive enum parsing - - // Handle loading Unity Objects (Materials, Textures, etc.) by path - if ( - typeof(UnityEngine.Object).IsAssignableFrom(targetType) - && token.Type == JTokenType.String - ) - { - string assetPath = AssetPathUtility.SanitizeAssetPath(token.ToString()); - UnityEngine.Object loadedAsset = AssetDatabase.LoadAssetAtPath( - assetPath, - targetType - ); - if (loadedAsset == null) - { - Debug.LogWarning( - $"[ConvertJTokenToType] Could not load asset of type {targetType.Name} from path: {assetPath}" - ); - } - return loadedAsset; - } - - // Fallback: Try direct conversion (might work for other simple value types) - return token.ToObject(targetType); - } - catch (Exception ex) - { - Debug.LogWarning( - $"[ConvertJTokenToType] Could not convert JToken '{token}' (type {token.Type}) to type '{targetType.Name}': {ex.Message}" - ); - return null; - } - } - - // --- Data Serialization --- /// diff --git a/MCPForUnity/Editor/Tools/ManageComponents.cs b/MCPForUnity/Editor/Tools/ManageComponents.cs index 921729e..57ec128 100644 --- a/MCPForUnity/Editor/Tools/ManageComponents.cs +++ b/MCPForUnity/Editor/Tools/ManageComponents.cs @@ -73,56 +73,48 @@ namespace MCPForUnity.Editor.Tools return new ErrorResponse($"Target GameObject ('{targetToken}') not found using method '{searchMethod ?? "default"}'."); } - string componentType = ParamCoercion.CoerceString(@params["componentType"] ?? @params["component_type"], null); - if (string.IsNullOrEmpty(componentType)) + string componentTypeName = ParamCoercion.CoerceString(@params["componentType"] ?? @params["component_type"], null); + if (string.IsNullOrEmpty(componentTypeName)) { return new ErrorResponse("'componentType' parameter is required for 'add' action."); } - // Resolve component type - Type type = FindComponentType(componentType); + // Resolve component type using unified type resolver + Type type = UnityTypeResolver.ResolveComponent(componentTypeName); if (type == null) { - return new ErrorResponse($"Component type '{componentType}' not found. Use a fully-qualified name if needed."); + return new ErrorResponse($"Component type '{componentTypeName}' not found. Use a fully-qualified name if needed."); } - // Optional properties to set on the new component + // Use ComponentOps for the actual operation + Component newComponent = ComponentOps.AddComponent(targetGo, type, out string error); + if (newComponent == null) + { + return new ErrorResponse(error ?? $"Failed to add component '{componentTypeName}'."); + } + + // Set properties if provided JObject properties = @params["properties"] as JObject ?? @params["componentProperties"] as JObject; - - try + if (properties != null && properties.HasValues) { - // Undo.AddComponent creates its own undo record, no need for RecordObject - Component newComponent = Undo.AddComponent(targetGo, type); - - if (newComponent == null) - { - return new ErrorResponse($"Failed to add component '{componentType}' to '{targetGo.name}'."); - } - - // Set properties if provided - if (properties != null && properties.HasValues) - { - SetPropertiesOnComponent(newComponent, properties); - } - - EditorUtility.SetDirty(targetGo); - - return new - { - success = true, - message = $"Component '{componentType}' added to '{targetGo.name}'.", - data = new - { - instanceID = targetGo.GetInstanceID(), - componentType = type.FullName, - componentInstanceID = newComponent.GetInstanceID() - } - }; + // Record for undo before modifying properties + Undo.RecordObject(newComponent, "Modify Component Properties"); + SetPropertiesOnComponent(newComponent, properties); } - catch (Exception e) + + EditorUtility.SetDirty(targetGo); + + return new { - return new ErrorResponse($"Error adding component '{componentType}': {e.Message}"); - } + success = true, + message = $"Component '{componentTypeName}' added to '{targetGo.name}'.", + data = new + { + instanceID = targetGo.GetInstanceID(), + componentType = type.FullName, + componentInstanceID = newComponent.GetInstanceID() + } + }; } private static object RemoveComponent(JObject @params, JToken targetToken, string searchMethod) @@ -133,50 +125,37 @@ namespace MCPForUnity.Editor.Tools return new ErrorResponse($"Target GameObject ('{targetToken}') not found using method '{searchMethod ?? "default"}'."); } - string componentType = ParamCoercion.CoerceString(@params["componentType"] ?? @params["component_type"], null); - if (string.IsNullOrEmpty(componentType)) + string componentTypeName = ParamCoercion.CoerceString(@params["componentType"] ?? @params["component_type"], null); + if (string.IsNullOrEmpty(componentTypeName)) { return new ErrorResponse("'componentType' parameter is required for 'remove' action."); } - // Resolve component type - Type type = FindComponentType(componentType); + // Resolve component type using unified type resolver + Type type = UnityTypeResolver.ResolveComponent(componentTypeName); if (type == null) { - return new ErrorResponse($"Component type '{componentType}' not found."); + return new ErrorResponse($"Component type '{componentTypeName}' not found."); } - // Prevent removal of Transform (check early before GetComponent) - if (type == typeof(Transform)) + // Use ComponentOps for the actual operation + bool removed = ComponentOps.RemoveComponent(targetGo, type, out string error); + if (!removed) { - return new ErrorResponse("Cannot remove the Transform component."); + return new ErrorResponse(error ?? $"Failed to remove component '{componentTypeName}'."); } - Component component = targetGo.GetComponent(type); - if (component == null) - { - return new ErrorResponse($"Component '{componentType}' not found on '{targetGo.name}'."); - } + EditorUtility.SetDirty(targetGo); - try + return new { - Undo.DestroyObjectImmediate(component); - EditorUtility.SetDirty(targetGo); - - return new + success = true, + message = $"Component '{componentTypeName}' removed from '{targetGo.name}'.", + data = new { - success = true, - message = $"Component '{componentType}' removed from '{targetGo.name}'.", - data = new - { - instanceID = targetGo.GetInstanceID() - } - }; - } - catch (Exception e) - { - return new ErrorResponse($"Error removing component '{componentType}': {e.Message}"); - } + instanceID = targetGo.GetInstanceID() + } + }; } private static object SetProperty(JObject @params, JToken targetToken, string searchMethod) @@ -193,8 +172,8 @@ namespace MCPForUnity.Editor.Tools return new ErrorResponse("'componentType' parameter is required for 'set_property' action."); } - // Resolve component type - Type type = FindComponentType(componentType); + // Resolve component type using unified type resolver + Type type = UnityTypeResolver.ResolveComponent(componentType); if (type == null) { return new ErrorResponse($"Component type '{componentType}' not found."); @@ -309,16 +288,6 @@ namespace MCPForUnity.Editor.Tools return GameObjectLookup.FindByTarget(targetToken, searchMethod ?? "by_name", true); } - /// - /// Finds a component type by name. Delegates to GameObjectLookup.FindComponentType. - /// - private static Type FindComponentType(string typeName) - { - if (string.IsNullOrEmpty(typeName)) - return null; - return GameObjectLookup.FindComponentType(typeName); - } - private static void SetPropertiesOnComponent(Component component, JObject properties) { if (component == null || properties == null) @@ -340,78 +309,20 @@ namespace MCPForUnity.Editor.Tools /// /// Attempts to set a property or field on a component. - /// Note: Property/field lookup is case-insensitive for better usability with external callers. + /// Delegates to ComponentOps.SetProperty for unified implementation. /// private static string TrySetProperty(Component component, string propertyName, JToken value) { if (component == null || string.IsNullOrEmpty(propertyName)) - return $"Invalid component or property name"; + return "Invalid component or property name"; - var type = component.GetType(); - - // Try property first - var propInfo = type.GetProperty(propertyName, BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase); - if (propInfo != null && propInfo.CanWrite) + if (ComponentOps.SetProperty(component, propertyName, value, out string error)) { - try - { - var convertedValue = ConvertValue(value, propInfo.PropertyType); - propInfo.SetValue(component, convertedValue); - return null; // Success - } - catch (Exception e) - { - Debug.LogWarning($"[ManageComponents] Failed to set property '{propertyName}': {e.Message}"); - return $"Failed to set property '{propertyName}': {e.Message}"; - } + return null; // Success } - // Try field - var fieldInfo = type.GetField(propertyName, BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase); - if (fieldInfo != null) - { - try - { - var convertedValue = ConvertValue(value, fieldInfo.FieldType); - fieldInfo.SetValue(component, convertedValue); - return null; // Success - } - catch (Exception e) - { - Debug.LogWarning($"[ManageComponents] Failed to set field '{propertyName}': {e.Message}"); - return $"Failed to set field '{propertyName}': {e.Message}"; - } - } - - Debug.LogWarning($"[ManageComponents] Property or field '{propertyName}' not found on {type.Name}"); - return $"Property '{propertyName}' not found on {type.Name}"; - } - - private static object ConvertValue(JToken token, Type targetType) - { - if (token == null || token.Type == JTokenType.Null) - return null; - - // Handle Unity types - if (targetType == typeof(Vector3)) - { - return VectorParsing.ParseVector3OrDefault(token); - } - if (targetType == typeof(Vector2)) - { - return VectorParsing.ParseVector2(token) ?? Vector2.zero; - } - if (targetType == typeof(Quaternion)) - { - return VectorParsing.ParseQuaternion(token) ?? Quaternion.identity; - } - if (targetType == typeof(Color)) - { - return VectorParsing.ParseColor(token) ?? Color.white; - } - - // Use Newtonsoft for other types - return token.ToObject(targetType); + Debug.LogWarning($"[ManageComponents] {error}"); + return error; } #endregion diff --git a/MCPForUnity/Editor/Tools/ManageGameObject.cs b/MCPForUnity/Editor/Tools/ManageGameObject.cs index 5c3fb14..28d8feb 100644 --- a/MCPForUnity/Editor/Tools/ManageGameObject.cs +++ b/MCPForUnity/Editor/Tools/ManageGameObject.cs @@ -22,20 +22,8 @@ namespace MCPForUnity.Editor.Tools [McpForUnityTool("manage_gameobject", AutoRegister = false)] public static class ManageGameObject { - // Shared JsonSerializer to avoid per-call allocation overhead - internal static readonly JsonSerializer InputSerializer = JsonSerializer.Create(new JsonSerializerSettings - { - Converters = new List - { - new Vector3Converter(), - new Vector2Converter(), - new QuaternionConverter(), - new ColorConverter(), - new RectConverter(), - new BoundsConverter(), - new UnityEngineObjectConverter() - } - }); + // Use shared serializer from helper class (backwards-compatible alias) + internal static JsonSerializer InputSerializer => UnityJsonSerializer.Instance; // --- Main Handler --- @@ -88,75 +76,23 @@ namespace MCPForUnity.Editor.Tools } } - // --- Prefab Redirection Check --- + // --- Prefab Asset Check --- + // Prefab assets require different tools. Only 'create' (instantiation) is valid here. string targetPath = targetToken?.Type == JTokenType.String ? targetToken.ToString() : null; if ( !string.IsNullOrEmpty(targetPath) && targetPath.EndsWith(".prefab", StringComparison.OrdinalIgnoreCase) + && action != "create" // Allow prefab instantiation ) { - // Allow 'create' (instantiate), 'find' (?), 'get_components' (?) - if (action == "modify" || action == "set_component_property") - { - Debug.Log( - $"[ManageGameObject->ManageAsset] Redirecting action '{action}' for prefab '{targetPath}' to ManageAsset." - ); - // Prepare params for ManageAsset.ModifyAsset - JObject assetParams = new JObject(); - assetParams["action"] = "modify"; // ManageAsset uses "modify" - assetParams["path"] = targetPath; - - // Extract properties. - // For 'set_component_property', combine componentName and componentProperties. - // For 'modify', directly use componentProperties. - JObject properties = null; - if (action == "set_component_property") - { - string compName = @params["componentName"]?.ToString(); - JObject compProps = @params["componentProperties"]?[compName] as JObject; // Handle potential nesting - if (string.IsNullOrEmpty(compName)) - return new ErrorResponse( - "Missing 'componentName' for 'set_component_property' on prefab." - ); - if (compProps == null) - return new ErrorResponse( - $"Missing or invalid 'componentProperties' for component '{compName}' for 'set_component_property' on prefab." - ); - - properties = new JObject(); - properties[compName] = compProps; - } - else // action == "modify" - { - properties = @params["componentProperties"] as JObject; - if (properties == null) - return new ErrorResponse( - "Missing 'componentProperties' for 'modify' action on prefab." - ); - } - - assetParams["properties"] = properties; - - // Call ManageAsset handler - return ManageAsset.HandleCommand(assetParams); - } - else if ( - action == "delete" - || action == "add_component" - || action == "remove_component" - || action == "get_components" - ) // Added get_components here too - { - // Explicitly block other modifications on the prefab asset itself via manage_gameobject - return new ErrorResponse( - $"Action '{action}' on a prefab asset ('{targetPath}') should be performed using the 'manage_asset' command." - ); - } - // Allow 'create' (instantiation) and 'find' to proceed, although finding a prefab asset by path might be less common via manage_gameobject. - // No specific handling needed here, the code below will run. + return new ErrorResponse( + $"Target '{targetPath}' is a prefab asset. " + + $"Use 'manage_asset' with action='modify' for prefab asset modifications, " + + $"or 'manage_prefabs' with action='open_stage' to edit the prefab in isolation mode." + ); } - // --- End Prefab Redirection Check --- + // --- End Prefab Asset Check --- try { @@ -398,43 +334,30 @@ namespace MCPForUnity.Editor.Tools // Set Tag (added for create action) if (!string.IsNullOrEmpty(tag)) { - // Similar logic as in ModifyGameObject for setting/creating tags - string tagToSet = string.IsNullOrEmpty(tag) ? "Untagged" : tag; - try + // Check if tag exists first (Unity doesn't throw exceptions for undefined tags, just logs a warning) + if (tag != "Untagged" && !System.Linq.Enumerable.Contains(InternalEditorUtility.tags, tag)) { - newGo.tag = tagToSet; - } - catch (UnityException ex) - { - if (ex.Message.Contains("is not defined")) + Debug.Log($"[ManageGameObject.Create] Tag '{tag}' not found. Creating it."); + try { - Debug.LogWarning( - $"[ManageGameObject.Create] Tag '{tagToSet}' not found. Attempting to create it." - ); - try - { - InternalEditorUtility.AddTag(tagToSet); - newGo.tag = tagToSet; // Retry - Debug.Log( - $"[ManageGameObject.Create] Tag '{tagToSet}' created and assigned successfully." - ); - } - catch (Exception innerEx) - { - UnityEngine.Object.DestroyImmediate(newGo); // Clean up - return new ErrorResponse( - $"Failed to create or assign tag '{tagToSet}' during creation: {innerEx.Message}." - ); - } + InternalEditorUtility.AddTag(tag); } - else + catch (Exception ex) { UnityEngine.Object.DestroyImmediate(newGo); // Clean up - return new ErrorResponse( - $"Failed to set tag to '{tagToSet}' during creation: {ex.Message}." - ); + return new ErrorResponse($"Failed to create tag '{tag}': {ex.Message}."); } } + + try + { + newGo.tag = tag; + } + catch (Exception ex) + { + UnityEngine.Object.DestroyImmediate(newGo); // Clean up + return new ErrorResponse($"Failed to set tag to '{tag}' during creation: {ex.Message}."); + } } // Set Layer (new for create action) @@ -666,49 +589,29 @@ namespace MCPForUnity.Editor.Tools { // Ensure the tag is not empty, if empty, it means "Untagged" implicitly string tagToSet = string.IsNullOrEmpty(tag) ? "Untagged" : tag; + + // Check if tag exists first (Unity doesn't throw exceptions for undefined tags, just logs a warning) + if (tagToSet != "Untagged" && !System.Linq.Enumerable.Contains(InternalEditorUtility.tags, tagToSet)) + { + Debug.Log($"[ManageGameObject] Tag '{tagToSet}' not found. Creating it."); + try + { + InternalEditorUtility.AddTag(tagToSet); + } + catch (Exception ex) + { + return new ErrorResponse($"Failed to create tag '{tagToSet}': {ex.Message}."); + } + } + try { targetGo.tag = tagToSet; modified = true; } - catch (UnityException ex) + catch (Exception ex) { - // Check if the error is specifically because the tag doesn't exist - if (ex.Message.Contains("is not defined")) - { - Debug.LogWarning( - $"[ManageGameObject] Tag '{tagToSet}' not found. Attempting to create it." - ); - try - { - // Attempt to create the tag using internal utility - InternalEditorUtility.AddTag(tagToSet); - // Wait a frame maybe? Not strictly necessary but sometimes helps editor updates. - // yield return null; // Cannot yield here, editor script limitation - - // Retry setting the tag immediately after creation - targetGo.tag = tagToSet; - modified = true; - Debug.Log( - $"[ManageGameObject] Tag '{tagToSet}' created and assigned successfully." - ); - } - catch (Exception innerEx) - { - // Handle failure during tag creation or the second assignment attempt - Debug.LogError( - $"[ManageGameObject] Failed to create or assign tag '{tagToSet}' after attempting creation: {innerEx.Message}" - ); - return new ErrorResponse( - $"Failed to create or assign tag '{tagToSet}': {innerEx.Message}. Check Tag Manager and permissions." - ); - } - } - else - { - // If the exception was for a different reason, return the original error - return new ErrorResponse($"Failed to set tag to '{tagToSet}': {ex.Message}."); - } + return new ErrorResponse($"Failed to set tag to '{tagToSet}': {ex.Message}."); } } @@ -1288,22 +1191,20 @@ namespace MCPForUnity.Editor.Tools Type componentType = FindType(searchTerm); if (componentType != null) { - // Determine FindObjectsInactive based on the searchInactive flag - FindObjectsInactive findInactive = searchInactive - ? FindObjectsInactive.Include - : FindObjectsInactive.Exclude; - // Replace FindObjectsOfType with FindObjectsByType, specifying the sorting mode and inactive state - var searchPoolComp = rootSearchObject - ? rootSearchObject + IEnumerable searchPoolComp; + if (rootSearchObject) + { + searchPoolComp = rootSearchObject .GetComponentsInChildren(componentType, searchInactive) - .Select(c => (c as Component).gameObject) - : UnityEngine - .Object.FindObjectsByType( - componentType, - findInactive, - FindObjectsSortMode.None - ) .Select(c => (c as Component).gameObject); + } + else + { + // Use FindObjectsOfType overload that respects includeInactive + searchPoolComp = UnityEngine.Object.FindObjectsOfType(componentType, searchInactive) + .Cast() + .Select(c => c.gameObject); + } results.AddRange(searchPoolComp.Where(go => go != null)); // Ensure GO is valid } else @@ -1560,7 +1461,7 @@ namespace MCPForUnity.Editor.Tools if (!setResult) { var availableProperties = ComponentResolver.GetAllComponentProperties(targetComponent.GetType()); - var suggestions = ComponentResolver.GetAIPropertySuggestions(propName, availableProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions(propName, availableProperties); var msg = suggestions.Any() ? $"Property '{propName}' not found. Did you mean: {string.Join(", ", suggestions)}? Available: [{string.Join(", ", availableProperties)}]" : $"Property '{propName}' not found. Available: [{string.Join(", ", availableProperties)}]"; @@ -1591,6 +1492,9 @@ namespace MCPForUnity.Editor.Tools BindingFlags flags = BindingFlags.Public | BindingFlags.Instance | BindingFlags.IgnoreCase; + // Normalize property name: "Use Gravity" → "useGravity", "is_kinematic" → "isKinematic" + string normalizedName = Helpers.ParamCoercion.NormalizePropertyName(memberName); + // Use shared serializer to avoid per-call allocation var inputSerializer = InputSerializer; @@ -1604,7 +1508,9 @@ namespace MCPForUnity.Editor.Tools return SetNestedProperty(target, memberName, value, inputSerializer); } - PropertyInfo propInfo = type.GetProperty(memberName, flags); + // Try both original and normalized names + PropertyInfo propInfo = type.GetProperty(memberName, flags) + ?? type.GetProperty(normalizedName, flags); if (propInfo != null && propInfo.CanWrite) { // Use the inputSerializer for conversion @@ -1621,7 +1527,9 @@ namespace MCPForUnity.Editor.Tools } else { - FieldInfo fieldInfo = type.GetField(memberName, flags); + // Try both original and normalized names for fields + FieldInfo fieldInfo = type.GetField(memberName, flags) + ?? type.GetField(normalizedName, flags); if (fieldInfo != null) // Check if !IsLiteral? { // Use the inputSerializer for conversion @@ -1638,8 +1546,9 @@ namespace MCPForUnity.Editor.Tools } else { - // Try NonPublic [SerializeField] fields - var npField = type.GetField(memberName, BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase); + // Try NonPublic [SerializeField] fields (with both original and normalized names) + var npField = type.GetField(memberName, BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase) + ?? type.GetField(normalizedName, BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase); if (npField != null && npField.GetCustomAttribute() != null) { object convertedValue = ConvertJTokenToType(value, npField.FieldType, inputSerializer); @@ -1871,45 +1780,14 @@ namespace MCPForUnity.Editor.Tools /// /// Simple JToken to Type conversion for common Unity types, using JsonSerializer. /// - // Pass the input serializer + /// + /// Delegates to PropertyConversion.ConvertToType for unified type handling. + /// The inputSerializer parameter is kept for backwards compatibility but is ignored + /// as PropertyConversion uses UnityJsonSerializer.Instance internally. + /// private static object ConvertJTokenToType(JToken token, Type targetType, JsonSerializer inputSerializer) { - if (token == null || token.Type == JTokenType.Null) - { - if (targetType.IsValueType && Nullable.GetUnderlyingType(targetType) == null) - { - Debug.LogWarning($"Cannot assign null to non-nullable value type {targetType.Name}. Returning default value."); - return Activator.CreateInstance(targetType); - } - return null; - } - - try - { - // Use the provided serializer instance which includes our custom converters - return token.ToObject(targetType, inputSerializer); - } - catch (JsonSerializationException jsonEx) - { - Debug.LogError($"JSON Deserialization Error converting token to {targetType.FullName}: {jsonEx.Message}\nToken: {token.ToString(Formatting.None)}"); - // Optionally re-throw or return null/default - // return targetType.IsValueType ? Activator.CreateInstance(targetType) : null; - throw; // Re-throw to indicate failure higher up - } - catch (ArgumentException argEx) - { - Debug.LogError($"Argument Error converting token to {targetType.FullName}: {argEx.Message}\nToken: {token.ToString(Formatting.None)}"); - throw; - } - catch (Exception ex) - { - Debug.LogError($"Unexpected error converting token to {targetType.FullName}: {ex}\nToken: {token.ToString(Formatting.None)}"); - throw; - } - // If ToObject succeeded, it would have returned. If it threw, we wouldn't reach here. - // This fallback logic is likely unreachable if ToObject covers all cases or throws on failure. - // Debug.LogWarning($"Conversion failed for token to {targetType.FullName}. Token: {token.ToString(Formatting.None)}"); - // return targetType.IsValueType ? Activator.CreateInstance(targetType) : null; + return PropertyConversion.ConvertToType(token, targetType); } // --- ParseJTokenTo... helpers are likely redundant now with the serializer approach --- @@ -2011,104 +1889,13 @@ namespace MCPForUnity.Editor.Tools /// Finds a specific UnityEngine.Object based on a find instruction JObject. /// Primarily used by UnityEngineObjectConverter during deserialization. /// - // Made public static so UnityEngineObjectConverter can call it. Moved from ConvertJTokenToType. + /// + /// This method now delegates to ObjectResolver.Resolve() for cleaner architecture. + /// Kept for backwards compatibility with existing code. + /// public static UnityEngine.Object FindObjectByInstruction(JObject instruction, Type targetType) { - string findTerm = instruction["find"]?.ToString(); - string method = instruction["method"]?.ToString()?.ToLower(); - string componentName = instruction["component"]?.ToString(); // Specific component to get - - if (string.IsNullOrEmpty(findTerm)) - { - Debug.LogWarning("Find instruction missing 'find' term."); - return null; - } - - // Use a flexible default search method if none provided - string searchMethodToUse = string.IsNullOrEmpty(method) ? "by_id_or_name_or_path" : method; - - // If the target is an asset (Material, Texture, ScriptableObject etc.) try AssetDatabase first - if (typeof(Material).IsAssignableFrom(targetType) || - typeof(Texture).IsAssignableFrom(targetType) || - typeof(ScriptableObject).IsAssignableFrom(targetType) || - targetType.FullName.StartsWith("UnityEngine.U2D") || // Sprites etc. - typeof(AudioClip).IsAssignableFrom(targetType) || - typeof(AnimationClip).IsAssignableFrom(targetType) || - typeof(Font).IsAssignableFrom(targetType) || - typeof(Shader).IsAssignableFrom(targetType) || - typeof(ComputeShader).IsAssignableFrom(targetType) || - typeof(GameObject).IsAssignableFrom(targetType) && findTerm.StartsWith("Assets/")) // Prefab check - { - // Try loading directly by path/GUID first - UnityEngine.Object asset = AssetDatabase.LoadAssetAtPath(findTerm, targetType); - if (asset != null) return asset; - asset = AssetDatabase.LoadAssetAtPath(findTerm); // Try generic if type specific failed - if (asset != null && targetType.IsAssignableFrom(asset.GetType())) return asset; - - - // If direct path failed, try finding by name/type using FindAssets - string searchFilter = $"t:{targetType.Name} {System.IO.Path.GetFileNameWithoutExtension(findTerm)}"; // Search by type and name - string[] guids = AssetDatabase.FindAssets(searchFilter); - - if (guids.Length == 1) - { - asset = AssetDatabase.LoadAssetAtPath(AssetDatabase.GUIDToAssetPath(guids[0]), targetType); - if (asset != null) return asset; - } - else if (guids.Length > 1) - { - Debug.LogWarning($"[FindObjectByInstruction] Ambiguous asset find: Found {guids.Length} assets matching filter '{searchFilter}'. Provide a full path or unique name."); - // Optionally return the first one? Or null? Returning null is safer. - return null; - } - // If still not found, fall through to scene search (though unlikely for assets) - } - - - // --- Scene Object Search --- - // Find the GameObject using the internal finder - GameObject foundGo = FindObjectInternal(new JValue(findTerm), searchMethodToUse); - - if (foundGo == null) - { - // Don't warn yet, could still be an asset not found above - // Debug.LogWarning($"Could not find GameObject using instruction: {instruction}"); - return null; - } - - // Now, get the target object/component from the found GameObject - if (targetType == typeof(GameObject)) - { - return foundGo; // We were looking for a GameObject - } - else if (typeof(Component).IsAssignableFrom(targetType)) - { - Type componentToGetType = targetType; - if (!string.IsNullOrEmpty(componentName)) - { - Type specificCompType = FindType(componentName); - if (specificCompType != null && typeof(Component).IsAssignableFrom(specificCompType)) - { - componentToGetType = specificCompType; - } - else - { - Debug.LogWarning($"Could not find component type '{componentName}' specified in find instruction. Falling back to target type '{targetType.Name}'."); - } - } - - Component foundComp = foundGo.GetComponent(componentToGetType); - if (foundComp == null) - { - Debug.LogWarning($"Found GameObject '{foundGo.name}' but could not find component of type '{componentToGetType.Name}'."); - } - return foundComp; - } - else - { - Debug.LogWarning($"Find instruction handling not implemented for target type: {targetType.Name}"); - return null; - } + return ObjectResolver.Resolve(instruction, targetType); } @@ -2134,125 +1921,18 @@ namespace MCPForUnity.Editor.Tools } /// - /// Robust component resolver that avoids Assembly.LoadFrom and supports assembly definitions. - /// Prioritizes runtime (Player) assemblies over Editor assemblies. + /// Component resolver that delegates to UnityTypeResolver. + /// Kept for backwards compatibility. /// internal static class ComponentResolver { - private static readonly Dictionary CacheByFqn = new(StringComparer.Ordinal); - private static readonly Dictionary CacheByName = new(StringComparer.Ordinal); - /// /// Resolve a Component/MonoBehaviour type by short or fully-qualified name. - /// Prefers runtime (Player) script assemblies; falls back to Editor assemblies. - /// Never uses Assembly.LoadFrom. + /// Delegates to UnityTypeResolver.TryResolve with Component constraint. /// public static bool TryResolve(string nameOrFullName, out Type type, out string error) { - error = string.Empty; - type = null!; - - // Handle null/empty input - if (string.IsNullOrWhiteSpace(nameOrFullName)) - { - error = "Component name cannot be null or empty"; - return false; - } - - // 1) Exact cache hits - if (CacheByFqn.TryGetValue(nameOrFullName, out type)) return true; - if (!nameOrFullName.Contains(".") && CacheByName.TryGetValue(nameOrFullName, out type)) return true; - type = Type.GetType(nameOrFullName, throwOnError: false); - if (IsValidComponent(type)) { Cache(type); return true; } - - // 2) Search loaded assemblies (prefer Player assemblies) - var candidates = FindCandidates(nameOrFullName); - if (candidates.Count == 1) { type = candidates[0]; Cache(type); return true; } - if (candidates.Count > 1) { error = Ambiguity(nameOrFullName, candidates); type = null!; return false; } - -#if UNITY_EDITOR - // 3) Last resort: Editor-only TypeCache (fast index) - var tc = TypeCache.GetTypesDerivedFrom() - .Where(t => NamesMatch(t, nameOrFullName)); - candidates = PreferPlayer(tc).ToList(); - if (candidates.Count == 1) { type = candidates[0]; Cache(type); return true; } - if (candidates.Count > 1) { error = Ambiguity(nameOrFullName, candidates); type = null!; return false; } -#endif - - error = $"Component type '{nameOrFullName}' not found in loaded runtime assemblies. " + - "Use a fully-qualified name (Namespace.TypeName) and ensure the script compiled."; - type = null!; - return false; - } - - private static bool NamesMatch(Type t, string q) => - t.Name.Equals(q, StringComparison.Ordinal) || - (t.FullName?.Equals(q, StringComparison.Ordinal) ?? false); - - private static bool IsValidComponent(Type t) => - t != null && typeof(Component).IsAssignableFrom(t); - - private static void Cache(Type t) - { - if (t.FullName != null) CacheByFqn[t.FullName] = t; - CacheByName[t.Name] = t; - } - - private static List FindCandidates(string query) - { - bool isShort = !query.Contains('.'); - var loaded = AppDomain.CurrentDomain.GetAssemblies(); - -#if UNITY_EDITOR - // Names of Player (runtime) script assemblies (asmdefs + Assembly-CSharp) - var playerAsmNames = new HashSet( - UnityEditor.Compilation.CompilationPipeline.GetAssemblies(UnityEditor.Compilation.AssembliesType.Player).Select(a => a.name), - StringComparer.Ordinal); - - IEnumerable playerAsms = loaded.Where(a => playerAsmNames.Contains(a.GetName().Name)); - IEnumerable editorAsms = loaded.Except(playerAsms); -#else - IEnumerable playerAsms = loaded; - IEnumerable editorAsms = Array.Empty(); -#endif - static IEnumerable SafeGetTypes(System.Reflection.Assembly a) - { - try { return a.GetTypes(); } - catch (ReflectionTypeLoadException rtle) { return rtle.Types.Where(t => t != null)!; } - } - - Func match = isShort - ? (t => t.Name.Equals(query, StringComparison.Ordinal)) - : (t => t.FullName!.Equals(query, StringComparison.Ordinal)); - - var fromPlayer = playerAsms.SelectMany(SafeGetTypes) - .Where(IsValidComponent) - .Where(match); - var fromEditor = editorAsms.SelectMany(SafeGetTypes) - .Where(IsValidComponent) - .Where(match); - - var list = new List(fromPlayer); - if (list.Count == 0) list.AddRange(fromEditor); - return list; - } - -#if UNITY_EDITOR - private static IEnumerable PreferPlayer(IEnumerable seq) - { - var player = new HashSet( - UnityEditor.Compilation.CompilationPipeline.GetAssemblies(UnityEditor.Compilation.AssembliesType.Player).Select(a => a.name), - StringComparer.Ordinal); - - return seq.OrderBy(t => player.Contains(t.Assembly.GetName().Name) ? 0 : 1); - } -#endif - - private static string Ambiguity(string query, IEnumerable cands) - { - var lines = cands.Select(t => $"{t.FullName} (assembly {t.Assembly.GetName().Name})"); - return $"Multiple component types matched '{query}':\n - " + string.Join("\n - ", lines) + - "\nProvide a fully qualified type name to disambiguate."; + return UnityTypeResolver.TryResolve(nameOrFullName, out type, out error, typeof(Component)); } /// @@ -2279,45 +1959,28 @@ namespace MCPForUnity.Editor.Tools } /// - /// Uses AI to suggest the most likely property matches for a user's input. + /// Suggests the most likely property matches for a user's input using fuzzy matching. + /// Uses Levenshtein distance, substring matching, and common naming pattern heuristics. /// - public static List GetAIPropertySuggestions(string userInput, List availableProperties) + public static List GetFuzzyPropertySuggestions(string userInput, List availableProperties) { if (string.IsNullOrWhiteSpace(userInput) || !availableProperties.Any()) return new List(); - // Simple caching to avoid repeated AI calls for the same input + // Simple caching to avoid repeated lookups for the same input var cacheKey = $"{userInput.ToLowerInvariant()}:{string.Join(",", availableProperties)}"; if (PropertySuggestionCache.TryGetValue(cacheKey, out var cached)) return cached; try { - var prompt = $"A Unity developer is trying to set a component property but used an incorrect name.\n\n" + - $"User requested: \"{userInput}\"\n" + - $"Available properties: [{string.Join(", ", availableProperties)}]\n\n" + - $"Find 1-3 most likely matches considering:\n" + - $"- Unity Inspector display names vs actual field names (e.g., \"Max Reach Distance\" → \"maxReachDistance\")\n" + - $"- camelCase vs PascalCase vs spaces\n" + - $"- Similar meaning/semantics\n" + - $"- Common Unity naming patterns\n\n" + - $"Return ONLY the matching property names, comma-separated, no quotes or explanation.\n" + - $"If confidence is low (<70%), return empty string.\n\n" + - $"Examples:\n" + - $"- \"Max Reach Distance\" → \"maxReachDistance\"\n" + - $"- \"Health Points\" → \"healthPoints, hp\"\n" + - $"- \"Move Speed\" → \"moveSpeed, movementSpeed\""; - - // For now, we'll use a simple rule-based approach that mimics AI behavior - // This can be replaced with actual AI calls later var suggestions = GetRuleBasedSuggestions(userInput, availableProperties); - PropertySuggestionCache[cacheKey] = suggestions; return suggestions; } catch (Exception ex) { - Debug.LogWarning($"[AI Property Matching] Error getting suggestions for '{userInput}': {ex.Message}"); + Debug.LogWarning($"[Property Matching] Error getting suggestions for '{userInput}': {ex.Message}"); return new List(); } } diff --git a/MCPForUnity/Editor/Tools/ManageMaterial.cs b/MCPForUnity/Editor/Tools/ManageMaterial.cs index 71913dc..0c61e4d 100644 --- a/MCPForUnity/Editor/Tools/ManageMaterial.cs +++ b/MCPForUnity/Editor/Tools/ManageMaterial.cs @@ -16,7 +16,7 @@ namespace MCPForUnity.Editor.Tools string action = @params["action"]?.ToString(); if (string.IsNullOrEmpty(action)) { - return new { status = "error", message = "Action is required" }; + return new ErrorResponse("Action is required"); } try @@ -24,7 +24,7 @@ namespace MCPForUnity.Editor.Tools switch (action) { case "ping": - return new { status = "success", tool = "manage_material" }; + return new SuccessResponse("pong", new { tool = "manage_material" }); case "create": return CreateMaterial(@params); @@ -45,12 +45,12 @@ namespace MCPForUnity.Editor.Tools return GetMaterialInfo(@params); default: - return new { status = "error", message = $"Unknown action: {action}" }; + return new ErrorResponse($"Unknown action: {action}"); } } catch (Exception ex) { - return new { status = "error", message = ex.Message, stackTrace = ex.StackTrace }; + return new ErrorResponse(ex.Message, new { stackTrace = ex.StackTrace }); } } @@ -78,16 +78,16 @@ namespace MCPForUnity.Editor.Tools if (string.IsNullOrEmpty(materialPath) || string.IsNullOrEmpty(property) || value == null) { - return new { status = "error", message = "materialPath, property, and value are required" }; + return new ErrorResponse("materialPath, property, and value are required"); } // Find material var findInstruction = new JObject { ["find"] = materialPath }; - Material mat = ManageGameObject.FindObjectByInstruction(findInstruction, typeof(Material)) as Material; + Material mat = ObjectResolver.Resolve(findInstruction, typeof(Material)) as Material; if (mat == null) { - return new { status = "error", message = $"Could not find material at path: {materialPath}" }; + return new ErrorResponse($"Could not find material at path: {materialPath}"); } Undo.RecordObject(mat, "Set Material Property"); @@ -101,27 +101,27 @@ namespace MCPForUnity.Editor.Tools // Check if it looks like an instruction if (value is JObject obj && (obj.ContainsKey("find") || obj.ContainsKey("method"))) { - Texture tex = ManageGameObject.FindObjectByInstruction(obj, typeof(Texture)) as Texture; + Texture tex = ObjectResolver.Resolve(obj, typeof(Texture)) as Texture; if (tex != null && mat.HasProperty(property)) { mat.SetTexture(property, tex); EditorUtility.SetDirty(mat); - return new { status = "success", message = $"Set texture property {property} on {mat.name}" }; + return new SuccessResponse($"Set texture property {property} on {mat.name}"); } } } // 2. Fallback to standard logic via MaterialOps (handles Colors, Floats, Strings->Path) - bool success = MaterialOps.TrySetShaderProperty(mat, property, value, ManageGameObject.InputSerializer); + bool success = MaterialOps.TrySetShaderProperty(mat, property, value, UnityJsonSerializer.Instance); if (success) { EditorUtility.SetDirty(mat); - return new { status = "success", message = $"Set property {property} on {mat.name}" }; + return new SuccessResponse($"Set property {property} on {mat.name}"); } else { - return new { status = "error", message = $"Failed to set property {property}. Value format might be unsupported or texture not found." }; + return new ErrorResponse($"Failed to set property {property}. Value format might be unsupported or texture not found."); } } @@ -133,25 +133,25 @@ namespace MCPForUnity.Editor.Tools if (string.IsNullOrEmpty(materialPath) || colorToken == null) { - return new { status = "error", message = "materialPath and color are required" }; + return new ErrorResponse("materialPath and color are required"); } var findInstruction = new JObject { ["find"] = materialPath }; - Material mat = ManageGameObject.FindObjectByInstruction(findInstruction, typeof(Material)) as Material; + Material mat = ObjectResolver.Resolve(findInstruction, typeof(Material)) as Material; if (mat == null) { - return new { status = "error", message = $"Could not find material at path: {materialPath}" }; + return new ErrorResponse($"Could not find material at path: {materialPath}"); } Color color; try { - color = MaterialOps.ParseColor(colorToken, ManageGameObject.InputSerializer); + color = MaterialOps.ParseColor(colorToken, UnityJsonSerializer.Instance); } catch (Exception e) { - return new { status = "error", message = $"Invalid color format: {e.Message}" }; + return new ErrorResponse($"Invalid color format: {e.Message}"); } Undo.RecordObject(mat, "Set Material Color"); @@ -185,11 +185,11 @@ namespace MCPForUnity.Editor.Tools if (foundProp) { EditorUtility.SetDirty(mat); - return new { status = "success", message = $"Set color on {property}" }; + return new SuccessResponse($"Set color on {property}"); } else { - return new { status = "error", message = "Could not find suitable color property (_BaseColor or _Color) or specified property does not exist." }; + return new ErrorResponse("Could not find suitable color property (_BaseColor or _Color) or specified property does not exist."); } } @@ -202,29 +202,29 @@ namespace MCPForUnity.Editor.Tools if (string.IsNullOrEmpty(target) || string.IsNullOrEmpty(materialPath)) { - return new { status = "error", message = "target and materialPath are required" }; + return new ErrorResponse("target and materialPath are required"); } var goInstruction = new JObject { ["find"] = target }; if (!string.IsNullOrEmpty(searchMethod)) goInstruction["method"] = searchMethod; - GameObject go = ManageGameObject.FindObjectByInstruction(goInstruction, typeof(GameObject)) as GameObject; + GameObject go = ObjectResolver.Resolve(goInstruction, typeof(GameObject)) as GameObject; if (go == null) { - return new { status = "error", message = $"Could not find target GameObject: {target}" }; + return new ErrorResponse($"Could not find target GameObject: {target}"); } Renderer renderer = go.GetComponent(); if (renderer == null) { - return new { status = "error", message = $"GameObject {go.name} has no Renderer component" }; + return new ErrorResponse($"GameObject {go.name} has no Renderer component"); } var matInstruction = new JObject { ["find"] = materialPath }; - Material mat = ManageGameObject.FindObjectByInstruction(matInstruction, typeof(Material)) as Material; + Material mat = ObjectResolver.Resolve(matInstruction, typeof(Material)) as Material; if (mat == null) { - return new { status = "error", message = $"Could not find material: {materialPath}" }; + return new ErrorResponse($"Could not find material: {materialPath}"); } Undo.RecordObject(renderer, "Assign Material"); @@ -232,14 +232,14 @@ namespace MCPForUnity.Editor.Tools Material[] sharedMats = renderer.sharedMaterials; if (slot < 0 || slot >= sharedMats.Length) { - return new { status = "error", message = $"Slot {slot} out of bounds (count: {sharedMats.Length})" }; + return new ErrorResponse($"Slot {slot} out of bounds (count: {sharedMats.Length})"); } sharedMats[slot] = mat; renderer.sharedMaterials = sharedMats; EditorUtility.SetDirty(renderer); - return new { status = "success", message = $"Assigned material {mat.name} to {go.name} slot {slot}" }; + return new SuccessResponse($"Assigned material {mat.name} to {go.name} slot {slot}"); } private static object SetRendererColor(JObject @params) @@ -252,39 +252,39 @@ namespace MCPForUnity.Editor.Tools if (string.IsNullOrEmpty(target) || colorToken == null) { - return new { status = "error", message = "target and color are required" }; + return new ErrorResponse("target and color are required"); } Color color; try { - color = MaterialOps.ParseColor(colorToken, ManageGameObject.InputSerializer); + color = MaterialOps.ParseColor(colorToken, UnityJsonSerializer.Instance); } catch (Exception e) { - return new { status = "error", message = $"Invalid color format: {e.Message}" }; + return new ErrorResponse($"Invalid color format: {e.Message}"); } var goInstruction = new JObject { ["find"] = target }; if (!string.IsNullOrEmpty(searchMethod)) goInstruction["method"] = searchMethod; - GameObject go = ManageGameObject.FindObjectByInstruction(goInstruction, typeof(GameObject)) as GameObject; + GameObject go = ObjectResolver.Resolve(goInstruction, typeof(GameObject)) as GameObject; if (go == null) { - return new { status = "error", message = $"Could not find target GameObject: {target}" }; + return new ErrorResponse($"Could not find target GameObject: {target}"); } Renderer renderer = go.GetComponent(); if (renderer == null) { - return new { status = "error", message = $"GameObject {go.name} has no Renderer component" }; + return new ErrorResponse($"GameObject {go.name} has no Renderer component"); } if (mode == "property_block") { if (slot < 0 || slot >= renderer.sharedMaterials.Length) { - return new { status = "error", message = $"Slot {slot} out of bounds (count: {renderer.sharedMaterials.Length})" }; + return new ErrorResponse($"Slot {slot} out of bounds (count: {renderer.sharedMaterials.Length})"); } MaterialPropertyBlock block = new MaterialPropertyBlock(); @@ -304,7 +304,7 @@ namespace MCPForUnity.Editor.Tools renderer.SetPropertyBlock(block, slot); EditorUtility.SetDirty(renderer); - return new { status = "success", message = $"Set renderer color (PropertyBlock) on slot {slot}" }; + return new SuccessResponse($"Set renderer color (PropertyBlock) on slot {slot}"); } else if (mode == "shared") { @@ -313,15 +313,15 @@ namespace MCPForUnity.Editor.Tools Material mat = renderer.sharedMaterials[slot]; if (mat == null) { - return new { status = "error", message = $"No material in slot {slot}" }; + return new ErrorResponse($"No material in slot {slot}"); } Undo.RecordObject(mat, "Set Material Color"); if (mat.HasProperty("_BaseColor")) mat.SetColor("_BaseColor", color); else mat.SetColor("_Color", color); EditorUtility.SetDirty(mat); - return new { status = "success", message = "Set shared material color" }; + return new SuccessResponse("Set shared material color"); } - return new { status = "error", message = "Invalid slot" }; + return new ErrorResponse("Invalid slot"); } else if (mode == "instance") { @@ -330,18 +330,18 @@ namespace MCPForUnity.Editor.Tools Material mat = renderer.materials[slot]; if (mat == null) { - return new { status = "error", message = $"No material in slot {slot}" }; + return new ErrorResponse($"No material in slot {slot}"); } // Note: Undo cannot fully revert material instantiation Undo.RecordObject(mat, "Set Instance Material Color"); if (mat.HasProperty("_BaseColor")) mat.SetColor("_BaseColor", color); else mat.SetColor("_Color", color); - return new { status = "success", message = "Set instance material color", warning = "Material instance created; Undo cannot fully revert instantiation." }; + return new SuccessResponse("Set instance material color", new { warning = "Material instance created; Undo cannot fully revert instantiation." }); } - return new { status = "error", message = "Invalid slot" }; + return new ErrorResponse("Invalid slot"); } - return new { status = "error", message = $"Unknown mode: {mode}" }; + return new ErrorResponse($"Unknown mode: {mode}"); } private static object GetMaterialInfo(JObject @params) @@ -349,15 +349,15 @@ namespace MCPForUnity.Editor.Tools string materialPath = NormalizePath(@params["materialPath"]?.ToString()); if (string.IsNullOrEmpty(materialPath)) { - return new { status = "error", message = "materialPath is required" }; + return new ErrorResponse("materialPath is required"); } var findInstruction = new JObject { ["find"] = materialPath }; - Material mat = ManageGameObject.FindObjectByInstruction(findInstruction, typeof(Material)) as Material; + Material mat = ObjectResolver.Resolve(findInstruction, typeof(Material)) as Material; if (mat == null) { - return new { status = "error", message = $"Could not find material at path: {materialPath}" }; + return new ErrorResponse($"Could not find material at path: {materialPath}"); } Shader shader = mat.shader; @@ -448,12 +448,11 @@ namespace MCPForUnity.Editor.Tools } #endif - return new { - status = "success", + return new SuccessResponse($"Retrieved material info for {mat.name}", new { material = mat.name, shader = shader.name, properties = properties - }; + }); } private static object CreateMaterial(JObject @params) @@ -470,7 +469,7 @@ namespace MCPForUnity.Editor.Tools if (propsToken.Type == JTokenType.String) { try { properties = JObject.Parse(propsToken.ToString()); } - catch (Exception ex) { return new { status = "error", message = $"Invalid JSON in properties: {ex.Message}" }; } + catch (Exception ex) { return new ErrorResponse($"Invalid JSON in properties: {ex.Message}"); } } else if (propsToken is JObject obj) { @@ -480,26 +479,26 @@ namespace MCPForUnity.Editor.Tools if (string.IsNullOrEmpty(materialPath)) { - return new { status = "error", message = "materialPath is required" }; + return new ErrorResponse("materialPath is required"); } - // Path normalization handled by helper above, explicit check removed - // but we ensure it's valid for CreateAsset + // Safety check: SanitizeAssetPath should guarantee Assets/ prefix + // This check catches edge cases where normalization might fail if (!materialPath.StartsWith("Assets/")) { - return new { status = "error", message = "Path must start with Assets/ (normalization failed)" }; + return new ErrorResponse($"Invalid path '{materialPath}'. Path must be within Assets/ folder."); } Shader shader = RenderPipelineUtility.ResolveShader(shaderName); if (shader == null) { - return new { status = "error", message = $"Could not find shader: {shaderName}" }; + return new ErrorResponse($"Could not find shader: {shaderName}"); } // Check for existing asset to avoid silent overwrite if (AssetDatabase.LoadAssetAtPath(materialPath) != null) { - return new { status = "error", message = $"Material already exists at {materialPath}" }; + return new ErrorResponse($"Material already exists at {materialPath}"); } Material material = null; @@ -534,11 +533,11 @@ namespace MCPForUnity.Editor.Tools Color color; try { - color = MaterialOps.ParseColor(colorToken, ManageGameObject.InputSerializer); + color = MaterialOps.ParseColor(colorToken, UnityJsonSerializer.Instance); } catch (Exception e) { - return new { status = "error", message = $"Invalid color format: {e.Message}" }; + return new ErrorResponse($"Invalid color format: {e.Message}"); } if (!string.IsNullOrEmpty(colorProperty)) @@ -549,11 +548,7 @@ namespace MCPForUnity.Editor.Tools } else { - return new - { - status = "error", - message = $"Specified color property '{colorProperty}' does not exist on this material." - }; + return new ErrorResponse($"Specified color property '{colorProperty}' does not exist on this material."); } } else if (material.HasProperty("_BaseColor")) @@ -566,11 +561,7 @@ namespace MCPForUnity.Editor.Tools } else { - return new - { - status = "error", - message = "Could not find suitable color property (_BaseColor or _Color) on this material's shader." - }; + return new ErrorResponse("Could not find suitable color property (_BaseColor or _Color) on this material's shader."); } } @@ -579,13 +570,13 @@ namespace MCPForUnity.Editor.Tools if (properties != null) { - MaterialOps.ApplyProperties(material, properties, ManageGameObject.InputSerializer); + MaterialOps.ApplyProperties(material, properties, UnityJsonSerializer.Instance); } EditorUtility.SetDirty(material); AssetDatabase.SaveAssets(); - return new { status = "success", message = $"Created material at {materialPath} with shader {shaderName}" }; + return new SuccessResponse($"Created material at {materialPath} with shader {shaderName}"); } finally { diff --git a/MCPForUnity/Editor/Tools/ManageScene.cs b/MCPForUnity/Editor/Tools/ManageScene.cs index 7bdf579..8bc5456 100644 --- a/MCPForUnity/Editor/Tools/ManageScene.cs +++ b/MCPForUnity/Editor/Tools/ManageScene.cs @@ -396,7 +396,8 @@ namespace MCPForUnity.Editor.Tools Camera cam = Camera.main; if (cam == null) { - var cams = UnityEngine.Object.FindObjectsByType(FindObjectsSortMode.None); + // Use FindObjectsOfType for Unity 2021 compatibility + var cams = UnityEngine.Object.FindObjectsOfType(); cam = cams.FirstOrDefault(); } diff --git a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs index 0ac467f..0de309c 100644 --- a/MCPForUnity/Editor/Tools/ManageScriptableObject.cs +++ b/MCPForUnity/Editor/Tools/ManageScriptableObject.cs @@ -887,45 +887,12 @@ namespace MCPForUnity.Editor.Tools return normalized == "create" || normalized == "createso"; } + /// + /// Resolves a type by name. Delegates to UnityTypeResolver.ResolveAny(). + /// 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; + return Helpers.UnityTypeResolver.ResolveAny(typeName); } } } diff --git a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs index 2f65c30..7f72e94 100644 --- a/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs +++ b/MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs @@ -547,24 +547,44 @@ namespace MCPForUnity.Editor.Windows.Components.Connection // Wait briefly for the HTTP server to become ready, then start the session. // This is called when THIS instance starts the server (not when detecting an external server). var bridgeService = MCPServiceLocator.Bridge; - const int maxAttempts = 10; - var delay = TimeSpan.FromSeconds(1); + // Windows/dev mode may take much longer due to uv package resolution, fresh downloads, antivirus scans, etc. + const int maxAttempts = 30; + // Use shorter delays initially, then longer delays to allow server startup + var shortDelay = TimeSpan.FromMilliseconds(500); + var longDelay = TimeSpan.FromSeconds(3); for (int attempt = 0; attempt < maxAttempts; attempt++) { - // Check if server is actually accepting connections - if (!MCPServiceLocator.Server.IsLocalHttpServerRunning()) - { - await Task.Delay(delay); - continue; - } + var delay = attempt < 6 ? shortDelay : longDelay; - bool started = await bridgeService.StartAsync(); - if (started) + // Check if server is actually accepting connections + bool serverDetected = MCPServiceLocator.Server.IsLocalHttpServerRunning(); + + if (serverDetected) { - await VerifyBridgeConnectionAsync(); - UpdateConnectionStatus(); - return; + // Server detected - try to connect + bool started = await bridgeService.StartAsync(); + if (started) + { + await VerifyBridgeConnectionAsync(); + UpdateConnectionStatus(); + return; + } + } + else if (attempt >= 20) + { + // After many attempts without detection, try connecting anyway as a last resort. + // This handles cases where process detection fails but the server is actually running. + // Only try once every 3 attempts to avoid spamming connection errors (at attempts 20, 23, 26, 29). + if ((attempt - 20) % 3 != 0) continue; + + bool started = await bridgeService.StartAsync(); + if (started) + { + await VerifyBridgeConnectionAsync(); + UpdateConnectionStatus(); + return; + } } if (attempt < maxAttempts - 1) diff --git a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs index 47c2652..3636dd3 100644 --- a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs +++ b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs @@ -160,6 +160,34 @@ namespace MCPForUnity.Runtime.Serialization } } + public class Vector4Converter : JsonConverter + { + public override void WriteJson(JsonWriter writer, Vector4 value, JsonSerializer serializer) + { + writer.WriteStartObject(); + writer.WritePropertyName("x"); + writer.WriteValue(value.x); + writer.WritePropertyName("y"); + writer.WriteValue(value.y); + writer.WritePropertyName("z"); + writer.WriteValue(value.z); + writer.WritePropertyName("w"); + writer.WriteValue(value.w); + writer.WriteEndObject(); + } + + public override Vector4 ReadJson(JsonReader reader, Type objectType, Vector4 existingValue, bool hasExistingValue, JsonSerializer serializer) + { + JObject jo = JObject.Load(reader); + return new Vector4( + (float)jo["x"], + (float)jo["y"], + (float)jo["z"], + (float)jo["w"] + ); + } + } + /// /// Safe converter for Matrix4x4 that only accesses raw matrix elements (m00-m33). /// Avoids computed properties (lossyScale, rotation, inverse) that call ValidTRS() diff --git a/README.md b/README.md index 131e78b..7454fbd 100644 --- a/README.md +++ b/README.md @@ -352,9 +352,9 @@ Replace `YOUR_USERNAME` and `AppSupport` path segments as needed for your platfo ### 💡 Performance Tip: Use `batch_execute` -When performing multiple operations, use the `batch_execute` tool instead of calling tools one-by-one. This dramatically reduces latency and token costs: +When performing multiple operations, use the `batch_execute` tool instead of calling tools one-by-one. This dramatically reduces latency and token costs (supports up to 25 commands per batch): -``` +```text ❌ Slow: Create 5 cubes → 5 separate manage_gameobject calls ✅ Fast: Create 5 cubes → 1 batch_execute call with 5 commands diff --git a/Server/src/services/tools/batch_execute.py b/Server/src/services/tools/batch_execute.py index 529f7fb..3ad7f88 100644 --- a/Server/src/services/tools/batch_execute.py +++ b/Server/src/services/tools/batch_execute.py @@ -20,7 +20,8 @@ MAX_COMMANDS_PER_BATCH = 25 "Executes multiple MCP commands in a single batch for dramatically better performance. " "STRONGLY RECOMMENDED when creating/modifying multiple objects, adding components to multiple targets, " "or performing any repetitive operations. Reduces latency and token costs by 10-100x compared to " - "sequential tool calls. Example: creating 5 cubes → use 1 batch_execute with 5 create commands instead of 5 separate calls." + "sequential tool calls. Supports up to 25 commands per batch. " + "Example: creating 5 cubes → use 1 batch_execute with 5 create commands instead of 5 separate calls." ), annotations=ToolAnnotations( title="Batch Execute", diff --git a/Server/src/services/tools/execute_menu_item.py b/Server/src/services/tools/execute_menu_item.py index 2c07496..4a2c07b 100644 --- a/Server/src/services/tools/execute_menu_item.py +++ b/Server/src/services/tools/execute_menu_item.py @@ -25,8 +25,6 @@ async def execute_menu_item( menu_path: Annotated[str, "Menu path for 'execute' or 'exists' (e.g., 'File/Save Project')"] | None = None, ) -> MCPResponse: - # Get active instance from session state - # Removed session_state import unity_instance = get_unity_instance_from_context(ctx) params_dict: dict[str, Any] = {"menuPath": menu_path} params_dict = {k: v for k, v in params_dict.items() if v is not None} diff --git a/Server/src/services/tools/find_gameobjects.py b/Server/src/services/tools/find_gameobjects.py index b894558..02b5db7 100644 --- a/Server/src/services/tools/find_gameobjects.py +++ b/Server/src/services/tools/find_gameobjects.py @@ -40,16 +40,17 @@ async def find_gameobjects( """ unity_instance = get_unity_instance_from_context(ctx) - gate = await preflight(ctx, wait_for_no_compile=True, refresh_if_dirty=True) - if gate is not None: - return gate.model_dump() - + # Validate required parameters before preflight I/O if not search_term: return { "success": False, "message": "Missing required parameter 'search_term'. Specify what to search for." } + gate = await preflight(ctx, wait_for_no_compile=True, refresh_if_dirty=True) + if gate is not None: + return gate.model_dump() + # Coerce parameters include_inactive = coerce_bool(include_inactive, default=False) page_size = coerce_int(page_size, default=50) diff --git a/Server/src/services/tools/manage_asset.py b/Server/src/services/tools/manage_asset.py index 896308a..e5328df 100644 --- a/Server/src/services/tools/manage_asset.py +++ b/Server/src/services/tools/manage_asset.py @@ -1,7 +1,6 @@ """ Defines the manage_asset tool for interacting with Unity assets. """ -import ast import asyncio import json from typing import Annotated, Any, Literal @@ -11,47 +10,12 @@ from mcp.types import ToolAnnotations from services.registry import mcp_for_unity_tool from services.tools import get_unity_instance_from_context -from services.tools.utils import parse_json_payload, coerce_int +from services.tools.utils import parse_json_payload, coerce_int, normalize_properties from transport.unity_transport import send_with_unity_instance from transport.legacy.unity_connection import async_send_command_with_retry from services.tools.preflight import preflight -def _normalize_properties(value: Any) -> tuple[dict[str, Any] | None, str | None]: - """ - Robustly normalize properties parameter to a dict. - Returns (parsed_dict, error_message). If error_message is set, parsed_dict is None. - """ - if value is None: - return {}, None - - # Already a dict - return as-is - if isinstance(value, dict): - return value, None - - # Try parsing as string - if isinstance(value, str): - # Check for obviously invalid values from serialization bugs - if value in ("[object Object]", "undefined", "null", ""): - return None, f"properties received invalid value: '{value}'. Expected a JSON object like {{\"key\": value}}" - - # Try JSON parsing first - parsed = parse_json_payload(value) - if isinstance(parsed, dict): - return parsed, None - - # Fallback to ast.literal_eval for Python dict literals - try: - parsed = ast.literal_eval(value) - if isinstance(parsed, dict): - return parsed, None - return None, f"properties must evaluate to a dict, got {type(parsed).__name__}" - except (ValueError, SyntaxError) as e: - return None, f"Failed to parse properties: {e}" - - return None, f"properties must be a dict or JSON string, got {type(value).__name__}" - - @mcp_for_unity_tool( description=( "Performs asset operations (import, create, modify, delete, etc.) in Unity.\n\n" @@ -96,7 +60,7 @@ async def manage_asset( return gate.model_dump() # --- Normalize properties using robust module-level helper --- - properties, parse_error = _normalize_properties(properties) + properties, parse_error = normalize_properties(properties) if parse_error: await ctx.error(f"manage_asset: {parse_error}") return {"success": False, "message": parse_error} diff --git a/Server/src/services/tools/manage_components.py b/Server/src/services/tools/manage_components.py index ff1fa27..1d5ddda 100644 --- a/Server/src/services/tools/manage_components.py +++ b/Server/src/services/tools/manage_components.py @@ -9,37 +9,10 @@ from services.registry import mcp_for_unity_tool from services.tools import get_unity_instance_from_context from transport.unity_transport import send_with_unity_instance from transport.legacy.unity_connection import async_send_command_with_retry -from services.tools.utils import parse_json_payload +from services.tools.utils import parse_json_payload, normalize_properties from services.tools.preflight import preflight -def _normalize_properties(value: Any) -> tuple[dict[str, Any] | None, str | None]: - """ - Robustly normalize properties parameter to a dict. - Returns (parsed_dict, error_message). If error_message is set, parsed_dict is None. - """ - if value is None: - return None, None - - # Already a dict - return as-is - if isinstance(value, dict): - return value, None - - # Try parsing as string - if isinstance(value, str): - # Check for obviously invalid values from serialization bugs - if value in ("[object Object]", "undefined", "null", ""): - return None, f"properties received invalid value: '{value}'. Expected a JSON object like {{\"propertyName\": value}}" - - parsed = parse_json_payload(value) - if isinstance(parsed, dict): - return parsed, None - - return None, f"properties must be a JSON object (dict), got string that parsed to {type(parsed).__name__}" - - return None, f"properties must be a dict or JSON string, got {type(value).__name__}" - - @mcp_for_unity_tool( description="Manages components on GameObjects (add, remove, set_property). For reading component data, use the unity://scene/gameobject/{id}/components resource." ) @@ -109,7 +82,7 @@ async def manage_components( } # --- Normalize properties with detailed error handling --- - properties, props_error = _normalize_properties(properties) + properties, props_error = normalize_properties(properties) if props_error: return {"success": False, "message": props_error} diff --git a/Server/src/services/tools/manage_editor.py b/Server/src/services/tools/manage_editor.py index bcb6fde..51480b7 100644 --- a/Server/src/services/tools/manage_editor.py +++ b/Server/src/services/tools/manage_editor.py @@ -46,13 +46,9 @@ async def manage_editor( params = { "action": action, "waitForCompletion": wait_for_completion, - "toolName": tool_name, # Corrected parameter name to match C# - "tagName": tag_name, # Pass tag name - "layerName": layer_name, # Pass layer name - # Add other parameters based on the action being performed - # "width": width, - # "height": height, - # etc. + "toolName": tool_name, + "tagName": tag_name, + "layerName": layer_name, } params = {k: v for k, v in params.items() if v is not None} diff --git a/Server/src/services/tools/manage_gameobject.py b/Server/src/services/tools/manage_gameobject.py index 218a566..eb44358 100644 --- a/Server/src/services/tools/manage_gameobject.py +++ b/Server/src/services/tools/manage_gameobject.py @@ -83,7 +83,7 @@ def _normalize_component_properties(value: Any) -> tuple[dict[str, dict[str, Any @mcp_for_unity_tool( - description="Performs CRUD operations on GameObjects and components. Read-only actions: find, get_components, get_component. Modifying actions: create, modify, delete, add_component, remove_component, set_component_property, duplicate, move_relative.", + description="Performs CRUD operations on GameObjects. Actions: create, modify, delete, duplicate, move_relative. For finding GameObjects use find_gameobjects tool. For component operations use manage_components tool.", annotations=ToolAnnotations( title="Manage GameObject", destructiveHint=True, @@ -91,7 +91,7 @@ def _normalize_component_properties(value: Any) -> tuple[dict[str, dict[str, Any ) async def manage_gameobject( ctx: Context, - action: Annotated[Literal["create", "modify", "delete", "find", "add_component", "remove_component", "set_component_property", "get_components", "get_component", "duplicate", "move_relative"], "Perform CRUD operations on GameObjects and components."] | None = None, + action: Annotated[Literal["create", "modify", "delete", "duplicate", "move_relative"], "Action to perform on GameObject."] | None = None, target: Annotated[str, "GameObject identifier by name or path for modify/delete/component actions"] | None = None, search_method: Annotated[Literal["by_id", "by_name", "by_path", "by_tag", "by_layer", "by_component"], @@ -175,7 +175,7 @@ async def manage_gameobject( if action is None: return { "success": False, - "message": "Missing required parameter 'action'. Valid actions: create, modify, delete, find, add_component, remove_component, set_component_property, get_components, get_component, duplicate, move_relative" + "message": "Missing required parameter 'action'. Valid actions: create, modify, delete, duplicate, move_relative. For finding GameObjects use find_gameobjects tool. For component operations use manage_components tool." } # --- Normalize vector parameters using robust helper --- @@ -205,23 +205,7 @@ async def manage_gameobject( return {"success": False, "message": comp_props_error} try: - # Map tag to search_term when search_method is by_tag for backward compatibility - if action == "find" and search_method == "by_tag" and tag is not None and search_term is None: - search_term = tag - # Validate parameter usage to prevent silent failures - if action == "find": - if name is not None: - return { - "success": False, - "message": "For 'find' action, use 'search_term' parameter, not 'name'. Remove 'name' parameter. Example: search_term='Player', search_method='by_name'" - } - if search_term is None: - return { - "success": False, - "message": "For 'find' action, 'search_term' parameter is required. Use search_term (not 'name') to specify what to find." - } - if action in ["create", "modify"]: if search_term is not None: return { diff --git a/Server/src/services/tools/manage_material.py b/Server/src/services/tools/manage_material.py index 0022a41..f131238 100644 --- a/Server/src/services/tools/manage_material.py +++ b/Server/src/services/tools/manage_material.py @@ -9,7 +9,7 @@ from mcp.types import ToolAnnotations from services.registry import mcp_for_unity_tool from services.tools import get_unity_instance_from_context -from services.tools.utils import parse_json_payload, coerce_int +from services.tools.utils import parse_json_payload, coerce_int, normalize_properties from transport.unity_transport import send_with_unity_instance from transport.legacy.unity_connection import async_send_command_with_retry @@ -47,28 +47,6 @@ def _normalize_color(value: Any) -> tuple[list[float] | None, str | None]: return None, f"color must be a list or JSON string, got {type(value).__name__}" -def _normalize_properties(value: Any) -> tuple[dict[str, Any] | None, str | None]: - """ - Normalize properties parameter to a dict. - """ - if value is None: - return None, None - - if isinstance(value, dict): - return value, None - - if isinstance(value, str): - if value in ("[object Object]", "undefined", "null", ""): - return None, f"properties received invalid value: '{value}'. Expected a JSON object" - - parsed = parse_json_payload(value) - if isinstance(parsed, dict): - return parsed, None - return None, f"properties must parse to a dict, got {type(parsed).__name__}" - - return None, f"properties must be a dict or JSON string, got {type(value).__name__}" - - @mcp_for_unity_tool( description="Manages Unity materials (set properties, colors, shaders, etc). Read-only actions: ping, get_material_info. Modifying actions: create, set_material_shader_property, set_material_color, assign_material_to_renderer, set_renderer_color.", annotations=ToolAnnotations( @@ -117,7 +95,7 @@ async def manage_material( return {"success": False, "message": color_error} # --- Normalize properties with validation --- - properties, props_error = _normalize_properties(properties) + properties, props_error = normalize_properties(properties) if props_error: return {"success": False, "message": props_error} diff --git a/Server/src/services/tools/manage_script.py b/Server/src/services/tools/manage_script.py index ac59c66..b8b2c1a 100644 --- a/Server/src/services/tools/manage_script.py +++ b/Server/src/services/tools/manage_script.py @@ -513,7 +513,7 @@ async def manage_script( name: Annotated[str, "Script name (no .cs extension)", "Name of the script to create"], path: Annotated[str, "Asset path (default: 'Assets/')", "Path under Assets/ to create the script at, e.g., 'Assets/Scripts/My.cs'"], contents: Annotated[str, "Contents of the script to create", - "C# code for 'create'/'update'"] | None = None, + "C# code for 'create' action"] | None = None, script_type: Annotated[str, "Script type (e.g., 'C#')", "Type hint (e.g., 'MonoBehaviour')"] | None = None, namespace: Annotated[str, "Namespace for the script"] | None = None, diff --git a/Server/src/services/tools/manage_shader.py b/Server/src/services/tools/manage_shader.py index 9562ef0..1865a70 100644 --- a/Server/src/services/tools/manage_shader.py +++ b/Server/src/services/tools/manage_shader.py @@ -14,7 +14,7 @@ from transport.legacy.unity_connection import async_send_command_with_retry description="Manages shader scripts in Unity (create, read, update, delete). Read-only action: read. Modifying actions: create, update, delete.", annotations=ToolAnnotations( title="Manage Shader", - destructiveHint=True, + destructiveHint=True, # Note: 'read' action is non-destructive; 'create', 'update', 'delete' are destructive ), ) async def manage_shader( diff --git a/Server/src/services/tools/utils.py b/Server/src/services/tools/utils.py index 8deaf38..32f7a23 100644 --- a/Server/src/services/tools/utils.py +++ b/Server/src/services/tools/utils.py @@ -75,3 +75,38 @@ def coerce_int(value: Any, default: int | None = None) -> int | None: return int(float(s)) except Exception: return default + + +def normalize_properties(value: Any) -> tuple[dict[str, Any] | None, str | None]: + """ + Robustly normalize a properties parameter to a dict. + + Handles various input formats from MCP clients/LLMs: + - None -> (None, None) + - dict -> (dict, None) + - JSON string -> (parsed_dict, None) or (None, error_message) + - Invalid values -> (None, error_message) + + Returns: + Tuple of (parsed_dict, error_message). If error_message is set, parsed_dict is None. + """ + if value is None: + return None, None + + # Already a dict - return as-is + if isinstance(value, dict): + return value, None + + # Try parsing as string + if isinstance(value, str): + # Check for obviously invalid values from serialization bugs + if value in ("[object Object]", "undefined", "null", ""): + return None, f"properties received invalid value: '{value}'. Expected a JSON object like {{\"key\": value}}" + + parsed = parse_json_payload(value) + if isinstance(parsed, dict): + return parsed, None + + return None, f"properties must be a JSON object (dict), got string that parsed to {type(parsed).__name__}" + + return None, f"properties must be a dict or JSON string, got {type(value).__name__}" diff --git a/Server/tests/integration/test_manage_asset_json_parsing.py b/Server/tests/integration/test_manage_asset_json_parsing.py index 852f3e9..b116b47 100644 --- a/Server/tests/integration/test_manage_asset_json_parsing.py +++ b/Server/tests/integration/test_manage_asset_json_parsing.py @@ -57,7 +57,7 @@ class TestManageAssetJsonParsing: # Verify behavior: parsing fails with a clear error assert result.get("success") is False - assert "Failed to parse properties" in result.get("message", "") + assert "properties must be" in result.get("message", "") @pytest.mark.asyncio async def test_properties_dict_unchanged(self, monkeypatch): diff --git a/Server/tests/integration/test_manage_gameobject_param_coercion.py b/Server/tests/integration/test_manage_gameobject_param_coercion.py index 60e381c..c5ca13e 100644 --- a/Server/tests/integration/test_manage_gameobject_param_coercion.py +++ b/Server/tests/integration/test_manage_gameobject_param_coercion.py @@ -5,7 +5,8 @@ import services.tools.manage_gameobject as manage_go_mod @pytest.mark.asyncio -async def test_manage_gameobject_boolean_and_tag_mapping(monkeypatch): +async def test_manage_gameobject_boolean_coercion(monkeypatch): + """Test that string boolean values are properly coerced for valid actions.""" captured = {} async def fake_send(cmd, params, **kwargs): @@ -18,26 +19,24 @@ async def test_manage_gameobject_boolean_and_tag_mapping(monkeypatch): fake_send, ) - # find by tag: allow tag to map to searchTerm + # Test boolean coercion with "modify" action (valid action) resp = await manage_go_mod.manage_gameobject( ctx=DummyContext(), - action="find", - search_method="by_tag", - tag="Player", - find_all="true", - search_inactive="0", + action="modify", + target="Player", + set_active="true", # String should be coerced to bool ) - # Loosen equality: wrapper may include a diagnostic message + assert resp.get("success") is True - assert "data" in resp - # ensure tag mapped to searchTerm and booleans passed through; C# side coerces true/false already - assert captured["params"]["searchTerm"] == "Player" - assert captured["params"]["findAll"] is True - assert captured["params"]["searchInactive"] is False + assert captured["params"]["action"] == "modify" + assert captured["params"]["target"] == "Player" + # setActive string "true" is coerced to bool True + assert captured["params"]["setActive"] is True @pytest.mark.asyncio -async def test_manage_gameobject_get_components_paging_params_pass_through(monkeypatch): +async def test_manage_gameobject_create_with_tag(monkeypatch): + """Test that create action properly passes tag parameter.""" captured = {} async def fake_send(cmd, params, **kwargs): @@ -52,21 +51,15 @@ async def test_manage_gameobject_get_components_paging_params_pass_through(monke resp = await manage_go_mod.manage_gameobject( ctx=DummyContext(), - action="get_components", - target="Player", - search_method="by_name", - page_size="25", - cursor="50", - max_components="100", - include_properties="true", + action="create", + name="TestObject", + tag="Player", + position=[1.0, 2.0, 3.0], ) assert resp.get("success") is True p = captured["params"] - assert p["action"] == "get_components" - assert p["target"] == "Player" - assert p["searchMethod"] == "by_name" - assert p["pageSize"] == 25 - assert p["cursor"] == 50 - assert p["maxComponents"] == 100 - assert p["includeProperties"] is True + assert p["action"] == "create" + assert p["name"] == "TestObject" + assert p["tag"] == "Player" + assert p["position"] == [1.0, 2.0, 3.0] diff --git a/TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs b/TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs index 413fc7e..b655786 100644 --- a/TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs +++ b/TestProjects/UnityMCPTests/Assets/Scripts/LongUnityScriptClaudeTest.cs @@ -21,9 +21,9 @@ public class LongUnityScriptClaudeTest : MonoBehaviour private int padAccumulator = 0; private Vector3 padVector = Vector3.zero; - // Animation blend hashes - private static readonly int BlendXHash = Animator.StringToHash("BlendX"); - private static readonly int BlendYHash = Animator.StringToHash("BlendY"); + // Animation blend hashes (match animator parameter names) + private static readonly int BlendXHash = Animator.StringToHash("reachX"); + private static readonly int BlendYHash = Animator.StringToHash("reachY"); [Header("Tuning")] diff --git a/TestProjects/UnityMCPTests/Assets/Scripts/TestScriptableObjectInstance.asset.meta b/TestProjects/UnityMCPTests/Assets/Scripts/TestScriptableObjectInstance.asset.meta new file mode 100644 index 0000000..c74c318 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Scripts/TestScriptableObjectInstance.asset.meta @@ -0,0 +1,8 @@ +fileFormatVersion: 2 +guid: 93766a50487224f02b29aae42971e08b +NativeFormatImporter: + externalObjects: {} + mainObjectFileID: 0 + 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/Helpers/PaginationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/PaginationTests.cs new file mode 100644 index 0000000..b15b9ed --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/PaginationTests.cs @@ -0,0 +1,209 @@ +using System.Collections.Generic; +using NUnit.Framework; +using Newtonsoft.Json.Linq; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnityTests.Editor.Helpers +{ + /// + /// Tests for the standard Pagination classes. + /// + public class PaginationTests + { + #region PaginationRequest Tests + + [Test] + public void PaginationRequest_FromParams_ParsesPageSizeSnakeCase() + { + var p = new JObject { ["page_size"] = 25 }; + var req = PaginationRequest.FromParams(p); + Assert.AreEqual(25, req.PageSize); + } + + [Test] + public void PaginationRequest_FromParams_ParsesPageSizeCamelCase() + { + var p = new JObject { ["pageSize"] = 30 }; + var req = PaginationRequest.FromParams(p); + Assert.AreEqual(30, req.PageSize); + } + + [Test] + public void PaginationRequest_FromParams_ParsesCursor() + { + var p = new JObject { ["cursor"] = 50 }; + var req = PaginationRequest.FromParams(p); + Assert.AreEqual(50, req.Cursor); + } + + [Test] + public void PaginationRequest_FromParams_ConvertsPageNumberToCursor() + { + // page_number is 1-based, should convert to 0-based cursor + var p = new JObject { ["page_number"] = 3, ["page_size"] = 10 }; + var req = PaginationRequest.FromParams(p); + // Page 3 with page size 10 means items 20-29, so cursor should be 20 + Assert.AreEqual(20, req.Cursor); + } + + [Test] + public void PaginationRequest_FromParams_CursorTakesPrecedenceOverPageNumber() + { + // If both cursor and page_number are specified, cursor should win + var p = new JObject { ["cursor"] = 100, ["page_number"] = 1 }; + var req = PaginationRequest.FromParams(p); + Assert.AreEqual(100, req.Cursor); + } + + [Test] + public void PaginationRequest_FromParams_UsesDefaultsForNullParams() + { + var req = PaginationRequest.FromParams(null); + Assert.AreEqual(50, req.PageSize); + Assert.AreEqual(0, req.Cursor); + } + + [Test] + public void PaginationRequest_FromParams_UsesDefaultsForEmptyParams() + { + var req = PaginationRequest.FromParams(new JObject()); + Assert.AreEqual(50, req.PageSize); + Assert.AreEqual(0, req.Cursor); + } + + [Test] + public void PaginationRequest_FromParams_AcceptsCustomDefaultPageSize() + { + var req = PaginationRequest.FromParams(new JObject(), defaultPageSize: 100); + Assert.AreEqual(100, req.PageSize); + } + + [Test] + public void PaginationRequest_FromParams_HandleStringValues() + { + // Some clients might send string values + var p = new JObject { ["page_size"] = "15", ["cursor"] = "5" }; + var req = PaginationRequest.FromParams(p); + Assert.AreEqual(15, req.PageSize); + Assert.AreEqual(5, req.Cursor); + } + + #endregion + + #region PaginationResponse Tests + + [Test] + public void PaginationResponse_Create_ReturnsCorrectPageOfItems() + { + var allItems = new List { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; + var request = new PaginationRequest { PageSize = 3, Cursor = 0 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(3, response.Items.Count); + Assert.AreEqual(new List { 1, 2, 3 }, response.Items); + } + + [Test] + public void PaginationResponse_Create_ReturnsCorrectMiddlePage() + { + var allItems = new List { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; + var request = new PaginationRequest { PageSize = 3, Cursor = 3 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(3, response.Items.Count); + Assert.AreEqual(new List { 4, 5, 6 }, response.Items); + } + + [Test] + public void PaginationResponse_Create_HandlesLastPage() + { + var allItems = new List { 1, 2, 3, 4, 5 }; + var request = new PaginationRequest { PageSize = 3, Cursor = 3 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(2, response.Items.Count); + Assert.AreEqual(new List { 4, 5 }, response.Items); + Assert.IsNull(response.NextCursor); + Assert.IsFalse(response.HasMore); + } + + [Test] + public void PaginationResponse_HasMore_TrueWhenNextCursorSet() + { + var allItems = new List { 1, 2, 3, 4, 5, 6 }; + var request = new PaginationRequest { PageSize = 3, Cursor = 0 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.IsTrue(response.HasMore); + Assert.AreEqual(3, response.NextCursor); + } + + [Test] + public void PaginationResponse_HasMore_FalseWhenNoMoreItems() + { + var allItems = new List { 1, 2, 3 }; + var request = new PaginationRequest { PageSize = 10, Cursor = 0 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.IsFalse(response.HasMore); + Assert.IsNull(response.NextCursor); + } + + [Test] + public void PaginationResponse_Create_SetsCorrectTotalCount() + { + var allItems = new List { "a", "b", "c", "d", "e" }; + var request = new PaginationRequest { PageSize = 2, Cursor = 0 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(5, response.TotalCount); + } + + [Test] + public void PaginationResponse_Create_HandlesEmptyList() + { + var allItems = new List(); + var request = new PaginationRequest { PageSize = 10, Cursor = 0 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(0, response.Items.Count); + Assert.AreEqual(0, response.TotalCount); + Assert.IsNull(response.NextCursor); + Assert.IsFalse(response.HasMore); + } + + [Test] + public void PaginationResponse_Create_ClampsCursorToValidRange() + { + var allItems = new List { 1, 2, 3 }; + var request = new PaginationRequest { PageSize = 10, Cursor = 100 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(0, response.Items.Count); + Assert.AreEqual(3, response.Cursor); // Clamped to totalCount + } + + [Test] + public void PaginationResponse_Create_HandlesNegativeCursor() + { + var allItems = new List { 1, 2, 3 }; + var request = new PaginationRequest { PageSize = 10, Cursor = -5 }; + + var response = PaginationResponse.Create(allItems, request); + + Assert.AreEqual(0, response.Cursor); // Clamped to 0 + Assert.AreEqual(3, response.Items.Count); + } + + #endregion + } +} + diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/PaginationTests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/PaginationTests.cs.meta new file mode 100644 index 0000000..bc43803 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/PaginationTests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: a6d0177f4432b41c6bf7e0013cd5a2f2 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AIPropertyMatchingTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AIPropertyMatchingTests.cs index 4512d91..4a13e33 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AIPropertyMatchingTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/AIPropertyMatchingTests.cs @@ -47,42 +47,42 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void GetAIPropertySuggestions_ReturnsEmpty_ForNullInput() + public void GetFuzzyPropertySuggestions_ReturnsEmpty_ForNullInput() { - var suggestions = ComponentResolver.GetAIPropertySuggestions(null, sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions(null, sampleProperties); Assert.IsEmpty(suggestions, "Null input should return no suggestions"); } [Test] - public void GetAIPropertySuggestions_ReturnsEmpty_ForEmptyInput() + public void GetFuzzyPropertySuggestions_ReturnsEmpty_ForEmptyInput() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("", sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("", sampleProperties); Assert.IsEmpty(suggestions, "Empty input should return no suggestions"); } [Test] - public void GetAIPropertySuggestions_ReturnsEmpty_ForEmptyPropertyList() + public void GetFuzzyPropertySuggestions_ReturnsEmpty_ForEmptyPropertyList() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("test", new List()); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("test", new List()); Assert.IsEmpty(suggestions, "Empty property list should return no suggestions"); } [Test] - public void GetAIPropertySuggestions_FindsExactMatch_AfterCleaning() + public void GetFuzzyPropertySuggestions_FindsExactMatch_AfterCleaning() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("Max Reach Distance", sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("Max Reach Distance", sampleProperties); Assert.Contains("maxReachDistance", suggestions, "Should find exact match after cleaning spaces"); Assert.GreaterOrEqual(suggestions.Count, 1, "Should return at least one match for exact match"); } [Test] - public void GetAIPropertySuggestions_FindsMultipleWordMatches() + public void GetFuzzyPropertySuggestions_FindsMultipleWordMatches() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("max distance", sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("max distance", sampleProperties); Assert.Contains("maxReachDistance", suggestions, "Should match maxReachDistance"); Assert.Contains("maxHorizontalDistance", suggestions, "Should match maxHorizontalDistance"); @@ -90,54 +90,54 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void GetAIPropertySuggestions_FindsSimilarStrings_WithTypos() + public void GetFuzzyPropertySuggestions_FindsSimilarStrings_WithTypos() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("movespeed", sampleProperties); // missing capital S + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("movespeed", sampleProperties); // missing capital S Assert.Contains("moveSpeed", suggestions, "Should find moveSpeed despite missing capital"); } [Test] - public void GetAIPropertySuggestions_FindsSemanticMatches_ForCommonTerms() + public void GetFuzzyPropertySuggestions_FindsSemanticMatches_ForCommonTerms() { - var suggestions = ComponentResolver.GetAIPropertySuggestions("weight", sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("weight", sampleProperties); // Note: Current algorithm might not find "mass" but should handle it gracefully Assert.IsNotNull(suggestions, "Should return valid suggestions list"); } [Test] - public void GetAIPropertySuggestions_LimitsResults_ToReasonableNumber() + public void GetFuzzyPropertySuggestions_LimitsResults_ToReasonableNumber() { // Test with input that might match many properties - var suggestions = ComponentResolver.GetAIPropertySuggestions("m", sampleProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("m", sampleProperties); Assert.LessOrEqual(suggestions.Count, 3, "Should limit suggestions to 3 or fewer"); } [Test] - public void GetAIPropertySuggestions_CachesResults() + public void GetFuzzyPropertySuggestions_CachesResults() { var input = "Max Reach Distance"; // First call - var suggestions1 = ComponentResolver.GetAIPropertySuggestions(input, sampleProperties); + var suggestions1 = ComponentResolver.GetFuzzyPropertySuggestions(input, sampleProperties); // Second call should use cache (tested indirectly by ensuring consistency) - var suggestions2 = ComponentResolver.GetAIPropertySuggestions(input, sampleProperties); + var suggestions2 = ComponentResolver.GetFuzzyPropertySuggestions(input, sampleProperties); Assert.AreEqual(suggestions1.Count, suggestions2.Count, "Cached results should be consistent"); CollectionAssert.AreEqual(suggestions1, suggestions2, "Cached results should be identical"); } [Test] - public void GetAIPropertySuggestions_HandlesUnityNamingConventions() + public void GetFuzzyPropertySuggestions_HandlesUnityNamingConventions() { var unityStyleProperties = new List { "isKinematic", "useGravity", "maxLinearVelocity" }; - var suggestions1 = ComponentResolver.GetAIPropertySuggestions("is kinematic", unityStyleProperties); - var suggestions2 = ComponentResolver.GetAIPropertySuggestions("use gravity", unityStyleProperties); - var suggestions3 = ComponentResolver.GetAIPropertySuggestions("max linear velocity", unityStyleProperties); + var suggestions1 = ComponentResolver.GetFuzzyPropertySuggestions("is kinematic", unityStyleProperties); + var suggestions2 = ComponentResolver.GetFuzzyPropertySuggestions("use gravity", unityStyleProperties); + var suggestions3 = ComponentResolver.GetFuzzyPropertySuggestions("max linear velocity", unityStyleProperties); Assert.Contains("isKinematic", suggestions1, "Should handle 'is' prefix convention"); Assert.Contains("useGravity", suggestions2, "Should handle 'use' prefix convention"); @@ -145,10 +145,10 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void GetAIPropertySuggestions_PrioritizesExactMatches() + public void GetFuzzyPropertySuggestions_PrioritizesExactMatches() { var properties = new List { "speed", "moveSpeed", "maxSpeed", "speedMultiplier" }; - var suggestions = ComponentResolver.GetAIPropertySuggestions("speed", properties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("speed", properties); Assert.IsNotEmpty(suggestions, "Should find suggestions"); Assert.Contains("speed", suggestions, "Exact match should be included in results"); @@ -156,10 +156,10 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void GetAIPropertySuggestions_HandlesCaseInsensitive() + public void GetFuzzyPropertySuggestions_HandlesCaseInsensitive() { - var suggestions1 = ComponentResolver.GetAIPropertySuggestions("MAXREACHDISTANCE", sampleProperties); - var suggestions2 = ComponentResolver.GetAIPropertySuggestions("maxreachdistance", sampleProperties); + var suggestions1 = ComponentResolver.GetFuzzyPropertySuggestions("MAXREACHDISTANCE", sampleProperties); + var suggestions2 = ComponentResolver.GetFuzzyPropertySuggestions("maxreachdistance", sampleProperties); Assert.Contains("maxReachDistance", suggestions1, "Should handle uppercase input"); Assert.Contains("maxReachDistance", suggestions2, "Should handle lowercase input"); diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs index 3fdc283..ade0801 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/GameObjectAPIStressTests.cs @@ -54,7 +54,16 @@ namespace MCPForUnityTests.Editor.Tools private static JObject ToJObject(object result) { if (result == null) return new JObject(); - return result as JObject ?? JObject.FromObject(result); + if (result is JObject jobj) return jobj; + try + { + return JObject.FromObject(result); + } + catch (Exception ex) + { + Debug.LogWarning($"[ToJObject] Failed to convert result: {ex.Message}"); + return new JObject { ["error"] = ex.Message }; + } } #region Bulk GameObject Creation @@ -85,7 +94,8 @@ namespace MCPForUnityTests.Editor.Tools sw.Stop(); Debug.Log($"[BulkCreate] Created {SMALL_BATCH} objects in {sw.ElapsedMilliseconds}ms"); - Assert.Less(sw.ElapsedMilliseconds, 5000, "Bulk create took too long"); + // Use generous threshold for CI variability + Assert.Less(sw.ElapsedMilliseconds, 10000, "Bulk create took too long (CI threshold)"); } [Test] diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs index fe24064..b396f18 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectCreateTests.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using NUnit.Framework; using UnityEngine; +using UnityEditorInternal; using Newtonsoft.Json.Linq; using MCPForUnity.Editor.Tools; @@ -401,25 +402,33 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void Create_WithInvalidTag_HandlesGracefully() + public void Create_WithNewTag_AutoCreatesTag() { - // Expect the error log from Unity about invalid tag - UnityEngine.TestTools.LogAssert.Expect(LogType.Error, - new System.Text.RegularExpressions.Regex("Tag:.*NonExistentTag12345.*not defined")); + const string testTag = "AutoCreatedTag12345"; + // Tags that don't exist are now auto-created var p = new JObject { ["action"] = "create", - ["name"] = "TestInvalidTag", - ["tag"] = "NonExistentTag12345" + ["name"] = "TestAutoTag", + ["tag"] = testTag }; var result = ManageGameObject.HandleCommand(p); - // Current behavior: logs error but may create object anyway - Assert.IsNotNull(result, "Should return a result"); + var resultObj = result as JObject ?? JObject.FromObject(result); - // Clean up if object was created - FindAndTrack("TestInvalidTag"); + Assert.IsTrue(resultObj.Value("success"), resultObj.ToString()); + + var created = FindAndTrack("TestAutoTag"); + Assert.IsNotNull(created, "Object should be created"); + Assert.AreEqual(testTag, created.tag, "Tag should be auto-created and assigned"); + + // Verify tag was actually added to the tag manager + Assert.That(UnityEditorInternal.InternalEditorUtility.tags, Does.Contain(testTag), + "Tag should exist in Unity's tag manager"); + + // Clean up the created tag + try { UnityEditorInternal.InternalEditorUtility.RemoveTag(testTag); } catch { } } #endregion diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectModifyTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectModifyTests.cs index c0ebb6d..d671b6b 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectModifyTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectModifyTests.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using NUnit.Framework; using UnityEngine; +using UnityEditorInternal; using Newtonsoft.Json.Linq; using MCPForUnity.Editor.Tools; @@ -392,22 +393,30 @@ namespace MCPForUnityTests.Editor.Tools } [Test] - public void Modify_InvalidTag_HandlesGracefully() + public void Modify_NewTag_AutoCreatesTag() { - // Expect the error log from Unity about invalid tag - UnityEngine.TestTools.LogAssert.Expect(LogType.Error, - new System.Text.RegularExpressions.Regex("Tag:.*NonExistentTag12345.*not defined")); + const string testTag = "AutoModifyTag12345"; + // Tags that don't exist are now auto-created var p = new JObject { ["action"] = "modify", ["target"] = "ModifyTestObject", - ["tag"] = "NonExistentTag12345" + ["tag"] = testTag }; var result = ManageGameObject.HandleCommand(p); - // Current behavior: logs error but continues - Assert.IsNotNull(result, "Should return a result"); + var resultObj = result as JObject ?? JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), resultObj.ToString()); + Assert.AreEqual(testTag, testObjects[0].tag, "Tag should be auto-created and assigned"); + + // Verify tag was actually added to the tag manager + Assert.That(UnityEditorInternal.InternalEditorUtility.tags, Does.Contain(testTag), + "Tag should exist in Unity's tag manager"); + + // Clean up the created tag + try { UnityEditorInternal.InternalEditorUtility.RemoveTag(testTag); } catch { } } #endregion diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs index df70a0a..2d8b184 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs @@ -122,7 +122,7 @@ namespace MCPForUnityTests.Editor.Tools Assert.Contains("useGravity", properties, "Rigidbody should have useGravity property"); // Test AI suggestions - var suggestions = ComponentResolver.GetAIPropertySuggestions("Use Gravity", properties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("Use Gravity", properties); Assert.Contains("useGravity", suggestions, "Should suggest useGravity for 'Use Gravity'"); } @@ -153,7 +153,7 @@ namespace MCPForUnityTests.Editor.Tools foreach (var (input, expected) in testCases) { - var suggestions = ComponentResolver.GetAIPropertySuggestions(input, testProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions(input, testProperties); Assert.Contains(expected, suggestions, $"Should suggest {expected} for input '{input}'"); } } @@ -163,13 +163,13 @@ namespace MCPForUnityTests.Editor.Tools { // This test verifies that error messages are helpful and contain suggestions var testProperties = new List { "mass", "velocity", "drag", "useGravity" }; - var suggestions = ComponentResolver.GetAIPropertySuggestions("weight", testProperties); + var suggestions = ComponentResolver.GetFuzzyPropertySuggestions("weight", testProperties); // Even if no perfect match, should return valid list Assert.IsNotNull(suggestions, "Should return valid suggestions list"); // Test with completely invalid input - var badSuggestions = ComponentResolver.GetAIPropertySuggestions("xyz123invalid", testProperties); + var badSuggestions = ComponentResolver.GetFuzzyPropertySuggestions("xyz123invalid", testProperties); Assert.IsNotNull(badSuggestions, "Should handle invalid input gracefully"); } @@ -181,12 +181,12 @@ namespace MCPForUnityTests.Editor.Tools // First call - populate cache var startTime = System.DateTime.UtcNow; - var suggestions1 = ComponentResolver.GetAIPropertySuggestions(input, properties); + var suggestions1 = ComponentResolver.GetFuzzyPropertySuggestions(input, properties); var firstCallTime = (System.DateTime.UtcNow - startTime).TotalMilliseconds; // Second call - should use cache startTime = System.DateTime.UtcNow; - var suggestions2 = ComponentResolver.GetAIPropertySuggestions(input, properties); + var suggestions2 = ComponentResolver.GetFuzzyPropertySuggestions(input, properties); var secondCallTime = (System.DateTime.UtcNow - startTime).TotalMilliseconds; Assert.AreEqual(suggestions1.Count, suggestions2.Count, "Cached results should be identical"); @@ -315,8 +315,9 @@ namespace MCPForUnityTests.Editor.Tools }; // Expect the error logs from the invalid property - LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex("Unexpected error converting token to UnityEngine.Vector3")); - LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex("SetProperty.*Failed to set 'velocity'")); + // Note: PropertyConversion logs "Error converting token to..." when conversion fails + LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex("Error converting token to UnityEngine.Vector3")); + LogAssert.Expect(LogType.Error, new System.Text.RegularExpressions.Regex(@"\[SetProperty\].*Failed to set 'velocity'")); LogAssert.Expect(LogType.Warning, new System.Text.RegularExpressions.Regex("Property 'velocity' not found")); // Act @@ -555,5 +556,81 @@ namespace MCPForUnityTests.Editor.Tools UnityEngine.Object.DestroyImmediate(material2); UnityEngine.Object.DestroyImmediate(testObject); } + + #region Prefab Asset Handling Tests + + [Test] + public void HandleCommand_WithPrefabPath_ReturnsGuidanceError_ForModifyAction() + { + // Arrange - Attempt to modify a prefab asset directly + var modifyParams = new JObject + { + ["action"] = "modify", + ["target"] = "Assets/Prefabs/MyPrefab.prefab" + }; + + // Act + var result = ManageGameObject.HandleCommand(modifyParams); + + // Assert - Should return an error with guidance to use correct tools + Assert.IsNotNull(result, "Should return a result"); + var errorResponse = result as MCPForUnity.Editor.Helpers.ErrorResponse; + Assert.IsNotNull(errorResponse, "Should return an ErrorResponse"); + Assert.IsFalse(errorResponse.Success, "Should indicate failure"); + Assert.That(errorResponse.Error, Does.Contain("prefab asset"), "Error should mention prefab asset"); + Assert.That(errorResponse.Error, Does.Contain("manage_asset"), "Error should guide to manage_asset"); + Assert.That(errorResponse.Error, Does.Contain("manage_prefabs"), "Error should guide to manage_prefabs"); + } + + [Test] + public void HandleCommand_WithPrefabPath_ReturnsGuidanceError_ForDeleteAction() + { + // Arrange - Attempt to delete a prefab asset directly + var deleteParams = new JObject + { + ["action"] = "delete", + ["target"] = "Assets/Prefabs/SomePrefab.prefab" + }; + + // Act + var result = ManageGameObject.HandleCommand(deleteParams); + + // Assert - Should return an error with guidance + Assert.IsNotNull(result, "Should return a result"); + var errorResponse = result as MCPForUnity.Editor.Helpers.ErrorResponse; + Assert.IsNotNull(errorResponse, "Should return an ErrorResponse"); + Assert.IsFalse(errorResponse.Success, "Should indicate failure"); + Assert.That(errorResponse.Error, Does.Contain("prefab asset"), "Error should mention prefab asset"); + } + + [Test] + public void HandleCommand_WithPrefabPath_AllowsCreateAction() + { + // Arrange - Create (instantiate) from a prefab should be allowed + // Note: This will fail because the prefab doesn't exist, but the error should NOT be + // the prefab redirection error - it should be a "prefab not found" type error + var createParams = new JObject + { + ["action"] = "create", + ["prefab_path"] = "Assets/Prefabs/NonExistent.prefab", + ["name"] = "TestInstance" + }; + + // Act + var result = ManageGameObject.HandleCommand(createParams); + + // Assert - Should NOT return the prefab redirection error + // (It may fail for other reasons like prefab not found, but not due to redirection) + var errorResponse = result as MCPForUnity.Editor.Helpers.ErrorResponse; + if (errorResponse != null) + { + // If there's an error, it should NOT be the prefab asset guidance error + Assert.That(errorResponse.Error, Does.Not.Contain("Use 'manage_asset'"), + "Create action should not be blocked by prefab check"); + } + // If it's not an error, that's also fine (means create was allowed) + } + + #endregion } } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs index 1bedfd6..464ee03 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialPropertiesTests.cs @@ -54,7 +54,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); var mat = AssetDatabase.LoadAssetAtPath(_matPath); Assert.AreEqual(Color.red, mat.color); } @@ -75,7 +75,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); var mat = AssetDatabase.LoadAssetAtPath(_matPath); Assert.AreEqual(Color.green, mat.color); } @@ -93,7 +93,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); - Assert.AreEqual("success", result.Value("status")); + Assert.IsTrue(result.Value("success"), result.ToString()); } [Test] @@ -112,8 +112,8 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); - Assert.AreEqual("error", result.Value("status")); - string msg = result.Value("message"); + Assert.IsFalse(result.Value("success")); + string msg = result.Value("error"); // Verify we get exception details Assert.IsTrue(msg.Contains("Invalid JSON"), "Should mention Invalid JSON"); @@ -140,9 +140,9 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // We accept either success (ignored) or specific error, but not crash - // Assert.AreNotEqual("internal_error", result.Value("status")); - var status = result.Value("status"); - Assert.IsTrue(status == "success" || status == "error", $"Status should be success or error, got {status}"); + // The new response format uses a bool "success" field + var success = result.Value("success"); + Assert.IsNotNull(success, "Response should have success field"); } } } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs index caaac78..67016d4 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialReproTests.cs @@ -61,10 +61,10 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("error", result.Value("status")); + Assert.IsFalse(result.Value("success")); // We expect more detailed error message after fix - var message = result.Value("message"); + var message = result.Value("error"); Assert.IsTrue(message.StartsWith("Invalid JSON in properties"), "Message should start with prefix"); Assert.AreNotEqual("Invalid JSON in properties", message, "Message should contain exception details"); } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs index f0b430b..f5b78de 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialStressTests.cs @@ -80,8 +80,8 @@ namespace MCPForUnityTests.Editor.Tools ["color"] = new JArray(1f, 0f, 0f, 1f) }; var resultBadPath = ToJObject(ManageMaterial.HandleCommand(paramsBadPath)); - Assert.AreEqual("error", resultBadPath.Value("status")); - StringAssert.Contains("Could not find material", resultBadPath.Value("message")); + Assert.IsFalse(resultBadPath.Value("success")); + StringAssert.Contains("Could not find material", resultBadPath.Value("error")); // 2. Bad color array (too short) var paramsBadColor = new JObject @@ -91,8 +91,8 @@ namespace MCPForUnityTests.Editor.Tools ["color"] = new JArray(1f) // Invalid }; var resultBadColor = ToJObject(ManageMaterial.HandleCommand(paramsBadColor)); - Assert.AreEqual("error", resultBadColor.Value("status")); - StringAssert.Contains("Invalid color format", resultBadColor.Value("message")); + Assert.IsFalse(resultBadColor.Value("success")); + StringAssert.Contains("Invalid color format", resultBadColor.Value("error")); // 3. Bad slot index // Assign material first @@ -108,8 +108,8 @@ namespace MCPForUnityTests.Editor.Tools ["slot"] = 99 }; var resultBadSlot = ToJObject(ManageMaterial.HandleCommand(paramsBadSlot)); - Assert.AreEqual("error", resultBadSlot.Value("status")); - StringAssert.Contains("out of bounds", resultBadSlot.Value("message")); + Assert.IsFalse(resultBadSlot.Value("success")); + StringAssert.Contains("out of bounds", resultBadSlot.Value("error")); } [Test] @@ -137,7 +137,7 @@ namespace MCPForUnityTests.Editor.Tools }; var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); - Assert.AreEqual("success", result.Value("status")); + Assert.IsTrue(result.Value("success"), result.ToString()); // Assert // 1. Renderer has property block with Red @@ -168,7 +168,7 @@ namespace MCPForUnityTests.Editor.Tools ["materialPath"] = _matPath }; var assignResult = ToJObject(ManageMaterial.HandleCommand(assignParams)); - Assert.AreEqual("success", assignResult.Value("status")); + Assert.IsTrue(assignResult.Value("success"), assignResult.ToString()); // Verify assignment var renderer = _cube.GetComponent(); @@ -183,7 +183,7 @@ namespace MCPForUnityTests.Editor.Tools ["color"] = new JArray(newColor.r, newColor.g, newColor.b, newColor.a) }; var colorResult = ToJObject(ManageMaterial.HandleCommand(colorParams)); - Assert.AreEqual("success", colorResult.Value("status")); + Assert.IsTrue(colorResult.Value("success"), colorResult.ToString()); // Verify color changed on renderer (because it's shared) var propName = renderer.sharedMaterial.HasProperty("_BaseColor") ? "_BaseColor" : "_Color"; diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.cs index 1285e42..cf587d8 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageMaterialTests.cs @@ -84,7 +84,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); mat = AssetDatabase.LoadAssetAtPath(_matPath); // Reload var prop = mat.shader.name == "Standard" ? "_Color" : "_BaseColor"; @@ -109,7 +109,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); var mat = AssetDatabase.LoadAssetAtPath(_matPath); var prop = mat.HasProperty("_BaseColor") ? "_BaseColor" : "_Color"; @@ -140,7 +140,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); var renderer = go.GetComponent(); Assert.IsNotNull(renderer.sharedMaterial); @@ -181,7 +181,7 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("success", result.Value("status"), result.ToString()); + Assert.IsTrue(result.Value("success"), result.ToString()); var renderer = go.GetComponent(); var block = new MaterialPropertyBlock(); @@ -215,10 +215,12 @@ namespace MCPForUnityTests.Editor.Tools var result = ToJObject(ManageMaterial.HandleCommand(paramsObj)); // Assert - Assert.AreEqual("success", result.Value("status"), result.ToString()); - Assert.IsNotNull(result["properties"]); - Assert.IsInstanceOf(result["properties"]); - var props = result["properties"] as JArray; + Assert.IsTrue(result.Value("success"), result.ToString()); + var data = result["data"] as JObject; + Assert.IsNotNull(data, "Response should have data object"); + Assert.IsNotNull(data["properties"]); + Assert.IsInstanceOf(data["properties"]); + var props = data["properties"] as JArray; Assert.IsTrue(props.Count > 0); // Check for standard properties diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.cs index 5d56459..329870a 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/MaterialParameterToolTests.cs @@ -144,7 +144,7 @@ namespace MCPForUnityTests.Editor.Tools }; var assignResult = ToJObject(ManageMaterial.HandleCommand(assignParams)); - Assert.AreEqual("success", assignResult.Value("status"), assignResult.ToString()); + Assert.IsTrue(assignResult.Value("success"), assignResult.ToString()); var renderer = _sphere.GetComponent(); Assert.IsNotNull(renderer, "Sphere should have MeshRenderer.");