Merge pull request #2548 from akortunov/stack

Improve items ownership handling

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

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)

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);

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))

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();

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;
         }

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;
 }

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.

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)

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);
 

apps/openmw/mwgui/tooltips.cpp

@@ -652,19 +652,22 @@ namespace MWGui
         std::string ret;
         ret += getMiscString(cellref.getOwner(), "Owner");
         const std::string factionId = cellref.getFaction();
-        ret += getMiscString(factionId, "Faction");
+        if (!factionId.empty())
-        if (!factionId.empty() && cellref.getFactionRank() >= 0)
         {
             const MWWorld::ESMStore &store = MWBase::Environment::get().getWorld()->getStore();
             const ESM::Faction *fact = store.get<ESM::Faction>().search(factionId);
             if (fact != nullptr)
             {
-                int rank = cellref.getFactionRank();
+                ret += getMiscString(fact->mName.empty() ? factionId : fact->mName, "Owner Faction");
-                const std::string rankName = fact->mRanks[rank];
+                if (cellref.getFactionRank() >= 0)
-                if (rankName.empty())
+                {
-                    ret += getValueString(cellref.getFactionRank(), "Rank");
+                    int rank = cellref.getFactionRank();
-                else
+                    const std::string rankName = fact->mRanks[rank];
-                    ret += getMiscString(rankName, "Rank");
+                    if (rankName.empty())
+                        ret += getValueString(cellref.getFactionRank(), "Rank");
+                    else
+                        ret += getMiscString(rankName, "Rank");
+                }
             }
         }
 

apps/openmw/mwworld/containerstore.cpp

@@ -241,7 +241,6 @@ bool MWWorld::ContainerStore::stacks(const ConstPtr& ptr1, const ConstPtr& ptr2)
     }
 
     return ptr1 != ptr2 // an item never stacks onto itself
-        && ptr1.getCellRef().getOwner() == ptr2.getCellRef().getOwner()
         && ptr1.getCellRef().getSoul() == ptr2.getCellRef().getSoul()
 
         && ptr1.getClass().getRemainingUsageTime(ptr1) == ptr2.getClass().getRemainingUsageTime(ptr2)
@@ -259,30 +258,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();
+    MWWorld::ContainerStoreIterator it = addImp(itemPtr, count);
-
-    // 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);
 
     // The copy of the original item we just made
     MWWorld::Ptr item = *it;
@@ -298,7 +281,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);

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.

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()

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).
             ///