From e545e96d2bc146765b3d663b24ab0fd7bf85abbd Mon Sep 17 00:00:00 2001 From: AFCMS Date: Wed, 4 Dec 2024 18:19:46 +0100 Subject: [PATCH] Make string to v3f parsing consistent, replace `core.setting_get_pos()` by `core.settings:get_pos()` (#15438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: sfan5 Co-authored-by: Lars Müller <34514239+appgurueu@users.noreply.github.com> --- builtin/game/misc_s.lua | 6 +--- builtin/game/static_spawn.lua | 2 +- doc/lua_api.md | 8 ++++- src/client/hud.cpp | 4 +-- src/database/database-files.cpp | 2 +- src/gui/guiChatConsole.cpp | 2 +- src/gui/guiFormSpecMenu.cpp | 2 +- src/inventory.cpp | 4 +-- src/mapgen/mapgen_fractal.cpp | 13 ++++++-- src/script/lua_api/l_settings.cpp | 34 +++++++++++++++++++++ src/script/lua_api/l_settings.h | 6 ++++ src/server.cpp | 11 +++++-- src/settings.cpp | 10 +++++-- src/settings.h | 4 +-- src/unittest/test_settings.cpp | 49 +++++++++++++++++++++++++------ src/util/string.cpp | 45 ++++++++++++++++++++++------ src/util/string.h | 5 ++-- 17 files changed, 162 insertions(+), 45 deletions(-) diff --git a/builtin/game/misc_s.lua b/builtin/game/misc_s.lua index 90092952d..07ab09b37 100644 --- a/builtin/game/misc_s.lua +++ b/builtin/game/misc_s.lua @@ -36,11 +36,7 @@ end function core.setting_get_pos(name) - local value = core.settings:get(name) - if not value then - return nil - end - return core.string_to_pos(value) + return core.settings:get_pos(name) end diff --git a/builtin/game/static_spawn.lua b/builtin/game/static_spawn.lua index 5b834310f..2d535251a 100644 --- a/builtin/game/static_spawn.lua +++ b/builtin/game/static_spawn.lua @@ -1,7 +1,7 @@ local static_spawnpoint_string = core.settings:get("static_spawnpoint") if static_spawnpoint_string and static_spawnpoint_string ~= "" and - not core.setting_get_pos("static_spawnpoint") then + not core.settings:get_pos("static_spawnpoint") then error('The static_spawnpoint setting is invalid: "' .. static_spawnpoint_string .. '"') end diff --git a/doc/lua_api.md b/doc/lua_api.md index 709bf2984..2d5405090 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -6180,7 +6180,7 @@ Setting-related * `core.settings`: Settings object containing all of the settings from the main config file (`minetest.conf`). See [`Settings`]. * `core.setting_get_pos(name)`: Loads a setting from the main settings and - parses it as a position (in the format `(1,2,3)`). Returns a position or nil. + parses it as a position (in the format `(1,2,3)`). Returns a position or nil. **Deprecated: use `core.settings:get_pos()` instead** Authentication -------------- @@ -9051,6 +9051,9 @@ means that no defaults will be returned for mod settings. * Is currently limited to mapgen flags `mg_flags` and mapgen-specific flags like `mgv5_spflags`. * Returns `nil` if `key` is not found. +* `get_pos(key)`: + * Returns a `vector` + * Returns `nil` if no value is found or parsing failed. * `set(key, value)` * Setting names can't contain whitespace or any of `="{}#`. * Setting values can't contain the sequence `\n"""`. @@ -9061,6 +9064,9 @@ means that no defaults will be returned for mod settings. * `set_np_group(key, value)` * `value` is a NoiseParams table. * Also, see documentation for `set()` above. +* `set_pos(key, value)` + * `value` is a `vector`. + * Also, see documentation for `set()` above. * `remove(key)`: returns a boolean (`true` for success) * `get_names()`: returns `{key1,...}` * `has(key)`: diff --git a/src/client/hud.cpp b/src/client/hud.cpp index e49327d30..4fbd8740f 100644 --- a/src/client/hud.cpp +++ b/src/client/hud.cpp @@ -55,14 +55,14 @@ Hud::Hud(Client *client, LocalPlayer *player, tsrc = client->getTextureSource(); - v3f crosshair_color = g_settings->getV3F("crosshair_color"); + v3f crosshair_color = g_settings->getV3F("crosshair_color").value_or(v3f()); u32 cross_r = rangelim(myround(crosshair_color.X), 0, 255); u32 cross_g = rangelim(myround(crosshair_color.Y), 0, 255); u32 cross_b = rangelim(myround(crosshair_color.Z), 0, 255); u32 cross_a = rangelim(g_settings->getS32("crosshair_alpha"), 0, 255); crosshair_argb = video::SColor(cross_a, cross_r, cross_g, cross_b); - v3f selectionbox_color = g_settings->getV3F("selectionbox_color"); + v3f selectionbox_color = g_settings->getV3F("selectionbox_color").value_or(v3f()); u32 sbox_r = rangelim(myround(selectionbox_color.X), 0, 255); u32 sbox_g = rangelim(myround(selectionbox_color.Y), 0, 255); u32 sbox_b = rangelim(myround(selectionbox_color.Z), 0, 255); diff --git a/src/database/database-files.cpp b/src/database/database-files.cpp index e19d51108..5001a2810 100644 --- a/src/database/database-files.cpp +++ b/src/database/database-files.cpp @@ -43,7 +43,7 @@ void PlayerDatabaseFiles::deSerialize(RemotePlayer *p, std::istream &is, } try { - sao->setBasePosition(args.getV3F("position")); + sao->setBasePosition(args.getV3F("position").value_or(v3f())); } catch (SettingNotFoundException &e) {} try { diff --git a/src/gui/guiChatConsole.cpp b/src/gui/guiChatConsole.cpp index 28aea66c1..689ad22e0 100644 --- a/src/gui/guiChatConsole.cpp +++ b/src/gui/guiChatConsole.cpp @@ -55,7 +55,7 @@ GUIChatConsole::GUIChatConsole( m_background_color.setGreen(255); m_background_color.setBlue(255); } else { - v3f console_color = g_settings->getV3F("console_color"); + v3f console_color = g_settings->getV3F("console_color").value_or(v3f()); m_background_color.setRed(clamp_u8(myround(console_color.X))); m_background_color.setGreen(clamp_u8(myround(console_color.Y))); m_background_color.setBlue(clamp_u8(myround(console_color.Z))); diff --git a/src/gui/guiFormSpecMenu.cpp b/src/gui/guiFormSpecMenu.cpp index ed1c3009d..2330f7c32 100644 --- a/src/gui/guiFormSpecMenu.cpp +++ b/src/gui/guiFormSpecMenu.cpp @@ -3010,7 +3010,7 @@ void GUIFormSpecMenu::regenerateGui(v2u32 screensize) m_tabheader_upper_edge = 0; { - v3f formspec_bgcolor = g_settings->getV3F("formspec_fullscreen_bg_color"); + v3f formspec_bgcolor = g_settings->getV3F("formspec_fullscreen_bg_color").value_or(v3f()); m_fullscreen_bgcolor = video::SColor( (u8) clamp_u8(g_settings->getS32("formspec_fullscreen_bg_opacity")), clamp_u8(myround(formspec_bgcolor.X)), diff --git a/src/inventory.cpp b/src/inventory.cpp index 0ba59ceea..246086d95 100644 --- a/src/inventory.cpp +++ b/src/inventory.cpp @@ -295,10 +295,8 @@ std::string ItemStack::getWieldOverlay(const IItemDefManager *itemdef) const v3f ItemStack::getWieldScale(const IItemDefManager *itemdef) const { std::string scale = metadata.getString("wield_scale"); - if (scale.empty()) - return getDefinition(itemdef).wield_scale; - return str_to_v3f(scale); + return str_to_v3f(scale).value_or(getDefinition(itemdef).wield_scale); } ItemStack ItemStack::addItem(ItemStack newitem, IItemDefManager *itemdef) diff --git a/src/mapgen/mapgen_fractal.cpp b/src/mapgen/mapgen_fractal.cpp index dcd62f63a..940fe0083 100644 --- a/src/mapgen/mapgen_fractal.cpp +++ b/src/mapgen/mapgen_fractal.cpp @@ -103,8 +103,17 @@ void MapgenFractalParams::readParams(const Settings *settings) settings->getS16NoEx("mgfractal_dungeon_ymax", dungeon_ymax); settings->getU16NoEx("mgfractal_fractal", fractal); settings->getU16NoEx("mgfractal_iterations", iterations); - settings->getV3FNoEx("mgfractal_scale", scale); - settings->getV3FNoEx("mgfractal_offset", offset); + + std::optional mgfractal_scale; + if (settings->getV3FNoEx("mgfractal_scale", mgfractal_scale) && mgfractal_scale.has_value()) { + scale = *mgfractal_scale; + } + + std::optional mgfractal_offset; + if (settings->getV3FNoEx("mgfractal_offset", mgfractal_offset) && mgfractal_offset.has_value()) { + offset = *mgfractal_offset; + } + settings->getFloatNoEx("mgfractal_slice_w", slice_w); settings->getFloatNoEx("mgfractal_julia_x", julia_x); settings->getFloatNoEx("mgfractal_julia_y", julia_y); diff --git a/src/script/lua_api/l_settings.cpp b/src/script/lua_api/l_settings.cpp index c4395e390..241eb2542 100644 --- a/src/script/lua_api/l_settings.cpp +++ b/src/script/lua_api/l_settings.cpp @@ -10,6 +10,7 @@ #include "settings.h" #include "noise.h" #include "log.h" +#include "common/c_converter.h" /* @@ -177,6 +178,21 @@ int LuaSettings::l_get_flags(lua_State *L) return 1; } +// get_pos(self, key) -> vector or nil +int LuaSettings::l_get_pos(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + LuaSettings *o = checkObject(L, 1); + std::string key = luaL_checkstring(L, 2); + + std::optional pos; + if (o->m_settings->getV3FNoEx(key, pos) && pos.has_value()) + push_v3f(L, *pos); + else + lua_pushnil(L); + return 1; +} + // set(self, key, value) int LuaSettings::l_set(lua_State* L) { @@ -227,6 +243,22 @@ int LuaSettings::l_set_np_group(lua_State *L) return 0; } +// set_pos(self, key, value) +int LuaSettings::l_set_pos(lua_State *L) +{ + NO_MAP_LOCK_REQUIRED; + LuaSettings *o = checkObject(L, 1); + + std::string key = luaL_checkstring(L, 2); + v3f value = check_v3f(L, 3); + + CHECK_SETTING_SECURITY(L, key); + + o->m_settings->setV3F(key, value); + + return 0; +} + // remove(self, key) -> success int LuaSettings::l_remove(lua_State* L) { @@ -356,9 +388,11 @@ const luaL_Reg LuaSettings::methods[] = { luamethod(LuaSettings, get_bool), luamethod(LuaSettings, get_np_group), luamethod(LuaSettings, get_flags), + luamethod(LuaSettings, get_pos), luamethod(LuaSettings, set), luamethod(LuaSettings, set_bool), luamethod(LuaSettings, set_np_group), + luamethod(LuaSettings, set_pos), luamethod(LuaSettings, remove), luamethod(LuaSettings, get_names), luamethod(LuaSettings, has), diff --git a/src/script/lua_api/l_settings.h b/src/script/lua_api/l_settings.h index ca609fc1f..63741477a 100644 --- a/src/script/lua_api/l_settings.h +++ b/src/script/lua_api/l_settings.h @@ -29,6 +29,9 @@ private: // get_flags(self, key) -> key/value table static int l_get_flags(lua_State *L); + // get_pos(self, key) -> vector or nil + static int l_get_pos(lua_State *L); + // set(self, key, value) static int l_set(lua_State *L); @@ -38,6 +41,9 @@ private: // set_np_group(self, key, value) static int l_set_np_group(lua_State *L); + // set_pos(self, key, value) + static int l_set_pos(lua_State *L); + // remove(self, key) -> success static int l_remove(lua_State *L); diff --git a/src/server.cpp b/src/server.cpp index ee9b0a859..b21955170 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -3863,9 +3863,14 @@ void Server::addShutdownError(const ModError &e) v3f Server::findSpawnPos() { ServerMap &map = m_env->getServerMap(); - v3f nodeposf; - if (g_settings->getV3FNoEx("static_spawnpoint", nodeposf)) - return nodeposf * BS; + + std::optional staticSpawnPoint; + if (g_settings->getV3FNoEx("static_spawnpoint", staticSpawnPoint) && staticSpawnPoint.has_value()) + { + return *staticSpawnPoint * BS; + } + + v3f nodeposf; bool is_good = false; // Limit spawn range to mapgen edges (determined by 'mapgen_limit') diff --git a/src/settings.cpp b/src/settings.cpp index 02c84437d..cec3f5239 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -541,7 +541,7 @@ v2f Settings::getV2F(const std::string &name) const } -v3f Settings::getV3F(const std::string &name) const +std::optional Settings::getV3F(const std::string &name) const { return str_to_v3f(get(name)); } @@ -626,7 +626,11 @@ bool Settings::getNoiseParamsFromGroup(const std::string &name, group->getFloatNoEx("offset", np.offset); group->getFloatNoEx("scale", np.scale); - group->getV3FNoEx("spread", np.spread); + + std::optional spread; + if (group->getV3FNoEx("spread", spread) && spread.has_value()) + np.spread = *spread; + group->getS32NoEx("seed", np.seed); group->getU16NoEx("octaves", np.octaves); group->getFloatNoEx("persistence", np.persist); @@ -783,7 +787,7 @@ bool Settings::getV2FNoEx(const std::string &name, v2f &val) const } -bool Settings::getV3FNoEx(const std::string &name, v3f &val) const +bool Settings::getV3FNoEx(const std::string &name, std::optional &val) const { try { val = getV3F(name); diff --git a/src/settings.h b/src/settings.h index 0638dad2d..9bc0e80d9 100644 --- a/src/settings.h +++ b/src/settings.h @@ -150,7 +150,7 @@ public: float getFloat(const std::string &name) const; float getFloat(const std::string &name, float min, float max) const; v2f getV2F(const std::string &name) const; - v3f getV3F(const std::string &name) const; + std::optional getV3F(const std::string &name) const; u32 getFlagStr(const std::string &name, const FlagDesc *flagdesc, u32 *flagmask) const; bool getNoiseParams(const std::string &name, NoiseParams &np) const; @@ -179,7 +179,7 @@ public: bool getU64NoEx(const std::string &name, u64 &val) const; bool getFloatNoEx(const std::string &name, float &val) const; bool getV2FNoEx(const std::string &name, v2f &val) const; - bool getV3FNoEx(const std::string &name, v3f &val) const; + bool getV3FNoEx(const std::string &name, std::optional &val) const; // Like other getters, but handling each flag individualy: // 1) Read default flags (or 0) diff --git a/src/unittest/test_settings.cpp b/src/unittest/test_settings.cpp index 94b222c1f..411ecb8de 100644 --- a/src/unittest/test_settings.cpp +++ b/src/unittest/test_settings.cpp @@ -42,6 +42,14 @@ const char *TestSettings::config_text_before = "floaty_thing = 1.1\n" "stringy_thing = asd /( ¤%&(/\" BLÖÄRP\n" "coord = (1, 2, 4.5)\n" + "coord_invalid = (1,2,3\n" + "coord_invalid_2 = 1, 2, 3 test\n" + "coord_invalid_3 = (test, something, stupid)\n" + "coord_invalid_4 = (1, test, 3)\n" + "coord_invalid_5 = ()\n" + "coord_invalid_6 = (1, 2)\n" + "coord_invalid_7 = (1)\n" + "coord_no_parenthesis = 1,2,3\n" " # this is just a comment\n" "this is an invalid line\n" "asdf = {\n" @@ -94,7 +102,15 @@ const char *TestSettings::config_text_after = " spread = (250,250,250)\n" "}\n" "zoop = true\n" - "coord2 = (1,2,3.3)\n" + "coord2 = (1,2,3.25)\n" + "coord_invalid = (1,2,3\n" + "coord_invalid_2 = 1, 2, 3 test\n" + "coord_invalid_3 = (test, something, stupid)\n" + "coord_invalid_4 = (1, test, 3)\n" + "coord_invalid_5 = ()\n" + "coord_invalid_6 = (1, 2)\n" + "coord_invalid_7 = (1)\n" + "coord_no_parenthesis = 1,2,3\n" "floaty_thing_2 = 1.25\n" "groupy_thing = {\n" " animals = cute\n" @@ -140,18 +156,33 @@ void TestSettings::testAllSettings() // Not sure if 1.1 is an exact value as a float, but doesn't matter UASSERT(fabs(s.getFloat("floaty_thing") - 1.1) < 0.001); UASSERT(s.get("stringy_thing") == u8"asd /( ¤%&(/\" BLÖÄRP"); - UASSERT(fabs(s.getV3F("coord").X - 1.0) < 0.001); - UASSERT(fabs(s.getV3F("coord").Y - 2.0) < 0.001); - UASSERT(fabs(s.getV3F("coord").Z - 4.5) < 0.001); + UASSERT(s.getV3F("coord").value().X == 1.0); + UASSERT(s.getV3F("coord").value().Y == 2.0); + UASSERT(s.getV3F("coord").value().Z == 4.5); // Test the setting of settings too s.setFloat("floaty_thing_2", 1.25); - s.setV3F("coord2", v3f(1, 2, 3.3)); + s.setV3F("coord2", v3f(1, 2, 3.25)); UASSERT(s.get("floaty_thing_2").substr(0,4) == "1.25"); - UASSERT(fabs(s.getFloat("floaty_thing_2") - 1.25) < 0.001); - UASSERT(fabs(s.getV3F("coord2").X - 1.0) < 0.001); - UASSERT(fabs(s.getV3F("coord2").Y - 2.0) < 0.001); - UASSERT(fabs(s.getV3F("coord2").Z - 3.3) < 0.001); + UASSERT(s.getFloat("floaty_thing_2") == 1.25); + UASSERT(s.getV3F("coord2").value().X == 1.0); + UASSERT(s.getV3F("coord2").value().Y == 2.0); + UASSERT(s.getV3F("coord2").value().Z == 3.25); + + std::optional testNotExist; + UASSERT(!s.getV3FNoEx("coord_not_exist", testNotExist)); + EXCEPTION_CHECK(SettingNotFoundException, s.getV3F("coord_not_exist")); + + UASSERT(!s.getV3F("coord_invalid").has_value()); + UASSERT(!s.getV3F("coord_invalid_2").has_value()); + UASSERT(!s.getV3F("coord_invalid_3").has_value()); + UASSERT(!s.getV3F("coord_invalid_4").has_value()); + UASSERT(!s.getV3F("coord_invalid_5").has_value()); + UASSERT(!s.getV3F("coord_invalid_6").has_value()); + UASSERT(!s.getV3F("coord_invalid_7").has_value()); + + std::optional testNoParenthesis = s.getV3F("coord_no_parenthesis"); + UASSERT(testNoParenthesis.value() == v3f(1, 2, 3)); // Test settings groups Settings *group = s.getGroup("asdf"); diff --git a/src/util/string.cpp b/src/util/string.cpp index 43e936788..a0eee85c1 100644 --- a/src/util/string.cpp +++ b/src/util/string.cpp @@ -1068,14 +1068,41 @@ void safe_print_string(std::ostream &os, std::string_view str) os.setf(flags); } - -v3f str_to_v3f(std::string_view str) +std::optional str_to_v3f(std::string_view str) { - v3f value; - Strfnd f(str); - f.next("("); - value.X = stof(f.next(",")); - value.Y = stof(f.next(",")); - value.Z = stof(f.next(")")); - return value; + str = trim(str); + + if (str.empty()) + return std::nullopt; + + // Strip parentheses if they exist + if (str.front() == '(' && str.back() == ')') { + str.remove_prefix(1); + str.remove_suffix(1); + str = trim(str); + } + + std::istringstream iss((std::string(str))); + + const auto expect_delimiter = [&]() { + const auto c = iss.get(); + return c == ' ' || c == ','; + }; + + v3f value; + if (!(iss >> value.X)) + return std::nullopt; + if (!expect_delimiter()) + return std::nullopt; + if (!(iss >> value.Y)) + return std::nullopt; + if (!expect_delimiter()) + return std::nullopt; + if (!(iss >> value.Z)) + return std::nullopt; + + if (!iss.eof()) + return std::nullopt; + + return value; } diff --git a/src/util/string.h b/src/util/string.h index 1874ccd2a..16c337b7f 100644 --- a/src/util/string.h +++ b/src/util/string.h @@ -20,6 +20,7 @@ #include #include #include +#include class Translations; @@ -789,9 +790,9 @@ std::string sanitize_untrusted(std::string_view str, bool keep_escapes = true); void safe_print_string(std::ostream &os, std::string_view str); /** - * Parses a string of form `(1, 2, 3)` to a v3f + * Parses a string of form `(1, 2, 3)` or `1, 2, 4` to a v3f * * @param str string * @return float vector */ -v3f str_to_v3f(std::string_view str); +std::optional str_to_v3f(std::string_view str);