diff --git a/.github/workflows/unity-tests.yml b/.github/workflows/unity-tests.yml index 8f0837bf..bfd04055 100644 --- a/.github/workflows/unity-tests.yml +++ b/.github/workflows/unity-tests.yml @@ -1,6 +1,7 @@ name: Unity Tests on: + workflow_dispatch: {} push: branches: [main] paths: diff --git a/MCPForUnity/Editor/Tools/ManageAsset.cs b/MCPForUnity/Editor/Tools/ManageAsset.cs index 770fd243..04dcbbfe 100644 --- a/MCPForUnity/Editor/Tools/ManageAsset.cs +++ b/MCPForUnity/Editor/Tools/ManageAsset.cs @@ -88,7 +88,8 @@ public static object HandleCommand(JObject @params) case "create": return CreateAsset(@params); case "modify": - return ModifyAsset(path, @params["properties"] as JObject); + var properties = @params["properties"] as JObject; + return ModifyAsset(path, properties); case "delete": return DeleteAsset(path); case "duplicate": @@ -988,28 +989,43 @@ private static bool ApplyMaterialProperties(Material mat, JObject properties) } } } - // Example: Set texture property - if (properties["texture"] is JObject texProps) + // Example: Set texture property (case-insensitive key and subkeys) { - string propName = texProps["name"]?.ToString() ?? "_MainTex"; // Default main texture - string texPath = texProps["path"]?.ToString(); - if (!string.IsNullOrEmpty(texPath)) + JObject texProps = null; + var direct = properties.Property("texture"); + if (direct != null && direct.Value is JObject t0) texProps = t0; + if (texProps == null) { - Texture newTex = AssetDatabase.LoadAssetAtPath( - AssetPathUtility.SanitizeAssetPath(texPath) - ); - if ( - newTex != null - && mat.HasProperty(propName) - && mat.GetTexture(propName) != newTex - ) - { - mat.SetTexture(propName, newTex); - modified = true; - } - else if (newTex == null) + var ci = properties.Properties().FirstOrDefault( + p => string.Equals(p.Name, "texture", StringComparison.OrdinalIgnoreCase)); + if (ci != null && ci.Value is JObject t1) texProps = t1; + } + if (texProps != null) + { + string rawName = (texProps["name"] ?? texProps["Name"])?.ToString(); + string texPath = (texProps["path"] ?? texProps["Path"])?.ToString(); + if (!string.IsNullOrEmpty(texPath)) { - Debug.LogWarning($"Texture not found at path: {texPath}"); + var newTex = AssetDatabase.LoadAssetAtPath( + AssetPathUtility.SanitizeAssetPath(texPath)); + if (newTex == null) + { + Debug.LogWarning($"Texture not found at path: {texPath}"); + } + else + { + // Reuse alias resolver so friendly names like 'albedo' work here too + string candidateName = string.IsNullOrEmpty(rawName) ? "_BaseMap" : rawName; + string targetProp = ResolvePropertyName(candidateName); + if (!string.IsNullOrEmpty(targetProp) && mat.HasProperty(targetProp)) + { + if (mat.GetTexture(targetProp) != newTex) + { + mat.SetTexture(targetProp, newTex); + modified = true; + } + } + } } } } @@ -1026,15 +1042,20 @@ string ResolvePropertyName(string name) { if (string.IsNullOrEmpty(name)) return name; string[] candidates; - switch (name) + var lower = name.ToLowerInvariant(); + switch (lower) { - case "_Color": candidates = new[] { "_Color", "_BaseColor" }; break; - case "_BaseColor": candidates = new[] { "_BaseColor", "_Color" }; break; - case "_MainTex": candidates = new[] { "_MainTex", "_BaseMap" }; break; - case "_BaseMap": candidates = new[] { "_BaseMap", "_MainTex" }; break; - case "_Glossiness": candidates = new[] { "_Glossiness", "_Smoothness" }; break; - case "_Smoothness": candidates = new[] { "_Smoothness", "_Glossiness" }; break; - default: candidates = new[] { name }; break; + case "_color": candidates = new[] { "_Color", "_BaseColor" }; break; + case "_basecolor": candidates = new[] { "_BaseColor", "_Color" }; break; + case "_maintex": candidates = new[] { "_MainTex", "_BaseMap" }; break; + case "_basemap": candidates = new[] { "_BaseMap", "_MainTex" }; break; + case "_glossiness": candidates = new[] { "_Glossiness", "_Smoothness" }; break; + case "_smoothness": candidates = new[] { "_Smoothness", "_Glossiness" }; break; + // Friendly names → shader property names + case "metallic": candidates = new[] { "_Metallic" }; break; + case "smoothness": candidates = new[] { "_Smoothness", "_Glossiness" }; break; + case "albedo": candidates = new[] { "_BaseMap", "_MainTex" }; break; + default: candidates = new[] { name }; break; // keep original as-is } foreach (var candidate in candidates) { diff --git a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPToolParameterTests.cs b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPToolParameterTests.cs index 325e200c..cb9395ef 100644 --- a/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPToolParameterTests.cs +++ b/TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPToolParameterTests.cs @@ -6,6 +6,7 @@ using Newtonsoft.Json.Linq; using MCPForUnity.Editor.Tools; using System; +using System.Text.RegularExpressions; namespace Tests.EditMode { @@ -18,6 +19,17 @@ namespace Tests.EditMode /// public class MCPToolParameterTests { + private static void AssertShaderIsSupported(Shader s) + { + Assert.IsNotNull(s, "Shader should not be null"); + // Accept common defaults across pipelines + var name = s.name; + bool ok = name == "Universal Render Pipeline/Lit" + || name == "HDRP/Lit" + || name == "Standard" + || name == "Unlit/Color"; + Assert.IsTrue(ok, $"Unexpected shader: {name}"); + } [Test] public void Test_ManageAsset_ShouldAcceptJSONProperties() { @@ -50,8 +62,9 @@ public void Test_ManageAsset_ShouldAcceptJSONProperties() Assert.IsNotNull(result, "Handler should return a JObject result"); Assert.IsTrue(result!.Value("success"), result.ToString()); - var mat = AssetDatabase.LoadAssetAtPath(matPath); - Assert.IsNotNull(mat, "Material should be created at path"); + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should be created at path"); + AssertShaderIsSupported(mat.shader); if (mat.HasProperty("_Color")) { Assert.AreEqual(Color.blue, mat.GetColor("_Color")); @@ -108,6 +121,17 @@ public void Test_ManageGameObject_ShouldAcceptJSONComponentProperties() var raw = ManageGameObject.HandleCommand(modify); var result = raw as JObject ?? JObject.FromObject(raw); Assert.IsTrue(result.Value("success"), result.ToString()); + + // Verify material assignment and shader on the GameObject's MeshRenderer + var goVerify = GameObject.Find("MCPParamTestSphere"); + Assert.IsNotNull(goVerify, "GameObject should exist"); + var renderer = goVerify.GetComponent(); + Assert.IsNotNull(renderer, "MeshRenderer should exist"); + var assignedMat = renderer.sharedMaterial; + Assert.IsNotNull(assignedMat, "sharedMaterial should be assigned"); + AssertShaderIsSupported(assignedMat.shader); + var createdMat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.AreEqual(createdMat, assignedMat, "Assigned material should match created material"); } finally { @@ -158,6 +182,11 @@ public void Test_JSONParsing_ShouldWorkInMCPTools() var modResRaw = ManageGameObject.HandleCommand(modify); var modRes = modResRaw as JObject ?? JObject.FromObject(modResRaw); Assert.IsTrue(modRes.Value("success"), modRes.ToString()); + + // Verify shader on created material + var createdMat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(createdMat); + AssertShaderIsSupported(createdMat.shader); } finally { @@ -168,5 +197,504 @@ public void Test_JSONParsing_ShouldWorkInMCPTools() } } + [Test] + public void Test_ManageAsset_JSONStringParsing_CreateMaterial() + { + // Test that JSON string properties are correctly parsed and applied + const string tempDir = "Assets/Temp/MCPToolParameterTests"; + var matPath = $"{tempDir}/JsonStringTest_{Guid.NewGuid().ToString("N")}.mat"; + + var p = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [1,0,0,1]}" + }; + + try + { + var raw = ManageAsset.HandleCommand(p); + var result = raw as JObject ?? JObject.FromObject(raw); + Assert.IsNotNull(result, "Handler should return a JObject result"); + Assert.IsTrue(result!.Value("success"), $"Create failed: {result}"); + + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should be created at path"); + AssertShaderIsSupported(mat.shader); + if (mat.HasProperty("_Color")) + { + Assert.AreEqual(Color.red, mat.GetColor("_Color"), "Material should have red color"); + } + } + finally + { + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + { + AssetDatabase.DeleteAsset(matPath); + } + AssetDatabase.Refresh(); + } + } + + [Test] + public void Test_ManageAsset_JSONStringParsing_ModifyMaterial() + { + // Test that JSON string properties work for modify operations + const string tempDir = "Assets/Temp/MCPToolParameterTests"; + var matPath = $"{tempDir}/JsonStringModifyTest_{Guid.NewGuid().ToString("N")}.mat"; + + // First create a material + var createParams = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"color\": [0,1,0,1]}" + }; + + try + { + var createRaw = ManageAsset.HandleCommand(createParams); + var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); + Assert.IsTrue(createResult!.Value("success"), "Create should succeed"); + + // Now modify with JSON string + var modifyParams = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"color\": [0,0,1,1]}" + }; + + var modifyRaw = ManageAsset.HandleCommand(modifyParams); + var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); + Assert.IsNotNull(modifyResult, "Modify should return a result"); + Assert.IsTrue(modifyResult!.Value("success"), $"Modify failed: {modifyResult}"); + + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should exist"); + AssertShaderIsSupported(mat.shader); + if (mat.HasProperty("_Color")) + { + Assert.AreEqual(Color.blue, mat.GetColor("_Color"), "Material should have blue color after modify"); + } + } + finally + { + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + { + AssetDatabase.DeleteAsset(matPath); + } + AssetDatabase.Refresh(); + } + } + + [Test] + public void Test_ManageAsset_InvalidJSONString_HandledGracefully() + { + // Test that invalid JSON strings are handled gracefully + const string tempDir = "Assets/Temp/MCPToolParameterTests"; + var matPath = $"{tempDir}/InvalidJsonTest_{Guid.NewGuid().ToString("N")}.mat"; + + var p = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = "{\"invalid\": json, \"missing\": quotes}" // Invalid JSON + }; + + try + { + LogAssert.Expect(LogType.Warning, new Regex("(failed to parse)|(Could not parse 'properties' JSON string)", RegexOptions.IgnoreCase)); + var raw = ManageAsset.HandleCommand(p); + var result = raw as JObject ?? JObject.FromObject(raw); + // Should either succeed with defaults or fail gracefully + Assert.IsNotNull(result, "Handler should return a result"); + // The result might be success (with defaults) or failure, both are acceptable + } + finally + { + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + { + AssetDatabase.DeleteAsset(matPath); + } + AssetDatabase.Refresh(); + } + } + + [Test] + public void Test_ManageAsset_JSONStringParsing_FloatProperty_Metallic_CreateAndModify() + { + // Validate float property handling via JSON string for create and modify + const string tempDir = "Assets/Temp/MCPToolParameterTests"; + if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); + if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); + var matPath = $"{tempDir}/JsonFloatTest_{Guid.NewGuid().ToString("N")}.mat"; + + var createParams = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = "{\"shader\": \"Universal Render Pipeline/Lit\", \"metallic\": 0.75}" + }; + + try + { + var createRaw = ManageAsset.HandleCommand(createParams); + var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); + Assert.IsTrue(createResult!.Value("success"), createResult.ToString()); + + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should be created at path"); + AssertShaderIsSupported(mat.shader); + if (mat.HasProperty("_Metallic")) + { + Assert.AreEqual(0.75f, mat.GetFloat("_Metallic"), 1e-3f, "Metallic should be ~0.75 after create"); + } + + var modifyParams = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"metallic\": 0.1}" + }; + + var modifyRaw = ManageAsset.HandleCommand(modifyParams); + var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); + Assert.IsTrue(modifyResult!.Value("success"), modifyResult.ToString()); + + var mat2 = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat2, "Material should still exist"); + if (mat2.HasProperty("_Metallic")) + { + Assert.AreEqual(0.1f, mat2.GetFloat("_Metallic"), 1e-3f, "Metallic should be ~0.1 after modify"); + } + } + finally + { + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + { + AssetDatabase.DeleteAsset(matPath); + } + AssetDatabase.Refresh(); + } + } + + [Test] + public void Test_ManageAsset_JSONStringParsing_TextureAssignment_CreateAndModify() + { + // Uses flexible direct property assignment to set _BaseMap/_MainTex by path + const string tempDir = "Assets/Temp/MCPToolParameterTests"; + if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); + if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests"); + var matPath = $"{tempDir}/JsonTexTest_{Guid.NewGuid().ToString("N")}.mat"; + var texPath = "Assets/Temp/LiveTests/TempBaseTex.asset"; // created by GenTempTex + + var createParams = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = new JObject + { + ["shader"] = "Universal Render Pipeline/Lit", + ["_BaseMap"] = texPath // resolves to _BaseMap or _MainTex internally + } + }; + + try + { + var createRaw = ManageAsset.HandleCommand(createParams); + var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); + Assert.IsTrue(createResult!.Value("success"), createResult.ToString()); + + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should be created at path"); + AssertShaderIsSupported(mat.shader); + var tex = AssetDatabase.LoadAssetAtPath(texPath); + if (tex == null) + { + // Create a tiny white texture if missing to make the test self-sufficient + if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); + if (!AssetDatabase.IsValidFolder("Assets/Temp/LiveTests")) AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); + var tex2D = new Texture2D(4, 4, TextureFormat.RGBA32, false); + var pixels = new Color[16]; + for (int i = 0; i < pixels.Length; i++) pixels[i] = Color.white; + tex2D.SetPixels(pixels); + tex2D.Apply(); + AssetDatabase.CreateAsset(tex2D, texPath); + AssetDatabase.SaveAssets(); + AssetDatabase.Refresh(); + tex = AssetDatabase.LoadAssetAtPath(texPath); + } + Assert.IsNotNull(tex, "Test texture should exist"); + // Verify either _BaseMap or _MainTex holds the texture + bool hasTexture = (mat.HasProperty("_BaseMap") && mat.GetTexture("_BaseMap") == tex) + || (mat.HasProperty("_MainTex") && mat.GetTexture("_MainTex") == tex); + Assert.IsTrue(hasTexture, "Material should reference the assigned texture"); + + // Modify by changing to same texture via alternate alias key + var modifyParams = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject { ["_MainTex"] = texPath } + }; + var modifyRaw = ManageAsset.HandleCommand(modifyParams); + var modifyResult = modifyRaw as JObject ?? JObject.FromObject(modifyRaw); + Assert.IsTrue(modifyResult!.Value("success"), modifyResult.ToString()); + + var mat2 = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat2); + bool hasTexture2 = (mat2.HasProperty("_BaseMap") && mat2.GetTexture("_BaseMap") == tex) + || (mat2.HasProperty("_MainTex") && mat2.GetTexture("_MainTex") == tex); + Assert.IsTrue(hasTexture2, "Material should keep the assigned texture after modify"); + } + finally + { + if (AssetDatabase.LoadAssetAtPath(matPath) != null) + { + AssetDatabase.DeleteAsset(matPath); + } + AssetDatabase.Refresh(); + } + } + + [Test] + public void Test_EndToEnd_PropertyHandling_AllScenarios() + { + // Comprehensive end-to-end test of all 10 property handling scenarios + const string tempDir = "Assets/Temp/LiveTests"; + if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp"); + if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "LiveTests"); + + string guidSuffix = Guid.NewGuid().ToString("N").Substring(0, 8); + string matPath = $"{tempDir}/Mat_{guidSuffix}.mat"; + string texPath = $"{tempDir}/TempBaseTex.asset"; + string sphereName = $"LiveSphere_{guidSuffix}"; + string badJsonPath = $"{tempDir}/BadJson_{guidSuffix}.mat"; + + // Ensure clean state from previous runs + var preSphere = GameObject.Find(sphereName); + if (preSphere != null) UnityEngine.Object.DestroyImmediate(preSphere); + if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); + if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) AssetDatabase.DeleteAsset(badJsonPath); + AssetDatabase.Refresh(); + + try + { + // 1. Create material via JSON string + var createParams = new JObject + { + ["action"] = "create", + ["path"] = matPath, + ["assetType"] = "Material", + ["properties"] = "{\"shader\":\"Universal Render Pipeline/Lit\",\"color\":[1,0,0,1]}" + }; + var createRaw = ManageAsset.HandleCommand(createParams); + var createResult = createRaw as JObject ?? JObject.FromObject(createRaw); + Assert.IsTrue(createResult.Value("success"), $"Test 1 failed: {createResult}"); + var mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.IsNotNull(mat, "Material should be created"); + var expectedRed = Color.red; + if (mat.HasProperty("_BaseColor")) + Assert.AreEqual(expectedRed, mat.GetColor("_BaseColor"), "Test 1: _BaseColor should be red"); + else if (mat.HasProperty("_Color")) + Assert.AreEqual(expectedRed, mat.GetColor("_Color"), "Test 1: _Color should be red"); + else + Assert.Inconclusive("Material has neither _BaseColor nor _Color"); + + // 2. Modify color and metallic (friendly names) + var modify1 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"color\":[0,0.5,1,1],\"metallic\":0.6}" + }; + var modifyRaw1 = ManageAsset.HandleCommand(modify1); + var modifyResult1 = modifyRaw1 as JObject ?? JObject.FromObject(modifyRaw1); + Assert.IsTrue(modifyResult1.Value("success"), $"Test 2 failed: {modifyResult1}"); + mat = AssetDatabase.LoadAssetAtPath(matPath); + var expectedCyan = new Color(0, 0.5f, 1, 1); + if (mat.HasProperty("_BaseColor")) + Assert.AreEqual(expectedCyan, mat.GetColor("_BaseColor"), "Test 2: _BaseColor should be cyan"); + else if (mat.HasProperty("_Color")) + Assert.AreEqual(expectedCyan, mat.GetColor("_Color"), "Test 2: _Color should be cyan"); + else + Assert.Inconclusive("Material has neither _BaseColor nor _Color"); + Assert.AreEqual(0.6f, mat.GetFloat("_Metallic"), 0.001f, "Test 2: Metallic should be 0.6"); + + // 3. Modify using structured float block + var modify2 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject + { + ["float"] = new JObject { ["name"] = "_Metallic", ["value"] = 0.1 } + } + }; + var modifyRaw2 = ManageAsset.HandleCommand(modify2); + var modifyResult2 = modifyRaw2 as JObject ?? JObject.FromObject(modifyRaw2); + Assert.IsTrue(modifyResult2.Value("success"), $"Test 3 failed: {modifyResult2}"); + mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.AreEqual(0.1f, mat.GetFloat("_Metallic"), 0.001f, "Test 3: Metallic should be 0.1"); + + // 4. Assign texture via direct prop alias (skip if texture doesn't exist) + if (AssetDatabase.LoadAssetAtPath(texPath) != null) + { + var modify3 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"_BaseMap\":\"" + texPath + "\"}" + }; + var modifyRaw3 = ManageAsset.HandleCommand(modify3); + var modifyResult3 = modifyRaw3 as JObject ?? JObject.FromObject(modifyRaw3); + Assert.IsTrue(modifyResult3.Value("success"), $"Test 4 failed: {modifyResult3}"); + Debug.Log("Test 4: Texture assignment successful"); + } + else + { + Debug.LogWarning("Test 4: Skipped - texture not found at " + texPath); + } + + // 5. Assign texture via structured block (skip if texture doesn't exist) + if (AssetDatabase.LoadAssetAtPath(texPath) != null) + { + var modify4 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject + { + ["texture"] = new JObject { ["name"] = "_MainTex", ["path"] = texPath } + } + }; + var modifyRaw4 = ManageAsset.HandleCommand(modify4); + var modifyResult4 = modifyRaw4 as JObject ?? JObject.FromObject(modifyRaw4); + Assert.IsTrue(modifyResult4.Value("success"), $"Test 5 failed: {modifyResult4}"); + Debug.Log("Test 5: Structured texture assignment successful"); + } + else + { + Debug.LogWarning("Test 5: Skipped - texture not found at " + texPath); + } + + // 6. Create sphere and assign material via componentProperties JSON string + var createSphere = new JObject + { + ["action"] = "create", + ["name"] = sphereName, + ["primitiveType"] = "Sphere" + }; + var sphereRaw = ManageGameObject.HandleCommand(createSphere); + var sphereResult = sphereRaw as JObject ?? JObject.FromObject(sphereRaw); + Assert.IsTrue(sphereResult.Value("success"), $"Test 6 - Create sphere failed: {sphereResult}"); + + var modifySphere = new JObject + { + ["action"] = "modify", + ["target"] = sphereName, + ["searchMethod"] = "by_name", + ["componentProperties"] = "{\"MeshRenderer\":{\"sharedMaterial\":\"" + matPath + "\"}}" + }; + var sphereModifyRaw = ManageGameObject.HandleCommand(modifySphere); + var sphereModifyResult = sphereModifyRaw as JObject ?? JObject.FromObject(sphereModifyRaw); + Assert.IsTrue(sphereModifyResult.Value("success"), $"Test 6 - Assign material failed: {sphereModifyResult}"); + var sphere = GameObject.Find(sphereName); + Assert.IsNotNull(sphere, "Test 6: Sphere should exist"); + var renderer = sphere.GetComponent(); + Assert.IsNotNull(renderer.sharedMaterial, "Test 6: Material should be assigned"); + + // 7. Use URP color alias key + var modify5 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject + { + ["_BaseColor"] = new JArray(0.2, 0.8, 0.3, 1) + } + }; + var modifyRaw5 = ManageAsset.HandleCommand(modify5); + var modifyResult5 = modifyRaw5 as JObject ?? JObject.FromObject(modifyRaw5); + Assert.IsTrue(modifyResult5.Value("success"), $"Test 7 failed: {modifyResult5}"); + mat = AssetDatabase.LoadAssetAtPath(matPath); + Color expectedColor = new Color(0.2f, 0.8f, 0.3f, 1f); + if (mat.HasProperty("_BaseColor")) + { + Assert.AreEqual(expectedColor, mat.GetColor("_BaseColor"), "Test 7: _BaseColor should be set"); + } + else if (mat.HasProperty("_Color")) + { + Assert.AreEqual(expectedColor, mat.GetColor("_Color"), "Test 7: Fallback _Color should be set"); + } + + // 8. Invalid JSON should warn (don't fail) + var invalidJson = new JObject + { + ["action"] = "create", + ["path"] = badJsonPath, + ["assetType"] = "Material", + ["properties"] = "{\"invalid\": json, \"missing\": quotes}" + }; + LogAssert.Expect(LogType.Warning, new Regex("(failed to parse)|(Could not parse 'properties' JSON string)", RegexOptions.IgnoreCase)); + var invalidRaw = ManageAsset.HandleCommand(invalidJson); + var invalidResult = invalidRaw as JObject ?? JObject.FromObject(invalidRaw); + // Should either succeed with defaults or fail gracefully + Assert.IsNotNull(invalidResult, "Test 8: Should return a result"); + Debug.Log($"Test 8: Invalid JSON handled - {invalidResult!["success"]}"); + + // 9. Switch shader pipeline dynamically + var modify6 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = "{\"shader\":\"Standard\",\"color\":[1,1,0,1]}" + }; + var modifyRaw6 = ManageAsset.HandleCommand(modify6); + var modifyResult6 = modifyRaw6 as JObject ?? JObject.FromObject(modifyRaw6); + Assert.IsTrue(modifyResult6.Value("success"), $"Test 9 failed: {modifyResult6}"); + mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.AreEqual("Standard", mat.shader.name, "Test 9: Shader should be Standard"); + var c9 = mat.GetColor("_Color"); + Assert.IsTrue(Mathf.Abs(c9.r - 1f) < 0.02f && Mathf.Abs(c9.g - 1f) < 0.02f && Mathf.Abs(c9.b - 0f) < 0.02f, "Test 9: Color should be near yellow"); + + // 10. Mixed friendly and alias keys in one go + var modify7 = new JObject + { + ["action"] = "modify", + ["path"] = matPath, + ["properties"] = new JObject + { + ["metallic"] = 0.8, + ["smoothness"] = 0.3, + ["albedo"] = texPath // Texture path if exists + } + }; + var modifyRaw7 = ManageAsset.HandleCommand(modify7); + var modifyResult7 = modifyRaw7 as JObject ?? JObject.FromObject(modifyRaw7); + Assert.IsTrue(modifyResult7.Value("success"), $"Test 10 failed: {modifyResult7}"); + mat = AssetDatabase.LoadAssetAtPath(matPath); + Assert.AreEqual(0.8f, mat.GetFloat("_Metallic"), 0.001f, "Test 10: Metallic should be 0.8"); + Assert.AreEqual(0.3f, mat.GetFloat("_Glossiness"), 0.001f, "Test 10: Smoothness should be 0.3"); + + Debug.Log("All 10 end-to-end property handling tests completed successfully!"); + } + finally + { + // Cleanup + var sphere = GameObject.Find(sphereName); + if (sphere != null) UnityEngine.Object.DestroyImmediate(sphere); + if (AssetDatabase.LoadAssetAtPath(matPath) != null) AssetDatabase.DeleteAsset(matPath); + if (AssetDatabase.LoadAssetAtPath(badJsonPath) != null) AssetDatabase.DeleteAsset(badJsonPath); + AssetDatabase.Refresh(); + } + } + } } \ No newline at end of file diff --git a/TestProjects/UnityMCPTests/Packages/manifest.json b/TestProjects/UnityMCPTests/Packages/manifest.json index 8710d5b3..0bd78a67 100644 --- a/TestProjects/UnityMCPTests/Packages/manifest.json +++ b/TestProjects/UnityMCPTests/Packages/manifest.json @@ -1,6 +1,6 @@ { "dependencies": { - "com.coplaydev.unity-mcp": "https://github.com/coplaydev/unity-mcp.git?path=MCPForUnity", + "com.coplaydev.unity-mcp": "file:../../../MCPForUnity", "com.unity.ai.navigation": "1.1.4", "com.unity.collab-proxy": "2.5.2", "com.unity.feature.development": "1.0.1", diff --git a/TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt b/TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt index 105db72a..1a62a673 100644 --- a/TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt +++ b/TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt @@ -1,2 +1,2 @@ -m_EditorVersion: 2022.3.6f1 -m_EditorVersionWithRevision: 2022.3.6f1 (b9e6e7e9fa2d) +m_EditorVersion: 2021.3.45f2 +m_EditorVersionWithRevision: 2021.3.45f2 (88f88f591b2e) diff --git a/tests/test_json_parsing_simple.py b/tests/test_json_parsing_simple.py new file mode 100644 index 00000000..5ee8bc54 --- /dev/null +++ b/tests/test_json_parsing_simple.py @@ -0,0 +1,112 @@ +""" +Simple tests for JSON string parameter parsing logic. +Tests the core JSON parsing functionality without MCP server dependencies. +""" +import json +import pytest + + +def parse_properties_json(properties): + """ + Test the JSON parsing logic that would be used in manage_asset. + This simulates the core parsing functionality. + """ + if isinstance(properties, str): + try: + parsed = json.loads(properties) + return parsed, "success" + except json.JSONDecodeError as e: + return properties, f"failed to parse: {e}" + return properties, "no_parsing_needed" + + +class TestJsonParsingLogic: + """Test the core JSON parsing logic.""" + + def test_valid_json_string_parsing(self): + """Test that valid JSON strings are correctly parsed.""" + json_string = '{"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]}' + + result, status = parse_properties_json(json_string) + + assert status == "success" + assert isinstance(result, dict) + assert result["shader"] == "Universal Render Pipeline/Lit" + assert result["color"] == [0, 0, 1, 1] + + def test_invalid_json_string_handling(self): + """Test that invalid JSON strings are handled gracefully.""" + invalid_json = '{"invalid": json, "missing": quotes}' + + result, status = parse_properties_json(invalid_json) + + assert "failed to parse" in status + assert result == invalid_json # Original string returned + + def test_dict_input_unchanged(self): + """Test that dict inputs are passed through unchanged.""" + original_dict = {"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]} + + result, status = parse_properties_json(original_dict) + + assert status == "no_parsing_needed" + assert result == original_dict + + def test_none_input_handled(self): + """Test that None input is handled correctly.""" + result, status = parse_properties_json(None) + + assert status == "no_parsing_needed" + assert result is None + + def test_complex_json_parsing(self): + """Test parsing of complex JSON with nested objects and arrays.""" + complex_json = ''' + { + "shader": "Universal Render Pipeline/Lit", + "color": [1, 0, 0, 1], + "float": {"name": "_Metallic", "value": 0.5}, + "texture": {"name": "_MainTex", "path": "Assets/Textures/Test.png"} + } + ''' + + result, status = parse_properties_json(complex_json) + + assert status == "success" + assert isinstance(result, dict) + assert result["shader"] == "Universal Render Pipeline/Lit" + assert result["color"] == [1, 0, 0, 1] + assert result["float"]["name"] == "_Metallic" + assert result["float"]["value"] == 0.5 + assert result["texture"]["name"] == "_MainTex" + assert result["texture"]["path"] == "Assets/Textures/Test.png" + + def test_empty_json_string(self): + """Test handling of empty JSON string.""" + empty_json = "{}" + + result, status = parse_properties_json(empty_json) + + assert status == "success" + assert isinstance(result, dict) + assert len(result) == 0 + + def test_malformed_json_edge_cases(self): + """Test various malformed JSON edge cases.""" + test_cases = [ + '{"incomplete": }', + '{"missing": "quote}', + '{"trailing": "comma",}', + '{"unclosed": [1, 2, 3}', + 'not json at all', + '{"nested": {"broken": }' + ] + + for malformed_json in test_cases: + result, status = parse_properties_json(malformed_json) + assert "failed to parse" in status + assert result == malformed_json + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tests/test_manage_asset_json_parsing.py b/tests/test_manage_asset_json_parsing.py new file mode 100644 index 00000000..96e51ec9 --- /dev/null +++ b/tests/test_manage_asset_json_parsing.py @@ -0,0 +1,143 @@ +""" +Tests for JSON string parameter parsing in manage_asset tool. +""" +import pytest +import json +from unittest.mock import Mock, AsyncMock +from tools.manage_asset import manage_asset + + +class TestManageAssetJsonParsing: + """Test JSON string parameter parsing functionality.""" + + @pytest.mark.asyncio + async def test_properties_json_string_parsing(self, monkeypatch): + """Test that JSON string properties are correctly parsed to dict.""" + # Mock context + ctx = Mock() + ctx.info = Mock() + ctx.warning = Mock() + + # Patch Unity transport + async def fake_async(cmd, params, loop=None): + return {"success": True, "message": "Asset created successfully", "data": {"path": "Assets/Test.mat"}} + monkeypatch.setattr("tools.manage_asset.async_send_command_with_retry", fake_async) + + # Test with JSON string properties + result = await manage_asset( + ctx=ctx, + action="create", + path="Assets/Test.mat", + asset_type="Material", + properties='{"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]}' + ) + + # Verify JSON parsing was logged + ctx.info.assert_any_call("manage_asset: coerced properties from JSON string to dict") + + # Verify the result + assert result["success"] is True + assert "Asset created successfully" in result["message"] + + @pytest.mark.asyncio + async def test_properties_invalid_json_string(self, monkeypatch): + """Test handling of invalid JSON string properties.""" + ctx = Mock() + ctx.info = Mock() + ctx.warning = Mock() + + async def fake_async(cmd, params, loop=None): + return {"success": True, "message": "Asset created successfully"} + monkeypatch.setattr("tools.manage_asset.async_send_command_with_retry", fake_async) + + # Test with invalid JSON string + result = await manage_asset( + ctx=ctx, + action="create", + path="Assets/Test.mat", + asset_type="Material", + properties='{"invalid": json, "missing": quotes}' + ) + + # Verify behavior: no coercion log for invalid JSON; warning may be emitted by some runtimes + assert not any("coerced properties" in str(c) for c in ctx.info.call_args_list) + assert result.get("success") is True + + @pytest.mark.asyncio + async def test_properties_dict_unchanged(self, monkeypatch): + """Test that dict properties are passed through unchanged.""" + ctx = Mock() + ctx.info = Mock() + + async def fake_async(cmd, params, loop=None): + return {"success": True, "message": "Asset created successfully"} + monkeypatch.setattr("tools.manage_asset.async_send_command_with_retry", fake_async) + + # Test with dict properties + properties_dict = {"shader": "Universal Render Pipeline/Lit", "color": [0, 0, 1, 1]} + + result = await manage_asset( + ctx=ctx, + action="create", + path="Assets/Test.mat", + asset_type="Material", + properties=properties_dict + ) + + # Verify no JSON parsing was attempted (allow initial Processing log) + assert not any("coerced properties" in str(c) for c in ctx.info.call_args_list) + assert result["success"] is True + + @pytest.mark.asyncio + async def test_properties_none_handling(self, monkeypatch): + """Test that None properties are handled correctly.""" + ctx = Mock() + ctx.info = Mock() + + async def fake_async(cmd, params, loop=None): + return {"success": True, "message": "Asset created successfully"} + monkeypatch.setattr("tools.manage_asset.async_send_command_with_retry", fake_async) + + # Test with None properties + result = await manage_asset( + ctx=ctx, + action="create", + path="Assets/Test.mat", + asset_type="Material", + properties=None + ) + + # Verify no JSON parsing was attempted (allow initial Processing log) + assert not any("coerced properties" in str(c) for c in ctx.info.call_args_list) + assert result["success"] is True + + +class TestManageGameObjectJsonParsing: + """Test JSON string parameter parsing for manage_gameobject tool.""" + + @pytest.mark.asyncio + async def test_component_properties_json_string_parsing(self, monkeypatch): + """Test that JSON string component_properties are correctly parsed.""" + from tools.manage_gameobject import manage_gameobject + + ctx = Mock() + ctx.info = Mock() + ctx.warning = Mock() + + def fake_send(cmd, params): + return {"success": True, "message": "GameObject created successfully"} + monkeypatch.setattr("tools.manage_gameobject.send_command_with_retry", fake_send) + + # Test with JSON string component_properties + result = manage_gameobject( + ctx=ctx, + action="create", + name="TestObject", + component_properties='{"MeshRenderer": {"material": "Assets/Materials/BlueMaterial.mat"}}' + ) + + # Verify JSON parsing was logged + ctx.info.assert_called_with("manage_gameobject: coerced component_properties from JSON string to dict") + + # Verify the result + assert result["success"] is True diff --git a/tests/test_script_tools.py b/tests/test_script_tools.py index 0aefa27b..43255722 100644 --- a/tests/test_script_tools.py +++ b/tests/test_script_tools.py @@ -178,9 +178,12 @@ async def fake_async(cmd, params, loop=None): return {"success": True} # Patch the async function in the tools module - import tools.manage_asset - monkeypatch.setattr(tools.manage_asset, + import tools.manage_asset as tools_manage_asset + # Patch both at the module and at the function closure location + monkeypatch.setattr(tools_manage_asset, "async_send_command_with_retry", fake_async) + # Also patch the globals of the function object (handles dynamically loaded module alias) + manage_asset.__globals__["async_send_command_with_retry"] = fake_async async def run(): resp = await manage_asset(