Remove asset store download button #348
Conversation
- Added `apply_text_edits` tool for precise text modifications with hash-based precondition checking - Added `script_apply_edits` tool for structured C# code edits with safer method/class boundaries - Added `validate_script` tool for quick syntax and structure validation - Updated tool documentation in server prompt to include new capabilities
- Added support for using remote uvx server when no embedded server is present (Asset Store installs) - Removed server download/install functionality since remote server is now used automatically - Updated configuration generation to use remote server URL when appropriate - Modified Claude Code registration to support both remote and local server modes - Updated UI to remove download button and simplify server status messaging - Added Sho
- Added ShouldUseRemoteUvx helper method to determine when to use remote uvx based on server availability - Modified CreateUnityMcpTable to support both remote uvx and local server configurations - Updated config generation to use remote uvx when no embedded server is present (Asset Store installs) - Added comprehensive test coverage for remote uvx configuration scenarios - Maintained Windows-specific environment settings for both local and remote configurations
- Updated Unity editor version from 2022.3.6f1 to 2022.3.62f2 - Upgraded several Unity packages to latest versions: - AI Navigation 1.1.4 → 1.1.6 - Collab Proxy 2.5.2 → 2.7.1 - IDE Rider 3.0.31 → 3.0.36 - TextMeshPro 3.0.6 → 3.0.7 - Timeline 1.7.5 → 1.7.7 - Added Editor.meta file for Unity MCP test project
WalkthroughThe PR refactors the MCP server configuration system to support both remote uvx-based installations (via Asset Store) and embedded local installations, removing the mandatory download-server flow and introducing conditional logic across multiple configuration helpers to branch between these installation modes. Changes
Sequence DiagramsequenceDiagram
participant ConfigFlow as Configuration Flow
participant Detector as Remote Detector<br/>(HasEmbeddedServer)
participant ConfigBuilder as Config Builder<br/>(ShouldUseRemoteUvx)
participant RemoteFlow as Remote Path<br/>(UVX + Git)
participant EmbeddedFlow as Embedded Path<br/>(Local UV + Server)
ConfigFlow->>Detector: Check ServerInstaller
alt Embedded Server Found
Detector-->>ConfigBuilder: useRemote = false
ConfigBuilder->>EmbeddedFlow: Validate pythonDir & uvPath
EmbeddedFlow-->>ConfigBuilder: Local command: uv + server.py
else No Embedded Server
Detector-->>ConfigBuilder: useRemote = true
ConfigBuilder->>RemoteFlow: Build remote registration
RemoteFlow-->>ConfigBuilder: Remote command: uvx --from<br/>git-url package-name
end
ConfigBuilder-->>ConfigFlow: Generated Config JSON
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes The PR introduces conditional branching logic across five core helper files with interconnected control flow, removes a significant installation workflow, and requires cross-file consistency verification. While the remote-vs-embedded pattern is repetitive, each implementation is context-specific and affects different aspects of the config pipeline (Codex, Claude, general CLI setup). Reviewers must trace the decision flow through multiple entry points and verify both branches handle edge cases (null values, missing directories, Windows env variables) correctly. Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
| "com.coplaydev.unity-mcp": "https://github.com/coplaydev/unity-mcp.git?path=MCPForUnity", | ||
| "com.unity.ai.navigation": "1.1.4", | ||
| "com.unity.collab-proxy": "2.5.2", | ||
| "com.coplaydev.unity-mcp": "file:../../../MCPForUnity", |
There was a problem hiding this comment.
@dsarno why was it changed to the Git URL? My test was failing and when I stepped through the code, it wasn't pointing to the new code I added in this PR. I then saw the manifest referenced the remote package.
This repo is how we run unit tests, I'm not sure how I can verify new tests locally if it's pointing to a remote Git package. Let me know if I'm missing something. We can always make more test projects for different use cases
| @@ -1,2 +1,2 @@ | |||
| m_EditorVersion: 2022.3.6f1 | |||
| m_EditorVersionWithRevision: 2022.3.6f1 (b9e6e7e9fa2d) | |||
| m_EditorVersion: 2022.3.62f2 | |||
There was a problem hiding this comment.
@dsarno sorry for not explaining some of the ideas with the test harness. I initially made the project with Unity 2021 because that was our minimum version supported. I'm not sure how many people still use so it may not be a big deal, but we lose a bit of coverage basing it on 2022.
In any case, I've updated it to the 2022 version w/o the security bug. Though it primarily affects builds, I figure it's good to be on the safe side in case someone uses this project in ways we don't intend them to
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (1)
179-221: Remote uvx configs may be misclassified as IncorrectPathCheckClientStatus infers “Configured” by matching a local directory arg. Remote uvx configs (uvx --from mcp-for-unity) have no directory, so configuredDir is null → mismatch → rewrite/IncorrectPath loops.
Detect remote uvx and treat as configured without directory matching:
- string configuredDir = McpConfigurationHelper.ExtractDirectoryArg(args); - bool matches = !string.IsNullOrEmpty(configuredDir) && - McpConfigurationHelper.PathsEqual(configuredDir, pythonDir); + string configuredDir = McpConfigurationHelper.ExtractDirectoryArg(args); + bool isRemoteUvx = args != null && args.Any(a => + string.Equals(a, "--from", StringComparison.OrdinalIgnoreCase)); + bool matches = isRemoteUvx + ? true + : (!string.IsNullOrEmpty(configuredDir) && + McpConfigurationHelper.PathsEqual(configuredDir, pythonDir));Alternatively, add a dedicated helper (e.g., McpConfigurationHelper.IsRemoteUvxArgs(args)).
Note: apply similar handling for VSCode/standard branches above that also rely on args.
🧹 Nitpick comments (6)
TestProjects/UnityMCPTests/Packages/manifest.json (1)
10-10: Pin Git dependency to a tag or commit for reproducible tests.com.unity.ide.windsurf points to HEAD; pin to a tag/commit to avoid flakiness.
MCPForUnity/Editor/Helpers/CodexConfigHelper.cs (1)
126-141: Deduplicate remote/local decision logic.ShouldUseRemoteUvx is duplicated here and in ConfigJsonBuilder. Extract a shared helper (e.g., ConfigModeDecider.ShouldUseRemote(uvPath, serverSrc)) to keep behavior consistent.
Also applies to: 150-177
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
633-643: Avoid false “update available” when versions are unknownIf GetInstalledServerVersion() or GetPackageVersion() returns "unknown", the mismatch branch may show incorrectly.
Apply this minimal guard:
-else if (hasEmbedded && !string.IsNullOrEmpty(installedVer) && installedVer != packageVer) +else if (hasEmbedded + && !string.IsNullOrEmpty(installedVer) + && !string.Equals(installedVer, "unknown", StringComparison.OrdinalIgnoreCase) + && !string.Equals(packageVer, "unknown", StringComparison.OrdinalIgnoreCase) + && installedVer != packageVer)MCPForUnity/Editor/Services/ClientConfigurationService.cs (1)
265-267: Preflight uvx availability for Claude CLI pathThe args rely on uvx being resolvable on PATH. Consider checking pathService.IsUvDetected() (or surfacing a clearer error) before invoking CLAUDE to reduce opaque failures.
- args = $"mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; + args = $"mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; + // Optionally: validate uv/uvx beforehand and surface a targeted message if missing.TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs (2)
324-372: Remote uvx Build test: LGTM; consider adding a negative check for unknown version once fallback is implementedGood coverage for command/args/env on Windows. After adding remote URL fallback, you may assert that args[1] does not contain "@vunknown".
373-426: Remote uvx Upsert test: add subdirectory assertion for completenessConsider asserting the subdirectory fragment as in the Build test to ensure parity.
- Assert.IsTrue(unityMcp.TryGetNode("args", out var argsNode), "unityMCP should contain args"); + Assert.IsTrue(unityMcp.TryGetNode("args", out var argsNode), "unityMCP should contain args"); var argsArray = argsNode as TomlArray; Assert.IsNotNull(argsArray, "args should be an array"); Assert.AreEqual(3, argsArray.ChildrenCount, "Remote uvx should have 3 args"); var args = argsArray.Children.OfType<TomlString>().Select(s => s.Value).ToArray(); Assert.AreEqual("--from", args[0], "First arg should be --from"); - Assert.IsTrue(args[1].StartsWith("git+https://github.com/CoplayDev/unity-mcp@v"), "Second arg should be git URL"); + Assert.IsTrue(args[1].StartsWith("git+https://github.com/CoplayDev/unity-mcp@"), "Second arg should be git URL"); + Assert.IsTrue(args[1].Contains("#subdirectory=MCPForUnity/UnityMcpServer~/src"), "URL should include subdirectory"); Assert.AreEqual("mcp-for-unity", args[2], "Third arg should be package name");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
MCPForUnity/Editor/Helpers/CodexConfigHelper.cs(1 hunks)MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs(2 hunks)MCPForUnity/Editor/Helpers/McpConfigurationHelper.cs(2 hunks)MCPForUnity/Editor/Helpers/ServerInstaller.cs(0 hunks)MCPForUnity/Editor/Services/ClientConfigurationService.cs(4 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs(1 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml(0 hunks)MCPForUnity/UnityMcpServer~/src/server.py(1 hunks)TestProjects/UnityMCPTests/Assets/Editor.meta(1 hunks)TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs(2 hunks)TestProjects/UnityMCPTests/Packages/manifest.json(1 hunks)TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt(1 hunks)
💤 Files with no reviewable changes (2)
- MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.uxml
- MCPForUnity/Editor/Helpers/ServerInstaller.cs
🧰 Additional context used
🧬 Code graph analysis (6)
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs (1)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (2)
ServerInstaller(14-898)HasEmbeddedServer(873-876)
MCPForUnity/Editor/Helpers/McpConfigurationHelper.cs (1)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (3)
ServerInstaller(14-898)HasEmbeddedServer(873-876)FindUvPath(654-842)
MCPForUnity/Editor/Helpers/CodexConfigHelper.cs (2)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (2)
ServerInstaller(14-898)HasEmbeddedServer(873-876)MCPForUnity/Editor/Helpers/PackageLifecycleManager.cs (1)
GetPackageVersion(99-115)
TestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs (3)
MCPForUnity/Editor/Services/MCPServiceLocator.cs (2)
MCPServiceLocator(8-76)Register(33-51)MCPForUnity/Editor/Helpers/CodexConfigHelper.cs (3)
CodexConfigHelper(15-259)TomlTable(103-124)TomlTable(147-190)UnityMcpBridge/Editor/External/Tommy.cs (10)
TomlTable(421-560)TomlTable(648-852)TomlTable(1668-1757)TomlTable(1780-1784)TOML(1776-1785)TryGetNode(93-97)TryGetNode(448-448)TomlString(168-193)TomlArray(310-419)TomlArray(1215-1283)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
MCPForUnity/Editor/Helpers/PackageLifecycleManager.cs (2)
PackageLifecycleManager(19-23)GetLastInstallError(208-221)
MCPForUnity/Editor/Services/ClientConfigurationService.cs (4)
MCPForUnity/Editor/Helpers/ServerInstaller.cs (2)
ServerInstaller(14-898)HasEmbeddedServer(873-876)MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
AssetPathUtility(13-161)GetPackageVersion(142-160)MCPForUnity/Editor/Services/IPathResolverService.cs (1)
GetUvPath(18-18)MCPForUnity/Editor/Services/PathResolverService.cs (1)
GetUvPath(36-54)
🔇 Additional comments (11)
TestProjects/UnityMCPTests/Assets/Editor.meta (1)
1-8: Meta file — no code changes to review.This is a Unity-generated metadata file for folder structure, not executable code. The format is correct. However, this file's presence indicates structural changes to the test project that should be reviewed in context of the actual code implementation.
Per the AI summary, this accompanies broader changes to
ConfigJsonBuilder.cs,McpConfigurationHelper.cs,CodexConfigHelper.cs, and related test files introducing remote uvx vs. local server logic. To fully validate this PR, please ensure those implementation files are reviewed to verify:
- Correct conditional logic between remote uvx (via Asset Store) and local embedded installations
- Proper argument construction for both installation modes
- Removal of the mandatory download-server flow
- Test coverage for both code paths
MCPForUnity/UnityMcpServer~/src/server.py (2)
171-174: Clarify the disconnect between PR objectives and visible changes.The PR title states "Remove asset store download button" and the description mentions removing the download button and prompting users to use
uvxinstead. However, the only visible changes in this file are documentation updates for new tools (apply_text_edits,script_apply_edits,validate_script).This raises the question: are there additional files in this PR that weren't included in the review? The AI summary mentions "refactors the MCP server configuration system to support both remote uvx-based installations and embedded local installations," but none of those changes are visible in this file.
Could you confirm whether:
- Additional files with UI/configuration changes are part of this PR but not included in the review?
- The PR objectives accurately reflect the scope of changes?
171-174: All documented tools are properly implemented and registered.Verification confirms that all three tools (
apply_text_edits,script_apply_edits,validate_script) are fully implemented in the tools module with@mcp_for_unity_tooldecorators, enabling auto-registration viaregister_all_tools(). Each tool has comprehensive test coverage and the documentation in lines 172-174 accurately describes their capabilities.TestProjects/UnityMCPTests/ProjectSettings/ProjectVersion.txt (1)
1-2: Verify CI/editor availability for 2022.3.62f2.Ensure build agents/installers have Unity 2022.3.62f2; update editor matrix if needed.
TestProjects/UnityMCPTests/Packages/manifest.json (1)
3-3: Confirm CI path correctness for local package source.file:../../../MCPForUnity must resolve on CI runners; otherwise package import fails.
MCPForUnity/Editor/Helpers/CodexConfigHelper.cs (1)
154-165: Validate remote tag naming (“v{version}”) consistency.Same concern as JSON path; ensure tags match AssetPathUtility.GetPackageVersion().
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs (1)
74-79: Tag v6.2.5 exists and matches the version returned by GetPackageVersion().Verification confirms the concern is resolved: The
MCPForUnity/package.jsonspecifies version "6.2.5", whichAssetPathUtility.GetPackageVersion()retrieves and returns. The git tagv6.2.5exists in the repository, so the remote install URL will resolve correctly.MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
616-625: Rebuild visibility logic: LGTMShowing Rebuild only for embedded installs is clear and consistent with the new flow.
MCPForUnity/Editor/Services/ClientConfigurationService.cs (2)
420-434: Config gating: LGTMOnly requiring paths when not using remote is correct and aligns with the new flow.
30-33: Null handling for remote uvx flow is properly implemented—no changes needed.The helpers safely tolerate null
pythonDir:
ResolveServerDirectoryguards allPath.Combinecalls with null checks (lines 316–317, 319–320)- When
pythonDiris null, it falls back toResolveServerSource(), which uses system-level resolution (EditorPrefs, ServerInstaller) independent ofpythonDir- Both
ConfigureCodexClientandWriteMcpConfigurationhandle this flow without errorsTestProjects/UnityMCPTests/Assets/Tests/EditMode/Helpers/CodexConfigHelperTests.cs (1)
6-6: Import: LGTMSystem.Linq is required for the args projection in the new tests.
| bool useRemote = ShouldUseRemoteUvx(serverSrc); | ||
|
|
||
| var argsArray = new TomlArray(); | ||
| argsArray.Add(new TomlString { Value = "run" }); | ||
| argsArray.Add(new TomlString { Value = "--directory" }); | ||
| argsArray.Add(new TomlString { Value = serverSrc }); | ||
| argsArray.Add(new TomlString { Value = "server.py" }); | ||
| unityMCP["args"] = argsArray; | ||
| if (useRemote) | ||
| { | ||
| // Asset Store install - use remote uvx | ||
| string version = AssetPathUtility.GetPackageVersion(); | ||
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | ||
|
|
||
| unityMCP["command"] = new TomlString { Value = "uvx" }; | ||
|
|
||
| var argsArray = new TomlArray(); | ||
| argsArray.Add(new TomlString { Value = "--from" }); | ||
| argsArray.Add(new TomlString { Value = remoteUrl }); | ||
| argsArray.Add(new TomlString { Value = "mcp-for-unity" }); | ||
| unityMCP["args"] = argsArray; | ||
| } | ||
| else | ||
| { | ||
| // Git/embedded install - use local path | ||
| unityMCP["command"] = new TomlString { Value = uvPath }; | ||
|
|
||
| var argsArray = new TomlArray(); | ||
| argsArray.Add(new TomlString { Value = "run" }); | ||
| argsArray.Add(new TomlString { Value = "--directory" }); | ||
| argsArray.Add(new TomlString { Value = serverSrc }); | ||
| argsArray.Add(new TomlString { Value = "server.py" }); | ||
| unityMCP["args"] = argsArray; | ||
| } |
There was a problem hiding this comment.
Force remote when uvPath is null to avoid invalid TOML.
Mirror the JSON fix so command is never null.
Apply this diff:
- bool useRemote = ShouldUseRemoteUvx(serverSrc);
+ bool useRemote = string.IsNullOrEmpty(uvPath) || ShouldUseRemoteUvx(serverSrc);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool useRemote = ShouldUseRemoteUvx(serverSrc); | |
| var argsArray = new TomlArray(); | |
| argsArray.Add(new TomlString { Value = "run" }); | |
| argsArray.Add(new TomlString { Value = "--directory" }); | |
| argsArray.Add(new TomlString { Value = serverSrc }); | |
| argsArray.Add(new TomlString { Value = "server.py" }); | |
| unityMCP["args"] = argsArray; | |
| if (useRemote) | |
| { | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| unityMCP["command"] = new TomlString { Value = "uvx" }; | |
| var argsArray = new TomlArray(); | |
| argsArray.Add(new TomlString { Value = "--from" }); | |
| argsArray.Add(new TomlString { Value = remoteUrl }); | |
| argsArray.Add(new TomlString { Value = "mcp-for-unity" }); | |
| unityMCP["args"] = argsArray; | |
| } | |
| else | |
| { | |
| // Git/embedded install - use local path | |
| unityMCP["command"] = new TomlString { Value = uvPath }; | |
| var argsArray = new TomlArray(); | |
| argsArray.Add(new TomlString { Value = "run" }); | |
| argsArray.Add(new TomlString { Value = "--directory" }); | |
| argsArray.Add(new TomlString { Value = serverSrc }); | |
| argsArray.Add(new TomlString { Value = "server.py" }); | |
| unityMCP["args"] = argsArray; | |
| } | |
| bool useRemote = string.IsNullOrEmpty(uvPath) || ShouldUseRemoteUvx(serverSrc); | |
| if (useRemote) | |
| { | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| unityMCP["command"] = new TomlString { Value = "uvx" }; | |
| var argsArray = new TomlArray(); | |
| argsArray.Add(new TomlString { Value = "--from" }); | |
| argsArray.Add(new TomlString { Value = remoteUrl }); | |
| argsArray.Add(new TomlString { Value = "mcp-for-unity" }); | |
| unityMCP["args"] = argsArray; | |
| } | |
| else | |
| { | |
| // Git/embedded install - use local path | |
| unityMCP["command"] = new TomlString { Value = uvPath }; | |
| var argsArray = new TomlArray(); | |
| argsArray.Add(new TomlString { Value = "run" }); | |
| argsArray.Add(new TomlString { Value = "--directory" }); | |
| argsArray.Add(new TomlString { Value = serverSrc }); | |
| argsArray.Add(new TomlString { Value = "server.py" }); | |
| unityMCP["args"] = argsArray; | |
| } |
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Helpers/CodexConfigHelper.cs around lines 150-177, ensure
unityMCP["command"] is never set to null by forcing the remote branch when
uvPath is null: after computing bool useRemote = ShouldUseRemoteUvx(serverSrc),
set useRemote = true if string.IsNullOrEmpty(uvPath) (or fold into the initial
condition), so the code executes the remote branch which assigns
unityMCP["command"] = new TomlString { Value = "uvx" }; and avoid leaving
command null for the TOML output.
| // Check if we should use remote uvx (Asset Store without embedded server) | ||
| bool useRemote = ShouldUseRemoteUvx(directory); | ||
|
|
||
| // For Cursor (non-VSCode) on macOS, prefer a no-spaces symlink path to avoid arg parsing issues in some runners | ||
| string effectiveDir = directory; | ||
| if (useRemote) | ||
| { | ||
| // Asset Store install - use remote uvx | ||
| string version = AssetPathUtility.GetPackageVersion(); | ||
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | ||
|
|
||
| unity["command"] = "uvx"; | ||
| unity["args"] = JArray.FromObject(new[] { "--from", remoteUrl, "mcp-for-unity" }); | ||
| } |
There was a problem hiding this comment.
Prevent null command: force remote when uvPath is null.
If pythonDir is empty but an embedded server exists, builder may choose local while uvPath is null → writes command = null. Make useRemote depend on uvPath.
Apply this diff:
- // Check if we should use remote uvx (Asset Store without embedded server)
- bool useRemote = ShouldUseRemoteUvx(directory);
+ // Check if we should use remote uvx (also force remote if uvPath is null)
+ bool useRemote = string.IsNullOrEmpty(uvPath) || ShouldUseRemoteUvx(directory);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check if we should use remote uvx (Asset Store without embedded server) | |
| bool useRemote = ShouldUseRemoteUvx(directory); | |
| // For Cursor (non-VSCode) on macOS, prefer a no-spaces symlink path to avoid arg parsing issues in some runners | |
| string effectiveDir = directory; | |
| if (useRemote) | |
| { | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| unity["command"] = "uvx"; | |
| unity["args"] = JArray.FromObject(new[] { "--from", remoteUrl, "mcp-for-unity" }); | |
| } | |
| // Check if we should use remote uvx (also force remote if uvPath is null) | |
| bool useRemote = string.IsNullOrEmpty(uvPath) || ShouldUseRemoteUvx(directory); | |
| if (useRemote) | |
| { | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| unity["command"] = "uvx"; | |
| unity["args"] = JArray.FromObject(new[] { "--from", remoteUrl, "mcp-for-unity" }); | |
| } |
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs around lines 68 to 79, the
code can set unity["command"] = null when uvPath is empty because useRemote is
determined only by ShouldUseRemoteUvx(directory); change the useRemote
calculation to also force remote when the local uvx path (uvPath) is null or
empty (e.g., useRemote = ShouldUseRemoteUvx(directory) ||
string.IsNullOrEmpty(uvPath)), so the Asset Store branch is taken and a valid
command/args are written whenever uvPath is missing.
| if (useRemote) | ||
| { | ||
| // Asset Store install - use remote uvx | ||
| string version = AssetPathUtility.GetPackageVersion(); | ||
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | ||
|
|
||
| unity["command"] = "uvx"; | ||
| unity["args"] = JArray.FromObject(new[] { "--from", remoteUrl, "mcp-for-unity" }); | ||
| } |
There was a problem hiding this comment.
Add Windows env parity (SystemRoot) for JSON configs (remote and local).
TOML adds SystemRoot for Windows; JSON path does not. This breaks some Windows clients. Add env consistently.
Apply this diff (after setting command/args, before type handling):
+ // Windows env parity with TOML (issue #315): always include SystemRoot
+ try
+ {
+ var platform = MCPForUnity.Editor.Services.MCPServiceLocator.Platform;
+ if (platform.IsWindows())
+ {
+ var env = unity["env"] as JObject ?? new JObject();
+ env["SystemRoot"] = platform.GetSystemRoot();
+ unity["env"] = env;
+ }
+ }
+ catch { /* best-effort */ }And add the import at top:
+using MCPForUnity.Editor.Services;Also applies to: 126-127
There was a problem hiding this comment.
This is only needed for Codex, a bug on Codex requires this env var. Other clients, so far, do not need it
There was a problem hiding this comment.
@msanatan, thank you for the clarification! I understand now — the SystemRoot environment variable is a Codex-specific requirement due to a bug in that client, and since Codex uses TOML configuration (handled separately in CodexConfigHelper.cs), the JSON configuration path in ConfigJsonBuilder.cs doesn't need it. I'll skip this suggestion for the JSON configs.
✏️ Learnings added
Learnt from: msanatan
PR: CoplayDev/unity-mcp#348
File: MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs:71-79
Timestamp: 2025-10-24T14:09:08.594Z
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).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // 1) Check if we should use remote uvx (Asset Store without embedded server) | ||
| bool useRemote = !ServerInstaller.HasEmbeddedServer() || string.IsNullOrEmpty(pythonDir); | ||
|
|
||
| string uvPath = null; | ||
| if (!useRemote) | ||
| { | ||
| var name = Path.GetFileName((existingCommand ?? string.Empty).Trim()).ToLowerInvariant(); | ||
| if ((name == "uv" || name == "uv.exe") && IsValidUvBinary(existingCommand)) | ||
| // Git/embedded install - need UV path | ||
| uvPath = ServerInstaller.FindUvPath(); | ||
| // Optionally trust existingCommand if it looks like uv/uv.exe | ||
| try | ||
| { | ||
| uvPath = existingCommand; | ||
| var name = Path.GetFileName((existingCommand ?? string.Empty).Trim()).ToLowerInvariant(); | ||
| if ((name == "uv" || name == "uv.exe") && IsValidUvBinary(existingCommand)) | ||
| { | ||
| uvPath = existingCommand; | ||
| } | ||
| } | ||
| catch { } | ||
| if (uvPath == null) return "UV package manager not found. Please install UV first."; | ||
| } |
There was a problem hiding this comment.
Align remote/local decision with builders to avoid null-command configs.
When useRemote is true, pass null serverSrc to the builder so it cannot pick local while uvPath is null.
Apply this diff:
@@
- string uvPath = null;
+ string uvPath = null;
if (!useRemote)
{
@@
- string serverSrc = ResolveServerDirectory(pythonDir, existingArgs);
+ string serverSrc = ResolveServerDirectory(pythonDir, existingArgs);
+ // Force remote downstream if requested
+ string serverDirForConfig = useRemote ? null : serverSrc;
@@
- existingRoot = ConfigJsonBuilder.ApplyUnityServerToExistingConfig(existingRoot, uvPath, serverSrc, mcpClient);
+ existingRoot = ConfigJsonBuilder.ApplyUnityServerToExistingConfig(existingRoot, uvPath, serverDirForConfig, mcpClient);Also applies to: 117-128
🤖 Prompt for AI Agents
In MCPForUnity/Editor/Helpers/McpConfigurationHelper.cs around lines 96-115 and
similarly 117-128, the logic sets useRemote=true when no embedded server or
pythonDir is empty but still allows builders to infer a local server by passing
a non-null serverSrc; update the builder calls so that when useRemote is true
you explicitly pass serverSrc as null (instead of leaving it as an empty or
existing path) to prevent the builder from selecting a local UV installation
when uvPath is null; ensure this change is applied to both code blocks mentioned
so remote mode always supplies null serverSrc and avoids producing
null-command/local-selection configurations.
| // Check if we should use remote uvx (Asset Store without embedded server) | ||
| bool useRemote = !ServerInstaller.HasEmbeddedServer() || string.IsNullOrEmpty(pythonDir); | ||
|
|
||
| string uvPath = null; | ||
| if (!useRemote) | ||
| { | ||
| var name = Path.GetFileName((existingCommand ?? string.Empty).Trim()).ToLowerInvariant(); | ||
| if ((name == "uv" || name == "uv.exe") && IsValidUvBinary(existingCommand)) | ||
| // Git/embedded install - need UV path | ||
| uvPath = ServerInstaller.FindUvPath(); | ||
| try | ||
| { | ||
| uvPath = existingCommand; | ||
| var name = Path.GetFileName((existingCommand ?? string.Empty).Trim()).ToLowerInvariant(); | ||
| if ((name == "uv" || name == "uv.exe") && IsValidUvBinary(existingCommand)) | ||
| { | ||
| uvPath = existingCommand; | ||
| } | ||
| } | ||
| } | ||
| catch { } | ||
| catch { } | ||
|
|
||
| if (uvPath == null) | ||
| { | ||
| return "UV package manager not found. Please install UV first."; | ||
| if (uvPath == null) | ||
| { | ||
| return "UV package manager not found. Please install UV first."; | ||
| } | ||
| } |
There was a problem hiding this comment.
Same fix for Codex path: pass null serverSrc when remote.
Mirrors the JSON path; prevents TOML with command=null if builder picks local.
Apply this diff:
@@
- string uvPath = null;
+ string uvPath = null;
if (!useRemote)
{
@@
- string serverSrc = ResolveServerDirectory(pythonDir, existingArgs);
+ string serverSrc = ResolveServerDirectory(pythonDir, existingArgs);
+ string serverDirForToml = useRemote ? null : serverSrc;
@@
- string updatedToml = CodexConfigHelper.UpsertCodexServerBlock(existingToml, uvPath, serverSrc);
+ string updatedToml = CodexConfigHelper.UpsertCodexServerBlock(existingToml, uvPath, serverDirForToml);Also applies to: 200-215
🤖 Prompt for AI Agents
MCPForUnity/Editor/Helpers/McpConfigurationHelper.cs around lines 176-198 (and
also apply same change to 200-215): when useRemote is true you must pass a null
serverSrc for the Codex/TOML path generation the same way the JSON path does to
avoid producing TOML with command=null when the builder later selects a local
server; update the code that constructs the Codex path/builder call to pass
serverSrc = null when useRemote (mirror the JSON handling), and make the
identical change in the block at lines ~200-215.
| // Check if we should use remote uvx (Asset Store without embedded server) | ||
| bool useRemote = !ServerInstaller.HasEmbeddedServer(); | ||
| string args; | ||
|
|
||
| if (useRemote) | ||
| { | ||
| // Asset Store install - use remote uvx | ||
| string version = AssetPathUtility.GetPackageVersion(); | ||
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | ||
| args = $"mcp add UnityMCP -- uvx --from {remoteUrl} mcp-for-unity"; | ||
| } | ||
| else | ||
| { | ||
| // Git/embedded install - use local path | ||
| if (string.IsNullOrEmpty(pythonDir)) | ||
| { | ||
| throw new InvalidOperationException("Cannot register: Python directory not found"); | ||
| } | ||
|
|
||
| string uvPath = pathService.GetUvPath() ?? "uv"; | ||
| args = $"mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; | ||
| } |
There was a problem hiding this comment.
Remote URL uses tag v{version}; fallback missing → remote registration can fail
AssetPathUtility.GetPackageVersion() may return "unknown" or a version without a matching Git tag, producing an invalid @v{version} ref. Registration will fail.
- Add a robust fallback (e.g., main) when version is null/unknown or non-semver.
- Centralize remote URL construction to avoid drift.
Minimal inline fix:
- string version = AssetPathUtility.GetPackageVersion();
- string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src";
+ string version = AssetPathUtility.GetPackageVersion();
+ bool invalid = string.IsNullOrEmpty(version) || version.Equals("unknown", StringComparison.OrdinalIgnoreCase);
+ string remoteRef = invalid ? "main" : $"v{version}";
+ string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@{remoteRef}#subdirectory=MCPForUnity/UnityMcpServer~/src";Optional: extract a helper (e.g., RemoteUrlBuilder.GetUnityMcpServerRef()) and reuse here and in GenerateConfigJson/CodexConfigHelper to keep behavior consistent.
| bool useRemote = !ServerInstaller.HasEmbeddedServer(); | ||
|
|
||
| // Claude Code uses CLI commands, not JSON config | ||
| if (client.mcpType == McpTypes.ClaudeCode) | ||
| { | ||
| if (string.IsNullOrEmpty(pythonDir) || string.IsNullOrEmpty(uvPath)) | ||
| string registerCommand; | ||
| if (useRemote) | ||
| { | ||
| return "# Error: Configuration not available - check paths in Advanced Settings"; | ||
| // Asset Store install - use remote uvx | ||
| string version = AssetPathUtility.GetPackageVersion(); | ||
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | ||
| registerCommand = $"claude mcp add UnityMCP -- uvx --from {remoteUrl} mcp-for-unity"; | ||
| } | ||
| else | ||
| { | ||
| // Git/embedded install - use local path | ||
| if (string.IsNullOrEmpty(pythonDir) || string.IsNullOrEmpty(uvPath)) | ||
| { | ||
| return "# Error: Configuration not available - check paths in Advanced Settings"; | ||
| } | ||
| registerCommand = $"claude mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; | ||
| } |
There was a problem hiding this comment.
Same remote URL fallback needed in config JSON
Mirror the fallback logic here to avoid generating unusable commands when version is unknown or tagless.
- string version = AssetPathUtility.GetPackageVersion();
- string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src";
+ string version = AssetPathUtility.GetPackageVersion();
+ bool invalid = string.IsNullOrEmpty(version) || version.Equals("unknown", StringComparison.OrdinalIgnoreCase);
+ string remoteRef = invalid ? "main" : $"v{version}";
+ string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@{remoteRef}#subdirectory=MCPForUnity/UnityMcpServer~/src";Also consider centralizing this in a single helper shared across ClientConfigurationService and CodexConfigHelper.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bool useRemote = !ServerInstaller.HasEmbeddedServer(); | |
| // Claude Code uses CLI commands, not JSON config | |
| if (client.mcpType == McpTypes.ClaudeCode) | |
| { | |
| if (string.IsNullOrEmpty(pythonDir) || string.IsNullOrEmpty(uvPath)) | |
| string registerCommand; | |
| if (useRemote) | |
| { | |
| return "# Error: Configuration not available - check paths in Advanced Settings"; | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@v{version}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| registerCommand = $"claude mcp add UnityMCP -- uvx --from {remoteUrl} mcp-for-unity"; | |
| } | |
| else | |
| { | |
| // Git/embedded install - use local path | |
| if (string.IsNullOrEmpty(pythonDir) || string.IsNullOrEmpty(uvPath)) | |
| { | |
| return "# Error: Configuration not available - check paths in Advanced Settings"; | |
| } | |
| registerCommand = $"claude mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; | |
| } | |
| bool useRemote = !ServerInstaller.HasEmbeddedServer(); | |
| // Claude Code uses CLI commands, not JSON config | |
| if (client.mcpType == McpTypes.ClaudeCode) | |
| { | |
| string registerCommand; | |
| if (useRemote) | |
| { | |
| // Asset Store install - use remote uvx | |
| string version = AssetPathUtility.GetPackageVersion(); | |
| bool invalid = string.IsNullOrEmpty(version) || version.Equals("unknown", StringComparison.OrdinalIgnoreCase); | |
| string remoteRef = invalid ? "main" : $"v{version}"; | |
| string remoteUrl = $"git+https://github.com/CoplayDev/unity-mcp@{remoteRef}#subdirectory=MCPForUnity/UnityMcpServer~/src"; | |
| registerCommand = $"claude mcp add UnityMCP -- uvx --from {remoteUrl} mcp-for-unity"; | |
| } | |
| else | |
| { | |
| // Git/embedded install - use local path | |
| if (string.IsNullOrEmpty(pythonDir) || string.IsNullOrEmpty(uvPath)) | |
| { | |
| return "# Error: Configuration not available - check paths in Advanced Settings"; | |
| } | |
| registerCommand = $"claude mcp add UnityMCP -- \"{uvPath}\" run --directory \"{pythonDir}\" server.py"; | |
| } |
🤖 Prompt for AI Agents
MCPForUnity/Editor/Services/ClientConfigurationService.cs around lines 389-410:
the remoteUrl construction for Asset Store installs uses
AssetPathUtility.GetPackageVersion() without a fallback, which can produce an
unusable register command when the package version is missing; change the logic
to detect an empty or tagless version and fall back to the existing repository
subdirectory URL (or a known default branch/commit) so registerCommand remains
valid, and extract this URL/version resolution into a shared helper (e.g.,
GetRemoteUvxUrl(version)) so both ClientConfigurationService and
CodexConfigHelper call the same routine.
|
@msanatan Apologies, those are mistakes. I had changed the manifest by accident during testing last night, and used my mcp-source script to change it back, forgetting that it needs to be relative. Unity version was not deliberate either. I did that while fixing some tests, and then didn't revert. You can change it all back. |
No worries. You could have been doing an automated test runner or something, there are valid reasons for changing both, so I wanted to be sure I wasn't doing some of your work. I'll just leave as is if that's all right |
@msanatan but you should definitely fix the manifest pathing. And at some point, we should go back to 2021, that was always a good idea. I can do it later if you like. |
Sure, but treat it low priority. I don't think Unity can downgrade version, so you may have to create a new one and copy the files |
2 participants
When the user does not install via Git, they're instead prompted to set up MCP servers with
uvx. This can be the default moving forward, once I test the custom tools with this methodSummary by CodeRabbit
New Features
Tests
Chores