Add distribution settings for Asset Store vs git defaults #404
Conversation
Introduce McpDistributionSettings ScriptableObject to configure different defaults for Asset Store and git distributions without code forking. Add skipSetupWindowWhenRemoteDefault flag to bypass setup wizard when shipping with hosted MCP URL. Replace hardcoded localhost:8080 defaults with configurable defaultHttpBaseUrl from distribution settings in HttpEndpointUtility and WebSocketTransportClient.
WalkthroughAdds a ScriptableObject-based distribution configuration (default HTTP base URL and skip-setup flag), replaces hardcoded defaults with values from that configuration, and conditions the setup window to skip when a remote default is active and not overridden by the user. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
15-18: Consider edge cases in remote detection logic.The
IsRemoteDefaultcheck only tests for "localhost" and "127.0.0.1". This may not catch other local addresses such as:
- IPv6 localhost (
::1)- All interfaces (
0.0.0.0)- Local network addresses (e.g.,
192.168.x.x,10.x.x.x)For the Asset Store vs git distribution use case, the current implementation is likely adequate. However, if broader local address detection becomes necessary, consider using
System.Net.IPAddress.IsLoopbackor checking private IP ranges.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MCPForUnity/Editor/Config.meta(1 hunks)MCPForUnity/Editor/Config/McpDistributionSettings.cs(1 hunks)MCPForUnity/Editor/Config/McpDistributionSettings.cs.meta(1 hunks)MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs(2 hunks)MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs(2 hunks)MCPForUnity/Editor/Setup/SetupWindowService.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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:
MCPForUnity/Editor/Config/McpDistributionSettings.cs.metaMCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.csMCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Config/McpDistributionSettings.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.csMCPForUnity/Editor/Config.meta
📚 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/Editor/Config/McpDistributionSettings.cs.metaMCPForUnity/Editor/Config.meta
📚 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:
MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.csMCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.cs
📚 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:
MCPForUnity/Editor/Setup/SetupWindowService.csMCPForUnity/Editor/Config/McpDistributionSettings.csMCPForUnity/Editor/Helpers/HttpEndpointUtility.cs
🧬 Code graph analysis (3)
MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
McpDistribution(21-47)
MCPForUnity/Editor/Setup/SetupWindowService.cs (3)
MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-40)MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
McpDistribution(21-47)MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Info(37-41)
MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs (1)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
McpDistribution(21-47)
🔇 Additional comments (6)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
26-46: LGTM: Lazy-loading pattern is appropriate for Unity editor context.The lazy-loading implementation correctly checks the cache, attempts resource loading, and falls back to runtime defaults. While not thread-safe (no
volatileor locking), this is acceptable since Unity's editor operates on a single thread.MCPForUnity/Editor/Services/Transport/Transports/WebSocketTransportClient.cs (1)
9-9: LGTM: Clean integration with distribution settings.The change correctly replaces the hardcoded default with the configurable value from
McpDistribution.Settings.defaultHttpBaseUrl. The existing URL validation logic (lines 674-677) ensures invalid values are caught, making this integration safe.Also applies to: 671-671
MCPForUnity/Editor/Config.meta (1)
1-8: Standard Unity folder metadata.This is standard Unity-generated metadata for the new Config folder with no issues.
MCPForUnity/Editor/Helpers/HttpEndpointUtility.cs (1)
4-4: LGTM: Proper integration with distribution settings.Converting
DefaultBaseUrlfrom a constant to a property that reads fromMcpDistribution.Settings.defaultHttpBaseUrlenables runtime configuration while maintaining backward compatibility. The property is private, so this is an implementation detail with no API surface changes.Also applies to: 16-16
MCPForUnity/Editor/Config/McpDistributionSettings.cs.meta (1)
1-11: Standard Unity script metadata.This is standard Unity-generated metadata for the C# source file with no issues.
MCPForUnity/Editor/Setup/SetupWindowService.cs (1)
48-57: LGTM: Well-designed conditional setup skip logic.The implementation correctly handles the Asset Store distribution scenario:
EditorPrefs.HasKeyproperly detects user overrides (even if set to the same value)- The three-condition check ensures the setup window is only skipped when:
- User hasn't customized the URL
- Distribution is configured to skip setup for remote defaults
- The default URL is actually remote (not localhost)
- The log message clearly communicates the skip reason and provides guidance for manual setup access
- Using
always: falseappropriately respects debug logging preferencesThis enables Asset Store distributions to ship with hosted MCP URLs while preserving user choice and git/dev workflow.
…ehensive IP range checks Replace simple string-based localhost/127.0.0.1 checks with robust IsLocalAddress method that validates loopback addresses, private IPv4 ranges (10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 169.254.0.0/16), IPv6 link-local and loopback addresses, and .local hostnames using proper URI parsing and IPAddress validation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (2)
49-59: Consider treating invalid/relative URLs as non‑remote to avoid skipping setup.Right now, if
Uri.TryCreatefails,IsLocalAddressreturnsfalse, which makesIsRemoteDefaulttruefor any non‑empty but invalid URL. That means a broken default URL can be treated as a “remote default” and may skip the setup flow.If you’d rather err on the side of showing setup when the URL is bad, invert that behavior:
- if (!Uri.TryCreate(url, UriKind.Absolute, out var uri)) - { - return false; - } + if (!Uri.TryCreate(url, UriKind.Absolute, out var uri)) + { + // If it's not even a valid absolute URI, don't consider it a remote default. + // This keeps the setup window visible for obviously broken values. + return true; + }Please double‑check this against how
skipSetupWindowWhenRemoteDefaultis consumed so the UX matches your intent.
27-47: Optionally log when falling back to runtime defaults.When no
McpDistributionSettingsasset exists in Resources, you silently create a runtime default. That’s fine behavior, but a one‑time log can make misconfiguration easier to spot in editor logs.- // No asset present (git/dev installs) - fall back to baked-in defaults. - _cached = ScriptableObject.CreateInstance<McpDistributionSettings>(); - _cached.name = "McpDistributionSettings (Runtime Defaults)"; + // No asset present (git/dev installs) - fall back to baked-in defaults. + _cached = ScriptableObject.CreateInstance<McpDistributionSettings>(); + _cached.name = "McpDistributionSettings (Runtime Defaults)"; +#if UNITY_EDITOR + Debug.LogWarning("McpDistributionSettings asset not found in a Resources folder; using runtime defaults."); +#endif return _cached;This keeps the current behavior but makes configuration issues more discoverable in the Editor.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MCPForUnity/Editor/Config/McpDistributionSettings.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
MCPForUnity/Editor/Config/McpDistributionSettings.cs (1)
MCPForUnity/Editor/Resources/Tests/GetTests.cs (1)
TryParse(79-104)
* 'main' of https://github.com/CoplayDev/unity-mcp: chore: bump version to 8.1.4 Fix Claude Windows config and CLI status refresh (CoplayDev#412) chore: bump version to 8.1.3 fix: restrict fastmcp version to avoid potential KeyError (CoplayDev#411) chore: bump version to 8.1.2 Revert project script change chore: bump version to 8.1.1 Fix CLI entry point path in pyproject.toml (CoplayDev#407) Fix manage prefabs (CoplayDev#405) Remove automatic version bumping from README files and switch to unversioned git URLs chore: bump version to 8.1.0 Add distribution settings for Asset Store vs git defaults (CoplayDev#404) Add CodeBuddy CLI configurator (CoplayDev#403) Fix stdio reloads (CoplayDev#402) Simplify MCP client configs (CoplayDev#401) chore: bump version to 8.0.1
1 participant
Introduce McpDistributionSettings ScriptableObject to configure different defaults for Asset Store and git distributions without code forking. Add skipSetupWindowWhenRemoteDefault flag to bypass setup wizard when shipping with hosted MCP URL. Replace hardcoded localhost:8080 defaults with configurable defaultHttpBaseUrl from distribution settings in HttpEndpointUtility and WebSocketTransportClient.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.