Skip to content

feat(weather-editor): expand inheritance, add indicators#2322

Merged
alandtse merged 5 commits into
community-shaders:devfrom
SkrubbySkrubInAShrub:grey-out-inherit-sliders
May 12, 2026
Merged

feat(weather-editor): expand inheritance, add indicators#2322
alandtse merged 5 commits into
community-shaders:devfrom
SkrubbySkrubInAShrub:grey-out-inherit-sliders

Conversation

@SkrubbySkrubInAShrub
Copy link
Copy Markdown
Collaborator

@SkrubbySkrubInAShrub SkrubbySkrubInAShrub commented May 11, 2026

  • expands inheritance coverage to all sliders in the weather widget
  • fixes bugs with application of inherit all
  • adds indicators for which values are inherited
  • adds synchronization between parent and child.
  • refactor code for cleanliness and DRY

Summary by CodeRabbit

  • Refactor

    • Consolidated fog and records UI code, simplified basic properties rendering, and unified per-field inheritance behavior.
  • New Features

    • Inherited values can be copied from parents and propagated to descendant weathers; group-based "inherit all" now applies matching inherit flags and marks for reinit.
  • Bug Fixes

    • Editing an inherited control clears its inherit flag and triggers reinitialization when needed.
  • Style

    • Added warning-themed styling and consistent "Inherited from parent weather" tooltips for inherited controls.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b4a63b1d-d2f5-4f10-9cf3-4a83f3a19094

📥 Commits

Reviewing files that changed from the base of the PR and between 9b47a0d and 265e9c3.

📒 Files selected for processing (2)
  • src/WeatherEditor/Weather/WeatherWidget.cpp
  • src/WeatherEditor/Weather/WeatherWidget.h

📝 Walkthrough

Walkthrough

WeatherWidget inheritance handling was centralized: new Push/Pop style helpers mark inherited controls, UI panels derive inherited state from flags and clear flags on user edits, and SyncInheritedValuesFromParent()/PropagateToChildren() enable copying from parent and cascading updates to descendants.

Changes

Weather Inheritance System Refactoring

Layer / File(s) Summary
Inherited Style Helpers
src/WeatherEditor/WeatherUtils.h, src/WeatherEditor/WeatherUtils.cpp
New PushInheritedStyle() and PopInheritedStyle() apply warning-themed ImGui styling (frame backgrounds, border color/size) for inherited controls.
TOD UI Inherited Styling
src/WeatherEditor/WeatherUtils.cpp
TOD float/color/slider rows compute isInherited, push/pop inherited style, show fixed “Inherited from parent weather” tooltip for inherited controls, and use isInherited for disabled state handling.
WeatherWidget Method Declarations
src/WeatherEditor/Weather/WeatherWidget.h
Adds private methods DrawFogSlider, DrawFogRow, SyncInheritedValuesFromParent(), and PropagateToChildren().
Records Tab Inheritance UI
src/WeatherEditor/Weather/WeatherWidget.cpp
drawInheritCheckbox returns inherit state; record/time pickers wrap inherited controls with styling/tooltips and clear inherit flags (setting pendingReinit) when edited.
Fog Settings Refactoring
src/WeatherEditor/Weather/WeatherWidget.cpp
Consolidates fog slider rows into DrawFogSlider/DrawFogRow, retaining inherit-checkbox/highlight behavior, optional parent-day/night sync when inherited, and clearing flags on user edits.
Basic Properties Inheritance Rendering
src/WeatherEditor/Weather/WeatherWidget.cpp
DrawProperties() derives isInherited from settings.inheritFlags[...], applies inherited styling/tooltips when set, and clears the flag only on user edits.
Sync and Propagation Implementation
src/WeatherEditor/Weather/WeatherWidget.cpp
Implements SyncInheritedValuesFromParent() to copy enabled inherited values from parent and PropagateToChildren() to cascade syncs to descendant weathers (uses thread-local unordered_set for cycle protection). ApplyChanges() now calls propagation after apply.
InheritAllFromParent Logic Rewrite
src/WeatherEditor/Weather/WeatherWidget.cpp
Rewritten to copy values per inheritance group, enable matching settings.inheritFlags[...], set pendingReinit, and optionally ApplyChanges().

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • alandtse

Poem

🐰 A rabbit hops through inheritance trees,

copying skies and fog with gentle ease.
Styles flag what's borrowed, children learn to show,
when parents change, their cascades softly flow.
Small refactors make the weather glow.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(weather-editor): expand inheritance, add indicators' directly aligns with the main changes: expanding inheritance support across all sliders and adding visual indicators for inherited values.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.20.0)

OpenGrep fatal error (exit code 2): [00.20][ERROR]: Error: exception Unix_error: No such file or directory stat src/WeatherEditor/Weather/WeatherWidget.h
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-2


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

@github-actions
Copy link
Copy Markdown

No actionable suggestions for changed features.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/WeatherEditor/Weather/WeatherWidget.cpp (1)

1-1: PR metadata: consider adding issue-link keywords in the description.

Since this PR includes both feature work and bug fixes, adding Fixes #... / Addresses #... improves traceability and release notes.

As per coding guidelines, "Issue References (if PR fixes bugs or implements features): Suggest adding appropriate GitHub keywords."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/WeatherEditor/Weather/WeatherWidget.cpp` at line 1, The PR needs
issue-link keywords added to its description for traceability: edit the PR
description to include GitHub keywords like "Fixes #<issue-number>" or
"Addresses #<issue-number>" for the bug fixes/features implemented (e.g.,
reference the WeatherWidget work) and ensure each affected issue number is
listed; if multiple issues are covered, add separate keywords for each and keep
the description concise so release notes can automatically pick them up.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/WeatherEditor/Weather/WeatherWidget.cpp`:
- Around line 1363-1452: SyncInheritedValuesFromParent/PropagateToChildren can
recurse indefinitely on cyclic parent chains (A→B→A); add a cycle guard by
tracking visited widget IDs (or a recursion depth counter) when propagating so a
child only processes propagation if its parent chain does not contain its own
GetEditorID(), and stop propagation if a cycle is detected; implement this in
PropagateToChildren (and optionally pass a visited set into
SyncInheritedValuesFromParent/ApplyChanges) using settings.parent to walk
parents and EditorWindow::GetSingleton()->weatherWidgets/GetEditorID() to
resolve IDs, and ensure ApplyChanges no longer blindly re-invokes
PropagateToChildren when a cycle was detected.

---

Nitpick comments:
In `@src/WeatherEditor/Weather/WeatherWidget.cpp`:
- Line 1: The PR needs issue-link keywords added to its description for
traceability: edit the PR description to include GitHub keywords like "Fixes
#<issue-number>" or "Addresses #<issue-number>" for the bug fixes/features
implemented (e.g., reference the WeatherWidget work) and ensure each affected
issue number is listed; if multiple issues are covered, add separate keywords
for each and keep the description concise so release notes can automatically
pick them up.
🪄 Autofix (Beta)

❌ Autofix failed (check again to retry)

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 Plus

Run ID: 9d314876-7fb5-4509-a5cb-72537065ab03

📥 Commits

Reviewing files that changed from the base of the PR and between 5a59442 and 986b0d1.

📒 Files selected for processing (4)
  • src/WeatherEditor/Weather/WeatherWidget.cpp
  • src/WeatherEditor/Weather/WeatherWidget.h
  • src/WeatherEditor/WeatherUtils.cpp
  • src/WeatherEditor/WeatherUtils.h

Comment thread src/WeatherEditor/Weather/WeatherWidget.cpp
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 11, 2026

✅ A pre-release build is available for this PR:
Download

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference

@SkrubbySkrubInAShrub
Copy link
Copy Markdown
Collaborator Author

SkrubbySkrubInAShrub commented May 11, 2026

@coderabbitai the autofix was an accident, don't continue

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Tip

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

@SkrubbySkrubInAShrub Got it! I've noted the autofix was unintentional — no further autofix actions will be taken. Let me know if there's anything else you'd like help with on this PR!

@alandtse alandtse merged commit 54b19ea into community-shaders:dev May 12, 2026
12 checks passed
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 15, 2026
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants