Move shared parameters sending to UnitSAO (#9968)

Better header sorting by topic
Make UnitSAO-specific parameters private
Skip redundant recursive entity sending code (since ~5.2.0)

diff --git a/src/client/content_cao.cpp b/src/client/content_cao.cpp
index d0682e51eab82f9743106a01ccb41ff2061ae1b8..702d089afe582a824929d9269c64c849a7dfb0e6 100644 (file)
--- a/src/client/content_cao.cpp
+++ b/src/client/content_cao.cpp
@@ -475,6 +475,7 @@ void GenericCAO::setAttachment(int parent_id, const std::string &bone, v3f posit
                        parent->addAttachmentChild(m_id);
        }
 
+
        updateAttachments();
 }
 

diff --git a/src/server/luaentity_sao.cpp b/src/server/luaentity_sao.cpp
index 8174da2650d1b1bac02f4bfff5e1ab367565daf3..d504c42caab762187cbe064f0c5a9b7be8b7f6b9 100644 (file)
--- a/src/server/luaentity_sao.cpp
+++ b/src/server/luaentity_sao.cpp
@@ -123,12 +123,11 @@ 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())
-       {
-               m_attachment_parent_id = 0;
-               m_attachment_bone = "";
-               m_attachment_position = v3f(0,0,0);
-               m_attachment_rotation = v3f(0,0,0);
+       if (m_attachment_parent_id && !isAttached()) {
+               // This is handled when objects are removed from the map
+               warningstream << "LuaEntitySAO::step() id=" << m_id <<
+                       " is attached to nonexistent parent. This is a bug." << std::endl;
+               clearParentAttachment();
                sendPosition(false, true);
        }
 
@@ -217,43 +216,7 @@ void LuaEntitySAO::step(float dtime, bool send_recommended)
                }
        }
 
-       if (!m_armor_groups_sent) {
-               m_armor_groups_sent = true;
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, generateUpdateArmorGroupsCommand());
-       }
-
-       if (!m_animation_sent) {
-               m_animation_sent = true;
-               std::string str = generateUpdateAnimationCommand();
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, str);
-       }
-
-       if (!m_animation_speed_sent) {
-               m_animation_speed_sent = true;
-               std::string str = generateUpdateAnimationSpeedCommand();
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, str);
-       }
-
-       if (!m_bone_position_sent) {
-               m_bone_position_sent = true;
-               for (std::unordered_map<std::string, core::vector2d<v3f>>::const_iterator
-                               ii = m_bone_position.begin(); ii != m_bone_position.end(); ++ii){
-                       std::string str = generateUpdateBonePositionCommand((*ii).first,
-                                       (*ii).second.X, (*ii).second.Y);
-                       // create message and add to list
-                       m_messages_out.emplace(getId(), true, str);
-               }
-       }
-
-       if (!m_attachment_sent) {
-               m_attachment_sent = true;
-               std::string str = generateUpdateAttachmentCommand();
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, str);
-       }
+       sendOutdatedData();
 }
 
 std::string LuaEntitySAO::getClientInitializationData(u16 protocol_version)
@@ -273,20 +236,19 @@ std::string LuaEntitySAO::getClientInitializationData(u16 protocol_version)
        msg_os << serializeLongString(getPropertyPacket()); // message 1
        msg_os << serializeLongString(generateUpdateArmorGroupsCommand()); // 2
        msg_os << serializeLongString(generateUpdateAnimationCommand()); // 3
-       for (std::unordered_map<std::string, core::vector2d<v3f>>::const_iterator
-                       ii = m_bone_position.begin(); ii != m_bone_position.end(); ++ii) {
-               msg_os << serializeLongString(generateUpdateBonePositionCommand((*ii).first,
-                               (*ii).second.X, (*ii).second.Y)); // m_bone_position.size
+       for (const auto &bone_pos : m_bone_position) {
+               msg_os << serializeLongString(generateUpdateBonePositionCommand(
+                       bone_pos.first, bone_pos.second.X, bone_pos.second.Y)); // m_bone_position.size
        }
        msg_os << serializeLongString(generateUpdateAttachmentCommand()); // 4
+
        int message_count = 4 + m_bone_position.size();
-       for (std::unordered_set<int>::const_iterator ii = m_attachment_child_ids.begin();
-                       (ii != m_attachment_child_ids.end()); ++ii) {
-               if (ServerActiveObject *obj = m_env->getActiveObject(*ii)) {
+
+       for (const auto &id : getAttachmentChildIds()) {
+               if (ServerActiveObject *obj = m_env->getActiveObject(id)) {
                        message_count++;
-                       // TODO after a protocol bump: only send the object initialization data
-                       // to older clients (superfluous since this message exists)
-                       msg_os << serializeLongString(obj->generateUpdateInfantCommand(*ii, protocol_version));
+                       msg_os << serializeLongString(obj->generateUpdateInfantCommand(
+                               id, protocol_version));
                }
        }
 
@@ -294,7 +256,8 @@ std::string LuaEntitySAO::getClientInitializationData(u16 protocol_version)
        message_count++;
 
        writeU8(os, message_count);
-       os.write(msg_os.str().c_str(), msg_os.str().size());
+       std::string serialized = msg_os.str();
+       os.write(serialized.c_str(), serialized.size());
 
        // return result
        return os.str();

diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp
index 9ea0743f764604616b568884674161fc84ed085b..67efed21045ece45aa35f4ad433be7ccf66d673c 100644 (file)
--- a/src/server/player_sao.cpp
+++ b/src/server/player_sao.cpp
@@ -120,24 +120,26 @@ std::string PlayerSAO::getClientInitializationData(u16 protocol_version)
        msg_os << serializeLongString(getPropertyPacket()); // message 1
        msg_os << serializeLongString(generateUpdateArmorGroupsCommand()); // 2
        msg_os << serializeLongString(generateUpdateAnimationCommand()); // 3
-       for (std::unordered_map<std::string, core::vector2d<v3f>>::const_iterator
-                       ii = m_bone_position.begin(); ii != m_bone_position.end(); ++ii) {
-               msg_os << serializeLongString(generateUpdateBonePositionCommand((*ii).first,
-                       (*ii).second.X, (*ii).second.Y)); // m_bone_position.size
+       for (const auto &bone_pos : m_bone_position) {
+               msg_os << serializeLongString(generateUpdateBonePositionCommand(
+                       bone_pos.first, bone_pos.second.X, bone_pos.second.Y)); // m_bone_position.size
        }
        msg_os << serializeLongString(generateUpdateAttachmentCommand()); // 4
        msg_os << serializeLongString(generateUpdatePhysicsOverrideCommand()); // 5
+
        int message_count = 5 + m_bone_position.size();
-       for (std::unordered_set<int>::const_iterator ii = m_attachment_child_ids.begin();
-                       ii != m_attachment_child_ids.end(); ++ii) {
-               if (ServerActiveObject *obj = m_env->getActiveObject(*ii)) {
+
+       for (const auto &id : getAttachmentChildIds()) {
+               if (ServerActiveObject *obj = m_env->getActiveObject(id)) {
                        message_count++;
-                       msg_os << serializeLongString(obj->generateUpdateInfantCommand(*ii, protocol_version));
+                       msg_os << serializeLongString(obj->generateUpdateInfantCommand(
+                               id, protocol_version));
                }
        }
 
        writeU8(os, message_count);
-       os.write(msg_os.str().c_str(), msg_os.str().size());
+       std::string serialized = msg_os.str();
+       os.write(serialized.c_str(), serialized.size());
 
        // return result
        return os.str();
@@ -227,10 +229,10 @@ 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()) {
-               m_attachment_parent_id = 0;
-               m_attachment_bone = "";
-               m_attachment_position = v3f(0.0f, 0.0f, 0.0f);
-               m_attachment_rotation = v3f(0.0f, 0.0f, 0.0f);
+               // 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(m_peer_id);
        }
@@ -290,40 +292,13 @@ void PlayerSAO::step(float dtime, bool send_recommended)
                m_messages_out.emplace(getId(), false, str);
        }
 
-       if (!m_armor_groups_sent) {
-               m_armor_groups_sent = true;
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, generateUpdateArmorGroupsCommand());
-       }
-
        if (!m_physics_override_sent) {
                m_physics_override_sent = true;
                // create message and add to list
                m_messages_out.emplace(getId(), true, generateUpdatePhysicsOverrideCommand());
        }
 
-       if (!m_animation_sent) {
-               m_animation_sent = true;
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, generateUpdateAnimationCommand());
-       }
-
-       if (!m_bone_position_sent) {
-               m_bone_position_sent = true;
-               for (std::unordered_map<std::string, core::vector2d<v3f>>::const_iterator
-                               ii = m_bone_position.begin(); ii != m_bone_position.end(); ++ii) {
-                       std::string str = generateUpdateBonePositionCommand((*ii).first,
-                                       (*ii).second.X, (*ii).second.Y);
-                       // create message and add to list
-                       m_messages_out.emplace(getId(), true, str);
-               }
-       }
-
-       if (!m_attachment_sent) {
-               m_attachment_sent = true;
-               // create message and add to list
-               m_messages_out.emplace(getId(), true, generateUpdateAttachmentCommand());
-       }
+       sendOutdatedData();
 }
 
 std::string PlayerSAO::generateUpdatePhysicsOverrideCommand() const

diff --git a/src/server/serveractiveobject.cpp b/src/server/serveractiveobject.cpp
index dbf25e3bc5895a35d7a63d46b52fb0441f0a807e..3341dc008ad259b8865812c5966883a3f80a5fc8 100644 (file)
--- a/src/server/serveractiveobject.cpp
+++ b/src/server/serveractiveobject.cpp
@@ -57,7 +57,12 @@ std::string ServerActiveObject::generateUpdateInfantCommand(u16 infant_id, u16 p
        // parameters
        writeU16(os, infant_id);
        writeU8(os, getSendType());
-       os << serializeLongString(getClientInitializationData(protocol_version));
+       if (protocol_version < 38) {
+               // Clients since 4aa9a66 so no longer need this data
+               // Version 38 is the first bump after that commit.
+               // See also: ClientEnvironment::addActiveObject
+               os << serializeLongString(getClientInitializationData(protocol_version));
+       }
        return os.str();
 }
 

diff --git a/src/server/unit_sao.cpp b/src/server/unit_sao.cpp
index 74b0508b8c1a01003ff0b794ea70e54b7f96a0e4..ef0e87f2cec1ecd0e49534f18fe4f6214fcc5385 100644 (file)
--- a/src/server/unit_sao.cpp
+++ b/src/server/unit_sao.cpp
@@ -88,6 +88,39 @@ void UnitSAO::getBonePosition(const std::string &bone, v3f *position, v3f *rotat
        *rotation = m_bone_position[bone].Y;
 }
 
+// clang-format off
+void UnitSAO::sendOutdatedData()
+{
+       if (!m_armor_groups_sent) {
+               m_armor_groups_sent = true;
+               m_messages_out.emplace(getId(), true, generateUpdateArmorGroupsCommand());
+       }
+
+       if (!m_animation_sent) {
+               m_animation_sent = true;
+               m_animation_speed_sent = true;
+               m_messages_out.emplace(getId(), true, generateUpdateAnimationCommand());
+       } else if (!m_animation_speed_sent) {
+               // Animation speed is also sent when 'm_animation_sent == false'
+               m_animation_speed_sent = true;
+               m_messages_out.emplace(getId(), true, generateUpdateAnimationSpeedCommand());
+       }
+
+       if (!m_bone_position_sent) {
+               m_bone_position_sent = true;
+               for (const auto &bone_pos : m_bone_position) {
+                       m_messages_out.emplace(getId(), true, generateUpdateBonePositionCommand(
+                               bone_pos.first, bone_pos.second.X, bone_pos.second.Y));
+               }
+       }
+
+       if (!m_attachment_sent) {
+               m_attachment_sent = true;
+               m_messages_out.emplace(getId(), true, generateUpdateAttachmentCommand());
+       }
+}
+// clang-format on
+
 void UnitSAO::setAttachment(
                int parent_id, const std::string &bone, v3f position, v3f rotation)
 {

diff --git a/src/server/unit_sao.h b/src/server/unit_sao.h
index c73115855c353abfbf3f3ce1111d445b14995e86..3cb7f0ad5349ff570894178aec0d7cf32e0c5cc5 100644 (file)
--- a/src/server/unit_sao.h
+++ b/src/server/unit_sao.h
@@ -29,6 +29,11 @@ public:
        UnitSAO(ServerEnvironment *env, v3f pos);
        virtual ~UnitSAO() = default;
 
+       u16 getHP() const { return m_hp; }
+       // Use a function, if isDead can be defined by other conditions
+       bool isDead() const { return m_hp == 0; }
+
+       // Rotation
        void setRotation(v3f rotation) { m_rotation = rotation; }
        const v3f &getRotation() const { return m_rotation; }
        v3f getRadRotation() { return m_rotation * core::DEGTORAD; }
@@ -36,26 +41,28 @@ public:
        // Deprecated
        f32 getRadYawDep() const { return (m_rotation.Y + 90.) * core::DEGTORAD; }
 
-       u16 getHP() const { return m_hp; }
-       // Use a function, if isDead can be defined by other conditions
-       bool isDead() const { return m_hp == 0; }
-
-       inline bool isAttached() const { return getParent(); }
-
+       // Armor groups
        inline bool isImmortal() const
        {
                return itemgroup_get(getArmorGroups(), "immortal");
        }
-
        void setArmorGroups(const ItemGroupList &armor_groups);
        const ItemGroupList &getArmorGroups() const;
+
+       // Animation
        void setAnimation(v2f frame_range, float frame_speed, float frame_blend,
                        bool frame_loop);
        void getAnimation(v2f *frame_range, float *frame_speed, float *frame_blend,
                        bool *frame_loop);
        void setAnimationSpeed(float frame_speed);
+
+       // Bone position
        void setBonePosition(const std::string &bone, v3f position, v3f rotation);
        void getBonePosition(const std::string &bone, v3f *position, v3f *rotation);
+
+       // Attachments
+       ServerActiveObject *getParent() const;
+       inline bool isAttached() const { return getParent(); }
        void setAttachment(int parent_id, const std::string &bone, v3f position,
                        v3f rotation);
        void getAttachment(int *parent_id, std::string *bone, v3f *position,
@@ -65,10 +72,13 @@ public:
        void addAttachmentChild(int child_id);
        void removeAttachmentChild(int child_id);
        const std::unordered_set<int> &getAttachmentChildIds() const;
-       ServerActiveObject *getParent() const;
+
+       // Object properties
        ObjectProperties *accessObjectProperties();
        void notifyObjectPropertiesModified();
+       void sendOutdatedData();
 
+       // Update packets
        std::string generateUpdateAttachmentCommand() const;
        std::string generateUpdateAnimationSpeedCommand() const;
        std::string generateUpdateAnimationCommand() const;
@@ -77,21 +87,36 @@ public:
                        const v3f &velocity, const v3f &acceleration, const v3f &rotation,
                        bool do_interpolate, bool is_movement_end, f32 update_interval);
        std::string generateSetPropertiesCommand(const ObjectProperties &prop) const;
-       void sendPunchCommand();
        static std::string generateUpdateBonePositionCommand(const std::string &bone,
                        const v3f &position, const v3f &rotation);
+       void sendPunchCommand();
 
 protected:
        u16 m_hp = 1;
 
        v3f m_rotation;
 
+       ItemGroupList m_armor_groups;
+
+       // Object properties
        bool m_properties_sent = true;
        ObjectProperties m_prop;
 
-       ItemGroupList m_armor_groups;
+       // Stores position and rotation for each bone name
+       std::unordered_map<std::string, core::vector2d<v3f>> m_bone_position;
+
+       int m_attachment_parent_id = 0;
+
+private:
+       void onAttach(int parent_id);
+       void onDetach(int parent_id);
+
+       std::string generatePunchCommand(u16 result_hp) const;
+
+       // Armor groups
        bool m_armor_groups_sent = false;
 
+       // Animation
        v2f m_animation_range;
        float m_animation_speed = 0.0f;
        float m_animation_blend = 0.0f;
@@ -99,20 +124,13 @@ protected:
        bool m_animation_sent = false;
        bool m_animation_speed_sent = false;
 
-       // Stores position and rotation for each bone name
-       std::unordered_map<std::string, core::vector2d<v3f>> m_bone_position;
+       // Bone positions
        bool m_bone_position_sent = false;
 
-       int m_attachment_parent_id = 0;
+       // Attachments
        std::unordered_set<int> m_attachment_child_ids;
        std::string m_attachment_bone = "";
        v3f m_attachment_position;
        v3f m_attachment_rotation;
        bool m_attachment_sent = false;
-
-private:
-       void onAttach(int parent_id);
-       void onDetach(int parent_id);
-
-       std::string generatePunchCommand(u16 result_hp) const;
 };