fix(weather-editor): Records tab serialization and tracking#2152
Merged
alandtse merged 13 commits intoApr 19, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWeatherWidget now stores form references in Settings (image spaces, volumetric lighting, precipitation, reference effect), serializes them via EditorID lookups, uses those refs in the Records UI and inheritance logic, and adds texture-path constants plus WeatherUtils helpers for EditorID↔form resolution. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as WeatherWidget UI
participant Settings
participant Resolver as WeatherUtils (EditorID↔Form)
participant Live as Weather (live pointers)
participant Disk as Serialized Settings
User->>UI: change Records UI (pick form / toggle inherit)
UI->>Settings: update settings.*Refs and inherit flags
UI->>Live: SetWeatherValues() sync settings.*Refs -> weather->*
UI->>Resolver: FindEditorIDByForm(form) when saving
Resolver->>Settings: return EditorID
Settings->>Disk: serialize EditorIDs (SaveSettings)
sequenceDiagram
participant Disk as Serialized Settings
participant Settings
participant Resolver as WeatherUtils (EditorID↔Form)
participant Live as Weather (live pointers)
participant UI as WeatherWidget UI
Disk->>Settings: load serialized EditorIDs (LoadSettings)
Settings->>Resolver: FindFormByEditorID(EditorID)
Resolver->>Settings: return form pointer (or nullptr / fallback)
Settings->>Live: LoadWeatherValues() syncs settings.*Refs -> weather->*
Live->>UI: UI reflects resolved forms and inherit state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/WeatherEditor/Weather/WeatherWidget.cpp (1)
258-272:⚠️ Potential issue | 🟡 MinorRecords-tab inherit flag is one-shot, unlike Fog/DALC.
For Fog (lines 1263-1268) and DALC, when
inheritFlags[key]istruethe parent value is re-copied every frame, so inherit remains "sticky". Here (and in the VolumetricLighting/Precipitation/ReferenceEffect blocks below) the copy only runs inside theif (ImGui::Checkbox(...))branch, i.e. exactly once at toggle time. Consequences:
- If the parent later changes its record, children with
inheritFlag == truedo not follow.- If the user edits the picker after toggling on,
settings.imageSpaceRefs[i]diverges from parent while the checkbox still reads "Inheriting from parent" —operator==will flag unsaved changes but the UI state is misleading.If one-shot copy is the intended semantics for form records, consider either auto-clearing
inheritFlagwhen the picker is edited (in theDrawFormPickerCachedsuccess branch) or re-copying each frame like Fog for consistency. Applies symmetrically to lines 308-321, 352-365, 394-407.
🧹 Nitpick comments (1)
src/WeatherEditor/Weather/WeatherWidget.cpp (1)
1488-1565:InheritAllFromParentdoesn't cover the new Records tab.Since the PR makes Records behave like other tracked tabs, "Inherit All" should also copy the new reference fields and mark their inherit flags, otherwise a user who clicks "Inherit All" still has to open the Records tab and check each box individually.
♻️ Proposed addition
// Copy cloud settings for (int i = 0; i < TESWeather::kTotalLayers; i++) { settings.clouds[i] = parentWidget->settings.clouds[i]; } + + // Copy record form references + for (int i = 0; i < ColorTimes::kTotal; i++) { + settings.imageSpaceRefs[i] = parentWidget->settings.imageSpaceRefs[i]; + settings.volumetricLightingRefs[i] = parentWidget->settings.volumetricLightingRefs[i]; + } + settings.precipitationData = parentWidget->settings.precipitationData; + settings.referenceEffect = parentWidget->settings.referenceEffect; @@ // Cloud settings for (int i = 0; i < TESWeather::kTotalLayers; i++) { settings.inheritFlags[std::format("Cloud{}_Color", i)] = true; settings.inheritFlags[std::format("Cloud{}_Alpha", i)] = true; } + + // Record inherit flags + for (int i = 0; i < ColorTimes::kTotal; i++) { + settings.inheritFlags[std::format("ImageSpace_{}", i)] = true; + settings.inheritFlags[std::format("VolumetricLighting_{}", i)] = true; + } + settings.inheritFlags["Precipitation"] = true; + settings.inheritFlags["ReferenceEffect"] = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/WeatherWidget.cpp` around lines 1488 - 1565, In WeatherWidget::InheritAllFromParent add logic to copy the new Records data and mark their inherit flags: iterate parentWidget->settings.records (or the actual records container introduced by the PR) and assign each entry into settings.records, then iterate the record keys and set settings.inheritFlags for each record (e.g. settings.inheritFlags["Record_"+key] = true or use the exact flag naming convention used elsewhere), keeping the same auto-apply and notification behavior; use the symbols WeatherWidget::InheritAllFromParent, settings.records, parentWidget->settings.records, and settings.inheritFlags to locate where to insert the two loops (copy records and mark inherit flags).
🤖 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/WeatherWidget.cpp`:
- Around line 1488-1565: In WeatherWidget::InheritAllFromParent add logic to
copy the new Records data and mark their inherit flags: iterate
parentWidget->settings.records (or the actual records container introduced by
the PR) and assign each entry into settings.records, then iterate the record
keys and set settings.inheritFlags for each record (e.g.
settings.inheritFlags["Record_"+key] = true or use the exact flag naming
convention used elsewhere), keeping the same auto-apply and notification
behavior; use the symbols WeatherWidget::InheritAllFromParent, settings.records,
parentWidget->settings.records, and settings.inheritFlags to locate where to
insert the two loops (copy records and mark inherit flags).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54ac3742-7976-4d5e-9359-c48dedc2d46d
📒 Files selected for processing (2)
src/WeatherEditor/Weather/WeatherWidget.cppsrc/WeatherEditor/Weather/WeatherWidget.h
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/WeatherEditor/Weather/WeatherWidget.cpp (2)
251-414:⚠️ Potential issue | 🟠 MajorKeep Records edits in
settingsuntil Apply.The Records tab still edits
weather->...directly, so changing a picker mutates the live weather even whenautoApplyChangesis disabled. Usesettings.*Refsas the picker model, then letApplyChanges()/SetWeatherValues()copy them toweather.Proposed direction
- weather->imageSpaces[i] = parentWidget->weather->imageSpaces[i]; - settings.imageSpaceRefs[i] = weather->imageSpaces[i]; + settings.imageSpaceRefs[i] = parentWidget->settings.imageSpaceRefs[i]; recordChanged = true; @@ - if (WeatherUtils::DrawFormPickerCached("##ImageSpace", weather->imageSpaces[i], editorWindow->imageSpaceWidgets, false, true, pickerWidth)) { - settings.imageSpaceRefs[i] = weather->imageSpaces[i]; + auto*& imageSpaceRef = settings.imageSpaceRefs[i]; + if (WeatherUtils::DrawFormPickerCached("##ImageSpace", imageSpaceRef, editorWindow->imageSpaceWidgets, false, true, pickerWidth)) { recordChanged = true; } // Add "Open" button - if (weather->imageSpaces[i]) { + if (imageSpaceRef) { ImGui::SameLine(); if (ImGui::SmallButton(std::format("Open##{}", i).c_str())) { for (auto& widget : editorWindow->imageSpaceWidgets) { - if (widget->form == weather->imageSpaces[i]) { + if (widget->form == imageSpaceRef) { widget->SetOpen(true); break; } @@ - weather->volumetricLighting[i] = parentWidget->weather->volumetricLighting[i]; - settings.volumetricLightingRefs[i] = weather->volumetricLighting[i]; + settings.volumetricLightingRefs[i] = parentWidget->settings.volumetricLightingRefs[i]; recordChanged = true; @@ - if (WeatherUtils::DrawFormPickerCached("##VolumetricLighting", weather->volumetricLighting[i], editorWindow->volumetricLightingWidgets, false, true, pickerWidth)) { - settings.volumetricLightingRefs[i] = weather->volumetricLighting[i]; + auto*& volumetricLightingRef = settings.volumetricLightingRefs[i]; + if (WeatherUtils::DrawFormPickerCached("##VolumetricLighting", volumetricLightingRef, editorWindow->volumetricLightingWidgets, false, true, pickerWidth)) { recordChanged = true; } // Add "Open" button - if (weather->volumetricLighting[i]) { + if (volumetricLightingRef) { ImGui::SameLine(); if (ImGui::SmallButton(std::format("Open##{}", i).c_str())) { for (auto& widget : editorWindow->volumetricLightingWidgets) { - if (widget->form == weather->volumetricLighting[i]) { + if (widget->form == volumetricLightingRef) { widget->SetOpen(true); break; } @@ - weather->precipitationData = parentWidget->weather->precipitationData; - settings.precipitationData = weather->precipitationData; + settings.precipitationData = parentWidget->settings.precipitationData; recordChanged = true; @@ - if (WeatherUtils::DrawFormPickerCached("##Precipitation", weather->precipitationData, editorWindow->precipitationWidgets, false, true, pickerWidth)) { - settings.precipitationData = weather->precipitationData; + if (WeatherUtils::DrawFormPickerCached("##Precipitation", settings.precipitationData, editorWindow->precipitationWidgets, false, true, pickerWidth)) { recordChanged = true; } // Add "Open" button - if (weather->precipitationData) { + if (settings.precipitationData) { ImGui::SameLine(); if (ImGui::SmallButton("Open##Precip")) { for (auto& widget : editorWindow->precipitationWidgets) { - if (widget->form == weather->precipitationData) { + if (widget->form == settings.precipitationData) { widget->SetOpen(true); break; } @@ - weather->referenceEffect = parentWidget->weather->referenceEffect; - settings.referenceEffect = weather->referenceEffect; + settings.referenceEffect = parentWidget->settings.referenceEffect; recordChanged = true; @@ - if (WeatherUtils::DrawFormPickerCached("##ReferenceEffect", weather->referenceEffect, editorWindow->referenceEffectWidgets, false, true, pickerWidth)) { - settings.referenceEffect = weather->referenceEffect; + if (WeatherUtils::DrawFormPickerCached("##ReferenceEffect", settings.referenceEffect, editorWindow->referenceEffectWidgets, false, true, pickerWidth)) { recordChanged = true; } // Add "Open" button - if (weather->referenceEffect) { + if (settings.referenceEffect) { ImGui::SameLine(); if (ImGui::SmallButton("Open##RefEffect")) { for (auto& widget : editorWindow->referenceEffectWidgets) { - if (widget->form == weather->referenceEffect) { + if (widget->form == settings.referenceEffect) { widget->SetOpen(true); break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/WeatherWidget.cpp` around lines 251 - 414, The pickers in this file currently write directly to the live weather object (e.g., weather->imageSpaces[i], weather->volumetricLighting[i], weather->precipitationData, weather->referenceEffect) which bypasses the settings buffer and causes immediate mutations when autoApplyChanges is off; change each WeatherUtils::DrawFormPickerCached call to use the corresponding settings.*Refs entry (settings.imageSpaceRefs[i], settings.volumetricLightingRefs[i], settings.precipitationData, settings.referenceEffect) as the model value, update settings.*Refs on picker change, and only copy settings.*Refs into weather->* inside ApplyChanges()/SetWeatherValues() so UI edits are staged until Apply is invoked (also update the "Open" button lookups to reference the staged refs where appropriate).
1472-1538:⚠️ Potential issue | 🟡 MinorInclude Records refs in “Inherit All”.
Inherit Allnow copies every other tab but skips the new Records fields and their inherit flags, so the button does not actually inherit all weather state.Proposed fix
// Copy cloud settings for (int i = 0; i < TESWeather::kTotalLayers; i++) { settings.clouds[i] = parentWidget->settings.clouds[i]; } + + // Copy record references + for (int i = 0; i < ColorTimes::kTotal; i++) { + settings.imageSpaceRefs[i] = parentWidget->settings.imageSpaceRefs[i]; + settings.volumetricLightingRefs[i] = parentWidget->settings.volumetricLightingRefs[i]; + } + settings.precipitationData = parentWidget->settings.precipitationData; + settings.referenceEffect = parentWidget->settings.referenceEffect; // Set all inherit flags to true + settings.inheritFlags["Precipitation"] = true; + settings.inheritFlags["ReferenceEffect"] = true; + + for (int i = 0; i < ColorTimes::kTotal; i++) { + settings.inheritFlags[std::format("ImageSpace_{}", i)] = true; + settings.inheritFlags[std::format("VolumetricLighting_{}", i)] = true; + } + settings.inheritFlags["DALC_Specular"] = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/WeatherWidget.cpp` around lines 1472 - 1538, The InheritAllFromParent function currently copies weatherProperties, weatherColors, fogProperties, atmosphereColors, dalc, clouds and sets inheritFlags, but it omits the new Records fields; update WeatherWidget::InheritAllFromParent to copy parentWidget->settings.records into settings.records (e.g. loop over parentWidget->settings.records and assign each entry to settings.records) and set the corresponding inherit flags in settings.inheritFlags for each record key (for example using a consistent key name like "Record_"+recordKey or whatever naming convention your UI uses), ensuring you use GetParent()/parentWidget->settings to source values and settings.inheritFlags to mark them inherited.
🤖 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/WeatherWidget.cpp`:
- Around line 482-496: LoadSettings in WeatherWidget treats an empty string the
same as a missing key and always falls back to vanillaSettings, so a user-chosen
explicit "None" (saved as "") is lost; change the logic in the load lambda and
subsequent assignments (loadEditorID, the loop that sets settings.imageSpaceRefs
and settings.volumetricLightingRefs, and the precipitationData/referenceEffect
assignments) to distinguish three cases: key missing -> use vanillaSettings, key
present with empty string -> set the setting to nullptr (explicit None), key
present with non-empty string -> lookup the form and use it (or nullptr if not
found). Ensure SaveSettings continues to write "" for None and LoadSettings uses
js.contains(key) and js[key].get<std::string>() == "" to detect explicit None
rather than falling back to vanilla.
In `@src/WeatherEditor/WeatherUtils.cpp`:
- Around line 68-75: FindEditorIDByForm currently returns empty when a form is
missing from the widgets vector, causing valid weather form refs to be
serialized as blank; update the function used by WeatherWidget::SaveSettings
(and any callers like volumetricLightingWidgets) to fall back to a direct
EditorID lookup and logging when the widget cache misses: first attempt the
existing widget match (loop over widgets and use w->GetEditorID()), if no match
call form->GetFormEditorID() or query the global form registry to obtain the
editor ID, log a warning indicating the widget cache miss and the fallback, and
return that EditorID; alternatively, ensure the widget lists (e.g.,
volumetricLightingWidgets) are populated for all serialized form types before
SaveSettings runs so FindEditorIDByForm's primary path succeeds.
---
Outside diff comments:
In `@src/WeatherEditor/Weather/WeatherWidget.cpp`:
- Around line 251-414: The pickers in this file currently write directly to the
live weather object (e.g., weather->imageSpaces[i],
weather->volumetricLighting[i], weather->precipitationData,
weather->referenceEffect) which bypasses the settings buffer and causes
immediate mutations when autoApplyChanges is off; change each
WeatherUtils::DrawFormPickerCached call to use the corresponding settings.*Refs
entry (settings.imageSpaceRefs[i], settings.volumetricLightingRefs[i],
settings.precipitationData, settings.referenceEffect) as the model value, update
settings.*Refs on picker change, and only copy settings.*Refs into weather->*
inside ApplyChanges()/SetWeatherValues() so UI edits are staged until Apply is
invoked (also update the "Open" button lookups to reference the staged refs
where appropriate).
- Around line 1472-1538: The InheritAllFromParent function currently copies
weatherProperties, weatherColors, fogProperties, atmosphereColors, dalc, clouds
and sets inheritFlags, but it omits the new Records fields; update
WeatherWidget::InheritAllFromParent to copy parentWidget->settings.records into
settings.records (e.g. loop over parentWidget->settings.records and assign each
entry to settings.records) and set the corresponding inherit flags in
settings.inheritFlags for each record key (for example using a consistent key
name like "Record_"+recordKey or whatever naming convention your UI uses),
ensuring you use GetParent()/parentWidget->settings to source values and
settings.inheritFlags to mark them inherited.
🪄 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: 50f67fdd-e83a-41aa-baf7-98a0ab8675c7
📒 Files selected for processing (3)
src/WeatherEditor/Weather/WeatherWidget.cppsrc/WeatherEditor/WeatherUtils.cppsrc/WeatherEditor/WeatherUtils.h
|
✅ A pre-release build is available for this PR: |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/WeatherEditor/Weather/WeatherWidget.cpp (2)
258-400: Only mark inherited records changed when the value actually changes.When any Records inherit flag is already enabled,
recordChangedis set every frame, causing repeatedApplyChanges()while the tab is open. Compare before assigning so parent updates still propagate without unnecessary full weather reapplies.♻️ Proposed refactor
const float todLabelOffset = (hasParent ? 120.0f : 100.0f) * scale; const float formLabelOffset = (hasParent ? 170.0f : 150.0f) * scale; const float pickerWidth = 225.0f * scale; + auto copyInheritedRef = [](auto*& target, auto* source) { + if (target == source) { + return false; + } + target = source; + return true; + }; // ImageSpace Records (per time of day) if (ImGui::CollapsingHeader("ImageSpace", ImGuiTreeNodeFlags_DefaultOpen)) { @@ ImGui::Checkbox(("##inherit_" + inheritKey).c_str(), &inheritFlag); if (inheritFlag && parentWidget) { - settings.imageSpaceRefs[i] = parentWidget->settings.imageSpaceRefs[i]; - recordChanged = true; + recordChanged |= copyInheritedRef(settings.imageSpaceRefs[i], parentWidget->settings.imageSpaceRefs[i]); } @@ ImGui::Checkbox(("##inherit_" + inheritKey).c_str(), &inheritFlag); if (inheritFlag && parentWidget) { - settings.volumetricLightingRefs[i] = parentWidget->settings.volumetricLightingRefs[i]; - recordChanged = true; + recordChanged |= copyInheritedRef(settings.volumetricLightingRefs[i], parentWidget->settings.volumetricLightingRefs[i]); } @@ ImGui::Checkbox("##inherit_Precipitation", &inheritFlag); if (inheritFlag && parentWidget) { - settings.precipitationData = parentWidget->settings.precipitationData; - recordChanged = true; + recordChanged |= copyInheritedRef(settings.precipitationData, parentWidget->settings.precipitationData); } @@ ImGui::Checkbox("##inherit_ReferenceEffect", &inheritFlag); if (inheritFlag && parentWidget) { - settings.referenceEffect = parentWidget->settings.referenceEffect; - recordChanged = true; + recordChanged |= copyInheritedRef(settings.referenceEffect, parentWidget->settings.referenceEffect); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/WeatherWidget.cpp` around lines 258 - 400, The inherit branches currently set recordChanged unconditionally whenever inheritFlag is true; change each block that copies from parent (e.g., the assignments in the ImageSpace loop that set settings.imageSpaceRefs[i], the Volumetric Lighting loop that sets settings.volumetricLightingRefs[i], the Precipitation block setting settings.precipitationData, the Visual Effect block setting settings.referenceEffect, etc.) to first compare the child value with the parent's value and only assign AND set recordChanged when the values differ; keep the parentWidget null check and preserve tooltips and SameLine calls.
1-1: Polish the PR metadata.Suggested title:
fix(weather-editor): track records tab stateIf there is a backing issue, add a closing reference such as
Fixes#123`` to the PR description.As per coding guidelines, “Conventional Commit Titles” should use
type(scope): description, lowercase description, and a 50-character title limit; bug fixes should include appropriate GitHub keywords such asFixes#123``.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/Weather/WeatherWidget.cpp` at line 1, Update the PR metadata: change the PR title to "fix(weather-editor): track records tab state" and, if there is a backing issue, add a closing reference like "Fixes `#123`" to the PR description; no source changes are needed in src/WeatherEditor/Weather/WeatherWidget.cpp — just edit the PR title and description for conformity with Conventional Commits and repository guidelines.
🤖 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/WeatherWidget.cpp`:
- Around line 473-480: The loadRef lambda currently returns nullptr for both
empty and unresolved non-empty EditorIDs which turns unresolved IDs into an
explicit None; change its logic so that if id.empty() it still returns nullptr
(explicit None), but if id is non-empty and WeatherUtils::FindFormByEditorID(id,
widgets) fails, return the original vanillaValue instead of nullptr; update the
lambda in WeatherWidget.cpp (the auto loadRef = [&]<typename T>(...) -> T*
closure that calls WeatherUtils::FindFormByEditorID) to implement this fallback
so malformed/unresolvable IDs do not silently clear the value.
---
Nitpick comments:
In `@src/WeatherEditor/Weather/WeatherWidget.cpp`:
- Around line 258-400: The inherit branches currently set recordChanged
unconditionally whenever inheritFlag is true; change each block that copies from
parent (e.g., the assignments in the ImageSpace loop that set
settings.imageSpaceRefs[i], the Volumetric Lighting loop that sets
settings.volumetricLightingRefs[i], the Precipitation block setting
settings.precipitationData, the Visual Effect block setting
settings.referenceEffect, etc.) to first compare the child value with the
parent's value and only assign AND set recordChanged when the values differ;
keep the parentWidget null check and preserve tooltips and SameLine calls.
- Line 1: Update the PR metadata: change the PR title to "fix(weather-editor):
track records tab state" and, if there is a backing issue, add a closing
reference like "Fixes `#123`" to the PR description; no source changes are needed
in src/WeatherEditor/Weather/WeatherWidget.cpp — just edit the PR title and
description for conformity with Conventional Commits and repository guidelines.
🪄 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: ae8b010b-b3b4-46f1-bc7f-8273c63e78ec
📒 Files selected for processing (1)
src/WeatherEditor/Weather/WeatherWidget.cpp
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
alandtse
approved these changes
Apr 19, 2026
InTheBottle
added a commit
to InTheBottle/skyrim-community-shaders
that referenced
this pull request
Apr 21, 2026
Reverts upstream PRs community-shaders#2151, community-shaders#2152, community-shaders#2154 which removed the inline ImageSpaceSettings struct from WeatherWidget and replaced HDR/bloom/cinematic/tint/DoF inline storage with TESImageSpace form pickers. These changes broke existing per-weather edits saved in user JSON files (old 'imageSpaces' array key is silently ignored by the new loader, and SetWeatherValues unconditionally overwrites weather->imageSpaces[] with possibly-null form pointers from missing keys). Restoring the old behavior on the fork pending a forward-compatible migration.
ParticleTroned
pushed a commit
to ParticleTroned/skyrim-community-shaders
that referenced
this pull request
May 2, 2026
…y-shaders#2152) (cherry picked from commit 4d61307)
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
2 participants
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.
Currently the records tab allows the user to change records tied to a weather, but this isn't actually tracked at all, nor is it saved.
The user expectation and intended behaviour is that it should be tracking and saving these changes. Just like the other tabs in this widget. This PR corrects this.
Summary by CodeRabbit