Skip to content

Commit dbd5cf6

Browse files
dsarnodsarno
committed
refactor: extract JSON coercion helper; docs + exception narrowing\nfix: fail fast on bad component_properties JSON\ntest: unify setup, avoid test-to-test calls\nchore: use relative MCP package path
1 parent 1bdbeb8 commit dbd5cf6

File tree

7 files changed

+51
-45
lines changed

7 files changed

+51
-45
lines changed

‎MCPForUnity/Editor/Tools/JsonUtil.cs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
using Newtonsoft.Json.Linq;
2+
using UnityEngine;
3+
4+
namespace MCPForUnity.Editor.Tools
5+
{
6+
internal static class JsonUtil
7+
{
8+
/// <summary>
9+
/// If @params[paramName] is a JSON string, parse it to a JObject in-place.
10+
/// Logs a warning on parse failure and leaves the original value.
11+
/// </summary>
12+
internal static void CoerceJsonStringParameter(JObject @params, string paramName)
13+
{
14+
if (@params == null || string.IsNullOrEmpty(paramName)) return;
15+
var token = @params[paramName];
16+
if (token != null && token.Type == JTokenType.String)
17+
{
18+
try
19+
{
20+
var parsed = JObject.Parse(token.ToString());
21+
@params[paramName] = parsed;
22+
}
23+
catch (Newtonsoft.Json.JsonReaderException e)
24+
{
25+
Debug.LogWarning($"[MCP] Could not parse '{paramName}' JSON string: {e.Message}");
26+
}
27+
}
28+
}
29+
}
30+
}
31+
32+

‎MCPForUnity/Editor/Tools/ManageAsset.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,7 @@ public static object HandleCommand(JObject @params)
6464
string path = @params["path"]?.ToString();
6565

6666
// Coerce string JSON to JObject for 'properties' if provided as a JSON string
67-
var propertiesToken = @params["properties"];
68-
if (propertiesToken != null && propertiesToken.Type == JTokenType.String)
69-
{
70-
try
71-
{
72-
var parsed = JObject.Parse(propertiesToken.ToString());
73-
@params["properties"] = parsed;
74-
}
75-
catch (Exception e)
76-
{
77-
Debug.LogWarning($"[ManageAsset] Could not parse 'properties' JSON string: {e.Message}");
78-
}
79-
}
67+
JsonUtil.CoerceJsonStringParameter(@params, "properties");
8068

8169
try
8270
{

‎MCPForUnity/Editor/Tools/ManageGameObject.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -67,19 +67,7 @@ public static object HandleCommand(JObject @params)
6767
// --- End add parameter ---
6868

6969
// Coerce string JSON to JObject for 'componentProperties' if provided as a JSON string
70-
var componentPropsToken = @params["componentProperties"];
71-
if (componentPropsToken != null && componentPropsToken.Type == JTokenType.String)
72-
{
73-
try
74-
{
75-
var parsed = JObject.Parse(componentPropsToken.ToString());
76-
@params["componentProperties"] = parsed;
77-
}
78-
catch (Exception e)
79-
{
80-
Debug.LogWarning($"[ManageGameObject] Could not parse 'componentProperties' JSON string: {e.Message}");
81-
}
82-
}
70+
JsonUtil.CoerceJsonStringParameter(@params, "componentProperties");
8371

8472
// --- Prefab Redirection Check ---
8573
string targetPath =

‎MCPForUnity/UnityMcpServer~/src/tools/manage_asset.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ async def manage_asset(
3939
try:
4040
properties = json.loads(properties)
4141
ctx.info("manage_asset: coerced properties from JSON string to dict")
42-
except Exception as e:
42+
except json.JSONDecodeError as e:
4343
ctx.warn(f"manage_asset: failed to parse properties JSON string: {e}")
4444
# Leave properties as-is; Unity side may handle defaults
4545
# Ensure properties is a dict if None

‎MCPForUnity/UnityMcpServer~/src/tools/manage_gameobject.py‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ def manage_gameobject(
4545
"List of component names to remove"] | None = None,
4646
component_properties: Annotated[dict[str, dict[str, Any]] | str,
4747
"""Dictionary of component names to their properties to set. For example:
48+
Can also be provided as a JSON string representation of the dict.
4849
`{"MyScript": {"otherObject": {"find": "Player", "method": "by_name"}}}` assigns GameObject
4950
`{"MyScript": {"playerHealth": {"find": "Player", "component": "HealthComponent"}}}` assigns Component
5051
Example set nested property:
@@ -119,8 +120,8 @@ def _to_vec3(parts):
119120
try:
120121
component_properties = json.loads(component_properties)
121122
ctx.info("manage_gameobject: coerced component_properties from JSON string to dict")
122-
except Exception as e:
123-
ctx.warn(f"manage_gameobject: failed to parse component_properties JSON string: {e}")
123+
except json.JSONDecodeError as e:
124+
return {"success": False, "message": f"Invalid JSON in component_properties: {e}"}
124125
try:
125126
# Map tag to search_term when search_method is by_tag for backward compatibility
126127
if action == "find" and search_method == "by_tag" and tag is not None and search_term is None:

‎TestProjects/UnityMCPTests/Assets/Tests/EditMode/MCPToolParameterTests.cs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,24 @@ namespace Tests.EditMode
1818
/// </summary>
1919
public class MCPToolParameterTests
2020
{
21-
[Test]
22-
public void Test_ManageAsset_ShouldAcceptJSONProperties()
21+
private const string TempDir = "Assets/Temp/MCPToolParameterTests";
22+
23+
[SetUp]
24+
public void SetUp()
2325
{
24-
// Arrange: create temp folder
25-
const string tempDir = "Assets/Temp/MCPToolParameterTests";
2626
if (!AssetDatabase.IsValidFolder("Assets/Temp"))
2727
{
2828
AssetDatabase.CreateFolder("Assets", "Temp");
2929
}
30-
if (!AssetDatabase.IsValidFolder(tempDir))
30+
if (!AssetDatabase.IsValidFolder(TempDir))
3131
{
3232
AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests");
3333
}
34-
35-
var matPath = $"{tempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
34+
}
35+
[Test]
36+
public void Test_ManageAsset_ShouldAcceptJSONProperties()
37+
{
38+
var matPath = $"{TempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
3639

3740
// Build params with properties as a JSON string (to be coerced)
3841
var p = new JObject
@@ -70,10 +73,7 @@ public void Test_ManageAsset_ShouldAcceptJSONProperties()
7073
[Test]
7174
public void Test_ManageGameObject_ShouldAcceptJSONComponentProperties()
7275
{
73-
const string tempDir = "Assets/Temp/MCPToolParameterTests";
74-
if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp");
75-
if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests");
76-
var matPath = $"{tempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
76+
var matPath = $"{TempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
7777

7878
// Create a material first (object-typed properties)
7979
var createMat = new JObject
@@ -121,10 +121,7 @@ public void Test_ManageGameObject_ShouldAcceptJSONComponentProperties()
121121
[Test]
122122
public void Test_JSONParsing_ShouldWorkInMCPTools()
123123
{
124-
const string tempDir = "Assets/Temp/MCPToolParameterTests";
125-
if (!AssetDatabase.IsValidFolder("Assets/Temp")) AssetDatabase.CreateFolder("Assets", "Temp");
126-
if (!AssetDatabase.IsValidFolder(tempDir)) AssetDatabase.CreateFolder("Assets/Temp", "MCPToolParameterTests");
127-
var matPath = $"{tempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
124+
var matPath = $"{TempDir}/JsonMat_{Guid.NewGuid().ToString("N")}.mat";
128125

129126
// manage_asset with JSON string properties (coercion path)
130127
var createMat = new JObject

‎TestProjects/UnityMCPTests/Packages/manifest.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"dependencies": {
3-
"com.coplaydev.unity-mcp": "file:/Users/davidsarno/unity-mcp/MCPForUnity",
3+
"com.coplaydev.unity-mcp": "file:../../../MCPForUnity",
44
"com.unity.ai.navigation": "1.1.4",
55
"com.unity.collab-proxy": "2.5.2",
66
"com.unity.feature.development": "1.0.1",

0 commit comments

Comments
 (0)