Move option change handlers out of `options.cpp`

Fixes #7638

Source/CMakeLists.txt

@@ -39,7 +39,6 @@ set(libdevilutionx_SRCS
 
   controls/axis_direction.cpp
   controls/controller.cpp
-  controls/controller_buttons.cpp
   controls/controller_motion.cpp
   controls/devices/joystick.cpp
   controls/devices/kbcontroller.cpp
@@ -213,6 +212,13 @@ target_link_dependencies(libdevilutionx_codec PRIVATE
   libdevilutionx_log
 )
 
+add_devilutionx_object_library(libdevilutionx_controller_buttons
+  controls/controller_buttons.cpp
+)
+target_link_dependencies(libdevilutionx_controller_buttons
+  DevilutionX::SDL
+)
+
 add_devilutionx_object_library(libdevilutionx_crawl
   crawl.cpp
 )
@@ -250,6 +256,10 @@ target_link_dependencies(libdevilutionx_format_int PUBLIC
 add_devilutionx_object_library(libdevilutionx_game_mode
   game_mode.cpp
 )
+target_link_dependencies(libdevilutionx_game_mode PRIVATE
+  tl
+  libdevilutionx_options
+)
 
 add_devilutionx_object_library(libdevilutionx_gendung
   levels/crypt.cpp
@@ -406,12 +416,9 @@ add_devilutionx_object_library(libdevilutionx_options
 )
 target_link_dependencies(libdevilutionx_options PUBLIC
   DevilutionX::SDL
-  SDL_audiolib::SDL_audiolib
   fmt::fmt
   tl
-  ${LUA_LIBRARIES}
-  sol2::sol2
-  libdevilutionx_game_mode
+  libdevilutionx_controller_buttons
   libdevilutionx_logged_fstream
   libdevilutionx_quick_messages
   libdevilutionx_strings
@@ -637,6 +644,7 @@ target_link_dependencies(libdevilutionx PUBLIC
   libdevilutionx_clx_render
   libdevilutionx_codec
   libdevilutionx_config
+  libdevilutionx_controller_buttons
   libdevilutionx_crawl
   libdevilutionx_direction
   libdevilutionx_surface

Source/controls/controller_buttons.cpp

@@ -1,6 +1,6 @@
 #include "controller_buttons.h"
 
-#include "plrctrls.h"
+#include "controls/game_controls.h"
 
 namespace devilution {
 namespace {
@@ -280,17 +280,21 @@ std::string_view ToXboxIcon(ControllerButton button)
 
 } // namespace
 
+// Defined in `plrctrls.cpp`.
+// Declared here to avoid having to depend on it in tests.
+extern GamepadLayout GamepadType;
+
 std::string_view ToString(ControllerButton button)
 {
 	switch (GamepadType) {
-	case devilution::GamepadLayout::PlayStation:
+	case GamepadLayout::PlayStation:
 		return ToPlayStationIcon(button);
-	case devilution::GamepadLayout::Nintendo:
+	case GamepadLayout::Nintendo:
 		return ToNintendoIcon(button);
-	case devilution::GamepadLayout::Xbox:
+	case GamepadLayout::Xbox:
 		return ToXboxIcon(button);
 	default:
-	case devilution::GamepadLayout::Generic:
+	case GamepadLayout::Generic:
 		return ToGenericButtonText(button);
 	}
 }

Source/discord/discord.cpp

@@ -28,6 +28,14 @@
 #include "utils/str_cat.hpp"
 
 namespace devilution {
+namespace {
+void IsHellfireChanged()
+{
+	discord_manager::UpdateMenu(true);
+}
+const auto IsHellfireChangedHandler = (AddIsHellfireChangeHandler(IsHellfireChanged), true);
+} // namespace
+
 namespace discord_manager {
 
 // App ID used for DevilutionX's Diablo (classic Diablo's is 496571953147150354)

Source/engine/render/scrollrt.cpp

@@ -1386,6 +1386,15 @@ void DrawMain(int dwHgt, bool drawDesc, bool drawHp, bool drawMana, bool drawSba
 	}
 }
 
+void OptionShowFPSChanged()
+{
+	if (*GetOptions().Graphics.showFPS)
+		EnableFrameCount();
+	else
+		frameflag = false;
+}
+const auto OptionChangeHandlerShowFPS = (GetOptions().Graphics.showFPS.SetValueChangedCallback(OptionShowFPSChanged), true);
+
 } // namespace
 
 Displacement GetOffsetForWalking(const AnimationInfo &animationInfo, const Direction dir, bool cameraMode /*= false*/)

Source/engine/render/text_render.cpp

@@ -26,6 +26,7 @@
 #include "engine/render/clx_render.hpp"
 #include "engine/render/primitive_render.hpp"
 #include "engine/ticks.hpp"
+#include "options.h"
 #include "utils/algorithm/container.hpp"
 #include "utils/display.h"
 #include "utils/is_of.hpp"
@@ -475,6 +476,15 @@ uint32_t DoDrawString(const Surface &out, std::string_view text, Rectangle rect,
 	return static_cast<uint32_t>(remaining.data() - text.data());
 }
 
+void OptionLanguageCodeChanged()
+{
+	UnloadFonts();
+	LanguageInitialize();
+	LoadLanguageArchive();
+}
+
+const auto OptionChangeHandlerResolution = (GetOptions().Language.code.SetValueChangedCallback(OptionLanguageCodeChanged), true);
+
 } // namespace
 
 void LoadSmallSelectionSpinner()

Source/engine/sound.cpp

@@ -150,6 +150,26 @@ int CapVolume(int volume)
 	return std::clamp(volume, VOLUME_MIN, VOLUME_MAX);
 }
 
+void OptionAudioChanged()
+{
+	effects_cleanup_sfx();
+	music_stop();
+	snd_deinit();
+	snd_init();
+	music_start(TMUSIC_INTRO);
+	if (gbRunGame)
+		sound_init();
+	else
+		ui_sound_init();
+}
+
+const auto OptionChangeSampleRate = (GetOptions().Audio.sampleRate.SetValueChangedCallback(OptionAudioChanged), true);
+const auto OptionChangeChannels = (GetOptions().Audio.channels.SetValueChangedCallback(OptionAudioChanged), true);
+const auto OptionChangeBufferSize = (GetOptions().Audio.bufferSize.SetValueChangedCallback(OptionAudioChanged), true);
+const auto OptionChangeResamplingQuality = (GetOptions().Audio.resamplingQuality.SetValueChangedCallback(OptionAudioChanged), true);
+const auto OptionChangeResampler = (GetOptions().Audio.resampler.SetValueChangedCallback(OptionAudioChanged), true);
+const auto OptionChangeDevice = (GetOptions().Audio.device.SetValueChangedCallback(OptionAudioChanged), true);
+
 } // namespace
 
 void ClearDuplicateSounds()

Source/game_mode.cpp

@@ -1,8 +1,37 @@
 #include "game_mode.hpp"
 
+#include <function_ref.hpp>
+
+#include "options.h"
+
 namespace devilution {
+namespace {
+std::vector<tl::function_ref<void()>> IsHellfireChangeHandlers;
+
+void OptionGameModeChanged()
+{
+	gbIsHellfire = *GetOptions().GameMode.gameMode == StartUpGameMode::Hellfire;
+	for (tl::function_ref<void()> handler : IsHellfireChangeHandlers) {
+		handler();
+	}
+}
+const auto OptionChangeHandlerGameMode = (GetOptions().GameMode.gameMode.SetValueChangedCallback(OptionGameModeChanged), true);
+
+void OptionSharewareChanged()
+{
+	gbIsSpawn = *GetOptions().GameMode.shareware;
+}
+const auto OptionChangeHandlerShareware = (GetOptions().GameMode.shareware.SetValueChangedCallback(OptionSharewareChanged), true);
+} // namespace
+
 bool gbIsSpawn;
 bool gbIsHellfire;
 bool gbVanilla;
 bool forceHellfire;
+
+void AddIsHellfireChangeHandler(tl::function_ref<void()> callback)
+{
+	IsHellfireChangeHandlers.push_back(callback);
+}
+
 } // namespace devilution

Source/game_mode.hpp

@@ -1,5 +1,7 @@
 #pragma once
 
+#include <function_ref.hpp>
+
 #include "utils/attributes.h"
 
 namespace devilution {
@@ -13,4 +15,7 @@ extern DVL_API_FOR_TEST bool gbVanilla;
 /** Whether the Hellfire mode is required (forced). */
 extern bool forceHellfire;
 
+/** Adds a handler to be called then `gbIsHellfire` changes after the initial startup. */
+void AddIsHellfireChangeHandler(tl::function_ref<void()> callback);
+
 } // namespace devilution

Source/lua/lua.cpp

@@ -244,6 +244,10 @@ void LuaInitialize()
 	// Used by the custom require implementation.
 	lua["setEnvironment"] = [](const sol::environment &env, const sol::function &fn) { sol::set_environment(env, fn); };
 
+	for (OptionEntryBase *mod : GetOptions().Mods.GetEntries()) {
+		mod->SetValueChangedCallback(LuaReloadActiveMods);
+	}
+
 	LuaReloadActiveMods();
 }
 

Source/options.cpp

@@ -3,35 +3,35 @@
  *
  * Load and save options from the diablo.ini file.
  */
+#include "options.h"
 
 #include <algorithm>
 #include <cerrno>
 #include <cstdint>
 #include <cstdio>
+#include <cstring>
+#include <functional>
+#include <iterator>
 #include <optional>
 #include <span>
 
+#include <SDL_version.h>
 #include <expected.hpp>
 #include <fmt/format.h>
+#include <function_ref.hpp>
 
+#include "appfat.h"
 #include "control.h"
 #include "controls/controller.h"
+#include "controls/controller_buttons.h"
 #include "controls/game_controls.h"
 #include "controls/plrctrls.h"
-#include "discord/discord.h"
 #include "engine/assets.hpp"
 #include "engine/demomode.h"
 #include "engine/sound_defs.hpp"
-#include "game_mode.hpp"
-#include "hwcursor.hpp"
-#include "lua/lua.hpp"
-#include "options.h"
 #include "platform/locale.hpp"
-#include "qol/monhealthbar.h"
-#include "qol/xpbar.h"
 #include "quick_messages.hpp"
 #include "utils/algorithm/container.hpp"
-#include "utils/display.h"
 #include "utils/file_util.h"
 #include "utils/ini.hpp"
 #include "utils/is_of.hpp"
@@ -142,85 +142,6 @@ bool HardwareCursorDefault()
 }
 #endif
 
-void OptionGrabInputChanged()
-{
-#ifdef USE_SDL1
-	SDL_WM_GrabInput(*GetOptions().Gameplay.grabInput ? SDL_GRAB_ON : SDL_GRAB_OFF);
-#else
-	if (ghMainWnd != nullptr)
-		SDL_SetWindowGrab(ghMainWnd, *GetOptions().Gameplay.grabInput ? SDL_TRUE : SDL_FALSE);
-#endif
-}
-
-void OptionExperienceBarChanged()
-{
-	if (!gbRunGame)
-		return;
-	if (*GetOptions().Gameplay.experienceBar)
-		InitXPBar();
-	else
-		FreeXPBar();
-}
-
-void OptionEnemyHealthBarChanged()
-{
-	if (!gbRunGame)
-		return;
-	if (*GetOptions().Gameplay.enemyHealthBar)
-		InitMonsterHealthBar();
-	else
-		FreeMonsterHealthBar();
-}
-
-#if !defined(USE_SDL1) || defined(__3DS__)
-void ResizeWindowAndUpdateResolutionOptions()
-{
-	ResizeWindow();
-#ifndef __3DS__
-	GetOptions().Graphics.resolution.InvalidateList();
-#endif
-}
-#endif
-
-void OptionShowFPSChanged()
-{
-	if (*GetOptions().Graphics.showFPS)
-		EnableFrameCount();
-	else
-		frameflag = false;
-}
-
-void OptionLanguageCodeChanged()
-{
-	UnloadFonts();
-	LanguageInitialize();
-	LoadLanguageArchive();
-}
-
-void OptionGameModeChanged()
-{
-	gbIsHellfire = *GetOptions().GameMode.gameMode == StartUpGameMode::Hellfire;
-	discord_manager::UpdateMenu(true);
-}
-
-void OptionSharewareChanged()
-{
-	gbIsSpawn = *GetOptions().GameMode.shareware;
-}
-
-void OptionAudioChanged()
-{
-	effects_cleanup_sfx();
-	music_stop();
-	snd_deinit();
-	snd_init();
-	music_start(TMUSIC_INTRO);
-	if (gbRunGame)
-		sound_init();
-	else
-		ui_sound_init();
-}
-
 } // namespace
 
 Options &GetOptions()
@@ -320,14 +241,13 @@ OptionEntryFlags OptionEntryBase::GetFlags() const
 {
 	return flags;
 }
-void OptionEntryBase::SetValueChangedCallback(std::function<void()> callback)
+void OptionEntryBase::SetValueChangedCallback(tl::function_ref<void()> callback)
 {
-	this->callback = std::move(callback);
+	callback_ = callback;
 }
 void OptionEntryBase::NotifyValueChanged()
 {
-	if (callback)
-		callback();
+	if (callback_.has_value()) (*callback_)();
 }
 
 void OptionEntryBoolean::LoadFromIni(std::string_view category)
@@ -471,8 +391,6 @@ GameModeOptions::GameModeOptions()
     , shareware("Shareware", OptionEntryFlags::NeedDiabloMpq | OptionEntryFlags::RecreateUI, N_("Restrict to Shareware"), N_("Makes the game compatible with the demo. Enables multiplayer with friends who don't own a full copy of Diablo."), false)
 
 {
-	gameMode.SetValueChangedCallback(OptionGameModeChanged);
-	shareware.SetValueChangedCallback(OptionSharewareChanged);
 }
 std::vector<OptionEntryBase *> GameModeOptions::GetEntries()
 {
@@ -553,12 +471,6 @@ AudioOptions::AudioOptions()
     , bufferSize("Buffer Size", OptionEntryFlags::CantChangeInGame, N_("Buffer Size"), N_("Buffer size (number of frames per channel)."), DEFAULT_AUDIO_BUFFER_SIZE, { 1024, 2048, 5120 })
     , resamplingQuality("Resampling Quality", OptionEntryFlags::CantChangeInGame, N_("Resampling Quality"), N_("Quality of the resampler, from 0 (lowest) to 10 (highest)."), DEFAULT_AUDIO_RESAMPLING_QUALITY, { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 })
 {
-	sampleRate.SetValueChangedCallback(OptionAudioChanged);
-	channels.SetValueChangedCallback(OptionAudioChanged);
-	bufferSize.SetValueChangedCallback(OptionAudioChanged);
-	resamplingQuality.SetValueChangedCallback(OptionAudioChanged);
-	resampler.SetValueChangedCallback(OptionAudioChanged);
-	device.SetValueChangedCallback(OptionAudioChanged);
 }
 std::vector<OptionEntryBase *> AudioOptions::GetEntries()
 {
@@ -811,6 +723,13 @@ 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);
@@ -904,17 +823,6 @@ GraphicsOptions::GraphicsOptions()
 #endif
     , showFPS("Show FPS", OptionEntryFlags::None, N_("Show FPS"), N_("Displays the FPS in the upper left corner of the screen."), false)
 {
-	resolution.SetValueChangedCallback(ResizeWindow);
-	fullscreen.SetValueChangedCallback(SetFullscreenMode);
-#if !defined(USE_SDL1) || defined(__3DS__)
-	fitToScreen.SetValueChangedCallback(ResizeWindowAndUpdateResolutionOptions);
-#endif
-#ifndef USE_SDL1
-	scaleQuality.SetValueChangedCallback(ReinitializeTexture);
-	integerScaling.SetValueChangedCallback(ReinitializeIntegerScale);
-	frameRateControl.SetValueChangedCallback(ReinitializeRenderer);
-#endif
-	showFPS.SetValueChangedCallback(OptionShowFPSChanged);
 }
 std::vector<OptionEntryBase *> GraphicsOptions::GetEntries()
 {
@@ -994,10 +902,8 @@ GameplayOptions::GameplayOptions()
           })
     , skipLoadingScreenThresholdMs("Skip loading screen threshold, ms", OptionEntryFlags::Invisible, "", "", 0)
 {
-	grabInput.SetValueChangedCallback(OptionGrabInputChanged);
-	experienceBar.SetValueChangedCallback(OptionExperienceBarChanged);
-	enemyHealthBar.SetValueChangedCallback(OptionEnemyHealthBarChanged);
 }
+
 std::vector<OptionEntryBase *> GameplayOptions::GetEntries()
 {
 	return {
@@ -1194,7 +1100,6 @@ void OptionEntryLanguageCode::SetActiveListIndex(size_t index)
 LanguageOptions::LanguageOptions()
     : OptionCategoryBase("Language", N_("Language"), N_("Language Settings"))
 {
-	code.SetValueChangedCallback(OptionLanguageCodeChanged);
 }
 std::vector<OptionEntryBase *> LanguageOptions::GetEntries()
 {
@@ -1860,7 +1765,6 @@ ModOptions::ModEntry::ModEntry(std::string_view name)
     : name(name)
     , enabled(this->name, OptionEntryFlags::None, this->name.c_str(), "", false)
 {
-	enabled.SetValueChangedCallback(LuaReloadActiveMods);
 }
 
 namespace {

Source/options.h

@@ -9,6 +9,7 @@
 
 #include <SDL_version.h>
 #include <ankerl/unordered_dense.h>
+#include <function_ref.hpp>
 
 #include "controls/controller.h"
 #include "controls/controller_buttons.h"
@@ -122,7 +123,7 @@ public:
 	[[nodiscard]] virtual OptionEntryType GetType() const = 0;
 	[[nodiscard]] OptionEntryFlags GetFlags() const;
 
-	void SetValueChangedCallback(std::function<void()> callback);
+	void SetValueChangedCallback(tl::function_ref<void()> callback);
 
 	[[nodiscard]] virtual std::string_view GetValueDescription() const = 0;
 	virtual void LoadFromIni(std::string_view category) = 0;
@@ -137,7 +138,7 @@ protected:
 	void NotifyValueChanged();
 
 private:
-	std::function<void()> callback;
+	std::optional<tl::function_ref<void()>> callback_;
 };
 
 class OptionEntryBoolean : public OptionEntryBase {

Source/qol/monhealthbar.cpp

@@ -28,6 +28,18 @@ OptionalOwnedClxSpriteList health;
 OptionalOwnedClxSpriteList healthBlue;
 OptionalOwnedClxSpriteList playerExpTags;
 
+void OptionEnemyHealthBarChanged()
+{
+	if (!gbRunGame)
+		return;
+	if (*GetOptions().Gameplay.enemyHealthBar)
+		InitMonsterHealthBar();
+	else
+		FreeMonsterHealthBar();
+}
+
+const auto OptionChangeHandler = (GetOptions().Gameplay.enemyHealthBar.SetValueChangedCallback(OptionEnemyHealthBarChanged), true);
+
 } // namespace
 
 void InitMonsterHealthBar()

Source/qol/xpbar.cpp

@@ -50,6 +50,18 @@ void DrawEndCap(const Surface &out, Point point, int idx, const ColorGradient &g
 	out.SetPixel({ point.x, point.y + 3 }, gradient[idx / 2]);
 }
 
+void OptionExperienceBarChanged()
+{
+	if (!gbRunGame)
+		return;
+	if (*GetOptions().Gameplay.experienceBar)
+		InitXPBar();
+	else
+		FreeXPBar();
+}
+
+const auto OptionChangeHandler = (GetOptions().Gameplay.experienceBar.SetValueChangedCallback(OptionExperienceBarChanged), true);
+
 } // namespace
 
 void InitXPBar()

Source/utils/display.cpp

@@ -193,6 +193,37 @@ Size GetPreferredWindowSize()
 	return windowSize;
 }
 
+const auto OptionChangeHandlerResolution = (GetOptions().Graphics.resolution.SetValueChangedCallback(ResizeWindow), true);
+const auto OptionChangeHandlerFullscreen = (GetOptions().Graphics.fullscreen.SetValueChangedCallback(SetFullscreenMode), true);
+
+void OptionGrabInputChanged()
+{
+#ifdef USE_SDL1
+	SDL_WM_GrabInput(*GetOptions().Gameplay.grabInput ? SDL_GRAB_ON : SDL_GRAB_OFF);
+#else
+	if (ghMainWnd != nullptr)
+		SDL_SetWindowGrab(ghMainWnd, *GetOptions().Gameplay.grabInput ? SDL_TRUE : SDL_FALSE);
+#endif
+}
+const auto OptionChangeHandlerGrabInput = (GetOptions().Gameplay.grabInput.SetValueChangedCallback(OptionGrabInputChanged), true);
+
+#if !defined(USE_SDL1) || defined(__3DS__)
+void ResizeWindowAndUpdateResolutionOptions()
+{
+	ResizeWindow();
+#ifndef __3DS__
+	GetOptions().Graphics.resolution.InvalidateList();
+#endif
+}
+const auto OptionChangeHandlerFitToScreen = (GetOptions().Graphics.fitToScreen.SetValueChangedCallback(ResizeWindowAndUpdateResolutionOptions), true);
+#endif
+
+#ifndef USE_SDL1
+const auto OptionChangeHandlerScaleQuality = (GetOptions().Graphics.scaleQuality.SetValueChangedCallback(ReinitializeTexture), true);
+const auto OptionChangeHandlerIntegerScaling = (GetOptions().Graphics.integerScaling.SetValueChangedCallback(ReinitializeIntegerScale), true);
+const auto OptionChangeHandlerVSync = (GetOptions().Graphics.frameRateControl.SetValueChangedCallback(ReinitializeRenderer), true);
+#endif
+
 } // namespace
 
 void AdjustToScreenGeometry(Size windowSize)