From 6c184947c3886ce80aa9eb9807a700025a344442 Mon Sep 17 00:00:00 2001 From: Loïc Blot Date: Fri, 16 Mar 2018 08:41:33 +0100 Subject: Server: delegate mod management & config to ServerModConfiguration (#7131) * Server: delegate mod management & config to ServerModConfiguration (rename it to ServerModManager) * Use c++11 range based loops * Add unittests + experimental/default mod as a test case to permit testing mod loading in future tests --- src/CMakeLists.txt | 2 + src/gui/guiConfirmRegistration.cpp | 2 +- src/mods.cpp | 14 --- src/mods.h | 22 ++--- src/server.cpp | 62 ++++--------- src/server.h | 4 +- src/server/CMakeLists.txt | 3 + src/server/mods.cpp | 100 ++++++++++++++++++++ src/server/mods.h | 43 +++++++++ src/unittest/CMakeLists.txt | 9 ++ src/unittest/test_config.h.in | 6 ++ src/unittest/test_servermodmanager.cpp | 165 +++++++++++++++++++++++++++++++++ src/unittest/test_world/world.mt | 1 + 13 files changed, 359 insertions(+), 74 deletions(-) create mode 100644 src/server/CMakeLists.txt create mode 100644 src/server/mods.cpp create mode 100644 src/server/mods.h create mode 100644 src/unittest/test_config.h.in create mode 100644 src/unittest/test_servermodmanager.cpp create mode 100644 src/unittest/test_world/world.mt diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f1445d2d9..7673b6fa5 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -379,10 +379,12 @@ add_subdirectory(script) add_subdirectory(unittest) add_subdirectory(util) add_subdirectory(irrlicht_changes) +add_subdirectory(server) set(common_SRCS ${database_SRCS} ${mapgen_SRCS} + ${server_SRCS} ban.cpp chat.cpp clientiface.cpp diff --git a/src/gui/guiConfirmRegistration.cpp b/src/gui/guiConfirmRegistration.cpp index 6228ea29b..81033eebf 100644 --- a/src/gui/guiConfirmRegistration.cpp +++ b/src/gui/guiConfirmRegistration.cpp @@ -113,7 +113,7 @@ void GUIConfirmRegistration::regenerateGui(v2u32 screensize) core::rect rect2(0, 0, 540, 30); rect2 += topleft_client + v2s32(30, ypos); gui::IGUIEditBox *e = Environment->addEditBox(m_pass_confirm.c_str(), - rect2, true, this, ID_confirmPassword); + rect2, true, this, ID_confirmPassword); e->setPasswordBox(true); } diff --git a/src/mods.cpp b/src/mods.cpp index a71644d01..2cfc02b66 100644 --- a/src/mods.cpp +++ b/src/mods.cpp @@ -322,20 +322,6 @@ void ModConfiguration::resolveDependencies() m_unsatisfied_mods.assign(unsatisfied.begin(), unsatisfied.end()); } -ServerModConfiguration::ServerModConfiguration(const std::string &worldpath): - ModConfiguration(worldpath) -{ - SubgameSpec gamespec = findWorldSubgame(worldpath); - - // Add all game mods and all world mods - addModsInPath(gamespec.gamemods_path); - addModsInPath(worldpath + DIR_DELIM + "worldmods"); - - // Load normal mods - std::string worldmt = worldpath + DIR_DELIM + "world.mt"; - addModsFromConfig(worldmt, gamespec.addon_mods_paths); -} - #ifndef SERVER ClientModConfiguration::ClientModConfiguration(const std::string &path): ModConfiguration(path) diff --git a/src/mods.h b/src/mods.h index 9f4d29739..037d6bd1c 100644 --- a/src/mods.h +++ b/src/mods.h @@ -78,7 +78,7 @@ public: return m_unsatisfied_mods.empty(); } - std::vector getMods() + const std::vector &getMods() const { return m_sorted_mods; } @@ -102,6 +102,13 @@ protected: void addModsFromConfig(const std::string &settings_path, const std::set &mods); void checkConflictsAndDeps(); +protected: + // list of mods sorted such that they can be loaded in the + // given order with all dependencies being fullfilled. I.e., + // every mod in this list has only dependencies on mods which + // appear earlier in the vector. + std::vector m_sorted_mods; + private: // move mods from m_unsatisfied_mods to m_sorted_mods // in an order that satisfies dependencies @@ -112,12 +119,6 @@ private: // only the ones with really unsatisfied dependencies. std::vector m_unsatisfied_mods; - // list of mods sorted such that they can be loaded in the - // given order with all dependencies being fullfilled. I.e., - // every mod in this list has only dependencies on mods which - // appear earlier in the vector. - std::vector m_sorted_mods; - // set of mod names for which an unresolved name conflict // exists. A name conflict happens when two or more mods // at the same level have the same name but different paths. @@ -132,13 +133,6 @@ private: }; -class ServerModConfiguration: public ModConfiguration -{ -public: - ServerModConfiguration(const std::string &worldpath); - -}; - #ifndef SERVER class ClientModConfiguration: public ModConfiguration { diff --git a/src/server.cpp b/src/server.cpp index 36d8b49f9..519843e83 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -58,6 +58,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "util/serialize.h" #include "util/thread.h" #include "defaultsettings.h" +#include "server/mods.h" #include "util/base64.h" #include "util/sha1.h" #include "util/hex.h" @@ -203,12 +204,12 @@ Server::Server( std::string ban_path = m_path_world + DIR_DELIM "ipban.txt"; m_banmanager = new BanManager(ban_path); - ServerModConfiguration modconf(m_path_world); - m_mods = modconf.getMods(); - std::vector unsatisfied_mods = modconf.getUnsatisfiedMods(); + m_modmgr = std::unique_ptr(new ServerModManager( + m_path_world)); + std::vector unsatisfied_mods = m_modmgr->getUnsatisfiedMods(); // complain about mods with unsatisfied dependencies - if (!modconf.isConsistent()) { - modconf.printUnsatisfiedModsError(); + if (!m_modmgr->isConsistent()) { + m_modmgr->printUnsatisfiedModsError(); } //lock environment @@ -224,27 +225,9 @@ Server::Server( m_script->loadMod(getBuiltinLuaPath() + DIR_DELIM "init.lua", BUILTIN_MOD_NAME); - // Print mods - infostream << "Server: Loading mods: "; - for (std::vector::const_iterator i = m_mods.begin(); - i != m_mods.end(); ++i) { - infostream << (*i).name << " "; - } - infostream << std::endl; - // Load and run "mod" scripts - for (std::vector::const_iterator it = m_mods.begin(); - it != m_mods.end(); ++it) { - const ModSpec &mod = *it; - if (!string_allowed(mod.name, MODNAME_ALLOWED_CHARS)) { - throw ModError("Error loading mod \"" + mod.name + - "\": Mod name does not follow naming conventions: " - "Only characters [a-z0-9_] are allowed."); - } - std::string script_path = mod.path + DIR_DELIM + "init.lua"; - infostream << " [" << padStringRight(mod.name, 12) << "] [\"" - << script_path << "\"]" << std::endl; - m_script->loadMod(script_path, mod.name); - } + m_mods = m_modmgr->getMods(); + + m_modmgr->loadMods(m_script); // Read Textures and calculate sha1 sums fillMediaCache(); @@ -580,7 +563,7 @@ void Server::AsyncRunStep(bool initial_step) m_lag, m_gamespec.id, Mapgen::getMapgenName(m_emerge->mgparams->mgtype), - m_mods, + m_modmgr->getMods(), m_dedicated); counter = 0.01; } @@ -2237,13 +2220,7 @@ void Server::fillMediaCache() // Collect all media file paths std::vector paths; - for (const ModSpec &mod : m_mods) { - paths.push_back(mod.path + DIR_DELIM + "textures"); - paths.push_back(mod.path + DIR_DELIM + "sounds"); - paths.push_back(mod.path + DIR_DELIM + "media"); - paths.push_back(mod.path + DIR_DELIM + "models"); - paths.push_back(mod.path + DIR_DELIM + "locale"); - } + m_modmgr->getModsMediaPaths(paths); fs::GetRecursiveDirs(paths, m_gamespec.path + DIR_DELIM + "textures"); fs::GetRecursiveDirs(paths, porting::path_user + DIR_DELIM + "textures" + DIR_DELIM + "server"); @@ -3326,22 +3303,19 @@ IWritableCraftDefManager *Server::getWritableCraftDefManager() return m_craftdef; } +const std::vector & Server::getMods() const +{ + return m_modmgr->getMods(); +} + const ModSpec *Server::getModSpec(const std::string &modname) const { - std::vector::const_iterator it; - for (it = m_mods.begin(); it != m_mods.end(); ++it) { - const ModSpec &mod = *it; - if (mod.name == modname) - return &mod; - } - return NULL; + return m_modmgr->getModSpec(modname); } void Server::getModNames(std::vector &modlist) { - std::vector::iterator it; - for (it = m_mods.begin(); it != m_mods.end(); ++it) - modlist.push_back(it->name); + m_modmgr->getModNames(modlist); } std::string Server::getBuiltinLuaPath() diff --git a/src/server.h b/src/server.h index b90971200..847642be9 100644 --- a/src/server.h +++ b/src/server.h @@ -61,6 +61,7 @@ class ServerEnvironment; struct SimpleSoundSpec; struct CloudParams; class ServerThread; +class ServerModManager; enum ClientDeletionReason { CDR_LEAVE, @@ -268,7 +269,7 @@ public: NodeDefManager* getWritableNodeDefManager(); IWritableCraftDefManager* getWritableCraftDefManager(); - virtual const std::vector &getMods() const { return m_mods; } + virtual const std::vector &getMods() const; virtual const ModSpec* getModSpec(const std::string &modname) const; void getModNames(std::vector &modlist); std::string getBuiltinLuaPath(); @@ -541,6 +542,7 @@ private: EventManager *m_event; // Mods + std::unique_ptr m_modmgr; std::vector m_mods; /* diff --git a/src/server/CMakeLists.txt b/src/server/CMakeLists.txt new file mode 100644 index 000000000..b892e83b3 --- /dev/null +++ b/src/server/CMakeLists.txt @@ -0,0 +1,3 @@ +set(server_SRCS + ${CMAKE_CURRENT_SOURCE_DIR}/mods.cpp + PARENT_SCOPE) diff --git a/src/server/mods.cpp b/src/server/mods.cpp new file mode 100644 index 000000000..778241b95 --- /dev/null +++ b/src/server/mods.cpp @@ -0,0 +1,100 @@ +/* +Minetest +Copyright (C) 2018 nerzhul, Loic Blot + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include "mods.h" +#include "filesys.h" +#include "log.h" +#include "scripting_server.h" +#include "subgame.h" + +/** + * Manage server mods + * + * All new calls to this class must be tested in test_servermodmanager.cpp + */ + +/** + * Creates a ServerModManager which targets worldpath + * @param worldpath + */ +ServerModManager::ServerModManager(const std::string &worldpath) : + ModConfiguration(worldpath) +{ + SubgameSpec gamespec = findWorldSubgame(worldpath); + + // Add all game mods and all world mods + addModsInPath(gamespec.gamemods_path); + addModsInPath(worldpath + DIR_DELIM + "worldmods"); + + // Load normal mods + std::string worldmt = worldpath + DIR_DELIM + "world.mt"; + addModsFromConfig(worldmt, gamespec.addon_mods_paths); +} + +// This function cannot be currenctly easily tested but it should be ASAP +void ServerModManager::loadMods(ServerScripting *script) +{ + // Print mods + infostream << "Server: Loading mods: "; + for (const ModSpec &mod : m_sorted_mods) { + infostream << mod.name << " "; + } + infostream << std::endl; + // Load and run "mod" scripts + for (const ModSpec &mod : m_sorted_mods) { + if (!string_allowed(mod.name, MODNAME_ALLOWED_CHARS)) { + throw ModError("Error loading mod \"" + mod.name + + "\": Mod name does not follow naming " + "conventions: " + "Only characters [a-z0-9_] are allowed."); + } + std::string script_path = mod.path + DIR_DELIM + "init.lua"; + infostream << " [" << padStringRight(mod.name, 12) << "] [\"" + << script_path << "\"]" << std::endl; + script->loadMod(script_path, mod.name); + } +} + +const ModSpec *ServerModManager::getModSpec(const std::string &modname) const +{ + std::vector::const_iterator it; + for (it = m_sorted_mods.begin(); it != m_sorted_mods.end(); ++it) { + const ModSpec &mod = *it; + if (mod.name == modname) + return &mod; + } + return NULL; +} + +void ServerModManager::getModNames(std::vector &modlist) const +{ + for (const ModSpec &spec : m_sorted_mods) + modlist.push_back(spec.name); +} + +void ServerModManager::getModsMediaPaths(std::vector &paths) const +{ + for (const ModSpec &spec : m_sorted_mods) { + paths.push_back(spec.path + DIR_DELIM + "textures"); + paths.push_back(spec.path + DIR_DELIM + "sounds"); + paths.push_back(spec.path + DIR_DELIM + "media"); + paths.push_back(spec.path + DIR_DELIM + "models"); + paths.push_back(spec.path + DIR_DELIM + "locale"); + } +} diff --git a/src/server/mods.h b/src/server/mods.h new file mode 100644 index 000000000..9e4b23f30 --- /dev/null +++ b/src/server/mods.h @@ -0,0 +1,43 @@ +/* +Minetest +Copyright (C) 2018 nerzhul, Loic Blot + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#pragma once + +#include "../mods.h" + +class ServerScripting; + +/** + * Manage server mods + * + * All new calls to this class must be tested in test_servermodmanager.cpp + */ +class ServerModManager : public ModConfiguration +{ +public: + /** + * Creates a ServerModManager which targets worldpath + * @param worldpath + */ + ServerModManager(const std::string &worldpath); + void loadMods(ServerScripting *script); + const ModSpec *getModSpec(const std::string &modname) const; + void getModNames(std::vector &modlist) const; + void getModsMediaPaths(std::vector &paths) const; +}; diff --git a/src/unittest/CMakeLists.txt b/src/unittest/CMakeLists.txt index 3373f9d11..aca736f3e 100644 --- a/src/unittest/CMakeLists.txt +++ b/src/unittest/CMakeLists.txt @@ -22,6 +22,7 @@ set (UNITTEST_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/test_serialization.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_settings.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_socket.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/test_servermodmanager.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_threading.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_utilities.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_voxelarea.cpp @@ -33,3 +34,11 @@ set (UNITTEST_CLIENT_SRCS ${CMAKE_CURRENT_SOURCE_DIR}/test_gameui.cpp ${CMAKE_CURRENT_SOURCE_DIR}/test_keycode.cpp PARENT_SCOPE) + +set (TEST_WORLDDIR ${CMAKE_CURRENT_SOURCE_DIR}/test_world) +set (TEST_SUBGAME_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../games/minimal) + +configure_file( + "${CMAKE_CURRENT_SOURCE_DIR}/test_config.h.in" + "${PROJECT_BINARY_DIR}/test_config.h" +) diff --git a/src/unittest/test_config.h.in b/src/unittest/test_config.h.in new file mode 100644 index 000000000..36850b00d --- /dev/null +++ b/src/unittest/test_config.h.in @@ -0,0 +1,6 @@ +// Filled in by the build system + +#pragma once + +#define TEST_WORLDDIR "@TEST_WORLDDIR@" +#define TEST_SUBGAME_PATH "@TEST_SUBGAME_PATH@" diff --git a/src/unittest/test_servermodmanager.cpp b/src/unittest/test_servermodmanager.cpp new file mode 100644 index 000000000..cb43babf9 --- /dev/null +++ b/src/unittest/test_servermodmanager.cpp @@ -0,0 +1,165 @@ +/* +Minetest +Copyright (C) 2018 nerzhul, Loic Blot + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this program; if not, write to the Free Software Foundation, Inc., +51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ + +#include "test.h" +#include +#include "server/mods.h" +#include "test_config.h" + +class TestServerModManager : public TestBase +{ +public: + TestServerModManager() { TestManager::registerTestModule(this); } + const char *getName() { return "TestServerModManager"; } + + void runTests(IGameDef *gamedef); + + void testCreation(); + void testIsConsistent(); + void testUnsatisfiedMods(); + void testGetMods(); + void testGetModsWrongDir(); + void testGetModspec(); + void testGetModNamesWrongDir(); + void testGetModNames(); + void testGetModMediaPathsWrongDir(); + void testGetModMediaPaths(); +}; + +static TestServerModManager g_test_instance; + +void TestServerModManager::runTests(IGameDef *gamedef) +{ + const char *saved_env_mt_subgame_path = getenv("MINETEST_SUBGAME_PATH"); +#ifdef WIN32 + { + std::string subgame_path("MINETEST_SUBGAME_PATH="); + subgame_path.append(TEST_SUBGAME_PATH); + _putenv(subgame_path.c_str()); + } +#else + setenv("MINETEST_SUBGAME_PATH", TEST_SUBGAME_PATH, 1); +#endif + + TEST(testCreation); + TEST(testIsConsistent); + TEST(testGetModsWrongDir); + TEST(testUnsatisfiedMods); + TEST(testGetMods); + TEST(testGetModspec); + TEST(testGetModNamesWrongDir); + TEST(testGetModNames); + TEST(testGetModMediaPathsWrongDir); + TEST(testGetModMediaPaths); + +#ifdef WIN32 + { + std::string subgame_path("MINETEST_SUBGAME_PATH="); + subgame_path.append(saved_env_mt_subgame_path); + _putenv(subgame_path.c_str()); + } +#else + setenv("MINETEST_SUBGAME_PATH", saved_env_mt_subgame_path, 1); +#endif +} + +void TestServerModManager::testCreation() +{ + ServerModManager sm(TEST_WORLDDIR); +} + +void TestServerModManager::testGetModsWrongDir() +{ + // Test in non worlddir to ensure no mods are found + ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + ".."); + UASSERTEQ(bool, sm.getMods().empty(), true); +} + +void TestServerModManager::testUnsatisfiedMods() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + UASSERTEQ(bool, sm.getUnsatisfiedMods().empty(), true); +} + +void TestServerModManager::testIsConsistent() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + UASSERTEQ(bool, sm.isConsistent(), true); +} + +void TestServerModManager::testGetMods() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + const auto &mods = sm.getMods(); + UASSERTEQ(bool, mods.empty(), false); + + // Ensure we found default mod inside the test folder + bool default_found = false; + for (const auto &m : mods) { + if (m.name == "default") + default_found = true; + + // Verify if paths are not empty + UASSERTEQ(bool, m.path.empty(), false); + } + + UASSERTEQ(bool, default_found, true); +} + +void TestServerModManager::testGetModspec() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + UASSERTEQ(const ModSpec *, sm.getModSpec("wrongmod"), NULL); + UASSERT(sm.getModSpec("default") != NULL); +} + +void TestServerModManager::testGetModNamesWrongDir() +{ + ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + ".."); + std::vector result; + sm.getModNames(result); + UASSERTEQ(bool, result.empty(), true); +} + +void TestServerModManager::testGetModNames() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + std::vector result; + sm.getModNames(result); + UASSERTEQ(bool, result.empty(), false); + UASSERT(std::find(result.begin(), result.end(), "default") != result.end()); +} + +void TestServerModManager::testGetModMediaPathsWrongDir() +{ + ServerModManager sm(std::string(TEST_WORLDDIR) + DIR_DELIM + ".."); + std::vector result; + sm.getModsMediaPaths(result); + UASSERTEQ(bool, result.empty(), true); +} + +void TestServerModManager::testGetModMediaPaths() +{ + ServerModManager sm(std::string(TEST_WORLDDIR)); + std::vector result; + sm.getModsMediaPaths(result); + UASSERTEQ(bool, result.empty(), false); + // We should have 5 folders for each mod (textures, media, locale, model, sounds) + UASSERTEQ(unsigned long, result.size() % 5, 0); +} diff --git a/src/unittest/test_world/world.mt b/src/unittest/test_world/world.mt new file mode 100644 index 000000000..ab9b5413a --- /dev/null +++ b/src/unittest/test_world/world.mt @@ -0,0 +1 @@ +gameid = minimal -- cgit v1.2.3