-
Notifications
You must be signed in to change notification settings - Fork 569
[FEATURE] Deployment of local source code to Unity #450
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
Conversation
* Update the .Bat file to include runtime folder * Fix the inconsistent EditorPrefs variable so the GUI change on Script Validation could cause real change.
String to Int for consistency
Allows users to generate/compile codes during Playmode
Upload the unity_claude_skill that can be uploaded to Claude for a combo of unity-mcp-skill.
1. Fix Original Roslyn Compilation Custom Tool to fit the V8 standard 2. Add a new panel in the GUI to see and toggle/untoggle the tools. The toggle feature will be implemented in the future, right now its implemented here to discuss with the team if this is a good feature to add; 3. Add few missing summary in certain tools
This reverts commit ae8cfe5.
This reverts commit f423c2f.
Merge and use the previous PR's solution to solve the windows issue.
Fix the layout problem of manage_script in the panel
Tested object generation/modification with batch and it works perfectly! We should push and let users test for a while and see PS: I tried both VS Copilot and Claude Desktop. Claude Desktop works but VS Copilot does not due to the nested structure of batch. Will look into it more.
Update on Batch
* Enable Camera Capture through both play and editor mode Notes: Because the standard ScreenCapture.CaptureScreenshot does not work in editor mode, so we use ScreenCapture.CaptureScreenshotIntoRenderTexture to enable it during play mode. * user can access the camera access through the tool menu, or through direct LLM calling. Both tested on Windows with Claude Desktop.
nitpicking changes
Similar to deploy.bat, but sideload it to MCP For Unity for easier deployment inside Unity menu.
|
Warning Rate limit exceeded@Scriptwonder has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughAdds a local package deployment feature: new EditorPrefs keys, an IPackageDeploymentService interface and PackageDeploymentService implementation, service-locator integration, and UI elements to select, deploy, and restore local MCPForUnity packages with backup tracking. (44 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as McpSettingsSection
participant Locator as MCPServiceLocator
participant Service as PackageDeploymentService
participant Prefs as EditorPrefs
participant FS as Filesystem
participant Editor as EditorUtility/AssetDatabase
User->>UI: Click "Deploy to Project"
UI->>Locator: request Deployment service
Locator->>Service: DeployFromStoredSource()
Service->>Prefs: GetStoredSourcePath()
Prefs-->>Service: stored source path
Service->>FS: Validate source path exists
FS-->>Service: validation result
alt source valid
Service->>FS: Create backup of target
FS-->>Service: backup path
Service->>FS: Replace target with source content (copy/replace)
FS-->>Service: success/failure
Service->>Prefs: Set last backup/target/source keys
Service->>Editor: AssetDatabase.Refresh()
Editor-->>Service: refreshed
Service-->>UI: PackageDeploymentResult (Success)
else invalid
Service-->>UI: PackageDeploymentResult (Fail + message)
end
UI->>User: Update status display
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
MCPForUnity/Editor/Services/IPackageDeploymentService.cs (1)
1-2: Remove unusedusing System;directive.The
Systemnamespace is not used in this file.-using System; - namespace MCPForUnity.Editor.ServicesMCPForUnity/Editor/Services/PackageDeploymentService.cs (1)
197-212: Consider adding backup cleanup to prevent accumulation.Backups are stored in
Library/MCPForUnityDeployBackupsbut are never cleaned up. Over time, this could accumulate significant disk space. Consider adding a cleanup mechanism to remove old backups (e.g., keep only the last N backups or backups from the last M days).This is a nice-to-have improvement that could be addressed in a follow-up.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MCPForUnity/Editor/Constants/EditorPrefKeys.cs(1 hunks)MCPForUnity/Editor/Services/IPackageDeploymentService.cs(1 hunks)MCPForUnity/Editor/Services/MCPServiceLocator.cs(5 hunks)MCPForUnity/Editor/Services/PackageDeploymentService.cs(1 hunks)MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs(6 hunks)MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/Constants/EditorPrefKeys.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/Windows/Components/Settings/McpSettingsSection.uxmlMCPForUnity/Editor/Services/PackageDeploymentService.csMCPForUnity/Editor/Services/IPackageDeploymentService.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 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:
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml
📚 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:
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml
🧬 Code graph analysis (4)
MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPForUnity/Editor/Services/PackageDeploymentService.cs (1)
PackageDeploymentService(14-302)
MCPForUnity/Editor/Services/PackageDeploymentService.cs (4)
MCPForUnity/Editor/Services/IPackageDeploymentService.cs (10)
GetStoredSourcePath(7-7)SetStoredSourcePath(8-8)ClearStoredSourcePath(9-9)GetTargetPath(11-11)GetTargetDisplayPath(12-12)GetLastBackupPath(14-14)HasBackup(15-15)PackageDeploymentResult(17-17)PackageDeploymentResult(18-18)PackageDeploymentResult(21-28)MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
EditorPrefKeys(7-48)MCPForUnity/Editor/Helpers/AssetPathUtility.cs (2)
AssetPathUtility(15-201)GetMcpPackageRootPath(41-80)MCPForUnity/Editor/Helpers/McpLog.cs (2)
McpLog(7-52)Error(48-51)
MCPForUnity/Editor/Services/IPackageDeploymentService.cs (1)
MCPForUnity/Editor/Services/PackageDeploymentService.cs (11)
GetStoredSourcePath(18-21)SetStoredSourcePath(23-27)ClearStoredSourcePath(29-32)GetTargetPath(34-67)GetTargetDisplayPath(69-75)GetLastBackupPath(77-80)HasBackup(82-86)PackageDeploymentResult(88-137)PackageDeploymentResult(139-176)PackageDeploymentResult(282-292)PackageDeploymentResult(294-301)
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs (3)
MCPForUnity/Editor/Services/MCPServiceLocator.cs (1)
MCPServiceLocator(11-91)MCPForUnity/Editor/Services/IPackageDeploymentService.cs (6)
GetStoredSourcePath(7-7)GetTargetDisplayPath(12-12)GetLastBackupPath(14-14)HasBackup(15-15)SetStoredSourcePath(8-8)ClearStoredSourcePath(9-9)MCPForUnity/Editor/Services/PackageDeploymentService.cs (6)
GetStoredSourcePath(18-21)GetTargetDisplayPath(69-75)GetLastBackupPath(77-80)HasBackup(82-86)SetStoredSourcePath(23-27)ClearStoredSourcePath(29-32)
🔇 Additional comments (19)
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.uxml (1)
43-60: LGTM! Well-structured deployment UI section.The new Local Package Deployment subsection follows existing UI patterns and provides a complete workflow for selecting a source folder, deploying, and restoring backups. The element naming is consistent with the corresponding C# wiring.
MCPForUnity/Editor/Windows/Components/Settings/McpSettingsSection.cs (5)
80-87: LGTM!The UI element caching follows the established pattern in this file.
253-273: LGTM!The method correctly updates all deployment UI elements and uses null-conditional access for the restore button. The approach is consistent with the existing code patterns in this file.
275-295: LGTM!Good error handling with user feedback via both dialog and status label. The UI is properly updated after the action.
304-319: LGTM!The deploy handler provides clear user feedback through both status labels and dialogs, with proper differentiation between success and failure states.
338-349: LGTM!Good null safety and the color handling follows the existing pattern used in
UpdateVersionLabel.MCPForUnity/Editor/Services/MCPServiceLocator.cs (2)
22-22: LGTM!The deployment service integration follows the established patterns for lazy initialization and service exposure.
Also applies to: 33-33
58-59: LGTM!Registration, disposal, and reset patterns are consistent with how other services are handled in this locator.
Also applies to: 78-78, 89-89
MCPForUnity/Editor/Constants/EditorPrefKeys.cs (1)
24-28: LGTM!The new EditorPrefs keys follow the established naming convention with the "MCPForUnity." prefix and use consistent dot notation for the subcategory.
MCPForUnity/Editor/Services/IPackageDeploymentService.cs (1)
5-28: LGTM!The interface is well-defined with clear method signatures. The
PackageDeploymentResultclass provides a straightforward way to return deployment outcomes with relevant path information.MCPForUnity/Editor/Services/PackageDeploymentService.cs (9)
18-32: LGTM!Clean implementation of source path management with proper validation on set.
34-67: LGTM!The target path resolution has a robust fallback chain supporting Package Manager and Asset Store installations.
88-137: LGTM!The deployment flow is well-structured with proper validation, backup creation before destructive operations, and clear error handling. The progress bar provides good user feedback.
139-176: LGTM!The restore flow correctly handles the case where the stored target path no longer exists by falling back to
GetTargetPath. The backup is preserved after restore for safety.
178-185: Verify: OnlyEditorandRuntimefolders are copied.This method only deploys the
EditorandRuntimesubfolders, leaving other package files (e.g.,package.json,CHANGELOG.md) unchanged. This is likely intentional for development iteration where you want to preserve the installed package's metadata while updating only the code.If the intent is to copy the entire package contents, consider adding a flag or copying all contents. Otherwise, documenting this behavior in the UI help text would be beneficial.
187-195: LGTM!Uses Unity's
FileUtilfor cross-platform file operations.
214-251: LGTM!The dual-mode validation design (throw vs return error string) is well-suited for different call sites. Error messages are clear and actionable.
275-280: LGTM!Path normalization and comparison approach is appropriate for Unity's cross-platform path handling.
282-301: LGTM!Clean factory methods for result creation.
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
🧹 Nitpick comments (1)
MCPForUnity/Editor/Services/PackageDeploymentService.cs (1)
197-212: Consider adding backup cleanup to prevent disk space accumulation.Backups are stored in
Library/MCPForUnityDeployBackupswith timestamps and accumulate over time. While theLibrary/folder is typically excluded from version control and can be manually cleaned, consider adding automatic cleanup of old backups (e.g., keep only the last N backups) to prevent disk space issues for users who deploy frequently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
MCPForUnity/Editor/Services/PackageDeploymentService.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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:
MCPForUnity/Editor/Services/PackageDeploymentService.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:
MCPForUnity/Editor/Services/PackageDeploymentService.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/Services/PackageDeploymentService.cs
🔇 Additional comments (8)
MCPForUnity/Editor/Services/PackageDeploymentService.cs (8)
1-17: LGTM!The imports and class structure are appropriate for the package deployment functionality.
18-32: LGTM!The EditorPrefs management methods properly validate and normalize paths before storage.
34-67: LGTM!The target path resolution uses multiple fallback strategies to robustly locate the installed package, with appropriate null checks and directory existence validation.
139-176: LGTM!The restoration logic properly validates paths and handles errors. Note that the backup path persists in EditorPrefs after restoration, allowing users to restore multiple times from the same backup if needed.
214-251: LGTM!The source validation properly checks for the required Editor and Runtime subfolders and supports both exception-throwing and error-message modes for flexible error handling.
253-268: LGTM! The MakeAbsolute bug has been fixed.The previous issue with incorrectly stripping the "Assets/" prefix has been resolved. The current implementation correctly preserves the full asset path when combining with the project root, resulting in the correct absolute path.
For example,
Assets/MCPForUnitycorrectly resolves to/path/to/project/Assets/MCPForUnity.
270-301: LGTM!The utility methods for project root resolution, path comparison, and result object creation are well-implemented and appropriate for the deployment service.
88-137: The referenced file and code do not exist in this repository.The review comment references
MCPForUnity/Editor/Services/PackageDeploymentService.csand itsDeployFromStoredSource()method, but neither the file nor theCopyCoreFolders()method exist in the current codebase. The Services directory contains 12 service implementations, butPackageDeploymentServiceis not among them. This review comment may have been created for a different repository, branch, or a PR that has not yet added this code to the project.Likely an incorrect or invalid review comment.
|
Nice. It would be awesome to have a readme section for this. I have my own way of switching back and forth for dev stuff, but I'll use yours because it's probably better -- but a few instructions wouldd be great! |
Yes, I agree, just modify the section in Readme-Dev.md (did not find a perfect .md file to put these in, and I believe at some point we need to restructure the .md before it becomes too much of a burden). I think deploy.bat works fine for me as well, but I assume people would also like to interact with the menu to make changes. Let me know what you think about this one (maybe your way is better!), gonna merge it for now because I have some missing .meta files from the last merge |
Summary by CodeRabbit
New Features
UI
✏️ Tip: You can customize this high-level summary in your review settings.