Settings: Proper priority hierarchy

Remove old defaults system Introduce priority-based fallback list Use new functions for map_meta special functions Change groups to use end tags Unittest changes: * Adapt unittest to the new code * Compare Settings objects
minetest · Jan 29, 2021 · 37a05ec · 37a05ec
1 parent 5e9dd16
commit 37a05ec
Show file tree

Hide file tree

Showing 21 changed files with 359 additions and 299 deletions.
diff --git a/src/content/subgames.cpp b/src/content/subgames.cpp
@@ -329,18 +329,16 @@ void loadGameConfAndInitWorld(const std::string &path, const std::string &name,
 		}
 	}
 
-	// Override defaults with those provided by the game.
-	// We clear and reload the defaults because the defaults
-	// might have been overridden by other subgame config
-	// files that were loaded before.
-	g_settings->clearDefaults();
-	set_default_settings(g_settings);
-
-	Settings game_defaults;
-	getGameMinetestConfig(gamespec.path, game_defaults);
-	game_defaults.removeSecureSettings();
+	Settings *game_settings = Settings::getLayer(SL_GAME);
+	const bool new_game_settings = (game_settings == nullptr);
+	if (new_game_settings) {
+		// Called by main-menu without a Server instance running
+		// -> create and free manually
+		game_settings = Settings::createLayer(SL_GAME);
+	}
 
-	g_settings->overrideDefaults(&game_defaults);
+	getGameMinetestConfig(gamespec.path, *game_settings);
+	game_settings->removeSecureSettings();
 
 	infostream << "Initializing world at " << final_path << std::endl;
 
@@ -381,4 +379,8 @@ void loadGameConfAndInitWorld(const std::string &path, const std::string &name,
 
 		fs::safeWriteToFile(map_meta_path, oss.str());
 	}
+
+	// The Settings object is no longer needed for created worlds
+	if (new_game_settings)
+		delete game_settings;
 }
diff --git a/src/database/database-files.cpp b/src/database/database-files.cpp
@@ -122,18 +122,17 @@ void PlayerDatabaseFiles::serialize(RemotePlayer *p, std::ostream &os)
 	args.set("name", p->m_name);
 
 	// This should not happen
-	assert(m_sao);
-	args.setU16("hp", p->m_sao->getHP());
-	args.setV3F("position", p->m_sao->getBasePosition());
-	args.setFloat("pitch", p->m_sao->getLookPitch());
-	args.setFloat("yaw", p->m_sao->getRotation().Y);
-	args.setU16("breath", p->m_sao->getBreath());
+	PlayerSAO *sao = p->getPlayerSAO();
+	assert(sao);
+	args.setU16("hp", sao->getHP());
+	args.setV3F("position", sao->getBasePosition());
+	args.setFloat("pitch", sao->getLookPitch());
+	args.setFloat("yaw", sao->getRotation().Y);
+	args.setU16("breath", sao->getBreath());
 
 	std::string extended_attrs;
 	{
 		// serializeExtraAttributes
-		PlayerSAO *sao = p->getPlayerSAO();
-		assert(sao);
 		Json::Value json_root;
 
 		const StringMap &attrs = sao->getMeta().getStrings();

diff --git a/src/database/database-files.h b/src/database/database-files.h
@@ -38,8 +38,8 @@ class PlayerDatabaseFiles : public PlayerDatabase
 	void listPlayers(std::vector<std::string> &res);
 
 private:
-	void deSerialize(RemotePlayer *p, std::istream &is,
-		const std::string &playername, PlayerSAO *sao);
+	void deSerialize(RemotePlayer *p, std::istream &is, const std::string &playername,
+			PlayerSAO *sao);
 	/*
 		serialize() writes a bunch of text that can contain
 		any characters except a '\0', and such an ending that

diff --git a/src/defaultsettings.cpp b/src/defaultsettings.cpp
@@ -27,8 +27,10 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "mapgen/mapgen.h" // Mapgen::setDefaultSettings
 #include "util/string.h"
 
-void set_default_settings(Settings *settings)
+void set_default_settings()
 {
+	Settings *settings = Settings::createLayer(SL_DEFAULTS);
+
 	// Client and server
 	settings->setDefault("language", "");
 	settings->setDefault("name", "");

diff --git a/src/defaultsettings.h b/src/defaultsettings.h
@@ -25,11 +25,4 @@ class Settings;
  * initialize basic default settings
  * @param settings pointer to settings
  */
-void set_default_settings(Settings *settings);
-
-/**
- * override a default settings by settings from another settings element
- * @param settings target settings pointer
- * @param from source settings pointer
- */
-void override_default_settings(Settings *settings, Settings *from);
+void set_default_settings();
diff --git a/src/gui/guiKeyChangeMenu.cpp b/src/gui/guiKeyChangeMenu.cpp
@@ -248,7 +248,7 @@ bool GUIKeyChangeMenu::acceptInput()
 {
 	for (key_setting *k : key_settings) {
 		std::string default_key;
-		g_settings->getDefaultNoEx(k->setting_name, default_key);
+		Settings::getLayer(SL_DEFAULTS)->getNoEx(k->setting_name, default_key);
 
 		if (k->key.sym() != default_key)
 			g_settings->set(k->setting_name, k->key.sym());

diff --git a/src/main.cpp b/src/main.cpp
@@ -487,12 +487,15 @@ static bool create_userdata_path()
 static bool init_common(const Settings &cmd_args, int argc, char *argv[])
 {
 	startup_message();
-	set_default_settings(g_settings);
+	set_default_settings();
 
 	// Initialize sockets
 	sockets_init();
 	atexit(sockets_cleanup);
 
+	// Initialize g_settings
+	Settings::createLayer(SL_GLOBAL);
+
 	if (!read_config_file(cmd_args))
 		return false;
 
@@ -524,6 +527,7 @@ static bool read_config_file(const Settings &cmd_args)
 	// Path of configuration file in use
 	sanity_check(g_settings_path == "");	// Sanity check
 
+
 	if (cmd_args.exists("config")) {
 		bool r = g_settings->readConfigFile(cmd_args.get("config").c_str());
 		if (!r) {

diff --git a/src/map.cpp b/src/map.cpp
@@ -1184,7 +1184,7 @@ bool Map::isBlockOccluded(MapBlock *block, v3s16 cam_pos_nodes)
 ServerMap::ServerMap(const std::string &savedir, IGameDef *gamedef,
 		EmergeManager *emerge, MetricsBackend *mb):
 	Map(gamedef),
-	settings_mgr(g_settings, savedir + DIR_DELIM + "map_meta.txt"),
+	settings_mgr(savedir + DIR_DELIM + "map_meta.txt"),
 	m_emerge(emerge)
 {
 	verbosestream<<FUNCTION_NAME<<std::endl;

diff --git a/src/map_settings_manager.cpp b/src/map_settings_manager.cpp
@@ -25,17 +25,11 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "map_settings_manager.h"
 
-MapSettingsManager::MapSettingsManager(Settings *user_settings,
-		const std::string &map_meta_path):
-	m_map_meta_path(map_meta_path),
-	m_map_settings(new Settings()),
-	m_user_settings(user_settings)
+MapSettingsManager::MapSettingsManager(const std::string &map_meta_path):
+	m_map_meta_path(map_meta_path)
 {
-	assert(m_user_settings != NULL);
-
-	Mapgen::setDefaultSettings(m_map_settings);
-	// This inherits the combined defaults provided by loadGameConfAndInitWorld.
-	m_map_settings->overrideDefaults(user_settings);
+	m_map_settings = Settings::createLayer(SL_MAP, "[end_of_params]");
+	Mapgen::setDefaultSettings(Settings::getLayer(SL_DEFAULTS));
 }
 
 
@@ -49,22 +43,23 @@ MapSettingsManager::~MapSettingsManager()
 bool MapSettingsManager::getMapSetting(
 	const std::string &name, std::string *value_out)
 {
+	// Get from map_meta.txt, then try from all other sources
 	if (m_map_settings->getNoEx(name, *value_out))
 		return true;
 
 	// Compatibility kludge
-	if (m_user_settings == g_settings && name == "seed")
-		return m_user_settings->getNoEx("fixed_map_seed", *value_out);
+	if (name == "seed")
+		return Settings::getLayer(SL_GLOBAL)->getNoEx("fixed_map_seed", *value_out);
 
-	return m_user_settings->getNoEx(name, *value_out);
+	return false;
 }
 
 
 bool MapSettingsManager::getMapSettingNoiseParams(
 	const std::string &name, NoiseParams *value_out)
 {
-	return m_map_settings->getNoiseParams(name, *value_out) ||
-		m_user_settings->getNoiseParams(name, *value_out);
+	// TODO: Rename to "getNoiseParams"
+	return m_map_settings->getNoiseParams(name, *value_out);
 }
 
 
@@ -77,7 +72,7 @@ bool MapSettingsManager::setMapSetting(
 	if (override_meta)
 		m_map_settings->set(name, value);
 	else
-		m_map_settings->setDefault(name, value);
+		Settings::getLayer(SL_GLOBAL)->set(name, value);
 
 	return true;
 }
@@ -89,7 +84,11 @@ bool MapSettingsManager::setMapSettingNoiseParams(
 	if (mapgen_params)
 		return false;
 
-	m_map_settings->setNoiseParams(name, *value, !override_meta);
+	if (override_meta)
+		m_map_settings->setNoiseParams(name, *value);
+	else
+		Settings::getLayer(SL_GLOBAL)->setNoiseParams(name, *value);
+
 	return true;
 }
 
@@ -104,8 +103,8 @@ bool MapSettingsManager::loadMapMeta()
 		return false;
 	}
 
-	if (!m_map_settings->parseConfigLines(is, "[end_of_params]")) {
-		errorstream << "loadMapMeta: [end_of_params] not found!" << std::endl;
+	if (!m_map_settings->parseConfigLines(is)) {
+		errorstream << "loadMapMeta: Format error. '[end_of_params]' missing?" << std::endl;
 		return false;
 	}
 
@@ -116,28 +115,22 @@ bool MapSettingsManager::loadMapMeta()
 bool MapSettingsManager::saveMapMeta()
 {
 	// If mapgen params haven't been created yet; abort
-	if (!mapgen_params)
+	if (!mapgen_params) {
+		errorstream << "saveMapMeta: mapgen_params not present!" << std::endl;
 		return false;
+	}
 
+	// Paths set up by subgames.cpp, but not in unittests
 	if (!fs::CreateAllDirs(fs::RemoveLastPathComponent(m_map_meta_path))) {
 		errorstream << "saveMapMeta: could not create dirs to "
 			<< m_map_meta_path;
 		return false;
 	}
 
-	std::ostringstream oss(std::ios_base::binary);
-	Settings conf;
+	mapgen_params->MapgenParams::writeParams(m_map_settings);
+	mapgen_params->writeParams(m_map_settings);
 
-	mapgen_params->MapgenParams::writeParams(&conf);
-	mapgen_params->writeParams(&conf);
-	conf.writeLines(oss);
-
-	// NOTE: If there are ever types of map settings other than
-	// those relating to map generation, save them here
-
-	oss << "[end_of_params]\n";
-
-	if (!fs::safeWriteToFile(m_map_meta_path, oss.str())) {
+	if (!m_map_settings->updateConfigFile(m_map_meta_path.c_str())) {
 		errorstream << "saveMapMeta: could not write "
 			<< m_map_meta_path << std::endl;
 		return false;
@@ -152,23 +145,21 @@ MapgenParams *MapSettingsManager::makeMapgenParams()
 	if (mapgen_params)
 		return mapgen_params;
 
-	assert(m_user_settings != NULL);
 	assert(m_map_settings != NULL);
 
 	// At this point, we have (in order of precedence):
-	// 1). m_mapgen_settings->m_settings containing map_meta.txt settings or
+	// 1). SL_MAP containing map_meta.txt settings or
 	//     explicit overrides from scripts
-	// 2). m_mapgen_settings->m_defaults containing script-set mgparams without
-	//     overrides
-	// 3). g_settings->m_settings containing all user-specified config file
+	// 2). SL_GLOBAL containing all user-specified config file
 	//     settings
-	// 4). g_settings->m_defaults containing any low-priority settings from
+	// 3). SL_DEFAULTS containing any low-priority settings from
 	//     scripts, e.g. mods using Lua as an enhanced config file)
 
 	// Now, get the mapgen type so we can create the appropriate MapgenParams
 	std::string mg_name;
 	MapgenType mgtype = getMapSetting("mg_name", &mg_name) ?
 		Mapgen::getMapgenType(mg_name) : MAPGEN_DEFAULT;
+
 	if (mgtype == MAPGEN_INVALID) {
 		errorstream << "EmergeManager: mapgen '" << mg_name <<
 			"' not valid; falling back to " <<

diff --git a/src/map_settings_manager.h b/src/map_settings_manager.h
@@ -44,8 +44,7 @@ struct MapgenParams;
 */
 class MapSettingsManager {
 public:
-	MapSettingsManager(Settings *user_settings,
-		const std::string &map_meta_path);
+	MapSettingsManager(const std::string &map_meta_path);
 	~MapSettingsManager();
 
 	// Finalized map generation parameters
@@ -71,6 +70,6 @@ class MapSettingsManager {
 
 private:
 	std::string m_map_meta_path;
+	// TODO: Rename to "m_settings"
 	Settings *m_map_settings;
-	Settings *m_user_settings;
 };
diff --git a/src/remoteplayer.h b/src/remoteplayer.h
@@ -140,7 +140,6 @@ class RemotePlayer : public Player
 	void onSuccessfulSave();
 
 private:
-
 	PlayerSAO *m_sao = nullptr;
 	bool m_dirty = false;
 

diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp
@@ -982,7 +982,7 @@ int ModApiMapgen::l_set_noiseparams(lua_State *L)
 
 	bool set_default = !lua_isboolean(L, 3) || readParam<bool>(L, 3);
 
-	g_settings->setNoiseParams(name, np, set_default);
+	Settings::getLayer(set_default ? SL_DEFAULTS : SL_GLOBAL)->setNoiseParams(name, np);
 
 	return 0;
 }

diff --git a/src/script/lua_api/l_settings.cpp b/src/script/lua_api/l_settings.cpp
@@ -197,7 +197,7 @@ int LuaSettings::l_set_np_group(lua_State *L)
 
 	SET_SECURITY_CHECK(L, key);
 
-	o->m_settings->setNoiseParams(key, value, false);
+	o->m_settings->setNoiseParams(key, value);
 
 	return 0;
 }

diff --git a/src/script/scripting_mainmenu.cpp b/src/script/scripting_mainmenu.cpp
@@ -31,7 +31,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 extern "C" {
 #include "lualib.h"
 }
-
+#include "settings.h"
 #define MAINMENU_NUM_ASYNC_THREADS 4
 
 

diff --git a/src/server.cpp b/src/server.cpp
@@ -351,6 +351,7 @@ Server::~Server()
 	// Deinitialize scripting
 	infostream << "Server: Deinitializing scripting" << std::endl;
 	delete m_script;
+	delete m_game_settings;
 
 	while (!m_unsent_map_edit_queue.empty()) {
 		delete m_unsent_map_edit_queue.front();
@@ -368,6 +369,8 @@ void Server::init()
 	infostream << "- world:  " << m_path_world << std::endl;
 	infostream << "- game:   " << m_gamespec.path << std::endl;
 
+	m_game_settings = Settings::createLayer(SL_GAME);
+
 	// Create world if it doesn't exist
 	try {
 		loadGameConfAndInitWorld(m_path_world,

diff --git a/src/server.h b/src/server.h
@@ -524,6 +524,7 @@ class Server : public con::PeerHandler, public MapEventReceiver,
 	u16 m_max_chatmessage_length;
 	// For "dedicated" server list flag
 	bool m_dedicated;
+	Settings *m_game_settings = nullptr;
 
 	// Thread can set; step() will throw as ServerError
 	MutexedVariable<std::string> m_async_fatal_error;

diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp
@@ -632,7 +632,7 @@ void ServerEnvironment::saveMeta()
 	// Open file and serialize
 	std::ostringstream ss(std::ios_base::binary);
 
-	Settings args;
+	Settings args("EnvArgsEnd");
 	args.setU64("game_time", m_game_time);
 	args.setU64("time_of_day", getTimeOfDay());
 	args.setU64("last_clear_objects_time", m_last_clear_objects_time);
@@ -641,7 +641,6 @@ void ServerEnvironment::saveMeta()
 		m_lbm_mgr.createIntroductionTimesString());
 	args.setU64("day_count", m_day_count);
 	args.writeLines(ss);
-	ss<<"EnvArgsEnd\n";
 
 	if(!fs::safeWriteToFile(path, ss.str()))
 	{
@@ -676,9 +675,9 @@ void ServerEnvironment::loadMeta()
 		throw SerializationError("Couldn't load env meta");
 	}
 
-	Settings args;
+	Settings args("EnvArgsEnd");
 
-	if (!args.parseConfigLines(is, "EnvArgsEnd")) {
+	if (!args.parseConfigLines(is)) {
 		throw SerializationError("ServerEnvironment::loadMeta(): "
 			"EnvArgsEnd not found!");
 	}