Skip to content

Commit 493b11d

Browse files
authored
Merge pull request #1152 from Scriptwonder/fix/restore-per-client-setup-visibility
fix(ui): default the per-client setup foldout to expanded so Unregister is visible
2 parents 2e5a5f3 + ad510eb commit 493b11d

4 files changed

Lines changed: 101 additions & 7 deletions

File tree

MCPForUnity/Editor/Clients/IMcpClientConfigurator.cs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,20 @@ public interface IMcpClientConfigurator
4848
/// <summary>Checks and updates status; returns current status.</summary>
4949
McpStatus CheckStatus(bool attemptAutoRewrite = true);
5050

51-
/// <summary>Runs auto-configuration (register/write file/CLI etc.).</summary>
51+
/// <summary>Runs auto-configuration (register/write file/CLI etc.). Always idempotent
52+
/// — calling twice with the same settings is safe and is what the bulk "Configure All"
53+
/// path relies on to refresh transport / server-version drift across every detected
54+
/// client.</summary>
5255
void Configure();
5356

57+
/// <summary>
58+
/// Removes UnityMCP from this client's config (JSON entry, CLI registration, etc.).
59+
/// Default is a no-op for client types that don't yet implement removal (Codex TOML);
60+
/// callers should treat this as best-effort. The UI's per-client button routes here
61+
/// when <see cref="Status"/> is <see cref="McpStatus.Configured"/>.
62+
/// </summary>
63+
void Unregister();
64+
5465
/// <summary>Returns the manual configuration snippet (JSON/TOML/commands).</summary>
5566
string GetManualSnippet();
5667

MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ protected McpClientConfiguratorBase(McpClient client)
4545
public abstract string GetConfigPath();
4646
public abstract McpStatus CheckStatus(bool attemptAutoRewrite = true);
4747
public abstract void Configure();
48+
49+
/// <summary>Default Unregister is a no-op. Override in JsonFileMcpConfigurator /
50+
/// ClaudeCliMcpConfigurator etc. where removal has a concrete implementation.</summary>
51+
public virtual void Unregister() { }
52+
4853
public abstract string GetManualSnippet();
4954
public abstract IList<string> GetInstallationSteps();
5055

@@ -334,6 +339,9 @@ public override McpStatus CheckStatus(bool attemptAutoRewrite = true)
334339

335340
public override void Configure()
336341
{
342+
// Always idempotent-write. The per-client UI button routes through Unregister
343+
// when the user clicks while the client is already Configured; the bulk
344+
// "Configure All" path calls this directly and expects an unconditional write.
337345
string path = GetConfigPath();
338346
McpConfigurationHelper.EnsureConfigDirectoryExists(path);
339347
string result = McpConfigurationHelper.WriteMcpConfiguration(path, client);
@@ -348,6 +356,59 @@ public override void Configure()
348356
}
349357
}
350358

359+
public override string GetConfigureActionLabel()
360+
=> client.status == McpStatus.Configured ? "Unregister" : "Configure";
361+
362+
/// <summary>
363+
/// Removes the unityMCP entry from the client's JSON config (both VS Code-style
364+
/// `servers` / `mcp.servers` layouts and the standard `mcpServers` layout). Leaves
365+
/// the file in place so we don't clobber other servers the user has configured.
366+
/// </summary>
367+
public override void Unregister()
368+
{
369+
string path = GetConfigPath();
370+
try
371+
{
372+
if (!File.Exists(path))
373+
{
374+
client.SetStatus(McpStatus.NotConfigured);
375+
client.configuredTransport = Models.ConfiguredTransport.Unknown;
376+
return;
377+
}
378+
379+
var root = JsonConvert.DeserializeObject<JToken>(File.ReadAllText(path)) as JObject;
380+
if (root == null)
381+
{
382+
client.SetStatus(McpStatus.NotConfigured);
383+
client.configuredTransport = Models.ConfiguredTransport.Unknown;
384+
return;
385+
}
386+
387+
bool removed = false;
388+
if (client.IsVsCodeLayout)
389+
{
390+
if ((root["servers"] as JObject)?.Remove("unityMCP") == true) removed = true;
391+
if ((root["mcp"]?["servers"] as JObject)?.Remove("unityMCP") == true) removed = true;
392+
}
393+
else
394+
{
395+
if ((root["mcpServers"] as JObject)?.Remove("unityMCP") == true) removed = true;
396+
}
397+
398+
if (removed)
399+
{
400+
File.WriteAllText(path, root.ToString(Formatting.Indented));
401+
}
402+
403+
client.SetStatus(McpStatus.NotConfigured);
404+
client.configuredTransport = Models.ConfiguredTransport.Unknown;
405+
}
406+
catch (Exception ex)
407+
{
408+
throw new InvalidOperationException($"Failed to unregister: {ex.Message}", ex);
409+
}
410+
}
411+
351412
public override string GetManualSnippet()
352413
{
353414
try
@@ -964,7 +1025,7 @@ private void Register()
9641025
client.configuredTransport = HttpEndpointUtility.GetCurrentServerTransport();
9651026
}
9661027

967-
private void Unregister()
1028+
public override void Unregister()
9681029
{
9691030
var pathService = MCPServiceLocator.Paths;
9701031
string claudePath = pathService.GetClaudeCliPath();

MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,15 @@ private void InitializeUI()
111111
manualConfigFoldout.value = false;
112112
}
113113

114-
// Restore the "Configure a single client" foldout state. Defaults to collapsed
115-
// so the prominent "Configure All Detected Clients" path is what users see first.
114+
// Default the per-client setup foldout to expanded. The Configure / Unregister
115+
// button for the currently-selected client lives inside it, and the 9.7.0
116+
// default-collapsed behavior made users believe the Unregister action had been
117+
// removed (community report: "the Unregister button was removed in 9.7.0,
118+
// something is stuck on stdio and I can't switch to local"). The state still
119+
// persists per-user, so anyone who explicitly collapses it keeps that preference.
116120
if (clientDetailsFoldout != null)
117121
{
118-
clientDetailsFoldout.value = EditorPrefs.GetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, false);
122+
clientDetailsFoldout.value = EditorPrefs.GetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, true);
119123
clientDetailsFoldout.RegisterValueChangedCallback(evt =>
120124
{
121125
EditorPrefs.SetBool(EditorPrefKeys.ClientDetailsFoldoutOpen, evt.newValue);
@@ -308,6 +312,13 @@ private void OnConfigureAllClientsClicked()
308312

309313
EditorUtility.DisplayDialog("Configure Detected Clients", message, "OK");
310314

315+
// The bulk path mutated state for every detected client. Clear the per-client
316+
// status cache so any subsequent dropdown switch (or the currently-selected
317+
// client refresh below) reads fresh status from disk instead of the pre-bulk
318+
// value — otherwise every other client in the dropdown looks like it still
319+
// has Missing/IncorrectPath status until 45 s elapses on the throttle.
320+
lastStatusChecks.Clear();
321+
311322
if (selectedClientIndex >= 0 && selectedClientIndex < configurators.Count)
312323
{
313324
UpdateClientStatus();
@@ -336,7 +347,18 @@ private void OnConfigureClicked()
336347

337348
try
338349
{
339-
MCPServiceLocator.Client.ConfigureClient(client);
350+
// Per-client toggle: Configure→register, Unregister→remove. The Configure path
351+
// on JsonFile / Codex configurators is always idempotent-write so the bulk
352+
// "Configure All" can call it unconditionally; the toggle lives here in the UI
353+
// handler so the manual button still does what its label says.
354+
if (client.Status == McpStatus.Configured)
355+
{
356+
client.Unregister();
357+
}
358+
else
359+
{
360+
MCPServiceLocator.Client.ConfigureClient(client);
361+
}
340362
lastStatusChecks.Remove(client);
341363
RefreshClientStatus(client, forceImmediate: true);
342364
UpdateManualConfiguration();

MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.uxml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
<ui:Label text="Client Configuration" class="section-title" />
55
<ui:VisualElement class="section-content">
66
<ui:Button name="configure-all-button" text="Configure All Detected Clients" class="primary-button" />
7-
<ui:Foldout name="client-details-foldout" text="Per-client setup" value="false" class="client-details-foldout">
7+
<ui:Foldout name="client-details-foldout" text="Per-client setup" value="true" class="client-details-foldout">
88
<ui:VisualElement class="setting-row">
99
<ui:Label text="Client:" class="setting-label" />
1010
<ui:DropdownField name="client-dropdown" class="setting-dropdown-inline" />

0 commit comments

Comments
 (0)