feat: improve editor window UI + add transport mismatch warning #613
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Transport Mismatch Detection: - Add detection for when MCP client config transport doesn't match Unity transport - Display warning banner when mismatch is detected - Add transport property to McpStatus and McpClient models - Detect transport from client configuration files (HTTP Local vs StdIO) UI Improvements: - Restructure editor window with cleaner tab-based layout - Update header styling with fixed height and improved version badge - Improve tab styling with proper borders and active state - Optimize spacing: remove bottom padding from panels, add space between sections - Rename Connection section to Server for clarity - Add flex-shrink: 0 to header to prevent clipping when dropdowns expand Refactoring: - Split monolithic Settings section into Advanced and Validation sections - Move path override controls to Advanced section - Move script validation to dedicated Validation section - Improve component organization and maintainability
Contributor
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds transport-type tracking (ConfiguredTransport enum) to client models and configurator; updates editor UI with separate Clients/Validation/Advanced/Tools panels, health-status callbacks, and transport-mismatch warnings; introduces new Validation and Advanced components and related UI/style changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Configurator as McpClientConfigurator
participant Model as McpClient (model)
participant Editor as MCPForUnityEditorWindow / UI
participant Server as Server (HTTP/CLI)
rect rgba(200,200,255,0.5)
Configurator->>Model: detect transport (json/toml/CLI)
Model-->>Configurator: store configuredTransport
end
rect rgba(200,255,200,0.5)
Configurator->>Editor: emit transport-detected event
Editor->>Editor: update ClientConfigSection UI (status, warnings)
Editor->>Configurator: request reconfirmation (on actions)
end
rect rgba(255,200,200,0.5)
Editor->>Server: perform health check / start/stop / test connection
Server-->>Editor: health response (Healthy / Unhealthy / NoResponse)
Editor->>Editor: propagate via health callback to Advanced section
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Collaborator
Author
|
Some screenshots 
|
Contributor
Reviewer's GuideRefactors the MCP for Unity editor window into a multi-tab layout (Clients, Validation, Advanced, Tools), moves advanced/server settings and health into a dedicated Advanced section, introduces a separate Validation section for script validation levels, and adds end-to-end transport type detection on MCP clients with a banner warning when client and server transports mismatch, along with assorted UI polish and tooling preferences tweaks. Sequence diagram for client transport mismatch detection and warning bannersequenceDiagram
actor User
participant MCPForUnityEditorWindow
participant McpClientConfigSection
participant IMcpClientConfigurator
participant McpClient
participant McpConnectionSection
participant EditorPrefs
User->>MCPForUnityEditorWindow: Open Clients tab
MCPForUnityEditorWindow->>McpClientConfigSection: UpdateClientStatus()
McpClientConfigSection->>IMcpClientConfigurator: CheckStatus(attemptAutoRewrite)
IMcpClientConfigurator->>McpClient: Inspect config files
IMcpClientConfigurator->>McpClient: Set status (McpStatus)
IMcpClientConfigurator->>McpClient: Set configuredTransport(Http|Stdio|Unknown)
IMcpClientConfigurator-->>McpClientConfigSection: McpStatus
McpClientConfigSection->>McpClientConfigSection: ApplyStatusToUi(client)
McpClientConfigSection->>EditorPrefs: Read UseHttpTransport
McpClientConfigSection->>McpClientConfigSection: Compare client.ConfiguredTransport with server transport
alt Transport mismatch
McpClientConfigSection->>McpClientConfigSection: Set clientStatusLabel Transport Mismatch
McpClientConfigSection->>McpClientConfigSection: Add warning class to clientStatusIndicator
else No mismatch
McpClientConfigSection->>McpClientConfigSection: Show normal status
end
McpClientConfigSection-->>McpConnectionSection: OnClientTransportDetected(clientName, ConfiguredTransport)
McpConnectionSection->>EditorPrefs: Read UseHttpTransport
McpConnectionSection->>McpConnectionSection: Determine server ConfiguredTransport
alt Mismatch
McpConnectionSection->>McpConnectionSection: UpdateTransportMismatchWarning()
McpConnectionSection->>McpConnectionSection: Show banner and message
else No mismatch or Unknown
McpConnectionSection->>McpConnectionSection: ClearTransportMismatchWarning()
end
Sequence diagram for health status testing via Advanced sectionsequenceDiagram
actor User
participant MCPForUnityEditorWindow
participant McpAdvancedSection
participant McpConnectionSection
participant BridgeService
User->>McpAdvancedSection: Click Test Connection button
McpAdvancedSection-->>MCPForUnityEditorWindow: OnTestConnectionRequested event
MCPForUnityEditorWindow->>McpConnectionSection: VerifyBridgeConnectionAsync()
McpConnectionSection->>McpConnectionSection: VerifyBridgeConnectionInternalAsync()
alt Bridge not running
McpConnectionSection-->>McpAdvancedSection: onHealthStatusUpdate(false, Unknown)
else Bridge running
McpConnectionSection->>BridgeService: VerifyAsync()
BridgeService-->>McpConnectionSection: result (Success, PingSucceeded, HandshakeValid)
McpConnectionSection->>McpConnectionSection: Determine statusText and isHealthy
McpConnectionSection-->>McpAdvancedSection: onHealthStatusUpdate(isHealthy, statusText)
end
McpAdvancedSection->>McpAdvancedSection: UpdateHealthStatus(isHealthy, statusText)
McpAdvancedSection->>McpAdvancedSection: Update healthIndicator classes and label
Class diagram for MCPForUnityEditorWindow tab layout and sectionsclassDiagram
class MCPForUnityEditorWindow {
-McpConnectionSection connectionSection
-McpClientConfigSection clientConfigSection
-McpValidationSection validationSection
-McpAdvancedSection advancedSection
-McpToolsSection toolsSection
-Label versionLabel
-VisualElement updateNotification
-Label updateNotificationText
-ToolbarToggle clientsTabToggle
-ToolbarToggle validationTabToggle
-ToolbarToggle advancedTabToggle
-ToolbarToggle toolsTabToggle
-VisualElement clientsPanel
-VisualElement validationPanel
-VisualElement advancedPanel
-VisualElement toolsPanel
-enum ActivePanel
+void CreateGUI()
+void RefreshAllData()
-void SetupTabs()
-void SwitchPanel(ActivePanel panel)
}
class ActivePanel {
<<enumeration>>
Clients
Validation
Advanced
Tools
}
class McpConnectionSection {
+VisualElement Root
-EnumField transportDropdown
-VisualElement transportMismatchWarning
-Label transportMismatchText
-VisualElement httpUrlRow
-VisualElement httpServerControlRow
-Foldout manualCommandFoldout
-VisualElement unitySocketPortRow
-TextField unityPortField
-VisualElement statusIndicator
-Label connectionStatusLabel
-Button connectionToggleButton
-Action onHealthStatusUpdate
+void SetHealthStatusUpdateCallback(Action callback)
+void UpdateConnectionStatus()
+Task VerifyBridgeConnectionAsync()
+void UpdateHttpServerCommandDisplay()
+void UpdateTransportMismatchWarning(string clientName, ConfiguredTransport clientTransport)
+void ClearTransportMismatchWarning()
}
class McpClientConfigSection {
+VisualElement Root
-PopupField~string~ clientDropdown
-Foldout manualConfigFoldout
-Label clientStatusLabel
-VisualElement clientStatusIndicator
-Button configureButton
-List~IMcpClientConfigurator~ configurators
-int selectedClientIndex
+event Action string ConfiguredTransport OnClientTransportDetected
+void UpdateManualConfiguration()
+void RefreshSelectedClient()
-void ApplyStatusToUi(IMcpClientConfigurator client, bool showChecking)
}
class McpAdvancedSection {
+VisualElement Root
-TextField uvxPathOverride
-Button browseUvxButton
-Button clearUvxButton
-Toggle debugLogsToggle
-TextField gitUrlOverride
-Toggle devModeForceRefreshToggle
-TextField deploySourcePath
-Button browseDeploySourceButton
-Button clearDeploySourceButton
-Button deployButton
-Button deployRestoreButton
-Label deployTargetLabel
-Label deployBackupLabel
-Label deployStatusLabel
-VisualElement healthIndicator
-Label healthStatus
-Button testConnectionButton
+event Action OnGitUrlChanged
+event Action OnHttpServerCommandUpdateRequested
+event Action OnTestConnectionRequested
+void UpdatePathOverrides()
+void UpdateHealthStatus(bool isHealthy, string statusText)
}
class McpValidationSection {
+VisualElement Root
-EnumField validationLevelField
-Label validationDescription
-ValidationLevel currentValidationLevel
+enum ValidationLevel
+McpValidationSection(VisualElement root)
}
class McpToolsSection {
+VisualElement Root
-Dictionary~string,Toggle~ toolToggleMap
-Toggle projectScopedToolsToggle
-Label summaryLabel
-Label noteLabel
-Button enableAllButton
-Button disableAllButton
}
MCPForUnityEditorWindow --> McpConnectionSection : owns
MCPForUnityEditorWindow --> McpClientConfigSection : owns
MCPForUnityEditorWindow --> McpValidationSection : owns
MCPForUnityEditorWindow --> McpAdvancedSection : owns
MCPForUnityEditorWindow --> McpToolsSection : owns
MCPForUnityEditorWindow --> ActivePanel : uses
McpConnectionSection --> McpAdvancedSection : healthStatusUpdateCallback
McpAdvancedSection --> McpConnectionSection : testConnectionRequested
McpClientConfigSection --> McpConnectionSection : transportMismatchWarning
Class diagram for MCP client transport detection and statusclassDiagram
class IMcpClientConfigurator {
<<interface>>
+string Id
+string DisplayName
+McpStatus Status
+ConfiguredTransport ConfiguredTransport
+bool SupportsAutoConfigure
+string GetConfigureActionLabel()
+McpStatus CheckStatus(bool attemptAutoRewrite)
+void Configure()
+string GetManualSnippet()
}
class McpClientConfiguratorBase {
-McpClient client
+McpClientConfiguratorBase(McpClient client)
+string Id
+string DisplayName
+McpStatus Status
+ConfiguredTransport ConfiguredTransport
+virtual bool SupportsAutoConfigure
+virtual string GetConfigureActionLabel()
+virtual McpStatus CheckStatus(bool attemptAutoRewrite)
+virtual void Configure()
+virtual string GetManualSnippet()
-McpStatus CheckStatusWithProjectDir(string projectDir, bool useHttpTransport)
-void RegisterWithCapturedValues(string projectDir, string claudePath, bool useHttpTransport)
-void UnregisterWithCapturedValues(string projectDir, string claudePath, bool useHttpTransport)
}
class McpClient {
+string name
+string windowsConfigPath
+string macConfigPath
+string linuxConfigPath
+string configStatus
+McpStatus status
+ConfiguredTransport configuredTransport
+bool IsVsCodeLayout
+bool RequiresArgsSection
}
class McpStatus {
<<enumeration>>
NotConfigured
Configured
Running
Connected
IncorrectPath
MissingConfig
CommunicationError
NoResponse
UnsupportedOS
Error
}
class ConfiguredTransport {
<<enumeration>>
Unknown
Stdio
Http
}
class McpConfigServer {
+string command
+string args
+string type
+string url
}
IMcpClientConfigurator <|.. McpClientConfiguratorBase
McpClientConfiguratorBase --> McpClient : wraps
McpClient --> McpStatus : uses
McpClient --> ConfiguredTransport : uses
McpClientConfiguratorBase --> ConfiguredTransport : sets
McpClientConfiguratorBase --> McpConfigServer : reads/writes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
MCPForUnityEditorWindow.CreateGUI, theOnTestConnectionRequestedhandler usesawait connectionSection?.VerifyBridgeConnectionAsync();– ifconnectionSectionis null this will await a nullTaskand throw; consider guarding with a null check before awaiting or using a non-nullable local reference. - In
McpConnectionSection.VerifyBridgeConnectionInternalAsyncandMcpAdvancedSection.UpdateHealthStatus, the health state is coordinated via the literal string "Unknown"; it would be more robust to share a constant or an enum for health state to avoid subtle bugs from string mismatches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `MCPForUnityEditorWindow.CreateGUI`, the `OnTestConnectionRequested` handler uses `await connectionSection?.VerifyBridgeConnectionAsync();` – if `connectionSection` is null this will await a null `Task` and throw; consider guarding with a null check before awaiting or using a non-nullable local reference.
- In `McpConnectionSection.VerifyBridgeConnectionInternalAsync` and `McpAdvancedSection.UpdateHealthStatus`, the health state is coordinated via the literal string "Unknown"; it would be more robust to share a constant or an enum for health state to avoid subtle bugs from string mismatches.
## Individual Comments
### Comment 1
<location> `MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs:250-251` </location>
<code_context>
+ clientConfigSection?.UpdateManualConfiguration();
+ advancedSection.OnHttpServerCommandUpdateRequested += () =>
+ connectionSection?.UpdateHttpServerCommandDisplay();
+ advancedSection.OnTestConnectionRequested += async () =>
+ await connectionSection?.VerifyBridgeConnectionAsync();
+
+ // Wire up health status updates from Connection to Advanced
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid awaiting a potentially null Task from a null-conditional invocation.
Due to the null-conditional, `connectionSection?.VerifyBridgeConnectionAsync()` can evaluate to `null` when `connectionSection` is uninitialized, and `await null` will throw at runtime. Capture `connectionSection` into a local and guard before awaiting, for example:
```csharp
advancedSection.OnTestConnectionRequested += async () =>
{
var connection = connectionSection;
if (connection != null)
{
await connection.VerifyBridgeConnectionAsync();
}
};
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
- Fixed null Task await in MCPForUnityEditorWindow.CreateGUI (line 251) - Added proper null check before awaiting connectionSection?.VerifyBridgeConnectionAsync() - Prevents NullReferenceException when connectionSection is null - Replaced magic string health status values with shared constants - Created new HealthStatus class with constants: Unknown, Healthy, PingFailed, Unhealthy - Updated McpConnectionSection to use shared constants instead of private ones - Updated McpAdvancedSection to use HealthStatus.Unknown constant - More robust coordination between Connection and Advanced sections Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
UI Improvements:
Restructure editor window with cleaner tab-based layout
Update header styling with fixed height and improved version badge
Improve tab styling with proper borders and active state
Optimize spacing: remove bottom padding from panels, add space between sections
Rename Connection section to Server for clarity
Add flex-shrink: 0 to header to prevent clipping when dropdowns expand
Refactoring:
Split monolithic Settings section into Advanced and Validation sections
Move path override controls to Advanced section
Move script validation to dedicated Validation section
Improve component organization and maintainability
Transport Mismatch Detection:
Add detection for when MCP client config transport doesn't match Unity transport
Display warning banner when mismatch is detected
Add transport property to McpStatus and McpClient models
Detect transport from client configuration files (HTTP Local vs StdIO)
Summary by Sourcery
Restructure the MCP for Unity editor window into dedicated Clients, Validation, Advanced, and Tools tabs with improved layout and header, while adding detection and UI warnings for mismatched client/server transports and reorganizing advanced settings and health diagnostics.
New Features:
Enhancements:
Summary by CodeRabbit
New Features
UI/UX Improvements
✏️ Tip: You can customize this high-level summary in your review settings.