From ccbf0fc9a2813a6f33db06fb3d3c05f6c2c4150f Mon Sep 17 00:00:00 2001 From: Gleb Mazovetskiy Date: Sat, 11 Jan 2025 13:00:33 +0000 Subject: [PATCH] Audio device settings: Fix name handling 1. The char pointer returned by SDL is only valid temporarily, keeping it around is not allowed so we copy the string. 2. Moves the trimming to DiabloUI. It now happens every frame but that's OK here (not performance-sensitive and there is little else happening on that screen). --- Source/DiabloUI/diabloui.cpp | 40 ++++++++++++++++++++++--------- Source/DiabloUI/hero/selhero.cpp | 4 ++-- Source/DiabloUI/multi/selconn.cpp | 4 +++- Source/DiabloUI/multi/selgame.cpp | 4 ++-- Source/DiabloUI/settingsmenu.cpp | 30 +++++++++++------------ Source/DiabloUI/ui_item.h | 11 +++++---- Source/options.cpp | 18 +------------- 7 files changed, 58 insertions(+), 53 deletions(-) diff --git a/Source/DiabloUI/diabloui.cpp b/Source/DiabloUI/diabloui.cpp index 044452dee61..c12c69a375d 100644 --- a/Source/DiabloUI/diabloui.cpp +++ b/Source/DiabloUI/diabloui.cpp @@ -21,6 +21,7 @@ #include "engine/dx.h" #include "engine/load_pcx.hpp" #include "engine/render/clx_render.hpp" +#include "engine/render/text_render.hpp" #include "engine/ticks.hpp" #include "hwcursor.hpp" #include "init.h" @@ -714,14 +715,24 @@ void UiFadeIn() RenderPresent(); } -void DrawSelector(const SDL_Rect &rect) +namespace { +ClxSpriteList GetListSelectorSprites(int itemHeight) { - int size = FOCUS_SMALL; - if (rect.h >= 42) + int size; + if (itemHeight >= 42) { size = FOCUS_BIG; - else if (rect.h >= 30) + } else if (itemHeight >= 30) { size = FOCUS_MED; - const ClxSpriteList sprites = *ArtFocus[size]; + } else { + size = FOCUS_SMALL; + } + return *ArtFocus[size]; +} +} // namespace + +void DrawSelector(const SDL_Rect &rect) +{ + const ClxSpriteList sprites = GetListSelectorSprites(rect.h); const ClxSprite sprite = sprites[GetAnimationFrame(sprites.numSprites())]; // TODO FOCUS_MED appears higher than the box @@ -813,18 +824,25 @@ void Render(const UiList &uiList) const Surface &out = Surface(DiabloUiSurface()); for (std::size_t i = listOffset; i < uiList.m_vecItems.size() && (i - listOffset) < ListViewportSize; ++i) { - SDL_Rect rect = uiList.itemRect(static_cast(i - listOffset)); + const SDL_Rect rect = uiList.itemRect(static_cast(i - listOffset)); const UiListItem &item = *uiList.GetItem(i); if (i == SelectedItem) DrawSelector(rect); - Rectangle rectangle = MakeRectangle(rect); + const Rectangle rectangle = MakeRectangle(rect).inset( + Displacement(GetListSelectorSprites(rect.h)[0].width(), 0)); + + const UiFlags uiFlags = uiList.GetFlags() | item.uiFlags; + const GameFontTables fontSize = GetFontSizeFromUiFlags(uiFlags); + std::string_view text = item.m_text.str(); + while (GetLineWidth(text, fontSize, 1) > rectangle.size.width) { + text = std::string_view(text.data(), FindLastUtf8Symbols(text)); + } + if (item.args.empty()) { - DrawString(out, item.m_text, rectangle, - { .flags = uiList.GetFlags() | item.uiFlags, .spacing = uiList.GetSpacing() }); + DrawString(out, text, rectangle, { .flags = uiFlags, .spacing = uiList.GetSpacing() }); } else { - DrawStringWithColors(out, item.m_text, item.args, rectangle, - { .flags = uiList.GetFlags() | item.uiFlags, .spacing = uiList.GetSpacing() }); + DrawStringWithColors(out, text, item.args, rectangle, { .flags = uiFlags, .spacing = uiList.GetSpacing() }); } } } diff --git a/Source/DiabloUI/hero/selhero.cpp b/Source/DiabloUI/hero/selhero.cpp index 31e61505fe2..b2c7be7af0e 100644 --- a/Source/DiabloUI/hero/selhero.cpp +++ b/Source/DiabloUI/hero/selhero.cpp @@ -514,11 +514,11 @@ void selhero_List_Init() vecSelHeroDlgItems.clear(); for (std::size_t i = 0; i < selhero_SaveCount; i++) { - vecSelHeroDlgItems.push_back(std::make_unique(selhero_heros[i].name, static_cast(i))); + vecSelHeroDlgItems.push_back(std::make_unique(std::string_view(selhero_heros[i].name), static_cast(i))); if (selhero_heros[i].saveNumber == selhero_heroInfo.saveNumber) selectedItem = i; } - vecSelHeroDlgItems.push_back(std::make_unique(_("New Hero").data(), static_cast(selhero_SaveCount))); + vecSelHeroDlgItems.push_back(std::make_unique(_("New Hero"), static_cast(selhero_SaveCount))); vecSelDlgItems.push_back(std::make_unique(vecSelHeroDlgItems, 6, uiPosition.x + 265, (uiPosition.y + 256), 320, 26, UiFlags::AlignCenter | UiFlags::FontSize24 | UiFlags::ColorUiGold)); diff --git a/Source/DiabloUI/multi/selconn.cpp b/Source/DiabloUI/multi/selconn.cpp index 654ad2945e5..5fc96b366a9 100644 --- a/Source/DiabloUI/multi/selconn.cpp +++ b/Source/DiabloUI/multi/selconn.cpp @@ -1,3 +1,5 @@ +#include + #include #include "DiabloUI/diabloui.h" @@ -39,7 +41,7 @@ void SelconnLoad() #ifndef NONET #ifndef DISABLE_ZERO_TIER - vecConnItems.push_back(std::make_unique(ConnectionNames[SELCONN_ZT], SELCONN_ZT)); + vecConnItems.push_back(std::make_unique(std::string_view(ConnectionNames[SELCONN_ZT]), SELCONN_ZT)); #endif #ifndef DISABLE_TCP vecConnItems.push_back(std::make_unique(_(ConnectionNames[SELCONN_TCP]), SELCONN_TCP)); diff --git a/Source/DiabloUI/multi/selgame.cpp b/Source/DiabloUI/multi/selgame.cpp index 9ae5d6b23af..eb19c202ae5 100644 --- a/Source/DiabloUI/multi/selgame.cpp +++ b/Source/DiabloUI/multi/selgame.cpp @@ -151,7 +151,7 @@ void UiInitGameSelectionList(std::string_view search) vecSelGameDlgItems.push_back(std::make_unique(_("Join Game"), 2, UiFlags::ColorUiGold)); if (provider == SELCONN_ZT) { - vecSelGameDlgItems.push_back(std::make_unique("", -1, UiFlags::ElementDisabled)); + vecSelGameDlgItems.push_back(std::make_unique(std::string_view {}, -1, UiFlags::ElementDisabled)); vecSelGameDlgItems.push_back(std::make_unique(_("Public Games"), -1, UiFlags::ElementDisabled | UiFlags::ColorWhitegold)); if (Gamelist.empty()) { @@ -162,7 +162,7 @@ void UiInitGameSelectionList(std::string_view search) vecSelGameDlgItems.push_back(std::make_unique(_("None"), -1, UiFlags::ElementDisabled | UiFlags::ColorUiSilver)); } else { for (unsigned i = 0; i < Gamelist.size(); i++) { - vecSelGameDlgItems.push_back(std::make_unique(Gamelist[i].name, i + 3, UiFlags::ColorUiGold)); + vecSelGameDlgItems.push_back(std::make_unique(std::string_view(Gamelist[i].name), i + 3, UiFlags::ColorUiGold)); } } } diff --git a/Source/DiabloUI/settingsmenu.cpp b/Source/DiabloUI/settingsmenu.cpp index f0c82d6f468..d30d73a5e70 100644 --- a/Source/DiabloUI/settingsmenu.cpp +++ b/Source/DiabloUI/settingsmenu.cpp @@ -76,8 +76,8 @@ bool IsValidEntry(OptionEntryBase *pOptionEntry) std::vector CreateDrawStringFormatArgForEntry(OptionEntryBase *pEntry) { return std::vector { - { pEntry->GetName().data(), UiFlags::ColorUiGold }, - { pEntry->GetValueDescription().data(), UiFlags::ColorUiSilver } + { pEntry->GetName(), UiFlags::ColorUiGold }, + { pEntry->GetValueDescription(), UiFlags::ColorUiSilver } }; } @@ -238,7 +238,7 @@ void ItemSelected(size_t value) case SpecialMenuEntry::UnbindKey: { auto *pOptionKey = static_cast(selectedOption); pOptionKey->SetValue(SDLK_UNKNOWN); - vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription().data(); + vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription(); break; } case SpecialMenuEntry::BindPadButton: @@ -247,7 +247,7 @@ void ItemSelected(size_t value) case SpecialMenuEntry::UnbindPadButton: auto *pOptionPad = static_cast(selectedOption); pOptionPad->SetValue(ControllerButton_NONE); - vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription().data(); + vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription(); break; } return; @@ -297,7 +297,7 @@ void ItemSelected(size_t value) for (auto &arg : args) vecItem->args.push_back(arg); if (optionUsesTwoLines) { - vecDialogItems[value + 1]->m_text = pOption->GetValueDescription().data(); + vecDialogItems[value + 1]->m_text = std::string(pOption->GetValueDescription()); } } } @@ -407,10 +407,10 @@ void UiSettingsMenu() auto formatArgs = CreateDrawStringFormatArgForEntry(pEntry); int optionId = static_cast(vecOptions.size()); if (NeedsTwoLinesToDisplayOption(formatArgs)) { - vecDialogItems.push_back(std::make_unique("{}:", formatArgs, optionId, UiFlags::ColorUiGold | UiFlags::NeedsNextElement)); - vecDialogItems.push_back(std::make_unique(pEntry->GetValueDescription(), optionId, UiFlags::ColorUiSilver | UiFlags::ElementDisabled)); + vecDialogItems.push_back(std::make_unique(std::string_view("{}:"), formatArgs, optionId, UiFlags::ColorUiGold | UiFlags::NeedsNextElement)); + vecDialogItems.push_back(std::make_unique(std::string(pEntry->GetValueDescription()), optionId, UiFlags::ColorUiSilver | UiFlags::ElementDisabled)); } else { - vecDialogItems.push_back(std::make_unique("{}: {}", formatArgs, optionId, UiFlags::ColorUiGold)); + vecDialogItems.push_back(std::make_unique(std::string_view("{}: {}"), formatArgs, optionId, UiFlags::ColorUiGold)); } vecOptions.push_back(pEntry); } @@ -425,7 +425,7 @@ void UiSettingsMenu() } break; case ShownMenuType::KeyInput: { vecDialogItems.push_back(std::make_unique(_("Bound key:"), static_cast(SpecialMenuEntry::None), UiFlags::ColorWhitegold | UiFlags::ElementDisabled)); - vecDialogItems.push_back(std::make_unique(selectedOption->GetValueDescription(), static_cast(SpecialMenuEntry::None), UiFlags::ColorUiGold)); + vecDialogItems.push_back(std::make_unique(std::string(selectedOption->GetValueDescription()), static_cast(SpecialMenuEntry::None), UiFlags::ColorUiGold)); assert(IndexKeyOrPadInput == vecDialogItems.size() - 1); itemToSelect = IndexKeyOrPadInput; eventHandler = [](SDL_Event &event) { @@ -470,11 +470,11 @@ void UiSettingsMenu() auto *pOptionKey = static_cast(selectedOption); if (!pOptionKey->SetValue(key)) return false; - vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription().data(); + vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription(); return true; }; vecDialogItems.push_back(std::make_unique(_("Press any key to change."), static_cast(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled)); - vecDialogItems.push_back(std::make_unique("", static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); + vecDialogItems.push_back(std::make_unique(std::string_view {}, static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); vecDialogItems.push_back(std::make_unique(_("Unbind key"), static_cast(SpecialMenuEntry::UnbindKey), UiFlags::ColorUiGold)); UpdateDescription(*selectedOption); } break; @@ -484,10 +484,10 @@ void UiSettingsMenu() assert(IndexKeyOrPadInput == vecDialogItems.size() - 1); itemToSelect = IndexKeyOrPadInput; - vecDialogItems.push_back(std::make_unique(padEntryTimerText, static_cast(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled)); + vecDialogItems.push_back(std::make_unique(std::string_view(padEntryTimerText), static_cast(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled)); assert(IndexPadTimerText == vecDialogItems.size() - 1); - vecDialogItems.push_back(std::make_unique("", static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); + vecDialogItems.push_back(std::make_unique(std::string_view {}, static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); vecDialogItems.push_back(std::make_unique(_("Unbind button combo"), static_cast(SpecialMenuEntry::UnbindPadButton), UiFlags::ColorUiGold)); padEntryStartTime = 0; @@ -523,7 +523,7 @@ void UiSettingsMenu() padEntryCombo.modifier = padEntryCombo.button; padEntryCombo.button = ctrlEvent.button; if (pOptionPad->SetValue(padEntryCombo)) - vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription().data(); + vecDialogItems[IndexKeyOrPadInput]->m_text = selectedOption->GetValueDescription(); } return true; }; @@ -531,7 +531,7 @@ void UiSettingsMenu() } break; } - vecDialogItems.push_back(std::make_unique("", static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); + vecDialogItems.push_back(std::make_unique(std::string_view {}, static_cast(SpecialMenuEntry::None), UiFlags::ElementDisabled)); vecDialogItems.push_back(std::make_unique(_("Previous Menu"), static_cast(SpecialMenuEntry::PreviousMenu), UiFlags::ColorUiGold)); constexpr int ListItemHeight = 26; diff --git a/Source/DiabloUI/ui_item.h b/Source/DiabloUI/ui_item.h index 9b6a23de0f3..f5631523a72 100644 --- a/Source/DiabloUI/ui_item.h +++ b/Source/DiabloUI/ui_item.h @@ -10,6 +10,7 @@ #include "engine/clx_sprite.hpp" #include "engine/render/text_render.hpp" #include "utils/enum_traits.h" +#include "utils/string_or_view.hpp" #include "utils/stubs.h" namespace devilution { @@ -350,15 +351,15 @@ class UiButton : public UiItemBase { class UiListItem { public: - UiListItem(std::string_view text = "", int value = 0, UiFlags uiFlags = UiFlags::None) - : m_text(text) + UiListItem(StringOrView &&text = {}, int value = 0, UiFlags uiFlags = UiFlags::None) + : m_text(std::move(text)) , m_value(value) , uiFlags(uiFlags) { } - UiListItem(std::string_view text, std::vector &args, int value = 0, UiFlags uiFlags = UiFlags::None) - : m_text(text) + UiListItem(StringOrView &&text, std::vector &args, int value = 0, UiFlags uiFlags = UiFlags::None) + : m_text(std::move(text)) , args(args) , m_value(value) , uiFlags(uiFlags) @@ -366,7 +367,7 @@ class UiListItem { } // private: - std::string_view m_text; + StringOrView m_text; std::vector args; int m_value; UiFlags uiFlags; diff --git a/Source/options.cpp b/Source/options.cpp index 01571e99377..fb2c1ab133e 100644 --- a/Source/options.cpp +++ b/Source/options.cpp @@ -723,24 +723,8 @@ size_t OptionEntryAudioDevice::GetListSize() const std::string_view OptionEntryAudioDevice::GetListDescription(size_t index) const { - // TODO: Fix the following problems with this function: - // 1. The `string_view` result of `GetDeviceName` is used in the UI but per SDL documentation: - // > If you need to keep the string for any length of time, you should make your own copy of it, - // > as it will be invalid next time any of several other SDL functions are called. - // - // 2. `GetLineWidth` introduces a circular dependency on `text_render` which we'd like to avoid. - // The clipping should be done in the UI instead (settingsmenu.cpp). - constexpr int MaxWidth = 500; - std::string_view deviceName = GetDeviceName(index); - if (deviceName.empty()) - return "System Default"; - - while (GetLineWidth(deviceName, GameFont24, 1) > MaxWidth) { - size_t lastSymbolIndex = FindLastUtf8Symbols(deviceName); - deviceName = std::string_view(deviceName.data(), lastSymbolIndex); - } - + if (deviceName.empty()) deviceName = "System Default"; return deviceName; }