[General] Rework PlayerSkill packets so they are of minimal size

Previously, whenever a single skill value changed for a player, that player then sent a PlayerSkill packet with all values for all 27 skills, plus the player's progress towards the next level and the bonuses to each attribute on the next level up as the result of sklll increases thus far.

This commit makes PlayerSkill contain only the values of specific skills, moves the player's progress towards the next level to PlayerLevel packets, and moves the bonuses to each attribute on the next level up to PlayerAttribute packets.

Players now also send a PlayerSkill packet whenever their progress towards a new point in a skill changes. This was previously avoided so as to not have massive packet spam.

(cherry picked from commit ef79a98544)

apps/openmw-mp/Script/Functions/Stats.cpp

@@ -1,6 +1,3 @@
-//
-// Created by koncord on 29.02.16.
-//
 #include "Stats.hpp"
 
 #include <iostream>
@@ -433,6 +430,8 @@ void StatsFunctions::SetSkillBase(unsigned short pid, unsigned short skillId, in
         return;
 
     player->npcStats.mSkills[skillId].mBase = value;
+
+    player->skillChanges.skillIndexes.push_back(skillId);
 }
 
 void StatsFunctions::ClearSkillModifier(unsigned short pid, unsigned short skillId) noexcept
@@ -444,6 +443,8 @@ void StatsFunctions::ClearSkillModifier(unsigned short pid, unsigned short skill
         return;
 
     player->npcStats.mSkills[skillId].mMod = 0;
+
+    player->skillChanges.skillIndexes.push_back(skillId);
 }
 
 void StatsFunctions::SetSkillProgress(unsigned short pid, unsigned short skillId, double value) noexcept
@@ -455,6 +456,8 @@ void StatsFunctions::SetSkillProgress(unsigned short pid, unsigned short skillId
         return;
 
     player->npcStats.mSkills[skillId].mProgress = value;
+
+    player->skillChanges.skillIndexes.push_back(skillId);
 }
 
 void StatsFunctions::SetSkillIncrease(unsigned short pid, unsigned int attributeId, int value) noexcept
@@ -527,6 +530,8 @@ void StatsFunctions::SendSkills(unsigned short pid) noexcept
     mwmp::Networking::get().getPlayerPacketController()->GetPacket(ID_PLAYER_SKILL)->setPlayer(player);
     mwmp::Networking::get().getPlayerPacketController()->GetPacket(ID_PLAYER_SKILL)->Send(false);
     mwmp::Networking::get().getPlayerPacketController()->GetPacket(ID_PLAYER_SKILL)->Send(true);
+
+    player->skillChanges.skillIndexes.clear();
 }
 
 void StatsFunctions::SendLevel(unsigned short pid) noexcept

apps/openmw-mp/Script/Functions/Stats.hpp

@@ -1,7 +1,3 @@
-//
-// Created by koncord on 30.08.16.
-//
-
 #ifndef OPENMW_STATAPI_HPP
 #define OPENMW_STATAPI_HPP
 
@@ -584,7 +580,9 @@ public:
     static void SendStatsDynamic(unsigned short pid) noexcept;
 
     /**
-    * \brief Send a PlayerAttribute packet with a player's attributes.
+    * \brief Send a PlayerAttribute packet with a player's attributes and bonuses
+    *        to those attributes at the next level up (the latter being called
+    *        "skill increases" as in OpenMW).
     *
     * It is always sent to all players.
     *
@@ -594,8 +592,7 @@ public:
     static void SendAttributes(unsigned short pid) noexcept;
 
     /**
-    * \brief Send a PlayerSkill packet with a player's skills, skill increases, and
+    * \brief Send a PlayerSkill packet with a player's skills.
-    *        progress towards the next level up.
     *
     * It is always sent to all players.
     *
@@ -605,7 +602,8 @@ public:
     static void SendSkills(unsigned short pid) noexcept;
 
     /**
-    * \brief Send a PlayerLevel packet with a player's character level.
+    * \brief Send a PlayerLevel packet with a player's character level and
+    *        progress towards the next level up
     *
     * It is always sent to all players.
     *

apps/openmw/mwmp/LocalPlayer.cpp

@@ -247,6 +247,12 @@ void LocalPlayer::updateAttributes(bool forceUpdate)
             ptrNpcStats.getAttribute(i).writeState(creatureStats.mAttributes[i]);
             attributesChanged = true;
         }
+
+        if (ptrNpcStats.getSkillIncrease(i) != npcStats.mSkillIncrease[i])
+        {
+            npcStats.mSkillIncrease[i] = ptrNpcStats.getSkillIncrease(i);
+            attributesChanged = true;
+        }
     }
 
     if (attributesChanged || forceUpdate)
@@ -265,53 +271,39 @@ void LocalPlayer::updateSkills(bool forceUpdate)
     MWWorld::Ptr ptrPlayer = getPlayerPtr();
     const MWMechanics::NpcStats &ptrNpcStats = ptrPlayer.getClass().getNpcStats(ptrPlayer);
 
-    // Track whether skills have changed their values, but not whether
-    // progress towards skill increases has changed (to not spam server
-    // with packets every time tiny progress is made)
-    bool skillsChanged = false;
-
     for (int i = 0; i < 27; ++i)
     {
+        // Update a skill if its base value has changed at all or its progress has changed enough
         if (ptrNpcStats.getSkill(i).getBase() != npcStats.mSkills[i].mBase ||
-            ptrNpcStats.getSkill(i).getModifier() != npcStats.mSkills[i].mMod)
+            ptrNpcStats.getSkill(i).getProgress() != npcStats.mSkills[i].mProgress ||
+            forceUpdate)
         {
             ptrNpcStats.getSkill(i).writeState(npcStats.mSkills[i]);
-            skillsChanged = true;
+            skillChanges.skillIndexes.push_back(i);
-        }
-        // If we only have skill progress, remember it for future packets,
-        // but don't send a packet just because of this
-        else if (ptrNpcStats.getSkill(i).getProgress() != npcStats.mSkills[i].mProgress)
-            ptrNpcStats.getSkill(i).writeState(npcStats.mSkills[i]);
         }
-
-    for (int i = 0; i < 8; i++)
-    {
-        if (ptrNpcStats.getSkillIncrease(i) != npcStats.mSkillIncrease[i])
-            npcStats.mSkillIncrease[i] = ptrNpcStats.getSkillIncrease(i);
     }
 
-    if (skillsChanged || forceUpdate)
+    if (skillChanges.skillIndexes.size() > 0)
     {
-        npcStats.mLevelProgress = ptrNpcStats.getLevelProgress();
         getNetworking()->getPlayerPacket(ID_PLAYER_SKILL)->setPlayer(this);
         getNetworking()->getPlayerPacket(ID_PLAYER_SKILL)->Send();
+        skillChanges.skillIndexes.clear();
     }
 }
 
 void LocalPlayer::updateLevel(bool forceUpdate)
 {
     MWWorld::Ptr ptrPlayer = getPlayerPtr();
-    const MWMechanics::CreatureStats &ptrCreatureStats = ptrPlayer.getClass().getCreatureStats(ptrPlayer);
+    const MWMechanics::NpcStats &ptrNpcStats = ptrPlayer.getClass().getNpcStats(ptrPlayer);
 
-    if (ptrCreatureStats.getLevel() != creatureStats.mLevel || forceUpdate)
+    if (ptrNpcStats.getLevel() != creatureStats.mLevel ||
+        ptrNpcStats.getLevelProgress() != npcStats.mLevelProgress ||
+        forceUpdate)
     {
-        creatureStats.mLevel = ptrCreatureStats.getLevel();
+        creatureStats.mLevel = ptrNpcStats.getLevel();
+        npcStats.mLevelProgress = ptrNpcStats.getLevelProgress();
         getNetworking()->getPlayerPacket(ID_PLAYER_LEVEL)->setPlayer(this);
         getNetworking()->getPlayerPacket(ID_PLAYER_LEVEL)->Send();
-
-        // Also update skills to refresh level progress and attribute bonuses
-        // for next level up
-        updateSkills(true);
     }
 }
 

components/openmw-mp/Base/BasePlayer.hpp

@@ -95,6 +95,14 @@ namespace mwmp
         int type; // 0 - Cell load, 1 - Cell unload
     };
 
+    // Track only the indexes of the skills that have been changed,
+    // with the skill values themselves being stored in npcStats.mSkills
+    struct SkillChanges
+    {
+        std::vector<int> skillIndexes;
+        unsigned int count;
+    };
+
     struct JournalChanges
     {
         std::vector<JournalItem> journalItems;
@@ -241,6 +249,8 @@ namespace mwmp
         int day;
         double hour;
 
+        SkillChanges skillChanges;
+
         InventoryChanges inventoryChanges;
         SpellbookChanges spellbookChanges;
         QuickKeyChanges quickKeyChanges;

components/openmw-mp/Packets/Player/PacketPlayerAttribute.cpp

@@ -17,4 +17,6 @@ void PacketPlayerAttribute::Packet(RakNet::BitStream *bs, bool send)
     PlayerPacket::Packet(bs, send);
 
     RW(player->creatureStats.mAttributes, send);
+
+    RW(player->npcStats.mSkillIncrease, send);
 }

components/openmw-mp/Packets/Player/PacketPlayerLevel.cpp

@@ -13,4 +13,6 @@ void PacketPlayerLevel::Packet(RakNet::BitStream *bs, bool send)
     PlayerPacket::Packet(bs, send);
 
     RW(player->creatureStats.mLevel, send);
+
+    RW(player->npcStats.mLevelProgress, send);
 }

components/openmw-mp/Packets/Player/PacketPlayerSkill.cpp

@@ -1,7 +1,3 @@
-//
-// Created by koncord on 17.03.16.
-//
-
 #include "PacketPlayerSkill.hpp"
 
 #include <components/openmw-mp/NetworkMessages.hpp>
@@ -18,10 +14,22 @@ void PacketPlayerSkill::Packet(RakNet::BitStream *bs, bool send)
 {
     PlayerPacket::Packet(bs, send);
 
-    RW(player->npcStats.mSkills, send);
+    if (send)
+        player->skillChanges.count = (unsigned int)(player->skillChanges.skillIndexes.size());
+    else
+        player->skillChanges.skillIndexes.clear();
+
+    RW(player->skillChanges.count, send);
+
+    for (unsigned int i = 0; i < player->skillChanges.count; i++)
+    {
+        int skillId;
 
+        if (send)
+            skillId = player->skillChanges.skillIndexes.at(i);
 
-    RW(player->npcStats.mSkillIncrease, send);
+        RW(skillId, send);
 
-    RW(player->npcStats.mLevelProgress, send);
+        RW(player->npcStats.mSkills[skillId], send);
+    }
 }