Skip to content

Commit 9b69ff8

Browse files
committed
refactor(unity): Improve server lifecycle, error handling, and logging in the unity code to handle the domain reload issue
1 parent 347ccf7 commit 9b69ff8

File tree

2 files changed

+146
-32
lines changed

2 files changed

+146
-32
lines changed

Editor/UnityBridge/McpUnityServer.cs

Lines changed: 130 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using WebSocketSharp.Server;
1111
using System.IO;
1212
using System.Diagnostics;
13+
using System.Net.Sockets;
1314

1415
namespace McpUnity.Unity
1516
{
@@ -18,7 +19,7 @@ namespace McpUnity.Unity
1819
/// Uses WebSockets to communicate with Node.js.
1920
/// </summary>
2021
[InitializeOnLoad]
21-
public class McpUnityServer
22+
public class McpUnityServer : IDisposable
2223
{
2324
private static McpUnityServer _instance;
2425

@@ -35,15 +36,10 @@ public class McpUnityServer
3536
/// </summary>
3637
static McpUnityServer()
3738
{
38-
// Initialize the singleton instance when Unity loads
39-
// This ensures the bridge is available as soon as Unity starts
40-
EditorApplication.quitting += Instance.StopServer;
41-
42-
// Auto-restart server after domain reload
43-
if (McpUnitySettings.Instance.AutoStartServer)
44-
{
45-
Instance.StartServer();
46-
}
39+
EditorApplication.delayCall += () => {
40+
// Ensure Instance is created and hooks are set up after initial domain load
41+
var currentInstance = Instance;
42+
};
4743
}
4844

4945
/// <summary>
@@ -76,34 +72,71 @@ public static McpUnityServer Instance
7672
/// </summary>
7773
private McpUnityServer()
7874
{
79-
InstallServer();
75+
EditorApplication.quitting -= OnEditorQuitting; // Prevent multiple subscriptions on domain reload
76+
EditorApplication.quitting += OnEditorQuitting;
77+
78+
AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload;
79+
AssemblyReloadEvents.beforeAssemblyReload += OnBeforeAssemblyReload;
80+
81+
AssemblyReloadEvents.afterAssemblyReload -= OnAfterAssemblyReload;
82+
AssemblyReloadEvents.afterAssemblyReload += OnAfterAssemblyReload;
83+
84+
EditorApplication.playModeStateChanged -= OnPlayModeStateChanged;
85+
EditorApplication.playModeStateChanged += OnPlayModeStateChanged;
86+
87+
InstallServer();
8088
InitializeServices();
8189
RegisterResources();
8290
RegisterTools();
91+
92+
// Initial start if auto-start is enabled and not recovering from a reload where it was off
93+
if (McpUnitySettings.Instance.AutoStartServer)
94+
{
95+
StartServer();
96+
}
97+
}
98+
99+
/// <summary>
100+
/// Disposes the McpUnityServer instance, stopping the WebSocket server and unsubscribing from Unity Editor events.
101+
/// This method ensures proper cleanup of resources and prevents memory leaks or unexpected behavior during domain reloads or editor shutdown.
102+
/// </summary>
103+
public void Dispose()
104+
{
105+
StopServer();
106+
107+
EditorApplication.quitting -= OnEditorQuitting;
108+
AssemblyReloadEvents.beforeAssemblyReload -= OnBeforeAssemblyReload;
109+
AssemblyReloadEvents.afterAssemblyReload -= OnAfterAssemblyReload;
110+
EditorApplication.playModeStateChanged -= OnPlayModeStateChanged;
111+
112+
GC.SuppressFinalize(this);
83113
}
84114

85115
/// <summary>
86116
/// Start the WebSocket Server to communicate with Node.js
87117
/// </summary>
88118
public void StartServer()
89119
{
90-
if (IsListening) return;
91-
120+
if (IsListening)
121+
{
122+
McpLogger.LogInfo($"Server start requested, but already listening on port {McpUnitySettings.Instance.Port}.");
123+
return;
124+
}
125+
92126
try
93127
{
94-
// Create a new WebSocket server
95128
_webSocketServer = new WebSocketServer($"ws://localhost:{McpUnitySettings.Instance.Port}");
96-
// Add the MCP service endpoint with a handler that references this server
97129
_webSocketServer.AddWebSocketService("/McpUnity", () => new McpUnitySocketHandler(this));
98-
99-
// Start the server
100130
_webSocketServer.Start();
101-
102-
McpLogger.LogInfo($"WebSocket server started on port {McpUnitySettings.Instance.Port}");
131+
McpLogger.LogInfo($"WebSocket server started successfully on port {McpUnitySettings.Instance.Port}.");
132+
}
133+
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.AddressAlreadyInUse)
134+
{
135+
McpLogger.LogError($"Failed to start WebSocket server: Port {McpUnitySettings.Instance.Port} is already in use. {ex.Message}");
103136
}
104137
catch (Exception ex)
105138
{
106-
McpLogger.LogError($"Failed to start WebSocket server: {ex.Message}");
139+
McpLogger.LogError($"Failed to start WebSocket server: {ex.Message}\n{ex.StackTrace}");
107140
}
108141
}
109142

@@ -112,17 +145,26 @@ public void StartServer()
112145
/// </summary>
113146
public void StopServer()
114147
{
115-
if (!IsListening) return;
116-
148+
if (!IsListening)
149+
{
150+
return;
151+
}
152+
117153
try
118154
{
119-
_webSocketServer?.Stop();
155+
_webSocketServer?.Stop();
120156

121157
McpLogger.LogInfo("WebSocket server stopped");
122158
}
123159
catch (Exception ex)
124160
{
125-
McpLogger.LogError($"Error stopping WebSocket server: {ex.Message}");
161+
McpLogger.LogError($"Error during WebSocketServer.Stop(): {ex.Message}\n{ex.StackTrace}");
162+
}
163+
finally
164+
{
165+
_webSocketServer = null;
166+
Clients.Clear();
167+
McpLogger.LogInfo("WebSocket server stopped and resources cleaned up.");
126168
}
127169
}
128170

@@ -252,5 +294,69 @@ private void InitializeServices()
252294
// Initialize the console logs service
253295
_consoleLogsService = new ConsoleLogsService();
254296
}
297+
298+
/// <summary>
299+
/// Handles the Unity Editor quitting event. Ensures the server is properly stopped and disposed.
300+
/// </summary>
301+
private static void OnEditorQuitting()
302+
{
303+
McpLogger.LogInfo("Editor is quitting. Ensuring server is stopped.");
304+
Instance.Dispose();
305+
}
306+
307+
/// <summary>
308+
/// Handles the Unity Editor's 'before assembly reload' event.
309+
/// Stops the WebSocket server to prevent port conflicts and ensure a clean state before scripts are recompiled.
310+
/// </summary>
311+
private static void OnBeforeAssemblyReload()
312+
{
313+
if (Instance.IsListening)
314+
{
315+
Instance.StopServer();
316+
}
317+
}
318+
319+
/// <summary>
320+
/// Handles the Unity Editor's 'after assembly reload' event.
321+
/// If auto-start is enabled, attempts to restart the WebSocket server if it's not already listening.
322+
/// This ensures the server is operational after script recompilation.
323+
/// </summary>
324+
private static void OnAfterAssemblyReload()
325+
{
326+
if (McpUnitySettings.Instance.AutoStartServer && !Instance.IsListening)
327+
{
328+
Instance.StartServer();
329+
}
330+
}
331+
332+
/// <summary>
333+
/// Handles changes in Unity Editor's play mode state.
334+
/// Stops the server when exiting Edit Mode if configured, and restarts it when entering Play Mode or returning to Edit Mode if auto-start is enabled.
335+
/// </summary>
336+
/// <param name="state">The current play mode state change.</param>
337+
private static void OnPlayModeStateChanged(PlayModeStateChange state)
338+
{
339+
switch (state)
340+
{
341+
case PlayModeStateChange.ExitingEditMode:
342+
// About to enter Play Mode
343+
if (Instance.IsListening)
344+
{
345+
Instance.StopServer();
346+
}
347+
break;
348+
case PlayModeStateChange.EnteredPlayMode:
349+
case PlayModeStateChange.ExitingPlayMode:
350+
// Server is disabled during play mode as domain reload will be triggered again when stopped.
351+
break;
352+
case PlayModeStateChange.EnteredEditMode:
353+
// Returned to Edit Mode
354+
if (!Instance.IsListening && McpUnitySettings.Instance.AutoStartServer)
355+
{
356+
Instance.StartServer();
357+
}
358+
break;
359+
}
360+
}
255361
}
256362
}

Editor/UnityBridge/McpUnitySocketHandler.cs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,19 @@ protected override async void OnMessage(MessageEventArgs e)
5656
try
5757
{
5858
McpLogger.LogInfo($"WebSocket message received: {e.Data}");
59-
60-
var requestJson = JObject.Parse(e.Data);
59+
JObject requestJson;
60+
try
61+
{
62+
requestJson = JObject.Parse(e.Data);
63+
}
64+
catch (JsonReaderException jre)
65+
{
66+
McpLogger.LogError($"Invalid JSON received: {jre.Message}. Data: {e.Data}");
67+
// Attempt to send a parse error response. No requestId is available yet.
68+
Send(CreateResponse(null, CreateErrorResponse($"Invalid JSON format: {jre.Message}", "invalid_json")).ToString(Formatting.None));
69+
return;
70+
}
71+
6172
var method = requestJson["method"]?.ToString();
6273
var parameters = requestJson["params"] as JObject ?? new JObject();
6374
var requestId = requestJson["id"]?.ToString();
@@ -81,14 +92,11 @@ protected override async void OnMessage(MessageEventArgs e)
8192
tcs.SetResult(CreateErrorResponse($"Unknown method: {method}", "unknown_method"));
8293
}
8394

84-
// Wait for the task to complete
8595
JObject responseJson = await tcs.Task;
86-
87-
// Format as JSON-RPC 2.0 response
8896
JObject jsonRpcResponse = CreateResponse(requestId, responseJson);
8997
string responseStr = jsonRpcResponse.ToString(Formatting.None);
9098

91-
McpLogger.LogInfo($"WebSocket message response: {responseStr}");
99+
McpLogger.LogInfo($"WebSocket message response for request ID '{requestId}': {responseStr}");
92100

93101
// Send the response back to the client
94102
Send(responseStr);
@@ -160,7 +168,7 @@ private IEnumerator ExecuteTool(McpToolBase tool, JObject parameters, TaskComple
160168
}
161169
catch (Exception ex)
162170
{
163-
McpLogger.LogError($"Error executing tool {tool.Name}: {ex.Message}");
171+
McpLogger.LogError($"Error executing tool {tool.Name}: {ex.Message}\n{ex.StackTrace}");
164172
tcs.SetResult(CreateErrorResponse(
165173
$"Failed to execute tool {tool.Name}: {ex.Message}",
166174
"tool_execution_error"
@@ -189,7 +197,7 @@ private IEnumerator FetchResourceCoroutine(McpResourceBase resource, JObject par
189197
}
190198
catch (Exception ex)
191199
{
192-
McpLogger.LogError($"Error fetching resource {resource.Name}: {ex}");
200+
McpLogger.LogError($"Error fetching resource {resource.Name}: {ex.Message}\n{ex.StackTrace}");
193201
tcs.SetResult(CreateErrorResponse(
194202
$"Failed to fetch resource {resource.Name}: {ex.Message}",
195203
"resource_fetch_error"

0 commit comments

Comments
 (0)