-
Notifications
You must be signed in to change notification settings - Fork 569
[FEATURE] Camera Capture #449
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.
WalkthroughAdds a screenshot capture feature: editor UI button, ManageScene ExecuteScreenshot API, ScreenshotUtility runtime helpers to save images to Assets/Screenshots, server-side parameter support for screenshot file name and supersize, and adjusted dev deployment script focusing on Unity Runtime assets. Changes
Sequence DiagramsequenceDiagram
participant User as Editor User
participant UI as McpToolsSection (UI)
participant ManageScene as ManageScene (Editor API)
participant ScreenshotUtil as ScreenshotUtility (Runtime)
participant Unity as Unity Engine
participant Assets as Assets/Screenshots
User->>UI: Click "Capture Screenshot"
UI->>ManageScene: ExecuteScreenshot(fileName, superSize)
ManageScene->>ManageScene: Build SceneCommand / decide Play vs Edit
alt Play Mode (from Camera)
ManageScene->>ScreenshotUtil: CaptureFromCameraToAssetsFolder(camera, fileName, superSize)
ScreenshotUtil->>Unity: Render Camera -> RenderTexture
else Edit Mode (screen)
ManageScene->>ScreenshotUtil: CaptureToAssetsFolder(fileName, superSize)
ScreenshotUtil->>Unity: ScreenCapture.CaptureScreenshot()
end
ScreenshotUtil->>ScreenshotUtil: BuildFileName / Sanitize / EnsureUnique
ScreenshotUtil->>Assets: Write PNG to Assets/Screenshots
ScreenshotUtil-->>ManageScene: ScreenshotCaptureResult(fullPath, assetsRelativePath, superSize)
ManageScene-->>UI: SuccessResponse / ErrorResponse with path info
UI-->>User: Log/display result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
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 |
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: 2
🧹 Nitpick comments (2)
deploy-dev.bat (1)
146-149: Outdated messaging refers to disabled server deployment.Step 2 mentions "Restart any MCP clients to use new Server code" but the server deployment is now disabled. Consider updating this message to reflect the current deployment scope.
echo Next steps: echo 1. Restart Unity Editor to load new Bridge code -echo 2. Restart any MCP clients to use new Server code +echo 2. (Server deployment disabled - deploy server manually if needed) echo 3. Use restore-dev.bat to rollback if neededMCPForUnity/Runtime/Helpers/ScreenshotUtility.cs (1)
64-120: Solid implementation with proper resource management.The method correctly renders the camera to a temporary RenderTexture and manually writes the PNG file, ensuring the screenshot is saved to the intended location. The try-finally block properly restores the camera state and releases the temporary RenderTexture.
Minor: The
Texture2Dcreated on line 98 is never destroyed. Consider adding cleanup to avoid a small memory leak:RenderTexture.active = rt; var tex = new Texture2D(width, height, TextureFormat.RGBA32, false); tex.ReadPixels(new Rect(0, 0, width, height), 0, 0); tex.Apply(); byte[] png = tex.EncodeToPNG(); + UnityEngine.Object.DestroyImmediate(tex); File.WriteAllBytes(normalizedFullPath, png); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
MCPForUnity/Editor/Tools/ManageScene.cs(5 hunks)MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs(3 hunks)MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs(1 hunks)MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs(1 hunks)Server/src/services/tools/manage_scene.py(2 hunks)deploy-dev.bat(3 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 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-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/Tools/ManageScene.csMCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.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/Tools/McpToolsSection.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:
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.csMCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs
📚 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/Editor/Windows/Components/Tools/McpToolsSection.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/Editor/Windows/MCPForUnityEditorWindow.cs
🧬 Code graph analysis (2)
MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs (1)
MCPForUnity/Editor/Tools/ManageScene.cs (1)
CaptureScreenshot(349-396)
MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (4)
MCPForUnity/Editor/Tools/ManageScene.cs (1)
ExecuteScreenshot(165-168)MCPForUnity/Editor/Helpers/McpLog.cs (3)
McpLog(7-52)Info(37-41)Error(48-51)MCPForUnity/Editor/Services/IToolDiscoveryService.cs (2)
ToolMetadata(8-22)ToolMetadata(49-49)MCPForUnity/Editor/Services/ToolDiscoveryService.cs (2)
ToolMetadata(62-70)ToolMetadata(107-157)
🔇 Additional comments (11)
MCPForUnity/Editor/Windows/MCPForUnityEditorWindow.cs (1)
1-363: LGTM!The EditorWindow implementation is well-structured with proper null checks, debouncing for refresh operations, and appropriate exception handling. The integration with the new
McpToolsSectionfor tools management is correctly wired up.MCPForUnity/Editor/Windows/Components/Tools/McpToolsSection.cs (2)
267-306: LGTM!The screenshot button integration is clean. The response handling correctly distinguishes between
SuccessResponseandErrorResponsetypes, and the fallback message provides reasonable UX. Exception handling ensures errors are logged appropriately.
315-315: LGTM!The helper correctly uses case-insensitive comparison and null-safe access.
Server/src/services/tools/manage_scene.py (1)
52-64: LGTM!The parameter coercion and mapping to camelCase (
fileName,superSize) correctly aligns with the Unity-sideSceneCommandproperties. The Unity implementation handles edge cases like zero or negativesuperSizevalues by defaulting to 1.MCPForUnity/Editor/Tools/ManageScene.cs (4)
349-396: LGTM!The screenshot implementation handles both Play mode and Edit mode appropriately:
- Defensive clamping of
superSizeto minimum 1- Proper camera fallback chain in Edit mode (
Camera.main→ first available camera)- Clear error message when no camera is available
AssetDatabase.Refresh()ensures the new file appears in the Project window
48-51: LGTM!Good defensive parsing that handles multiple key naming conventions (
camelCase,snake_case,lowercase), ensuring compatibility between the Python server and Unity client.
160-168: LGTM!Clean public API that enables code reuse between the command handler and the tools UI. The documentation clearly explains the dual-mode availability.
366-367: FindObjectsByType migration not applicable for Unity 2021.3 target.The project targets Unity 2021.3 (per
MCPForUnity/package.json), butFindObjectsByTypewas only introduced in Unity 2022.2+. WhileFindObjectsOfType<Camera>()is indeed deprecated in Unity 2023.1+, it remains the correct choice for maintaining 2021.3 compatibility. No refactor is needed unless the minimum Unity version requirement is increased to 2022.2 or later.Likely an incorrect or invalid review comment.
MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs (3)
9-21: LGTM: Clean, immutable result type.The readonly struct design with immutable properties is appropriate for a capture result. The structure clearly communicates the captured screenshot information.
122-179: LGTM: Helper methods are well-implemented.The helper methods handle filename generation, sanitization, uniqueness, and path resolution correctly. The logic is clear and follows standard patterns.
27-59: The fileMCPForUnity/Runtime/Helpers/ScreenshotUtility.csdoes not exist in the current repository. The review comment cannot be finalized without confirming the actual code state. However, the API behavior analysis in the review is technically correct: Unity'sScreenCapture.CaptureScreenshotwith a bare filename parameter will save relative to the project root (not the intendedAssets/Screenshotsfolder), and the call is asynchronous. The critical severity assessment is justified by official Unity documentation.
nitpicking changes
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 (5)
deploy-dev.bat (3)
31-32: SERVER_PATH now unused; consider cleaning up related config.With the server install path prompt disabled and
SERVER_PATHforced empty,DEFAULT_SERVER_PATHandSERVER_SOURCEat the top are effectively unused except in commented-out blocks. Consider removing them or adding a short comment indicating that server deployment is fully deprecated to avoid future confusion.
100-110: Legacy server backup block is safely disabled; consider full removal later.Commenting out the deprecated Python server backup is fine for now and clearly labeled as legacy. Longer term, you may want to delete this block entirely (and the now-unused server variables) once you are confident you won’t resurrect this deployment path.
129-137: Python server deploy disabled; align messaging with current behavior.Disabling the Python server deploy here is consistent with the earlier skipped server path and backup. However, the “Next steps” section still mentions restarting MCP clients “to use new Server code” even though this script no longer updates the server. Consider updating that message (or adding a brief note near these comments) so users don’t assume server bits were redeployed by this script.
Server/src/services/tools/manage_scene.py (2)
15-23: Newscreenshotaction wiring looks consistent with existing manage_scene actions.Extending the
Literalwith"screenshot"and updating the annotated description keeps the public surface self-describing and should integrate cleanly with existing tooling that inspects the action set. You may also want to consider updating the@mcp_for_unity_tooldescription string (Line 11) so it mentions screenshots as well, for consistency.
28-29: Screenshot parameters are clearly documented; check contract vs. validation.The new parameters are well-described and optional, and the fixed typo in the
screenshot_file_nameannotation makes the contract clearer. However,screenshot_super_sizeis documented as “integer ≥1” but there’s no explicit lower-bound check yet; invalid values (0, negatives) will be passed through if supplied.Consider clamping or rejecting values <1 (e.g., by returning an error or falling back to a safe default) so the runtime behavior always matches the annotated contract.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Server/src/services/tools/manage_scene.py(2 hunks)deploy-dev.bat(3 hunks)
🔇 Additional comments (4)
deploy-dev.bat (2)
90-98: Runtime backup block looks correct and consistent with Editor backup.The new Runtime backup mirrors the Editor backup semantics (guards on existence, uses
/E /I /Yand errorlevel checks) and writes into a clearUnityBridge\Runtime\subfolder. No issues from a robustness standpoint.
121-127: Runtime deployment command is correctly formed (including trailing backslash).The new Runtime deployment mirrors the Editor deploy (
xcopywith wildcard source and a directory-terminated destination) and includes proper error handling with!errorlevel!checks. This resolves prior ambiguity about the destination being a file vs. directory.Server/src/services/tools/manage_scene.py (2)
52-52: Coercion forscreenshot_super_sizematches existing numeric handling.Reusing
_coerce_intforscreenshot_super_sizekeeps parsing rules consistent withbuild_index(handles strings, trims whitespace, filters out obvious “null” tokens, and ignores booleans). This is a reasonable defensive approach.
54-54: Parameter payload construction for screenshot fields is clean and conditional.Building
paramsfromactionplus only non-empty fields (fileNameandsuperSizeadded conditionally,coerced_super_sizegated onis not None) keeps the payload minimal and avoids sending meaningless values. This should also preserve behavior for non-screenshot actions.Also applies to: 61-64
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.
The user can access the camera access through the tool menu or through direct LLM calling. Both were tested on Windows with Claude Desktop.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.