From 85e717fcd1081278fc0c91cc4bba79e4e59d7aca Mon Sep 17 00:00:00 2001 From: sfan5 Date: Mon, 12 Aug 2024 15:32:18 +0200 Subject: [PATCH] Rework object attachment handling to fix bugs (#14825) --- doc/lua_api.md | 11 +- games/devtest/mods/unittests/entity.lua | 30 ++++ src/activeobject.h | 28 ++-- src/client/clientenvironment.cpp | 2 +- src/client/clientobject.h | 4 +- src/client/content_cao.cpp | 30 ++-- src/client/content_cao.h | 15 +- src/script/lua_api/l_object.cpp | 20 +-- src/server.cpp | 3 - src/server/clientiface.cpp | 20 +-- src/server/luaentity_sao.cpp | 4 +- src/server/luaentity_sao.h | 10 +- src/server/player_sao.cpp | 5 +- src/server/serveractiveobject.h | 8 +- src/server/unit_sao.cpp | 177 ++++++++++++++---------- src/server/unit_sao.h | 39 ++++-- src/util/numeric.h | 11 ++ 17 files changed, 245 insertions(+), 172 deletions(-) diff --git a/doc/lua_api.md b/doc/lua_api.md index fd21fb3f1..e5435e273 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -5100,12 +5100,15 @@ Callbacks: to the object (not necessarily an actual rightclick) * `clicker`: an `ObjectRef` (may or may not be a player) * `on_attach_child(self, child)` - * `child`: an `ObjectRef` of the child that attaches + * Called after another object is attached to this object. + * `child`: an `ObjectRef` of the child * `on_detach_child(self, child)` - * `child`: an `ObjectRef` of the child that detaches + * Called after another object has detached from this object. + * `child`: an `ObjectRef` of the child * `on_detach(self, parent)` - * `parent`: an `ObjectRef` (can be `nil`) from where it got detached - * This happens before the parent object is removed from the world + * Called after detaching from another object. + * `parent`: an `ObjectRef` from where it got detached + * Note: this is also called before removal from the world. * `get_staticdata(self)` * Should return a string that will be passed to `on_activate` when the object is instantiated the next time. diff --git a/games/devtest/mods/unittests/entity.lua b/games/devtest/mods/unittests/entity.lua index 38d026663..8e8bd1c48 100644 --- a/games/devtest/mods/unittests/entity.lua +++ b/games/devtest/mods/unittests/entity.lua @@ -40,12 +40,36 @@ core.register_entity("unittests:callbacks", { end, on_attach_child = function(self, child) insert_log("on_attach_child(%s)", objref_str(self, child)) + assert(child:get_attach() == self.object) + local ok = false + for _, obj in ipairs(self.object:get_children()) do + if obj == child then + ok = true + end + end + assert(ok, "Child not found in get_children") end, on_detach_child = function(self, child) insert_log("on_detach_child(%s)", objref_str(self, child)) + assert(child:get_attach() == nil) + local ok = true + for _, obj in ipairs(self.object:get_children()) do + if obj == child then + ok = false + end + end + assert(ok, "Former child found in get_children") end, on_detach = function(self, parent) insert_log("on_detach(%s)", objref_str(self, parent)) + assert(self.object:get_attach() == nil) + local ok = true + for _, obj in ipairs(parent:get_children()) do + if obj == self.object then + ok = false + end + end + assert(ok, "Former child found in get_children") end, get_staticdata = function(self) assert(false) @@ -118,19 +142,25 @@ local function test_entity_attach(player, pos) -- attach player to entity player:set_attach(obj) check_log({"on_attach_child(player)"}) + assert(player:get_attach() == obj) player:set_detach() check_log({"on_detach_child(player)"}) + assert(player:get_attach() == nil) -- attach entity to player obj:set_attach(player) check_log({}) + assert(obj:get_attach() == player) obj:set_detach() check_log({"on_detach(player)"}) + assert(obj:get_attach() == nil) obj:remove() end unittests.register("test_entity_attach", test_entity_attach, {player=true, map=true}) +--------- + core.register_entity("unittests:dummy", { initial_properties = { hp_max = 1, diff --git a/src/activeobject.h b/src/activeobject.h index 52f997fdf..989b48e91 100644 --- a/src/activeobject.h +++ b/src/activeobject.h @@ -152,17 +152,19 @@ typedef std::unordered_map BoneOverrideMap; class ActiveObject { public: - ActiveObject(u16 id): + typedef u16 object_t; + + ActiveObject(object_t id): m_id(id) { } - u16 getId() const + object_t getId() const { return m_id; } - void setId(u16 id) + void setId(object_t id) { m_id = id; } @@ -193,14 +195,22 @@ public: virtual bool collideWithObjects() const = 0; - virtual void setAttachment(int parent_id, const std::string &bone, v3f position, + virtual void setAttachment(object_t parent_id, const std::string &bone, v3f position, v3f rotation, bool force_visible) {} - virtual void getAttachment(int *parent_id, std::string *bone, v3f *position, + virtual void getAttachment(object_t *parent_id, std::string *bone, v3f *position, v3f *rotation, bool *force_visible) const {} + // Detach all children virtual void clearChildAttachments() {} - virtual void clearParentAttachment() {} - virtual void addAttachmentChild(int child_id) {} - virtual void removeAttachmentChild(int child_id) {} + // Detach from parent + virtual void clearParentAttachment() + { + setAttachment(0, "", v3f(), v3f(), false); + } + + // To be be called from setAttachment() and descendants, but not manually! + virtual void addAttachmentChild(object_t child_id) {} + virtual void removeAttachmentChild(object_t child_id) {} + protected: - u16 m_id; // 0 is invalid, "no id" + object_t m_id; // 0 is invalid, "no id" }; diff --git a/src/client/clientenvironment.cpp b/src/client/clientenvironment.cpp index 7e1676ffe..f1ec14d3b 100644 --- a/src/client/clientenvironment.cpp +++ b/src/client/clientenvironment.cpp @@ -368,7 +368,7 @@ void ClientEnvironment::addActiveObject(u16 id, u8 type, void ClientEnvironment::removeActiveObject(u16 id) { // Get current attachment childs to detach them visually - std::unordered_set attachment_childs; + std::unordered_set attachment_childs; if (auto *obj = getActiveObject(id)) attachment_childs = obj->getAttachmentChildIds(); diff --git a/src/client/clientobject.h b/src/client/clientobject.h index f63681313..f02815e04 100644 --- a/src/client/clientobject.h +++ b/src/client/clientobject.h @@ -57,8 +57,8 @@ public: virtual bool isLocalPlayer() const { return false; } virtual ClientActiveObject *getParent() const { return nullptr; }; - virtual const std::unordered_set &getAttachmentChildIds() const - { static std::unordered_set rv; return rv; } + virtual const std::unordered_set &getAttachmentChildIds() const + { static std::unordered_set rv; return rv; } virtual void updateAttachments() {}; virtual bool doShowSelectionBox() { return true; } diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp index 0044cc16e..d24dc8433 100644 --- a/src/client/content_cao.cpp +++ b/src/client/content_cao.cpp @@ -465,7 +465,7 @@ scene::IAnimatedMeshSceneNode *GenericCAO::getAnimatedMeshSceneNode() const void GenericCAO::setChildrenVisible(bool toset) { - for (u16 cao_id : m_attachment_child_ids) { + for (object_t cao_id : m_attachment_child_ids) { GenericCAO *obj = m_env->getGenericCAO(cao_id); if (obj) { // Check if the entity is forced to appear in first person. @@ -474,10 +474,10 @@ void GenericCAO::setChildrenVisible(bool toset) } } -void GenericCAO::setAttachment(int parent_id, const std::string &bone, +void GenericCAO::setAttachment(object_t parent_id, const std::string &bone, v3f position, v3f rotation, bool force_visible) { - int old_parent = m_attachment_parent_id; + const auto old_parent = m_attachment_parent_id; m_attachment_parent_id = parent_id; m_attachment_bone = bone; m_attachment_position = position; @@ -509,7 +509,7 @@ void GenericCAO::setAttachment(int parent_id, const std::string &bone, } } -void GenericCAO::getAttachment(int *parent_id, std::string *bone, v3f *position, +void GenericCAO::getAttachment(object_t *parent_id, std::string *bone, v3f *position, v3f *rotation, bool *force_visible) const { *parent_id = m_attachment_parent_id; @@ -523,29 +523,21 @@ void GenericCAO::clearChildAttachments() { // Cannot use for-loop here: setAttachment() modifies 'm_attachment_child_ids'! while (!m_attachment_child_ids.empty()) { - int child_id = *m_attachment_child_ids.begin(); + const auto child_id = *m_attachment_child_ids.begin(); - if (ClientActiveObject *child = m_env->getActiveObject(child_id)) - child->setAttachment(0, "", v3f(), v3f(), false); - - removeAttachmentChild(child_id); + if (auto *child = m_env->getActiveObject(child_id)) + child->clearParentAttachment(); + else + removeAttachmentChild(child_id); } } -void GenericCAO::clearParentAttachment() -{ - if (m_attachment_parent_id) - setAttachment(0, "", m_attachment_position, m_attachment_rotation, false); - else - setAttachment(0, "", v3f(), v3f(), false); -} - -void GenericCAO::addAttachmentChild(int child_id) +void GenericCAO::addAttachmentChild(object_t child_id) { m_attachment_child_ids.insert(child_id); } -void GenericCAO::removeAttachmentChild(int child_id) +void GenericCAO::removeAttachmentChild(object_t child_id) { m_attachment_child_ids.erase(child_id); } diff --git a/src/client/content_cao.h b/src/client/content_cao.h index 7fdcb73da..3fdf01bc7 100644 --- a/src/client/content_cao.h +++ b/src/client/content_cao.h @@ -106,8 +106,8 @@ private: // stores position and rotation for each bone name BoneOverrideMap m_bone_override; - int m_attachment_parent_id = 0; - std::unordered_set m_attachment_child_ids; + object_t m_attachment_parent_id = 0; + std::unordered_set m_attachment_child_ids; std::string m_attachment_bone = ""; v3f m_attachment_position; v3f m_attachment_rotation; @@ -226,16 +226,15 @@ public: } void setChildrenVisible(bool toset); - void setAttachment(int parent_id, const std::string &bone, v3f position, + void setAttachment(object_t parent_id, const std::string &bone, v3f position, v3f rotation, bool force_visible) override; - void getAttachment(int *parent_id, std::string *bone, v3f *position, + void getAttachment(object_t *parent_id, std::string *bone, v3f *position, v3f *rotation, bool *force_visible) const override; void clearChildAttachments() override; - void clearParentAttachment() override; - void addAttachmentChild(int child_id) override; - void removeAttachmentChild(int child_id) override; + void addAttachmentChild(object_t child_id) override; + void removeAttachmentChild(object_t child_id) override; ClientActiveObject *getParent() const override; - const std::unordered_set &getAttachmentChildIds() const override + const std::unordered_set &getAttachmentChildIds() const override { return m_attachment_child_ids; } void updateAttachments() override; diff --git a/src/script/lua_api/l_object.cpp b/src/script/lua_api/l_object.cpp index 6baf146e5..f3aa816a5 100644 --- a/src/script/lua_api/l_object.cpp +++ b/src/script/lua_api/l_object.cpp @@ -37,11 +37,12 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "server/serverinventorymgr.h" #include "server/unit_sao.h" +using object_t = ServerActiveObject::object_t; + /* ObjectRef */ - ServerActiveObject* ObjectRef::getobject(ObjectRef *ref) { ServerActiveObject *sao = ref->m_object; @@ -99,9 +100,6 @@ int ObjectRef::l_remove(lua_State *L) if (sao->getType() == ACTIVEOBJECT_TYPE_PLAYER) return 0; - sao->clearChildAttachments(); - sao->clearParentAttachment(); - verbosestream << "ObjectRef::l_remove(): id=" << sao->getId() << std::endl; sao->markForRemoval(); return 0; @@ -724,25 +722,17 @@ int ObjectRef::l_set_attach(lua_State *L) if (sao == parent) throw LuaError("ObjectRef::set_attach: attaching object to itself is not allowed."); - int parent_id; std::string bone; v3f position; v3f rotation; bool force_visible; - sao->getAttachment(&parent_id, &bone, &position, &rotation, &force_visible); - if (parent_id) { - ServerActiveObject *old_parent = env->getActiveObject(parent_id); - old_parent->removeAttachmentChild(sao->getId()); - } - bone = readParam(L, 3, ""); position = readParam(L, 4, v3f(0, 0, 0)); rotation = readParam(L, 5, v3f(0, 0, 0)); force_visible = readParam(L, 6, false); sao->setAttachment(parent->getId(), bone, position, rotation, force_visible); - parent->addAttachmentChild(sao->getId()); return 0; } @@ -755,7 +745,7 @@ int ObjectRef::l_get_attach(lua_State *L) if (sao == nullptr) return 0; - int parent_id; + object_t parent_id; std::string bone; v3f position; v3f rotation; @@ -783,11 +773,11 @@ int ObjectRef::l_get_children(lua_State *L) if (sao == nullptr) return 0; - const std::unordered_set child_ids = sao->getAttachmentChildIds(); + const auto &child_ids = sao->getAttachmentChildIds(); int i = 0; lua_createtable(L, child_ids.size(), 0); - for (const int id : child_ids) { + for (const object_t id : child_ids) { ServerActiveObject *child = env->getActiveObject(id); getScriptApiBase(L)->objectrefGetOrCreate(L, child); lua_rawseti(L, -2, ++i); diff --git a/src/server.cpp b/src/server.cpp index 6e42b22a0..86966d0ab 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2926,9 +2926,6 @@ void Server::DeleteClient(session_t peer_id, ClientDeletionReason reason) PlayerSAO *playersao = player->getPlayerSAO(); assert(playersao); - playersao->clearChildAttachments(); - playersao->clearParentAttachment(); - // inform connected clients const std::string &player_name = player->getName(); NetworkPacket notice(TOCLIENT_UPDATE_PLAYER_LIST, 0, PEER_ID_INEXISTENT); diff --git a/src/server/clientiface.cpp b/src/server/clientiface.cpp index 451e74407..e5c07b3d8 100644 --- a/src/server/clientiface.cpp +++ b/src/server/clientiface.cpp @@ -85,23 +85,13 @@ void RemoteClient::ResendBlockIfOnWire(v3s16 p) } } -LuaEntitySAO *getAttachedObject(PlayerSAO *sao, ServerEnvironment *env) +static LuaEntitySAO *getAttachedObject(PlayerSAO *sao, ServerEnvironment *env) { - if (!sao->isAttached()) - return nullptr; + ServerActiveObject *ao = sao; + while (ao->getParent()) + ao = ao->getParent(); - int id; - std::string bone; - v3f dummy; - bool force_visible; - sao->getAttachment(&id, &bone, &dummy, &dummy, &force_visible); - ServerActiveObject *ao = env->getActiveObject(id); - while (id && ao) { - ao->getAttachment(&id, &bone, &dummy, &dummy, &force_visible); - if (id) - ao = env->getActiveObject(id); - } - return dynamic_cast(ao); + return ao == sao ? nullptr : dynamic_cast(ao); } void RemoteClient::GetNextBlocks ( diff --git a/src/server/luaentity_sao.cpp b/src/server/luaentity_sao.cpp index 020f13fa5..e932f418b 100644 --- a/src/server/luaentity_sao.cpp +++ b/src/server/luaentity_sao.cpp @@ -147,7 +147,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended) } // If attached, check that our parent is still there. If it isn't, detach. - if (m_attachment_parent_id && !isAttached()) { + if (m_attachment_parent_id && !getParent()) { // This is handled when objects are removed from the map warningstream << "LuaEntitySAO::step() " << m_init_name << " at " << m_last_sent_position << ", id=" << m_id << " is attached to nonexistent parent. This is a bug." << std::endl; @@ -415,8 +415,6 @@ void LuaEntitySAO::setHP(s32 hp, const PlayerHPChangeReason &reason) sendPunchCommand(); if (m_hp == 0 && !isGone()) { - clearParentAttachment(); - clearChildAttachments(); if (m_registered) { ServerActiveObject *killer = nullptr; if (reason.type == PlayerHPChangeReason::PLAYER_PUNCH) diff --git a/src/server/luaentity_sao.h b/src/server/luaentity_sao.h index 2080df9c3..6bc7a8409 100644 --- a/src/server/luaentity_sao.h +++ b/src/server/luaentity_sao.h @@ -81,8 +81,14 @@ public: protected: void dispatchScriptDeactivate(bool removal); - virtual void onMarkedForDeactivation() { dispatchScriptDeactivate(false); } - virtual void onMarkedForRemoval() { dispatchScriptDeactivate(true); } + virtual void onMarkedForDeactivation() { + UnitSAO::onMarkedForDeactivation(); + dispatchScriptDeactivate(false); + } + virtual void onMarkedForRemoval() { + UnitSAO::onMarkedForRemoval(); + dispatchScriptDeactivate(true); + } private: std::string getPropertyPacket(); diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp index b2e2351c9..4abb1f920 100644 --- a/src/server/player_sao.cpp +++ b/src/server/player_sao.cpp @@ -233,13 +233,12 @@ void PlayerSAO::step(float dtime, bool send_recommended) } // If attached, check that our parent is still there. If it isn't, detach. - if (m_attachment_parent_id && !isAttached()) { + if (m_attachment_parent_id && !getParent()) { // This is handled when objects are removed from the map warningstream << "PlayerSAO::step() id=" << m_id << " is attached to nonexistent parent. This is a bug." << std::endl; clearParentAttachment(); - setBasePosition(m_last_good_position); - m_env->getGameDef()->SendMovePlayer(this); + setPos(m_last_good_position); } //dstream<<"PlayerSAO::step: dtime: "< &getAttachmentChildIds() const - { static std::unordered_set rv; return rv; } + virtual const std::unordered_set &getAttachmentChildIds() const + { static std::unordered_set rv; return rv; } virtual ServerActiveObject *getParent() const { return nullptr; } virtual ObjectProperties *accessObjectProperties() { return NULL; } @@ -240,8 +240,8 @@ protected: virtual void onMarkedForDeactivation() {} virtual void onMarkedForRemoval() {} - virtual void onAttach(int parent_id) {} - virtual void onDetach(int parent_id) {} + virtual void onAttach(object_t parent_id) {} + virtual void onDetach(object_t parent_id) {} ServerEnvironment *m_env; v3f m_base_position; diff --git a/src/server/unit_sao.cpp b/src/server/unit_sao.cpp index 70594589b..d764b5d16 100644 --- a/src/server/unit_sao.cpp +++ b/src/server/unit_sao.cpp @@ -130,50 +130,92 @@ void UnitSAO::sendOutdatedData() } } -void UnitSAO::setAttachment(int parent_id, const std::string &bone, v3f position, +void UnitSAO::setAttachment(const object_t new_parent, 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; + const auto call_count = ++m_attachment_call_counter; + + const auto check_nesting = [&] (const char *descr) -> bool { + // The counter is never decremented, so if it differs that means + // a nested call to setAttachment() has happened. + if (m_attachment_call_counter == call_count) + return false; + verbosestream << "UnitSAO::setAttachment() id=" << m_id << + " nested call detected (" << descr << ")." << std::endl; + return true; + }; + + // Do checks to avoid circular references + { + auto *obj = new_parent ? m_env->getActiveObject(new_parent) : nullptr; + if (obj == this) { + assert(false); + return; + } + bool problem = false; + if (obj) { + // The chain of wanted parent must not refer or contain "this" + for (obj = obj->getParent(); obj; obj = obj->getParent()) { + if (obj == this) { + problem = true; + break; + } } } + if (problem) { + warningstream << "Mod bug: Attempted to attach object " << m_id << " to parent " + << new_parent << " 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 - // lagging, plus we can't read and attach to skeletal bones. If we just attach on - // the client, the server still sees the child at its original location. This - // breaks some things so we also give the server the most accurate representation - // even if players only see the client changes. + // Detach first + // Note: make sure to apply data changes before running callbacks. + const auto old_parent = m_attachment_parent_id; + m_attachment_parent_id = 0; + m_attachment_sent = false; - int old_parent = m_attachment_parent_id; - m_attachment_parent_id = parent_id; + if (old_parent && old_parent != new_parent) { + auto *parent = m_env->getActiveObject(old_parent); + if (parent) { + onDetach(parent); + } else { + warningstream << "UnitSAO::setAttachment() id=" << m_id << + " is attached to nonexistent parent. This is a bug." << std::endl; + // we can pretend it never happened + } + } - // The detach callbacks might call to setAttachment() again. - // Ensure the attachment params are applied after this callback is run. - if (parent_id != old_parent) - onDetach(old_parent); + if (check_nesting("onDetach")) { + // Don't touch anything after the other call has completed. + return; + } - m_attachment_parent_id = parent_id; + if (isGone()) + return; + + // Now attach to new parent + m_attachment_parent_id = new_parent; m_attachment_bone = bone; m_attachment_position = position; m_attachment_rotation = rotation; m_force_visible = force_visible; - m_attachment_sent = false; - if (parent_id != old_parent) - onAttach(parent_id); + if (new_parent && old_parent != new_parent) { + auto *parent = m_env->getActiveObject(new_parent); + if (parent) { + onAttach(parent); + } else { + warningstream << "UnitSAO::setAttachment() id=" << m_id << + " tried to attach to nonexistent parent. This is a bug." << std::endl; + m_attachment_parent_id = 0; // detach + } + } + + check_nesting("onAttach"); } -void UnitSAO::getAttachment(int *parent_id, std::string *bone, v3f *position, +void UnitSAO::getAttachment(object_t *parent_id, std::string *bone, v3f *position, v3f *rotation, bool *force_visible) const { *parent_id = m_attachment_parent_id; @@ -183,79 +225,70 @@ void UnitSAO::getAttachment(int *parent_id, std::string *bone, v3f *position, *force_visible = m_force_visible; } +void UnitSAO::clearAnyAttachments() +{ + // This is called before this SAO is marked for removal/deletion and unlinks + // any parent or child relationships. + // This is done at this point and not in ~UnitSAO() so attachments to + // "phantom objects" don't stay around while we're waiting to be actually deleted. + // (which can take several server steps) + clearParentAttachment(); + clearChildAttachments(); +} + void UnitSAO::clearChildAttachments() { // Cannot use for-loop here: setAttachment() modifies 'm_attachment_child_ids'! while (!m_attachment_child_ids.empty()) { - int child_id = *m_attachment_child_ids.begin(); + const auto child_id = *m_attachment_child_ids.begin(); - // Child can be NULL if it was deleted earlier - if (ServerActiveObject *child = m_env->getActiveObject(child_id)) - child->setAttachment(0, "", v3f(0, 0, 0), v3f(0, 0, 0), false); - - removeAttachmentChild(child_id); + if (auto *child = m_env->getActiveObject(child_id)) { + child->clearParentAttachment(); + } else { + // should not happen but we need to handle it to prevent an infinite loop + removeAttachmentChild(child_id); + } } } -void UnitSAO::clearParentAttachment() -{ - ServerActiveObject *parent = nullptr; - if (m_attachment_parent_id) { - parent = m_env->getActiveObject(m_attachment_parent_id); - setAttachment(0, "", m_attachment_position, m_attachment_rotation, false); - } else { - setAttachment(0, "", v3f(0, 0, 0), v3f(0, 0, 0), false); - } - // Do it - if (parent) - parent->removeAttachmentChild(m_id); -} - -void UnitSAO::addAttachmentChild(int child_id) +void UnitSAO::addAttachmentChild(object_t child_id) { m_attachment_child_ids.insert(child_id); } -void UnitSAO::removeAttachmentChild(int child_id) +void UnitSAO::removeAttachmentChild(object_t child_id) { m_attachment_child_ids.erase(child_id); } -const std::unordered_set &UnitSAO::getAttachmentChildIds() const +void UnitSAO::onAttach(ServerActiveObject *parent) { - return m_attachment_child_ids; -} + assert(parent); -void UnitSAO::onAttach(int parent_id) -{ - if (!parent_id) - return; + parent->addAttachmentChild(m_id); - ServerActiveObject *parent = m_env->getActiveObject(parent_id); - - if (!parent || parent->isGone()) - return; // Do not try to notify soon gone parent - - if (parent->getType() == ACTIVEOBJECT_TYPE_LUAENTITY) { - // Call parent's on_attach field - m_env->getScriptIface()->luaentity_on_attach_child(parent_id, this); + // Do not try to notify soon gone parent + if (!parent->isGone()) { + if (parent->getType() == ACTIVEOBJECT_TYPE_LUAENTITY) + m_env->getScriptIface()->luaentity_on_attach_child(parent->getId(), this); } } -void UnitSAO::onDetach(int parent_id) +void UnitSAO::onDetach(ServerActiveObject *parent) { - if (!parent_id) - return; + assert(parent); + + parent->removeAttachmentChild(m_id); - ServerActiveObject *parent = m_env->getActiveObject(parent_id); if (getType() == ACTIVEOBJECT_TYPE_LUAENTITY) m_env->getScriptIface()->luaentity_on_detach(m_id, parent); - if (!parent || parent->isGone()) - return; // Do not try to notify soon gone parent + // callback could affect the parent + if (parent->isGone()) + return; if (parent->getType() == ACTIVEOBJECT_TYPE_LUAENTITY) - m_env->getScriptIface()->luaentity_on_detach_child(parent_id, this); + m_env->getScriptIface()->luaentity_on_detach_child(parent->getId(), this); } ObjectProperties *UnitSAO::accessObjectProperties() diff --git a/src/server/unit_sao.h b/src/server/unit_sao.h index cbd845708..c7f8c4aec 100644 --- a/src/server/unit_sao.h +++ b/src/server/unit_sao.h @@ -77,16 +77,17 @@ public: // Attachments ServerActiveObject *getParent() const; - inline bool isAttached() const { return getParent(); } - void setAttachment(int parent_id, const std::string &bone, v3f position, + inline bool isAttached() const { return m_attachment_parent_id != 0; } + void setAttachment(object_t parent_id, const std::string &bone, v3f position, v3f rotation, bool force_visible); - void getAttachment(int *parent_id, std::string *bone, v3f *position, + void getAttachment(object_t *parent_id, std::string *bone, v3f *position, v3f *rotation, bool *force_visible) const; - void clearChildAttachments(); - void clearParentAttachment(); - void addAttachmentChild(int child_id); - void removeAttachmentChild(int child_id); - const std::unordered_set &getAttachmentChildIds() const; + void clearChildAttachments() override; + void addAttachmentChild(object_t child_id) override; + void removeAttachmentChild(object_t child_id) override; + const std::unordered_set &getAttachmentChildIds() const { + return m_attachment_child_ids; + } // Object properties ObjectProperties *accessObjectProperties(); @@ -121,14 +122,28 @@ protected: // Stores position and rotation for each bone name std::unordered_map m_bone_override; - int m_attachment_parent_id = 0; + object_t m_attachment_parent_id = 0; + + void clearAnyAttachments(); + virtual void onMarkedForDeactivation() { + ServerActiveObject::onMarkedForDeactivation(); + clearAnyAttachments(); + } + virtual void onMarkedForRemoval() { + ServerActiveObject::onMarkedForRemoval(); + clearAnyAttachments(); + } private: - void onAttach(int parent_id); - void onDetach(int parent_id); + void onAttach(ServerActiveObject *parent); + void onDetach(ServerActiveObject *parent); std::string generatePunchCommand(u16 result_hp) const; + // Used to detect nested calls to setAttachments(), which can happen due to + // Lua callbacks + u8 m_attachment_call_counter = 0; + // Armor groups bool m_armor_groups_sent = false; @@ -144,7 +159,7 @@ private: bool m_bone_override_sent = false; // Attachments - std::unordered_set m_attachment_child_ids; + std::unordered_set m_attachment_child_ids; std::string m_attachment_bone = ""; v3f m_attachment_position; v3f m_attachment_rotation; diff --git a/src/util/numeric.h b/src/util/numeric.h index c31dcb9fb..73b4fb5cd 100644 --- a/src/util/numeric.h +++ b/src/util/numeric.h @@ -439,6 +439,17 @@ inline u32 npot2(u32 orig) { return orig + 1; } +// Distance between two values in a wrapped (circular) system +template +inline unsigned wrappedDifference(T a, T b, const T maximum) +{ + if (a > b) + std::swap(a, b); + // now b >= a + unsigned s = b - a, l = static_cast(maximum - b) + a + 1; + return std::min(s, l); +} + // Gradual steps towards the target value in a wrapped (circular) system // using the shorter of both ways template