[General] Reduce inventory-sending hooks to just 2 in ContainerStore

Whenever an item is added to or removed from the player's ContainerStore, that player sends a PlayerInventory packet with just that addition or removal.

This eliminates all the unnecessary packet spam related to oversized PlayerInventory packets that had existed in one form or another since the initial implementation of inventory sync in 1b259e2d33

Additionally, move booleans from BasePlayer to LocalPlayer when they are only needed on the client, and make the usage of the isReceivingQuickKeys boolean consistent with the new isReceivingInventory boolean by having them both in the processors of their associated packets.

apps/openmw/mwgui/container.cpp

@@ -276,7 +276,6 @@ namespace MWGui
             Mark this container as closed for multiplayer logic purposes
         */
         mwmp::Main::get().getLocalPlayer()->clearCurrentContainer();
-        mwmp::Main::get().getLocalPlayer()->updateInventory();
         /*
             End of tes3mp addition
         */

apps/openmw/mwgui/hud.cpp

@@ -15,7 +15,6 @@
 #include "../mwmp/Main.hpp"
 #include "../mwmp/Networking.hpp"
 #include "../mwmp/ObjectList.hpp"
-#include "../mwmp/LocalPlayer.hpp"
 #include "../mwworld/cellstore.hpp"
 /*
     End of tes3mp addition
@@ -71,16 +70,12 @@ namespace MWGui
 
                 Send an ID_OBJECT_PLACE packet every time an object is dropped into the world from
                 the inventory screen
-
-                Send an ID_PLAYER_INVENTORY packet about the item's removal
             */
             mwmp::ObjectList *objectList = mwmp::Main::get().getNetworking()->getObjectList();
             objectList->reset();
             objectList->packetOrigin = mwmp::CLIENT_GAMEPLAY;
             objectList->addObjectPlace(dropped, true);
             objectList->sendObjectPlace();
-
-            mwmp::Main::get().getLocalPlayer()->sendItemChange(dropped, count, mwmp::InventoryChanges::REMOVE);
             /*
                 End of tes3mp addition
             */

apps/openmw/mwgui/inventorywindow.cpp

@@ -723,20 +723,12 @@ namespace MWGui
 
             Send an ID_OBJECT_DELETE packet every time an item from the world is picked up
             by the player through the inventory HUD
-
-            Send an ID_PLAYER_INVENTORY packet as well because of the item thus gained
-            by the player
         */
         mwmp::ObjectList *objectList = mwmp::Main::get().getNetworking()->getObjectList();
         objectList->reset();
         objectList->packetOrigin = mwmp::CLIENT_GAMEPLAY;
         objectList->addObjectDelete(object);
         objectList->sendObjectDelete();
-
-        // If the item is gold, make sure we get the correct value
-        unsigned int itemCount = object.getClass().isGold(object) ? object.getCellRef().getGoldValue() : object.getRefData().getCount();
-
-        mwmp::Main::get().getLocalPlayer()->sendItemChange(object, itemCount, mwmp::InventoryChanges::ADD);
         /*
             End of tes3mp addition
         */

apps/openmw/mwgui/tradewindow.cpp

@@ -6,17 +6,6 @@
 #include <MyGUI_InputManager.h>
 #include <MyGUI_ControllerManager.h>
 
-/*
-    Start of tes3mp addition
-
-    Include additional headers for multiplayer purposes
-*/
-#include "../mwmp/Main.hpp"
-#include "../mwmp/LocalPlayer.hpp"
-/*
-    End of tes3mp addition
-*/
-
 #include <components/widgets/numericeditbox.hpp>
 
 #include "../mwbase/environment.hpp"
@@ -367,16 +356,6 @@ namespace MWGui
                         mPtr.getClass().getCreatureStats(mPtr).getGoldPool() - mCurrentBalance );
         }
 
-        /*
-            Start of tes3mp addition
-
-            Send an ID_PLAYER_INVENTORY packet every time a player completes a trade
-        */
-        mwmp::Main::get().getLocalPlayer()->sendInventory();
-        /*
-            End of tes3mp addition
-        */
-
         eventTradeDone();
 
         MWBase::Environment::get().getWindowManager()->playSound("Item Gold Up");

apps/openmw/mwgui/trainingwindow.cpp

@@ -2,17 +2,6 @@
 
 #include <MyGUI_Gui.h>
 
-/*
-    Start of tes3mp addition
-
-    Include additional headers for multiplayer purposes
-*/
-#include "../mwmp/Main.hpp"
-#include "../mwmp/LocalPlayer.hpp"
-/*
-    End of tes3mp addition
-*/
-
 #include "../mwbase/windowmanager.hpp"
 #include "../mwbase/environment.hpp"
 #include "../mwbase/world.hpp"
@@ -178,16 +167,6 @@ namespace MWGui
         // remove gold
         player.getClass().getContainerStore(player).remove(MWWorld::ContainerStore::sGoldId, price, player);
 
-        /*
-            Start of tes3mp addition
-
-            Send an ID_PLAYER_INVENTORY packet with the gold lost
-        */
-        mwmp::Main::get().getLocalPlayer()->sendItemChange(MWWorld::ContainerStore::sGoldId, price, mwmp::InventoryChanges::REMOVE);
-        /*
-            End of tes3mp addition
-        */
-
         // add gold to NPC trading gold pool
         npcStats.setGoldPool(npcStats.getGoldPool() + price);
 

apps/openmw/mwmechanics/enchanting.cpp

@@ -10,7 +10,6 @@
 #include <components/openmw-mp/Log.hpp>
 #include "../mwmp/Main.hpp"
 #include "../mwmp/Networking.hpp"
-#include "../mwmp/LocalPlayer.hpp"
 #include "../mwmp/Worldstate.hpp"
 /*
     End of tes3mp addition
@@ -108,9 +107,6 @@ namespace MWMechanics
 
             Don't add the new item to the player's inventory and instead expect the server to
             add it
-
-            Before using the applyEnchantment() method, send an ID_PLAYER_INVENTORY packet
-            removing the old item from the player's inventory
             
             The applyEnchantment() method is where the record of the newly enchanted will be sent
             to the server, causing the server to send back the player's inventory with the new item
@@ -123,8 +119,6 @@ namespace MWMechanics
         if(!mSelfEnchanting)
             payForEnchantment();
 
-        mwmp::Main::get().getLocalPlayer()->sendItemChange(mOldItemPtr, 1, mwmp::InventoryChanges::REMOVE);
-
         std::string newItemId = mOldItemPtr.getClass().applyEnchantment(mOldItemPtr, enchantmentPtr->mId, getGemCharge(), mNewItemName);
         /*
             End of tes3mp change (major)
@@ -337,16 +331,6 @@ namespace MWMechanics
 
         store.remove(MWWorld::ContainerStore::sGoldId, getEnchantPrice(), player);
 
-        /*
-            Start of tes3mp addition
-
-            Send an ID_PLAYER_INVENTORY packet removing the gold from the player
-        */
-        mwmp::Main::get().getLocalPlayer()->sendItemChange(MWWorld::ContainerStore::sGoldId, getEnchantPrice(), mwmp::InventoryChanges::REMOVE);
-        /*
-            End of tes3mp addition
-        */
-
         // add gold to NPC trading gold pool
         CreatureStats& enchanterStats = mEnchanter.getClass().getCreatureStats(mEnchanter);
         enchanterStats.setGoldPool(enchanterStats.getGoldPool() + getEnchantPrice());

apps/openmw/mwmp/LocalPlayer.cpp

@@ -76,6 +76,7 @@ LocalPlayer::LocalPlayer()
     scale = 1;
     isWerewolf = false;
 
+    isReceivingInventory = false;
     isReceivingQuickKeys = false;
     isPlayingAnimation = false;
     diedSinceArrestAttempt = false;
@@ -456,9 +457,6 @@ void LocalPlayer::updateCell(bool forceUpdate)
         getNetworking()->getPlayerPacket(ID_PLAYER_CELL_CHANGE)->Send();
 
         isChangingRegion = false;
-
-        // Also check if the inventory needs to be updated
-        updateInventory();
     }
 }
 
@@ -1204,10 +1202,6 @@ void LocalPlayer::setQuickKeys()
 
     LOG_MESSAGE_SIMPLE(Log::LOG_INFO, "Received ID_PLAYER_QUICKKEYS from server");
 
-    // Because we send QuickKeys packets from the same OpenMW methods that we use to set received ones with,
-    // we need a boolean to prevent their sending here
-    isReceivingQuickKeys = true;
-
     for (const auto &quickKey : quickKeyChanges.quickKeys)
     {
         LOG_APPEND(Log::LOG_INFO, "- slot: %i, type: %i, itemId: %s", quickKey.slot, quickKey.type, quickKey.itemId.c_str());
@@ -1245,8 +1239,6 @@ void LocalPlayer::setQuickKeys()
         else
             MWBase::Environment::get().getWindowManager()->setQuickKey(quickKey.slot, quickKey.type, 0);
     }
-
-    isReceivingQuickKeys = false;
 }
 
 void LocalPlayer::setFactions()

apps/openmw/mwmp/LocalPlayer.hpp

@@ -17,6 +17,11 @@ namespace mwmp
 
         time_t deathTime;
 
+        bool isReceivingInventory;
+        bool isReceivingQuickKeys;
+        bool isPlayingAnimation;
+        bool diedSinceArrestAttempt;
+
         void update();
 
         bool processCharGen();

apps/openmw/mwmp/processors/player/ProcessorPlayerInventory.hpp

@@ -1,7 +1,3 @@
-//
-// Created by koncord on 16.04.17.
-//
-
 #ifndef OPENMW_PROCESSORPLAYERUPDATEINVENTORY_HPP
 #define OPENMW_PROCESSORPLAYERUPDATEINVENTORY_HPP
 
@@ -30,12 +26,18 @@ namespace mwmp
                 LocalPlayer &localPlayer = static_cast<LocalPlayer&>(*player);
                 int inventoryAction = localPlayer.inventoryChanges.action;
 
+                // Because we send PlayerInventory packets from the same OpenMW methods that we use to set the
+                // items received, we need to set a boolean to prevent resending the items set here
+                localPlayer.isReceivingInventory = true;
+
                 if (inventoryAction == InventoryChanges::ADD)
                     localPlayer.addItems();
                 else if (inventoryAction == InventoryChanges::REMOVE)
                     localPlayer.removeItems();
                 else // InventoryChanges::SET
                     localPlayer.setInventory();
+
+                localPlayer.isReceivingInventory = false;
             }
         }
     };

apps/openmw/mwmp/processors/player/ProcessorPlayerQuickKeys.hpp

@@ -23,7 +23,14 @@ namespace mwmp
             if (!isRequest())
             {
                 LocalPlayer &localPlayer = static_cast<LocalPlayer&>(*player);
+
+                // Because we send PlayerQuickKeys packets from the same OpenMW methods that we use to set the
+                // quick keys received, we need to set a boolean to prevent resending the keys set here
+                localPlayer.isReceivingQuickKeys = true;
+
                 localPlayer.setQuickKeys();
+
+                localPlayer.isReceivingQuickKeys = false;
             }
         }
     };

apps/openmw/mwscript/containerextensions.cpp

@@ -92,17 +92,6 @@ namespace MWScript
                         End of tes3mp change (major)
                     */
 
-                        /*
-                            Start of tes3mp addition
-
-                            Send an ID_PLAYER_INVENTORY packet every time a player gains an item
-                            through a script
-                        */
-                        mwmp::Main::get().getLocalPlayer()->sendItemChange(item, count, mwmp::InventoryChanges::ADD);
-                        /*
-                            End of tes3mp addition
-                        */
-
                         // The two GMST entries below expand to strings informing the player of what, and how many of it has been added to their inventory
                         std::string msgBox;
                         std::string itemName = itemPtr.getClass().getName(itemPtr);
@@ -226,7 +215,6 @@ namespace MWScript
                     if ((numRemoved > 0)
                         && (ptr == MWMechanics::getPlayer()))
                     {
-                        mwmp::Main::get().getLocalPlayer()->sendItemChange(item, count, mwmp::InventoryChanges::REMOVE);
                     /*
                         End of tes3mp change (major)
                     */

apps/openmw/mwworld/actiontake.cpp

@@ -8,7 +8,6 @@
 #include "../mwmp/Main.hpp"
 #include "../mwmp/Networking.hpp"
 #include "../mwmp/ObjectList.hpp"
-#include "../mwmp/LocalPlayer.hpp"
 /*
     End of tes3mp addition
 */
@@ -36,20 +35,12 @@ namespace MWWorld
 
             Send an ID_OBJECT_DELETE packet every time an item is taken from the world
             by the player outside of the inventory screen
-
-            Send an ID_PLAYER_INVENTORY packet as well with the item thus gained
-            by the player
         */
         mwmp::ObjectList *objectList = mwmp::Main::get().getNetworking()->getObjectList();
         objectList->reset();
         objectList->packetOrigin = mwmp::CLIENT_GAMEPLAY;
         objectList->addObjectDelete(getTarget());
         objectList->sendObjectDelete();
-
-        // If the item is gold, make sure we get the correct value
-        unsigned int itemCount = getTarget().getClass().isGold(newitem) ? getTarget().getCellRef().getGoldValue() : getTarget().getRefData().getCount();
-
-        mwmp::Main::get().getLocalPlayer()->sendItemChange(getTarget(), itemCount, mwmp::InventoryChanges::ADD);
         /*
             End of tes3mp addition
         */

apps/openmw/mwworld/containerstore.cpp

@@ -4,6 +4,18 @@
 #include <typeinfo>
 #include <stdexcept>
 
+/*
+    Start of tes3mp addition
+
+    Include additional headers for multiplayer purposes
+*/
+#include "../mwmp/Main.hpp"
+#include "../mwmp/Networking.hpp"
+#include "../mwmp/LocalPlayer.hpp"
+/*
+    End of tes3mp addition
+*/
+
 #include <components/esm/inventorystate.hpp>
 
 #include "../mwbase/environment.hpp"
@@ -284,6 +296,31 @@ MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add (const Ptr& itemPtr
     // The copy of the original item we just made
     MWWorld::Ptr item = *it;
 
+    /*
+        Start of tes3mp addition
+
+        Send an ID_PLAYER_INVENTORY packet every time an item gets added for a player here
+    */
+    if (actorPtr == player && this == &player.getClass().getContainerStore(player))
+    {
+        mwmp::LocalPlayer *localPlayer = mwmp::Main::get().getLocalPlayer();
+
+        if (!localPlayer->isReceivingInventory)
+        {
+            int realCount = count;
+
+            if (itemPtr.getClass().isGold(itemPtr))
+            {
+                realCount = realCount * itemPtr.getClass().getValue(itemPtr);
+            }
+
+            localPlayer->sendItemChange(item, realCount, mwmp::InventoryChanges::ADD);
+        }
+    }
+    /*
+        End of tes3mp addition
+    */
+
     // we may have copied an item from the world, so reset a few things first
     item.getRefData().setBaseNode(NULL); // Especially important, otherwise scripts on the item could think that it's actually in a cell
     ESM::Position pos;
@@ -422,6 +459,24 @@ int MWWorld::ContainerStore::remove(const Ptr& item, int count, const Ptr& actor
 {
     assert(this == item.getContainerStore());
 
+    /*
+        Start of tes3mp addition
+
+        Send an ID_PLAYER_INVENTORY packet every time an item gets removed for a player here
+    */
+    Ptr player = MWBase::Environment::get().getWorld()->getPlayerPtr();
+
+    if (actor == player && this == &player.getClass().getContainerStore(player))
+    {
+        mwmp::LocalPlayer *localPlayer = mwmp::Main::get().getLocalPlayer();
+
+        if (!localPlayer->isReceivingInventory)
+            localPlayer->sendItemChange(item, count, mwmp::InventoryChanges::REMOVE);
+    }
+    /*
+        End of tes3mp addition
+    */
+
     int toRemove = count;
     RefData& itemRef = item.getRefData();
 

components/openmw-mp/Base/BasePlayer.hpp

@@ -325,10 +325,6 @@ namespace mwmp
         std::string selectedSpellId;
 
         mwmp::Item usedItem;
-
-        bool isReceivingQuickKeys;
-        bool isPlayingAnimation;
-        bool diedSinceArrestAttempt;
     };
 }