Make string to v3f parsing consistent, replace core.setting_get_pos() by core.settings:get_pos() (#15438)

Co-authored-by: sfan5 <sfan5@live.de>
Co-authored-by: Lars Müller <34514239+appgurueu@users.noreply.github.com>

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);

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 {

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)));

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)),

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)

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<v3f> mgfractal_scale;
+    if (settings->getV3FNoEx("mgfractal_scale", mgfractal_scale) && mgfractal_scale.has_value()) {
+        scale = *mgfractal_scale;
+    }
+
+    std::optional<v3f> 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);

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<LuaSettings>(L, 1);
+    std::string key = luaL_checkstring(L, 2);
+
+    std::optional<v3f> 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<LuaSettings>(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),

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);
 

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<v3f> 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')

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<v3f> 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<v3f> 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<v3f> &val) const
 {
 	try {
 		val = getV3F(name);

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<v3f> 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<v3f> &val) const;
 
 	// Like other getters, but handling each flag individualy:
 	// 1) Read default flags (or 0)

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<v3f> 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<v3f> testNoParenthesis = s.getV3F("coord_no_parenthesis");
+    UASSERT(testNoParenthesis.value() == v3f(1, 2, 3));
 
 	// Test settings groups
 	Settings *group = s.getGroup("asdf");

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<v3f> 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;
 }

src/util/string.h

@@ -20,6 +20,7 @@
 #include <cctype>
 #include <cwctype>
 #include <unordered_map>
+#include <optional>
 
 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<v3f> str_to_v3f(std::string_view str);