From 5bf72468f3a0925a9fc3c9acacf3f6e138bff35e Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 29 May 2021 11:44:48 +0200 Subject: UnitSAO: Prevent circular attachments --- src/server/unit_sao.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'src/server') diff --git a/src/server/unit_sao.cpp b/src/server/unit_sao.cpp index fa6c8f0f4..acbdd478a 100644 --- a/src/server/unit_sao.cpp +++ b/src/server/unit_sao.cpp @@ -124,6 +124,19 @@ void UnitSAO::sendOutdatedData() void UnitSAO::setAttachment(int parent_id, const std::string &bone, v3f position, v3f rotation, bool force_visible) { + auto *obj = parent_id ? m_env->getActiveObject(parent_id) : nullptr; + if (obj) { + // Do checks to avoid circular references + // The chain of wanted parent must not refer or contain "this" + for (obj = obj->getParent(); obj; obj = obj->getParent()) { + if (obj == this) { + warningstream << "Mod bug: Attempted to attach object " << m_id << " to parent " + << parent_id << " but former is an (in)direct parent of latter." << std::endl; + return; + } + } + } + // Attachments need to be handled on both the server and client. // If we just attach on the server, we can only copy the position of the parent. // Attachments are still sent to clients at an interval so players might see them -- cgit v1.2.3 From b93bbfde2c0f6f6217ed3e358ed898049f98e448 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 10 Jul 2021 14:18:35 +0200 Subject: Script API: Fix segfault in remove_detached_inventory when minetest.remove_detached_inventory is called on script init, the environment is yet not set up, hence m_env is still nullptr until all scripts are loaded --- src/server/serverinventorymgr.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/server') diff --git a/src/server/serverinventorymgr.cpp b/src/server/serverinventorymgr.cpp index 2a80c9bbe..3aee003b4 100644 --- a/src/server/serverinventorymgr.cpp +++ b/src/server/serverinventorymgr.cpp @@ -157,8 +157,8 @@ bool ServerInventoryManager::removeDetachedInventory(const std::string &name) m_env->getGameDef()->sendDetachedInventory( nullptr, name, player->getPeerId()); - } else { - // Notify all players about the change + } else if (m_env) { + // Notify all players about the change as soon ServerEnv exists m_env->getGameDef()->sendDetachedInventory( nullptr, name, PEER_ID_INEXISTENT); } -- cgit v1.2.3 From 32cb9d0828828da3068259c9e0a3c0f5da170439 Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Sat, 31 Jul 2021 19:54:52 +0200 Subject: Mods: Combine mod loading checks and deprection logging (#11503) This limits the logged deprecation messages to the mods that are loaded Unifies the mod naming convention check for CSM & SSM --- src/client/client.cpp | 6 +----- src/content/mods.cpp | 44 ++++++++++++++++++++++++++------------------ src/content/mods.h | 5 +++++ src/server/mods.cpp | 8 ++------ 4 files changed, 34 insertions(+), 29 deletions(-) (limited to 'src/server') diff --git a/src/client/client.cpp b/src/client/client.cpp index 17661c242..923369afe 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -177,11 +177,7 @@ void Client::loadMods() // Load "mod" scripts for (const ModSpec &mod : m_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."); - } + mod.checkAndLog(); scanModIntoMemory(mod.name, mod.path); } diff --git a/src/content/mods.cpp b/src/content/mods.cpp index 434004b29..6f088a5b3 100644 --- a/src/content/mods.cpp +++ b/src/content/mods.cpp @@ -30,6 +30,29 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "convert_json.h" #include "script/common/c_internal.h" +void ModSpec::checkAndLog() const +{ + if (!string_allowed(name, MODNAME_ALLOWED_CHARS)) { + throw ModError("Error loading mod \"" + name + + "\": Mod name does not follow naming conventions: " + "Only characters [a-z0-9_] are allowed."); + } + + // Log deprecation messages + auto handling_mode = get_deprecated_handling_mode(); + if (!deprecation_msgs.empty() && handling_mode != DeprecatedHandlingMode::Ignore) { + std::ostringstream os; + os << "Mod " << name << " at " << path << ":" << std::endl; + for (auto msg : deprecation_msgs) + os << "\t" << msg << std::endl; + + if (handling_mode == DeprecatedHandlingMode::Error) + throw ModError(os.str()); + else + warningstream << os.str(); + } +} + bool parseDependsString(std::string &dep, std::unordered_set &symbols) { dep = trim(dep); @@ -45,21 +68,6 @@ bool parseDependsString(std::string &dep, std::unordered_set &symbols) return !dep.empty(); } -static void log_mod_deprecation(const ModSpec &spec, const std::string &warning) -{ - auto handling_mode = get_deprecated_handling_mode(); - if (handling_mode != DeprecatedHandlingMode::Ignore) { - std::ostringstream os; - os << warning << " (" << spec.name << " at " << spec.path << ")" << std::endl; - - if (handling_mode == DeprecatedHandlingMode::Error) { - throw ModError(os.str()); - } else { - warningstream << os.str(); - } - } -} - void parseModContents(ModSpec &spec) { // NOTE: this function works in mutual recursion with getModsInPath @@ -89,7 +97,7 @@ void parseModContents(ModSpec &spec) if (info.exists("name")) spec.name = info.get("name"); else - log_mod_deprecation(spec, "Mods not having a mod.conf file with the name is deprecated."); + spec.deprecation_msgs.push_back("Mods not having a mod.conf file with the name is deprecated."); if (info.exists("author")) spec.author = info.get("author"); @@ -130,7 +138,7 @@ void parseModContents(ModSpec &spec) std::ifstream is((spec.path + DIR_DELIM + "depends.txt").c_str()); if (is.good()) - log_mod_deprecation(spec, "depends.txt is deprecated, please use mod.conf instead."); + spec.deprecation_msgs.push_back("depends.txt is deprecated, please use mod.conf instead."); while (is.good()) { std::string dep; @@ -153,7 +161,7 @@ void parseModContents(ModSpec &spec) if (info.exists("description")) spec.desc = info.get("description"); else if (fs::ReadFile(spec.path + DIR_DELIM + "description.txt", spec.desc)) - log_mod_deprecation(spec, "description.txt is deprecated, please use mod.conf instead."); + spec.deprecation_msgs.push_back("description.txt is deprecated, please use mod.conf instead."); } } diff --git a/src/content/mods.h b/src/content/mods.h index b3500fbc8..b56a97edb 100644 --- a/src/content/mods.h +++ b/src/content/mods.h @@ -49,6 +49,9 @@ struct ModSpec bool part_of_modpack = false; bool is_modpack = false; + // For logging purposes + std::vector deprecation_msgs; + // if modpack: std::map modpack_content; ModSpec(const std::string &name = "", const std::string &path = "") : @@ -59,6 +62,8 @@ struct ModSpec name(name), path(path), part_of_modpack(part_of_modpack) { } + + void checkAndLog() const; }; // Retrieves depends, optdepends, is_modpack and modpack_content diff --git a/src/server/mods.cpp b/src/server/mods.cpp index 83fa12da9..609d8c346 100644 --- a/src/server/mods.cpp +++ b/src/server/mods.cpp @@ -61,12 +61,8 @@ void ServerModManager::loadMods(ServerScripting *script) 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."); - } + mod.checkAndLog(); + std::string script_path = mod.path + DIR_DELIM + "init.lua"; auto t = porting::getTimeMs(); script->loadMod(script_path, mod.name); -- cgit v1.2.3 From 3f1adb49ae8da5bb02bea52609524d3645b6a665 Mon Sep 17 00:00:00 2001 From: savilli <78875209+savilli@users.noreply.github.com> Date: Sat, 28 Aug 2021 12:14:16 +0200 Subject: Remove redundant on_dieplayer calls --- src/network/serverpackethandler.cpp | 16 ---------------- src/script/common/c_content.cpp | 2 -- src/script/lua_api/l_object.cpp | 18 ------------------ src/server.cpp | 3 +-- src/server/player_sao.cpp | 4 ++-- 5 files changed, 3 insertions(+), 40 deletions(-) (limited to 'src/server') diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp index dc5864be3..77fde2a66 100644 --- a/src/network/serverpackethandler.cpp +++ b/src/network/serverpackethandler.cpp @@ -828,7 +828,6 @@ void Server::handleCommand_Damage(NetworkPacket* pkt) PlayerHPChangeReason reason(PlayerHPChangeReason::FALL); playersao->setHP((s32)playersao->getHP() - (s32)damage, reason); - SendPlayerHPOrDie(playersao, reason); } } @@ -1113,9 +1112,6 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) float time_from_last_punch = playersao->resetTimeFromLastPunch(); - u16 src_original_hp = pointed_object->getHP(); - u16 dst_origin_hp = playersao->getHP(); - u16 wear = pointed_object->punch(dir, &toolcap, playersao, time_from_last_punch); @@ -1125,18 +1121,6 @@ void Server::handleCommand_Interact(NetworkPacket *pkt) if (changed) playersao->setWieldedItem(selected_item); - // If the object is a player and its HP changed - if (src_original_hp != pointed_object->getHP() && - pointed_object->getType() == ACTIVEOBJECT_TYPE_PLAYER) { - SendPlayerHPOrDie((PlayerSAO *)pointed_object, - PlayerHPChangeReason(PlayerHPChangeReason::PLAYER_PUNCH, playersao)); - } - - // If the puncher is a player and its HP changed - if (dst_origin_hp != playersao->getHP()) - SendPlayerHPOrDie(playersao, - PlayerHPChangeReason(PlayerHPChangeReason::PLAYER_PUNCH, pointed_object)); - return; } // action == INTERACT_START_DIGGING diff --git a/src/script/common/c_content.cpp b/src/script/common/c_content.cpp index f13287375..5a095fd8f 100644 --- a/src/script/common/c_content.cpp +++ b/src/script/common/c_content.cpp @@ -200,8 +200,6 @@ void read_object_properties(lua_State *L, int index, if (prop->hp_max < sao->getHP()) { PlayerHPChangeReason reason(PlayerHPChangeReason::SET_HP); sao->setHP(prop->hp_max, reason); - if (sao->getType() == ACTIVEOBJECT_TYPE_PLAYER) - sao->getEnv()->getGameDef()->SendPlayerHPOrDie((PlayerSAO *)sao, reason); } } diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp index c8fa7d806..b7185f7ec 100644 --- a/src/script/lua_api/l_object.cpp +++ b/src/script/lua_api/l_object.cpp @@ -172,27 +172,11 @@ int ObjectRef::l_punch(lua_State *L) float time_from_last_punch = readParam(L, 3, 1000000.0f); ToolCapabilities toolcap = read_tool_capabilities(L, 4); v3f dir = readParam(L, 5, sao->getBasePosition() - puncher->getBasePosition()); - dir.normalize(); - u16 src_original_hp = sao->getHP(); - u16 dst_origin_hp = puncher->getHP(); u16 wear = sao->punch(dir, &toolcap, puncher, time_from_last_punch); lua_pushnumber(L, wear); - // If the punched is a player, and its HP changed - if (src_original_hp != sao->getHP() && - sao->getType() == ACTIVEOBJECT_TYPE_PLAYER) { - getServer(L)->SendPlayerHPOrDie((PlayerSAO *)sao, - PlayerHPChangeReason(PlayerHPChangeReason::PLAYER_PUNCH, puncher)); - } - - // If the puncher is a player, and its HP changed - if (dst_origin_hp != puncher->getHP() && - puncher->getType() == ACTIVEOBJECT_TYPE_PLAYER) { - getServer(L)->SendPlayerHPOrDie((PlayerSAO *)puncher, - PlayerHPChangeReason(PlayerHPChangeReason::PLAYER_PUNCH, sao)); - } return 1; } @@ -238,8 +222,6 @@ int ObjectRef::l_set_hp(lua_State *L) } sao->setHP(hp, reason); - if (sao->getType() == ACTIVEOBJECT_TYPE_PLAYER) - getServer(L)->SendPlayerHPOrDie((PlayerSAO *)sao, reason); if (reason.hasLuaReference()) luaL_unref(L, LUA_REGISTRYINDEX, reason.lua_reference); return 0; diff --git a/src/server.cpp b/src/server.cpp index 8339faa76..b96db1209 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -1079,8 +1079,7 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id) if (playersao->isDead()) SendDeathscreen(peer_id, false, v3f(0,0,0)); else - SendPlayerHPOrDie(playersao, - PlayerHPChangeReason(PlayerHPChangeReason::SET_HP)); + SendPlayerHP(peer_id); // Send Breath SendPlayerBreath(playersao); diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp index 0d31f2e0b..d4d036726 100644 --- a/src/server/player_sao.cpp +++ b/src/server/player_sao.cpp @@ -167,7 +167,6 @@ void PlayerSAO::step(float dtime, bool send_recommended) if (m_breath == 0) { PlayerHPChangeReason reason(PlayerHPChangeReason::DROWNING); setHP(m_hp - c.drowning, reason); - m_env->getGameDef()->SendPlayerHPOrDie(this, reason); } } } @@ -216,7 +215,6 @@ void PlayerSAO::step(float dtime, bool send_recommended) s32 newhp = (s32)m_hp - (s32)damage_per_second; PlayerHPChangeReason reason(PlayerHPChangeReason::NODE_DAMAGE, nodename); setHP(newhp, reason); - m_env->getGameDef()->SendPlayerHPOrDie(this, reason); } } @@ -491,6 +489,8 @@ void PlayerSAO::setHP(s32 hp, const PlayerHPChangeReason &reason) // Update properties on death if ((hp == 0) != (oldhp == 0)) m_properties_sent = false; + + m_env->getGameDef()->SendPlayerHPOrDie(this, reason); } void PlayerSAO::setBreath(const u16 breath, bool send) -- cgit v1.2.3