Refactored/simplified UI and setup from CoplayDev original source for our needs. #437
Conversation
Changed from parent github to prophecygamesstudio fork
WalkthroughConsolidates transport to stdio-only, removes HTTP/WebSocket transport code and related reload handlers, simplifies migration and client-configuration flows, strips connection/client UI and menu item, and updates repository metadata/URLs from CoplayDev to prophecygamestudio. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Editor as Unity Editor UI
participant Bridge as BridgeControlService / TransportManager
participant Stdio as StdioTransportClient
Note over Editor,Bridge: Old flow (removed)
Editor->>Bridge: StartAsync(TransportMode.Http)
Bridge->>WebSocket: create/start WebSocketTransportClient
WebSocket-->>Bridge: state updates
Note over Editor,Bridge: New flow (stdio-only)
Editor->>Bridge: StartAsync()
Bridge->>Stdio: Initialize & StartAsync()
Stdio-->>Bridge: Started / TransportState
Editor->>Bridge: VerifyAsync()
Bridge->>Stdio: VerifyAsync()
Stdio-->>Bridge: VerificationResult
Editor->>Bridge: StopAsync()
Bridge->>Stdio: StopAsync()
Stdio-->>Bridge: Stopped
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
README.md (3)
13-14: Update GitHub badge URL to new repository owner.The commit activity and issue count badges still reference
CoplayDev/unity-mcp. These should be updated toprophecygamestudio/unity-mcpfor consistency with the migration.- - + +
160-160: Outdated OpenUPM package name.Line 160 still references the old package identifier
com.coplaydev.unity-mcp. Update to the new package namecom.prophecygamestudio.unity-mcp.-3. Run `openupm add com.coplaydev.unity-mcp` +3. Run `openupm add com.prophecygamestudio.unity-mcp`
435-435: Update Star History badge URL to new repository.The Star History badge at line 435 still references
CoplayDev/unity-mcpand should be updated toprophecygamestudio/unity-mcp.-[](https://www.star-history.com/#CoplayDev/unity-mcp&Date) +[](https://www.star-history.com/#prophecygamestudio/unity-mcp&Date)
🧹 Nitpick comments (14)
README.md (3)
162-162: Stale reference to previous maintainer.Line 162 mentions "Coplay's maintenance," which is outdated given the repository migration. Consider updating this note to reflect the new ownership or removing it entirely.
-**Note:** If you installed the MCP Server before Coplay's maintenance, you will need to uninstall the old package before re-installing the new one. +**Note:** If you upgraded from a previous version, you will need to uninstall the old package before re-installing.
208-210: UseAppSupportpath on macOS to avoid quoting issues.Per established documentation patterns, line 208 should use the shortened
~/Library/AppSupportpath instead of the full path with spaces. This avoids argument parsing and quoting complications in some MCP clients.- * *Claude Example (macOS):* `~/Library/Application Support/Claude/claude_desktop_config.json` + * *Claude Example (macOS):* `~/Library/AppSupport/Claude/claude_desktop_config.json`
293-293: Normalize path separator in Windows example.Line 293 uses backslash separators while macOS/Linux examples use forward slashes. Use forward slashes consistently across all examples for clarity.
- "/Users/YOUR_USERNAME/Library/AppSupport/UnityMCP/UnityMcpServer/src", + "/Users/YOUR_USERNAME/Library/AppSupport/UnityMCP/UnityMcpServer/src",(Note: macOS example already uses forward slashes; ensure Windows path at line 309 also uses forward slashes for consistency.)
MCPForUnity/package.json (1)
21-25: Consider updating author field for consistency with repository migration.The author field (lines 21–25) still lists "Coplay" as the maintainer. For consistency with the repository migration to
prophecygamestudio, consider updating the author information to reflect the new owner or maintainer."author": { - "name": "Coplay", - "email": "support@coplay.dev", - "url": "https://coplay.dev" + "name": "Prophecy Game Studio", + "email": "...", + "url": "https://prophecygamestudio.com" }MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs (2)
49-55: Persisting last processed version is fine; unused helpers could be cleaned upRecording
LastStdIoUpgradeVersionviaEditorPrefs.SetStringbehind a try/catch is low-risk—failure just means the migration will rerun on the next domain reload. If you revisit this later, you might either add a debug-level log on failure or remove the now-unusedConfigUsesStdIo/JsonConfigUsesStdIohelpers below, since the migration no longer inspects or rewrites configs.
56-56: Info log matches new configuration model; consider debug-only if it becomes noisyThe
McpLog.Infomessage clearly explains that client configuration is now owned by the MCP client (e.g., Cursor’smcp.json), which should help users understand the behavioral change. If this ever feels too noisy in large projects, you could flip toMcpLog.Info(message, always: false)so it only appears when debug logging is enabled, but as a once-per-version notice the current choice is reasonable.MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs (2)
45-48: Consider removing the empty method or marking it for future cleanup.The empty
InitializeUI()method with just a comment could be removed if there's no intent to add initialization logic later. Alternatively, consider adding a// TODO: Remove if no longer neededto track cleanup.
58-62: Stub method maintained for compatibility - consider documenting deprecation.The no-op stub is acceptable for maintaining API compatibility. If this method is called externally, consider marking it with
[Obsolete]to signal callers that it no longer performs any action.+ [System.Obsolete("Connection status UI has been removed. This method is a no-op.")] public void UpdateConnectionStatus() { // Connection status UI elements have been removed // This method is kept for compatibility but does nothing }MCPForUnity/Editor/Services/Transport/TransportManager.cs (3)
23-47: Simplification looks good; consider removing redundant null check.The stdio-only flow is clearer and the error handling is solid. The null check and recreation at lines 25-28 is defensive but unnecessary since the constructor always initializes
_stdioClient.Consider removing the redundant null check:
public async Task<bool> StartAsync() { - if (_stdioClient == null) - { - _stdioClient = new StdioTransportClient(); - } - bool started = await _stdioClient.StartAsync();
49-64: Good error handling; redundant null check.The try-catch-finally pattern ensures the state is updated even on errors, which is good practice. However, the null check at line 51 is unnecessary since
_stdioClientis always initialized in the constructor.Optionally remove the null check:
public async Task StopAsync() { - if (_stdioClient == null) return; try { await _stdioClient.StopAsync();
66-77: State synchronization is appropriate; null check is redundant.The method properly synchronizes the cached state with the client's state after verification. The null check at line 68 is unnecessary given the constructor initialization.
Consider removing the null check:
public async Task<bool> VerifyAsync() { - if (_stdioClient == null) - { - return false; - } - bool ok = await _stdioClient.VerifyAsync();MCPForUnity/Editor/Migrations/LegacyServerSrcMigration.cs (1)
39-57: Cleanup‑only migration looks good; consider making the log message client‑agnosticThe new behavior of just deleting
MCPForUnity.ServerSrc/MCPForUnity.UseEmbeddedServerwhen present is simple and safe, and the log clearly signals the migration path has changed.One small suggestion: the final info log explicitly calls out “Cursor's mcp.json”. If this package is meant to be client‑agnostic, you might want to rephrase that to describe “the MCP client configuration (e.g. an
mcp.jsonfile)” without naming a specific tool, to avoid implying a hard coupling to Cursor.MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (2)
189-192: Deduplicate advisory dialog text and consider slightly more generic wordingBoth configure handlers now show the same “configuration no longer supported” dialog. To keep this maintainable and avoid drift, you could extract the message into a shared constant and reuse it in both places. While you’re there, you might optionally make the example wording a bit more generic (e.g., “Cursor or another MCP client”, and “mcp.json or equivalent config file”) so the copy stays accurate if additional clients are commonly used.
@@ - // Data + // Data + private const string ClientConfigNotSupportedMessage = + "Client configuration is no longer supported by this plugin.\n\n" + + "Please configure your MCP client (e.g., Cursor) directly using its configuration file (e.g., mcp.json)."; @@ - private void OnConfigureAllClientsClicked() - { - EditorUtility.DisplayDialog("Client Configuration", - "Client configuration is no longer supported by this plugin.\n\n" + - "Please configure your MCP client (e.g., Cursor) directly using its configuration file (e.g., mcp.json).", - "OK"); - } + private void OnConfigureAllClientsClicked() + { + EditorUtility.DisplayDialog("Client Configuration", ClientConfigNotSupportedMessage, "OK"); + } @@ - private void OnConfigureClicked() - { - EditorUtility.DisplayDialog("Client Configuration", - "Client configuration is no longer supported by this plugin.\n\n" + - "Please configure your MCP client (e.g., Cursor) directly using its configuration file (e.g., mcp.json).", - "OK"); - } + private void OnConfigureClicked() + { + EditorUtility.DisplayDialog("Client Configuration", ClientConfigNotSupportedMessage, "OK"); + }Also applies to: 197-200
273-278:SetStatuserrorDetails forNotConfiguredare effectively unusedBoth
RefreshClientStatusandRefreshClaudeCliStatusnow callbaseConfigurator.Client.SetStatus(McpStatus.NotConfigured, "Client configuration not supported"). Given the currentMcpClient.SetStatusimplementation inMCPForUnity/Editor/Models/McpClient.cs, theerrorDetailsparameter is only incorporated intoconfigStatuswhennewStatus == McpStatus.Error; forNotConfiguredit’s ignored, and this section’s UI usesclient.StatusplusGetStatusDisplayStringrather than any detailed message. That makes the string argument misleading from a reader’s perspective.If you don’t intend to surface that explanatory text anywhere, consider dropping the second parameter so the code more accurately reflects what the user actually sees:
- if (client is McpClientConfiguratorBase baseConfigurator) - { - baseConfigurator.Client.SetStatus(McpStatus.NotConfigured, "Client configuration not supported"); - } + if (client is McpClientConfiguratorBase baseConfigurator) + { + baseConfigurator.Client.SetStatus(McpStatus.NotConfigured); + }If instead you do want users to see “Client configuration not supported” somewhere in this section, it may be cleaner to explicitly set
clientStatusLabel.text(or another dedicated label) for this “no client configuration service” mode rather than relying onSetStatus/McpStatusto carry that message.Also applies to: 284-291
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs(1 hunks)MCPForUnity/Editor/MenuItems/MCPForUnityMenu.cs(0 hunks)MCPForUnity/Editor/Migrations/LegacyServerSrcMigration.cs(2 hunks)MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs(1 hunks)MCPForUnity/Editor/Resources/Editor/Selection.cs(1 hunks)MCPForUnity/Editor/Services/BridgeControlService.cs(4 hunks)MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs(0 hunks)MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs.meta(0 hunks)MCPForUnity/Editor/Services/IBridgeControlService.cs(1 hunks)MCPForUnity/Editor/Services/MCPServiceLocator.cs(0 hunks)MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs(3 hunks)MCPForUnity/Editor/Services/Transport/TransportManager.cs(1 hunks)MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs(1 hunks)MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs(0 hunks)MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs.meta(0 hunks)MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs(3 hunks)MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs(3 hunks)MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.uxml(0 hunks)MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs(3 hunks)MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml(0 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs(0 hunks)MCPForUnity/package.json(1 hunks)README.md(4 hunks)Server/README.md(8 hunks)Server/pyproject.toml(1 hunks)TestProjects/UnityMCPTests/Packages/manifest.json(1 hunks)
💤 Files with no reviewable changes (9)
- MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.uxml
- MCPForUnity/Editor/MenuItems/MCPForUnityMenu.cs
- MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs
- MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs
- MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs.meta
- MCPForUnity/Editor/Services/MCPServiceLocator.cs
- MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml
- MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
- MCPForUnity/Editor/Services/HttpBridgeReloadHandler.cs.meta
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
📚 Learning: 2025-11-27T21:09:35.011Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 401
File: MCPForUnity/Editor/Clients/Configurators/VSCodeConfigurator.cs:10-18
Timestamp: 2025-11-27T21:09:35.011Z
Learning: VS Code GitHub Copilot MCP configuration supports mcp.json files placed directly in the Code/User directory: %APPDATA%\Code\User\mcp.json on Windows, ~/Library/Application Support/Code/User/mcp.json on macOS, and ~/.config/Code/User/mcp.json on Linux. This is in addition to workspace-scoped .vscode/mcp.json files.
Applied to files:
README.mdMCPForUnity/package.jsonServer/README.md
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Applied to files:
README.mdServer/README.mdMCPForUnity/Editor/Helpers/AssetPathUtility.cs
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to avoid spaces in paths that can cause arg parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path.
Applied to files:
README.mdServer/README.md
📚 Learning: 2025-10-24T14:09:08.615Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 348
File: MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs:71-79
Timestamp: 2025-10-24T14:09:08.615Z
Learning: The SystemRoot environment variable on Windows is only required for Codex MCP client configurations due to a Codex bug. Other MCP clients (VSCode, Cursor, Windsurf, Kiro) do not need this environment variable. Codex configurations use TOML format (CodexConfigHelper.cs), while other clients use JSON format (ConfigJsonBuilder.cs).
Applied to files:
README.md
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
README.mdTestProjects/UnityMCPTests/Packages/manifest.jsonServer/pyproject.tomlServer/README.mdMCPForUnity/Editor/Migrations/StdIoVersionMigration.cs
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
README.mdMCPForUnity/package.jsonMCPForUnity/Editor/Migrations/LegacyServerSrcMigration.csTestProjects/UnityMCPTests/Packages/manifest.jsonServer/pyproject.tomlServer/README.mdMCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.csMCPForUnity/Editor/Migrations/StdIoVersionMigration.csMCPForUnity/Editor/Services/IBridgeControlService.csMCPForUnity/Editor/Helpers/AssetPathUtility.csMCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.csMCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs
📚 Learning: 2025-09-04T01:01:11.927Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
Applied to files:
MCPForUnity/package.jsonTestProjects/UnityMCPTests/Packages/manifest.jsonServer/pyproject.tomlServer/README.mdMCPForUnity/Editor/Migrations/StdIoVersionMigration.csMCPForUnity/Editor/Services/IBridgeControlService.csMCPForUnity/Editor/Helpers/AssetPathUtility.csMCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs
📚 Learning: 2025-11-05T18:23:12.349Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Applied to files:
MCPForUnity/package.jsonServer/pyproject.tomlServer/README.md
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
MCPForUnity/package.jsonServer/pyproject.tomlServer/README.md
📚 Learning: 2025-09-03T16:00:55.839Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 0
File: :0-0
Timestamp: 2025-09-03T16:00:55.839Z
Learning: ComponentResolver in UnityMcpBridge/Editor/Tools/ManageGameObject.cs is a nested static class within ManageGameObject, not a sibling type. The `using static MCPForUnity.Editor.Tools.ManageGameObject;` import is required to access ComponentResolver methods directly without the outer class qualifier.
Applied to files:
TestProjects/UnityMCPTests/Packages/manifest.json
🧬 Code graph analysis (3)
MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs (1)
MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Info(37-41)
MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (4)
MCPForUnity/Editor/Clients/McpClientRegistry.cs (1)
List(28-57)MCPForUnity/Editor/Clients/McpClientConfiguratorBase.cs (6)
McpClientConfiguratorBase(17-80)McpClientConfiguratorBase(21-24)McpStatus(35-35)McpStatus(89-191)McpStatus(232-289)McpStatus(337-392)MCPForUnity/Editor/Models/McpClient.cs (1)
SetStatus(42-54)MCPForUnity/Editor/Clients/IMcpClientConfigurator.cs (1)
McpStatus(30-30)
MCPForUnity/Editor/Services/Transport/TransportManager.cs (2)
MCPForUnity/Editor/Services/BridgeControlService.cs (3)
Task(66-82)Task(84-94)Task(96-101)MCPForUnity/Editor/Services/IBridgeControlService.cs (3)
Task(29-29)Task(34-34)Task(47-47)
🪛 markdownlint-cli2 (0.18.1)
README.md
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
199-199: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
200-200: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (22)
MCPForUnity/Editor/Helpers/AssetPathUtility.cs (1)
145-163: Updated MCP server Git URLs are consistent with repo ownership changeThe fallback and versioned Git URLs now correctly point to
prophecygamestudio/unity-mcpwhile preserving the existing override/unknown-version behavior. No control-flow or parsing changes are introduced, and this aligns with the PR’s move to the new GitHub org.MCPForUnity/Editor/Resources/Editor/Selection.cs (1)
24-24: Null‑safe selection instance ID looks goodUsing
activeObject?.GetInstanceID() ?? 0is a clear, null‑safe way to surface the active selection’s instance ID and avoids any direct dependency onSelection.activeInstanceID. No issues spotted with this refactor.Server/pyproject.toml (1)
3-3: Version bump is clean.The patch version bump aligns with the refactoring scope. No dependency or configuration changes.
README.md (1)
37-37: Contradicts PR summary: Verify HTTP transport status.Line 37 claims "HTTP-First Transport" is the default, but the PR summary states "limits support to stdio only." Please clarify the actual transport support in this release and update the documentation to match the implementation.
Server/README.md (2)
76-76: Server invocation method change is clear and consistent here.The migration from
server.pyto-m src.mainis correctly reflected throughout this file. This is the proper Python module invocation pattern for this release.Also applies to: 79-79, 103-104, 123-124
14-14: Repository migration references are thorough and consistent.All external links and documentation references have been properly updated to
prophecygamestudio/unity-mcp. Version numbers are consistent with the pyproject.toml bump.Also applies to: 72-72, 171-171, 185-185
MCPForUnity/package.json (1)
2-2: Package metadata updated correctly.The package name and all external documentation/license URLs have been properly migrated to
prophecygamestudio/unity-mcp.Also applies to: 7-8
TestProjects/UnityMCPTests/Packages/manifest.json (1)
3-3: Package dependency name updated consistently.The test project manifest has been correctly updated to reference the new package name
com.prophecygamestudio.unity-mcpwhile preserving the local path reference.MCPForUnity/Editor/Migrations/StdIoVersionMigration.cs (1)
44-47: Version short-circuit correctly prevents redundant migrationsThe early return when
lastUpgradeVersionmatchescurrentVersionis a clean way to ensure this migration only runs once per package version and avoids unnecessary work. The comment accurately reflects the behavior.MCPForUnity/Editor/Services/Transport/Transports/StdioBridgeHost.cs (1)
209-213: LGTM - Simplified auto-start aligns with PR objectives.The method now unconditionally returns
true, removing the previous EditorPrefs-based configuration. This aligns with the PR's goal to have the bridge auto-start on plugin load. Batch mode is still correctly guarded at line 150-153.MCPForUnity/Editor/Services/IBridgeControlService.cs (1)
43-47: LGTM - Documentation simplified to reflect stdio-only transport.The XML documentation correctly reflects the simplified transport model. The removal of the
ActiveModeproperty (mentioned in AI summary) and updated documentation align with the stdio-only consolidation.MCPForUnity/Editor/Windows/Components/Connection/McpConnectionSection.cs (1)
83-86: Good defensive null check.The null guard prevents potential
NullReferenceExceptionwhen UI elements are missing. This is a solid defensive programming practice.MCPForUnity/Editor/Services/BridgeControlService.cs (2)
10-12: LGTM - Documentation updated for stdio-only transport.The class documentation correctly reflects the simplified transport model.
22-40: LGTM - Well-structured helper method.
BuildVerificationResultcleanly handles nullable states and provides sensible defaults (e.g., "stdio" as default transport label). The method consolidates verification result construction in one place.MCPForUnity/Editor/Services/StdioBridgeReloadHandler.cs (3)
8-10: LGTM - Documentation correctly reflects stdio-only focus.The class documentation update aligns with the simplified transport model.
20-48: LGTM - Simplified reload persistence logic.The simplified check at line 25-26 using
IsRunning()is cleaner than the previous mode-specific logic. The fire-and-forget pattern forStopAsync()is acceptable here since assembly reload will proceed regardless, and error handling viaContinueWithwithTaskScheduler.Defaultavoids synchronization context issues.
75-92: LGTM - Proper async error handling pattern.The
ContinueWithpattern correctly handles both faulted tasks and unsuccessful results, logging appropriate warnings. UsingTaskScheduler.Defaultensures the continuation doesn't capture Unity's synchronization context, which is the right approach for background error handling.MCPForUnity/Editor/Services/Transport/TransportManager.cs (3)
16-19: LGTM! Clean initialization.The constructor properly initializes the stdio client and state. The simplification to a single transport mode is clear and straightforward.
79-84: LGTM! State caching pattern is appropriate.The methods return the cached state, which is efficient and appropriate since this class coordinates all transport operations and updates the cache after each lifecycle method. The
IsRunning()convenience method provides a clear API for connection status checks.
8-10: Excellent simplification! Documentation updated correctly.The updated summary accurately reflects the stdio-only architecture. The removal of multi-transport orchestration significantly simplifies the codebase while meeting the PR's stated objectives.
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs (1)
59-79: Version label null-check is solid; tooltip Git URL is currentThe added null-check in
UpdateVersionLabelis good defensive coding.The tooltip Git URL (
https://github.com/CoplayDev/unity-mcp.git?path=/MCPForUnity) is correct and points to the current repository location. No update needed.MCPForUnity/Editor/Windows/Components/ClientConfig/McpClientConfigSection.cs (1)
52-54: Empty configurator list safely disables dynamic client configurationSetting
configuratorsto an empty list, combined with the existing bounds checks before indexing, cleanly disables this section’s dynamic behavior while allowing the UI to load without exceptions. This matches the PR goal of dropping in‑plugin client configuration.
| public BridgeVerificationResult Verify(int port) | ||
| { | ||
| var mode = ResolvePreferredMode(); | ||
| bool pingSucceeded = _transportManager.VerifyAsync(mode).GetAwaiter().GetResult(); | ||
| var state = _transportManager.GetState(mode); | ||
|
|
||
| if (mode == TransportMode.Stdio) | ||
| { | ||
| bool handshakeValid = state.IsConnected && port == CurrentPort; | ||
| string message = handshakeValid | ||
| ? $"STDIO transport listening on port {CurrentPort}" | ||
| : $"STDIO transport port mismatch (expected {CurrentPort}, got {port})"; | ||
| return BuildVerificationResult(state, mode, pingSucceeded && handshakeValid, message, handshakeValid); | ||
| } | ||
|
|
||
| return BuildVerificationResult(state, mode, pingSucceeded); | ||
| bool pingSucceeded = _transportManager.VerifyAsync().GetAwaiter().GetResult(); | ||
| var state = _transportManager.GetState(); | ||
|
|
||
| bool handshakeValid = state.IsConnected && port == CurrentPort; | ||
| string message = handshakeValid | ||
| ? $"STDIO transport listening on port {CurrentPort}" | ||
| : $"STDIO transport port mismatch (expected {CurrentPort}, got {port})"; | ||
| return BuildVerificationResult(state, pingSucceeded && handshakeValid, message, handshakeValid); | ||
| } |
There was a problem hiding this comment.
Potential deadlock risk with synchronous blocking on async method.
Verify(int port) calls .GetAwaiter().GetResult() on an async method. In Unity's editor context with a SynchronizationContext, this pattern can cause deadlocks if the async continuation tries to marshal back to the main thread.
Consider either:
- Making
Verifyasync and renaming the existing one - Using
Task.Runto avoid capturing the synchronization context
public BridgeVerificationResult Verify(int port)
{
- bool pingSucceeded = _transportManager.VerifyAsync().GetAwaiter().GetResult();
+ // Avoid deadlock by running on thread pool without capturing sync context
+ bool pingSucceeded = Task.Run(() => _transportManager.VerifyAsync()).GetAwaiter().GetResult();
var state = _transportManager.GetState();
bool handshakeValid = state.IsConnected && port == CurrentPort;Committable suggestion skipped: line range outside the PR's diff.
| private void InitializeUI() | ||
| { | ||
| UpdateVersionLabel(); | ||
|
|
||
| bool debugEnabled = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false); | ||
| debugLogsToggle.value = debugEnabled; | ||
| McpLog.SetDebugLoggingEnabled(debugEnabled); | ||
|
|
||
| validationLevelField.Init(ValidationLevel.Standard); | ||
| int savedLevel = EditorPrefs.GetInt(EditorPrefKeys.ValidationLevel, 1); | ||
| currentValidationLevel = (ValidationLevel)Mathf.Clamp(savedLevel, 0, 3); | ||
| validationLevelField.value = currentValidationLevel; | ||
| UpdateValidationDescription(); | ||
| if (versionLabel != null) | ||
| { | ||
| UpdateVersionLabel(); | ||
| } | ||
|
|
||
| advancedSettingsFoldout.value = false; | ||
| gitUrlOverride.value = EditorPrefs.GetString(EditorPrefKeys.GitUrlOverride, ""); | ||
| if (debugLogsToggle != null) | ||
| { | ||
| bool debugEnabled = EditorPrefs.GetBool(EditorPrefKeys.DebugLogs, false); | ||
| debugLogsToggle.value = debugEnabled; | ||
| McpLog.SetDebugLoggingEnabled(debugEnabled); | ||
| } | ||
| } |
There was a problem hiding this comment.
Guard debug toggle initialization and persist its state
The null checks in InitializeUI are good, but there are two behavioral gaps around the debug logs toggle:
RegisterCallbacksis called unconditionally, and it unconditionally dereferencesdebugLogsToggle. If the UXML no longer defines"debug-logs-toggle", you’ll hit aNullReferenceExceptionat editor window construction.- The initial value is loaded from
EditorPrefs, but changes are never written back, so users’ choices won’t persist across editor sessions.
You can address both with a small change:
private void RegisterCallbacks()
{
- debugLogsToggle.RegisterValueChangedCallback(evt =>
- {
- McpLog.SetDebugLoggingEnabled(evt.newValue);
- });
+ if (debugLogsToggle == null)
+ return;
+
+ debugLogsToggle.RegisterValueChangedCallback(evt =>
+ {
+ McpLog.SetDebugLoggingEnabled(evt.newValue);
+ EditorPrefs.SetBool(EditorPrefKeys.DebugLogs, evt.newValue);
+ });
}This keeps the null‑safety consistent with InitializeUI and ensures the preference actually behaves like a persistent setting.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs around
lines 36 to 49, RegisterCallbacks currently dereferences debugLogsToggle
unconditionally and changes to the toggle aren’t persisted; fix by guarding
registration so you only add the callback if debugLogsToggle is non-null (and
remove/unregister it in cleanup if applicable), and in the toggle callback write
the new value to EditorPrefs.SetBool(EditorPrefKeys.DebugLogs, value) as well as
calling McpLog.SetDebugLoggingEnabled(value) so the choice persists across
editor sessions.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
410-410: Fix path separator in macOS documentation.The macOS path on line 410 uses a backslash (
\src) instead of a forward slash (/src), which will break the path on Unix-like systems.Apply this diff:
- - **macOS:** `~/Library/AppSupport/UnityMCP/UnityMcpServer\src` + - **macOS:** `~/Library/AppSupport/UnityMCP/UnityMcpServer/src`
435-435: Update Star History chart to reflect new repository ownership.Line 435 still references
CoplayDev/unity-mcpin the star history chart; update it toprophecygamestudio/unity-mcpto align with the repository migration.Apply this diff:
-[](https://www.star-history.com/#CoplayDev/unity-mcp&Date) +[](https://www.star-history.com/#prophecygamestudio/unity-mcp&Date)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
README.md(4 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 260
File: UnityMcpBridge/UnityMcpServer~/src/server_version.txt:1-1
Timestamp: 2025-09-04T01:01:11.927Z
Learning: The UnityMcpBridge project is not maintaining changelogs yet, so don't suggest adding changelog entries for version bumps.
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to mitigate argument parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path with spaces.
Applied to files:
README.md
📚 Learning: 2025-09-05T16:22:04.960Z
Learnt from: dsarno
Repo: CoplayDev/unity-mcp PR: 265
File: README.md:204-204
Timestamp: 2025-09-05T16:22:04.960Z
Learning: In the Unity MCP project, the ServerInstaller.cs creates a symlink from ~/Library/AppSupport to ~/Library/Application Support on macOS to avoid spaces in paths that can cause arg parsing and quoting issues in some MCP clients. The README documentation should use the shortened AppSupport path, not the full "Application Support" path.
Applied to files:
README.md
📚 Learning: 2025-11-27T21:09:35.011Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 401
File: MCPForUnity/Editor/Clients/Configurators/VSCodeConfigurator.cs:10-18
Timestamp: 2025-11-27T21:09:35.011Z
Learning: VS Code GitHub Copilot MCP configuration supports mcp.json files placed directly in the Code/User directory: %APPDATA%\Code\User\mcp.json on Windows, ~/Library/Application Support/Code/User/mcp.json on macOS, and ~/.config/Code/User/mcp.json on Linux. This is in addition to workspace-scoped .vscode/mcp.json files.
Applied to files:
README.md
📚 Learning: 2025-10-24T14:09:08.615Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 348
File: MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs:71-79
Timestamp: 2025-10-24T14:09:08.615Z
Learning: The SystemRoot environment variable on Windows is only required for Codex MCP client configurations due to a Codex bug. Other MCP clients (VSCode, Cursor, Windsurf, Kiro) do not need this environment variable. Codex configurations use TOML format (CodexConfigHelper.cs), while other clients use JSON format (ConfigJsonBuilder.cs).
Applied to files:
README.md
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
README.md
📚 Learning: 2025-10-13T13:27:23.040Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 316
File: TestProjects/UnityMCPTests/Assets/Tests/EditMode/Resources.meta:1-8
Timestamp: 2025-10-13T13:27:23.040Z
Learning: UnityMcpBridge is a legacy project kept for backwards compatibility; MCPForUnity is the only active Unity plugin project. GUID collisions between UnityMcpBridge and MCPForUnity are acceptable.
Applied to files:
README.md
🪛 markdownlint-cli2 (0.18.1)
README.md
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
199-199: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
200-200: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
🔇 Additional comments (1)
README.md (1)
180-180: Version mismatch between installation examples.Line 153 pins the package to
v8.1.6, but line 180 referencesv8.1.0. Ensure the manual server invocation uses the same version tag as the git package URL for consistency, unless the server version intentionally lags behind the package version.
| 5. Click `Add`. | ||
|
|
||
| **Need a fixed version?** Use a tagged URL instead (updates require uninstalling and re-installing): | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
The code block starting at line 152 is missing a language specifier. Since it contains a git URL, use the bash identifier or leave it empty for plain text.
Apply this diff:
**Need a fixed version?** Use a tagged URL instead (updates require uninstalling and re-installing):
-```
+```bash
https://github.com/prophecygamestudio/unity-mcp.git?path=/MCPForUnity#v8.1.6
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
152-152: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In README.md around line 152, the fenced code block is missing a language
specifier; update that code block to use the bash identifier (i.e., replace the
opening withbash) and ensure the block contains the git URL line
"https://github.com/prophecygamestudio/unity-mcp.git?path=/MCPForUnity#v8.1.6"
followed by the closing ``` so the snippet is fenced as bash.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.