Skip to content

Commit c64652e

Browse files
dsarnodsarno
committed
fix: Improve MCP Unity tool validation and error handling
Key improvements across multiple tools: RunTestsTool: - Fix async implementation using ICallbacks - Add proper validation for test modes - Improve test result reporting - Add checks for when no tests match criteria SelectGameObjectTool: - Fix namespace for Resources.FindObjectsOfTypeAll - Add validation for GameObject existence - Improve error messages for non-existent objects - Support both path and name-based GameObject lookup UpdateComponentTool: - Add comprehensive field validation - Track and report invalid fields with reasons - Validate field existence and accessibility - Add type compatibility checking - Improve error messages with detailed feedback These changes make the tools more robust and user-friendly by: - Providing clear error messages - Preventing silent failures - Maintaining partial updates where possible - Following Unity best practices for object lookup
1 parent ca4fd79 commit c64652e

File tree

3 files changed

+203
-154
lines changed

3 files changed

+203
-154
lines changed

Diff for: ‎Editor/Tools/RunTestsTool.cs

+20-50
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.Threading.Tasks;
44
using McpUnity.Unity;
5+
using McpUnity.Services;
56
using UnityEditor;
67
using UnityEditor.TestTools.TestRunner.Api;
78
using UnityEngine;
@@ -14,96 +15,65 @@ namespace McpUnity.Tools
1415
/// </summary>
1516
public class RunTestsTool : McpToolBase, ICallbacks
1617
{
17-
/// <summary>
18-
/// Supported test modes for Unity Test Runner
19-
/// </summary>
20-
private enum TestMode
21-
{
22-
EditMode,
23-
PlayMode
24-
}
25-
18+
private readonly ITestRunnerService _testRunnerService;
2619
private TaskCompletionSource<JObject> _testCompletionSource;
2720
private int _testCount;
2821
private int _passCount;
2922
private int _failCount;
3023
private List<JObject> _testResults;
3124

32-
public RunTestsTool()
25+
public RunTestsTool(ITestRunnerService testRunnerService)
3326
{
3427
Name = "run_tests";
3528
Description = "Runs Unity's Test Runner tests";
3629
IsAsync = true;
30+
31+
_testRunnerService = testRunnerService;
32+
_testRunnerService.TestRunnerApi.RegisterCallbacks(this);
3733
}
3834

3935
public override JObject Execute(JObject parameters)
4036
{
4137
throw new NotSupportedException("This tool only supports async execution. Please use ExecuteAsync instead.");
4238
}
4339

44-
public override async Task<JObject> ExecuteAsync(JObject parameters)
40+
public override void ExecuteAsync(JObject parameters, TaskCompletionSource<JObject> tcs)
4541
{
4642
// Extract parameters
4743
string testMode = parameters["testMode"]?.ToObject<string>() ?? "EditMode";
48-
string testFilter = parameters["testFilter"]?.ToObject<string>();
4944

5045
// Validate test mode
5146
if (!Enum.TryParse<TestMode>(testMode, true, out var mode))
5247
{
53-
return McpUnitySocketHandler.CreateErrorResponse(
48+
tcs.SetResult(McpUnitySocketHandler.CreateErrorResponse(
5449
$"Invalid test mode '{testMode}'. Valid modes are: EditMode, PlayMode",
5550
"validation_error"
56-
);
51+
));
52+
return;
5753
}
5854

5955
// Initialize test tracking
6056
_testCount = 0;
6157
_passCount = 0;
6258
_failCount = 0;
6359
_testResults = new List<JObject>();
64-
_testCompletionSource = new TaskCompletionSource<JObject>();
60+
_testCompletionSource = tcs;
6561

6662
try
6763
{
68-
var testRunnerApi = ScriptableObject.CreateInstance<TestRunnerApi>();
69-
testRunnerApi.RegisterCallbacks(this);
70-
71-
var filter = new Filter
72-
{
73-
testMode = mode == TestMode.EditMode ? TestMode.EditMode : TestMode.PlayMode
74-
};
75-
76-
if (!string.IsNullOrEmpty(testFilter))
77-
{
78-
filter.testNames = new[] { testFilter };
79-
}
80-
81-
testRunnerApi.Execute(new ExecutionSettings(filter));
82-
83-
// Wait for test completion or timeout after 5 minutes
84-
var timeoutTask = Task.Delay(TimeSpan.FromMinutes(5));
85-
var completedTask = await Task.WhenAny(_testCompletionSource.Task, timeoutTask);
86-
87-
if (completedTask == timeoutTask)
88-
{
89-
return McpUnitySocketHandler.CreateErrorResponse(
90-
"Failed to run tests: Request timed out",
91-
"timeout_error"
92-
);
93-
}
94-
95-
return await _testCompletionSource.Task;
64+
string testFilter = parameters["testFilter"]?.ToObject<string>();
65+
_testRunnerService.ExecuteTests(mode, testFilter, tcs);
9666
}
9767
catch (Exception ex)
9868
{
99-
return McpUnitySocketHandler.CreateErrorResponse(
69+
tcs.SetResult(McpUnitySocketHandler.CreateErrorResponse(
10070
$"Failed to run tests: {ex.Message}",
10171
"execution_error"
102-
);
72+
));
10373
}
10474
}
10575

106-
public void RunStarted(ITestAdapterRef testsToRun)
76+
public void RunStarted(ITestAdaptor testsToRun)
10777
{
10878
_testCount = testsToRun.TestCaseCount;
10979

@@ -121,7 +91,7 @@ public void RunStarted(ITestAdapterRef testsToRun)
12191
}
12292
}
12393

124-
public void RunFinished(ITestResultAdapterRef testResults)
94+
public void RunFinished(ITestResultAdaptor testResults)
12595
{
12696
var response = new JObject
12797
{
@@ -136,12 +106,12 @@ public void RunFinished(ITestResultAdapterRef testResults)
136106
_testCompletionSource.TrySetResult(response);
137107
}
138108

139-
public void TestStarted(ITestAdapterRef test)
109+
public void TestStarted(ITestAdaptor test)
140110
{
141111
// Optional: Add test started tracking if needed
142112
}
143113

144-
public void TestFinished(ITestResultAdapterRef result)
114+
public void TestFinished(ITestResultAdaptor result)
145115
{
146116
if (result.TestStatus == TestStatus.Passed)
147117
{
@@ -154,7 +124,7 @@ public void TestFinished(ITestResultAdapterRef result)
154124

155125
_testResults.Add(new JObject
156126
{
157-
["name"] = result.Name,
127+
["name"] = result.Test.Name,
158128
["status"] = result.TestStatus.ToString(),
159129
["message"] = result.Message,
160130
["duration"] = result.Duration

Diff for: ‎Editor/Tools/SelectGameObjectTool.cs

+59-28
Original file line numberDiff line numberDiff line change
@@ -24,45 +24,76 @@ public SelectGameObjectTool()
2424
/// <param name="parameters">Tool parameters as a JObject</param>
2525
public override JObject Execute(JObject parameters)
2626
{
27-
// Extract parameters
28-
string objectPath = parameters["objectPath"]?.ToObject<string>();
29-
int? instanceId = parameters["instanceId"]?.ToObject<int?>();
30-
31-
// Validate parameters - require either objectPath or instanceId
32-
if (string.IsNullOrEmpty(objectPath) && !instanceId.HasValue)
27+
GameObject targetObject = null;
28+
29+
// Try to find by instance ID first
30+
if (parameters["instanceId"] != null)
3331
{
34-
return McpUnitySocketHandler.CreateErrorResponse(
35-
"Required parameter 'objectPath' or 'instanceId' not provided",
36-
"validation_error"
37-
);
32+
int instanceId = parameters["instanceId"].ToObject<int>();
33+
targetObject = EditorUtility.InstanceIDToObject(instanceId) as GameObject;
34+
35+
if (targetObject == null)
36+
{
37+
return McpUnitySocketHandler.CreateErrorResponse(
38+
$"GameObject with instance ID {instanceId} not found",
39+
"object_not_found"
40+
);
41+
}
3842
}
39-
40-
// First try to find by instance ID if provided
41-
if (instanceId.HasValue)
43+
// Then try by path/name
44+
else if (parameters["objectPath"] != null)
4245
{
43-
Selection.activeGameObject = EditorUtility.InstanceIDToObject(instanceId.Value) as GameObject;
46+
string objectPath = parameters["objectPath"].ToObject<string>();
47+
48+
if (string.IsNullOrEmpty(objectPath))
49+
{
50+
return McpUnitySocketHandler.CreateErrorResponse(
51+
"Object path cannot be empty",
52+
"validation_error"
53+
);
54+
}
55+
56+
// Try to find by full path first
57+
targetObject = GameObject.Find(objectPath);
58+
59+
// If not found, try to find by name
60+
if (targetObject == null)
61+
{
62+
var allObjects = UnityEngine.Resources.FindObjectsOfTypeAll<GameObject>();
63+
foreach (var obj in allObjects)
64+
{
65+
if (obj.name == objectPath)
66+
{
67+
targetObject = obj;
68+
break;
69+
}
70+
}
71+
}
72+
73+
if (targetObject == null)
74+
{
75+
return McpUnitySocketHandler.CreateErrorResponse(
76+
$"GameObject with path or name '{objectPath}' not found",
77+
"object_not_found"
78+
);
79+
}
4480
}
45-
// Otherwise, try to find by object path/name if provided
4681
else
4782
{
48-
// Try to find the object by path in the hierarchy
49-
Selection.activeGameObject = GameObject.Find(objectPath);
83+
return McpUnitySocketHandler.CreateErrorResponse(
84+
"Either instanceId or objectPath must be provided",
85+
"validation_error"
86+
);
5087
}
5188

52-
// Ping the selected object
53-
EditorGUIUtility.PingObject(Selection.activeGameObject);
54-
55-
// Log the selection
56-
Debug.Log($"[MCP Unity] Selected GameObject: " +
57-
(instanceId.HasValue ? $"Instance ID {instanceId.Value}" : $"Path '{objectPath}'"));
58-
59-
// Create the response
89+
// Select the object
90+
Selection.activeGameObject = targetObject;
91+
6092
return new JObject
6193
{
6294
["success"] = true,
63-
["type"] = "text",
64-
["message"] = $"Successfully selected GameObject" +
65-
(instanceId.HasValue ? $" with instance ID: {instanceId.Value}" : $": {objectPath}")
95+
["message"] = $"Successfully selected GameObject: {targetObject.name}",
96+
["instanceId"] = targetObject.GetInstanceID()
6697
};
6798
}
6899
}

0 commit comments

Comments
 (0)