diff --git a/MCPForUnity/Editor/Tools/BatchExecute.cs b/MCPForUnity/Editor/Tools/BatchExecute.cs index aca4a1994..e8884e1fb 100644 --- a/MCPForUnity/Editor/Tools/BatchExecute.cs +++ b/MCPForUnity/Editor/Tools/BatchExecute.cs @@ -86,7 +86,7 @@ public static async Task HandleCommand(JObject @params) string toolName = commandObj["tool"]?.ToString(); var rawParams = commandObj["params"] as JObject ?? new JObject(); - var commandParams = NormalizeParameterKeys(rawParams); + var commandParams = NormalizeCommandParams(rawParams); if (string.IsNullOrWhiteSpace(toolName)) { @@ -214,7 +214,7 @@ private static bool DetermineCallSucceeded(object result) return true; } - private static JObject NormalizeParameterKeys(JObject source) + private static JObject NormalizeCommandParams(JObject source) { if (source == null) { @@ -225,11 +225,73 @@ private static JObject NormalizeParameterKeys(JObject source) foreach (var property in source.Properties()) { string normalizedName = ToCamelCase(property.Name); - normalized[normalizedName] = property.Value; + normalized[normalizedName] = NormalizeStructuredJsonStrings(property.Value); } return normalized; } + private static JToken NormalizeStructuredJsonStrings(JToken token) + { + if (token == null) + { + return JValue.CreateNull(); + } + + return token.Type switch + { + JTokenType.Object => NormalizeObject((JObject)token), + JTokenType.Array => NormalizeArray((JArray)token), + JTokenType.String => TryParseStructuredJsonString(token.Value()), + _ => token.DeepClone() + }; + } + + private static JObject NormalizeObject(JObject source) + { + var normalized = new JObject(); + foreach (var property in source.Properties()) + { + normalized[property.Name] = NormalizeStructuredJsonStrings(property.Value); + } + return normalized; + } + + private static JArray NormalizeArray(JArray source) + { + var normalized = new JArray(); + foreach (var item in source) + { + normalized.Add(NormalizeStructuredJsonStrings(item)); + } + return normalized; + } + + private static JToken TryParseStructuredJsonString(string value) + { + if (string.IsNullOrWhiteSpace(value)) + { + return new JValue(value); + } + + string trimmed = value.Trim(); + bool looksLikeObject = trimmed.StartsWith("{", StringComparison.Ordinal) && trimmed.EndsWith("}", StringComparison.Ordinal); + bool looksLikeArray = trimmed.StartsWith("[", StringComparison.Ordinal) && trimmed.EndsWith("]", StringComparison.Ordinal); + + if (!looksLikeObject && !looksLikeArray) + { + return new JValue(value); + } + + try + { + return NormalizeStructuredJsonStrings(JToken.Parse(trimmed)); + } + catch + { + return new JValue(value); + } + } + private static string ToCamelCase(string key) => StringCaseUtility.ToCamelCase(key); } } diff --git a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs index b593fb98e..e6fb207f2 100644 --- a/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs +++ b/MCPForUnity/Runtime/Serialization/UnityTypeConverters.cs @@ -113,12 +113,21 @@ public override void WriteJson(JsonWriter writer, Color value, JsonSerializer se public override Color ReadJson(JsonReader reader, Type objectType, Color existingValue, bool hasExistingValue, JsonSerializer serializer) { - JObject jo = JObject.Load(reader); + JToken token = JToken.Load(reader); + if (token is JArray arr && arr.Count >= 3) + { + float alpha = arr.Count >= 4 ? (float)arr[3] : 1f; + return new Color((float)arr[0], (float)arr[1], (float)arr[2], alpha); + } + + if (token is not JObject jo) + throw new JsonSerializationException($"Cannot deserialize Color from {token.Type}: '{token}'"); + return new Color( (float)jo["r"], (float)jo["g"], (float)jo["b"], - (float)jo["a"] + jo["a"] != null ? (float)jo["a"] : 1f ); } } @@ -141,7 +150,13 @@ public override void WriteJson(JsonWriter writer, Rect value, JsonSerializer ser public override Rect ReadJson(JsonReader reader, Type objectType, Rect existingValue, bool hasExistingValue, JsonSerializer serializer) { - JObject jo = JObject.Load(reader); + JToken token = JToken.Load(reader); + if (token is JArray arr && arr.Count >= 4) + return new Rect((float)arr[0], (float)arr[1], (float)arr[2], (float)arr[3]); + + if (token is not JObject jo) + throw new JsonSerializationException($"Cannot deserialize Rect from {token.Type}: '{token}'"); + return new Rect( (float)jo["x"], (float)jo["y"], @@ -468,4 +483,4 @@ private static bool IsValidGuid(string str) return true; } } -} \ No newline at end of file +} diff --git a/Server/src/services/tools/manage_components.py b/Server/src/services/tools/manage_components.py index da2cdba18..9d1d330b8 100644 --- a/Server/src/services/tools/manage_components.py +++ b/Server/src/services/tools/manage_components.py @@ -104,10 +104,12 @@ async def manage_components( if props_error: return {"success": False, "message": props_error} - # --- Validate value parameter for serialization issues --- + # --- Validate/normalize value parameter for serialization issues --- if value is not None and isinstance(value, str) and value in ("[object Object]", "undefined"): return {"success": False, "message": f"value received invalid input: '{value}'. Expected an actual value."} + value = parse_json_payload(value) + try: params = { "action": action, diff --git a/Server/tests/integration/test_manage_components.py b/Server/tests/integration/test_manage_components.py index 7e777088e..5be5c6aa9 100644 --- a/Server/tests/integration/test_manage_components.py +++ b/Server/tests/integration/test_manage_components.py @@ -156,6 +156,96 @@ async def fake_send(cmd, params, **kwargs): assert captured["params"]["properties"] == {"mass": 10.0} +@pytest.mark.asyncio +@pytest.mark.parametrize( + ("property_name", "raw_value", "expected_value"), + [ + ( + "position", + "[1.0, 2.0, 3.0]", + [1.0, 2.0, 3.0], + ), + ( + "color", + '{"r": 1.0, "g": 1.0, "b": 0.0, "a": 1.0}', + {"r": 1.0, "g": 1.0, "b": 0.0, "a": 1.0}, + ), + ], +) +async def test_manage_components_set_property_single_structured_json_value( + monkeypatch, + property_name, + raw_value, + expected_value, +): + """Test JSON-string single values are normalized before dispatch. + + The Python-side contract is intentionally generic: + - array-shaped JSON should become Python lists + - object-shaped JSON should become Python dicts + + Detailed Unity struct compatibility is covered by Unity-side tests. + """ + captured = {} + + async def fake_send(cmd, params, **kwargs): + captured["params"] = params + return {"success": True, "data": {"instanceID": 12345}} + + monkeypatch.setattr( + manage_comp_mod, + "async_send_command_with_retry", + fake_send, + ) + + resp = await manage_comp_mod.manage_components( + ctx=DummyContext(), + action="set_property", + target="TestObject", + component_type="Transform", + property=property_name, + value=raw_value, + ) + + assert resp.get("success") is True + assert captured["params"]["property"] == property_name + assert captured["params"]["value"] == expected_value + + +@pytest.mark.asyncio +async def test_manage_components_set_property_single_plain_string_value_stays_string(monkeypatch): + """Test ordinary string values are forwarded unchanged. + + This guards the conservative behavior of parse_json_payload: plain strings + should not be coerced just because structured JSON-string values are now supported. + """ + captured = {} + + async def fake_send(cmd, params, **kwargs): + captured["params"] = params + return {"success": True, "data": {"instanceID": 12345}} + + monkeypatch.setattr( + manage_comp_mod, + "async_send_command_with_retry", + fake_send, + ) + + resp = await manage_comp_mod.manage_components( + ctx=DummyContext(), + action="set_property", + target="GameManager", + component_type="ExampleComponent", + property="displayName", + value="Player One", + ) + + assert resp.get("success") is True + assert captured["params"]["property"] == "displayName" + assert captured["params"]["value"] == "Player One" + assert isinstance(captured["params"]["value"], str) + + @pytest.mark.asyncio async def test_manage_components_add_with_properties(monkeypatch): """Test adding a component with initial properties.""" diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteKeyPreservationTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteKeyPreservationTests.cs index 3419dfed5..e1a360bac 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteKeyPreservationTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/BatchExecuteKeyPreservationTests.cs @@ -162,5 +162,74 @@ public void Regression_CreateGameObject_StillWorksViaBatch() Object.DestroyImmediate(created); } } + + [Test] + public void StructuredJsonStrings_InCommandParams_AreParsedBeforeDispatch() + { + var light = testGo.AddComponent(); + testGo.name = "BatchStructuredJson_" + System.Guid.NewGuid().ToString("N").Substring(0, 8); + + var batchParams = new JObject + { + ["commands"] = new JArray + { + new JObject + { + ["tool"] = "manage_components", + ["params"] = new JObject + { + ["action"] = "set_property", + ["target"] = testGo.name, + ["search_method"] = "by_name", + ["component_type"] = "Light", + ["property"] = "color", + ["value"] = "[0.0, 1.0, 1.0, 1.0]" + } + } + } + }; + + var result = BatchExecute.HandleCommand(batchParams).GetAwaiter().GetResult(); + var resultObj = JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), $"Batch should succeed: {resultObj}"); + Assert.AreEqual(new Color(0f, 1f, 1f, 1f), light.color); + } + + [Test] + public void PlainStrings_InCommandParams_ArePreserved() + { + testGo.AddComponent(); + + var batchParams = new JObject + { + ["commands"] = new JArray + { + new JObject + { + ["tool"] = "manage_components", + ["params"] = new JObject + { + ["action"] = "set_property", + ["target"] = testGo.name, + ["search_method"] = "by_name", + ["component_type"] = "CustomComponent", + ["property"] = "customText", + ["value"] = "Player One" + } + } + } + }; + + var result = BatchExecute.HandleCommand(batchParams).GetAwaiter().GetResult(); + var resultObj = JObject.FromObject(result); + + Assert.IsTrue(resultObj.Value("success"), $"Batch should succeed: {resultObj}"); + Assert.AreEqual("Player One", + testGo.GetComponent() + .GetType() + .GetField("customText", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance) + ?.GetValue(testGo.GetComponent()) as string); + } } } diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs new file mode 100644 index 000000000..bcdb1ac7d --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs @@ -0,0 +1,47 @@ +using NUnit.Framework; +using Newtonsoft.Json.Linq; +using UnityEngine; +using MCPForUnity.Editor.Helpers; + +namespace MCPForUnityTests.Editor.Tools +{ + /// + /// Guards array compatibility for Unity structs that commonly flow through + /// manage_components as JSON-stringified values. + /// + public class PropertyConversion_UnityStructArraySupport_Tests + { + [Test] + public void ConvertToType_ColorArrayWithAlpha_Succeeds() + { + var result = (Color)PropertyConversion.ConvertToType( + JArray.Parse("[1.0, 0.5, 0.25, 0.75]"), + typeof(Color) + ); + + Assert.AreEqual(new Color(1.0f, 0.5f, 0.25f, 0.75f), result); + } + + [Test] + public void ConvertToType_ColorArrayWithoutAlpha_DefaultsToOne() + { + var result = (Color)PropertyConversion.ConvertToType( + JArray.Parse("[1.0, 0.5, 0.25]"), + typeof(Color) + ); + + Assert.AreEqual(new Color(1.0f, 0.5f, 0.25f, 1.0f), result); + } + + [Test] + public void ConvertToType_RectArray_Succeeds() + { + var result = (Rect)PropertyConversion.ConvertToType( + JArray.Parse("[10.0, 20.0, 30.0, 40.0]"), + typeof(Rect) + ); + + Assert.AreEqual(new Rect(10.0f, 20.0f, 30.0f, 40.0f), result); + } + } +} diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs.meta b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs.meta new file mode 100644 index 000000000..c1075f1d2 --- /dev/null +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/Tools/PropertyConversion_UnityStructArraySupport_Tests.cs.meta @@ -0,0 +1,11 @@ +fileFormatVersion: 2 +guid: 81cb10f4d3334814a85f7e6d52db99f1 +MonoImporter: + externalObjects: {} + serializedVersion: 2 + defaultReferences: [] + executionOrder: 0 + icon: {instanceID: 0} + userData: + assetBundleName: + assetBundleVariant: