feat(weather editor): separate left and right pane in objects window#1896
feat(weather editor): separate left and right pane in objects window#1896SkrubbySkrubInAShrub wants to merge 6 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughReplaces the static left-column category display with a collapsible ListBox driving a two-pane editor: left category list, right dynamic content. Adds Interior Only early-exit panel, Current Active Weather display, Ctrl+F-filter focus, per-item markers/menus, recent-items, hover tooltips, and preserved open/save behaviors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/WeatherEditor/EditorWindow.cpp (5)
433-515: Large code duplication between "current lighting template" block and the regular widget loop.Lines 433-516 (current cell's lighting template rendering) and lines 518-636 (regular widget rendering) share nearly identical logic for: favorite button, selectable row, enter-key handling, context menu, form ID / file / status columns, and background coloring. This duplication makes maintenance error-prone — any fix needs to be applied in both places.
Consider extracting a
DrawWidgetRow(Widget*, bool isCurrent)helper.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` around lines 433 - 515, The current-cell lighting-template rendering duplicates the regular widget row rendering (favorite toggle, selectable/open logic, Enter key handling, context menu, FormID/Filename/Status columns, and background coloring) found around the same area; extract a reusable helper like DrawWidgetRow(Widget* widget, bool isCurrent, const std::string& editorLabel) and move common logic (IconButton, IsFavorite, ToggleFavorite, Selectable/Open + ImGuiKey_Enter handling, AddToRecent, BeginPopupContextItem + settings.markedRecords/Save, TableNextColumn/Text for GetFormID/GetFilename/status) into it; call DrawWidgetRow from both the current-cell block (pass isCurrent=true and apply highlightColor via Menu::GetSingleton()->GetSettings().Theme.StatusPalette.InfoColor before calling or provide a parameter to set row background) and the regular loop (isCurrent=false), keeping unique pre-filters (ContainsStringIgnoreCase, showOnlyFavorites, showOnlyFlagged, dynamic_cast to LightingTemplateWidget) in their original places.
301-301:emptyWidgetsvector is reconstructed every frame.
std::vector<std::unique_ptr<Widget>> emptyWidgets;is declared inside the per-frame render path. While it's empty and cheap, making itstaticor using a file-scope constant would be cleaner and avoid any allocation overhead.Proposed fix
-std::vector<std::unique_ptr<Widget>> emptyWidgets; +static const std::vector<std::unique_ptr<Widget>> emptyWidgets;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` at line 301, The local variable emptyWidgets (std::vector<std::unique_ptr<Widget>>) is being reconstructed every frame inside the per-frame render path in EditorWindow.cpp; change it to a single, long-lived instance by either making it static within the function (e.g., static ...) or moving it to file scope as a constant empty vector so it is initialized once and reused, avoiding per-frame allocation overhead; keep the type as std::vector<std::unique_ptr<Widget>> and ensure the initializer is the empty state so existing call sites using emptyWidgets continue to work unchanged.
247-274: Extract category → widget-collection mapping to eliminate duplicate ternary chains.The same category → widget-vector mapping is repeated at lines 257-264 and 300-309 (and partially at line 204 for Weather). Divergence between these chains is already causing a bug (missing "Cell Lighting" in the first chain). A small helper method returning a
const std::vector<...>&would centralize this, preventing future drift.Also applies to: 300-309
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` around lines 247 - 274, Extract the repeated category→widget-vector logic into a single helper method (e.g., EditorWindow::GetWidgetCollectionForCategory(const std::string& category) -> std::vector<WidgetType>& or const std::vector<WidgetType>& as appropriate) that returns the correct vector for each category (weatherWidgets, lightingTemplateWidgets, imageSpaceWidgets, volumetricLightingWidgets, precipitationWidgets, lensFlareWidgets, referenceEffectWidgets, plus the missing "Cell Lighting" collection), then replace the duplicated ternary chains in the recent-widget block (the code around the SmallButton loop) and the other occurrences (the block at lines ~300-309 and the earlier usage around line ~204) to call this helper instead; ensure the returned reference type matches usage (mutable when you need to call widget->SetOpen) and centralize the mapping to prevent future drift.
435-435: Signed/unsigned comparison:int ivssize_t.
sortedWidgets.size()returnssize_t, butiis declared asint. This triggers compiler warnings and, in theory, can produce incorrect comparisons for extremely large collections.Proposed fix
-for (int i = 0; i < sortedWidgets.size(); ++i) { +for (size_t i = 0; i < sortedWidgets.size(); ++i) {Apply at both line 435 and line 519.
Also applies to: 519-519
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` at line 435, Replace the signed loop index with an unsigned type to match sortedWidgets.size(): change the loop variable declaration from "int i" to a size-compatible type (e.g., size_t or auto) in the for-loops that iterate over sortedWidgets (the loops using "for (int i = 0; i < sortedWidgets.size(); ++i)") so the comparison with sortedWidgets.size() is between the same types; apply the same change to both occurrences (the loop around sortedWidgets at the location referenced and the second occurrence mentioned).
139-144: Static locals for UI state are shared across the process lifetime.
selectedCategory,filterBuffer,showOnlyFlagged, andshowOnlyFavoritesarestaticlocals. This is idiomatic for ImGui single-instance windows, so it's fine here as long as only oneEditorWindowinstance exists. Just worth noting that these persist even if the window is closed and reopened — the filter text won't reset, which may or may not be desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` around lines 139 - 144, The UI state variables selectedCategory, filterBuffer, showOnlyFlagged, and showOnlyFavorites are declared as static locals in EditorWindow.cpp which makes them persist for the process lifetime; to avoid surprising persistent filters between opens, move these into the EditorWindow instance (make them member variables on the EditorWindow class) or explicitly reset them when the window is opened/closed (e.g., in EditorWindow::Open or EditorWindow::OnShow/OnHide). Update all references to use the instance members instead of the static locals (search for selectedCategory, filterBuffer, showOnlyFlagged, showOnlyFavorites).
🤖 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/EditorWindow.cpp`:
- Around line 257-264: The recent-widgets selection branch using
selectedCategory currently omits "Cell Lighting" so it falls through to
weatherWidgets and never finds cell lighting entries; update the ternary chain
to explicitly handle "Cell Lighting" (map it to emptyWidgets or otherwise skip
recent widgets for that category) or factor the mapping into a helper (e.g., a
categoryToWidgets function used by both the recent-widgets lookup and the later
mapping) and use that helper in place of the duplicated ternary; ensure you
reference selectedCategory, weatherWidgets, emptyWidgets (and other widget
collections like lightingTemplateWidgets, imageSpaceWidgets, etc.) so the new
branch returns the correct collection.
- Around line 566-593: The tooltip block calls ImGui::BeginTooltip() but does
not check its boolean return before calling ImGui::EndTooltip(), risking an
unmatched End call; update the block around ImGui::BeginTooltip() inside the
Weather category hover branch to use the guarded pattern (if
(ImGui::BeginTooltip()) { ... ImGui::EndTooltip(); }) so that
ImGui::EndTooltip() is only called when BeginTooltip() returns true—apply this
change to the code handling the WeatherWidget (references: ImGui::BeginTooltip,
ImGui::EndTooltip, WeatherWidget, weatherWidget->weather).
- Around line 504-508: The ImGui::Text calls are using user-controlled strings
as format strings (e.g., sortedWidgets[i]->GetFormID().c_str() and
sortedWidgets[i]->GetFilename().c_str()), which is unsafe; change those calls to
either ImGui::Text("%s", sortedWidgets[i]->GetFormID().c_str()) /
ImGui::Text("%s", sortedWidgets[i]->GetFilename().c_str()) or use
ImGui::TextUnformatted(sortedWidgets[i]->GetFormID()) and
ImGui::TextUnformatted(sortedWidgets[i]->GetFilename()) wherever those symbols
appear (also apply the same fix to the other occurrences noted in the review).
---
Nitpick comments:
In `@src/WeatherEditor/EditorWindow.cpp`:
- Around line 433-515: The current-cell lighting-template rendering duplicates
the regular widget row rendering (favorite toggle, selectable/open logic, Enter
key handling, context menu, FormID/Filename/Status columns, and background
coloring) found around the same area; extract a reusable helper like
DrawWidgetRow(Widget* widget, bool isCurrent, const std::string& editorLabel)
and move common logic (IconButton, IsFavorite, ToggleFavorite, Selectable/Open +
ImGuiKey_Enter handling, AddToRecent, BeginPopupContextItem +
settings.markedRecords/Save, TableNextColumn/Text for
GetFormID/GetFilename/status) into it; call DrawWidgetRow from both the
current-cell block (pass isCurrent=true and apply highlightColor via
Menu::GetSingleton()->GetSettings().Theme.StatusPalette.InfoColor before calling
or provide a parameter to set row background) and the regular loop
(isCurrent=false), keeping unique pre-filters (ContainsStringIgnoreCase,
showOnlyFavorites, showOnlyFlagged, dynamic_cast to LightingTemplateWidget) in
their original places.
- Line 301: The local variable emptyWidgets
(std::vector<std::unique_ptr<Widget>>) is being reconstructed every frame inside
the per-frame render path in EditorWindow.cpp; change it to a single, long-lived
instance by either making it static within the function (e.g., static ...) or
moving it to file scope as a constant empty vector so it is initialized once and
reused, avoiding per-frame allocation overhead; keep the type as
std::vector<std::unique_ptr<Widget>> and ensure the initializer is the empty
state so existing call sites using emptyWidgets continue to work unchanged.
- Around line 247-274: Extract the repeated category→widget-vector logic into a
single helper method (e.g., EditorWindow::GetWidgetCollectionForCategory(const
std::string& category) -> std::vector<WidgetType>& or const
std::vector<WidgetType>& as appropriate) that returns the correct vector for
each category (weatherWidgets, lightingTemplateWidgets, imageSpaceWidgets,
volumetricLightingWidgets, precipitationWidgets, lensFlareWidgets,
referenceEffectWidgets, plus the missing "Cell Lighting" collection), then
replace the duplicated ternary chains in the recent-widget block (the code
around the SmallButton loop) and the other occurrences (the block at lines
~300-309 and the earlier usage around line ~204) to call this helper instead;
ensure the returned reference type matches usage (mutable when you need to call
widget->SetOpen) and centralize the mapping to prevent future drift.
- Line 435: Replace the signed loop index with an unsigned type to match
sortedWidgets.size(): change the loop variable declaration from "int i" to a
size-compatible type (e.g., size_t or auto) in the for-loops that iterate over
sortedWidgets (the loops using "for (int i = 0; i < sortedWidgets.size(); ++i)")
so the comparison with sortedWidgets.size() is between the same types; apply the
same change to both occurrences (the loop around sortedWidgets at the location
referenced and the second occurrence mentioned).
- Around line 139-144: The UI state variables selectedCategory, filterBuffer,
showOnlyFlagged, and showOnlyFavorites are declared as static locals in
EditorWindow.cpp which makes them persist for the process lifetime; to avoid
surprising persistent filters between opens, move these into the EditorWindow
instance (make them member variables on the EditorWindow class) or explicitly
reset them when the window is opened/closed (e.g., in EditorWindow::Open or
EditorWindow::OnShow/OnHide). Update all references to use the instance members
instead of the static locals (search for selectedCategory, filterBuffer,
showOnlyFlagged, showOnlyFavorites).
| ImGui::Text(sortedWidgets[i]->GetFormID().c_str()); | ||
|
|
||
| // File column | ||
| ImGui::TableNextColumn(); | ||
| auto file = cell->GetFile(0); | ||
| if (file) { | ||
| ImGui::Text("%s", file->fileName); | ||
| } | ||
| ImGui::Text(sortedWidgets[i]->GetFilename().c_str()); |
There was a problem hiding this comment.
Format string vulnerability — ImGui::Text called with raw user data.
ImGui::Text(sortedWidgets[i]->GetFormID().c_str()) passes user-controlled strings directly as a format string. If the editor ID or filename contains %s, %n, etc., this can crash or corrupt memory. Use "%s" format or TextUnformatted.
Same issue at lines 508, 622, and 626.
Proposed fix
// Form ID column
ImGui::TableNextColumn();
-ImGui::Text(sortedWidgets[i]->GetFormID().c_str());
+ImGui::TextUnformatted(sortedWidgets[i]->GetFormID().c_str());
// File column
ImGui::TableNextColumn();
-ImGui::Text(sortedWidgets[i]->GetFilename().c_str());
+ImGui::TextUnformatted(sortedWidgets[i]->GetFilename().c_str());Apply the same fix at lines 622 and 626:
// Form ID column
ImGui::TableNextColumn();
-ImGui::Text(sortedWidgets[i]->GetFormID().c_str());
+ImGui::TextUnformatted(sortedWidgets[i]->GetFormID().c_str());
// File column
ImGui::TableNextColumn();
-ImGui::Text(sortedWidgets[i]->GetFilename().c_str());
+ImGui::TextUnformatted(sortedWidgets[i]->GetFilename().c_str());As per coding guidelines: "Validate user input and prevent DirectX crashes from malformed configurations."
📝 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.
| ImGui::Text(sortedWidgets[i]->GetFormID().c_str()); | |
| // File column | |
| ImGui::TableNextColumn(); | |
| auto file = cell->GetFile(0); | |
| if (file) { | |
| ImGui::Text("%s", file->fileName); | |
| } | |
| ImGui::Text(sortedWidgets[i]->GetFilename().c_str()); | |
| ImGui::TextUnformatted(sortedWidgets[i]->GetFormID().c_str()); | |
| // File column | |
| ImGui::TableNextColumn(); | |
| ImGui::TextUnformatted(sortedWidgets[i]->GetFilename().c_str()); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/WeatherEditor/EditorWindow.cpp` around lines 504 - 508, The ImGui::Text
calls are using user-controlled strings as format strings (e.g.,
sortedWidgets[i]->GetFormID().c_str() and
sortedWidgets[i]->GetFilename().c_str()), which is unsafe; change those calls to
either ImGui::Text("%s", sortedWidgets[i]->GetFormID().c_str()) /
ImGui::Text("%s", sortedWidgets[i]->GetFilename().c_str()) or use
ImGui::TextUnformatted(sortedWidgets[i]->GetFormID()) and
ImGui::TextUnformatted(sortedWidgets[i]->GetFilename()) wherever those symbols
appear (also apply the same fix to the other occurrences noted in the review).
| if (selectedCategory == "Weather" && ImGui::IsItemHovered()) { | ||
| auto* weatherWidget = dynamic_cast<WeatherWidget*>(sortedWidgets[i]); | ||
| if (weatherWidget && weatherWidget->weather) { | ||
| ImGui::BeginTooltip(); | ||
|
|
||
| // ImageSpace info | ||
| ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "ImageSpace:"); | ||
| for (int tod = 0; tod < 4; tod++) { | ||
| auto imgSpace = weatherWidget->weather->imageSpaces[tod]; | ||
| ImGui::Text(" %s: %s", | ||
| TOD::GetPeriodName(tod), | ||
| imgSpace ? imgSpace->GetFormEditorID() : "None"); | ||
| } | ||
|
|
||
| ImGui::Spacing(); | ||
|
|
||
| // VolumetricLighting info | ||
| ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "Volumetric Lighting:"); | ||
| for (int tod = 0; tod < 4; tod++) { | ||
| auto volLight = weatherWidget->weather->volumetricLighting[tod]; | ||
| ImGui::Text(" %s: %s", | ||
| TOD::GetPeriodName(tod), | ||
| volLight ? volLight->GetFormEditorID() : "None"); | ||
| } | ||
|
|
||
| ImGui::EndTooltip(); | ||
| } | ||
| } |
There was a problem hiding this comment.
BeginTooltip() return value not checked.
At line 569, ImGui::BeginTooltip() is called without checking its return value, yet EndTooltip() is called unconditionally at line 591. The rest of the codebase (line 26) correctly uses if (ImGui::BeginTooltip()). If BeginTooltip returns false (e.g., in newer Dear ImGui versions), calling EndTooltip without a matching successful Begin is undefined behavior.
Proposed fix
- ImGui::BeginTooltip();
-
- // ImageSpace info
- ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "ImageSpace:");
- for (int tod = 0; tod < 4; tod++) {
- auto imgSpace = weatherWidget->weather->imageSpaces[tod];
- ImGui::Text(" %s: %s",
- TOD::GetPeriodName(tod),
- imgSpace ? imgSpace->GetFormEditorID() : "None");
- }
-
- ImGui::Spacing();
-
- // VolumetricLighting info
- ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "Volumetric Lighting:");
- for (int tod = 0; tod < 4; tod++) {
- auto volLight = weatherWidget->weather->volumetricLighting[tod];
- ImGui::Text(" %s: %s",
- TOD::GetPeriodName(tod),
- volLight ? volLight->GetFormEditorID() : "None");
- }
-
- ImGui::EndTooltip();
+ if (ImGui::BeginTooltip()) {
+ // ImageSpace info
+ ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "ImageSpace:");
+ for (int tod = 0; tod < 4; tod++) {
+ auto imgSpace = weatherWidget->weather->imageSpaces[tod];
+ ImGui::Text(" %s: %s",
+ TOD::GetPeriodName(tod),
+ imgSpace ? imgSpace->GetFormEditorID() : "None");
+ }
+
+ ImGui::Spacing();
+
+ // VolumetricLighting info
+ ImGui::TextColored(Menu::GetSingleton()->GetTheme().StatusPalette.InfoColor, "Volumetric Lighting:");
+ for (int tod = 0; tod < 4; tod++) {
+ auto volLight = weatherWidget->weather->volumetricLighting[tod];
+ ImGui::Text(" %s: %s",
+ TOD::GetPeriodName(tod),
+ volLight ? volLight->GetFormEditorID() : "None");
+ }
+
+ ImGui::EndTooltip();
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/WeatherEditor/EditorWindow.cpp` around lines 566 - 593, The tooltip block
calls ImGui::BeginTooltip() but does not check its boolean return before calling
ImGui::EndTooltip(), risking an unmatched End call; update the block around
ImGui::BeginTooltip() inside the Weather category hover branch to use the
guarded pattern (if (ImGui::BeginTooltip()) { ... ImGui::EndTooltip(); }) so
that ImGui::EndTooltip() is only called when BeginTooltip() returns true—apply
this change to the code handling the WeatherWidget (references:
ImGui::BeginTooltip, ImGui::EndTooltip, WeatherWidget, weatherWidget->weather).
|
✅ A pre-release build is available for this PR: |
SkrubbySkrubInAShrub
force-pushed
the
scroll-objects-window
branch
from
9914fb4 to
f28bc29
Compare
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/WeatherEditor/EditorWindow.cpp (1)
435-435: Signed/unsigned comparison warning.
sortedWidgets.size()returnssize_tbut the loop variableiisint. This triggers compiler warnings and could theoretically wrap on very large collections. Usesize_torintwith a cast.Proposed fix
- for (int i = 0; i < sortedWidgets.size(); ++i) { + for (size_t i = 0; i < sortedWidgets.size(); ++i) {Apply at both lines 435 and 519.
Also applies to: 519-519
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/WeatherEditor/EditorWindow.cpp` at line 435, The for-loops iterating over sortedWidgets use an int loop variable (i) compared against sortedWidgets.size() (a size_t), causing signed/unsigned warnings; change the loop variable from int to size_t (or use auto i = size_t{0}) in the loops that reference sortedWidgets (e.g., the loops with variable i at the occurrences around sortedWidgets in EditorWindow.cpp) to ensure the types match and avoid potential wrap issues—apply the same replacement at both occurrences flagged (around the currently referenced lines).
🤖 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/EditorWindow.cpp`:
- Line 179: Replace the deprecated bool third parameter in the ImGui::BeginChild
call inside EditorWindow.cpp: change ImGui::BeginChild("##ObjectsContent", { 0,
0 }, true) to use the ImGuiChildFlags enum (e.g., ImGuiChildFlags_Border) so it
matches the rest of the codebase and current ImGui API; update the call to
ImGui::BeginChild("##ObjectsContent", { 0, 0 }, ImGuiChildFlags_Border) and
ensure any necessary includes or using directives for ImGuiChildFlags are
present.
---
Duplicate comments:
In `@src/WeatherEditor/EditorWindow.cpp`:
- Around line 504-508: The ImGui::Text calls are passing user-controlled strings
as format strings (e.g. ImGui::Text(sortedWidgets[i]->GetFormID().c_str()) and
ImGui::Text(sortedWidgets[i]->GetFilename().c_str()), and the duplicates around
the GetFormID/GetFilename uses at lines ~622/626); replace these unsafe calls
with either ImGui::TextUnformatted(sortedWidgets[i]->GetFormID().c_str()) /
ImGui::TextUnformatted(sortedWidgets[i]->GetFilename().c_str()) or use the
explicit format form ImGui::Text("%s", sortedWidgets[i]->GetFormID().c_str())
and ImGui::Text("%s", sortedWidgets[i]->GetFilename().c_str()) to eliminate
format-string vulnerability while preserving output.
- Around line 257-264: The selectedCategory ternary chain in EditorWindow.cpp
does not handle "Cell Lighting", causing it to fall back to weatherWidgets;
update the chain that assigns to widgets (based on selectedCategory) to include
an explicit branch for "Cell Lighting" mapping to the appropriate
cellLightingWidgets collection (e.g., add selectedCategory == "Cell Lighting" ?
cellLightingWidgets), preserving the existing default fallback to weatherWidgets
and keeping the same variable names (selectedCategory, widgets, weatherWidgets)
so the lookup behaves correctly.
- Around line 569-591: Wrap the tooltip content and ImGui::EndTooltip() call in
a guard that checks ImGui::BeginTooltip()'s return value; replace the unguarded
BeginTooltip()/EndTooltip() pair with: if (ImGui::BeginTooltip()) { /* the
ImageSpace and Volumetric Lighting loops using
weatherWidget->weather->imageSpaces, weatherWidget->weather->volumetricLighting
and TOD::GetPeriodName */ ImGui::EndTooltip(); } so EndTooltip() is only called
when BeginTooltip() returns true.
---
Nitpick comments:
In `@src/WeatherEditor/EditorWindow.cpp`:
- Line 435: The for-loops iterating over sortedWidgets use an int loop variable
(i) compared against sortedWidgets.size() (a size_t), causing signed/unsigned
warnings; change the loop variable from int to size_t (or use auto i =
size_t{0}) in the loops that reference sortedWidgets (e.g., the loops with
variable i at the occurrences around sortedWidgets in EditorWindow.cpp) to
ensure the types match and avoid potential wrap issues—apply the same
replacement at both occurrences flagged (around the currently referenced lines).
| } | ||
| ImGui::SameLine(); | ||
| ImGui::Text("Flagged"); | ||
| if (ImGui::BeginChild("##ObjectsContent", { 0, 0 }, true)) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check ImGui version configuration
rg -n 'IMGUI_VERSION' --type=cpp --type=h | head -10
# Also check in common header files
find . -name "imgui.h" -o -name "imgui_config.h" | head -5Repository: doodlum/skyrim-community-shaders
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Check if ImGuiChildFlags is used anywhere
rg -n 'ImGuiChildFlags' --type=cpp --type=h
# Check how BeginChild is being called elsewhere
rg -n 'BeginChild' --type=cpp --type=h | head -20Repository: doodlum/skyrim-community-shaders
Length of output: 2385
🏁 Script executed:
#!/bin/bash
# Look at the specific file and line in context
sed -n '170,190p' src/WeatherEditor/EditorWindow.cppRepository: doodlum/skyrim-community-shaders
Length of output: 560
🏁 Script executed:
#!/bin/bash
# Search for imgui headers and version info
find . -name "imgui.h" -o -name "imconfig.h" 2>/dev/null | head -10Repository: doodlum/skyrim-community-shaders
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Check project configuration files for ImGui version
rg -n 'imgui|IMGUI' --type=json --type=txt --type=cmake | grep -i version | head -20Repository: doodlum/skyrim-community-shaders
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Look for vcpkg manifest or conan files
cat vcpkg.json 2>/dev/null || cat conanfile.txt 2>/dev/null || echo "No vcpkg/conan file found"Repository: doodlum/skyrim-community-shaders
Length of output: 1594
🏁 Script executed:
#!/bin/bash
# Look for imgui header files in the vcpkg directory or local includes
find . -type f -name "imgui.h" 2>/dev/null | head -3Repository: doodlum/skyrim-community-shaders
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Search for BeginChild function signature in code to see supported overloads
rg -A 5 'IMGUI_API.*BeginChild' --type=h | head -30Repository: doodlum/skyrim-community-shaders
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Check if there are any build warnings or deprecation comments in the code
rg -n 'deprecated|DEPRECATED|ImGuiChildFlags_Borders' --type=cpp --type=h | head -20Repository: doodlum/skyrim-community-shaders
Length of output: 317
🌐 Web query:
ImGui 1.90 BeginChild bool border parameter deprecated ImGuiChildFlags
💡 Result:
In Dear ImGui v1.90.0 (2023-11-02), BeginChild()’s bool border parameter was replaced by ImGuiChildFlags, with a new flag meant to be the direct equivalent of the old border=true. (skia.googlesource.com)
What to change
Old:
ImGui::BeginChild("Name", size, true);
ImGui::BeginChild("Name", size, false);New (v1.90.x):
ImGui::BeginChild("Name", size, ImGuiChildFlags_Border); // was: true
ImGui::BeginChild("Name", size); // was: false
// or: ImGui::BeginChild("Name", size, ImGuiChildFlags_None);Compatibility note (why old code often still compiles)
ImGui kept bit 0 of ImGuiChildFlags compatible with the old boolean, so passing true can still map to the border flag during the transition. (skia.googlesource.com)
Rename you may hit in newer versions
From v1.91.1, ImGuiChildFlags_Border was renamed to ImGuiChildFlags_Borders. (skia.googlesource.com)
Citations:
- 1: https://skia.googlesource.com/external/github.com/ocornut/imgui/%2B/4cc464eadc12a158fb7ccd9b2843277ee894f163/imgui.cpp?utm_source=openai
- 2: https://skia.googlesource.com/external/github.com/ocornut/imgui/%2B/4cc464eadc12a158fb7ccd9b2843277ee894f163/imgui.cpp?utm_source=openai
- 3: https://skia.googlesource.com/external/github.com/ocornut/imgui/%2B/4cc464eadc12a158fb7ccd9b2843277ee894f163/imgui.cpp?utm_source=openai
- 4: https://skia.googlesource.com/external/github.com/ocornut/imgui/%2B/4cc464eadc12a158fb7ccd9b2843277ee894f163/imgui.cpp?utm_source=openai
Update BeginChild to use ImGuiChildFlags instead of deprecated bool parameter.
ImGui::BeginChild("##ObjectsContent", { 0, 0 }, true) at line 179 uses the deprecated bool parameter. ImGui 1.90 (currently in use per vcpkg.json) replaced this with ImGuiChildFlags. Replace true with ImGuiChildFlags_Border:
ImGui::BeginChild("##ObjectsContent", { 0, 0 }, ImGuiChildFlags_Border)The codebase already uses ImGuiChildFlags in other places (e.g., AdvancedSettingsRenderer.cpp:233), so this aligns with the existing pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/WeatherEditor/EditorWindow.cpp` at line 179, Replace the deprecated bool
third parameter in the ImGui::BeginChild call inside EditorWindow.cpp: change
ImGui::BeginChild("##ObjectsContent", { 0, 0 }, true) to use the ImGuiChildFlags
enum (e.g., ImGuiChildFlags_Border) so it matches the rest of the codebase and
current ImGui API; update the call to ImGui::BeginChild("##ObjectsContent", { 0,
0 }, ImGuiChildFlags_Border) and ensure any necessary includes or using
directives for ImGuiChildFlags are present.
Automated formatting by clang-format, prettier, and other hooks. See https://pre-commit.ci for details.
SkrubbySkrubInAShrub
force-pushed
the
scroll-objects-window
branch
from
3b41256 to
ef86aef
Compare
1 participant
This PR separates the left and right pane of the objects window in the weather editor, so that if a user scrolls the right pane the left pane doesn't get scrolled.
Summary by CodeRabbit