MetaDataRef: Make set_float preserve numbers exactly (#16090)

doc/lua_api.md

@@ -8168,8 +8168,8 @@ of the `${k}` syntax in formspecs is not deprecated.
       The value will be converted into a string when stored.
 * `get_int(key)`: Returns `0` if key not present.
 * `set_float(key, value)`
-    * The range for the value is system-dependent (usually 32 bits).
+    * Store a number (a 64-bit float) exactly.
-      The value will be converted into a string when stored.
+    * The value will be converted into a string when stored.
 * `get_float(key)`: Returns `0` if key not present.
 * `get_keys()`: returns a list of all keys in the metadata.
 * `to_table()`:

games/devtest/mods/unittests/metadata.lua

@@ -8,6 +8,8 @@ compare_meta:from_table({
 		c = "3",
 		d = "4",
 		e = "e",
+		["0.3"] = "0.29999999999999999",
+		["0.1+0.2"] = "0.30000000000000004",
 	},
 })
 
@@ -21,6 +23,9 @@ local function test_metadata(meta)
 	meta:set_string("", "!")
 	meta:set_string("", "")
 
+	meta:set_float("0.3", 0.3)
+	meta:set_float("0.1+0.2", 0.1 + 0.2)
+
 	assert(meta:equals(compare_meta))
 
 	local tab = meta:to_table()
@@ -29,6 +34,8 @@ local function test_metadata(meta)
 	assert(tab.fields.c == "3")
 	assert(tab.fields.d == "4")
 	assert(tab.fields.e == "e")
+	assert(tab.fields["0.3"] == "0.29999999999999999")
+	assert(tab.fields["0.1+0.2"] == "0.30000000000000004")
 
 	local keys = meta:get_keys()
 	assert(table.indexof(keys, "a") > 0)
@@ -36,7 +43,7 @@ local function test_metadata(meta)
 	assert(table.indexof(keys, "c") > 0)
 	assert(table.indexof(keys, "d") > 0)
 	assert(table.indexof(keys, "e") > 0)
-	assert(#keys == 5)
+	assert(#keys == 7)
 
 	assert(not meta:contains(""))
 	assert(meta:contains("a"))
@@ -55,6 +62,8 @@ local function test_metadata(meta)
 	assert(meta:get_float("a") == 1.0)
 	assert(meta:get_int("e") == 0)
 	assert(meta:get_float("e") == 0.0)
+	assert(meta:get_float("0.3") == 0.3)
+	assert(meta:get_float("0.1+0.2") == 0.1 + 0.2)
 
 	meta:set_float("f", 1.1)
 	meta:set_string("g", "${f}")

src/script/lua_api/l_metadata.cpp

@@ -10,6 +10,7 @@
 #include "map.h"
 #include "server.h"
 #include "util/basic_macros.h"
+#include "util/string.h"
 
 MetaDataRef *MetaDataRef::checkAnyMetadata(lua_State *L, int narg)
 {
@@ -166,9 +167,9 @@ int MetaDataRef::l_get_float(lua_State *L)
 
 	std::string str_;
 	const std::string &str = meta->getString(name, &str_);
-	// Convert with Lua, as is done in set_float.
+	// TODO this silently produces 0.0 if conversion fails, which is a footgun
-	lua_pushlstring(L, str.data(), str.size());
+	f64 number = my_string_to_double(str).value_or(0.0);
-	lua_pushnumber(L, lua_tonumber(L, -1));
+	lua_pushnumber(L, number);
 	return 1;
 }
 
@@ -179,12 +180,11 @@ int MetaDataRef::l_set_float(lua_State *L)
 
 	MetaDataRef *ref = checkAnyMetadata(L, 1);
 	std::string name = luaL_checkstring(L, 2);
-	luaL_checknumber(L, 3);
+	f64 number = luaL_checknumber(L, 3);
-	// Convert number to string with Lua as it gives good precision.
-	std::string str = readParam<std::string>(L, 3);
 
 	IMetadata *meta = ref->getmeta(true);
-	if (meta != NULL && meta->setString(name, str))
+	// Note: Do not use Lua's tostring for the conversion - it rounds.
+	if (meta != NULL && meta->setString(name, my_double_to_string(number)))
 		ref->reportMetadataChange(&name);
 	return 0;
 }
@@ -289,8 +289,11 @@ bool MetaDataRef::handleFromTable(lua_State *L, int table, IMetadata *meta)
 		while (lua_next(L, fieldstable) != 0) {
 			// key at index -2 and value at index -1
 			std::string name = readParam<std::string>(L, -2);
-			auto value = readParam<std::string_view>(L, -1);
+			if (lua_type(L, -1) == LUA_TNUMBER) {
-			meta->setString(name, value);
+				log_deprecated(L, "Passing `fields` with number values "
+					"is deprecated and may result in loss of precision.");
+			}
+			meta->setString(name, readParam<std::string_view>(L, -1));
 			lua_pop(L, 1); // Remove value, keep key for next iteration
 		}
 		lua_pop(L, 1);

src/unittest/test_utilities.cpp

@@ -5,6 +5,7 @@
 #include "test.h"
 
 #include <cmath>
+#include <limits>
 #include "util/enriched_string.h"
 #include "util/numeric.h"
 #include "util/string.h"
@@ -48,6 +49,7 @@ public:
 	void testColorizeURL();
 	void testSanitizeUntrusted();
 	void testReadSeed();
+	void testMyDoubleStringConversions();
 };
 
 static TestUtilities g_test_instance;
@@ -84,6 +86,7 @@ void TestUtilities::runTests(IGameDef *gamedef)
 	TEST(testColorizeURL);
 	TEST(testSanitizeUntrusted);
 	TEST(testReadSeed);
+	TEST(testMyDoubleStringConversions);
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -763,3 +766,37 @@ void TestUtilities::testReadSeed()
 	// hashing should produce some non-zero number
 	UASSERT(read_seed("hello") != 0);
 }
+
+void TestUtilities::testMyDoubleStringConversions()
+{
+	const auto expect_parse_failure = [](const std::string &s) {
+		UASSERT(!my_string_to_double(s).has_value());
+	};
+	expect_parse_failure("");
+	expect_parse_failure("helloworld");
+	expect_parse_failure("42x");
+
+	const auto expect_double = [](const std::string &s, double expected) {
+		auto got = my_string_to_double(s);
+		UASSERT(got.has_value());
+		UASSERTEQ(double, *got, expected);
+	};
+	expect_double("1", 1.0);
+	expect_double("42", 42.0);
+	expect_double("42.25", 42.25);
+	expect_double("3e3", 3000.0);
+	expect_double("0xff", 255.0);
+	expect_double("0x1.0p+1", 2.0);
+
+	UASSERT(std::isnan(my_string_to_double(my_double_to_string(
+			std::numeric_limits<double>::quiet_NaN())).value()));
+	const auto test_round_trip = [](double number) {
+		auto got = my_string_to_double(my_double_to_string(number));
+		UASSERT(got.has_value());
+		UASSERTEQ(double, *got, number);
+	};
+	test_round_trip(std::numeric_limits<double>::infinity());
+	test_round_trip(-std::numeric_limits<double>::infinity());
+	test_round_trip(0.3);
+	test_round_trip(0.1 + 0.2);
+}

src/util/string.cpp

@@ -1065,3 +1065,30 @@ std::optional<v3f> str_to_v3f(std::string_view str)
 
 	return value;
 }
+
+std::string my_double_to_string(double number)
+{
+	if (std::isfinite(number)) {
+		char buf[64];
+		snprintf(buf, sizeof(buf), "%.17g", number);
+		return buf;
+	}
+	if (number < 0)
+		return "-inf";
+	if (number > 0)
+		return "inf";
+	return "nan";
+}
+
+std::optional<double> my_string_to_double(const std::string &s)
+{
+	if (s.empty())
+		return std::nullopt;
+	char *end = nullptr;
+	errno = 0;
+	// Note: this also supports hexadecimal notation like "0x1.0p+1"
+	double number = std::strtod(s.data(), &end);
+	if (end != &*s.end())
+		return std::nullopt;
+	return number;
+}

src/util/string.h

@@ -465,6 +465,10 @@ inline std::string ftos(float f)
 	return oss.str();
 }
 
+/// @brief Converts double to string. Handles high precision and inf/nan.
+std::string my_double_to_string(double number);
+/// @brief Converts string to double. Handles high precision and inf/nan.
+std::optional<double> my_string_to_double(const std::string &s);
 
 /**
  * Replace all occurrences of \p pattern in \p str with \p replacement.