-
Notifications
You must be signed in to change notification settings - Fork 679
Support testpypi #639
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: beta
Are you sure you want to change the base?
Support testpypi #639
Conversation
- Add UseTestPyPI editor preference key - Add TestPyPI toggle to Advanced settings UI with tooltip - Configure uvx to use test.pypi.org when TestPyPI mode enabled - Skip version pinning in TestPyPI mode to get latest pre-release - Update ConfigJsonBuilder to handle TestPyPI index URL
Reviewer's GuideAdds an editor preference and advanced panel toggle to optionally use TestPyPI as the default index for the MCP server package, and wires this flag into the uvx argument construction so server installs can target TestPyPI instead of PyPI. Sequence diagram for TestPyPI toggle affecting uvx argument constructionsequenceDiagram
actor UnityDeveloper
participant McpAdvancedSection
participant EditorPrefs
participant ConfigJsonBuilder
UnityDeveloper->>McpAdvancedSection: Toggle useTestPyPIToggle
McpAdvancedSection->>EditorPrefs: SetBool(EditorPrefKeys.UseTestPyPI, value)
McpAdvancedSection->>McpAdvancedSection: OnHttpServerCommandUpdateRequested
UnityDeveloper->>ConfigJsonBuilder: BuildUvxArgs(fromUrl, packageName)
ConfigJsonBuilder->>EditorPrefs: GetBool(EditorPrefKeys.UseTestPyPI, false)
EditorPrefs-->>ConfigJsonBuilder: useTestPyPI
alt useTestPyPI is true
ConfigJsonBuilder->>ConfigJsonBuilder: Add --default-index https://test.pypi.org/simple/
ConfigJsonBuilder->>ConfigJsonBuilder: Add --from mcpforunityserver
else useTestPyPI is false
ConfigJsonBuilder->>ConfigJsonBuilder: Use fromUrl and packageName
end
Updated class diagram for TestPyPI support in MCP editor componentsclassDiagram
class McpAdvancedSection {
- Toggle debugLogsToggle
- Toggle devModeForceRefreshToggle
- Toggle useTestPyPIToggle
- void CacheUIElements()
- void InitializeUI()
- void RegisterCallbacks()
+ void UpdatePathOverrides()
}
class ConfigJsonBuilder {
- static IList~string~ BuildUvxArgs(string fromUrl, string packageName)
}
class EditorPrefKeys {
<<static>>
+ const string UseTestPyPI
}
class EditorPrefs {
<<external>>
+ static void SetBool(string key, bool value)
+ static bool GetBool(string key, bool defaultValue)
}
McpAdvancedSection ..> EditorPrefKeys : uses
McpAdvancedSection ..> EditorPrefs : reads_writes
ConfigJsonBuilder ..> EditorPrefKeys : uses
ConfigJsonBuilder ..> EditorPrefs : reads
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis pull request adds TestPyPI support to the MCP for Unity system, introducing a new EditorPref flag to allow users to optionally use the test PyPI registry. The feature includes UI controls, configuration logic to apply TestPyPI settings when building UVX arguments, and updated metadata for the OpenCodeConfigurator script. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- Several places assume
useTestPyPIToggleis non-null (e.g., setting.valueand registering callbacks inInitializeUI,RegisterCallbacks, andUpdatePathOverrides) without null checks, which could throw if the UXML element is missing; consider mirroring the null-guard pattern used when setting tooltips. - The TestPyPI URL and the unversioned package name (
"https://test.pypi.org/simple/"and"mcpforunityserver") are hardcoded insideBuildUvxArgs; consider extracting these into named constants to avoid duplication and make future changes easier.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Several places assume `useTestPyPIToggle` is non-null (e.g., setting `.value` and registering callbacks in `InitializeUI`, `RegisterCallbacks`, and `UpdatePathOverrides`) without null checks, which could throw if the UXML element is missing; consider mirroring the null-guard pattern used when setting tooltips.
- The TestPyPI URL and the unversioned package name (`"https://test.pypi.org/simple/"` and `"mcpforunityserver"`) are hardcoded inside `BuildUvxArgs`; consider extracting these into named constants to avoid duplication and make future changes easier.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
MCPForUnity/Editor/Helpers/ConfigJsonBuilder.cs (1)
173-190: Logic error: TestPyPI mode uses incorrect package name in uvx command.When
useTestPyPIis true, the code adds"mcpforunityserver"via--from mcpforunityserver(line 183) but then line 190 unconditionally adds"mcp-for-unity"as the package argument. This results in--from mcpforunityserver mcp-for-unity, which attempts to install the wrong package from TestPyPI.The comment on line 181 ("Use unversioned package name to get latest from TestPyPI") indicates the intent is to use
"mcpforunityserver"as the final package argument, not"mcp-for-unity". Either line 190 should be moved into theelseblock, orpackageNameshould be reassigned to"mcpforunityserver"in TestPyPI mode.
🧹 Nitpick comments (1)
MCPForUnity/Editor/Clients/Configurators/OpenCodeConfigurator.cs.meta (1)
1-11: Consider verifying if this metadata update is necessary for the PR.This file contains auto-generated Unity metadata for
OpenCodeConfigurator.cs. The change from a bare GUID to a fullMonoImporterblock is standard Unity behavior when assets are reimported (e.g., after Unity version changes or git operations).The metadata structure itself is correct with standard default values. However, since this PR focuses on TestPyPI support and
OpenCodeConfigurator.csdoesn't appear directly related to that feature based on the PR objectives, consider whether this file needs to be included in the PR or if it was unintentionally modified during development.
|
Thanks for adding this @msanatan! One concern: the Python server and C# client are tightly coupled, so if schemas diverge -- which they do anytime there's a significant server change -- things break silently. That's the state we're in right now for Since So we should flip the default to true on the beta branch:
That way beta "just works" out of the box, and we flip it back to false when releasing to main. |
|
@dsarno shouldn't it be the other way around? We default for release settings which most customers use, and do patches for beta testers? |
|
I thought that's what I'm suggesting? Default for most users is main with pypi -- or if you opt in to beta via the beta git link download, you get testpypi automatically (and not via a second opt in in the advanced settings, which this PR proposes). Maybe I'm not understanding something about the flow here? |
@msanatan are we intending to have the main download link default to beta? I think that's what we're doing. But that seems to defeat the point of users opting in to beta and makes everyone a beta tester. Either way, beta needs to default to testpypi for server or there will be a breaking mismatch between c# and server. https://github.com/CoplayDev/unity-mcp.git?path=/MCPForUnity <-- points to beta. |
|
@msanatan for a more concrete example: I just tested the "way it is right now" : Because we updated how manage_prefabs work a few days ago (on beta) and the schema changed, manage prefabs is currently broken on beta. The PyPi (older) version bundled with beta uses a schema that doesn't match with the C#:
The schemas are already out of sync. Beta C# expects modify_contents, PyPI 9.2.0 still has open_stage in its enum. Both directions are broken. This is why TestPyPI needs to be the default on beta, not opt-in. |
Description
Type of Change
Save your change type
Changes Made
Testing/Screenshots/Recordings
Documentation Updates
tools/UPDATE_DOCS_PROMPT.md(recommended)tools/UPDATE_DOCS.mdRelated Issues
Additional Notes
Summary by Sourcery
Add configurable support for using TestPyPI as the package source for the MCP server and persist this preference in editor settings.
New Features:
Enhancements:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.