Avoid duplication of mod metadata in memory (#12562)

Co-authored-by: sfan5 <sfan5@live.de>

builtin/game/features.lua

@@ -25,6 +25,7 @@ core.features = {
 	dynamic_add_media_table = true,
 	get_sky_as_table = true,
 	get_light_data_buffer = true,
+	mod_storage_on_disk = true,
 }
 
 function core.has_feature(arg)

doc/lua_api.txt

@@ -4904,6 +4904,10 @@ Utilities
           get_sky_as_table = true,
           -- VoxelManip:get_light_data accepts an optional buffer argument (5.7.0)
           get_light_data_buffer = true,
+          -- When using a mod storage backend that is not "files" or "dummy",
+          -- the amount of data in mod storage is not constrained by
+          -- the amount of RAM available. (5.7.0)
+          mod_storage_on_disk = true,
       }
 
 * `minetest.has_feature(arg)`: returns `boolean, missing_features`

games/devtest/mods/unittests/init.lua

@@ -179,6 +179,7 @@ dofile(modpath .. "/itemdescription.lua")
 dofile(modpath .. "/async_env.lua")
 dofile(modpath .. "/entity.lua")
 dofile(modpath .. "/content_ids.lua")
+dofile(modpath .. "/metadata.lua")
 
 --------------
 

games/devtest/mods/unittests/metadata.lua

@@ -0,0 +1,79 @@
+-- Tests of generic and specific metadata functionality
+
+local compare_meta = ItemStack("unittests:iron_lump"):get_meta()
+compare_meta:from_table({
+	fields = {
+		a = "1",
+		b = "2",
+		c = "3",
+		d = "4",
+		e = "e",
+	},
+})
+
+local function test_metadata(meta)
+	meta:from_table({fields = {a = 1, b = "2"}})
+	meta:set_string("c", "3")
+	meta:set_int("d", 4)
+	meta:set_string("e", "e")
+
+	meta:set_string("", "!")
+	meta:set_string("", "")
+
+	assert(meta:equals(compare_meta))
+
+	local tab = meta:to_table()
+	assert(tab.fields.a == "1")
+	assert(tab.fields.b == "2")
+	assert(tab.fields.c == "3")
+	assert(tab.fields.d == "4")
+	assert(tab.fields.e == "e")
+
+	assert(not meta:contains(""))
+	assert(meta:contains("a"))
+	assert(meta:contains("b"))
+	assert(meta:contains("c"))
+	assert(meta:contains("d"))
+	assert(meta:contains("e"))
+
+	assert(meta:get("") == nil)
+	assert(meta:get_string("") == "")
+	assert(meta:get_int("") == 0)
+	assert(meta:get_float("") == 0.0)
+	assert(meta:get("a") == "1")
+	assert(meta:get_string("a") == "1")
+	assert(meta:get_int("a") == 1)
+	assert(meta:get_float("a") == 1.0)
+	assert(meta:get_int("e") == 0)
+	assert(meta:get_float("e") == 0.0)
+
+	meta:set_float("f", 1.1)
+	meta:set_string("g", "${f}")
+	meta:set_string("h", "${g}")
+	meta:set_string("i", "${h}")
+	assert(meta:get_float("h") > 1)
+	assert(meta:get_string("i") == "${f}")
+
+	meta:from_table()
+	assert(next(meta:to_table().fields) == nil)
+
+	assert(not meta:equals(compare_meta))
+end
+
+local storage_a = core.get_mod_storage()
+local storage_b = core.get_mod_storage()
+local function test_mod_storage()
+	assert(rawequal(storage_a, storage_b))
+	test_metadata(storage_a)
+end
+unittests.register("test_mod_storage", test_mod_storage)
+
+local function test_item_metadata()
+	test_metadata(ItemStack("unittest:coal_lump"):get_meta())
+end
+unittests.register("test_item_metadata", test_item_metadata)
+
+local function test_node_metadata(player, pos)
+	test_metadata(minetest.get_meta(pos))
+end
+unittests.register("test_node_metadata", test_node_metadata, {map=true})

src/client/client.cpp

@@ -1991,26 +1991,6 @@ const std::string* Client::getModFile(std::string filename)
 	return &it->second;
 }
 
-bool Client::registerModStorage(ModMetadata *storage)
-{
-	if (m_mod_storages.find(storage->getModName()) != m_mod_storages.end()) {
-		errorstream << "Unable to register same mod storage twice. Storage name: "
-				<< storage->getModName() << std::endl;
-		return false;
-	}
-
-	m_mod_storages[storage->getModName()] = storage;
-	return true;
-}
-
-void Client::unregisterModStorage(const std::string &name)
-{
-	std::unordered_map<std::string, ModMetadata *>::const_iterator it =
-		m_mod_storages.find(name);
-	if (it != m_mod_storages.end())
-		m_mod_storages.erase(name);
-}
-
 /*
  * Mod channels
  */

src/client/client.h

@@ -383,9 +383,6 @@ public:
 	const std::string* getModFile(std::string filename);
 	ModMetadataDatabase *getModStorageDatabase() override { return m_mod_storage_database; }
 
-	bool registerModStorage(ModMetadata *meta) override;
-	void unregisterModStorage(const std::string &name) override;
-
 	// Migrates away old files-based mod storage if necessary
 	void migrateModStorage();
 
@@ -593,7 +590,6 @@ private:
 
 	// Client modding
 	ClientScripting *m_script = nullptr;
-	std::unordered_map<std::string, ModMetadata *> m_mod_storages;
 	ModMetadataDatabase *m_mod_storage_database = nullptr;
 	float m_mod_storage_save_timer = 10.0f;
 	std::vector<ModSpec> m_mods;

src/content/mods.cpp

@@ -220,26 +220,34 @@ std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods)
 ModMetadata::ModMetadata(const std::string &mod_name, ModMetadataDatabase *database):
 	m_mod_name(mod_name), m_database(database)
 {
-	m_database->getModEntries(m_mod_name, &m_stringvars);
 }
 
 void ModMetadata::clear()
 {
-	for (const auto &pair : m_stringvars) {
-		m_database->removeModEntry(m_mod_name, pair.first);
+	m_database->removeModEntries(m_mod_name);
 }
-	Metadata::clear();
+
+bool ModMetadata::contains(const std::string &name) const
+{
+	return m_database->hasModEntry(m_mod_name, name);
 }
 
 bool ModMetadata::setString(const std::string &name, const std::string &var)
 {
-	if (Metadata::setString(name, var)) {
-		if (var.empty()) {
-			m_database->removeModEntry(m_mod_name, name);
-		} else {
-			m_database->setModEntry(m_mod_name, name, var);
+	if (var.empty())
+		return m_database->removeModEntry(m_mod_name, name);
+	else
+		return m_database->setModEntry(m_mod_name, name, var);
 }
-		return true;
+
+const StringMap &ModMetadata::getStrings(StringMap *place) const
+{
+	place->clear();
+	m_database->getModEntries(m_mod_name, place);
+	return *place;
 }
-	return false;
+
+const std::string *ModMetadata::getStringRaw(const std::string &name, std::string *place) const
+{
+	return m_database->getModEntry(m_mod_name, name, place) ? place : nullptr;
 }

src/content/mods.h

@@ -110,18 +110,26 @@ std::map<std::string, ModSpec> getModsInPath(const std::string &path,
 std::vector<ModSpec> flattenMods(const std::map<std::string, ModSpec> &mods);
 
 
-class ModMetadata : public Metadata
+class ModMetadata : public IMetadata
 {
 public:
 	ModMetadata() = delete;
 	ModMetadata(const std::string &mod_name, ModMetadataDatabase *database);
 	~ModMetadata() = default;
 
-	virtual void clear();
-
 	const std::string &getModName() const { return m_mod_name; }
 
-	virtual bool setString(const std::string &name, const std::string &var);
+	void clear() override;
+
+	bool contains(const std::string &name) const override;
+
+	bool setString(const std::string &name, const std::string &var) override;
+
+	const StringMap &getStrings(StringMap *place) const override;
+
+protected:
+	const std::string *getStringRaw(const std::string &name,
+			std::string *place) const override;
 
 private:
 	std::string m_mod_name;

src/database/database-dummy.cpp

@@ -92,6 +92,32 @@ bool Database_Dummy::getModEntries(const std::string &modname, StringMap *storag
 	return true;
 }
 
+bool Database_Dummy::getModEntry(const std::string &modname,
+	const std::string &key, std::string *value)
+{
+	auto mod_pair = m_mod_meta_database.find(modname);
+	if (mod_pair == m_mod_meta_database.end())
+		return false;
+	const StringMap &meta = mod_pair->second;
+
+	auto pair = meta.find(key);
+	if (pair != meta.end()) {
+		*value = pair->second;
+		return true;
+	}
+	return false;
+}
+
+bool Database_Dummy::hasModEntry(const std::string &modname, const std::string &key)
+{
+	auto mod_pair = m_mod_meta_database.find(modname);
+	if (mod_pair == m_mod_meta_database.end())
+		return false;
+	const StringMap &meta = mod_pair->second;
+
+	return meta.find(key) != meta.cend();
+}
+
 bool Database_Dummy::setModEntry(const std::string &modname,
 	const std::string &key, const std::string &value)
 {
@@ -112,6 +138,16 @@ bool Database_Dummy::removeModEntry(const std::string &modname, const std::strin
 	return false;
 }
 
+bool Database_Dummy::removeModEntries(const std::string &modname)
+{
+	auto mod_pair = m_mod_meta_database.find(modname);
+	if (mod_pair != m_mod_meta_database.end() && !mod_pair->second.empty()) {
+		mod_pair->second.clear();
+		return true;
+	}
+	return false;
+}
+
 void Database_Dummy::listMods(std::vector<std::string> *res)
 {
 	for (const auto &pair : m_mod_meta_database) {

src/database/database-dummy.h

@@ -38,9 +38,13 @@ public:
 	void listPlayers(std::vector<std::string> &res);
 
 	bool getModEntries(const std::string &modname, StringMap *storage);
+	bool getModEntry(const std::string &modname,
+			const std::string &key, std::string *value);
+	bool hasModEntry(const std::string &modname, const std::string &key);
 	bool setModEntry(const std::string &modname,
 			const std::string &key, const std::string &value);
 	bool removeModEntry(const std::string &modname, const std::string &key);
+	bool removeModEntries(const std::string &modname);
 	void listMods(std::vector<std::string> *res);
 
 	void beginSave() {}

src/database/database-files.cpp

@@ -396,6 +396,26 @@ bool ModMetadataDatabaseFiles::getModEntries(const std::string &modname, StringM
 	return true;
 }
 
+bool ModMetadataDatabaseFiles::getModEntry(const std::string &modname,
+	const std::string &key, std::string *value)
+{
+	Json::Value *meta = getOrCreateJson(modname);
+	if (!meta)
+		return false;
+
+	if (meta->isMember(key)) {
+		*value = (*meta)[key].asString();
+		return true;
+	}
+	return false;
+}
+
+bool ModMetadataDatabaseFiles::hasModEntry(const std::string &modname, const std::string &key)
+{
+	Json::Value *meta = getOrCreateJson(modname);
+	return meta && meta->isMember(key);
+}
+
 bool ModMetadataDatabaseFiles::setModEntry(const std::string &modname,
 	const std::string &key, const std::string &value)
 {
@@ -424,6 +444,17 @@ bool ModMetadataDatabaseFiles::removeModEntry(const std::string &modname,
 	return false;
 }
 
+bool ModMetadataDatabaseFiles::removeModEntries(const std::string &modname)
+{
+	Json::Value *meta = getOrCreateJson(modname);
+	if (!meta || meta->empty())
+		return false;
+
+	meta->clear();
+	m_modified.insert(modname);
+	return true;
+}
+
 void ModMetadataDatabaseFiles::beginSave()
 {
 }

src/database/database-files.h

@@ -79,9 +79,13 @@ public:
 	virtual ~ModMetadataDatabaseFiles() = default;
 
 	virtual bool getModEntries(const std::string &modname, StringMap *storage);
+	virtual bool getModEntry(const std::string &modname,
+		const std::string &key, std::string *value);
+	virtual bool hasModEntry(const std::string &modname, const std::string &key);
 	virtual bool setModEntry(const std::string &modname,
 		const std::string &key, const std::string &value);
 	virtual bool removeModEntry(const std::string &modname, const std::string &key);
+	virtual bool removeModEntries(const std::string &modname);
 	virtual void listMods(std::vector<std::string> *res);
 
 	virtual void beginSave();

src/database/database-sqlite3.cpp

@@ -770,9 +770,12 @@ ModMetadataDatabaseSQLite3::ModMetadataDatabaseSQLite3(const std::string &savedi
 
 ModMetadataDatabaseSQLite3::~ModMetadataDatabaseSQLite3()
 {
+	FINALIZE_STATEMENT(m_stmt_remove_all)
 	FINALIZE_STATEMENT(m_stmt_remove)
 	FINALIZE_STATEMENT(m_stmt_set)
+	FINALIZE_STATEMENT(m_stmt_has)
 	FINALIZE_STATEMENT(m_stmt_get)
+	FINALIZE_STATEMENT(m_stmt_get_all)
 }
 
 void ModMetadataDatabaseSQLite3::createDatabase()
@@ -792,31 +795,74 @@ void ModMetadataDatabaseSQLite3::createDatabase()
 
 void ModMetadataDatabaseSQLite3::initStatements()
 {
-	PREPARE_STATEMENT(get, "SELECT `key`, `value` FROM `entries` WHERE `modname` = ?");
+	PREPARE_STATEMENT(get_all, "SELECT `key`, `value` FROM `entries` WHERE `modname` = ?");
+	PREPARE_STATEMENT(get,
+		"SELECT `value` FROM `entries` WHERE `modname` = ? AND `key` = ? LIMIT 1");
+	PREPARE_STATEMENT(has,
+		"SELECT 1 FROM `entries` WHERE `modname` = ? AND `key` = ? LIMIT 1");
 	PREPARE_STATEMENT(set,
 		"REPLACE INTO `entries` (`modname`, `key`, `value`) VALUES (?, ?, ?)");
 	PREPARE_STATEMENT(remove, "DELETE FROM `entries` WHERE `modname` = ? AND `key` = ?");
+	PREPARE_STATEMENT(remove_all, "DELETE FROM `entries` WHERE `modname` = ?");
 }
 
 bool ModMetadataDatabaseSQLite3::getModEntries(const std::string &modname, StringMap *storage)
 {
 	verifyDatabase();
 
-	str_to_sqlite(m_stmt_get, 1, modname);
-	while (sqlite3_step(m_stmt_get) == SQLITE_ROW) {
-		const char *key_data = (const char *) sqlite3_column_blob(m_stmt_get, 0);
-		size_t key_len = sqlite3_column_bytes(m_stmt_get, 0);
-		const char *value_data = (const char *) sqlite3_column_blob(m_stmt_get, 1);
-		size_t value_len = sqlite3_column_bytes(m_stmt_get, 1);
+	str_to_sqlite(m_stmt_get_all, 1, modname);
+	while (sqlite3_step(m_stmt_get_all) == SQLITE_ROW) {
+		const char *key_data = (const char *) sqlite3_column_blob(m_stmt_get_all, 0);
+		size_t key_len = sqlite3_column_bytes(m_stmt_get_all, 0);
+		const char *value_data = (const char *) sqlite3_column_blob(m_stmt_get_all, 1);
+		size_t value_len = sqlite3_column_bytes(m_stmt_get_all, 1);
 		(*storage)[std::string(key_data, key_len)] = std::string(value_data, value_len);
 	}
 	sqlite3_vrfy(sqlite3_errcode(m_database), SQLITE_DONE);
 
-	sqlite3_reset(m_stmt_get);
+	sqlite3_reset(m_stmt_get_all);
 
 	return true;
 }
 
+bool ModMetadataDatabaseSQLite3::getModEntry(const std::string &modname,
+	const std::string &key, std::string *value)
+{
+	verifyDatabase();
+
+	str_to_sqlite(m_stmt_get, 1, modname);
+	SQLOK(sqlite3_bind_blob(m_stmt_get, 2, key.data(), key.size(), NULL),
+		"Internal error: failed to bind query at " __FILE__ ":" TOSTRING(__LINE__));
+	bool found = sqlite3_step(m_stmt_get) == SQLITE_ROW;
+	if (found) {
+		const char *value_data = (const char *) sqlite3_column_blob(m_stmt_get, 0);
+		size_t value_len = sqlite3_column_bytes(m_stmt_get, 0);
+		value->assign(value_data, value_len);
+		sqlite3_step(m_stmt_get);
+	}
+
+	sqlite3_reset(m_stmt_get);
+
+	return found;
+}
+
+bool ModMetadataDatabaseSQLite3::hasModEntry(const std::string &modname,
+		const std::string &key)
+{
+	verifyDatabase();
+
+	str_to_sqlite(m_stmt_has, 1, modname);
+	SQLOK(sqlite3_bind_blob(m_stmt_has, 2, key.data(), key.size(), NULL),
+		"Internal error: failed to bind query at " __FILE__ ":" TOSTRING(__LINE__));
+	bool found = sqlite3_step(m_stmt_has) == SQLITE_ROW;
+	if (found)
+		sqlite3_step(m_stmt_has);
+
+	sqlite3_reset(m_stmt_has);
+
+	return found;
+}
+
 bool ModMetadataDatabaseSQLite3::setModEntry(const std::string &modname,
 	const std::string &key, const std::string &value)
 {
@@ -850,6 +896,19 @@ bool ModMetadataDatabaseSQLite3::removeModEntry(const std::string &modname,
 	return changes > 0;
 }
 
+bool ModMetadataDatabaseSQLite3::removeModEntries(const std::string &modname)
+{
+	verifyDatabase();
+
+	str_to_sqlite(m_stmt_remove_all, 1, modname);
+	sqlite3_vrfy(sqlite3_step(m_stmt_remove_all), SQLITE_DONE);
+	int changes = sqlite3_changes(m_database);
+
+	sqlite3_reset(m_stmt_remove_all);
+
+	return changes > 0;
+}
+
 void ModMetadataDatabaseSQLite3::listMods(std::vector<std::string> *res)
 {
 	verifyDatabase();

src/database/database-sqlite3.h

@@ -240,9 +240,13 @@ public:
 	virtual ~ModMetadataDatabaseSQLite3();
 
 	virtual bool getModEntries(const std::string &modname, StringMap *storage);
+	virtual bool getModEntry(const std::string &modname,
+		const std::string &key, std::string *value);
+	virtual bool hasModEntry(const std::string &modname, const std::string &key);
 	virtual bool setModEntry(const std::string &modname,
 		const std::string &key, const std::string &value);
 	virtual bool removeModEntry(const std::string &modname, const std::string &key);
+	virtual bool removeModEntries(const std::string &modname);
 	virtual void listMods(std::vector<std::string> *res);
 
 	virtual void beginSave() { Database_SQLite3::beginSave(); }
@@ -253,7 +257,10 @@ protected:
 	virtual void initStatements();
 
 private:
+	sqlite3_stmt *m_stmt_get_all = nullptr;
 	sqlite3_stmt *m_stmt_get = nullptr;
+	sqlite3_stmt *m_stmt_has = nullptr;
 	sqlite3_stmt *m_stmt_set = nullptr;
 	sqlite3_stmt *m_stmt_remove = nullptr;
+	sqlite3_stmt *m_stmt_remove_all = nullptr;
 };

src/database/database.h

@@ -92,8 +92,12 @@ public:
 	virtual ~ModMetadataDatabase() = default;
 
 	virtual bool getModEntries(const std::string &modname, StringMap *storage) = 0;
+	virtual bool hasModEntry(const std::string &modname, const std::string &key) = 0;
+	virtual bool getModEntry(const std::string &modname,
+		const std::string &key, std::string *value) = 0;
 	virtual bool setModEntry(const std::string &modname,
 		const std::string &key, const std::string &value) = 0;
 	virtual bool removeModEntry(const std::string &modname, const std::string &key) = 0;
+	virtual bool removeModEntries(const std::string &modname) = 0;
 	virtual void listMods(std::vector<std::string> *res) = 0;
 };

src/dummygamedef.h

@@ -61,8 +61,6 @@ public:
 		return emptymodspec;
 	}
 	const ModSpec* getModSpec(const std::string &modname) const override { return nullptr; }
-	bool registerModStorage(ModMetadata *meta) override { return true; }
-	void unregisterModStorage(const std::string &name) override {}
 	ModMetadataDatabase *getModStorageDatabase() override { return m_mod_storage_database; }
 
 	bool joinModChannel(const std::string &channel) override { return false; }

src/gamedef.h

@@ -73,8 +73,6 @@ public:
 	virtual const std::vector<ModSpec> &getMods() const = 0;
 	virtual const ModSpec* getModSpec(const std::string &modname) const = 0;
 	virtual std::string getWorldPath() const { return ""; }
-	virtual bool registerModStorage(ModMetadata *storage) = 0;
-	virtual void unregisterModStorage(const std::string &name) = 0;
 	virtual ModMetadataDatabase *getModStorageDatabase() = 0;
 
 	virtual bool joinModChannel(const std::string &channel) = 0;

src/itemstackmetadata.cpp

@@ -34,7 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 void ItemStackMetadata::clear()
 {
-	Metadata::clear();
+	SimpleMetadata::clear();
 	updateToolCapabilities();
 }
 
@@ -52,7 +52,7 @@ bool ItemStackMetadata::setString(const std::string &name, const std::string &va
 	sanitize_string(clean_name);
 	sanitize_string(clean_var);
 
-	bool result = Metadata::setString(clean_name, clean_var);
+	bool result = SimpleMetadata::setString(clean_name, clean_var);
 	if (clean_name == TOOLCAP_KEY)
 		updateToolCapabilities();
 	return result;

src/itemstackmetadata.h

@@ -25,7 +25,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 class Inventory;
 class IItemDefManager;
 
-class ItemStackMetadata : public Metadata
+class ItemStackMetadata : public SimpleMetadata
 {
 public:
 	ItemStackMetadata() : toolcaps_overridden(false) {}

src/metadata.cpp

@@ -21,95 +21,110 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "log.h"
 
 /*
-	Metadata
+	IMetadata
 */
 
-void Metadata::clear()
+bool IMetadata::operator==(const IMetadata &other) const
+{
+	StringMap this_map_, other_map_;
+	const StringMap &this_map = getStrings(&this_map_);
+	const StringMap &other_map = other.getStrings(&other_map_);
+
+	if (this_map.size() != other_map.size())
+		return false;
+
+	for (const auto &this_pair : this_map) {
+		const auto &other_pair = other_map.find(this_pair.first);
+		if (other_pair == other_map.cend() || other_pair->second != this_pair.second)
+			return false;
+	}
+
+	return true;
+}
+
+const std::string &IMetadata::getString(const std::string &name, std::string *place,
+		u16 recursion) const
+{
+	const std::string *raw = getStringRaw(name, place);
+	if (!raw) {
+		static const std::string empty_string = std::string("");
+		return empty_string;
+	}
+
+	return resolveString(*raw, place, recursion);
+}
+
+bool IMetadata::getStringToRef(const std::string &name,
+		std::string &str, u16 recursion) const
+{
+	const std::string *raw = getStringRaw(name, &str);
+	if (!raw)
+		return false;
+
+	const std::string &resolved = resolveString(*raw, &str, recursion);
+	if (&resolved != &str)
+		str = resolved;
+	return true;
+}
+
+const std::string &IMetadata::resolveString(const std::string &str, std::string *place,
+		u16 recursion) const
+{
+	if (recursion <= 1 && str.substr(0, 2) == "${" && str[str.length() - 1] == '}') {
+		// It may be the case that &str == place, but that's fine.
+		return getString(str.substr(2, str.length() - 3), place, recursion + 1);
+	}
+
+	return str;
+}
+
+/*
+	SimpleMetadata
+*/
+
+void SimpleMetadata::clear()
 {
 	m_stringvars.clear();
 	m_modified = true;
 }
 
-bool Metadata::empty() const
+bool SimpleMetadata::empty() const
 {
 	return m_stringvars.empty();
 }
 
-size_t Metadata::size() const
+size_t SimpleMetadata::size() const
 {
 	return m_stringvars.size();
 }
 
-bool Metadata::contains(const std::string &name) const
+bool SimpleMetadata::contains(const std::string &name) const
 {
 	return m_stringvars.find(name) != m_stringvars.end();
 }
 
-bool Metadata::operator==(const Metadata &other) const
+const StringMap &SimpleMetadata::getStrings(StringMap *) const
 {
-	if (size() != other.size())
-		return false;
-
-	for (const auto &sv : m_stringvars) {
-		if (!other.contains(sv.first) || other.getString(sv.first) != sv.second)
-			return false;
+	return m_stringvars;
 }
 
-	return true;
-}
-
-const std::string &Metadata::getString(const std::string &name, u16 recursion) const
+const std::string *SimpleMetadata::getStringRaw(const std::string &name, std::string *) const
 {
-	StringMap::const_iterator it = m_stringvars.find(name);
-	if (it == m_stringvars.end()) {
-		static const std::string empty_string = std::string("");
-		return empty_string;
+	const auto found = m_stringvars.find(name);
+	return found != m_stringvars.cend() ? &found->second : nullptr;
 }
 
-	return resolveString(it->second, recursion);
-}
-
-bool Metadata::getStringToRef(
-		const std::string &name, std::string &str, u16 recursion) const
-{
-	StringMap::const_iterator it = m_stringvars.find(name);
-	if (it == m_stringvars.end()) {
-		return false;
-	}
-
-	str = resolveString(it->second, recursion);
-	return true;
-}
-
-/**
- * Sets var to name key in the metadata storage
- *
- * @param name
- * @param var
- * @return true if key-value pair is created or changed
- */
-bool Metadata::setString(const std::string &name, const std::string &var)
+bool SimpleMetadata::setString(const std::string &name, const std::string &var)
 {
 	if (var.empty()) {
-		m_stringvars.erase(name);
-		return true;
-	}
-
-	StringMap::iterator it = m_stringvars.find(name);
-	if (it != m_stringvars.end() && it->second == var) {
+		if (m_stringvars.erase(name) == 0)
+			return false;
+	} else {
+		StringMap::iterator it = m_stringvars.find(name);
+		if (it != m_stringvars.end() && it->second == var)
 			return false;
-	}
-
 		m_stringvars[name] = var;
+	}
 	m_modified = true;
 	return true;
 }
-
-const std::string &Metadata::resolveString(const std::string &str, u16 recursion) const
-{
-	if (recursion <= 1 && str.substr(0, 2) == "${" && str[str.length() - 1] == '}') {
-		return getString(str.substr(2, str.length() - 3), recursion + 1);
-	}
-
-	return str;
-}

src/metadata.h

@@ -24,17 +24,16 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include <vector>
 #include "util/string.h"
 
-class Metadata
+// Basic metadata interface
+class IMetadata
 {
-	bool m_modified = false;
 public:
-	virtual ~Metadata() = default;
+	virtual ~IMetadata() = default;
 
-	virtual void clear();
-	virtual bool empty() const;
+	virtual void clear() = 0;
 
-	bool operator==(const Metadata &other) const;
-	inline bool operator!=(const Metadata &other) const
+	bool operator==(const IMetadata &other) const;
+	inline bool operator!=(const IMetadata &other) const
 	{
 		return !(*this == other);
 	}
@@ -43,21 +42,76 @@ public:
 	// Key-value related
 	//
 
-	size_t size() const;
-	bool contains(const std::string &name) const;
-	const std::string &getString(const std::string &name, u16 recursion = 0) const;
+	virtual bool contains(const std::string &name) const = 0;
+
+	// May (not must!) put a string in `place` and return a reference to that string.
+	const std::string &getString(const std::string &name, std::string *place,
+			u16 recursion = 0) const;
+
+	// If the entry is present, puts the value in str and returns true;
+	// otherwise just returns false.
 	bool getStringToRef(const std::string &name, std::string &str, u16 recursion = 0) const;
-	virtual bool setString(const std::string &name, const std::string &var);
+
+	// Returns whether the metadata was (potentially) changed.
+	virtual bool setString(const std::string &name, const std::string &var) = 0;
+
 	inline bool removeString(const std::string &name) { return setString(name, ""); }
-	const StringMap &getStrings() const
+
+	// May (not must!) put strings in `place` and return a reference to these strings.
+	virtual const StringMap &getStrings(StringMap *place) const = 0;
+
+	// Add support for variable names in values. Uses place like getString.
+	const std::string &resolveString(const std::string &str, std::string *place,
+			u16 recursion = 0) const;
+
+protected:
+	// Returns nullptr to indicate absence of value. Uses place like getString.
+	virtual const std::string *getStringRaw(const std::string &name,
+			std::string *place) const = 0;
+};
+
+// Simple metadata parent class (in-memory storage)
+class SimpleMetadata: public virtual IMetadata
 {
-		return m_stringvars;
+	bool m_modified = false;
+public:
+	virtual ~SimpleMetadata() = default;
+
+	virtual void clear() override;
+	virtual bool empty() const;
+
+	//
+	// Key-value related
+	//
+
+	size_t size() const;
+	bool contains(const std::string &name) const override;
+	virtual bool setString(const std::string &name, const std::string &var) override;
+	const StringMap &getStrings(StringMap *) const override final;
+
+	// Simple version of getters, possible due to in-memory storage:
+
+	inline const std::string &getString(const std::string &name, u16 recursion = 0) const
+	{
+		return IMetadata::getString(name, nullptr, recursion);
+	}
+
+	inline const std::string &resolveString(const std::string &str, u16 recursion = 0) const
+	{
+		return IMetadata::resolveString(str, nullptr, recursion);
+	}
+
+	inline const StringMap &getStrings() const
+	{
+		return SimpleMetadata::getStrings(nullptr);
 	}
-	// Add support for variable names in values
-	const std::string &resolveString(const std::string &str, u16 recursion = 0) const;
 
 	inline bool isModified() const  { return m_modified; }
 	inline void setModified(bool v) { m_modified = v; }
+
 protected:
 	StringMap m_stringvars;
+
+	const std::string *getStringRaw(const std::string &name,
+			std::string *) const override final;
 };

src/nodemetadata.cpp

@@ -77,14 +77,14 @@ void NodeMetadata::deSerialize(std::istream &is, u8 version)
 
 void NodeMetadata::clear()
 {
-	Metadata::clear();
+	SimpleMetadata::clear();
 	m_privatevars.clear();
 	m_inventory->clear();
 }
 
 bool NodeMetadata::empty() const
 {
-	return Metadata::empty() && m_inventory->getLists().empty();
+	return SimpleMetadata::empty() && m_inventory->getLists().empty();
 }
 
 

src/nodemetadata.h

@@ -34,7 +34,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 class Inventory;
 class IItemDefManager;
 
-class NodeMetadata : public Metadata
+class NodeMetadata : public SimpleMetadata
 {
 public:
 	NodeMetadata(IItemDefManager *item_def_mgr);

src/script/lua_api/l_itemstackmeta.cpp

@@ -36,7 +36,7 @@ ItemStackMetaRef* ItemStackMetaRef::checkobject(lua_State *L, int narg)
 	return *(ItemStackMetaRef**)ud;  // unbox pointer
 }
 
-Metadata* ItemStackMetaRef::getmeta(bool auto_create)
+IMetadata* ItemStackMetaRef::getmeta(bool auto_create)
 {
 	return &istack->getItem().metadata;
 }

src/script/lua_api/l_itemstackmeta.h

@@ -36,7 +36,7 @@ private:
 
 	static ItemStackMetaRef *checkobject(lua_State *L, int narg);
 
-	virtual Metadata* getmeta(bool auto_create);
+	virtual IMetadata* getmeta(bool auto_create);
 
 	virtual void clearMeta();
 

src/script/lua_api/l_metadata.cpp

@@ -59,7 +59,7 @@ int MetaDataRef::l_contains(lua_State *L)
 	MetaDataRef *ref = checkobject(L, 1);
 	std::string name = luaL_checkstring(L, 2);
 
-	Metadata *meta = ref->getmeta(false);
+	IMetadata *meta = ref->getmeta(false);
 	if (meta == NULL)
 		return 0;
 
@@ -75,7 +75,7 @@ int MetaDataRef::l_get(lua_State *L)
 	MetaDataRef *ref = checkobject(L, 1);
 	std::string name = luaL_checkstring(L, 2);
 
-	Metadata *meta = ref->getmeta(false);
+	IMetadata *meta = ref->getmeta(false);
 	if (meta == NULL)
 		return 0;
 
@@ -96,13 +96,14 @@ int MetaDataRef::l_get_string(lua_State *L)
 	MetaDataRef *ref = checkobject(L, 1);
 	std::string name = luaL_checkstring(L, 2);
 
-	Metadata *meta = ref->getmeta(false);
+	IMetadata *meta = ref->getmeta(false);
 	if (meta == NULL) {
 		lua_pushlstring(L, "", 0);
 		return 1;
 	}
 
-	const std::string &str = meta->getString(name);
+	std::string str_;
+	const std::string &str = meta->getString(name, &str_);
 	lua_pushlstring(L, str.c_str(), str.size());
 	return 1;
 }
@@ -118,11 +119,8 @@ int MetaDataRef::l_set_string(lua_State *L)
 	const char *s = lua_tolstring(L, 3, &len);
 	std::string str(s, len);
 
-	Metadata *meta = ref->getmeta(!str.empty());
-	if (meta == NULL || str == meta->getString(name))
-		return 0;
-
-	meta->setString(name, str);
+	IMetadata *meta = ref->getmeta(!str.empty());
+	if (meta != NULL && meta->setString(name, str))
 		ref->reportMetadataChange(&name);
 	return 0;
 }
@@ -135,13 +133,14 @@ int MetaDataRef::l_get_int(lua_State *L)
 	MetaDataRef *ref = checkobject(L, 1);
 	std::string name = luaL_checkstring(L, 2);
 
-	Metadata *meta = ref->getmeta(false);
+	IMetadata *meta = ref->getmeta(false);
 	if (meta == NULL) {
 		lua_pushnumber(L, 0);
 		return 1;
 	}
 
-	const std::string &str = meta->getString(name);
+	std::string str_;
+	const std::string &str = meta->getString(name, &str_);
 	lua_pushnumber(L, stoi(str));
 	return 1;
 }
@@ -156,11 +155,8 @@ int MetaDataRef::l_set_int(lua_State *L)
 	int a = luaL_checkint(L, 3);
 	std::string str = itos(a);
 
-	Metadata *meta = ref->getmeta(true);
-	if (meta == NULL || str == meta->getString(name))
-		return 0;
-
-	meta->setString(name, str);
+	IMetadata *meta = ref->getmeta(true);
+	if (meta != NULL && meta->setString(name, str))
 		ref->reportMetadataChange(&name);
 	return 0;
 }
@@ -173,13 +169,14 @@ int MetaDataRef::l_get_float(lua_State *L)
 	MetaDataRef *ref = checkobject(L, 1);
 	std::string name = luaL_checkstring(L, 2);
 
-	Metadata *meta = ref->getmeta(false);
+	IMetadata *meta = ref->getmeta(false);
 	if (meta == NULL) {
 		lua_pushnumber(L, 0);
 		return 1;
 	}
 
-	const std::string &str = meta->getString(name);
+	std::string str_;
+	const std::string &str = meta->getString(name, &str_);
 	lua_pushnumber(L, stof(str));
 	return 1;
 }
@@ -194,11 +191,8 @@ int MetaDataRef::l_set_float(lua_State *L)
 	float a = readParam<float>(L, 3);
 	std::string str = ftos(a);
 
-	Metadata *meta = ref->getmeta(true);
-	if (meta == NULL || str == meta->getString(name))
-		return 0;
-
-	meta->setString(name, str);
+	IMetadata *meta = ref->getmeta(true);
+	if (meta != NULL && meta->setString(name, str))
 		ref->reportMetadataChange(&name);
 	return 0;
 }
@@ -210,7 +204,7 @@ int MetaDataRef::l_to_table(lua_State *L)
 
 	MetaDataRef *ref = checkobject(L, 1);
 
-	Metadata *meta = ref->getmeta(true);
+	IMetadata *meta = ref->getmeta(true);
 	if (meta == NULL) {
 		lua_pushnil(L);
 		return 1;
@@ -239,7 +233,7 @@ int MetaDataRef::l_from_table(lua_State *L)
 	}
 
 	// Create new metadata
-	Metadata *meta = ref->getmeta(true);
+	IMetadata *meta = ref->getmeta(true);
 	if (meta == NULL) {
 		lua_pushboolean(L, false);
 		return 1;
@@ -251,11 +245,12 @@ int MetaDataRef::l_from_table(lua_State *L)
 	return 1;
 }
 
-void MetaDataRef::handleToTable(lua_State *L, Metadata *meta)
+void MetaDataRef::handleToTable(lua_State *L, IMetadata *meta)
 {
 	lua_newtable(L);
 	{
-		const StringMap &fields = meta->getStrings();
+		StringMap fields_;
+		const StringMap &fields = meta->getStrings(&fields_);
 		for (const auto &field : fields) {
 			const std::string &name = field.first;
 			const std::string &value = field.second;
@@ -267,7 +262,7 @@ void MetaDataRef::handleToTable(lua_State *L, Metadata *meta)
 	lua_setfield(L, -2, "fields");
 }
 
-bool MetaDataRef::handleFromTable(lua_State *L, int table, Metadata *meta)
+bool MetaDataRef::handleFromTable(lua_State *L, int table, IMetadata *meta)
 {
 	// Set fields
 	lua_getfield(L, table, "fields");
@@ -292,9 +287,9 @@ bool MetaDataRef::handleFromTable(lua_State *L, int table, Metadata *meta)
 int MetaDataRef::l_equals(lua_State *L)
 {
 	MetaDataRef *ref1 = checkobject(L, 1);
-	Metadata *data1 = ref1->getmeta(false);
+	IMetadata *data1 = ref1->getmeta(false);
 	MetaDataRef *ref2 = checkobject(L, 2);
-	Metadata *data2 = ref2->getmeta(false);
+	IMetadata *data2 = ref2->getmeta(false);
 	if (data1 == NULL || data2 == NULL)
 		lua_pushboolean(L, data1 == data2);
 	else

src/script/lua_api/l_metadata.h

@@ -23,7 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "irrlichttypes_bloated.h"
 #include "lua_api/l_base.h"
 
-class Metadata;
+class IMetadata;
 
 /*
 	NodeMetaRef
@@ -38,11 +38,11 @@ protected:
 	static MetaDataRef *checkobject(lua_State *L, int narg);
 
 	virtual void reportMetadataChange(const std::string *name = nullptr) {}
-	virtual Metadata *getmeta(bool auto_create) = 0;
+	virtual IMetadata *getmeta(bool auto_create) = 0;
 	virtual void clearMeta() = 0;
 
-	virtual void handleToTable(lua_State *L, Metadata *meta);
-	virtual bool handleFromTable(lua_State *L, int table, Metadata *meta);
+	virtual void handleToTable(lua_State *L, IMetadata *meta);
+	virtual bool handleFromTable(lua_State *L, int table, IMetadata *meta);
 
 	// Exported functions
 

src/script/lua_api/l_nodemeta.cpp

@@ -37,7 +37,7 @@ NodeMetaRef* NodeMetaRef::checkobject(lua_State *L, int narg)
 	return *(NodeMetaRef**)ud;  // unbox pointer
 }
 
-Metadata* NodeMetaRef::getmeta(bool auto_create)
+IMetadata* NodeMetaRef::getmeta(bool auto_create)
 {
 	if (m_is_local)
 		return m_local_meta;
@@ -127,12 +127,13 @@ int NodeMetaRef::l_mark_as_private(lua_State *L)
 	return 0;
 }
 
-void NodeMetaRef::handleToTable(lua_State *L, Metadata *_meta)
+void NodeMetaRef::handleToTable(lua_State *L, IMetadata *_meta)
 {
 	// fields
 	MetaDataRef::handleToTable(L, _meta);
 
-	NodeMetadata *meta = (NodeMetadata *) _meta;
+	NodeMetadata *meta = dynamic_cast<NodeMetadata*>(_meta);
+	assert(meta);
 
 	// inventory
 	Inventory *inv = meta->getInventory();
@@ -145,13 +146,14 @@ void NodeMetaRef::handleToTable(lua_State *L, Metadata *_meta)
 }
 
 // from_table(self, table)
-bool NodeMetaRef::handleFromTable(lua_State *L, int table, Metadata *_meta)
+bool NodeMetaRef::handleFromTable(lua_State *L, int table, IMetadata *_meta)
 {
 	// fields
 	if (!MetaDataRef::handleFromTable(L, table, _meta))
 		return false;
 
-	NodeMetadata *meta = (NodeMetadata*) _meta;
+	NodeMetadata *meta = dynamic_cast<NodeMetadata*>(_meta);
+	assert(meta);
 
 	// inventory
 	Inventory *inv = meta->getInventory();
@@ -178,7 +180,7 @@ NodeMetaRef::NodeMetaRef(v3s16 p, ServerEnvironment *env):
 {
 }
 
-NodeMetaRef::NodeMetaRef(Metadata *meta):
+NodeMetaRef::NodeMetaRef(IMetadata *meta):
 	m_is_local(true),
 	m_local_meta(meta)
 {
@@ -196,7 +198,7 @@ void NodeMetaRef::create(lua_State *L, v3s16 p, ServerEnvironment *env)
 }
 
 // Client-sided version of the above
-void NodeMetaRef::createClient(lua_State *L, Metadata *meta)
+void NodeMetaRef::createClient(lua_State *L, IMetadata *meta)
 {
 	NodeMetaRef *o = new NodeMetaRef(meta);
 	*(void **)(lua_newuserdata(L, sizeof(void *))) = o;

src/script/lua_api/l_nodemeta.h

@@ -38,7 +38,7 @@ private:
 	v3s16 m_p;
 	ServerEnvironment *m_env = nullptr;
 	// Set for client metadata
-	Metadata *m_local_meta = nullptr;
+	IMetadata *m_local_meta = nullptr;
 
 	static const char className[];
 	static const luaL_Reg methodsServer[];
@@ -59,13 +59,13 @@ private:
 	 * @param auto_create when true, try to create metadata information for the node if it has none.
 	 * @return pointer to a @c NodeMetadata object or @c NULL in case of error.
 	 */
-	virtual Metadata* getmeta(bool auto_create);
+	virtual IMetadata* getmeta(bool auto_create);
 	virtual void clearMeta();
 
 	virtual void reportMetadataChange(const std::string *name = nullptr);
 
-	virtual void handleToTable(lua_State *L, Metadata *_meta);
-	virtual bool handleFromTable(lua_State *L, int table, Metadata *_meta);
+	virtual void handleToTable(lua_State *L, IMetadata *_meta);
+	virtual bool handleFromTable(lua_State *L, int table, IMetadata *_meta);
 
 	// Exported functions
 
@@ -80,7 +80,7 @@ private:
 
 public:
 	NodeMetaRef(v3s16 p, ServerEnvironment *env);
-	NodeMetaRef(Metadata *meta);
+	NodeMetaRef(IMetadata *meta);
 
 	~NodeMetaRef() = default;
 
@@ -89,7 +89,7 @@ public:
 	static void create(lua_State *L, v3s16 p, ServerEnvironment *env);
 
 	// Client-sided version of the above
-	static void createClient(lua_State *L, Metadata *meta);
+	static void createClient(lua_State *L, IMetadata *meta);
 
 	static void RegisterCommon(lua_State *L);
 	static void Register(lua_State *L);

src/script/lua_api/l_playermeta.cpp

@@ -35,7 +35,7 @@ PlayerMetaRef *PlayerMetaRef::checkobject(lua_State *L, int narg)
 	return *(PlayerMetaRef **)ud; // unbox pointer
 }
 
-Metadata *PlayerMetaRef::getmeta(bool auto_create)
+IMetadata *PlayerMetaRef::getmeta(bool auto_create)
 {
 	return metadata;
 }
@@ -60,7 +60,7 @@ int PlayerMetaRef::gc_object(lua_State *L)
 
 // Creates an PlayerMetaRef and leaves it on top of stack
 // Not callable from Lua; all references are created on the C side.
-void PlayerMetaRef::create(lua_State *L, Metadata *metadata)
+void PlayerMetaRef::create(lua_State *L, IMetadata *metadata)
 {
 	PlayerMetaRef *o = new PlayerMetaRef(metadata);
 	*(void **)(lua_newuserdata(L, sizeof(void *))) = o;

src/script/lua_api/l_playermeta.h

@@ -29,14 +29,14 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 class PlayerMetaRef : public MetaDataRef
 {
 private:
-	Metadata *metadata = nullptr;
+	IMetadata *metadata = nullptr;
 
 	static const char className[];
 	static const luaL_Reg methods[];
 
 	static PlayerMetaRef *checkobject(lua_State *L, int narg);
 
-	virtual Metadata *getmeta(bool auto_create);
+	virtual IMetadata *getmeta(bool auto_create);
 
 	virtual void clearMeta();
 
@@ -46,12 +46,12 @@ private:
 	static int gc_object(lua_State *L);
 
 public:
-	PlayerMetaRef(Metadata *metadata) : metadata(metadata) {}
+	PlayerMetaRef(IMetadata *metadata) : metadata(metadata) {}
 	~PlayerMetaRef() = default;
 
 	// Creates an ItemStackMetaRef and leaves it on top of stack
 	// Not callable from Lua; all references are created on the C side.
-	static void create(lua_State *L, Metadata *metadata);
+	static void create(lua_State *L, IMetadata *metadata);
 
 	static void Register(lua_State *L);
 };

src/script/lua_api/l_storage.cpp

@@ -20,7 +20,6 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "lua_api/l_storage.h"
 #include "l_internal.h"
-#include "content/mods.h"
 #include "server.h"
 
 int ModApiStorage::l_get_mod_storage(lua_State *L)
@@ -28,23 +27,12 @@ int ModApiStorage::l_get_mod_storage(lua_State *L)
 	// Note that this is wrapped in Lua, see builtin/common/mod_storage.lua
 	std::string mod_name = readParam<std::string>(L, 1);
 
-	ModMetadata *store = nullptr;
-
 	if (IGameDef *gamedef = getGameDef(L)) {
-		store = new ModMetadata(mod_name, gamedef->getModStorageDatabase());
-		if (gamedef->registerModStorage(store)) {
-			StorageRef::create(L, store);
-			int object = lua_gettop(L);
-			lua_pushvalue(L, object);
-			return 1;
-		}
+		StorageRef::create(L, mod_name, gamedef->getModStorageDatabase());
 	} else {
 		assert(false); // this should not happen
-	}
-
-	delete store;
-
 		lua_pushnil(L);
+	}
 	return 1;
 }
 
@@ -53,19 +41,9 @@ void ModApiStorage::Initialize(lua_State *L, int top)
 	API_FCT(get_mod_storage);
 }
 
-StorageRef::StorageRef(ModMetadata *object):
-	m_object(object)
+void StorageRef::create(lua_State *L, const std::string &mod_name, ModMetadataDatabase *db)
 {
-}
-
-StorageRef::~StorageRef()
-{
-	delete m_object;
-}
-
-void StorageRef::create(lua_State *L, ModMetadata *object)
-{
-	StorageRef *o = new StorageRef(object);
+	StorageRef *o = new StorageRef(mod_name, db);
 	*(void **)(lua_newuserdata(L, sizeof(void *))) = o;
 	luaL_getmetatable(L, className);
 	lua_setmetatable(L, -2);
@@ -74,9 +52,6 @@ void StorageRef::create(lua_State *L, ModMetadata *object)
 int StorageRef::gc_object(lua_State *L)
 {
 	StorageRef *o = *(StorageRef **)(lua_touserdata(L, 1));
-	// Server side
-	if (IGameDef *gamedef = getGameDef(L))
-		gamedef->unregisterModStorage(getobject(o)->getModName());
 	delete o;
 	return 0;
 }
@@ -122,20 +97,14 @@ StorageRef* StorageRef::checkobject(lua_State *L, int narg)
 	return *(StorageRef**)ud;  // unbox pointer
 }
 
-ModMetadata* StorageRef::getobject(StorageRef *ref)
+IMetadata* StorageRef::getmeta(bool auto_create)
 {
-	ModMetadata *co = ref->m_object;
-	return co;
-}
-
-Metadata* StorageRef::getmeta(bool auto_create)
-{
-	return m_object;
+	return &m_object;
 }
 
 void StorageRef::clearMeta()
 {
-	m_object->clear();
+	m_object.clear();
 }
 
 const char StorageRef::className[] = "StorageRef";

src/script/lua_api/l_storage.h

@@ -22,8 +22,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 
 #include "l_metadata.h"
 #include "lua_api/l_base.h"
-
-class ModMetadata;
+#include "content/mods.h"
 
 class ModApiStorage : public ModApiBase
 {
@@ -37,24 +36,23 @@ public:
 class StorageRef : public MetaDataRef
 {
 private:
-	ModMetadata *m_object = nullptr;
+	ModMetadata m_object;
 
 	static const char className[];
 	static const luaL_Reg methods[];
 
-	virtual Metadata *getmeta(bool auto_create);
+	virtual IMetadata *getmeta(bool auto_create);
 	virtual void clearMeta();
 
 	// garbage collector
 	static int gc_object(lua_State *L);
 
 public:
-	StorageRef(ModMetadata *object);
-	~StorageRef();
+	StorageRef(const std::string &mod_name, ModMetadataDatabase *db): m_object(mod_name, db) {}
+	~StorageRef() = default;
 
 	static void Register(lua_State *L);
-	static void create(lua_State *L, ModMetadata *object);
+	static void create(lua_State *L, const std::string &mod_name, ModMetadataDatabase *db);
 
 	static StorageRef *checkobject(lua_State *L, int narg);
-	static ModMetadata *getobject(StorageRef *ref);
 };

src/server.cpp

@@ -3882,25 +3882,6 @@ PlayerSAO* Server::emergePlayer(const char *name, session_t peer_id, u16 proto_v
 	return playersao;
 }
 
-bool Server::registerModStorage(ModMetadata *storage)
-{
-	if (m_mod_storages.find(storage->getModName()) != m_mod_storages.end()) {
-		errorstream << "Unable to register same mod storage twice. Storage name: "
-				<< storage->getModName() << std::endl;
-		return false;
-	}
-
-	m_mod_storages[storage->getModName()] = storage;
-	return true;
-}
-
-void Server::unregisterModStorage(const std::string &name)
-{
-	std::unordered_map<std::string, ModMetadata *>::const_iterator it = m_mod_storages.find(name);
-	if (it != m_mod_storages.end())
-		m_mod_storages.erase(name);
-}
-
 void dedicated_server_loop(Server &server, bool &kill)
 {
 	verbosestream<<"dedicated_server_loop()"<<std::endl;

src/server.h

@@ -357,9 +357,6 @@ public:
 
 	void sendDetachedInventories(session_t peer_id, bool incremental);
 
-	virtual bool registerModStorage(ModMetadata *storage);
-	virtual void unregisterModStorage(const std::string &name);
-
 	bool joinModChannel(const std::string &channel);
 	bool leaveModChannel(const std::string &channel);
 	bool sendModChannelMessage(const std::string &channel, const std::string &message);
@@ -695,7 +692,6 @@ private:
 	s32 m_next_sound_id = 0; // positive values only
 	s32 nextSoundId();
 
-	std::unordered_map<std::string, ModMetadata *> m_mod_storages;
 	ModMetadataDatabase *m_mod_storage_database = nullptr;
 	float m_mod_storage_save_timer = 10.0f;
 

src/server/player_sao.h

@@ -181,7 +181,7 @@ public:
 	v3f getEyeOffset() const;
 	float getZoomFOV() const;
 
-	inline Metadata &getMeta() { return m_meta; }
+	inline SimpleMetadata &getMeta() { return m_meta; }
 
 private:
 	std::string getPropertyPacket();
@@ -218,7 +218,7 @@ private:
 	f32 m_fov = 0.0f;
 	s16 m_wanted_range = 0.0f;
 
-	Metadata m_meta;
+	SimpleMetadata m_meta;
 
 public:
 	bool m_physics_override_sent = false;

src/unittest/test_modmetadatadatabase.cpp

@@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc.,
 #include "test.h"
 
 #include <algorithm>
+#include "database/database-dummy.h"
 #include "database/database-files.h"
 #include "database/database-sqlite3.h"
 #include "filesys.h"
@@ -133,14 +134,27 @@ void TestModMetadataDatabase::runTests(IGameDef *gamedef)
 	// fixed directory, for persistence
 	thread_local const std::string test_dir = getTestTempDirectory();
 
-	// Each set of tests is run twice for each database type:
+	// Each set of tests is run twice for each database type except dummy:
 	// one where we reuse the same ModMetadataDatabase object (to test local caching),
 	// and one where we create a new ModMetadataDatabase object for each call
 	// (to test actual persistence).
+	// Since the dummy database is only in-memory, it has no persistence to test.
+
+	ModMetadataDatabase *mod_meta_db;
+
+	rawstream << "-------- Dummy database (same object only)" << std::endl;
+
+	mod_meta_db = new Database_Dummy();
+	mod_meta_provider = new FixedProvider(mod_meta_db);
+
+	runTestsForCurrentDB();
+
+	delete mod_meta_db;
+	delete mod_meta_provider;
 
 	rawstream << "-------- Files database (same object)" << std::endl;
 
-	ModMetadataDatabase *mod_meta_db = new ModMetadataDatabaseFiles(test_dir);
+	mod_meta_db = new ModMetadataDatabaseFiles(test_dir);
 	mod_meta_provider = new FixedProvider(mod_meta_db);
 
 	runTestsForCurrentDB();
@@ -201,6 +215,9 @@ void TestModMetadataDatabase::testRecallFail()
 	StringMap recalled;
 	mod_meta_db->getModEntries("mod1", &recalled);
 	UASSERT(recalled.empty());
+	std::string key1_value;
+	UASSERT(!mod_meta_db->getModEntry("mod1", "key1", &key1_value));
+	UASSERT(!mod_meta_db->hasModEntry("mod1", "key1"));
 }
 
 void TestModMetadataDatabase::testCreate()
@@ -214,8 +231,12 @@ void TestModMetadataDatabase::testRecall()
 	ModMetadataDatabase *mod_meta_db = mod_meta_provider->getModMetadataDatabase();
 	StringMap recalled;
 	mod_meta_db->getModEntries("mod1", &recalled);
-	UASSERT(recalled.size() == 1);
-	UASSERT(recalled["key1"] == "value1");
+	UASSERTCMP(std::size_t, ==, recalled.size(), 1);
+	UASSERTCMP(std::string, ==, recalled["key1"], "value1");
+	std::string key1_value;
+	UASSERT(mod_meta_db->getModEntry("mod1", "key1", &key1_value));
+	UASSERTCMP(std::string, ==, key1_value, "value1");
+	UASSERT(mod_meta_db->hasModEntry("mod1", "key1"));
 }
 
 void TestModMetadataDatabase::testChange()
@@ -229,8 +250,12 @@ void TestModMetadataDatabase::testRecallChanged()
 	ModMetadataDatabase *mod_meta_db = mod_meta_provider->getModMetadataDatabase();
 	StringMap recalled;
 	mod_meta_db->getModEntries("mod1", &recalled);
-	UASSERT(recalled.size() == 1);
-	UASSERT(recalled["key1"] == "value2");
+	UASSERTCMP(std::size_t, ==, recalled.size(), 1);
+	UASSERTCMP(std::string, ==, recalled["key1"], "value2");
+	std::string key1_value;
+	UASSERT(mod_meta_db->getModEntry("mod1", "key1", &key1_value));
+	UASSERTCMP(std::string, ==, key1_value, "value2");
+	UASSERT(mod_meta_db->hasModEntry("mod1", "key1"));
 }
 
 void TestModMetadataDatabase::testListMods()
@@ -239,7 +264,7 @@ void TestModMetadataDatabase::testListMods()
 	UASSERT(mod_meta_db->setModEntry("mod2", "key1", "value1"));
 	std::vector<std::string> mod_list;
 	mod_meta_db->listMods(&mod_list);
-	UASSERT(mod_list.size() == 2);
+	UASSERTCMP(size_t, ==, mod_list.size(), 2);
 	UASSERT(std::find(mod_list.cbegin(), mod_list.cend(), "mod1") != mod_list.cend());
 	UASSERT(std::find(mod_list.cbegin(), mod_list.cend(), "mod2") != mod_list.cend());
 }
@@ -248,4 +273,6 @@ void TestModMetadataDatabase::testRemove()
 {
 	ModMetadataDatabase *mod_meta_db = mod_meta_provider->getModMetadataDatabase();
 	UASSERT(mod_meta_db->removeModEntry("mod1", "key1"));
+	UASSERT(!mod_meta_db->removeModEntries("mod1"));
+	UASSERT(mod_meta_db->removeModEntries("mod2"));
 }