Do not store owners for items in container stores (bug OpenMW#1933)

akortunov · Oct 5, 2019 · bfe1f6e · bfe1f6e
1 parent 9a27714
commit bfe1f6e
Show file tree

Hide file tree

Showing 14 changed files with 25 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 ------
 
     Bug #1515: Opening console masks dialogue, inventory menu
+    Bug #1933: Actors can have few stocks of the same item
     Bug #2395: Duplicated plugins in the launcher when multiple data directories provide the same plugin
     Bug #2969: Scripted items can stack
     Bug #2976: Data lines in global openmw.cfg take priority over user openmw.cfg

diff --git a/apps/openmw/mwgui/companionitemmodel.cpp b/apps/openmw/mwgui/companionitemmodel.cpp
@@ -25,12 +25,12 @@ namespace MWGui
     {
     }
 
-    MWWorld::Ptr CompanionItemModel::copyItem (const ItemStack& item, size_t count, bool setNewOwner=false)
+    MWWorld::Ptr CompanionItemModel::copyItem (const ItemStack& item, size_t count)
     {
         if (hasProfit(mActor))
             modifyProfit(mActor, item.mBase.getClass().getValue(item.mBase) * count);
 
-        return InventoryItemModel::copyItem(item, count, setNewOwner);
+        return InventoryItemModel::copyItem(item, count);
     }
 
     void CompanionItemModel::removeItem (const ItemStack& item, size_t count)

diff --git a/apps/openmw/mwgui/companionitemmodel.hpp b/apps/openmw/mwgui/companionitemmodel.hpp
@@ -13,7 +13,7 @@ namespace MWGui
     public:
         CompanionItemModel (const MWWorld::Ptr& actor);
 
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner);
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count);
         virtual void removeItem (const ItemStack& item, size_t count);
 
         bool hasProfit(const MWWorld::Ptr& actor);

diff --git a/apps/openmw/mwgui/containeritemmodel.cpp b/apps/openmw/mwgui/containeritemmodel.cpp
@@ -91,7 +91,7 @@ ItemModel::ModelIndex ContainerItemModel::getIndex (ItemStack item)
     return -1;
 }
 
-MWWorld::Ptr ContainerItemModel::copyItem (const ItemStack& item, size_t count, bool setNewOwner)
+MWWorld::Ptr ContainerItemModel::copyItem (const ItemStack& item, size_t count)
 {
     const MWWorld::Ptr& source = mItemSources[mItemSources.size()-1];
     if (item.mBase.getContainerStore() == &source.getClass().getContainerStore(source))

diff --git a/apps/openmw/mwgui/containeritemmodel.hpp b/apps/openmw/mwgui/containeritemmodel.hpp
@@ -26,7 +26,7 @@ namespace MWGui
         virtual ModelIndex getIndex (ItemStack item);
         virtual size_t getItemCount();
 
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner=false);
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count);
         virtual void removeItem (const ItemStack& item, size_t count);
 
         virtual void update();

diff --git a/apps/openmw/mwgui/hud.cpp b/apps/openmw/mwgui/hud.cpp
@@ -38,7 +38,7 @@ namespace MWGui
     public:
         WorldItemModel(float left, float top) : mLeft(left), mTop(top) {}
         virtual ~WorldItemModel() {}
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner=false)
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count)
         {
             MWBase::World* world = MWBase::Environment::get().getWorld();
 
@@ -47,8 +47,7 @@ namespace MWGui
                 dropped = world->placeObject(item.mBase, mLeft, mTop, count);
             else
                 dropped = world->dropObjectOnGround(world->getPlayerPtr(), item.mBase, count);
-            if (setNewOwner)
-                dropped.getCellRef().setOwner("");
+            dropped.getCellRef().setOwner("");
 
             return dropped;
         }

diff --git a/apps/openmw/mwgui/inventoryitemmodel.cpp b/apps/openmw/mwgui/inventoryitemmodel.cpp
@@ -46,11 +46,11 @@ ItemModel::ModelIndex InventoryItemModel::getIndex (ItemStack item)
     return -1;
 }
 
-MWWorld::Ptr InventoryItemModel::copyItem (const ItemStack& item, size_t count, bool setNewOwner)
+MWWorld::Ptr InventoryItemModel::copyItem (const ItemStack& item, size_t count)
 {
     if (item.mBase.getContainerStore() == &mActor.getClass().getContainerStore(mActor))
         throw std::runtime_error("Item to copy needs to be from a different container!");
-    return *mActor.getClass().getContainerStore(mActor).add(item.mBase, count, mActor, setNewOwner);
+    return *mActor.getClass().getContainerStore(mActor).add(item.mBase, count, mActor);
 }
 
 void InventoryItemModel::removeItem (const ItemStack& item, size_t count)
@@ -88,7 +88,7 @@ MWWorld::Ptr InventoryItemModel::moveItem(const ItemStack &item, size_t count, I
     if (item.mFlags & ItemStack::Flag_Bound)
         return MWWorld::Ptr();
 
-    MWWorld::Ptr ret = otherModel->copyItem(item, count, false);
+    MWWorld::Ptr ret = otherModel->copyItem(item, count);
     removeItem(item, count);
     return ret;
 }

diff --git a/apps/openmw/mwgui/inventoryitemmodel.hpp b/apps/openmw/mwgui/inventoryitemmodel.hpp
@@ -17,7 +17,7 @@ namespace MWGui
 
         virtual bool onTakeItem(const MWWorld::Ptr &item, int count);
 
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner=false);
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count);
         virtual void removeItem (const ItemStack& item, size_t count);
 
         /// Move items from this model to \a otherModel.

diff --git a/apps/openmw/mwgui/itemmodel.cpp b/apps/openmw/mwgui/itemmodel.cpp
@@ -116,9 +116,9 @@ namespace MWGui
         return mSourceModel->allowedToUseItems();
     }
 
-    MWWorld::Ptr ProxyItemModel::copyItem (const ItemStack& item, size_t count, bool setNewOwner)
+    MWWorld::Ptr ProxyItemModel::copyItem (const ItemStack& item, size_t count)
     {
-        return mSourceModel->copyItem (item, count, setNewOwner);
+        return mSourceModel->copyItem (item, count);
     }
 
     void ProxyItemModel::removeItem (const ItemStack& item, size_t count)

diff --git a/apps/openmw/mwgui/itemmodel.hpp b/apps/openmw/mwgui/itemmodel.hpp
@@ -67,7 +67,7 @@ namespace MWGui
 
         /// @param setNewOwner If true, set the copied item's owner to the actor we are copying to,
         ///                    otherwise reset owner to ""
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner=false) = 0;
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count) = 0;
         virtual void removeItem (const ItemStack& item, size_t count) = 0;
 
         /// Is the player allowed to use items from this item model? (default true)
@@ -97,7 +97,7 @@ namespace MWGui
         virtual bool onDropItem(const MWWorld::Ptr &item, int count);
         virtual bool onTakeItem(const MWWorld::Ptr &item, int count);
 
-        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count, bool setNewOwner=false);
+        virtual MWWorld::Ptr copyItem (const ItemStack& item, size_t count);
         virtual void removeItem (const ItemStack& item, size_t count);
         virtual ModelIndex getIndex (ItemStack item);
 

diff --git a/apps/openmw/mwworld/containerstore.cpp b/apps/openmw/mwworld/containerstore.cpp
@@ -259,30 +259,14 @@ bool MWWorld::ContainerStore::stacks(const ConstPtr& ptr1, const ConstPtr& ptr2)
 MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add(const std::string &id, int count, const Ptr &actorPtr)
 {
     MWWorld::ManualRef ref(MWBase::Environment::get().getWorld()->getStore(), id, count);
-    return add(ref.getPtr(), count, actorPtr, true);
+    return add(ref.getPtr(), count, actorPtr);
 }
 
-MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add (const Ptr& itemPtr, int count, const Ptr& actorPtr, bool setOwner)
+MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add (const Ptr& itemPtr, int count, const Ptr& actorPtr)
 {
     Ptr player = MWBase::Environment::get().getWorld ()->getPlayerPtr();
 
-    MWWorld::ContainerStoreIterator it = end();
-
-    // HACK: Set owner on the original item, then reset it after we have copied it
-    // If we set the owner on the copied item, it would not stack correctly...
-    std::string oldOwner = itemPtr.getCellRef().getOwner();
-    if (!setOwner || actorPtr == MWMechanics::getPlayer()) // No point in setting owner to the player - NPCs will not respect this anyway
-    {
-        itemPtr.getCellRef().setOwner("");
-    }
-    else
-    {
-        itemPtr.getCellRef().setOwner(actorPtr.getCellRef().getRefId());
-    }
-
-    it = addImp(itemPtr, count);
-
-    itemPtr.getCellRef().setOwner(oldOwner);
+    MWWorld::ContainerStoreIterator it = addImp(itemPtr, count);
 
     // The copy of the original item we just made
     MWWorld::Ptr item = *it;
@@ -298,7 +282,8 @@ MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add (const Ptr& itemPtr
     pos.pos[2] = 0;
     item.getCellRef().setPosition(pos);
 
-    // reset ownership stuff, owner was already handled above
+    // We do not need to store owners for items in container stores - we do not use it anyway.
+    item.getCellRef().setOwner("");
     item.getCellRef().resetGlobalVariable();
     item.getCellRef().setFaction("");
     item.getCellRef().setFactionRank(-1);

diff --git a/apps/openmw/mwworld/containerstore.hpp b/apps/openmw/mwworld/containerstore.hpp
@@ -131,7 +131,7 @@ namespace MWWorld
 
             bool hasVisibleItems() const;
 
-            virtual ContainerStoreIterator add (const Ptr& itemPtr, int count, const Ptr& actorPtr, bool setOwner=false);
+            virtual ContainerStoreIterator add (const Ptr& itemPtr, int count, const Ptr& actorPtr);
             ///< Add the item pointed to by \a ptr to this container. (Stacks automatically if needed)
             ///
             /// \note The item pointed to is not required to exist beyond this function call.

diff --git a/apps/openmw/mwworld/inventorystore.cpp b/apps/openmw/mwworld/inventorystore.cpp
@@ -133,9 +133,9 @@ MWWorld::InventoryStore& MWWorld::InventoryStore::operator= (const InventoryStor
     return *this;
 }
 
-MWWorld::ContainerStoreIterator MWWorld::InventoryStore::add(const Ptr& itemPtr, int count, const Ptr& actorPtr, bool setOwner)
+MWWorld::ContainerStoreIterator MWWorld::InventoryStore::add(const Ptr& itemPtr, int count, const Ptr& actorPtr)
 {
-    const MWWorld::ContainerStoreIterator& retVal = MWWorld::ContainerStore::add(itemPtr, count, actorPtr, setOwner);
+    const MWWorld::ContainerStoreIterator& retVal = MWWorld::ContainerStore::add(itemPtr, count, actorPtr);
 
     // Auto-equip items if an armor/clothing or weapon item is added, but not for the player nor werewolves
     if (actorPtr != MWMechanics::getPlayer()

diff --git a/apps/openmw/mwworld/inventorystore.hpp b/apps/openmw/mwworld/inventorystore.hpp
@@ -132,7 +132,7 @@ namespace MWWorld
 
             virtual InventoryStore* clone() { return new InventoryStore(*this); }
 
-            virtual ContainerStoreIterator add (const Ptr& itemPtr, int count, const Ptr& actorPtr, bool setOwner=false);
+            virtual ContainerStoreIterator add (const Ptr& itemPtr, int count, const Ptr& actorPtr);
             ///< Add the item pointed to by \a ptr to this container. (Stacks automatically if needed)
             /// Auto-equip items if specific conditions are fulfilled (see the implementation).
             ///