Skip to content

Implement ephemeral sun-direction overrides for lighting iteration#396

Open
drsnuggles8 wants to merge 2 commits into
masterfrom
feature/mcp-scene-time-of-day
Open

Implement ephemeral sun-direction overrides for lighting iteration#396
drsnuggles8 wants to merge 2 commits into
masterfrom
feature/mcp-scene-time-of-day

Conversation

@drsnuggles8

Copy link
Copy Markdown
Owner
  • Added olo_scene_set_time_of_day and olo_scene_set_sun_angle tools to adjust the procedural sky's sun direction without modifying the serialized component.
  • Introduced methods in Renderer3D to set and clear sun direction overrides.
  • Updated Scene and McpServer to handle sun direction overrides during play mode.
  • Enhanced unit tests for sun direction calculations and JSON output for overrides.
  • Documented new tools and their usage in the diagnostics server documentation.

- Added `olo_scene_set_time_of_day` and `olo_scene_set_sun_angle` tools to adjust the procedural sky's sun direction without modifying the serialized component.
- Introduced methods in `Renderer3D` to set and clear sun direction overrides.
- Updated `Scene` and `McpServer` to handle sun direction overrides during play mode.
- Enhanced unit tests for sun direction calculations and JSON output for overrides.
- Documented new tools and their usage in the diagnostics server documentation.
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@drsnuggles8, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 59 minutes and 11 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0c22e112-b938-4026-bd5f-8b43795bc3c8

📥 Commits

Reviewing files that changed from the base of the PR and between 0f95ec8 and 41069f2.

📒 Files selected for processing (9)
  • OloEditor/src/EditorLayer.cpp
  • OloEditor/src/MCP/McpRenderOverrides.h
  • OloEditor/src/MCP/McpServer.cpp
  • OloEditor/src/MCP/McpTools.cpp
  • OloEngine/src/OloEngine/Renderer/Renderer3D.h
  • OloEngine/src/OloEngine/Renderer/Renderer3DState.cpp
  • OloEngine/src/OloEngine/Scene/Scene.cpp
  • OloEngine/tests/MCP/McpRenderOverridesTest.cpp
  • docs/mcp-diagnostics-server.md

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

- McpRenderOverridesTest: integer loop counter (cpp:S2193 reliability gate)
- EditorLayer::SetEditorScene: clear ephemeral sun override on scene swap so a reload restores the authored sun (documented "resets on scene reload")
- McpTools: extract shared ApplySunClear/ApplySunIntrospect helpers
@drsnuggles8

Copy link
Copy Markdown
Owner Author

🤖 Self-review (finish-pr) @ 41069f2d601abd85ebc3760fe25639bbada19cc1

Reviewed the full PR diff against master at high effort (8 finder angles → verify), fixed the one red CI check, and verified the feature live.

  • Findings: 2 · Fixed:
    • Correctness — the override is documented (in 5 places) to "reset on scene reload", but no edit-mode scene-load path actually cleared it. Added Renderer3D::ClearSunDirectionOverride() at the EditorLayer::SetEditorScene choke point (covers New/Open/recovery), beside the existing PostProcess/Fog reset.
    • Cleanup — the two sun handlers duplicated their clear + introspect branches verbatim; extracted shared ApplySunClear/ApplySunIntrospect helpers.
  • CI fix — SonarCloud reliability gate (B on new code) was cpp:S2193 flagging a double loop counter in McpRenderOverridesTest; rewrote as an integer step counter (the loop was exact, but the rule fails the gate regardless).
  • Dismissed: none.
  • Verified live — built OloEngine-Tests+OloEditor clean; 25 McpRenderOverrides* tests pass; drove both tools over the MCP attach loop (set/clear/introspect + error paths); captured the sun sweep in ProceduralSkyTest.olo (noon=day, sunset=amber, midnight=below-horizon note) and confirmed clear restores the authored sky bake byte-for-byte.

Note: CodeRabbit was rate-limited and has not posted a substantive review yet; @coderabbitai review can be triggered if a third-party pass is wanted.

@drsnuggles8 drsnuggles8 added the self-reviewed finish-pr self-review done label Jun 25, 2026
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@drsnuggles8 Thanks for the thorough self-review and verification pass — the reset-path fix at EditorLayer::SetEditorScene, the helper extraction, and the SonarCloud-friendly integer-step test update all sound aligned with the PR’s intent.

Good catch on the edit-mode reload gap in particular; resetting the ephemeral sun override at the same scene-load choke point as the other renderer overrides is the right place for that behavior.


✏️ Learnings added
Learnt from: drsnuggles8
URL: https://github.com/drsnuggles8/OloEngineBase/pull/396

Timestamp: 2026-06-25T17:28:35.180Z
Learning: In the OloEngineBase renderer/editor flow for ephemeral sun-direction overrides, scene reloads are expected to clear the override, and the edit-mode choke point for that reset is `EditorLayer::SetEditorScene`, which now calls `Renderer3D::ClearSunDirectionOverride()` alongside the existing PostProcess/Fog reset logic.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

self-reviewed finish-pr self-review done

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant