Skip to content

Commit b57a2ec

Browse files
fix: Changed flag management to EditorPrefs (#408)
* fix: Changed flag management to EditorPrefs * refactor: Improve code readability and error handling in MCP window toggle * fix: Refactor MCP window toggle logic to use new helper methods for better readability and maintainability * fix: Reorder using directives and improve error logging format in MCP window * Address CodeRabbit feedback: use McpLog.Warn and guard against repeated CreateGUI calls --------- Co-authored-by: David Sarno <david@lighthaus.us>
1 parent 0d6c274 commit b57a2ec

File tree

3 files changed

+75
-38
lines changed

3 files changed

+75
-38
lines changed

MCPForUnity/Editor/MenuItems/MCPForUnityMenu.cs

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,42 +5,25 @@
55

66
namespace MCPForUnity.Editor.MenuItems
77
{
8-
/// <summary>
9-
/// Centralized menu items for MCP For Unity
10-
/// </summary>
118
public static class MCPForUnityMenu
129
{
13-
// ========================================
14-
// Main Menu Items
15-
// ========================================
16-
17-
/// <summary>
18-
/// Show the Setup Window
19-
/// </summary>
2010
[MenuItem("Window/MCP For Unity/Setup Window", priority = 1)]
2111
public static void ShowSetupWindow()
2212
{
2313
SetupWindowService.ShowSetupWindow();
2414
}
2515

26-
/// <summary>
27-
/// Toggle the main MCP For Unity window
28-
/// </summary>
2916
[MenuItem("Window/MCP For Unity/Toggle MCP Window %#m", priority = 2)]
3017
public static void ToggleMCPWindow()
3118
{
32-
if (EditorWindow.HasOpenInstances<MCPForUnityEditorWindow>())
19+
if (MCPForUnityEditorWindow.HasAnyOpenWindow())
3320
{
34-
foreach (var window in UnityEngine.Resources.FindObjectsOfTypeAll<MCPForUnityEditorWindow>())
35-
{
36-
window.Close();
37-
}
21+
MCPForUnityEditorWindow.CloseAllOpenWindows();
3822
}
3923
else
4024
{
4125
MCPForUnityEditorWindow.ShowWindow();
4226
}
4327
}
44-
4528
}
4629
}

MCPForUnity/Editor/Setup/SetupWindowService.cs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@ public static class SetupWindowService
1818
{
1919
private const string SETUP_COMPLETED_KEY = EditorPrefKeys.SetupCompleted;
2020
private const string SETUP_DISMISSED_KEY = EditorPrefKeys.SetupDismissed;
21-
private static bool _hasCheckedThisSession = false;
21+
22+
// Use SessionState to persist "checked this editor session" across domain reloads.
23+
// SessionState survives assembly reloads within the same Editor session, which prevents
24+
// the setup window from reappearing after code reloads / playmode transitions.
25+
private const string SessionCheckedKey = "MCPForUnity.SetupWindowCheckedThisEditorSession";
2226

2327
static SetupWindowService()
2428
{
@@ -35,10 +39,12 @@ static SetupWindowService()
3539
/// </summary>
3640
private static void CheckSetupNeeded()
3741
{
38-
if (_hasCheckedThisSession)
42+
// Ensure we only run once per Editor session (survives domain reloads).
43+
// This avoids showing the setup dialog repeatedly when scripts recompile or Play mode toggles.
44+
if (SessionState.GetBool(SessionCheckedKey, false))
3945
return;
4046

41-
_hasCheckedThisSession = true;
47+
SessionState.SetBool(SessionCheckedKey, true);
4248

4349
try
4450
{
@@ -48,11 +54,16 @@ private static void CheckSetupNeeded()
4854
bool userOverrodeHttpUrl = EditorPrefs.HasKey(EditorPrefKeys.HttpBaseUrl);
4955

5056
// In Asset Store builds with a remote default URL (and no user override), skip the local setup wizard.
51-
if (!userOverrodeHttpUrl
57+
if (
58+
!userOverrodeHttpUrl
5259
&& McpDistribution.Settings.skipSetupWindowWhenRemoteDefault
53-
&& McpDistribution.Settings.IsRemoteDefault)
60+
&& McpDistribution.Settings.IsRemoteDefault
61+
)
5462
{
55-
McpLog.Info("Skipping Setup Window because this distribution ships with a hosted MCP URL. Open Window/MCP For Unity/Setup Window if you want to configure a local runtime.", always: false);
63+
McpLog.Info(
64+
"Skipping Setup Window because this distribution ships with a hosted MCP URL. Open Window/MCP For Unity/Setup Window if you want to configure a local runtime.",
65+
always: false
66+
);
5667
return;
5768
}
5869

@@ -66,7 +77,10 @@ private static void CheckSetupNeeded()
6677
}
6778
else
6879
{
69-
McpLog.Info("Setup Window skipped - previously completed or dismissed", always: false);
80+
McpLog.Info(
81+
"Setup Window skipped - previously completed or dismissed",
82+
always: false
83+
);
7084
}
7185
}
7286
catch (Exception ex)
@@ -108,6 +122,5 @@ public static void MarkSetupDismissed()
108122
EditorPrefs.SetBool(SETUP_DISMISSED_KEY, true);
109123
McpLog.Info("Setup marked as dismissed");
110124
}
111-
112125
}
113126
}

MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Threading.Tasks;
4-
using UnityEditor;
5-
using UnityEngine;
6-
using UnityEngine.UIElements;
74
using MCPForUnity.Editor.Helpers;
85
using MCPForUnity.Editor.Services;
9-
using MCPForUnity.Editor.Windows.Components.Settings;
10-
using MCPForUnity.Editor.Windows.Components.Connection;
116
using MCPForUnity.Editor.Windows.Components.ClientConfig;
7+
using MCPForUnity.Editor.Windows.Components.Connection;
8+
using MCPForUnity.Editor.Windows.Components.Settings;
9+
using UnityEditor;
10+
using UnityEngine;
11+
using UnityEngine.UIElements;
1212

1313
namespace MCPForUnity.Editor.Windows
1414
{
@@ -20,14 +20,47 @@ public class MCPForUnityEditorWindow : EditorWindow
2020
private McpClientConfigSection clientConfigSection;
2121

2222
private static readonly HashSet<MCPForUnityEditorWindow> OpenWindows = new();
23+
private bool guiCreated = false;
2324

2425
public static void ShowWindow()
2526
{
2627
var window = GetWindow<MCPForUnityEditorWindow>("MCP For Unity");
2728
window.minSize = new Vector2(500, 600);
2829
}
30+
31+
// Helper to check and manage open windows from other classes
32+
public static bool HasAnyOpenWindow()
33+
{
34+
return OpenWindows.Count > 0;
35+
}
36+
37+
public static void CloseAllOpenWindows()
38+
{
39+
if (OpenWindows.Count == 0)
40+
return;
41+
42+
// Copy to array to avoid modifying the collection while iterating
43+
var arr = new MCPForUnityEditorWindow[OpenWindows.Count];
44+
OpenWindows.CopyTo(arr);
45+
foreach (var window in arr)
46+
{
47+
try
48+
{
49+
window?.Close();
50+
}
51+
catch (Exception ex)
52+
{
53+
McpLog.Warn($"Error closing MCP window: {ex.Message}");
54+
}
55+
}
56+
}
57+
2958
public void CreateGUI()
3059
{
60+
// Guard against repeated CreateGUI calls (e.g., domain reloads)
61+
if (guiCreated)
62+
return;
63+
3164
string basePath = AssetPathUtility.GetMcpPackageRootPath();
3265

3366
// Load main window UXML
@@ -37,7 +70,9 @@ public void CreateGUI()
3770

3871
if (visualTree == null)
3972
{
40-
McpLog.Error($"Failed to load UXML at: {basePath}/Editor/Windows/MCPForUnityEditorWindow.uxml");
73+
McpLog.Error(
74+
$"Failed to load UXML at: {basePath}/Editor/Windows/MCPForUnityEditorWindow.uxml"
75+
);
4176
return;
4277
}
4378

@@ -78,8 +113,10 @@ public void CreateGUI()
78113
var settingsRoot = settingsTree.Instantiate();
79114
sectionsContainer.Add(settingsRoot);
80115
settingsSection = new McpSettingsSection(settingsRoot);
81-
settingsSection.OnGitUrlChanged += () => clientConfigSection?.UpdateManualConfiguration();
82-
settingsSection.OnHttpServerCommandUpdateRequested += () => connectionSection?.UpdateHttpServerCommandDisplay();
116+
settingsSection.OnGitUrlChanged += () =>
117+
clientConfigSection?.UpdateManualConfiguration();
118+
settingsSection.OnHttpServerCommandUpdateRequested += () =>
119+
connectionSection?.UpdateHttpServerCommandDisplay();
83120
}
84121

85122
// Load and initialize Connection section
@@ -91,7 +128,8 @@ public void CreateGUI()
91128
var connectionRoot = connectionTree.Instantiate();
92129
sectionsContainer.Add(connectionRoot);
93130
connectionSection = new McpConnectionSection(connectionRoot);
94-
connectionSection.OnManualConfigUpdateRequested += () => clientConfigSection?.UpdateManualConfiguration();
131+
connectionSection.OnManualConfigUpdateRequested += () =>
132+
clientConfigSection?.UpdateManualConfiguration();
95133
}
96134

97135
// Load and initialize Client Configuration section
@@ -105,6 +143,8 @@ public void CreateGUI()
105143
clientConfigSection = new McpClientConfigSection(clientConfigRoot);
106144
}
107145

146+
guiCreated = true;
147+
108148
// Initial updates
109149
RefreshAllData();
110150
}
@@ -119,6 +159,7 @@ private void OnDisable()
119159
{
120160
EditorApplication.update -= OnEditorUpdate;
121161
OpenWindows.Remove(this);
162+
guiCreated = false;
122163
}
123164

124165
private void OnFocus()
@@ -163,11 +204,11 @@ private void ScheduleHealthCheck()
163204
{
164205
EditorApplication.delayCall += async () =>
165206
{
166-
if (this == null)
207+
if (this == null || connectionSection == null)
167208
{
168209
return;
169210
}
170-
await connectionSection?.VerifyBridgeConnectionAsync();
211+
await connectionSection.VerifyBridgeConnectionAsync();
171212
};
172213
}
173214
}

0 commit comments

Comments
 (0)