Refactor the preferences into a proper singleton.

The current preferences handling is a mess: * it's essentially a global config object that anything can modify in any way the caller wants, which is managed across multiple source files which have their own oddities and interdependencies. * the general preferences has its own bit of SDL event handling and while I get the idea behind `events::sdl_handler` there's no reason to have SDL events handled in the preferences instead of just calling the relevant preferences setter for each event when it happens. * the general preferences is where most of the preferences are handled and has its `base_manager` struct, which is part of the `manager` struct in the game preferences, which is then implicitly initialized as part of game_launcher's constructor. * the editor preferences are the only preferences in a sub-namespace `preferences::editor` while all other preferences are just in the `preferences` namespace. * the display, editor, and lobby preferences are all dependent on including the game preferences, the credentials are dependent on including the general preferences (but not the game preferences), the game preferences are dependent on including the general preferences, and the advanced preferences are entirely their own thing which is dependent on none of the other preference functionality and manages its own singleton. * nothing checks whether the preferences file has actually been loaded before allowing values to be read from or written to the preferences config - if you attempt to get a value too early in wesnoth's initialization it will silently just give you whatever the default value for that preference happens to be. With this there is instead a single access point (with exceptions handled via friend functions/classes), all predefined preferences are accessed via their own setter/getter, and all mainline preferences are defined in a single file (preference_list.hpp) so it's easily findable what preferences exist and where they're used. Having the list of all mainline preferences listed out also allows the lua preferences API to provide that full list rather than just the list of the preferences that have been set so far. Also it now checks for whether the location of the preferences file is known before attempting to load the preferences file and asserts if someone attempts to use the preferences too early.
Pentarctagon · Jun 2, 2024 · 9c39232 · 9c39232
1 parent 28f93b4
commit 9c39232
Show file tree

Hide file tree

Showing 140 changed files with 5,079 additions and 5,047 deletions.
diff --git a/.gitignore b/.gitignore
@@ -221,6 +221,7 @@ config.h
 callgrind.out.*
 data/dist
 clean.sh
+widgets_tested.log
 
 # clangd cache
 .cache/clangd
diff --git a/projectfiles/CodeBlocks-SCons/wesnoth.cbp b/projectfiles/CodeBlocks-SCons/wesnoth.cbp
@@ -889,20 +889,9 @@
 		<Unit filename="../../src/playsingle_controller.hpp" />
 		<Unit filename="../../src/playturn_network_adapter.cpp" />
 		<Unit filename="../../src/playturn_network_adapter.hpp" />
-		<Unit filename="../../src/preferences/advanced.cpp" />
-		<Unit filename="../../src/preferences/advanced.hpp" />
-		<Unit filename="../../src/preferences/credentials.cpp" />
-		<Unit filename="../../src/preferences/credentials.hpp" />
-		<Unit filename="../../src/preferences/display.cpp" />
-		<Unit filename="../../src/preferences/display.hpp" />
-		<Unit filename="../../src/preferences/editor.cpp" />
-		<Unit filename="../../src/preferences/editor.hpp" />
-		<Unit filename="../../src/preferences/game.cpp" />
-		<Unit filename="../../src/preferences/game.hpp" />
-		<Unit filename="../../src/preferences/general.cpp" />
-		<Unit filename="../../src/preferences/general.hpp" />
-		<Unit filename="../../src/preferences/lobby.cpp" />
-		<Unit filename="../../src/preferences/lobby.hpp" />
+		<Unit filename="../../src/preferences/preferences.cpp" />
+		<Unit filename="../../src/preferences/preferences.hpp" />
+		<Unit filename="../../src/preferences/preferences_list.hpp" />
 		<Unit filename="../../src/quit_confirmation.cpp" />
 		<Unit filename="../../src/quit_confirmation.hpp" />
 		<Unit filename="../../src/random.cpp" />

diff --git a/projectfiles/CodeBlocks/tests.cbp b/projectfiles/CodeBlocks/tests.cbp
@@ -971,20 +971,9 @@
 		<Unit filename="../../src/playsingle_controller.hpp" />
 		<Unit filename="../../src/playturn_network_adapter.cpp" />
 		<Unit filename="../../src/playturn_network_adapter.hpp" />
-		<Unit filename="../../src/preferences/advanced.cpp" />
-		<Unit filename="../../src/preferences/advanced.hpp" />
-		<Unit filename="../../src/preferences/credentials.cpp" />
-		<Unit filename="../../src/preferences/credentials.hpp" />
-		<Unit filename="../../src/preferences/display.cpp" />
-		<Unit filename="../../src/preferences/display.hpp" />
-		<Unit filename="../../src/preferences/editor.cpp" />
-		<Unit filename="../../src/preferences/editor.hpp" />
-		<Unit filename="../../src/preferences/game.cpp" />
-		<Unit filename="../../src/preferences/game.hpp" />
-		<Unit filename="../../src/preferences/general.cpp" />
-		<Unit filename="../../src/preferences/general.hpp" />
-		<Unit filename="../../src/preferences/lobby.cpp" />
-		<Unit filename="../../src/preferences/lobby.hpp" />
+		<Unit filename="../../src/preferences/preferences.cpp" />
+		<Unit filename="../../src/preferences/preferences.hpp" />
+		<Unit filename="../../src/preferences/preferences_list.hpp" />
 		<Unit filename="../../src/quit_confirmation.cpp" />
 		<Unit filename="../../src/quit_confirmation.hpp" />
 		<Unit filename="../../src/random.cpp" />

diff --git a/projectfiles/CodeBlocks/wesnoth.cbp b/projectfiles/CodeBlocks/wesnoth.cbp
@@ -1003,20 +1003,9 @@
 		<Unit filename="../../src/playsingle_controller.hpp" />
 		<Unit filename="../../src/playturn_network_adapter.cpp" />
 		<Unit filename="../../src/playturn_network_adapter.hpp" />
-		<Unit filename="../../src/preferences/advanced.cpp" />
-		<Unit filename="../../src/preferences/advanced.hpp" />
-		<Unit filename="../../src/preferences/credentials.cpp" />
-		<Unit filename="../../src/preferences/credentials.hpp" />
-		<Unit filename="../../src/preferences/display.cpp" />
-		<Unit filename="../../src/preferences/display.hpp" />
-		<Unit filename="../../src/preferences/editor.cpp" />
-		<Unit filename="../../src/preferences/editor.hpp" />
-		<Unit filename="../../src/preferences/game.cpp" />
-		<Unit filename="../../src/preferences/game.hpp" />
-		<Unit filename="../../src/preferences/general.cpp" />
-		<Unit filename="../../src/preferences/general.hpp" />
-		<Unit filename="../../src/preferences/lobby.cpp" />
-		<Unit filename="../../src/preferences/lobby.hpp" />
+		<Unit filename="../../src/preferences/preferences.cpp" />
+		<Unit filename="../../src/preferences/preferences.hpp" />
+		<Unit filename="../../src/preferences/preferences_list.hpp" />
 		<Unit filename="../../src/quit_confirmation.cpp" />
 		<Unit filename="../../src/quit_confirmation.hpp" />
 		<Unit filename="../../src/random.cpp" />

diff --git a/projectfiles/Xcode/The Battle for Wesnoth.xcodeproj/project.pbxproj b/projectfiles/Xcode/The Battle for Wesnoth.xcodeproj/project.pbxproj
diff --git a/source_lists/libwesnoth b/source_lists/libwesnoth
@@ -30,8 +30,7 @@ hotkey/hotkey_item.cpp
 hotkey/hotkey_manager.cpp
 picture.cpp
 image_modifications.cpp
-preferences/credentials.cpp
-preferences/general.cpp
+preferences/preferences.cpp
 key.cpp
 language.cpp
 map/label.cpp

diff --git a/source_lists/wesnoth b/source_lists/wesnoth
@@ -312,11 +312,7 @@ play_controller.cpp
 playmp_controller.cpp
 playsingle_controller.cpp
 playturn_network_adapter.cpp
-preferences/advanced.cpp
-preferences/display.cpp
-preferences/editor.cpp
-preferences/game.cpp
-preferences/lobby.cpp
+preferences/preferences.cpp
 random_deterministic.cpp
 random_synced.cpp
 recall_list_manager.cpp

diff --git a/src/achievements.cpp b/src/achievements.cpp
@@ -17,7 +17,7 @@
 
 #include "filesystem.hpp"
 #include "log.hpp"
-#include "preferences/general.hpp"
+#include "preferences/preferences.hpp"
 #include "serialization/parser.hpp"
 #include "serialization/preprocessor.hpp"
 
@@ -65,7 +65,7 @@ achievement::achievement(const config& cfg, const std::string& content_for, bool
 		if(sub_id.empty()) {
 			ERR_CONFIG << "Achievement " << id_ << " has a sub-achievement missing the id attribute:\n" << sub_ach.debug();
 		} else {
-			sub_achievements_.emplace_back(sub_ach, achieved_ || preferences::sub_achievement(content_for, id_, sub_id));
+			sub_achievements_.emplace_back(sub_ach, achieved_ || prefs::get().sub_achievement(content_for, id_, sub_id));
 			max_progress_++;
 		}
 	}
@@ -85,7 +85,7 @@ achievement_group::achievement_group(const config& cfg)
 			ERR_CONFIG << content_for_ + " achievement id " << id << " contains a comma, skipping.";
 			continue;
 		} else {
-			achievements_.emplace_back(ach, content_for_, preferences::achievement(content_for_, id), preferences::progress_achievement(content_for_, id));
+			achievements_.emplace_back(ach, content_for_, prefs::get().achievement(content_for_, id), prefs::get().progress_achievement(content_for_, id));
 		}
 	}
 }

diff --git a/src/actions/advancement.cpp b/src/actions/advancement.cpp
@@ -24,7 +24,7 @@
 #include "ai/manager.hpp"  // for manager, holder
 #include "ai/lua/aspect_advancements.hpp"
 #include "game_events/pump.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "game_data.hpp" //resources::gamedata->phase()
 #include "gettext.hpp"
 #include "gui/dialogs/unit_advance.hpp"
@@ -67,7 +67,7 @@ namespace
 		std::vector<unit_const_ptr> previews;
 
 		for (const std::string& advance : u.advances_to()) {
-			preferences::encountered_units().insert(advance);
+			prefs::get().encountered_units().insert(advance);
 			previews.push_back(get_advanced_unit(u, advance));
 		}
 
@@ -386,7 +386,7 @@ void advance_unit(map_location loc, const advancement_option &advance_to, bool f
 	if ( !use_amla )
 	{
 		resources::controller->statistics().advance_unit(*new_unit);
-		preferences::encountered_units().insert(new_unit->type_id());
+		prefs::get().encountered_units().insert(new_unit->type_id());
 		LOG_CF << "Added '" << new_unit->type_id() << "' to the encountered units.";
 	}
 	u->anim_comp().clear_haloes();

diff --git a/src/actions/attack.cpp b/src/actions/attack.cpp
@@ -32,7 +32,7 @@
 #include "map/map.hpp"
 #include "mouse_handler_base.hpp"
 #include "play_controller.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "random.hpp"
 #include "replay.hpp"
 #include "resources.hpp"
@@ -1311,7 +1311,7 @@ void attack::unit_killed(unit_info& attacker,
 			game_events::entity_location reanim_loc(defender.loc_, newunit->underlying_id());
 			resources::game_events->pump().fire("unit_placed", reanim_loc);
 
-			preferences::encountered_units().insert(newunit->type_id());
+			prefs::get().encountered_units().insert(newunit->type_id());
 
 			if(update_display_) {
 				display::get_singleton()->invalidate(death_loc);

diff --git a/src/actions/create.cpp b/src/actions/create.cpp
@@ -29,7 +29,7 @@
 #include "filter_context.hpp"
 #include "game_events/pump.hpp"
 #include "game_state.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "gettext.hpp"
 #include "log.hpp"
 #include "map/map.hpp"
@@ -671,7 +671,7 @@ place_recruit_result place_recruit(unit_ptr u, const map_location &recruit_locat
 			return std::tuple(true, 0, false);
 		new_unit_itor->set_hidden(true);
 	}
-	preferences::encountered_units().insert(new_unit_itor->type_id());
+	prefs::get().encountered_units().insert(new_unit_itor->type_id());
 	current_team.spend_gold(cost);
 
 	if ( show ) {

diff --git a/src/actions/move.cpp b/src/actions/move.cpp
@@ -24,7 +24,7 @@
 #include "actions/vision.hpp"
 
 #include "game_events/pump.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "gettext.hpp"
 #include "hotkey/hotkey_item.hpp"
 #include "hotkey/hotkey_command.hpp"
@@ -1363,7 +1363,7 @@ std::size_t move_unit_and_record(const std::vector<map_location> &steps,
 			resources::gameboard->units().find(steps.front())->side() - 1];
 		continued_move |= !current_team.fog_or_shroud();
 	}
-	const bool skip_ally_sighted = !preferences::interrupt_when_ally_sighted();
+	const bool skip_ally_sighted = !prefs::get().interrupt_when_ally_sighted();
 
 	// Evaluate this move.
 	unit_mover mover(steps, move_spectator, continued_move, skip_ally_sighted);

diff --git a/src/actions/unit_creator.cpp b/src/actions/unit_creator.cpp
@@ -26,7 +26,7 @@
 #include "display.hpp"
 #include "game_board.hpp"
 #include "game_events/pump.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "game_data.hpp" // for resources::gamedata conversion variable_set
 #include "game_version.hpp"
 #include "log.hpp"
@@ -193,7 +193,7 @@ void unit_creator::add_unit(const config &cfg, const vconfig* vcfg)
 			//add to recall list
 			team_.recall_list().add(new_unit);
 			DBG_NG << "inserting unit with id=["<<id<<"] on recall list for side " << new_unit->side();
-			preferences::encountered_units().insert(new_unit->type_id());
+			prefs::get().encountered_units().insert(new_unit->type_id());
 		}
 	} else {
 		//get unit from recall list
@@ -217,7 +217,7 @@ void unit_creator::post_create(const map_location &loc, const unit &new_unit, bo
 {
 
 	if (discover_) {
-		preferences::encountered_units().insert(new_unit.type_id());
+		prefs::get().encountered_units().insert(new_unit.type_id());
 	}
 
 	bool show = show_ && (display::get_singleton() !=nullptr) && !display::get_singleton()->fogged(loc);

diff --git a/src/addon/client.cpp b/src/addon/client.cpp
@@ -29,8 +29,7 @@
 #include "gui/dialogs/message.hpp"
 #include "gui/widgets/retval.hpp"
 #include "log.hpp"
-#include "preferences/credentials.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "serialization/string_utils.hpp"
 #include "serialization/utf8_exception.hpp"
 #include "utils/parse_network_address.hpp"
@@ -286,14 +285,14 @@ bool addons_client::delete_remote_addon(const std::string& id, std::string& resp
 
 	// if the passphrase isn't provided from the _server.pbl, try to pre-populate it from the preferences before prompting for it
 	if(cfg["passphrase"].empty()) {
-		cfg["passphrase"] = preferences::password(preferences::campaign_server(), cfg["author"]);
+		cfg["passphrase"] = prefs::get().password(prefs::get().campaign_server(), cfg["author"]);
 		if(!gui2::dialogs::addon_auth::execute(cfg)) {
 			config dummy;
 			config& error = dummy.add_child("error");
 			error["message"] = "Password not provided.";
 			return !update_last_error(dummy);
 		} else {
-			preferences::set_password(preferences::campaign_server(), cfg["author"], cfg["passphrase"]);
+			prefs::get().set_password(prefs::get().campaign_server(), cfg["author"], cfg["passphrase"]);
 		}
 	}
 

diff --git a/src/addon/manager_ui.cpp b/src/addon/manager_ui.cpp
@@ -22,7 +22,7 @@
 #include "config_cache.hpp"
 #include "filesystem.hpp"
 #include "formula/string_utils.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "gettext.hpp"
 #include "gui/dialogs/addon/manager.hpp"
 #include "gui/dialogs/addon/uninstall_list.hpp"
@@ -66,7 +66,7 @@ bool addons_manager_ui(const std::string& remote_address)
 {
 	bool need_wml_cache_refresh = false;
 
-	preferences::set_campaign_server(remote_address);
+	prefs::get().set_campaign_server(remote_address);
 
 	try {
 		addons_client client(remote_address);
@@ -231,7 +231,7 @@ bool manage_addons()
 	// NOTE: the following two values are also known by WML, so don't change them.
 	static const int addon_uninstall = 2;
 
-	std::string host_name = preferences::campaign_server();
+	std::string host_name = prefs::get().campaign_server();
 	const bool have_addons = !installed_addons().empty();
 
 	gui2::dialogs::addon_connect addon_dlg(host_name, have_addons);
@@ -254,7 +254,7 @@ bool manage_addons()
 
 bool ad_hoc_addon_fetch_session(const std::vector<std::string>& addon_ids)
 {
-	std::string remote_address = preferences::campaign_server();
+	std::string remote_address = prefs::get().campaign_server();
 
 	// These exception handlers copied from addon_manager_ui fcn above.
 	try {

diff --git a/src/ai/actions.cpp b/src/ai/actions.cpp
@@ -41,7 +41,7 @@
 #include "actions/attack.hpp"
 #include "actions/create.hpp"
 #include "attack_prediction.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 #include "log.hpp"
 #include "map/map.hpp"
 #include "mouse_handler_base.hpp"
@@ -476,7 +476,7 @@ void move_result::do_execute()
 			/*std::vector<map_location> steps*/ route_->steps,
 			/*::actions::undo_list* undo_stack*/ nullptr,
 			/*bool continue_move*/ true,
-			/*bool show_move*/ !preferences::skip_ai_moves(),
+			/*bool show_move*/ !prefs::get().skip_ai_moves(),
 			/*bool* interrupted*/ nullptr,
 			/*::actions::move_unit_spectator* move_spectator*/ &move_spectator);
 
@@ -658,7 +658,7 @@ void recall_result::do_execute()
 	synced_context::run_in_synced_context_if_not_already("recall",
 		replay_helper::get_recall(unit_id_, recall_location_, recall_from_),
 		false,
-		!preferences::skip_ai_moves(),
+		!prefs::get().skip_ai_moves(),
 		synced_context::ignore_error_function);
 
 	set_gamestate_changed();
@@ -804,7 +804,7 @@ void recruit_result::do_execute()
 	}
 
 	resources::undo_stack->clear();
-	synced_context::run_in_synced_context_if_not_already("recruit", replay_helper::get_recruit(u->id(), recruit_location_, recruit_from_), false, !preferences::skip_ai_moves());
+	synced_context::run_in_synced_context_if_not_already("recruit", replay_helper::get_recruit(u->id(), recruit_location_, recruit_from_), false, !prefs::get().skip_ai_moves());
 
 	set_gamestate_changed();
 	try {

diff --git a/src/attack_prediction.cpp b/src/attack_prediction.cpp
@@ -32,7 +32,7 @@
 
 #include "actions/attack.hpp"
 #include "game_config.hpp"
-#include "preferences/general.hpp"
+#include "preferences/preferences.hpp"
 #include "random.hpp"
 
 #include <array>
@@ -2333,7 +2333,7 @@ void combatant::fight(combatant& opponent, bool levelup_considered)
 
 	bool use_monte_carlo_simulation =
 		fight_complexity(split.size(), opp_split.size(), u_, opponent.u_) > MONTE_CARLO_SIMULATION_THRESHOLD
-		&& preferences::damage_prediction_allow_monte_carlo_simulation();
+		&& prefs::get().damage_prediction_allow_monte_carlo_simulation();
 
 	if(use_monte_carlo_simulation) {
 		// A very complex fight. Use Monte Carlo simulation instead of exact

diff --git a/src/chat_command_handler.cpp b/src/chat_command_handler.cpp
@@ -20,7 +20,7 @@
 #include "gui/dialogs/multiplayer/mp_report.hpp"
 #include "gui/dialogs/preferences_dialog.hpp"
 #include "map_command_handler.hpp"
-#include "preferences/game.hpp"
+#include "preferences/preferences.hpp"
 
 namespace events {
 
@@ -77,7 +77,7 @@ void chat_command_handler::do_ignore()
 		utils::string_map symbols;
 		symbols["nick"] = get_arg(1);
 
-		if (preferences::add_acquaintance(get_arg(1), "ignore", get_data(2)).first) {
+		if (prefs::get().add_acquaintance(get_arg(1), "ignore", get_data(2)).first) {
 			print(_("ignores list"), VGETTEXT("Added to ignore list: $nick", symbols));
 			chat_handler_.user_relation_changed(get_arg(1));
 		}
@@ -96,7 +96,7 @@ void chat_command_handler::do_friend()
 		utils::string_map symbols;
 		symbols["nick"] = get_arg(1);
 
-		if (preferences::add_acquaintance(get_arg(1), "friend", get_data(2)).first) {
+		if (prefs::get().add_acquaintance(get_arg(1), "friend", get_data(2)).first) {
 			print(_("friends list"), VGETTEXT("Added to friends list: $nick", symbols));
 			chat_handler_.user_relation_changed(get_arg(1));
 		}
@@ -109,7 +109,7 @@ void chat_command_handler::do_friend()
 void chat_command_handler::do_remove()
 {
 	for (int i = 1;!get_arg(i).empty();i++) {
-		preferences::remove_acquaintance(get_arg(i));
+		prefs::get().remove_acquaintance(get_arg(i));
 		chat_handler_.user_relation_changed(get_arg(i));
 		utils::string_map symbols;
 		symbols["nick"] = get_arg(i);
@@ -119,7 +119,7 @@ void chat_command_handler::do_remove()
 
 void chat_command_handler::do_display()
 {
-	gui2::dialogs::preferences_dialog::display(preferences::VIEW_FRIENDS);
+	gui2::dialogs::preferences_dialog::display(pref_constants::VIEW_FRIENDS);
 }
 
 void chat_command_handler::do_version() {