From f4a01f3a5dc0d8fe2f4f6d804d790da91d1bc30c Mon Sep 17 00:00:00 2001 From: Jude Melton-Houghton Date: Mon, 26 Sep 2022 17:03:43 -0400 Subject: [PATCH] Avoid duplication of mod metadata in memory (#12562) Co-authored-by: sfan5 --- builtin/game/features.lua | 1 + doc/lua_api.txt | 4 + games/devtest/mods/unittests/init.lua | 1 + games/devtest/mods/unittests/metadata.lua | 79 +++++++++++++ src/client/client.cpp | 20 ---- src/client/client.h | 4 - src/content/mods.cpp | 36 +++--- src/content/mods.h | 16 ++- src/database/database-dummy.cpp | 36 ++++++ src/database/database-dummy.h | 4 + src/database/database-files.cpp | 31 +++++ src/database/database-files.h | 4 + src/database/database-sqlite3.cpp | 75 ++++++++++-- src/database/database-sqlite3.h | 7 ++ src/database/database.h | 4 + src/dummygamedef.h | 2 - src/gamedef.h | 2 - src/itemstackmetadata.cpp | 4 +- src/itemstackmetadata.h | 2 +- src/metadata.cpp | 137 ++++++++++++---------- src/metadata.h | 84 ++++++++++--- src/nodemetadata.cpp | 4 +- src/nodemetadata.h | 2 +- src/script/lua_api/l_itemstackmeta.cpp | 2 +- src/script/lua_api/l_itemstackmeta.h | 2 +- src/script/lua_api/l_metadata.cpp | 61 +++++----- src/script/lua_api/l_metadata.h | 8 +- src/script/lua_api/l_nodemeta.cpp | 16 +-- src/script/lua_api/l_nodemeta.h | 12 +- src/script/lua_api/l_playermeta.cpp | 4 +- src/script/lua_api/l_playermeta.h | 8 +- src/script/lua_api/l_storage.cpp | 45 ++----- src/script/lua_api/l_storage.h | 14 +-- src/server.cpp | 19 --- src/server.h | 4 - src/server/player_sao.h | 4 +- src/unittest/test_modmetadatadatabase.cpp | 41 +++++-- 37 files changed, 527 insertions(+), 272 deletions(-) create mode 100644 games/devtest/mods/unittests/metadata.lua diff --git a/builtin/game/features.lua b/builtin/game/features.lua index 896a893b2..bdc1602a2 100644 --- a/builtin/game/features.lua +++ b/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) diff --git a/doc/lua_api.txt b/doc/lua_api.txt index 1982654c4..a801c6019 100644 --- a/doc/lua_api.txt +++ b/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` diff --git a/games/devtest/mods/unittests/init.lua b/games/devtest/mods/unittests/init.lua index f93d516d7..936f05c6d 100644 --- a/games/devtest/mods/unittests/init.lua +++ b/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") -------------- diff --git a/games/devtest/mods/unittests/metadata.lua b/games/devtest/mods/unittests/metadata.lua new file mode 100644 index 000000000..477ea8037 --- /dev/null +++ b/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}) diff --git a/src/client/client.cpp b/src/client/client.cpp index 35634e77b..a0e867ae4 100644 --- a/src/client/client.cpp +++ b/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::const_iterator it = - m_mod_storages.find(name); - if (it != m_mod_storages.end()) - m_mod_storages.erase(name); -} - /* * Mod channels */ diff --git a/src/client/client.h b/src/client/client.h index bdcc2a3dd..4342a0087 100644 --- a/src/client/client.h +++ b/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 m_mod_storages; ModMetadataDatabase *m_mod_storage_database = nullptr; float m_mod_storage_save_timer = 10.0f; std::vector m_mods; diff --git a/src/content/mods.cpp b/src/content/mods.cpp index cec6fc2cd..ef268055f 100644 --- a/src/content/mods.cpp +++ b/src/content/mods.cpp @@ -220,26 +220,34 @@ std::vector flattenMods(const std::map &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); - } - Metadata::clear(); + m_database->removeModEntries(m_mod_name); +} + +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); - } - return true; - } - return false; + if (var.empty()) + return m_database->removeModEntry(m_mod_name, name); + else + return m_database->setModEntry(m_mod_name, name, var); +} + +const StringMap &ModMetadata::getStrings(StringMap *place) const +{ + place->clear(); + m_database->getModEntries(m_mod_name, place); + return *place; +} + +const std::string *ModMetadata::getStringRaw(const std::string &name, std::string *place) const +{ + return m_database->getModEntry(m_mod_name, name, place) ? place : nullptr; } diff --git a/src/content/mods.h b/src/content/mods.h index 5ed5b8171..0b0e26b50 100644 --- a/src/content/mods.h +++ b/src/content/mods.h @@ -110,18 +110,26 @@ std::map getModsInPath(const std::string &path, std::vector flattenMods(const std::map &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; diff --git a/src/database/database-dummy.cpp b/src/database/database-dummy.cpp index a44762d8a..ee88c6684 100644 --- a/src/database/database-dummy.cpp +++ b/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 *res) { for (const auto &pair : m_mod_meta_database) { diff --git a/src/database/database-dummy.h b/src/database/database-dummy.h index 44b9e8d68..86c680ef1 100644 --- a/src/database/database-dummy.h +++ b/src/database/database-dummy.h @@ -38,9 +38,13 @@ public: void listPlayers(std::vector &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 *res); void beginSave() {} diff --git a/src/database/database-files.cpp b/src/database/database-files.cpp index 00bc6d765..eff08de12 100644 --- a/src/database/database-files.cpp +++ b/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() { } diff --git a/src/database/database-files.h b/src/database/database-files.h index 962e4d7bb..089e071fe 100644 --- a/src/database/database-files.h +++ b/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 *res); virtual void beginSave(); diff --git a/src/database/database-sqlite3.cpp b/src/database/database-sqlite3.cpp index 5e565be55..0406fadc8 100644 --- a/src/database/database-sqlite3.cpp +++ b/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 *res) { verifyDatabase(); diff --git a/src/database/database-sqlite3.h b/src/database/database-sqlite3.h index 5e3d7c96c..566f6cebb 100644 --- a/src/database/database-sqlite3.h +++ b/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 *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; }; diff --git a/src/database/database.h b/src/database/database.h index fbb5befea..0f22deebc 100644 --- a/src/database/database.h +++ b/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 *res) = 0; }; diff --git a/src/dummygamedef.h b/src/dummygamedef.h index 97281ffcd..b27e6e64c 100644 --- a/src/dummygamedef.h +++ b/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; } diff --git a/src/gamedef.h b/src/gamedef.h index 45b9c4750..690ff1257 100644 --- a/src/gamedef.h +++ b/src/gamedef.h @@ -73,8 +73,6 @@ public: virtual const std::vector &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; diff --git a/src/itemstackmetadata.cpp b/src/itemstackmetadata.cpp index 529e0149f..c8ce2449f 100644 --- a/src/itemstackmetadata.cpp +++ b/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; diff --git a/src/itemstackmetadata.h b/src/itemstackmetadata.h index a7f134955..48a029c51 100644 --- a/src/itemstackmetadata.h +++ b/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) {} diff --git a/src/metadata.cpp b/src/metadata.cpp index 453ac1c9a..c0ae9a404 100644 --- a/src/metadata.cpp +++ b/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 { - m_stringvars.clear(); - m_modified = true; -} + StringMap this_map_, other_map_; + const StringMap &this_map = getStrings(&this_map_); + const StringMap &other_map = other.getStrings(&other_map_); -bool Metadata::empty() const -{ - return m_stringvars.empty(); -} - -size_t Metadata::size() const -{ - return m_stringvars.size(); -} - -bool Metadata::contains(const std::string &name) const -{ - return m_stringvars.find(name) != m_stringvars.end(); -} - -bool Metadata::operator==(const Metadata &other) const -{ - if (size() != other.size()) + if (this_map.size() != other_map.size()) return false; - for (const auto &sv : m_stringvars) { - if (!other.contains(sv.first) || other.getString(sv.first) != sv.second) + 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 &Metadata::getString(const std::string &name, u16 recursion) const +const std::string &IMetadata::getString(const std::string &name, std::string *place, + u16 recursion) const { - StringMap::const_iterator it = m_stringvars.find(name); - if (it == m_stringvars.end()) { + const std::string *raw = getStringRaw(name, place); + if (!raw) { static const std::string empty_string = std::string(""); return empty_string; } - return resolveString(it->second, recursion); + return resolveString(*raw, place, recursion); } -bool Metadata::getStringToRef( - const std::string &name, std::string &str, u16 recursion) const +bool IMetadata::getStringToRef(const std::string &name, + std::string &str, u16 recursion) const { - StringMap::const_iterator it = m_stringvars.find(name); - if (it == m_stringvars.end()) { + const std::string *raw = getStringRaw(name, &str); + if (!raw) return false; - } - str = resolveString(it->second, recursion); + const std::string &resolved = resolveString(*raw, &str, recursion); + if (&resolved != &str) + str = resolved; 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) -{ - if (var.empty()) { - m_stringvars.erase(name); - return true; - } - - 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 +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] == '}') { - return getString(str.substr(2, str.length() - 3), recursion + 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 SimpleMetadata::empty() const +{ + return m_stringvars.empty(); +} + +size_t SimpleMetadata::size() const +{ + return m_stringvars.size(); +} + +bool SimpleMetadata::contains(const std::string &name) const +{ + return m_stringvars.find(name) != m_stringvars.end(); +} + +const StringMap &SimpleMetadata::getStrings(StringMap *) const +{ + return m_stringvars; +} + +const std::string *SimpleMetadata::getStringRaw(const std::string &name, std::string *) const +{ + const auto found = m_stringvars.find(name); + return found != m_stringvars.cend() ? &found->second : nullptr; +} + +bool SimpleMetadata::setString(const std::string &name, const std::string &var) +{ + if (var.empty()) { + 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; +} diff --git a/src/metadata.h b/src/metadata.h index 5333f8a9d..df61b3f7a 100644 --- a/src/metadata.h +++ b/src/metadata.h @@ -24,17 +24,16 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #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 +{ + 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 m_stringvars; + 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; }; diff --git a/src/nodemetadata.cpp b/src/nodemetadata.cpp index b5052c3b8..a2de1eac4 100644 --- a/src/nodemetadata.cpp +++ b/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(); } diff --git a/src/nodemetadata.h b/src/nodemetadata.h index 4b5b4d887..a791cc5df 100644 --- a/src/nodemetadata.h +++ b/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); diff --git a/src/script/lua_api/l_itemstackmeta.cpp b/src/script/lua_api/l_itemstackmeta.cpp index c17bb8995..5bae21a40 100644 --- a/src/script/lua_api/l_itemstackmeta.cpp +++ b/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; } diff --git a/src/script/lua_api/l_itemstackmeta.h b/src/script/lua_api/l_itemstackmeta.h index 68d2ba8fa..701d04020 100644 --- a/src/script/lua_api/l_itemstackmeta.h +++ b/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(); diff --git a/src/script/lua_api/l_metadata.cpp b/src/script/lua_api/l_metadata.cpp index d00cb4daa..5f07989eb 100644 --- a/src/script/lua_api/l_metadata.cpp +++ b/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,12 +119,9 @@ 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); - ref->reportMetadataChange(&name); + 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,12 +155,9 @@ 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); - ref->reportMetadataChange(&name); + 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,12 +191,9 @@ int MetaDataRef::l_set_float(lua_State *L) float a = readParam(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); - ref->reportMetadataChange(&name); + 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 diff --git a/src/script/lua_api/l_metadata.h b/src/script/lua_api/l_metadata.h index e655eb684..b61957642 100644 --- a/src/script/lua_api/l_metadata.h +++ b/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 diff --git a/src/script/lua_api/l_nodemeta.cpp b/src/script/lua_api/l_nodemeta.cpp index bdc4844c0..e28fc2669 100644 --- a/src/script/lua_api/l_nodemeta.cpp +++ b/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(_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(_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; diff --git a/src/script/lua_api/l_nodemeta.h b/src/script/lua_api/l_nodemeta.h index 265ece3d0..40df9438d 100644 --- a/src/script/lua_api/l_nodemeta.h +++ b/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); diff --git a/src/script/lua_api/l_playermeta.cpp b/src/script/lua_api/l_playermeta.cpp index 2706c99df..0fe308e38 100644 --- a/src/script/lua_api/l_playermeta.cpp +++ b/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; diff --git a/src/script/lua_api/l_playermeta.h b/src/script/lua_api/l_playermeta.h index 9e23c071c..e8ac08894 100644 --- a/src/script/lua_api/l_playermeta.h +++ b/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); }; diff --git a/src/script/lua_api/l_storage.cpp b/src/script/lua_api/l_storage.cpp index b6c53e353..d1cb1fa9c 100644 --- a/src/script/lua_api/l_storage.cpp +++ b/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(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 + lua_pushnil(L); } - - 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"; diff --git a/src/script/lua_api/l_storage.h b/src/script/lua_api/l_storage.h index bfcf5ef38..ddf078aa8 100644 --- a/src/script/lua_api/l_storage.h +++ b/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); }; diff --git a/src/server.cpp b/src/server.cpp index f27914b37..36efe8ec4 100644 --- a/src/server.cpp +++ b/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::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()"< m_mod_storages; ModMetadataDatabase *m_mod_storage_database = nullptr; float m_mod_storage_save_timer = 10.0f; diff --git a/src/server/player_sao.h b/src/server/player_sao.h index 049e92cda..bd190d323 100644 --- a/src/server/player_sao.h +++ b/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; diff --git a/src/unittest/test_modmetadatadatabase.cpp b/src/unittest/test_modmetadatadatabase.cpp index 047e6f711..99b33d7d4 100644 --- a/src/unittest/test_modmetadatadatabase.cpp +++ b/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 +#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 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")); }