fix: include UI Toolkit overlays in game_view screenshots with include_image#1040
Open
KennerMiner wants to merge 2 commits intoCoplayDev:betafrom
Open
fix: include UI Toolkit overlays in game_view screenshots with include_image#1040KennerMiner wants to merge 2 commits intoCoplayDev:betafrom
KennerMiner wants to merge 2 commits intoCoplayDev:betafrom
Conversation
Use ScreenCapture.CaptureScreenshotAsTexture() for game_view screenshots when include_image=true and in Play mode. This captures the final composited frame including UI Toolkit overlays, which camera.Render() misses since UI Toolkit renders at the compositor level after camera rendering. The camera-based path is still used when a specific camera is requested or when not in Play mode. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Contributor
Reviewer's GuideAdjusts game_view screenshot capture when include_image is requested to use a composited ScreenCapture path in Play Mode (so UI Toolkit overlays are included), while preserving camera-based capture behavior for explicit camera targets and non-Play-Mode screenshots. Sequence diagram for include_image game_view screenshot in Play ModesequenceDiagram
actor Caller
participant ManageScene as ManageScene_CaptureScreenshot
participant ScreenshotUtility
participant ScreenCapture
participant FileIO as FileSystem
Caller->>ManageScene: manage_scene(action=screenshot, capture_source=game_view, include_image=true)
ManageScene->>ManageScene: Resolve targetCamera
alt targetCamera is not null
ManageScene->>ManageScene: EnsureGameView
ManageScene->>ScreenshotUtility: CaptureFromCameraToAssetsFolder(targetCamera, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult(result)
else targetCamera is null and includeImage true and Application.isPlaying true
ManageScene->>ManageScene: EnsureGameView
ManageScene->>ScreenshotUtility: CaptureComposited(fileName, superSize, ensureUniqueFileName, includeImage=true, maxResolution)
ScreenshotUtility->>ScreenCapture: CaptureScreenshotAsTexture(superSize)
ScreenCapture-->>ScreenshotUtility: Texture2D(tex)
alt tex is null
ScreenshotUtility->>ScreenshotUtility: Get Camera.main
alt Camera.main not null
ScreenshotUtility->>ScreenshotUtility: CaptureFromCameraToAssetsFolder(Camera.main, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult(result)
else Camera.main null
ScreenshotUtility-->>ManageScene: throw InvalidOperationException
end
else tex is valid
ScreenshotUtility->>ScreenshotUtility: EncodeToPNG(tex)
ScreenshotUtility->>FileIO: WriteAllBytes(result.FullPath, png)
alt includeImage true
ScreenshotUtility->>ScreenshotUtility: Optional downscale and base64 encode
end
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult(result)
end
else includeImage true and not Application.isPlaying
ManageScene->>ManageScene: Find Camera.main or any Camera
alt camera found
ManageScene->>ManageScene: EnsureGameView
ManageScene->>ScreenshotUtility: CaptureFromCameraToAssetsFolder(targetCamera, fileName, superSize, ensureUniqueFileName, includeImage=true, maxResolution)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult(result)
else no camera found
ManageScene-->>Caller: ErrorResponse(No camera found outside Play mode)
end
else includeImage false
ManageScene->>ScreenshotUtility: Existing non inline image capture path
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult(result)
end
opt success with image
ManageScene->>ManageScene: ImportAsset(result.AssetsRelativePath)
ManageScene-->>Caller: SuccessResponse(path, imageBase64, metadata)
end
Class diagram for ScreenshotUtility composited capture changesclassDiagram
class ScreenshotUtility {
+ScreenshotCaptureResult CaptureFromCameraToAssetsFolder(Camera targetCamera, string fileName, int superSize, bool ensureUniqueFileName, bool includeImage, int maxResolution)
+ScreenshotCaptureResult CaptureComposited(string fileName, int superSize, bool ensureUniqueFileName, bool includeImage, int maxResolution)
-ScreenshotCaptureResult PrepareCaptureResult(string fileName, int superSize, bool ensureUniqueFileName, bool isAsync)
-Texture2D DownscaleTexture(Texture2D source, int maxResolution)
-void DestroyTexture(Texture2D texture)
}
class ScreenshotCaptureResult {
+string FullPath
+string AssetsRelativePath
+int SuperSize
+bool IsAsync
+string ImageBase64
+int ImageWidth
+int ImageHeight
+ScreenshotCaptureResult(string fullPath, string assetsRelativePath, int superSize, bool isAsync, string imageBase64, int imageWidth, int imageHeight)
}
class Camera {
+string name
+static Camera main
}
class ScreenCapture {
+static Texture2D CaptureScreenshotAsTexture(int superSize)
}
class Texture2D {
+int width
+int height
+byte[] EncodeToPNG()
}
class FileSystem {
+static void WriteAllBytes(string path, byte[] bytes)
}
ScreenshotUtility --> ScreenshotCaptureResult : returns
ScreenshotUtility --> Camera : uses
ScreenshotUtility --> ScreenCapture : uses
ScreenshotUtility --> Texture2D : creates
ScreenshotUtility --> FileSystem : writes PNG files
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughCaptureScreenshot now chooses composited frame capture in Play mode when inline images are requested and no explicit camera is specified; explicit camera requests still use camera-based capture. A new ScreenshotUtility.CaptureComposited(...) was added with fallbacks and optional inline Base64 image encoding. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ManageScene
participant ScreenshotUtility
participant ScreenCapture
participant Camera
participant AssetDB
Client->>ManageScene: manage_scene(action="screenshot", includeImage, targetCamera?)
ManageScene-->>ManageScene: decide capture strategy
alt explicit targetCamera resolved
ManageScene->>ScreenshotUtility: CaptureFromCameraToAssetsFolder(camera)
ScreenshotUtility->>Camera: render & read pixels
ScreenshotUtility->>AssetDB: write file + ImportAsset(ForceSynchronousImport)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult (maybe imageBase64)
else includeImage && Application.isPlaying
ManageScene->>ScreenshotUtility: CaptureComposited(...)
ScreenshotUtility->>ScreenCapture: CaptureScreenshotAsTexture()
alt ScreenCapture returned texture
ScreenshotUtility->>AssetDB: write file + ImportAsset(ForceSynchronousImport)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult (maybe imageBase64)
else fallback to camera
ScreenshotUtility->>Camera: CaptureFromCameraToAssetsFolder(camera)
ScreenshotUtility->>AssetDB: write file + ImportAsset(ForceSynchronousImport)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult
end
else (no Play mode, no explicit camera)
ManageScene->>ScreenshotUtility: CaptureFromCameraToAssetsFolder(Camera.main or first)
ScreenshotUtility->>Camera: render & read pixels
ScreenshotUtility->>AssetDB: write file + ImportAsset(ForceSynchronousImport)
ScreenshotUtility-->>ManageScene: ScreenshotCaptureResult
end
ManageScene-->>Client: response JSON (camera, path, optional imageBase64/dimensions)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The success response construction for camera-based and composited captures in
CaptureScreenshotis nearly identical; consider extracting a small helper to build theSuccessResponse/data dictionary to reduce duplication and keep the two paths in sync. - In
ScreenshotUtility.CaptureComposited, theInvalidOperationExceptionwhen bothScreenCaptureandCamera.mainare unavailable will bubble up differently than the camera-based paths that return error responses; consider aligning the error-handling strategy so callers see consistent behavior in failure cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The success response construction for camera-based and composited captures in `CaptureScreenshot` is nearly identical; consider extracting a small helper to build the `SuccessResponse`/data dictionary to reduce duplication and keep the two paths in sync.
- In `ScreenshotUtility.CaptureComposited`, the `InvalidOperationException` when both `ScreenCapture` and `Camera.main` are unavailable will bubble up differently than the camera-based paths that return error responses; consider aligning the error-handling strategy so callers see consistent behavior in failure cases.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs (1)
231-298: NewCaptureCompositedmethod looks well-structured with proper resource cleanup.Two observations for robustness:
Module availability check missing: Unlike
CaptureToAssetsFolder(line 111), this method callsScreenCapture.CaptureScreenshotAsTexturedirectly without first checkingIsScreenCaptureModuleAvailable. If the Screen Capture module is disabled, this could throw instead of gracefully falling back.Fallback uses only
Camera.main: The fallback at line 254 only checksCamera.main, whereasCaptureWithCameraFallback(line 135) usesFindAvailableCamera()which also searches for any camera in the scene. Consider using the same pattern for consistency.Suggested improvements
public static ScreenshotCaptureResult CaptureComposited( string fileName = null, int superSize = 1, bool ensureUniqueFileName = true, bool includeImage = false, int maxResolution = 0) { + // Early fallback if ScreenCapture module is unavailable + if (!IsScreenCaptureModuleAvailable) + { + var cam = FindAvailableCamera(); + if (cam != null) + return CaptureFromCameraToAssetsFolder(cam, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution); + throw new InvalidOperationException("ScreenCapture module is unavailable and no fallback camera found."); + } + ScreenshotCaptureResult result = PrepareCaptureResult(fileName, superSize, ensureUniqueFileName, isAsync: false); Texture2D tex = null; Texture2D downscaled = null; string imageBase64 = null; int imgW = 0, imgH = 0; try { tex = ScreenCapture.CaptureScreenshotAsTexture(result.SuperSize); if (tex == null) { // Fallback to camera-based if ScreenCapture fails - var cam = Camera.main; + var cam = FindAvailableCamera(); if (cam != null) return CaptureFromCameraToAssetsFolder(cam, fileName, superSize, ensureUniqueFileName, includeImage, maxResolution); throw new InvalidOperationException("ScreenCapture.CaptureScreenshotAsTexture returned null and no fallback camera available."); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs` around lines 231 - 298, The method CaptureComposited calls ScreenCapture.CaptureScreenshotAsTexture directly and only falls back to Camera.main; update it to first check IsScreenCaptureModuleAvailable (like CaptureToAssetsFolder) and if the module is not available immediately call CaptureFromCameraToAssetsFolder using FindAvailableCamera() (reuse the same camera discovery logic as CaptureWithCameraFallback), and also change the null-result fallback after CaptureScreenshotAsTexture to use FindAvailableCamera() before throwing so both code paths are consistent and robust.MCPForUnity/Editor/Tools/ManageScene.cs (1)
606-621: Consider extracting the response-building logic to reduce duplication.The three capture branches (explicit camera, composited, and non-Play-mode fallback) all build nearly identical
Dictionary<string, object>payloads with the same keys. This is a minor code smell but acceptable for clarity in this context.Optional: Extract helper method
private static Dictionary<string, object> BuildScreenshotResponseData( ScreenshotCaptureResult result, string cameraName, bool includeImage) { var data = new Dictionary<string, object> { { "path", result.AssetsRelativePath }, { "fullPath", result.FullPath }, { "superSize", result.SuperSize }, { "isAsync", false }, { "camera", cameraName }, { "captureSource", "game_view" }, }; if (includeImage && result.ImageBase64 != null) { data["imageBase64"] = result.ImageBase64; data["imageWidth"] = result.ImageWidth; data["imageHeight"] = result.ImageHeight; } return data; }Also applies to: 636-651, 681-696
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@MCPForUnity/Editor/Tools/ManageScene.cs` around lines 606 - 621, Extract the repeated dictionary construction into a helper method (e.g., BuildScreenshotResponseData) that accepts a ScreenshotCaptureResult, a camera name string, and includeImage bool and returns Dictionary<string, object>; replace the three duplicated blocks that currently create the payload and pass that dictionary into SuccessResponse(message, data) (these blocks reference result, targetCamera.name or other camera name, includeImage and return new SuccessResponse) to call the helper instead to reduce duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@MCPForUnity/Editor/Tools/ManageScene.cs`:
- Around line 606-621: Extract the repeated dictionary construction into a
helper method (e.g., BuildScreenshotResponseData) that accepts a
ScreenshotCaptureResult, a camera name string, and includeImage bool and returns
Dictionary<string, object>; replace the three duplicated blocks that currently
create the payload and pass that dictionary into SuccessResponse(message, data)
(these blocks reference result, targetCamera.name or other camera name,
includeImage and return new SuccessResponse) to call the helper instead to
reduce duplication.
In `@MCPForUnity/Runtime/Helpers/ScreenshotUtility.cs`:
- Around line 231-298: The method CaptureComposited calls
ScreenCapture.CaptureScreenshotAsTexture directly and only falls back to
Camera.main; update it to first check IsScreenCaptureModuleAvailable (like
CaptureToAssetsFolder) and if the module is not available immediately call
CaptureFromCameraToAssetsFolder using FindAvailableCamera() (reuse the same
camera discovery logic as CaptureWithCameraFallback), and also change the
null-result fallback after CaptureScreenshotAsTexture to use
FindAvailableCamera() before throwing so both code paths are consistent and
robust.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89930cfb-d9e6-47b3-8798-99b236394ff8
📒 Files selected for processing (2)
MCPForUnity/Editor/Tools/ManageScene.csMCPForUnity/Runtime/Helpers/ScreenshotUtility.cs
Author
|
Addressed the follow-up feedback in Changes:
Validation:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix the Play Mode
game_viewscreenshot path used wheninclude_image=trueso the returned inline image includes UI Toolkit overlays.Problem
When
manage_scene(action="screenshot", capture_source="game_view", include_image=true)is used without an explicit camera target, the previous implementation captured the image through camera rendering.That misses UI Toolkit overlays because UI Toolkit is composited after the camera render.
Change
ScreenCapture.CaptureScreenshotAsTexture()forgame_viewscreenshots when:include_image=trueinclude_imagescreenshotsinclude_imageis omitted orfalseValidation
MCPForUnity/Editor/Tools/ManageScene.csMCPForUnity/Runtime/Helpers/ScreenshotUtility.csbetaNotes
This PR intentionally keeps the fix narrow to the inline-image screenshot path.
Closes #1039.
Summary by Sourcery
Update game view screenshot capture to use composited frame grabs for inline images while preserving existing camera-based workflows.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
New Features
Bug Fixes