Attachments: Fix attachments to temporary removed objects (#8989)

Does not clear the parent's attachment information when the child is deleted locally.
Either it was removed permanently, or just temporary - we don't know, but it's up to the server to send a *detach from child" packet for the parent.

diff --git a/src/client/clientenvironment.cpp b/src/client/clientenvironment.cpp
index e1b20ec8498397969b3837a462d210bb63ef4f72..5eb0333029fd11743d150a8c6997a22b5d481324 100644 (file)
--- a/src/client/clientenvironment.cpp
+++ b/src/client/clientenvironment.cpp
@@ -402,6 +402,23 @@ void ClientEnvironment::addActiveObject(u16 id, u8 type,
        }
 }
 
+
+void ClientEnvironment::removeActiveObject(u16 id)
+{
+       // Get current attachment childs to detach them visually
+       std::unordered_set<int> attachment_childs;
+       if (auto *obj = getActiveObject(id))
+               attachment_childs = obj->getAttachmentChildIds();
+
+       m_ao_manager.removeObject(id);
+
+       // Perform a proper detach in Irrlicht
+       for (auto c_id : attachment_childs) {
+               if (ClientActiveObject *child = getActiveObject(c_id))
+                       child->updateAttachments();
+       }
+}
+
 void ClientEnvironment::processActiveObjectMessage(u16 id, const std::string &data)
 {
        ClientActiveObject *obj = getActiveObject(id);

diff --git a/src/client/clientenvironment.h b/src/client/clientenvironment.h
index f182b5951864aae35c789db5a4fbe1bc4bef673a..864496a415bf58546717a59be640b0436a0866dc 100644 (file)
--- a/src/client/clientenvironment.h
+++ b/src/client/clientenvironment.h
@@ -104,10 +104,7 @@ public:
        u16 addActiveObject(ClientActiveObject *object);
 
        void addActiveObject(u16 id, u8 type, const std::string &init_data);
-       void removeActiveObject(u16 id)
-       {
-               m_ao_manager.removeObject(id);
-       }
+       void removeActiveObject(u16 id);
 
        void processActiveObjectMessage(u16 id, const std::string &data);
 

diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp
index d7ab8e945740fa7ea81726016c9c38896f1b98cb..71646840260885a79227f47566f1d5896da3e4b0 100644 (file)
--- a/src/client/content_cao.cpp
+++ b/src/client/content_cao.cpp
@@ -523,7 +523,9 @@ void GenericCAO::removeFromScene(bool permanent)
        // Should be true when removing the object permanently
        // and false when refreshing (eg: updating visuals)
        if (m_env && permanent) {
-               clearChildAttachments();
+               // The client does not know whether this object does re-appear to
+               // a later time, thus do not clear child attachments.
+
                clearParentAttachment();
        }
 
@@ -1330,10 +1332,17 @@ void GenericCAO::updateAttachments()
 
        m_attached_to_local = parent && parent->isLocalPlayer();
 
-       if (!parent && m_attachment_parent_id) {
-               //m_is_visible = false; maybe later. needs better handling
-               return;
-       }
+       /*
+       Following cases exist:
+               m_attachment_parent_id == 0 && !parent
+                       This object is not attached
+               m_attachment_parent_id != 0 && parent
+                       This object is attached
+               m_attachment_parent_id != 0 && !parent
+                       This object will be attached as soon the parent is known
+               m_attachment_parent_id == 0 && parent
+                       Impossible case
+       */
 
        if (!parent) { // Detach or don't attach
                if (m_matrixnode) {

diff --git a/src/client/content_cao.h b/src/client/content_cao.h
index 3a071101f5ff69a7c1e937c965c183b180353988..2c2d1107760d26449489303385cd73dd4a22d921 100644 (file)
--- a/src/client/content_cao.h
+++ b/src/client/content_cao.h
@@ -156,6 +156,11 @@ public:
 
        const v3f getPosition() const;
 
+       void setPosition(const v3f &pos)
+       {
+               pos_translator.val_current = pos;
+       }
+
        inline const v3f &getRotation() const { return m_rotation; }
 
        const bool isImmortal();

diff --git a/src/content_sao.cpp b/src/content_sao.cpp
index d6baa15803d1d845bf64fc6f4867460b5d9e4289..87e6466a9c91ee0f0c6671af2e014a8542137c25 100644 (file)
--- a/src/content_sao.cpp
+++ b/src/content_sao.cpp
@@ -1107,14 +1107,14 @@ void PlayerSAO::step(float dtime, bool send_recommended)
        if (!send_recommended)
                return;
 
-       // If the object is attached client-side, don't waste bandwidth sending its
-       // position or rotation to clients.
-       if (m_position_not_sent && !isAttached()) {
+       if (m_position_not_sent) {
                m_position_not_sent = false;
                float update_interval = m_env->getSendRecommendedInterval();
                v3f pos;
-               if (isAttached()) // Just in case we ever do send attachment position too
-                       pos = m_env->getActiveObject(m_attachment_parent_id)->getBasePosition();
+               // When attached, the position is only sent to clients where the
+               // parent isn't known
+               if (isAttached())
+                       pos = m_last_good_position;
                else
                        pos = m_base_position;
 

diff --git a/src/server.cpp b/src/server.cpp
index 0e677199aa1d517a31078ec1c7513b39cf33865b..4aa8375c80b9d35ea55edb72b2cbab5358e28c92 100644 (file)
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -694,19 +694,33 @@ void Server::AsyncRunStep(bool initial_step)
                // Route data to every client
                for (const auto &client_it : clients) {
                        RemoteClient *client = client_it.second;
+                       PlayerSAO *player = getPlayerSAO(client->peer_id);
                        std::string reliable_data;
                        std::string unreliable_data;
                        // Go through all objects in message buffer
                        for (const auto &buffered_message : buffered_messages) {
-                               // If object is not known by client, skip it
+                               // If object does not exist or is not known by client, skip it
                                u16 id = buffered_message.first;
-                               if (client->m_known_objects.find(id) == client->m_known_objects.end())
+                               ServerActiveObject *sao = m_env->getActiveObject(id);
+                               if (!sao || client->m_known_objects.find(id) == client->m_known_objects.end())
                                        continue;
 
                                // Get message list of object
                                std::vector<ActiveObjectMessage>* list = buffered_message.second;
                                // Go through every message
                                for (const ActiveObjectMessage &aom : *list) {
+                                       // Send position updates to players who do not see the attachment
+                                       if (aom.datastring[0] == GENERIC_CMD_UPDATE_POSITION) {
+                                               if (sao->getId() == player->getId())
+                                                       continue;
+
+                                               // Do not send position updates for attached players
+                                               // as long the parent is known to the client
+                                               ServerActiveObject *parent = sao->getParent();
+                                               if (parent && client->m_known_objects.find(parent->getId()) !=
+                                                               client->m_known_objects.end())
+                                                       continue;
+                                       }
                                        // Compose the full new data with header
                                        std::string new_data;
                                        // Add object id
@@ -1907,14 +1921,17 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa
        while (!added_objects.empty()) {
                // Get object
                u16 id = added_objects.front();
-               ServerActiveObject* obj = m_env->getActiveObject(id);
+               ServerActiveObject *obj = m_env->getActiveObject(id);
+               added_objects.pop();
+
+               if (!obj) {
+                       warningstream << FUNCTION_NAME << ": NULL object id="
+                               << (int)id << std::endl;
+                       continue;
+               }
 
                // Get object type
-               u8 type = ACTIVEOBJECT_TYPE_INVALID;
-               if (!obj)
-                       warningstream << FUNCTION_NAME << ": NULL object" << std::endl;
-               else
-                       type = obj->getSendType();
+               u8 type = obj->getSendType();
 
                // Add to data buffer for sending
                writeU16((u8*)buf, id);
@@ -1922,19 +1939,13 @@ void Server::SendActiveObjectRemoveAdd(RemoteClient *client, PlayerSAO *playersa
                writeU8((u8*)buf, type);
                data.append(buf, 1);
 
-               if (obj)
-                       data.append(serializeLongString(
-                               obj->getClientInitializationData(client->net_proto_version)));
-               else
-                       data.append(serializeLongString(""));
+               data.append(serializeLongString(
+                       obj->getClientInitializationData(client->net_proto_version)));
 
                // Add to known objects
                client->m_known_objects.insert(id);
 
-               if (obj)
-                       obj->m_known_by_count++;
-
-               added_objects.pop();
+               obj->m_known_by_count++;
        }
 
        NetworkPacket pkt(TOCLIENT_ACTIVE_OBJECT_REMOVE_ADD, data.size(), client->peer_id);