From 45ccfe26fb6e0a130e4925ec362cccb1f045a829 Mon Sep 17 00:00:00 2001 From: Zughy <63455151+Zughy@users.noreply.github.com> Date: Thu, 21 Jan 2021 18:17:09 +0000 Subject: Removed some obsolete code (#10562) Co-authored-by: Zughy <4279489-marco_a@users.noreply.gitlab.com> --- src/script/lua_api/l_mapgen.cpp | 3 --- 1 file changed, 3 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 834938e56..498859f14 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -873,9 +873,6 @@ int ModApiMapgen::l_set_mapgen_params(lua_State *L) if (lua_isnumber(L, -1)) settingsmgr->setMapSetting("chunksize", readParam(L, -1), true); - warn_if_field_exists(L, 1, "flagmask", - "Obsolete: flags field now includes unset flags."); - lua_getfield(L, 1, "flags"); if (lua_isstring(L, -1)) settingsmgr->setMapSetting("mg_flags", readParam(L, -1), true); -- cgit v1.2.3 From 4fcd000e20a26120349184cb9d40342b7876e6b8 Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Thu, 21 Jan 2021 19:08:06 +0000 Subject: MgOre: Fix invalid field polymorphism (#10846) --- src/mapgen/mg_ore.h | 47 ++++++++++++++++++----------------------- src/script/lua_api/l_mapgen.cpp | 2 +- 2 files changed, 21 insertions(+), 28 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/mapgen/mg_ore.h b/src/mapgen/mg_ore.h index 76420fab4..a58fa9bfe 100644 --- a/src/mapgen/mg_ore.h +++ b/src/mapgen/mg_ore.h @@ -52,7 +52,7 @@ extern FlagDesc flagdesc_ore[]; class Ore : public ObjDef, public NodeResolver { public: - static const bool NEEDS_NOISE = false; + const bool needs_noise; content_t c_ore; // the node to place std::vector c_wherein; // the nodes to be placed in @@ -68,7 +68,7 @@ public: Noise *noise = nullptr; std::unordered_set biomes; - Ore() = default;; + explicit Ore(bool needs_noise): needs_noise(needs_noise) {} virtual ~Ore(); virtual void resolveNodeNames(); @@ -83,17 +83,17 @@ protected: class OreScatter : public Ore { public: - static const bool NEEDS_NOISE = false; + OreScatter() : Ore(false) {} ObjDef *clone() const; - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OreSheet : public Ore { public: - static const bool NEEDS_NOISE = true; + OreSheet() : Ore(true) {} ObjDef *clone() const; @@ -101,14 +101,12 @@ public: u16 column_height_max; float column_midpoint_factor; - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OrePuff : public Ore { public: - static const bool NEEDS_NOISE = true; - ObjDef *clone() const; NoiseParams np_puff_top; @@ -116,55 +114,50 @@ public: Noise *noise_puff_top = nullptr; Noise *noise_puff_bottom = nullptr; - OrePuff() = default; + OrePuff() : Ore(true) {} virtual ~OrePuff(); - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OreBlob : public Ore { public: - static const bool NEEDS_NOISE = true; - ObjDef *clone() const; - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + OreBlob() : Ore(true) {} + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OreVein : public Ore { public: - static const bool NEEDS_NOISE = true; - ObjDef *clone() const; float random_factor; Noise *noise2 = nullptr; int sizey_prev = 0; - OreVein() = default; + OreVein() : Ore(true) {} virtual ~OreVein(); - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OreStratum : public Ore { public: - static const bool NEEDS_NOISE = false; - ObjDef *clone() const; NoiseParams np_stratum_thickness; Noise *noise_stratum_thickness = nullptr; u16 stratum_thickness; - OreStratum() = default; + OreStratum() : Ore(false) {} virtual ~OreStratum(); - virtual void generate(MMVManip *vm, int mapseed, u32 blockseed, - v3s16 nmin, v3s16 nmax, biome_t *biomemap); + void generate(MMVManip *vm, int mapseed, u32 blockseed, + v3s16 nmin, v3s16 nmax, biome_t *biomemap) override; }; class OreManager : public ObjDefManager { diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 498859f14..fad08e1f6 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -1335,7 +1335,7 @@ int ModApiMapgen::l_register_ore(lua_State *L) lua_getfield(L, index, "noise_params"); if (read_noiseparams(L, -1, &ore->np)) { ore->flags |= OREFLAG_USE_NOISE; - } else if (ore->NEEDS_NOISE) { + } else if (ore->needs_noise) { errorstream << "register_ore: specified ore type requires valid " "'noise_params' parameter" << std::endl; delete ore; -- cgit v1.2.3 From 37a05ec8d6cbf9ff4432225cffe78c16fdd0647d Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sun, 22 Nov 2020 17:49:30 +0100 Subject: 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 --- src/content/subgames.cpp | 24 +-- src/database/database-files.cpp | 15 +- src/database/database-files.h | 4 +- src/defaultsettings.cpp | 4 +- src/defaultsettings.h | 9 +- src/gui/guiKeyChangeMenu.cpp | 2 +- src/main.cpp | 6 +- src/map.cpp | 2 +- src/map_settings_manager.cpp | 67 +++---- src/map_settings_manager.h | 5 +- src/remoteplayer.h | 1 - src/script/lua_api/l_mapgen.cpp | 2 +- src/script/lua_api/l_settings.cpp | 2 +- src/script/scripting_mainmenu.cpp | 2 +- src/server.cpp | 3 + src/server.h | 1 + src/serverenvironment.cpp | 7 +- src/settings.cpp | 300 ++++++++++++++--------------- src/settings.h | 43 +++-- src/unittest/test_map_settings_manager.cpp | 86 ++++++--- src/unittest/test_settings.cpp | 73 +++++-- 21 files changed, 359 insertions(+), 299 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/content/subgames.cpp b/src/content/subgames.cpp index c6350f2dd..e9dc609b0 100644 --- 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 index 529fb8763..d9e8f24ea 100644 --- 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 index a041cb1ff..e647a2e24 100644 --- a/src/database/database-files.h +++ b/src/database/database-files.h @@ -38,8 +38,8 @@ public: void listPlayers(std::vector &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 index 114351d86..d34ec324b 100644 --- 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 index c81e88502..c239b3c12 100644 --- 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 index eb641d952..4dcb47779 100644 --- 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 index f7238176b..57768dbb2 100644 --- 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 index aff545921..7c59edbaa 100644 --- 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<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 index 5baa38455..9258d3032 100644 --- 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 @@ public: 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 index b82cbc08e..8d086fc5a 100644 --- a/src/remoteplayer.h +++ b/src/remoteplayer.h @@ -140,7 +140,6 @@ public: 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 index fad08e1f6..183f20540 100644 --- 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(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 index 33eb02392..bcbaf15fa 100644 --- 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 index 0f672f917..9b377366e 100644 --- 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 index b5352749c..aba7b6401 100644 --- 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 index a7e85d0e1..1dd181794 100644 --- a/src/server.h +++ b/src/server.h @@ -524,6 +524,7 @@ private: 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 m_async_fatal_error; diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index 56dbb0632..3d9ba132b 100644 --- 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!"); } diff --git a/src/settings.cpp b/src/settings.cpp index f30ef34e9..cf2a16aa6 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -33,27 +33,50 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include -static Settings main_settings; -Settings *g_settings = &main_settings; +Settings *g_settings = nullptr; std::string g_settings_path; -Settings::~Settings() +Settings *Settings::s_layers[SL_TOTAL_COUNT] = {0}; // Zeroed by compiler +std::unordered_map Settings::s_flags; + + +Settings *Settings::createLayer(SettingsLayer sl, const std::string &end_tag) { - clear(); + if ((int)sl < 0 || sl >= SL_TOTAL_COUNT) + throw new BaseException("Invalid settings layer"); + + Settings *&pos = s_layers[(size_t)sl]; + if (pos) + throw new BaseException("Setting layer " + std::to_string(sl) + " already exists"); + + pos = new Settings(end_tag); + pos->m_settingslayer = sl; + + if (sl == SL_GLOBAL) + g_settings = pos; + return pos; } -Settings & Settings::operator += (const Settings &other) +Settings *Settings::getLayer(SettingsLayer sl) { - if (&other == this) - return *this; + sanity_check((int)sl >= 0 && sl < SL_TOTAL_COUNT); + return s_layers[(size_t)sl]; +} + +Settings::~Settings() +{ MutexAutoLock lock(m_mutex); - MutexAutoLock lock2(other.m_mutex); - updateNoLock(other); + if (m_settingslayer < SL_TOTAL_COUNT) + s_layers[(size_t)m_settingslayer] = nullptr; - return *this; + // Compatibility + if (m_settingslayer == SL_GLOBAL) + g_settings = nullptr; + + clearNoLock(); } @@ -62,11 +85,15 @@ Settings & Settings::operator = (const Settings &other) if (&other == this) return *this; + FATAL_ERROR_IF(m_settingslayer != SL_TOTAL_COUNT && other.m_settingslayer != SL_TOTAL_COUNT, + ("Tried to copy unique Setting layer " + std::to_string(m_settingslayer)).c_str()); + MutexAutoLock lock(m_mutex); MutexAutoLock lock2(other.m_mutex); clearNoLock(); - updateNoLock(other); + m_settings = other.m_settings; + m_callbacks = other.m_callbacks; return *this; } @@ -130,11 +157,11 @@ bool Settings::readConfigFile(const char *filename) if (!is.good()) return false; - return parseConfigLines(is, ""); + return parseConfigLines(is); } -bool Settings::parseConfigLines(std::istream &is, const std::string &end) +bool Settings::parseConfigLines(std::istream &is) { MutexAutoLock lock(m_mutex); @@ -142,7 +169,7 @@ bool Settings::parseConfigLines(std::istream &is, const std::string &end) while (is.good()) { std::getline(is, line); - SettingsParseEvent event = parseConfigObject(line, end, name, value); + SettingsParseEvent event = parseConfigObject(line, name, value); switch (event) { case SPE_NONE: @@ -155,8 +182,8 @@ bool Settings::parseConfigLines(std::istream &is, const std::string &end) case SPE_END: return true; case SPE_GROUP: { - Settings *group = new Settings; - if (!group->parseConfigLines(is, "}")) { + Settings *group = new Settings("}"); + if (!group->parseConfigLines(is)) { delete group; return false; } @@ -169,7 +196,8 @@ bool Settings::parseConfigLines(std::istream &is, const std::string &end) } } - return end.empty(); + // false (failure) if end tag not found + return m_end_tag.empty(); } @@ -179,6 +207,13 @@ void Settings::writeLines(std::ostream &os, u32 tab_depth) const for (const auto &setting_it : m_settings) printEntry(os, setting_it.first, setting_it.second, tab_depth); + + if (!m_end_tag.empty()) { + for (u32 i = 0; i < tab_depth; i++) + os << "\t"; + + os << m_end_tag << "\n"; + } } @@ -193,9 +228,7 @@ void Settings::printEntry(std::ostream &os, const std::string &name, entry.group->writeLines(os, tab_depth + 1); - for (u32 i = 0; i != tab_depth; i++) - os << "\t"; - os << "}\n"; + // Closing bracket handled by writeLines } else { os << name << " = "; @@ -207,8 +240,7 @@ void Settings::printEntry(std::ostream &os, const std::string &name, } -bool Settings::updateConfigObject(std::istream &is, std::ostream &os, - const std::string &end, u32 tab_depth) +bool Settings::updateConfigObject(std::istream &is, std::ostream &os, u32 tab_depth) { SettingEntries::const_iterator it; std::set present_entries; @@ -220,11 +252,11 @@ bool Settings::updateConfigObject(std::istream &is, std::ostream &os, // in the object if existing while (is.good() && !end_found) { std::getline(is, line); - SettingsParseEvent event = parseConfigObject(line, end, name, value); + SettingsParseEvent event = parseConfigObject(line, name, value); switch (event) { case SPE_END: - os << line << (is.eof() ? "" : "\n"); + // Skip end tag. Append later. end_found = true; break; case SPE_MULTILINE: @@ -252,14 +284,13 @@ bool Settings::updateConfigObject(std::istream &is, std::ostream &os, if (it != m_settings.end() && it->second.is_group) { os << line << "\n"; sanity_check(it->second.group != NULL); - was_modified |= it->second.group->updateConfigObject(is, os, - "}", tab_depth + 1); + was_modified |= it->second.group->updateConfigObject(is, os, tab_depth + 1); } else if (it == m_settings.end()) { // Remove by skipping was_modified = true; - Settings removed_group; // Move 'is' to group end + Settings removed_group("}"); // Move 'is' to group end std::stringstream ss; - removed_group.updateConfigObject(is, ss, "}", tab_depth + 1); + removed_group.updateConfigObject(is, ss, tab_depth + 1); break; } else { printEntry(os, name, it->second, tab_depth); @@ -273,6 +304,9 @@ bool Settings::updateConfigObject(std::istream &is, std::ostream &os, } } + if (!line.empty() && is.eof()) + os << "\n"; + // Add any settings in the object that don't exist in the config file yet for (it = m_settings.begin(); it != m_settings.end(); ++it) { if (present_entries.find(it->first) != present_entries.end()) @@ -282,6 +316,12 @@ bool Settings::updateConfigObject(std::istream &is, std::ostream &os, was_modified = true; } + // Append ending tag + if (!m_end_tag.empty()) { + os << m_end_tag << "\n"; + was_modified |= !end_found; + } + return was_modified; } @@ -293,7 +333,7 @@ bool Settings::updateConfigFile(const char *filename) std::ifstream is(filename); std::ostringstream os(std::ios_base::binary); - bool was_modified = updateConfigObject(is, os, ""); + bool was_modified = updateConfigObject(is, os); is.close(); if (!was_modified) @@ -366,29 +406,37 @@ bool Settings::parseCommandLine(int argc, char *argv[], * Getters * ***********/ - -const SettingsEntry &Settings::getEntry(const std::string &name) const +Settings *Settings::getParent() const { - MutexAutoLock lock(m_mutex); + // If the Settings object is within the hierarchy structure, + // iterate towards the origin (0) to find the next fallback layer + if (m_settingslayer >= SL_TOTAL_COUNT) + return nullptr; - SettingEntries::const_iterator n; - if ((n = m_settings.find(name)) == m_settings.end()) { - if ((n = m_defaults.find(name)) == m_defaults.end()) - throw SettingNotFoundException("Setting [" + name + "] not found."); + for (int i = (int)m_settingslayer - 1; i >= 0; --i) { + if (s_layers[i]) + return s_layers[i]; } - return n->second; + + // No parent + return nullptr; } -const SettingsEntry &Settings::getEntryDefault(const std::string &name) const +const SettingsEntry &Settings::getEntry(const std::string &name) const { - MutexAutoLock lock(m_mutex); + { + MutexAutoLock lock(m_mutex); - SettingEntries::const_iterator n; - if ((n = m_defaults.find(name)) == m_defaults.end()) { - throw SettingNotFoundException("Setting [" + name + "] not found."); + SettingEntries::const_iterator n; + if ((n = m_settings.find(name)) != m_settings.end()) + return n->second; } - return n->second; + + if (auto parent = getParent()) + return parent->getEntry(name); + + throw SettingNotFoundException("Setting [" + name + "] not found."); } @@ -412,10 +460,15 @@ const std::string &Settings::get(const std::string &name) const const std::string &Settings::getDefault(const std::string &name) const { - const SettingsEntry &entry = getEntryDefault(name); - if (entry.is_group) + const SettingsEntry *entry; + if (auto parent = getParent()) + entry = &parent->getEntry(name); + else + entry = &getEntry(name); // Bottom of the chain + + if (entry->is_group) throw SettingNotFoundException("Setting [" + name + "] is a group."); - return entry.value; + return entry->value; } @@ -491,29 +544,25 @@ u32 Settings::getFlagStr(const std::string &name, const FlagDesc *flagdesc, u32 *flagmask) const { u32 flags = 0; - u32 mask_default = 0; - std::string value; // Read default value (if there is any) - if (getDefaultNoEx(name, value)) { - flags = std::isdigit(value[0]) - ? stoi(value) - : readFlagString(value, flagdesc, &mask_default); - } + if (auto parent = getParent()) + flags = parent->getFlagStr(name, flagdesc, flagmask); // Apply custom flags "on top" - value = get(name); - u32 flags_user; - u32 mask_user = U32_MAX; - flags_user = std::isdigit(value[0]) - ? stoi(value) // Override default - : readFlagString(value, flagdesc, &mask_user); - - flags &= ~mask_user; - flags |= flags_user; - - if (flagmask) - *flagmask = mask_default | mask_user; + if (m_settings.find(name) != m_settings.end()) { + std::string value = get(name); + u32 flags_user; + u32 mask_user = U32_MAX; + flags_user = std::isdigit(value[0]) + ? stoi(value) // Override default + : readFlagString(value, flagdesc, &mask_user); + + flags &= ~mask_user; + flags |= flags_user; + if (flagmask) + *flagmask |= mask_user; + } return flags; } @@ -521,7 +570,12 @@ u32 Settings::getFlagStr(const std::string &name, const FlagDesc *flagdesc, bool Settings::getNoiseParams(const std::string &name, NoiseParams &np) const { - return getNoiseParamsFromGroup(name, np) || getNoiseParamsFromValue(name, np); + if (getNoiseParamsFromGroup(name, np) || getNoiseParamsFromValue(name, np)) + return true; + if (auto parent = getParent()) + return parent->getNoiseParams(name, np); + + return false; } @@ -583,13 +637,18 @@ bool Settings::exists(const std::string &name) const { MutexAutoLock lock(m_mutex); - return (m_settings.find(name) != m_settings.end() || - m_defaults.find(name) != m_defaults.end()); + if (m_settings.find(name) != m_settings.end()) + return true; + if (auto parent = getParent()) + return parent->exists(name); + return false; } std::vector Settings::getNames() const { + MutexAutoLock lock(m_mutex); + std::vector names; for (const auto &settings_it : m_settings) { names.push_back(settings_it.first); @@ -625,17 +684,6 @@ bool Settings::getNoEx(const std::string &name, std::string &val) const } -bool Settings::getDefaultNoEx(const std::string &name, std::string &val) const -{ - try { - val = getDefault(name); - return true; - } catch (SettingNotFoundException &e) { - return false; - } -} - - bool Settings::getFlag(const std::string &name) const { try { @@ -746,24 +794,25 @@ bool Settings::getFlagStrNoEx(const std::string &name, u32 &val, ***********/ bool Settings::setEntry(const std::string &name, const void *data, - bool set_group, bool set_default) + bool set_group) { - Settings *old_group = NULL; - if (!checkNameValid(name)) return false; if (!set_group && !checkValueValid(*(const std::string *)data)) return false; + Settings *old_group = NULL; { MutexAutoLock lock(m_mutex); - SettingsEntry &entry = set_default ? m_defaults[name] : m_settings[name]; + SettingsEntry &entry = m_settings[name]; old_group = entry.group; entry.value = set_group ? "" : *(const std::string *)data; entry.group = set_group ? *(Settings **)data : NULL; entry.is_group = set_group; + if (set_group) + entry.group->m_end_tag = "}"; } delete old_group; @@ -774,7 +823,7 @@ bool Settings::setEntry(const std::string &name, const void *data, bool Settings::set(const std::string &name, const std::string &value) { - if (!setEntry(name, &value, false, false)) + if (!setEntry(name, &value, false)) return false; doCallbacks(name); @@ -782,9 +831,10 @@ bool Settings::set(const std::string &name, const std::string &value) } +// TODO: Remove this function bool Settings::setDefault(const std::string &name, const std::string &value) { - return setEntry(name, &value, false, true); + return getLayer(SL_DEFAULTS)->set(name, value); } @@ -794,17 +844,7 @@ bool Settings::setGroup(const std::string &name, const Settings &group) // avoid double-free by copying the source Settings *copy = new Settings(); *copy = group; - return setEntry(name, ©, true, false); -} - - -bool Settings::setGroupDefault(const std::string &name, const Settings &group) -{ - // Settings must own the group pointer - // avoid double-free by copying the source - Settings *copy = new Settings(); - *copy = group; - return setEntry(name, ©, true, true); + return setEntry(name, ©, true); } @@ -874,8 +914,7 @@ bool Settings::setFlagStr(const std::string &name, u32 flags, } -bool Settings::setNoiseParams(const std::string &name, - const NoiseParams &np, bool set_default) +bool Settings::setNoiseParams(const std::string &name, const NoiseParams &np) { Settings *group = new Settings; @@ -888,7 +927,7 @@ bool Settings::setNoiseParams(const std::string &name, group->setFloat("lacunarity", np.lacunarity); group->setFlagStr("flags", np.flags, flagdesc_noiseparams, np.flags); - return setEntry(name, &group, true, set_default); + return setEntry(name, &group, true); } @@ -912,20 +951,8 @@ bool Settings::remove(const std::string &name) } -void Settings::clear() -{ - MutexAutoLock lock(m_mutex); - clearNoLock(); -} - -void Settings::clearDefaults() -{ - MutexAutoLock lock(m_mutex); - clearDefaultsNoLock(); -} - SettingsParseEvent Settings::parseConfigObject(const std::string &line, - const std::string &end, std::string &name, std::string &value) + std::string &name, std::string &value) { std::string trimmed_line = trim(line); @@ -933,7 +960,7 @@ SettingsParseEvent Settings::parseConfigObject(const std::string &line, return SPE_NONE; if (trimmed_line[0] == '#') return SPE_COMMENT; - if (trimmed_line == end) + if (trimmed_line == m_end_tag) return SPE_END; size_t pos = trimmed_line.find('='); @@ -952,67 +979,26 @@ SettingsParseEvent Settings::parseConfigObject(const std::string &line, } -void Settings::updateNoLock(const Settings &other) -{ - m_settings.insert(other.m_settings.begin(), other.m_settings.end()); - m_defaults.insert(other.m_defaults.begin(), other.m_defaults.end()); -} - - void Settings::clearNoLock() { - for (SettingEntries::const_iterator it = m_settings.begin(); it != m_settings.end(); ++it) delete it->second.group; m_settings.clear(); - - clearDefaultsNoLock(); } -void Settings::clearDefaultsNoLock() -{ - for (SettingEntries::const_iterator it = m_defaults.begin(); - it != m_defaults.end(); ++it) - delete it->second.group; - m_defaults.clear(); -} void Settings::setDefault(const std::string &name, const FlagDesc *flagdesc, u32 flags) { - m_flags[name] = flagdesc; + s_flags[name] = flagdesc; setDefault(name, writeFlagString(flags, flagdesc, U32_MAX)); } -void Settings::overrideDefaults(Settings *other) -{ - for (const auto &setting : other->m_settings) { - if (setting.second.is_group) { - setGroupDefault(setting.first, *setting.second.group); - continue; - } - const FlagDesc *flagdesc = getFlagDescFallback(setting.first); - if (flagdesc) { - // Flags cannot be copied directly. - // 1) Get the current set flags - u32 flags = getFlagStr(setting.first, flagdesc, nullptr); - // 2) Set the flags as defaults - other->setDefault(setting.first, flagdesc, flags); - // 3) Get the newly set flags and override the default setting value - setDefault(setting.first, flagdesc, - other->getFlagStr(setting.first, flagdesc, nullptr)); - continue; - } - // Also covers FlagDesc settings - setDefault(setting.first, setting.second.value); - } -} - const FlagDesc *Settings::getFlagDescFallback(const std::string &name) const { - auto it = m_flags.find(name); - return it == m_flags.end() ? nullptr : it->second; + auto it = s_flags.find(name); + return it == s_flags.end() ? nullptr : it->second; } void Settings::registerChangedCallback(const std::string &name, diff --git a/src/settings.h b/src/settings.h index 6db2f9481..af4cae3cd 100644 --- a/src/settings.h +++ b/src/settings.h @@ -30,7 +30,7 @@ class Settings; struct NoiseParams; // Global objects -extern Settings *g_settings; +extern Settings *g_settings; // Same as Settings::getLayer(SL_GLOBAL); extern std::string g_settings_path; // Type for a settings changed callback function @@ -60,6 +60,14 @@ enum SettingsParseEvent { SPE_MULTILINE, }; +enum SettingsLayer { + SL_DEFAULTS, + SL_GAME, + SL_GLOBAL, + SL_MAP, + SL_TOTAL_COUNT +}; + struct ValueSpec { ValueSpec(ValueType a_type, const char *a_help=NULL) { @@ -92,8 +100,13 @@ typedef std::unordered_map SettingEntries; class Settings { public: - Settings() = default; + static Settings *createLayer(SettingsLayer sl, const std::string &end_tag = ""); + static Settings *getLayer(SettingsLayer sl); + SettingsLayer getLayerType() const { return m_settingslayer; } + Settings(const std::string &end_tag = "") : + m_end_tag(end_tag) + {} ~Settings(); Settings & operator += (const Settings &other); @@ -110,7 +123,7 @@ public: // NOTE: Types of allowed_options are ignored. Returns success. bool parseCommandLine(int argc, char *argv[], std::map &allowed_options); - bool parseConfigLines(std::istream &is, const std::string &end = ""); + bool parseConfigLines(std::istream &is); void writeLines(std::ostream &os, u32 tab_depth=0) const; /*********** @@ -146,7 +159,6 @@ public: bool getGroupNoEx(const std::string &name, Settings *&val) const; bool getNoEx(const std::string &name, std::string &val) const; - bool getDefaultNoEx(const std::string &name, std::string &val) const; bool getFlag(const std::string &name) const; bool getU16NoEx(const std::string &name, u16 &val) const; bool getS16NoEx(const std::string &name, s16 &val) const; @@ -170,11 +182,10 @@ public: // N.B. Groups not allocated with new must be set to NULL in the settings // tree before object destruction. bool setEntry(const std::string &name, const void *entry, - bool set_group, bool set_default); + bool set_group); bool set(const std::string &name, const std::string &value); bool setDefault(const std::string &name, const std::string &value); bool setGroup(const std::string &name, const Settings &group); - bool setGroupDefault(const std::string &name, const Settings &group); bool setBool(const std::string &name, bool value); bool setS16(const std::string &name, s16 value); bool setU16(const std::string &name, u16 value); @@ -185,21 +196,16 @@ public: bool setV3F(const std::string &name, v3f value); bool setFlagStr(const std::string &name, u32 flags, const FlagDesc *flagdesc = nullptr, u32 flagmask = U32_MAX); - bool setNoiseParams(const std::string &name, const NoiseParams &np, - bool set_default=false); + bool setNoiseParams(const std::string &name, const NoiseParams &np); // remove a setting bool remove(const std::string &name); - void clear(); - void clearDefaults(); /************** * Miscellany * **************/ void setDefault(const std::string &name, const FlagDesc *flagdesc, u32 flags); - // Takes the provided setting values and uses them as new defaults - void overrideDefaults(Settings *other); const FlagDesc *getFlagDescFallback(const std::string &name) const; void registerChangedCallback(const std::string &name, @@ -215,9 +221,9 @@ private: ***********************/ SettingsParseEvent parseConfigObject(const std::string &line, - const std::string &end, std::string &name, std::string &value); + std::string &name, std::string &value); bool updateConfigObject(std::istream &is, std::ostream &os, - const std::string &end, u32 tab_depth=0); + u32 tab_depth=0); static bool checkNameValid(const std::string &name); static bool checkValueValid(const std::string &value); @@ -228,9 +234,9 @@ private: /*********** * Getters * ***********/ + Settings *getParent() const; const SettingsEntry &getEntry(const std::string &name) const; - const SettingsEntry &getEntryDefault(const std::string &name) const; // Allow TestSettings to run sanity checks using private functions. friend class TestSettings; @@ -242,14 +248,15 @@ private: void doCallbacks(const std::string &name) const; SettingEntries m_settings; - SettingEntries m_defaults; - std::unordered_map m_flags; - SettingsCallbackMap m_callbacks; + std::string m_end_tag; mutable std::mutex m_callback_mutex; // All methods that access m_settings/m_defaults directly should lock this. mutable std::mutex m_mutex; + static Settings *s_layers[SL_TOTAL_COUNT]; + SettingsLayer m_settingslayer = SL_TOTAL_COUNT; + static std::unordered_map s_flags; }; diff --git a/src/unittest/test_map_settings_manager.cpp b/src/unittest/test_map_settings_manager.cpp index 3d642a9b4..81ca68705 100644 --- a/src/unittest/test_map_settings_manager.cpp +++ b/src/unittest/test_map_settings_manager.cpp @@ -30,7 +30,7 @@ public: TestMapSettingsManager() { TestManager::registerTestModule(this); } const char *getName() { return "TestMapSettingsManager"; } - void makeUserConfig(Settings *conf); + void makeUserConfig(); std::string makeMetaFile(bool make_corrupt); void runTests(IGameDef *gamedef); @@ -65,8 +65,11 @@ void check_noise_params(const NoiseParams *np1, const NoiseParams *np2) } -void TestMapSettingsManager::makeUserConfig(Settings *conf) +void TestMapSettingsManager::makeUserConfig() { + delete Settings::getLayer(SL_GLOBAL); + Settings *conf = Settings::createLayer(SL_GLOBAL); + conf->set("mg_name", "v7"); conf->set("seed", "5678"); conf->set("water_level", "20"); @@ -103,12 +106,11 @@ std::string TestMapSettingsManager::makeMetaFile(bool make_corrupt) void TestMapSettingsManager::testMapSettingsManager() { - Settings user_settings; - makeUserConfig(&user_settings); + makeUserConfig(); std::string test_mapmeta_path = makeMetaFile(false); - MapSettingsManager mgr(&user_settings, test_mapmeta_path); + MapSettingsManager mgr(test_mapmeta_path); std::string value; UASSERT(mgr.getMapSetting("mg_name", &value)); @@ -140,6 +142,12 @@ void TestMapSettingsManager::testMapSettingsManager() mgr.setMapSettingNoiseParams("mgv5_np_height", &script_np_height); mgr.setMapSettingNoiseParams("mgv5_np_factor", &script_np_factor); + { + NoiseParams dummy; + mgr.getMapSettingNoiseParams("mgv5_np_factor", &dummy); + check_noise_params(&dummy, &script_np_factor); + } + // Now make our Params and see if the values are correctly sourced MapgenParams *params = mgr.makeMapgenParams(); UASSERT(params->mgtype == MAPGEN_V5); @@ -188,50 +196,66 @@ void TestMapSettingsManager::testMapSettingsManager() void TestMapSettingsManager::testMapMetaSaveLoad() { - Settings conf; std::string path = getTestTempDirectory() + DIR_DELIM + "foobar" + DIR_DELIM + "map_meta.txt"; + makeUserConfig(); + Settings &conf = *Settings::getLayer(SL_GLOBAL); + + // There cannot be two MapSettingsManager + // copy the mapgen params to compare them + MapgenParams params1, params2; // Create a set of mapgen params and save them to map meta - conf.set("seed", "12345"); - conf.set("water_level", "5"); - MapSettingsManager mgr1(&conf, path); - MapgenParams *params1 = mgr1.makeMapgenParams(); - UASSERT(params1); - UASSERT(mgr1.saveMapMeta()); + { + conf.set("seed", "12345"); + conf.set("water_level", "5"); + MapSettingsManager mgr(path); + MapgenParams *params = mgr.makeMapgenParams(); + UASSERT(params); + params1 = *params; + params1.bparams = nullptr; // No double-free + UASSERT(mgr.saveMapMeta()); + } // Now try loading the map meta to mapgen params - conf.set("seed", "67890"); - conf.set("water_level", "32"); - MapSettingsManager mgr2(&conf, path); - UASSERT(mgr2.loadMapMeta()); - MapgenParams *params2 = mgr2.makeMapgenParams(); - UASSERT(params2); + { + conf.set("seed", "67890"); + conf.set("water_level", "32"); + MapSettingsManager mgr(path); + UASSERT(mgr.loadMapMeta()); + MapgenParams *params = mgr.makeMapgenParams(); + UASSERT(params); + params2 = *params; + params2.bparams = nullptr; // No double-free + } // Check that both results are correct - UASSERTEQ(u64, params1->seed, 12345); - UASSERTEQ(s16, params1->water_level, 5); - UASSERTEQ(u64, params2->seed, 12345); - UASSERTEQ(s16, params2->water_level, 5); + UASSERTEQ(u64, params1.seed, 12345); + UASSERTEQ(s16, params1.water_level, 5); + UASSERTEQ(u64, params2.seed, 12345); + UASSERTEQ(s16, params2.water_level, 5); } void TestMapSettingsManager::testMapMetaFailures() { std::string test_mapmeta_path; - Settings conf; // Check to see if it'll fail on a non-existent map meta file - test_mapmeta_path = "woobawooba/fgdfg/map_meta.txt"; - UASSERT(!fs::PathExists(test_mapmeta_path)); + { + test_mapmeta_path = "woobawooba/fgdfg/map_meta.txt"; + UASSERT(!fs::PathExists(test_mapmeta_path)); - MapSettingsManager mgr1(&conf, test_mapmeta_path); - UASSERT(!mgr1.loadMapMeta()); + MapSettingsManager mgr1(test_mapmeta_path); + UASSERT(!mgr1.loadMapMeta()); + } // Check to see if it'll fail on a corrupt map meta file - test_mapmeta_path = makeMetaFile(true); - UASSERT(fs::PathExists(test_mapmeta_path)); + { + test_mapmeta_path = makeMetaFile(true); + UASSERT(fs::PathExists(test_mapmeta_path)); - MapSettingsManager mgr2(&conf, test_mapmeta_path); - UASSERT(!mgr2.loadMapMeta()); + MapSettingsManager mgr2(test_mapmeta_path); + UASSERT(!mgr2.loadMapMeta()); + } } diff --git a/src/unittest/test_settings.cpp b/src/unittest/test_settings.cpp index f91ba5b67..d2d35c357 100644 --- a/src/unittest/test_settings.cpp +++ b/src/unittest/test_settings.cpp @@ -21,6 +21,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include "settings.h" +#include "defaultsettings.h" #include "noise.h" class TestSettings : public TestBase { @@ -31,6 +32,7 @@ public: void runTests(IGameDef *gamedef); void testAllSettings(); + void testDefaults(); void testFlagDesc(); static const char *config_text_before; @@ -42,6 +44,7 @@ static TestSettings g_test_instance; void TestSettings::runTests(IGameDef *gamedef) { TEST(testAllSettings); + TEST(testDefaults); TEST(testFlagDesc); } @@ -70,7 +73,8 @@ const char *TestSettings::config_text_before = " with leading whitespace!\n" "\"\"\"\n" "np_terrain = 5, 40, (250, 250, 250), 12341, 5, 0.7, 2.4\n" - "zoop = true"; + "zoop = true\n" + "[dummy_eof_end_tag]\n"; const std::string TestSettings::config_text_after = "leet = 1337\n" @@ -111,12 +115,34 @@ const std::string TestSettings::config_text_after = " animals = cute\n" " num_apples = 4\n" " num_oranges = 53\n" - "}\n"; + "}\n" + "[dummy_eof_end_tag]"; + +void compare_settings(const std::string &name, Settings *a, Settings *b) +{ + auto keys = a->getNames(); + Settings *group1, *group2; + std::string value1, value2; + for (auto &key : keys) { + if (a->getGroupNoEx(key, group1)) { + UASSERT(b->getGroupNoEx(key, group2)); + + compare_settings(name + "->" + key, group1, group2); + continue; + } + + UASSERT(b->getNoEx(key, value1)); + // For identification + value1 = name + "->" + key + "=" + value1; + value2 = name + "->" + key + "=" + a->get(key); + UASSERTCMP(std::string, ==, value2, value1); + } +} void TestSettings::testAllSettings() { try { - Settings s; + Settings s("[dummy_eof_end_tag]"); // Test reading of settings std::istringstream is(config_text_before); @@ -197,21 +223,44 @@ void TestSettings::testAllSettings() is.clear(); is.seekg(0); - UASSERT(s.updateConfigObject(is, os, "", 0) == true); - //printf(">>>> expected config:\n%s\n", TEST_CONFIG_TEXT_AFTER); - //printf(">>>> actual config:\n%s\n", os.str().c_str()); -#if __cplusplus < 201103L - // This test only works in older C++ versions than C++11 because we use unordered_map - UASSERT(os.str() == config_text_after); -#endif + UASSERT(s.updateConfigObject(is, os, 0) == true); + + { + // Confirm settings + Settings s2("[dummy_eof_end_tag]"); + std::istringstream is(config_text_after, std::ios_base::binary); + s2.parseConfigLines(is); + + compare_settings("(main)", &s, &s2); + } + } catch (SettingNotFoundException &e) { UASSERT(!"Setting not found!"); } } +void TestSettings::testDefaults() +{ + Settings *game = Settings::createLayer(SL_GAME); + Settings *def = Settings::getLayer(SL_DEFAULTS); + + def->set("name", "FooBar"); + UASSERT(def->get("name") == "FooBar"); + UASSERT(game->get("name") == "FooBar"); + + game->set("name", "Baz"); + UASSERT(game->get("name") == "Baz"); + + delete game; + + // Restore default settings + delete Settings::getLayer(SL_DEFAULTS); + set_default_settings(); +} + void TestSettings::testFlagDesc() { - Settings s; + Settings &s = *Settings::createLayer(SL_GAME); FlagDesc flagdesc[] = { { "biomes", 0x01 }, { "trees", 0x02 }, @@ -242,4 +291,6 @@ void TestSettings::testFlagDesc() // Enabled: tables s.set("test_flags", "16"); UASSERT(s.getFlagStr("test_flags", flagdesc, nullptr) == 0x10); + + delete &s; } -- cgit v1.2.3 From fbb9ef3818b17894843f967c0c23b5d57a1e3771 Mon Sep 17 00:00:00 2001 From: rubenwardy Date: Sat, 6 Feb 2021 12:46:45 +0000 Subject: Reduce ore noise_parms error to deprecation warning (#10921) Fixes #10914 --- src/script/lua_api/l_mapgen.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 183f20540..12a497b1e 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -1336,10 +1336,8 @@ int ModApiMapgen::l_register_ore(lua_State *L) if (read_noiseparams(L, -1, &ore->np)) { ore->flags |= OREFLAG_USE_NOISE; } else if (ore->needs_noise) { - errorstream << "register_ore: specified ore type requires valid " - "'noise_params' parameter" << std::endl; - delete ore; - return 0; + log_deprecated(L, + "register_ore: ore type requires 'noise_params' but it is not specified, falling back to defaults"); } lua_pop(L, 1); -- cgit v1.2.3 From 05719913aca97e53ff5b1dde49e1a033a327551f Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 20 Mar 2021 13:02:15 +0100 Subject: Schematic: Properly deal with before/after node resolving and document (#11011) This fixes an out-of-bounds index access when the node resolver was already applied to the schematic (i.e. biome decoration). Also improves the handling of the two cases: prior node resolving (m_nodenames), and after node resolving (manual lookup) --- src/mapgen/mg_schematic.cpp | 129 ++++++++++++++++++++++--------------- src/mapgen/mg_schematic.h | 16 ++--- src/nodedef.cpp | 16 ++++- src/nodedef.h | 31 ++++++++- src/script/lua_api/l_mapgen.cpp | 5 +- src/unittest/test_noderesolver.cpp | 2 + src/unittest/test_schematic.cpp | 40 +++++++----- 7 files changed, 153 insertions(+), 86 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/mapgen/mg_schematic.cpp b/src/mapgen/mg_schematic.cpp index e70e97e48..653bad4fe 100644 --- a/src/mapgen/mg_schematic.cpp +++ b/src/mapgen/mg_schematic.cpp @@ -76,10 +76,6 @@ void SchematicManager::clear() /////////////////////////////////////////////////////////////////////////////// -Schematic::Schematic() -= default; - - Schematic::~Schematic() { delete []schemdata; @@ -108,13 +104,19 @@ ObjDef *Schematic::clone() const void Schematic::resolveNodeNames() { + c_nodes.clear(); getIdsFromNrBacklog(&c_nodes, true, CONTENT_AIR); size_t bufsize = size.X * size.Y * size.Z; for (size_t i = 0; i != bufsize; i++) { content_t c_original = schemdata[i].getContent(); - content_t c_new = c_nodes[c_original]; - schemdata[i].setContent(c_new); + if (c_original >= c_nodes.size()) { + errorstream << "Corrupt schematic. name=\"" << name + << "\" at index " << i << std::endl; + c_original = 0; + } + // Unfold condensed ID layout to content_t + schemdata[i].setContent(c_nodes[c_original]); } } @@ -279,8 +281,7 @@ void Schematic::placeOnMap(ServerMap *map, v3s16 p, u32 flags, } -bool Schematic::deserializeFromMts(std::istream *is, - std::vector *names) +bool Schematic::deserializeFromMts(std::istream *is) { std::istream &ss = *is; content_t cignore = CONTENT_IGNORE; @@ -312,6 +313,8 @@ bool Schematic::deserializeFromMts(std::istream *is, slice_probs[y] = (version >= 3) ? readU8(ss) : MTSCHEM_PROB_ALWAYS_OLD; //// Read node names + NodeResolver::reset(); + u16 nidmapcount = readU16(ss); for (int i = 0; i != nidmapcount; i++) { std::string name = deSerializeString16(ss); @@ -324,9 +327,12 @@ bool Schematic::deserializeFromMts(std::istream *is, have_cignore = true; } - names->push_back(name); + m_nodenames.push_back(name); } + // Prepare for node resolver + m_nnlistsizes.push_back(m_nodenames.size()); + //// Read node data size_t nodecount = size.X * size.Y * size.Z; @@ -358,9 +364,11 @@ bool Schematic::deserializeFromMts(std::istream *is, } -bool Schematic::serializeToMts(std::ostream *os, - const std::vector &names) const +bool Schematic::serializeToMts(std::ostream *os) const { + // Nodes must not be resolved (-> condensed) + // checking here is not possible because "schemdata" might be temporary. + std::ostream &ss = *os; writeU32(ss, MTSCHEM_FILE_SIGNATURE); // signature @@ -370,9 +378,10 @@ bool Schematic::serializeToMts(std::ostream *os, for (int y = 0; y != size.Y; y++) // Y slice probabilities writeU8(ss, slice_probs[y]); - writeU16(ss, names.size()); // name count - for (size_t i = 0; i != names.size(); i++) - ss << serializeString16(names[i]); // node names + writeU16(ss, m_nodenames.size()); // name count + for (size_t i = 0; i != m_nodenames.size(); i++) { + ss << serializeString16(m_nodenames[i]); // node names + } // compressed bulk node data MapNode::serializeBulk(ss, SER_FMT_VER_HIGHEST_WRITE, @@ -382,8 +391,7 @@ bool Schematic::serializeToMts(std::ostream *os, } -bool Schematic::serializeToLua(std::ostream *os, - const std::vector &names, bool use_comments, +bool Schematic::serializeToLua(std::ostream *os, bool use_comments, u32 indent_spaces) const { std::ostream &ss = *os; @@ -392,6 +400,9 @@ bool Schematic::serializeToLua(std::ostream *os, if (indent_spaces > 0) indent.assign(indent_spaces, ' '); + bool resolve_done = isResolveDone(); + FATAL_ERROR_IF(resolve_done && !m_ndef, "serializeToLua: NodeDefManager is required"); + //// Write header { ss << "schematic = {" << std::endl; @@ -436,9 +447,22 @@ bool Schematic::serializeToLua(std::ostream *os, u8 probability = schemdata[i].param1 & MTSCHEM_PROB_MASK; bool force_place = schemdata[i].param1 & MTSCHEM_FORCE_PLACE; - ss << indent << indent << "{" - << "name=\"" << names[schemdata[i].getContent()] - << "\", prob=" << (u16)probability * 2 + // After node resolving: real content_t, lookup using NodeDefManager + // Prior node resolving: condensed ID, lookup using m_nodenames + content_t c = schemdata[i].getContent(); + + ss << indent << indent << "{" << "name=\""; + + if (!resolve_done) { + // Prior node resolving (eg. direct schematic load) + FATAL_ERROR_IF(c >= m_nodenames.size(), "Invalid node list"); + ss << m_nodenames[c]; + } else { + // After node resolving (eg. biome decoration) + ss << m_ndef->get(c).name; + } + + ss << "\", prob=" << (u16)probability * 2 << ", param2=" << (u16)schemdata[i].param2; if (force_place) @@ -467,25 +491,24 @@ bool Schematic::loadSchematicFromFile(const std::string &filename, return false; } - size_t origsize = m_nodenames.size(); - if (!deserializeFromMts(&is, &m_nodenames)) - return false; + if (!m_ndef) + m_ndef = ndef; - m_nnlistsizes.push_back(m_nodenames.size() - origsize); + if (!deserializeFromMts(&is)) + return false; name = filename; if (replace_names) { - for (size_t i = origsize; i < m_nodenames.size(); i++) { - std::string &node_name = m_nodenames[i]; + for (std::string &node_name : m_nodenames) { StringMap::iterator it = replace_names->find(node_name); if (it != replace_names->end()) node_name = it->second; } } - if (ndef) - ndef->pendNodeResolve(this); + if (m_ndef) + m_ndef->pendNodeResolve(this); return true; } @@ -494,33 +517,26 @@ bool Schematic::loadSchematicFromFile(const std::string &filename, bool Schematic::saveSchematicToFile(const std::string &filename, const NodeDefManager *ndef) { - MapNode *orig_schemdata = schemdata; - std::vector ndef_nodenames; - std::vector *names; + Schematic *schem = this; - if (m_resolve_done && ndef == NULL) - ndef = m_ndef; + bool needs_condense = isResolveDone(); - if (ndef) { - names = &ndef_nodenames; + if (!m_ndef) + m_ndef = ndef; - u32 volume = size.X * size.Y * size.Z; - schemdata = new MapNode[volume]; - for (u32 i = 0; i != volume; i++) - schemdata[i] = orig_schemdata[i]; + if (needs_condense) { + if (!m_ndef) + return false; - generate_nodelist_and_update_ids(schemdata, volume, names, ndef); - } else { // otherwise, use the names we have on hand in the list - names = &m_nodenames; + schem = (Schematic *)this->clone(); + schem->condenseContentIds(); } std::ostringstream os(std::ios_base::binary); - bool status = serializeToMts(&os, *names); + bool status = schem->serializeToMts(&os); - if (ndef) { - delete []schemdata; - schemdata = orig_schemdata; - } + if (needs_condense) + delete schem; if (!status) return false; @@ -556,6 +572,10 @@ bool Schematic::getSchematicFromMap(Map *map, v3s16 p1, v3s16 p2) } delete vm; + + // Reset and mark as complete + NodeResolver::reset(true); + return true; } @@ -584,26 +604,29 @@ void Schematic::applyProbabilities(v3s16 p0, } -void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount, - std::vector *usednodes, const NodeDefManager *ndef) +void Schematic::condenseContentIds() { std::unordered_map nodeidmap; content_t numids = 0; + // Reset node resolve fields + NodeResolver::reset(); + + size_t nodecount = size.X * size.Y * size.Z; for (size_t i = 0; i != nodecount; i++) { content_t id; - content_t c = nodes[i].getContent(); + content_t c = schemdata[i].getContent(); - std::unordered_map::const_iterator it = nodeidmap.find(c); + auto it = nodeidmap.find(c); if (it == nodeidmap.end()) { id = numids; numids++; - usednodes->push_back(ndef->get(c).name); - nodeidmap.insert(std::make_pair(c, id)); + m_nodenames.push_back(m_ndef->get(c).name); + nodeidmap.emplace(std::make_pair(c, id)); } else { id = it->second; } - nodes[i].setContent(id); + schemdata[i].setContent(id); } } diff --git a/src/mapgen/mg_schematic.h b/src/mapgen/mg_schematic.h index 6b31251b6..5f64ea280 100644 --- a/src/mapgen/mg_schematic.h +++ b/src/mapgen/mg_schematic.h @@ -92,7 +92,7 @@ enum SchematicFormatType { class Schematic : public ObjDef, public NodeResolver { public: - Schematic(); + Schematic() = default; virtual ~Schematic(); ObjDef *clone() const; @@ -105,11 +105,9 @@ public: const NodeDefManager *ndef); bool getSchematicFromMap(Map *map, v3s16 p1, v3s16 p2); - bool deserializeFromMts(std::istream *is, std::vector *names); - bool serializeToMts(std::ostream *os, - const std::vector &names) const; - bool serializeToLua(std::ostream *os, const std::vector &names, - bool use_comments, u32 indent_spaces) const; + bool deserializeFromMts(std::istream *is); + bool serializeToMts(std::ostream *os) const; + bool serializeToLua(std::ostream *os, bool use_comments, u32 indent_spaces) const; void blitToVManip(MMVManip *vm, v3s16 p, Rotation rot, bool force_place); bool placeOnVManip(MMVManip *vm, v3s16 p, u32 flags, Rotation rot, bool force_place); @@ -124,6 +122,10 @@ public: v3s16 size; MapNode *schemdata = nullptr; u8 *slice_probs = nullptr; + +private: + // Counterpart to the node resolver: Condense content_t to a sequential "m_nodenames" list + void condenseContentIds(); }; class SchematicManager : public ObjDefManager { @@ -151,5 +153,3 @@ private: Server *m_server; }; -void generate_nodelist_and_update_ids(MapNode *nodes, size_t nodecount, - std::vector *usednodes, const NodeDefManager *ndef); diff --git a/src/nodedef.cpp b/src/nodedef.cpp index 57d4c008f..8a1f6203b 100644 --- a/src/nodedef.cpp +++ b/src/nodedef.cpp @@ -1675,8 +1675,7 @@ bool NodeDefManager::nodeboxConnects(MapNode from, MapNode to, NodeResolver::NodeResolver() { - m_nodenames.reserve(16); - m_nnlistsizes.reserve(4); + reset(); } @@ -1779,3 +1778,16 @@ bool NodeResolver::getIdsFromNrBacklog(std::vector *result_out, return success; } + +void NodeResolver::reset(bool resolve_done) +{ + m_nodenames.clear(); + m_nodenames_idx = 0; + m_nnlistsizes.clear(); + m_nnlistsizes_idx = 0; + + m_resolve_done = resolve_done; + + m_nodenames.reserve(16); + m_nnlistsizes.reserve(4); +} diff --git a/src/nodedef.h b/src/nodedef.h index 6fc20518d..3e77624eb 100644 --- a/src/nodedef.h +++ b/src/nodedef.h @@ -44,6 +44,9 @@ class ITextureSource; class IShaderSource; class IGameDef; class NodeResolver; +#if BUILD_UNITTESTS +class TestSchematic; +#endif enum ContentParamType { @@ -789,10 +792,13 @@ private: NodeDefManager *createNodeDefManager(); +// NodeResolver: Queue for node names which are then translated +// to content_t after the NodeDefManager was initialized class NodeResolver { public: NodeResolver(); virtual ~NodeResolver(); + // Callback which is run as soon NodeDefManager is ready virtual void resolveNodeNames() = 0; // required because this class is used as mixin for ObjDef @@ -804,12 +810,31 @@ public: bool getIdsFromNrBacklog(std::vector *result_out, bool all_required = false, content_t c_fallback = CONTENT_IGNORE); - void nodeResolveInternal(); + inline bool isResolveDone() const { return m_resolve_done; } + void reset(bool resolve_done = false); - u32 m_nodenames_idx = 0; - u32 m_nnlistsizes_idx = 0; + // Vector containing all node names in the resolve "queue" std::vector m_nodenames; + // Specifies the "set size" of node names which are to be processed + // this is used for getIdsFromNrBacklog + // TODO: replace or remove std::vector m_nnlistsizes; + +protected: + friend class NodeDefManager; // m_ndef + const NodeDefManager *m_ndef = nullptr; + // Index of the next "m_nodenames" entry to resolve + u32 m_nodenames_idx = 0; + +private: +#if BUILD_UNITTESTS + // Unittest requires access to m_resolve_done + friend class TestSchematic; +#endif + void nodeResolveInternal(); + + // Index of the next "m_nnlistsizes" entry to process + u32 m_nnlistsizes_idx = 0; bool m_resolve_done = false; }; diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index 12a497b1e..cc93bdbd0 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -1734,11 +1734,10 @@ int ModApiMapgen::l_serialize_schematic(lua_State *L) std::ostringstream os(std::ios_base::binary); switch (schem_format) { case SCHEM_FMT_MTS: - schem->serializeToMts(&os, schem->m_nodenames); + schem->serializeToMts(&os); break; case SCHEM_FMT_LUA: - schem->serializeToLua(&os, schem->m_nodenames, - use_comments, indent_spaces); + schem->serializeToLua(&os, use_comments, indent_spaces); break; default: return 0; diff --git a/src/unittest/test_noderesolver.cpp b/src/unittest/test_noderesolver.cpp index 28da43620..ed66093a9 100644 --- a/src/unittest/test_noderesolver.cpp +++ b/src/unittest/test_noderesolver.cpp @@ -56,6 +56,8 @@ void TestNodeResolver::runTests(IGameDef *gamedef) class Foobar : public NodeResolver { public: + friend class TestNodeResolver; // m_ndef + void resolveNodeNames(); content_t test_nr_node1; diff --git a/src/unittest/test_schematic.cpp b/src/unittest/test_schematic.cpp index da4ce50d2..d2f027eb4 100644 --- a/src/unittest/test_schematic.cpp +++ b/src/unittest/test_schematic.cpp @@ -66,13 +66,14 @@ void TestSchematic::testMtsSerializeDeserialize(const NodeDefManager *ndef) std::stringstream ss(std::ios_base::binary | std::ios_base::in | std::ios_base::out); - std::vector names; - names.emplace_back("foo"); - names.emplace_back("bar"); - names.emplace_back("baz"); - names.emplace_back("qux"); - - Schematic schem, schem2; + Schematic schem; + { + std::vector &names = schem.m_nodenames; + names.emplace_back("foo"); + names.emplace_back("bar"); + names.emplace_back("baz"); + names.emplace_back("qux"); + } schem.flags = 0; schem.size = size; @@ -83,18 +84,21 @@ void TestSchematic::testMtsSerializeDeserialize(const NodeDefManager *ndef) for (s16 y = 0; y != size.Y; y++) schem.slice_probs[y] = MTSCHEM_PROB_ALWAYS; - UASSERT(schem.serializeToMts(&ss, names)); + UASSERT(schem.serializeToMts(&ss)); ss.seekg(0); - names.clear(); - UASSERT(schem2.deserializeFromMts(&ss, &names)); + Schematic schem2; + UASSERT(schem2.deserializeFromMts(&ss)); - UASSERTEQ(size_t, names.size(), 4); - UASSERTEQ(std::string, names[0], "foo"); - UASSERTEQ(std::string, names[1], "bar"); - UASSERTEQ(std::string, names[2], "baz"); - UASSERTEQ(std::string, names[3], "qux"); + { + std::vector &names = schem2.m_nodenames; + UASSERTEQ(size_t, names.size(), 4); + UASSERTEQ(std::string, names[0], "foo"); + UASSERTEQ(std::string, names[1], "bar"); + UASSERTEQ(std::string, names[2], "baz"); + UASSERTEQ(std::string, names[3], "qux"); + } UASSERT(schem2.size == size); for (size_t i = 0; i != volume; i++) @@ -120,14 +124,14 @@ void TestSchematic::testLuaTableSerialize(const NodeDefManager *ndef) for (s16 y = 0; y != size.Y; y++) schem.slice_probs[y] = MTSCHEM_PROB_ALWAYS; - std::vector names; + std::vector &names = schem.m_nodenames; names.emplace_back("air"); names.emplace_back("default:lava_source"); names.emplace_back("default:glass"); std::ostringstream ss(std::ios_base::binary); - UASSERT(schem.serializeToLua(&ss, names, false, 0)); + UASSERT(schem.serializeToLua(&ss, false, 0)); UASSERTEQ(std::string, ss.str(), expected_lua_output); } @@ -159,6 +163,8 @@ void TestSchematic::testFileSerializeDeserialize(const NodeDefManager *ndef) schem1.slice_probs[0] = 80; schem1.slice_probs[1] = 160; schem1.slice_probs[2] = 240; + // Node resolving happened manually. + schem1.m_resolve_done = true; for (size_t i = 0; i != volume; i++) { content_t c = content_map[test_schem2_data[i]]; -- cgit v1.2.3 From 2da1eee394554879bf1cee6bc0f7b77acf0b6c43 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 23 Mar 2021 15:43:26 +0100 Subject: Fix broken `BiomeGen` abstraction (#11107) --- src/emerge.cpp | 16 ++++-- src/emerge.h | 7 ++- src/mapgen/mapgen.cpp | 4 +- src/mapgen/mapgen_valleys.cpp | 3 +- src/mapgen/mg_biome.cpp | 90 ++++++------------------------- src/mapgen/mg_biome.h | 27 ++++++---- src/noise.cpp | 6 +-- src/noise.h | 6 +-- src/script/lua_api/l_mapgen.cpp | 116 ++++++++++------------------------------ 9 files changed, 90 insertions(+), 185 deletions(-) (limited to 'src/script/lua_api/l_mapgen.cpp') diff --git a/src/emerge.cpp b/src/emerge.cpp index e0dc5628e..32e7d9f24 100644 --- a/src/emerge.cpp +++ b/src/emerge.cpp @@ -113,13 +113,15 @@ EmergeParams::~EmergeParams() { infostream << "EmergeParams: destroying " << this << std::endl; // Delete everything that was cloned on creation of EmergeParams + delete biomegen; delete biomemgr; delete oremgr; delete decomgr; delete schemmgr; } -EmergeParams::EmergeParams(EmergeManager *parent, const BiomeManager *biomemgr, +EmergeParams::EmergeParams(EmergeManager *parent, const BiomeGen *biomegen, + const BiomeManager *biomemgr, const OreManager *oremgr, const DecorationManager *decomgr, const SchematicManager *schemmgr) : ndef(parent->ndef), @@ -129,6 +131,7 @@ EmergeParams::EmergeParams(EmergeManager *parent, const BiomeManager *biomemgr, biomemgr(biomemgr->clone()), oremgr(oremgr->clone()), decomgr(decomgr->clone()), schemmgr(schemmgr->clone()) { + this->biomegen = biomegen->clone(this->biomemgr); } //// @@ -143,6 +146,10 @@ EmergeManager::EmergeManager(Server *server) this->decomgr = new DecorationManager(server); this->schemmgr = new SchematicManager(server); + // initialized later + this->mgparams = nullptr; + this->biomegen = nullptr; + // Note that accesses to this variable are not synchronized. // This is because the *only* thread ever starting or stopping // EmergeThreads should be the ServerThread. @@ -240,9 +247,12 @@ void EmergeManager::initMapgens(MapgenParams *params) mgparams = params; + v3s16 csize = v3s16(1, 1, 1) * (params->chunksize * MAP_BLOCKSIZE); + biomegen = biomemgr->createBiomeGen(BIOMEGEN_ORIGINAL, params->bparams, csize); + for (u32 i = 0; i != m_threads.size(); i++) { - EmergeParams *p = new EmergeParams( - this, biomemgr, oremgr, decomgr, schemmgr); + EmergeParams *p = new EmergeParams(this, biomegen, + biomemgr, oremgr, decomgr, schemmgr); infostream << "EmergeManager: Created params " << p << " for thread " << i << std::endl; m_mapgens.push_back(Mapgen::createMapgen(params->mgtype, params, p)); diff --git a/src/emerge.h b/src/emerge.h index da845e243..aac3e7dd3 100644 --- a/src/emerge.h +++ b/src/emerge.h @@ -99,13 +99,15 @@ public: u32 gen_notify_on; const std::set *gen_notify_on_deco_ids; // shared + BiomeGen *biomegen; BiomeManager *biomemgr; OreManager *oremgr; DecorationManager *decomgr; SchematicManager *schemmgr; private: - EmergeParams(EmergeManager *parent, const BiomeManager *biomemgr, + EmergeParams(EmergeManager *parent, const BiomeGen *biomegen, + const BiomeManager *biomemgr, const OreManager *oremgr, const DecorationManager *decomgr, const SchematicManager *schemmgr); }; @@ -140,6 +142,8 @@ public: ~EmergeManager(); DISABLE_CLASS_COPY(EmergeManager); + const BiomeGen *getBiomeGen() const { return biomegen; } + // no usage restrictions const BiomeManager *getBiomeManager() const { return biomemgr; } const OreManager *getOreManager() const { return oremgr; } @@ -196,6 +200,7 @@ private: // Managers of various map generation-related components // Note that each Mapgen gets a copy(!) of these to work with + BiomeGen *biomegen; BiomeManager *biomemgr; OreManager *oremgr; DecorationManager *decomgr; diff --git a/src/mapgen/mapgen.cpp b/src/mapgen/mapgen.cpp index e0dfd2d71..7984ff609 100644 --- a/src/mapgen/mapgen.cpp +++ b/src/mapgen/mapgen.cpp @@ -595,7 +595,8 @@ MapgenBasic::MapgenBasic(int mapgenid, MapgenParams *params, EmergeParams *emerg this->heightmap = new s16[csize.X * csize.Z]; //// Initialize biome generator - biomegen = m_bmgr->createBiomeGen(BIOMEGEN_ORIGINAL, params->bparams, csize); + biomegen = emerge->biomegen; + biomegen->assertChunkSize(csize); biomemap = biomegen->biomemap; //// Look up some commonly used content @@ -621,7 +622,6 @@ MapgenBasic::MapgenBasic(int mapgenid, MapgenParams *params, EmergeParams *emerg MapgenBasic::~MapgenBasic() { - delete biomegen; delete []heightmap; delete m_emerge; // destroying EmergeParams is our responsibility diff --git a/src/mapgen/mapgen_valleys.cpp b/src/mapgen/mapgen_valleys.cpp index c4234857e..80a99b1f0 100644 --- a/src/mapgen/mapgen_valleys.cpp +++ b/src/mapgen/mapgen_valleys.cpp @@ -57,7 +57,8 @@ FlagDesc flagdesc_mapgen_valleys[] = { MapgenValleys::MapgenValleys(MapgenValleysParams *params, EmergeParams *emerge) : MapgenBasic(MAPGEN_VALLEYS, params, emerge) { - // NOTE: MapgenValleys has a hard dependency on BiomeGenOriginal + FATAL_ERROR_IF(biomegen->getType() != BIOMEGEN_ORIGINAL, + "MapgenValleys has a hard dependency on BiomeGenOriginal"); m_bgen = (BiomeGenOriginal *)biomegen; spflags = params->spflags; diff --git a/src/mapgen/mg_biome.cpp b/src/mapgen/mg_biome.cpp index 610c38594..f08cc190f 100644 --- a/src/mapgen/mg_biome.cpp +++ b/src/mapgen/mg_biome.cpp @@ -101,71 +101,6 @@ BiomeManager *BiomeManager::clone() const return mgr; } - -// For BiomeGen type 'BiomeGenOriginal' -float BiomeManager::getHeatAtPosOriginal(v3s16 pos, NoiseParams &np_heat, - NoiseParams &np_heat_blend, u64 seed) const -{ - return - NoisePerlin2D(&np_heat, pos.X, pos.Z, seed) + - NoisePerlin2D(&np_heat_blend, pos.X, pos.Z, seed); -} - - -// For BiomeGen type 'BiomeGenOriginal' -float BiomeManager::getHumidityAtPosOriginal(v3s16 pos, NoiseParams &np_humidity, - NoiseParams &np_humidity_blend, u64 seed) const -{ - return - NoisePerlin2D(&np_humidity, pos.X, pos.Z, seed) + - NoisePerlin2D(&np_humidity_blend, pos.X, pos.Z, seed); -} - - -// For BiomeGen type 'BiomeGenOriginal' -const Biome *BiomeManager::getBiomeFromNoiseOriginal(float heat, - float humidity, v3s16 pos) const -{ - Biome *biome_closest = nullptr; - Biome *biome_closest_blend = nullptr; - float dist_min = FLT_MAX; - float dist_min_blend = FLT_MAX; - - for (size_t i = 1; i < getNumObjects(); i++) { - Biome *b = (Biome *)getRaw(i); - if (!b || - pos.Y < b->min_pos.Y || pos.Y > b->max_pos.Y + b->vertical_blend || - pos.X < b->min_pos.X || pos.X > b->max_pos.X || - pos.Z < b->min_pos.Z || pos.Z > b->max_pos.Z) - continue; - - float d_heat = heat - b->heat_point; - float d_humidity = humidity - b->humidity_point; - float dist = (d_heat * d_heat) + (d_humidity * d_humidity); - - if (pos.Y <= b->max_pos.Y) { // Within y limits of biome b - if (dist < dist_min) { - dist_min = dist; - biome_closest = b; - } - } else if (dist < dist_min_blend) { // Blend area above biome b - dist_min_blend = dist; - biome_closest_blend = b; - } - } - - const u64 seed = pos.Y + (heat + humidity) * 0.9f; - PcgRandom rng(seed); - - if (biome_closest_blend && dist_min_blend <= dist_min && - rng.range(0, biome_closest_blend->vertical_blend) >= - pos.Y - biome_closest_blend->max_pos.Y) - return biome_closest_blend; - - return (biome_closest) ? biome_closest : (Biome *)getRaw(BIOME_NONE); -} - - //////////////////////////////////////////////////////////////////////////////// void BiomeParamsOriginal::readParams(const Settings *settings) @@ -189,7 +124,7 @@ void BiomeParamsOriginal::writeParams(Settings *settings) const //////////////////////////////////////////////////////////////////////////////// BiomeGenOriginal::BiomeGenOriginal(BiomeManager *biomemgr, - BiomeParamsOriginal *params, v3s16 chunksize) + const BiomeParamsOriginal *params, v3s16 chunksize) { m_bmgr = biomemgr; m_params = params; @@ -224,17 +159,26 @@ BiomeGenOriginal::~BiomeGenOriginal() delete noise_humidity_blend; } -// Only usable in a mapgen thread -Biome *BiomeGenOriginal::calcBiomeAtPoint(v3s16 pos) const +BiomeGen *BiomeGenOriginal::clone(BiomeManager *biomemgr) const +{ + return new BiomeGenOriginal(biomemgr, m_params, m_csize); +} + +float BiomeGenOriginal::calcHeatAtPoint(v3s16 pos) const { - float heat = - NoisePerlin2D(&m_params->np_heat, pos.X, pos.Z, m_params->seed) + + return NoisePerlin2D(&m_params->np_heat, pos.X, pos.Z, m_params->seed) + NoisePerlin2D(&m_params->np_heat_blend, pos.X, pos.Z, m_params->seed); - float humidity = - NoisePerlin2D(&m_params->np_humidity, pos.X, pos.Z, m_params->seed) + +} + +float BiomeGenOriginal::calcHumidityAtPoint(v3s16 pos) const +{ + return NoisePerlin2D(&m_params->np_humidity, pos.X, pos.Z, m_params->seed) + NoisePerlin2D(&m_params->np_humidity_blend, pos.X, pos.Z, m_params->seed); +} - return calcBiomeFromNoise(heat, humidity, pos); +Biome *BiomeGenOriginal::calcBiomeAtPoint(v3s16 pos) const +{ + return calcBiomeFromNoise(calcHeatAtPoint(pos), calcHumidityAtPoint(pos), pos); } diff --git a/src/mapgen/mg_biome.h b/src/mapgen/mg_biome.h index be4cfea4d..c85afc3a0 100644 --- a/src/mapgen/mg_biome.h +++ b/src/mapgen/mg_biome.h @@ -97,6 +97,15 @@ public: virtual BiomeGenType getType() const = 0; + // Clone this BiomeGen and set a the new BiomeManager to be used by the copy + virtual BiomeGen *clone(BiomeManager *biomemgr) const = 0; + + // Check that the internal chunk size is what the mapgen expects, just to be sure. + inline void assertChunkSize(v3s16 expect) const + { + FATAL_ERROR_IF(m_csize != expect, "Chunk size mismatches"); + } + // Calculates the biome at the exact position provided. This function can // be called at any time, but may be less efficient than the latter methods, // depending on implementation. @@ -158,12 +167,18 @@ struct BiomeParamsOriginal : public BiomeParams { class BiomeGenOriginal : public BiomeGen { public: BiomeGenOriginal(BiomeManager *biomemgr, - BiomeParamsOriginal *params, v3s16 chunksize); + const BiomeParamsOriginal *params, v3s16 chunksize); virtual ~BiomeGenOriginal(); BiomeGenType getType() const { return BIOMEGEN_ORIGINAL; } + BiomeGen *clone(BiomeManager *biomemgr) const; + + // Slower, meant for Script API use + float calcHeatAtPoint(v3s16 pos) const; + float calcHumidityAtPoint(v3s16 pos) const; Biome *calcBiomeAtPoint(v3s16 pos) const; + void calcBiomeNoise(v3s16 pmin); biome_t *getBiomes(s16 *heightmap, v3s16 pmin); @@ -176,7 +191,7 @@ public: float *humidmap; private: - BiomeParamsOriginal *m_params; + const BiomeParamsOriginal *m_params; Noise *noise_heat; Noise *noise_humidity; @@ -229,14 +244,6 @@ public: virtual void clear(); - // For BiomeGen type 'BiomeGenOriginal' - float getHeatAtPosOriginal(v3s16 pos, NoiseParams &np_heat, - NoiseParams &np_heat_blend, u64 seed) const; - float getHumidityAtPosOriginal(v3s16 pos, NoiseParams &np_humidity, - NoiseParams &np_humidity_blend, u64 seed) const; - const Biome *getBiomeFromNoiseOriginal(float heat, float humidity, - v3s16 pos) const; - private: BiomeManager() {}; diff --git a/src/noise.cpp b/src/noise.cpp index e16564b05..a10efa3c4 100644 --- a/src/noise.cpp +++ b/src/noise.cpp @@ -369,7 +369,7 @@ float contour(float v) ///////////////////////// [ New noise ] //////////////////////////// -float NoisePerlin2D(NoiseParams *np, float x, float y, s32 seed) +float NoisePerlin2D(const NoiseParams *np, float x, float y, s32 seed) { float a = 0; float f = 1.0; @@ -395,7 +395,7 @@ float NoisePerlin2D(NoiseParams *np, float x, float y, s32 seed) } -float NoisePerlin3D(NoiseParams *np, float x, float y, float z, s32 seed) +float NoisePerlin3D(const NoiseParams *np, float x, float y, float z, s32 seed) { float a = 0; float f = 1.0; @@ -422,7 +422,7 @@ float NoisePerlin3D(NoiseParams *np, float x, float y, float z, s32 seed) } -Noise::Noise(NoiseParams *np_, s32 seed, u32 sx, u32 sy, u32 sz) +Noise::Noise(const NoiseParams *np_, s32 seed, u32 sx, u32 sy, u32 sz) { np = *np_; this->seed = seed; diff --git a/src/noise.h b/src/noise.h index 613879890..854781731 100644 --- a/src/noise.h +++ b/src/noise.h @@ -146,7 +146,7 @@ public: float *persist_buf = nullptr; float *result = nullptr; - Noise(NoiseParams *np, s32 seed, u32 sx, u32 sy, u32 sz=1); + Noise(const NoiseParams *np, s32 seed, u32 sx, u32 sy, u32 sz=1); ~Noise(); void setSize(u32 sx, u32 sy, u32 sz=1); @@ -192,8 +192,8 @@ private: }; -float NoisePerlin2D(NoiseParams *np, float x, float y, s32 seed); -float NoisePerlin3D(NoiseParams *np, float x, float y, float z, s32 seed); +float NoisePerlin2D(const NoiseParams *np, float x, float y, s32 seed); +float NoisePerlin3D(const NoiseParams *np, float x, float y, float z, s32 seed); inline float NoisePerlin2D_PO(NoiseParams *np, float x, float xoff, float y, float yoff, s32 seed) diff --git a/src/script/lua_api/l_mapgen.cpp b/src/script/lua_api/l_mapgen.cpp index cc93bdbd0..eb3d49a5e 100644 --- a/src/script/lua_api/l_mapgen.cpp +++ b/src/script/lua_api/l_mapgen.cpp @@ -482,9 +482,7 @@ int ModApiMapgen::l_get_biome_id(lua_State *L) { NO_MAP_LOCK_REQUIRED; - const char *biome_str = lua_tostring(L, 1); - if (!biome_str) - return 0; + const char *biome_str = luaL_checkstring(L, 1); const BiomeManager *bmgr = getServer(L)->getEmergeManager()->getBiomeManager(); if (!bmgr) @@ -527,30 +525,12 @@ int ModApiMapgen::l_get_heat(lua_State *L) v3s16 pos = read_v3s16(L, 1); - NoiseParams np_heat; - NoiseParams np_heat_blend; - - MapSettingsManager *settingsmgr = - getServer(L)->getEmergeManager()->map_settings_mgr; + const BiomeGen *biomegen = getServer(L)->getEmergeManager()->getBiomeGen(); - if (!settingsmgr->getMapSettingNoiseParams("mg_biome_np_heat", - &np_heat) || - !settingsmgr->getMapSettingNoiseParams("mg_biome_np_heat_blend", - &np_heat_blend)) - return 0; - - std::string value; - if (!settingsmgr->getMapSetting("seed", &value)) - return 0; - std::istringstream ss(value); - u64 seed; - ss >> seed; - - const BiomeManager *bmgr = getServer(L)->getEmergeManager()->getBiomeManager(); - if (!bmgr) + if (!biomegen || biomegen->getType() != BIOMEGEN_ORIGINAL) return 0; - float heat = bmgr->getHeatAtPosOriginal(pos, np_heat, np_heat_blend, seed); + float heat = ((BiomeGenOriginal*) biomegen)->calcHeatAtPoint(pos); lua_pushnumber(L, heat); @@ -566,31 +546,12 @@ int ModApiMapgen::l_get_humidity(lua_State *L) v3s16 pos = read_v3s16(L, 1); - NoiseParams np_humidity; - NoiseParams np_humidity_blend; - - MapSettingsManager *settingsmgr = - getServer(L)->getEmergeManager()->map_settings_mgr; + const BiomeGen *biomegen = getServer(L)->getEmergeManager()->getBiomeGen(); - if (!settingsmgr->getMapSettingNoiseParams("mg_biome_np_humidity", - &np_humidity) || - !settingsmgr->getMapSettingNoiseParams("mg_biome_np_humidity_blend", - &np_humidity_blend)) + if (!biomegen || biomegen->getType() != BIOMEGEN_ORIGINAL) return 0; - std::string value; - if (!settingsmgr->getMapSetting("seed", &value)) - return 0; - std::istringstream ss(value); - u64 seed; - ss >> seed; - - const BiomeManager *bmgr = getServer(L)->getEmergeManager()->getBiomeManager(); - if (!bmgr) - return 0; - - float humidity = bmgr->getHumidityAtPosOriginal(pos, np_humidity, - np_humidity_blend, seed); + float humidity = ((BiomeGenOriginal*) biomegen)->calcHumidityAtPoint(pos); lua_pushnumber(L, humidity); @@ -606,45 +567,11 @@ int ModApiMapgen::l_get_biome_data(lua_State *L) v3s16 pos = read_v3s16(L, 1); - NoiseParams np_heat; - NoiseParams np_heat_blend; - NoiseParams np_humidity; - NoiseParams np_humidity_blend; - - MapSettingsManager *settingsmgr = - getServer(L)->getEmergeManager()->map_settings_mgr; - - if (!settingsmgr->getMapSettingNoiseParams("mg_biome_np_heat", - &np_heat) || - !settingsmgr->getMapSettingNoiseParams("mg_biome_np_heat_blend", - &np_heat_blend) || - !settingsmgr->getMapSettingNoiseParams("mg_biome_np_humidity", - &np_humidity) || - !settingsmgr->getMapSettingNoiseParams("mg_biome_np_humidity_blend", - &np_humidity_blend)) + const BiomeGen *biomegen = getServer(L)->getEmergeManager()->getBiomeGen(); + if (!biomegen) return 0; - std::string value; - if (!settingsmgr->getMapSetting("seed", &value)) - return 0; - std::istringstream ss(value); - u64 seed; - ss >> seed; - - const BiomeManager *bmgr = getServer(L)->getEmergeManager()->getBiomeManager(); - if (!bmgr) - return 0; - - float heat = bmgr->getHeatAtPosOriginal(pos, np_heat, np_heat_blend, seed); - if (!heat) - return 0; - - float humidity = bmgr->getHumidityAtPosOriginal(pos, np_humidity, - np_humidity_blend, seed); - if (!humidity) - return 0; - - const Biome *biome = bmgr->getBiomeFromNoiseOriginal(heat, humidity, pos); + const Biome *biome = biomegen->calcBiomeAtPoint(pos); if (!biome || biome->index == OBJDEF_INVALID_INDEX) return 0; @@ -653,11 +580,16 @@ int ModApiMapgen::l_get_biome_data(lua_State *L) lua_pushinteger(L, biome->index); lua_setfield(L, -2, "biome"); - lua_pushnumber(L, heat); - lua_setfield(L, -2, "heat"); + if (biomegen->getType() == BIOMEGEN_ORIGINAL) { + float heat = ((BiomeGenOriginal*) biomegen)->calcHeatAtPoint(pos); + float humidity = ((BiomeGenOriginal*) biomegen)->calcHumidityAtPoint(pos); - lua_pushnumber(L, humidity); - lua_setfield(L, -2, "humidity"); + lua_pushnumber(L, heat); + lua_setfield(L, -2, "heat"); + + lua_pushnumber(L, humidity); + lua_setfield(L, -2, "humidity"); + } return 1; } @@ -1493,9 +1425,12 @@ int ModApiMapgen::l_generate_ores(lua_State *L) NO_MAP_LOCK_REQUIRED; EmergeManager *emerge = getServer(L)->getEmergeManager(); + if (!emerge || !emerge->mgparams) + return 0; Mapgen mg; - mg.seed = emerge->mgparams->seed; + // Intentionally truncates to s32, see Mapgen::Mapgen() + mg.seed = (s32)emerge->mgparams->seed; mg.vm = LuaVoxelManip::checkobject(L, 1)->vm; mg.ndef = getServer(L)->getNodeDefManager(); @@ -1519,9 +1454,12 @@ int ModApiMapgen::l_generate_decorations(lua_State *L) NO_MAP_LOCK_REQUIRED; EmergeManager *emerge = getServer(L)->getEmergeManager(); + if (!emerge || !emerge->mgparams) + return 0; Mapgen mg; - mg.seed = emerge->mgparams->seed; + // Intentionally truncates to s32, see Mapgen::Mapgen() + mg.seed = (s32)emerge->mgparams->seed; mg.vm = LuaVoxelManip::checkobject(L, 1)->vm; mg.ndef = getServer(L)->getNodeDefManager(); -- cgit v1.2.3