feat(UI): consolidate searchbar to util and add to weather editor

[SkrubbySkrubInAShrub]

Consolidates searchbar functionality to the util namespace in UI.cpp and adds its functionality to the weather picker.

Summary by CodeRabbit

• New Features

  • Per-item search added to the weather selector with real-time, case-insensitive filtering by name and ID; matched items keep their type-specific coloring.
  • Search input persists per-selector, auto-focuses when opened, clears on selection or close, and resets focus/default selection when reopened.
  • Selecting a weather applies instantly if instant-change is enabled.

• Style

  • Improved search input visuals: icon, sizing, offsets, and padding for clearer layout.

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds a per-combo searchable input API and theme constants, refactors SearchableCombo to use per-id search state, and integrates a filtered, color-aware weather selector in WeatherEditor with focus/reset and immediate or transitioned apply behavior.

Changes

Cohort / File(s)	Summary
UI Search Infrastructure src/Utils/UI.h, src/Utils/UI.cpp	Adds DrawComboSearchInput(const char* id) and ClearComboSearch(const char* id); introduces detail::ComboSearchState and a per-id state map; implements persistent per-combo search buffer and focus handling.
Theme Styling Constants src/Menu/ThemeManager.h	Adds four ThemeManager::Constants floats: COMBO_SEARCH_ICON_SIZE, COMBO_SEARCH_ICON_ALPHA, COMBO_SEARCH_ICON_OFFSET_X, COMBO_SEARCH_PADDING_LEFT to control combo search input layout/visuals.
Weather Editor Integration src/Features/WeatherEditor.cpp	Renders combo search input; performs case-insensitive filtering across editorId/name/FormID (hex); applies per-item coloring only to matched items; maps selection through filtered index and clears/reset search on selection and combo open/close; supports instant vs. transitioned weather changes.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as Util::DrawComboSearchInput
    participant State as Util::ComboSearchState (map)
    participant WeatherEditor
    participant WeatherSys as Weather System

    User->>UI: Open combo / type search
    UI->>State: read/update buffer for id
    UI-->>User: render input + filtered list
    User->>WeatherEditor: select item (uses filtered index)
    WeatherEditor->>State: ClearComboSearch(id)
    alt accelerate change enabled
        WeatherEditor->>WeatherSys: apply weather instantly
    else
        WeatherEditor->>WeatherSys: request normal transition
    end
    WeatherSys-->>WeatherEditor: ack
    WeatherEditor-->>User: close combo (reset focus)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(weather editor): improve weather picker UI and functionality #1863: Touches the same WeatherEditor weather picker/filtering and s_filteredWeathers handling.

Suggested reviewers

• alandtse
• davo0411

Poem

🐰 I found a tiny search beneath the combobox,
I sniffed through names and hex like sneaky little coss,
I painted matches with a joyful hop,
Cleared my whiskers when the selection popped,
Now skies obey my focused, fluffy boss. 🌦️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately describes the main changes: consolidating searchbar functionality into util namespace and adding it to the weather editor.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

Using provided base ref: 6d9f431
Using base ref: 6d9f431
Base commit date: 2026-02-19T11:38:52Z (Thursday, February 19, 2026 11:38 AM)
No actionable suggestions for changed features.

[coderabbitai]

🧹 Nitpick comments (2)

src/Features/WeatherEditor.cpp (2)

710-712: std::tolower on plain char is technically undefined for negative values.

If char is signed (common on MSVC), passing a negative value to std::tolower(int) is UB per the C standard. The safe idiom is to cast through unsigned char:

♻️ Suggested fix

 				auto matchesSearch = [&](const std::string& text) {
 					return !text.empty() && std::search(text.begin(), text.end(), s_weatherSearchBuffer, s_weatherSearchBuffer + strlen(s_weatherSearchBuffer),
-											[](char a, char b) { return std::tolower(a) == std::tolower(b); }) != text.end();
+											[](char a, char b) { return std::tolower(static_cast<unsigned char>(a)) == std::tolower(static_cast<unsigned char>(b)); }) != text.end();
 				};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/WeatherEditor.cpp` around lines 710 - 712, The lambda
matchesSearch uses std::tolower on plain char which is undefined for negative
char values; update the comparison in that lambda (the call site where
std::tolower is used) to cast both characters to unsigned char before passing to
std::tolower (e.g. static_cast<unsigned char>(a) and static_cast<unsigned
char>(b)) so the case-insensitive search using s_weatherSearchBuffer is safe
across platforms.

---

693-697: Consider auto-focusing the search input when the combo opens.

Without SetKeyboardFocusHere, users must click the search box every time the combo opens. A common pattern for filterable combos is:

♻️ Suggested improvement

 	if (ImGui::BeginCombo("Weather", comboPreview)) {
+		// Auto-focus search box on first frame
+		static bool s_comboJustOpened = true;
+		if (s_comboJustOpened) {
+			ImGui::SetKeyboardFocusHere();
+			s_comboJustOpened = false;
+		}
 		ImGui::PushStyleVar(ImGuiStyleVar_FramePadding, ImVec2(24.0f, ImGui::GetStyle().FramePadding.y));
 		ImGui::InputText("##weather_search", s_weatherSearchBuffer, IM_ARRAYSIZE(s_weatherSearchBuffer));
 		ImGui::PopStyleVar();
 		Util::DrawSearchIcon(ImVec2(ImGui::GetItemRectMin().x + 5.0f, ImGui::GetItemRectMin().y + (ImGui::GetItemRectSize().y - 16.0f) * 0.5f), 16.0f, 0.5f);
 		ImGui::Separator();

And reset s_comboJustOpened = true in the else branch (line 746-748) when the combo is closed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/WeatherEditor.cpp` around lines 693 - 697, When the filterable
combo opens the search InputText (s_weatherSearchBuffer) isn't auto-focused,
forcing users to click; modify the combo-open branch where ImGui::InputText is
called to call ImGui::SetKeyboardFocusHere() just before ImGui::InputText when
s_comboJustOpened is true, and after calling SetKeyboardFocusHere clear
s_comboJustOpened so focus only happens once; also ensure you reset
s_comboJustOpened = true in the else branch when the combo closes so the next
open will re-focus.

🤖 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/Features/WeatherEditor.cpp`:
- Around line 710-712: The lambda matchesSearch uses std::tolower on plain char
which is undefined for negative char values; update the comparison in that
lambda (the call site where std::tolower is used) to cast both characters to
unsigned char before passing to std::tolower (e.g. static_cast<unsigned char>(a)
and static_cast<unsigned char>(b)) so the case-insensitive search using
s_weatherSearchBuffer is safe across platforms.
- Around line 693-697: When the filterable combo opens the search InputText
(s_weatherSearchBuffer) isn't auto-focused, forcing users to click; modify the
combo-open branch where ImGui::InputText is called to call
ImGui::SetKeyboardFocusHere() just before ImGui::InputText when
s_comboJustOpened is true, and after calling SetKeyboardFocusHere clear
s_comboJustOpened so focus only happens once; also ensure you reset
s_comboJustOpened = true in the else branch when the combo closes so the next
open will re-focus.

[github-actions]

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

[alandtse]

Is this something that should be generalized so other features can use a search?

[SkrubbySkrubInAShrub]

Is this something that should be generalized so other features can use a search?

good point, may come in useful as a general function. I'll have a look

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (4)

src/Utils/UI.cpp (3)

881-885: GetComboSearchStates() map grows without bound

Every unique id creates a permanent entry that is never removed. For the current use-cases (static string literals like kWeatherSearchId and combo labels), the map stays small and this is harmless. If DrawComboSearchInput is ever called with dynamically constructed IDs the map will grow unboundedly.

Consider documenting that id must be a stable, finite set of string literals, or add an explicit RemoveComboSearch(const char* id) API for cleanup.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Utils/UI.cpp` around lines 881 - 885, GetComboSearchStates() currently
stores ComboSearchState by arbitrary id strings and never removes entries,
causing unbounded growth if callers pass dynamic IDs; update the API by adding a
removal function (e.g., RemoveComboSearch(const char* id) or
RemoveComboSearchState(const std::string& id)) that erases the corresponding
entry from the static unordered_map (returned by GetComboSearchStates()), and
update call sites such as DrawComboSearchInput to document that callers must
call this removal for dynamically created IDs (or alternatively document that
ids must be stable, finite string literals).

---

888-913: string_view into mutable map storage creates stale-data hazard

DrawComboSearchInput returns a std::string_view whose .data() and .size() are computed once at construction from state.buffer. When a caller subsequently calls ClearComboSearch for the same id, state.buffer[0] is set to '\0', but the view's stored size is unchanged. This means:

• searchText.empty() still returns false (size > 0),
• std::string(searchText) produces a string with an embedded null at position 0,
• StringMatchesSearch returns false for every item name, effectively hiding all remaining entries.

In SearchableCombo (UI.h line 947), ClearComboSearch(label) is called inside the iteration loop without a break. All subsequent loop iterations encounter the corrupted view. Visually harmless only because the combo popup is already closing on that frame, but it is a silent correctness hole and a design footgun for future callers.

Returning std::string eliminates both the stale-data issue and the latent rehash invalidation risk:

♻️ Proposed fix – return `std::string` instead of `std::string_view`

In src/Utils/UI.cpp:

-std::string_view DrawComboSearchInput(const char* id)
+std::string DrawComboSearchInput(const char* id)
 {
     auto& state = detail::GetComboSearchStates()[id];
     // ...
-    return state.buffer[0] != '\0' ? std::string_view(state.buffer) : std::string_view{};
+    return state.buffer[0] != '\0' ? std::string(state.buffer) : std::string{};
 }

In src/Utils/UI.h:

-std::string_view DrawComboSearchInput(const char* id);
+std::string DrawComboSearchInput(const char* id);

Update SearchableCombo accordingly:

-auto searchText = DrawComboSearchInput(label);
+std::string searchText = DrawComboSearchInput(label);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Utils/UI.cpp` around lines 888 - 913, DrawComboSearchInput currently
returns a std::string_view into mutable state.buffer which can become stale when
ClearComboSearch mutates that buffer; change DrawComboSearchInput to return a
std::string (constructing it from state.buffer) and update its declaration in
the header and every caller (notably SearchableCombo) to accept/handle a
std::string instead of std::string_view so callers get an owning, stable copy;
ensure any function signatures and includes are adjusted accordingly and remove
reliance on the old non-owning view to avoid size/data mismatch after buffer
mutations.

---

903-903: std::format allocates a heap string every render frame

std::format("##{}_search", id).c_str() creates a temporary std::string on every call. Since id is a const char* (typically a compile-time string literal), a fixed-size stack buffer suffices:

♻️ Proposed fix – stack-allocated ID string

-ImGui::InputText(std::format("##{}_search", id).c_str(), state.buffer, IM_ARRAYSIZE(state.buffer));
+char widgetId[128];
+snprintf(widgetId, sizeof(widgetId), "##%s_search", id);
+ImGui::InputText(widgetId, state.buffer, IM_ARRAYSIZE(state.buffer));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Utils/UI.cpp` at line 903, The call to ImGui::InputText uses
std::format("##{}_search", id).c_str(), which allocates a temporary std::string
each frame; replace this with a small stack buffer and snprintf to avoid heap
allocations. Inside the function where ImGui::InputText(...) is called, create a
local char buffer (e.g., char id_buf[64];) and populate it with
std::snprintf(id_buf, sizeof(id_buf), "##%s_search", id) (or allocate size based
on strlen(id)+...). Then pass id_buf to ImGui::InputText instead of the
std::format expression; keep using state.buffer and IM_ARRAYSIZE(state.buffer)
unchanged. Ensure the buffer is large enough for worst-case id to avoid
truncation.

src/Utils/UI.h (1)

710-728: PR title doesn't conform to Conventional Commits style

The current title is feat(weather editor): Add search bar to weather picker:

• Add should be lowercase.
• Scopes conventionally use kebab-case (no spaces).
• Character count: "feat(weather-editor): add search bar to weather picker" = 55 chars, which exceeds the 50-char limit.

Suggested alternatives (both under 50 chars):

• feat(weather-editor): add weather picker search (48 chars)
• feat(weather): add search bar to weather picker (47 chars)

As per coding guidelines: "Format: type(scope): description — Length: 50 characters limit for title — Style: lowercase description, no ending period".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Utils/UI.h` around lines 710 - 728, Update the PR title to follow
Conventional Commits: use lowercase description, kebab-case scope, no trailing
period, and keep <=50 characters; for example change "feat(weather editor): Add
search bar to weather picker" to one of the suggested alternatives such as
"feat(weather-editor): add weather picker search" or "feat(weather): add search
bar to weather picker" and ensure the final title is under 50 chars and uses the
proper format (type(scope): description).

🤖 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/Utils/UI.cpp`:
- Around line 903-904: Replace the plain ImGui::InputText call in
DrawFeatureSearchBar's combo search with ImGui::InputTextWithHint so the field
shows a placeholder (e.g., "Search Features..."); keep the same unique id
pattern std::format("##{}_search", id).c_str(), pass state.buffer and
IM_ARRAYSIZE(state.buffer) as before, and preserve the surrounding
ImGui::PopStyleVar() call.

In `@src/Utils/UI.h`:
- Around line 940-950: The loop continues iterating after a selection because
ClearComboSearch(label) clears the underlying buffer but searchText (from
DrawComboSearchInput) remains stale; fix by immediately exiting the for-loop
after a successful ImGui::Selectable selection: when
ImGui::Selectable(itemName.c_str(), itemName == selectedName) triggers, set
selectedName and valueChanged, call ClearComboSearch(label) and then break out
of the loop to stop further iterations and avoid relying on the stale
searchText; locate this behavior around DrawComboSearchInput, searchText,
itemMap, selectedName, valueChanged, StringMatchesSearch and ClearComboSearch.

---

Nitpick comments:
In `@src/Utils/UI.cpp`:
- Around line 881-885: GetComboSearchStates() currently stores ComboSearchState
by arbitrary id strings and never removes entries, causing unbounded growth if
callers pass dynamic IDs; update the API by adding a removal function (e.g.,
RemoveComboSearch(const char* id) or RemoveComboSearchState(const std::string&
id)) that erases the corresponding entry from the static unordered_map (returned
by GetComboSearchStates()), and update call sites such as DrawComboSearchInput
to document that callers must call this removal for dynamically created IDs (or
alternatively document that ids must be stable, finite string literals).
- Around line 888-913: DrawComboSearchInput currently returns a std::string_view
into mutable state.buffer which can become stale when ClearComboSearch mutates
that buffer; change DrawComboSearchInput to return a std::string (constructing
it from state.buffer) and update its declaration in the header and every caller
(notably SearchableCombo) to accept/handle a std::string instead of
std::string_view so callers get an owning, stable copy; ensure any function
signatures and includes are adjusted accordingly and remove reliance on the old
non-owning view to avoid size/data mismatch after buffer mutations.
- Line 903: The call to ImGui::InputText uses std::format("##{}_search",
id).c_str(), which allocates a temporary std::string each frame; replace this
with a small stack buffer and snprintf to avoid heap allocations. Inside the
function where ImGui::InputText(...) is called, create a local char buffer
(e.g., char id_buf[64];) and populate it with std::snprintf(id_buf,
sizeof(id_buf), "##%s_search", id) (or allocate size based on strlen(id)+...).
Then pass id_buf to ImGui::InputText instead of the std::format expression; keep
using state.buffer and IM_ARRAYSIZE(state.buffer) unchanged. Ensure the buffer
is large enough for worst-case id to avoid truncation.

In `@src/Utils/UI.h`:
- Around line 710-728: Update the PR title to follow Conventional Commits: use
lowercase description, kebab-case scope, no trailing period, and keep <=50
characters; for example change "feat(weather editor): Add search bar to weather
picker" to one of the suggested alternatives such as "feat(weather-editor): add
weather picker search" or "feat(weather): add search bar to weather picker" and
ensure the final title is under 50 chars and uses the proper format
(type(scope): description).

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Utils/UI.h (1)

1-1: ⚠️ Potential issue | 🟡 Minor

PR title exceeds the 50-character limit from Conventional Commits

The current title feat(UI): consolidate searchbar to util and add to weather editor is 65 characters. Consider shortening, e.g.:

feat(UI): centralize combo searchbar, add to weather editor

(60 chars — still over; a tighter version: feat(ui): add reusable combo searchbar util at 43 chars)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Utils/UI.h` at line 1, Update the PR title to conform to the Conventional
Commits 50-character limit by replacing "feat(UI): consolidate searchbar to util
and add to weather editor" with a shorter title such as "feat(ui): add reusable
combo searchbar util" (43 chars), ensuring the scope is lowercase and the
message stays concise.

🤖 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/Features/WeatherEditor.cpp`:
- Around line 713-727: ImGui::PushStyleColor is currently followed by an early
break inside the ImGui::Selectable branch which skips ImGui::PopStyleColor and
corrupts the ImGui style stack; fix by calling ImGui::Selectable into a
temporary bool (e.g. bool clicked = ImGui::Selectable(weatherLabels[i].c_str(),
isSelected)), then immediately call ImGui::PopStyleColor(), and only after
popping handle the selection logic that sets s_selectedWeatherIdx, reads
selectedWeather from s_filteredWeathers, calls sky->ForceWeather or
sky->SetWeather, calls Util::ClearComboSearch(kWeatherSearchId), logs via
logger::info and breaks; this ensures PopStyleColor always runs even when the
item is selected.

---

Outside diff comments:
In `@src/Utils/UI.h`:
- Line 1: Update the PR title to conform to the Conventional Commits
50-character limit by replacing "feat(UI): consolidate searchbar to util and add
to weather editor" with a shorter title such as "feat(ui): add reusable combo
searchbar util" (43 chars), ensuring the scope is lowercase and the message
stays concise.

[coderabbitai]

🧹 Nitpick comments (1)

src/Features/WeatherEditor.cpp (1)

701-711: Minor: per-frame heap allocations in the search hot path.

editorId, name, and formId are re-constructed as std::string/std::format for every visible weather item on every rendered frame while the search box is non-empty. For a modded load order with several hundred weather records this is a large number of small heap allocations per frame.

Consider building and caching these searchable strings once (alongside weatherLabels) in UpdateFilteredWeathers, or passing the raw const char* directly to StringMatchesSearch if that overload exists.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/Features/WeatherEditor.cpp` around lines 701 - 711, The per-frame
allocations come from constructing editorId, name, and formId inside the
render-time search loop; update the code so those searchable strings are built
once and cached (e.g., alongside weatherLabels in UpdateFilteredWeathers)
instead of reallocating each frame: in UpdateFilteredWeathers compute and store
editorId (from GetFormEditorID), name (from GetName), and formatted formId (from
GetFormID) for each weather entry, then modify the render-time loop to query
those cached strings when calling Util::StringMatchesSearch (or, if an overload
accepting const char* exists, pass raw pointers from GetFormEditorID/GetName
directly). Ensure references to UpdateFilteredWeathers, weatherLabels,
Util::StringMatchesSearch, GetFormEditorID, GetName, and GetFormID are used to
locate and update the relevant code.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@src/Features/WeatherEditor.cpp`:
- Around line 713-715: Move the ImGui style-pop and selection capture so the
style stack is never left unbalanced: call ImGui::PushStyleColor(ImGuiCol_Text,
GetWeatherTypeColor(weather)) before rendering, capture the selection result
into bool didSelect = ImGui::Selectable(weatherLabels[i].c_str(), isSelected),
then immediately call ImGui::PopStyleColor(); only after PopStyleColor check
didSelect and enter the selection branch (avoid placing PopStyleColor after a
break). Ensure the referenced symbols ImGui::PushStyleColor, ImGui::Selectable,
ImGui::PopStyleColor, didSelect, and weatherLabels are used exactly as
described.

---

Nitpick comments:
In `@src/Features/WeatherEditor.cpp`:
- Around line 701-711: The per-frame allocations come from constructing
editorId, name, and formId inside the render-time search loop; update the code
so those searchable strings are built once and cached (e.g., alongside
weatherLabels in UpdateFilteredWeathers) instead of reallocating each frame: in
UpdateFilteredWeathers compute and store editorId (from GetFormEditorID), name
(from GetName), and formatted formId (from GetFormID) for each weather entry,
then modify the render-time loop to query those cached strings when calling
Util::StringMatchesSearch (or, if an overload accepting const char* exists, pass
raw pointers from GetFormEditorID/GetName directly). Ensure references to
UpdateFilteredWeathers, weatherLabels, Util::StringMatchesSearch,
GetFormEditorID, GetName, and GetFormID are used to locate and update the
relevant code.

[davo0411]

Is this something that should be generalized so other features can use a search?

yes, the search bar i made for this is significantly better than the old one cause it has the partial string match and pretty auto-complete for settings. this would be nice to upgrade the OG feature search bar to allow searching of specific settings too. So users can search for "parallax" for example and find the settings