Conversation
📝 WalkthroughWalkthroughCamera rendering logic is refactored across two files to delegate rendering to a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Assets/Scripts/Camera/CameraControler.cs (1)
149-151: Release screenshot texture after saving to reduce transient memory growth.After
SaveTexture, destroy the temporaryTexture2D.Suggested patch
yield return null; SaveTexture(screenShot, path); + Destroy(screenShot); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Assets/Scripts/Camera/CameraControler.cs` around lines 149 - 151, After calling SaveTexture(screenShot, path) in the CameraControler coroutine, explicitly release the temporary Texture2D to avoid transient memory growth: call Destroy(screenShot) (or DestroyImmediate(screenShot) if running in the Editor) and then set screenShot = null so the GC can collect it; locate the SaveTexture call and add the destroy/null assignment immediately after it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Assets/Scripts/Camera/CameraControler.cs`:
- Line 79: The Rect assigned to Cam.rect can become negative on small windows;
fix by computing clamped width and height before creating the Rect (use
Mathf.Max or Mathf.Clamp to ensure width = Mathf.Max(0f, 1 - RemoveCamPropWidth)
and height = Mathf.Max(0f, 1 - RemoveCamPropHeight)), then assign Cam.rect = new
Rect(x, y, clampedWidth, clampedHeight) (ensure x/y are also clamped into [0,1]
if derived), updating the code in CameraControler.cs where Cam.rect is set.
---
Nitpick comments:
In `@Assets/Scripts/Camera/CameraControler.cs`:
- Around line 149-151: After calling SaveTexture(screenShot, path) in the
CameraControler coroutine, explicitly release the temporary Texture2D to avoid
transient memory growth: call Destroy(screenShot) (or
DestroyImmediate(screenShot) if running in the Editor) and then set screenShot =
null so the GC can collect it; locate the SaveTexture call and add the
destroy/null assignment immediately after it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b684ddbd-3849-40f1-ac98-876bbe62fb1d
📒 Files selected for processing (2)
Assets/Scripts/Camera/CameraControler.csAssets/Scripts/Ozone SCMAP Code/MapLuaParser_App.cs
| RemoveCamPropWidth = 309f / (float)Screen.width; | ||
| } | ||
|
|
||
| Cam.rect = new Rect(RemoveCamPropWidth, 0, 1 - RemoveCamPropWidth, 1 - RemoveCamPropHeight); |
There was a problem hiding this comment.
Clamp camera rect components to avoid negative viewport sizes.
Line 79 can produce negative Rect width/height on tiny window sizes (e.g., width < 309, height < 30).
Suggested patch
- Cam.rect = new Rect(RemoveCamPropWidth, 0, 1 - RemoveCamPropWidth, 1 - RemoveCamPropHeight);
+ float x = Mathf.Clamp01(RemoveCamPropWidth);
+ float w = Mathf.Clamp01(1f - x);
+ float h = Mathf.Clamp01(1f - RemoveCamPropHeight);
+ Cam.rect = new Rect(x, 0f, w, h);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Cam.rect = new Rect(RemoveCamPropWidth, 0, 1 - RemoveCamPropWidth, 1 - RemoveCamPropHeight); | |
| float x = Mathf.Clamp01(RemoveCamPropWidth); | |
| float w = Mathf.Clamp01(1f - x); | |
| float h = Mathf.Clamp01(1f - RemoveCamPropHeight); | |
| Cam.rect = new Rect(x, 0f, w, h); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Assets/Scripts/Camera/CameraControler.cs` at line 79, The Rect assigned to
Cam.rect can become negative on small windows; fix by computing clamped width
and height before creating the Rect (use Mathf.Max or Mathf.Clamp to ensure
width = Mathf.Max(0f, 1 - RemoveCamPropWidth) and height = Mathf.Max(0f, 1 -
RemoveCamPropHeight)), then assign Cam.rect = new Rect(x, y, clampedWidth,
clampedHeight) (ensure x/y are also clamped into [0,1] if derived), updating the
code in CameraControler.cs where Cam.rect is set.
| { | ||
| OtherCams[i].rect = Cam.rect; | ||
| } | ||
| } |
There was a problem hiding this comment.
It seems you inadvertantly changed the indentation here. Please fix it.
| public IEnumerator RenderCamera(int resWidth, int resHeight, string path){ | ||
| for (int i = 0; i < 5; i++) | ||
| yield return new WaitForEndOfFrame(); | ||
|
|
||
| Texture2D screenShot = ScmapEditor.Current.PreviewRenderer.RenderPreview( | ||
| 0f, resWidth, resHeight, false, true | ||
| ); | ||
|
|
||
| yield return null; | ||
| SaveTexture(screenShot, path); | ||
| } |
2 participants
Fix: CLI -renderPreviewImage never wrote output file, and rendered black when fixed
Problem 1: No output file was produced
When invoking the editor via command line with -renderPreviewImage, the preview PNG was never written to disk. The root cause was in MapLuaParser_App.cs: RenderCamera was called as a plain method instead of as a coroutine, so ForceQuit() killed the process before File.WriteAllBytes could complete.
Fix: Await the coroutine properly:
// Before:
CameraControler.Current.RenderCamera(Width, Height, ImagePath);
// After:
yield return CameraControler.Current.StartCoroutine(CameraControler.Current.RenderCamera(Width, Height, ImagePath));
Problem 2: Output was completely black
After fixing the coroutine, the PNG was written but contained only a black image. The cause was that RenderCamera used PreviewTex.ForcePreviewMode(true) + Cam.Render() directly, which only sets a boolean flag but never enables the PREVIEW_ON shader keyword on the terrain material, nor positions the preview camera correctly. In GUI mode this works because the user triggers the render manually after everything is loaded. In CLI mode the render happens immediately, bypassing all the setup that PreviewTex.RenderPreview() performs.
Additionally, UpdateRect() caused a division by zero crash in CLI mode because Screen.height and Screen.width can be 0 before the window is fully initialized.
Fix 1: Use PreviewRenderer.RenderPreview() directly, which handles terrain shader keywords, camera positioning, and render texture setup correctly:
Texture2D screenShot = ScmapEditor.Current.PreviewRenderer.RenderPreview(
0f, resWidth, resHeight, false, false
);
Fix 2: Guard against division by zero in UpdateRect():
if (!NoRect && Screen.height > 0 && Screen.width > 0)
{
RemoveCamPropHeight = 30f / (float)Screen.height;
RemoveCamPropWidth = 309f / (float)Screen.width;
}
Files changed:
Assets/Scripts/Camera/CameraControler.cs
Assets/Scripts/Ozone SCMAP Code/MapLuaParser_App.cs
Summary by CodeRabbit