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

@@ -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<int>(i - listOffset));
+		const SDL_Rect rect = uiList.itemRect(static_cast<int>(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() });
 		}
 	}
 }

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<UiListItem>(selhero_heros[i].name, static_cast<int>(i)));
+		vecSelHeroDlgItems.push_back(std::make_unique<UiListItem>(std::string_view(selhero_heros[i].name), static_cast<int>(i)));
 		if (selhero_heros[i].saveNumber == selhero_heroInfo.saveNumber)
 			selectedItem = i;
 	}
-	vecSelHeroDlgItems.push_back(std::make_unique<UiListItem>(_("New Hero").data(), static_cast<int>(selhero_SaveCount)));
+	vecSelHeroDlgItems.push_back(std::make_unique<UiListItem>(_("New Hero"), static_cast<int>(selhero_SaveCount)));
 
 	vecSelDlgItems.push_back(std::make_unique<UiList>(vecSelHeroDlgItems, 6, uiPosition.x + 265, (uiPosition.y + 256), 320, 26, UiFlags::AlignCenter | UiFlags::FontSize24 | UiFlags::ColorUiGold));
 

Source/DiabloUI/multi/selconn.cpp

@@ -1,3 +1,5 @@
+#include <string_view>
+
 #include <fmt/format.h>
 
 #include "DiabloUI/diabloui.h"
@@ -39,7 +41,7 @@ void SelconnLoad()
 
 #ifndef NONET
 #ifndef DISABLE_ZERO_TIER
-	vecConnItems.push_back(std::make_unique<UiListItem>(ConnectionNames[SELCONN_ZT], SELCONN_ZT));
+	vecConnItems.push_back(std::make_unique<UiListItem>(std::string_view(ConnectionNames[SELCONN_ZT]), SELCONN_ZT));
 #endif
 #ifndef DISABLE_TCP
 	vecConnItems.push_back(std::make_unique<UiListItem>(_(ConnectionNames[SELCONN_TCP]), SELCONN_TCP));

Source/DiabloUI/multi/selgame.cpp

@@ -151,7 +151,7 @@ void UiInitGameSelectionList(std::string_view search)
 	vecSelGameDlgItems.push_back(std::make_unique<UiListItem>(_("Join Game"), 2, UiFlags::ColorUiGold));
 
 	if (provider == SELCONN_ZT) {
-		vecSelGameDlgItems.push_back(std::make_unique<UiListItem>("", -1, UiFlags::ElementDisabled));
+		vecSelGameDlgItems.push_back(std::make_unique<UiListItem>(std::string_view {}, -1, UiFlags::ElementDisabled));
 		vecSelGameDlgItems.push_back(std::make_unique<UiListItem>(_("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<UiListItem>(_("None"), -1, UiFlags::ElementDisabled | UiFlags::ColorUiSilver));
 		} else {
 			for (unsigned i = 0; i < Gamelist.size(); i++) {
-				vecSelGameDlgItems.push_back(std::make_unique<UiListItem>(Gamelist[i].name, i + 3, UiFlags::ColorUiGold));
+				vecSelGameDlgItems.push_back(std::make_unique<UiListItem>(std::string_view(Gamelist[i].name), i + 3, UiFlags::ColorUiGold));
 			}
 		}
 	}

Source/DiabloUI/settingsmenu.cpp

@@ -76,8 +76,8 @@ bool IsValidEntry(OptionEntryBase *pOptionEntry)
 std::vector<DrawStringFormatArg> CreateDrawStringFormatArgForEntry(OptionEntryBase *pEntry)
 {
 	return std::vector<DrawStringFormatArg> {
-		{ 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<KeymapperOptions::Action *>(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<PadmapperOptions::Action *>(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<int>(vecOptions.size());
 				if (NeedsTwoLinesToDisplayOption(formatArgs)) {
-					vecDialogItems.push_back(std::make_unique<UiListItem>("{}:", formatArgs, optionId, UiFlags::ColorUiGold | UiFlags::NeedsNextElement));
-					vecDialogItems.push_back(std::make_unique<UiListItem>(pEntry->GetValueDescription(), optionId, UiFlags::ColorUiSilver | UiFlags::ElementDisabled));
+					vecDialogItems.push_back(std::make_unique<UiListItem>(std::string_view("{}:"), formatArgs, optionId, UiFlags::ColorUiGold | UiFlags::NeedsNextElement));
+					vecDialogItems.push_back(std::make_unique<UiListItem>(std::string(pEntry->GetValueDescription()), optionId, UiFlags::ColorUiSilver | UiFlags::ElementDisabled));
 				} else {
-					vecDialogItems.push_back(std::make_unique<UiListItem>("{}: {}", formatArgs, optionId, UiFlags::ColorUiGold));
+					vecDialogItems.push_back(std::make_unique<UiListItem>(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<UiListItem>(_("Bound key:"), static_cast<int>(SpecialMenuEntry::None), UiFlags::ColorWhitegold | UiFlags::ElementDisabled));
-			vecDialogItems.push_back(std::make_unique<UiListItem>(selectedOption->GetValueDescription(), static_cast<int>(SpecialMenuEntry::None), UiFlags::ColorUiGold));
+			vecDialogItems.push_back(std::make_unique<UiListItem>(std::string(selectedOption->GetValueDescription()), static_cast<int>(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<KeymapperOptions::Action *>(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<UiListItem>(_("Press any key to change."), static_cast<int>(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled));
-			vecDialogItems.push_back(std::make_unique<UiListItem>("", static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
+			vecDialogItems.push_back(std::make_unique<UiListItem>(std::string_view {}, static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
 			vecDialogItems.push_back(std::make_unique<UiListItem>(_("Unbind key"), static_cast<int>(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<UiListItem>(padEntryTimerText, static_cast<int>(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled));
+			vecDialogItems.push_back(std::make_unique<UiListItem>(std::string_view(padEntryTimerText), static_cast<int>(SpecialMenuEntry::None), UiFlags::ColorUiSilver | UiFlags::ElementDisabled));
 			assert(IndexPadTimerText == vecDialogItems.size() - 1);
 
-			vecDialogItems.push_back(std::make_unique<UiListItem>("", static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
+			vecDialogItems.push_back(std::make_unique<UiListItem>(std::string_view {}, static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
 			vecDialogItems.push_back(std::make_unique<UiListItem>(_("Unbind button combo"), static_cast<int>(SpecialMenuEntry::UnbindPadButton), UiFlags::ColorUiGold));
 
 			padEntryStartTime = 0;
@@ -523,15 +523,15 @@ 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;
 			};
 			UpdateDescription(*selectedOption);
 		} break;
 		}
 
-		vecDialogItems.push_back(std::make_unique<UiListItem>("", static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
+		vecDialogItems.push_back(std::make_unique<UiListItem>(std::string_view {}, static_cast<int>(SpecialMenuEntry::None), UiFlags::ElementDisabled));
 		vecDialogItems.push_back(std::make_unique<UiListItem>(_("Previous Menu"), static_cast<int>(SpecialMenuEntry::PreviousMenu), UiFlags::ColorUiGold));
 
 		constexpr int ListItemHeight = 26;

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,23 +351,23 @@ 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<DrawStringFormatArg> &args, int value = 0, UiFlags uiFlags = UiFlags::None)
-	    : m_text(text)
+	UiListItem(StringOrView &&text, std::vector<DrawStringFormatArg> &args, int value = 0, UiFlags uiFlags = UiFlags::None)
+	    : m_text(std::move(text))
 	    , args(args)
 	    , m_value(value)
 	    , uiFlags(uiFlags)
 	{
 	}
 
 	// private:
-	std::string_view m_text;
+	StringOrView m_text;
 	std::vector<DrawStringFormatArg> args;
 	int m_value;
 	UiFlags uiFlags;

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;
 }