From 81c2370c8b1a66a279a5ff450c78caf5dfef77bf Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Wed, 2 Oct 2019 19:11:27 +0200 Subject: [PATCH] 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. --- src/client/clientenvironment.cpp | 17 ++++++++++++ src/client/clientenvironment.h | 5 +--- src/client/content_cao.cpp | 19 ++++++++++---- src/client/content_cao.h | 5 ++++ src/content_sao.cpp | 10 +++---- src/server.cpp | 45 ++++++++++++++++++++------------ 6 files changed, 70 insertions(+), 31 deletions(-) diff --git a/src/client/clientenvironment.cpp b/src/client/clientenvironment.cpp index e1b20ec84..5eb033302 100644 --- 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 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 f182b5951..864496a41 100644 --- 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 d7ab8e945..716468402 100644 --- 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 3a071101f..2c2d11077 100644 --- 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 d6baa1580..87e6466a9 100644 --- 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 0e677199a..4aa8375c8 100644 --- 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* 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); -- 2.25.1