fix(weather-editor): remove time of day for lighting templates#2133
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplace DALC collapsing headers and TOD table with always-visible section headers and per-axis directional color editors; "Specular" and "Fresnel Power" controls render only when their labels match the search. Change-tracking and ApplyChanges() behavior unchanged. (≈35 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
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 |
|
No actionable suggestions for changed features. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/WeatherEditor/Weather/LightingTemplateWidget.cpp`:
- Around line 162-180: Specular and Fresnel Power controls are rendered
unconditionally; wrap their draws with the same search gating as the directional
color controls by calling MatchesSearch("Specular") and MatchesSearch("Fresnel
Power") before invoking WeatherUtils::DrawColorEdit("Specular",
settings.dalc.specular) and WeatherUtils::DrawSliderFloat("Fresnel Power",
settings.dalc.fresnelPower") respectively, so they only set changed when the
search matches (keep existing changed assignment logic).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b5573b9d-003f-40b4-8249-76a9b0d9cc0f
📒 Files selected for processing (1)
src/WeatherEditor/Weather/LightingTemplateWidget.cpp
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/WeatherEditor/Weather/LightingTemplateWidget.cpp (1)
168-180: Keep the oldDirectionalsearch term available.These labels are clearer, but the current
MatchesSearch()keys no longer match generic searches like"Directional", and they now diverge fromsrc/WeatherEditor/Weather/WeatherWidget.cpp:799-906. KeepingDirectionalin the search text would preserve discoverability without giving up the new labels.♻️ Minimal tweak
-if (MatchesSearch("X+ (Right)") && WeatherUtils::DrawColorEdit("X+ (Right)", settings.dalc.directional[0].max)) +if (MatchesSearch("Directional X+ Right") && WeatherUtils::DrawColorEdit("X+ (Right)", settings.dalc.directional[0].max)) changed = true; -if (MatchesSearch("X- (Left)") && WeatherUtils::DrawColorEdit("X- (Left)", settings.dalc.directional[0].min)) +if (MatchesSearch("Directional X- Left") && WeatherUtils::DrawColorEdit("X- (Left)", settings.dalc.directional[0].min)) changed = true; -if (MatchesSearch("Y+ (Front)") && WeatherUtils::DrawColorEdit("Y+ (Front)", settings.dalc.directional[1].max)) +if (MatchesSearch("Directional Y+ Front") && WeatherUtils::DrawColorEdit("Y+ (Front)", settings.dalc.directional[1].max)) changed = true; -if (MatchesSearch("Y- (Back)") && WeatherUtils::DrawColorEdit("Y- (Back)", settings.dalc.directional[1].min)) +if (MatchesSearch("Directional Y- Back") && WeatherUtils::DrawColorEdit("Y- (Back)", settings.dalc.directional[1].min)) changed = true; -if (MatchesSearch("Z+ (Up)") && WeatherUtils::DrawColorEdit("Z+ (Up)", settings.dalc.directional[2].max)) +if (MatchesSearch("Directional Z+ Up") && WeatherUtils::DrawColorEdit("Z+ (Up)", settings.dalc.directional[2].max)) changed = true; -if (MatchesSearch("Z- (Down)") && WeatherUtils::DrawColorEdit("Z- (Down)", settings.dalc.directional[2].min)) +if (MatchesSearch("Directional Z- Down") && WeatherUtils::DrawColorEdit("Z- (Down)", settings.dalc.directional[2].min)) changed = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/LightingTemplateWidget.cpp` around lines 168 - 180, The MatchesSearch calls for the directional color labels no longer match the generic "Directional" search term, so update each search condition (the MatchesSearch(...) checks around WeatherUtils::DrawColorEdit for settings.dalc.directional indices 0..2 min/max) to also allow "Directional" — e.g., change each condition to check MatchesSearch("Directional") || MatchesSearch("X+ (Right)") (and similarly for "X- (Left)", "Y+ (Front)", "Y- (Back)", "Z+ (Up)", "Z- (Down)"), so generic searches still find these directional color controls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/WeatherEditor/Weather/LightingTemplateWidget.cpp`:
- Around line 168-180: The MatchesSearch calls for the directional color labels
no longer match the generic "Directional" search term, so update each search
condition (the MatchesSearch(...) checks around WeatherUtils::DrawColorEdit for
settings.dalc.directional indices 0..2 min/max) to also allow "Directional" —
e.g., change each condition to check MatchesSearch("Directional") ||
MatchesSearch("X+ (Right)") (and similarly for "X- (Left)", "Y+ (Front)", "Y-
(Back)", "Z+ (Up)", "Z- (Down)"), so generic searches still find these
directional color controls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0ca632f9-d2bf-40ba-9517-d95564b6e3e8
📒 Files selected for processing (1)
src/WeatherEditor/Weather/LightingTemplateWidget.cpp
|
✅ A pre-release build is available for this PR: |
…nity-shaders#2133) (cherry picked from commit 0802a42)
Lighting templates for interiors do not have time of day. Matches cell lighting format.
Summary by CodeRabbit