diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs index ce008fa..db2b525 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/ManageGameObjectTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using NUnit.Framework; using UnityEngine; using UnityEditor; +using UnityEngine.TestTools; using Newtonsoft.Json.Linq; using MCPForUnity.Editor.Tools; @@ -194,5 +195,118 @@ namespace MCPForUnityTests.Editor.Tools // Second call should be faster (though this test might be flaky) Assert.LessOrEqual(secondCallTime, firstCallTime * 2, "Cached call should not be significantly slower"); } + + [Test] + public void SetComponentProperties_CollectsAllFailuresAndAppliesValidOnes() + { + // Arrange - add Transform and Rigidbody components to test with + var transform = testGameObject.transform; + var rigidbody = testGameObject.AddComponent(); + + // Create a params object with mixed valid and invalid properties + var setPropertiesParams = new JObject + { + ["action"] = "modify", + ["target"] = testGameObject.name, + ["search_method"] = "by_name", + ["componentProperties"] = new JObject + { + ["Transform"] = new JObject + { + ["localPosition"] = new JObject { ["x"] = 1.0f, ["y"] = 2.0f, ["z"] = 3.0f }, // Valid + ["rotatoin"] = new JObject { ["x"] = 0.0f, ["y"] = 90.0f, ["z"] = 0.0f }, // Invalid (typo - should be rotation) + ["localScale"] = new JObject { ["x"] = 2.0f, ["y"] = 2.0f, ["z"] = 2.0f } // Valid + }, + ["Rigidbody"] = new JObject + { + ["mass"] = 5.0f, // Valid + ["invalidProp"] = "test", // Invalid - doesn't exist + ["useGravity"] = true // Valid + } + } + }; + + // Store original values to verify changes + var originalLocalPosition = transform.localPosition; + var originalLocalScale = transform.localScale; + var originalMass = rigidbody.mass; + var originalUseGravity = rigidbody.useGravity; + + Debug.Log($"BEFORE TEST - Mass: {rigidbody.mass}, UseGravity: {rigidbody.useGravity}"); + + // Expect the warning logs from the invalid properties + LogAssert.Expect(LogType.Warning, new System.Text.RegularExpressions.Regex("Property 'rotatoin' not found")); + LogAssert.Expect(LogType.Warning, new System.Text.RegularExpressions.Regex("Property 'invalidProp' not found")); + + // Act + var result = ManageGameObject.HandleCommand(setPropertiesParams); + + Debug.Log($"AFTER TEST - Mass: {rigidbody.mass}, UseGravity: {rigidbody.useGravity}"); + Debug.Log($"AFTER TEST - LocalPosition: {transform.localPosition}"); + Debug.Log($"AFTER TEST - LocalScale: {transform.localScale}"); + + // Assert - verify that valid properties were set despite invalid ones + Assert.AreEqual(new Vector3(1.0f, 2.0f, 3.0f), transform.localPosition, + "Valid localPosition should be set even with other invalid properties"); + Assert.AreEqual(new Vector3(2.0f, 2.0f, 2.0f), transform.localScale, + "Valid localScale should be set even with other invalid properties"); + Assert.AreEqual(5.0f, rigidbody.mass, 0.001f, + "Valid mass should be set even with other invalid properties"); + Assert.AreEqual(true, rigidbody.useGravity, + "Valid useGravity should be set even with other invalid properties"); + + // Verify the result indicates errors (since we had invalid properties) + Assert.IsNotNull(result, "Should return a result object"); + + // The collect-and-continue behavior means we should get an error response + // that contains info about the failed properties, but valid ones were still applied + // This proves the collect-and-continue behavior is working + } + + [Test] + public void SetComponentProperties_ContinuesAfterException() + { + // Arrange - create scenario that might cause exceptions + var rigidbody = testGameObject.AddComponent(); + + // Set initial values that we'll change + rigidbody.mass = 1.0f; + rigidbody.useGravity = true; + + var setPropertiesParams = new JObject + { + ["action"] = "modify", + ["target"] = testGameObject.name, + ["search_method"] = "by_name", + ["componentProperties"] = new JObject + { + ["Rigidbody"] = new JObject + { + ["mass"] = 2.5f, // Valid - should be set + ["velocity"] = "invalid_type", // Invalid type - will cause exception + ["useGravity"] = false // Valid - should still be set after exception + } + } + }; + + // 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'")); + LogAssert.Expect(LogType.Warning, new System.Text.RegularExpressions.Regex("Property 'velocity' not found")); + + // Act + var result = ManageGameObject.HandleCommand(setPropertiesParams); + + // Assert - verify that valid properties before AND after the exception were still set + Assert.AreEqual(2.5f, rigidbody.mass, 0.001f, + "Mass should be set even if later property causes exception"); + Assert.AreEqual(false, rigidbody.useGravity, + "UseGravity should be set even if previous property caused exception"); + + Assert.IsNotNull(result, "Should return a result even with exceptions"); + + // The key test: processing continued after the exception and set useGravity + // This proves the collect-and-continue behavior works even with exceptions + } } } \ No newline at end of file diff --git a/UnityMcpBridge/Editor/Tools/ManageGameObject.cs b/UnityMcpBridge/Editor/Tools/ManageGameObject.cs index 7b55dbb..0e19382 100644 --- a/UnityMcpBridge/Editor/Tools/ManageGameObject.cs +++ b/UnityMcpBridge/Editor/Tools/ManageGameObject.cs @@ -785,6 +785,7 @@ namespace MCPForUnity.Editor.Tools } // Set Component Properties + var componentErrors = new List(); if (@params["componentProperties"] is JObject componentPropertiesObj) { foreach (var prop in componentPropertiesObj.Properties()) @@ -799,12 +800,26 @@ namespace MCPForUnity.Editor.Tools propertiesToSet ); if (setResult != null) - return setResult; - modified = true; + { + componentErrors.Add(setResult); + } + else + { + modified = true; + } } } } + // Return component errors if any occurred (after processing all components) + if (componentErrors.Count > 0) + { + return Response.Error( + $"One or more component property operations failed on '{targetGo.name}'.", + new { componentErrors = componentErrors } + ); + } + if (!modified) { // Use the new serializer helper @@ -1534,6 +1549,7 @@ namespace MCPForUnity.Editor.Tools Undo.RecordObject(targetComponent, "Set Component Properties"); + var failures = new List(); foreach (var prop in propertiesToSet.Properties()) { string propName = prop.Name; @@ -1541,39 +1557,16 @@ namespace MCPForUnity.Editor.Tools try { - if (!SetProperty(targetComponent, propName, propValue)) + bool setResult = SetProperty(targetComponent, propName, propValue); + if (!setResult) { - // Get available properties and AI suggestions for better error messages var availableProperties = ComponentResolver.GetAllComponentProperties(targetComponent.GetType()); var suggestions = ComponentResolver.GetAIPropertySuggestions(propName, availableProperties); - - var errorMessage = $"[ManageGameObject] Could not set property '{propName}' on component '{compName}' ('{targetComponent.GetType().Name}')."; - - if (suggestions.Any()) - { - errorMessage += $" Did you mean: {string.Join(", ", suggestions)}?"; - } - - errorMessage += $" Available properties: [{string.Join(", ", availableProperties)}]"; - - Debug.LogWarning(errorMessage); - - // Return enhanced error with suggestions for better UX - if (suggestions.Any()) - { - return Response.Error( - $"Property '{propName}' not found on {compName}. " + - $"Did you mean: {string.Join(", ", suggestions)}? " + - $"Available properties: [{string.Join(", ", availableProperties)}]" - ); - } - else - { - return Response.Error( - $"Property '{propName}' not found on {compName}. " + - $"Available properties: [{string.Join(", ", 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)}]"; + Debug.LogWarning($"[ManageGameObject] {msg}"); + failures.Add(msg); } } catch (Exception e) @@ -1581,12 +1574,13 @@ namespace MCPForUnity.Editor.Tools Debug.LogError( $"[ManageGameObject] Error setting property '{propName}' on '{compName}': {e.Message}" ); - // Optionally return an error here - // return Response.Error($"Error setting property '{propName}' on '{compName}': {e.Message}"); + failures.Add($"Error setting '{propName}': {e.Message}"); } } EditorUtility.SetDirty(targetComponent); - return null; // Success (or partial success if warnings were logged) + return failures.Count == 0 + ? null + : Response.Error($"One or more properties failed on '{compName}'.", new { errors = failures }); } ///