Edit ContainerStore::stacks for clarifications and correctness

Rename arguments and fix some potential errors (add checks).

apps/openmw/mwworld/containerstore.cpp

@@ -77,22 +77,31 @@ MWWorld::ContainerStoreIterator MWWorld::ContainerStore::end()
     return ContainerStoreIterator (this);
 }
 
-bool MWWorld::ContainerStore::stacks(const Ptr& ptr1, const Ptr& ptr2)
+bool MWWorld::ContainerStore::stacks(const Ptr& stack, const Ptr& item)
 {
-    /// \todo add current enchantment charge here when it is implemented
+    const MWWorld::Class& cls1 = MWWorld::Class::get(stack);
-    if (  Misc::StringUtils::ciEqual(ptr1.getCellRef().mRefID, ptr2.getCellRef().mRefID)
+    const MWWorld::Class& cls2 = MWWorld::Class::get(item);
-          && MWWorld::Class::get(ptr1).getScript(ptr1) == "" // item with a script never stacks
-          && MWWorld::Class::get(ptr1).getEnchantment(ptr1) == "" // item with enchantment never stacks (we could revisit this later, but for now it makes selecting items in the spell window much easier)
-        && ptr1.getCellRef().mOwner == ptr2.getCellRef().mOwner
-        && ptr1.getCellRef().mSoul == ptr2.getCellRef().mSoul
-          // item that is already partly used up never stacks
-          && (!MWWorld::Class::get(ptr1).hasItemHealth(ptr1) || ptr1.getCellRef().mCharge == -1
-              || MWWorld::Class::get(ptr1).getItemMaxHealth(ptr1) == ptr1.getCellRef().mCharge)
-        && (!MWWorld::Class::get(ptr2).hasItemHealth(ptr2) || ptr2.getCellRef().mCharge == -1
-            || MWWorld::Class::get(ptr2).getItemMaxHealth(ptr2) == ptr2.getCellRef().mCharge))
-        return true;
 
-    return false;
+    /// \todo add current enchantment charge here when it is implemented
+    return stack != item // an item never stacks onto itself
+        && Misc::StringUtils::ciEqual(stack.getCellRef().mRefID, item.getCellRef().mRefID)
+        && stack.getCellRef().mOwner == item.getCellRef().mOwner
+        && stack.getCellRef().mSoul == item.getCellRef().mSoul
+
+        // item with a script never stacks
+        && cls1.getScript(stack) == ""
+        && cls2.getScript(item) == ""
+
+        // item with enchantment never stacks (we could revisit this later,
+        // but for now it makes selecting items in the spell window much easier)
+        && cls1.getEnchantment(stack) == ""
+        && cls2.getEnchantment(item) == ""
+
+        // item that is already partly used up never stacks
+        && (!cls1.hasItemHealth(stack) || stack.getCellRef().mCharge == -1
+            || cls1.getItemMaxHealth(stack) == stack.getCellRef().mCharge)
+        && (!cls2.hasItemHealth(item) || item.getCellRef().mCharge == -1
+            || cls2.getItemMaxHealth(item) == item.getCellRef().mCharge);
 }
 
 MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add (const Ptr& itemPtr, const Ptr& actorPtr)

apps/openmw/mwworld/containerstore.hpp

@@ -91,8 +91,9 @@ namespace MWWorld
 
         public:
 
-            virtual bool stacks (const Ptr& ptr1, const Ptr& ptr2);
+            virtual bool stacks (const Ptr& stack, const Ptr& item);
             ///< @return true if the two specified objects can stack with each other
+            /// @note stack is the item that is already in this container
 
             void fill (const ESM::InventoryList& items, const std::string& owner, const MWWorld::ESMStore& store);
             ///< Insert items into *this.

apps/openmw/mwworld/inventorystore.cpp

@@ -276,21 +276,23 @@ void MWWorld::InventoryStore::flagAsModified()
     mMagicEffectsUpToDate = false;
 }
 
-bool MWWorld::InventoryStore::stacks(const Ptr& ptr1, const Ptr& ptr2)
+bool MWWorld::InventoryStore::stacks(const Ptr& stack, const Ptr& item)
 {
-    bool canStack = MWWorld::ContainerStore::stacks(ptr1, ptr2);
+    bool canStack = MWWorld::ContainerStore::stacks(stack, item);
     if (!canStack)
         return false;
 
-    // don't stack if the item being checked against is currently equipped.
+    // don't stack if 'stack' (the item being checked against) is currently equipped.
     for (TSlots::const_iterator iter (mSlots.begin());
         iter!=mSlots.end(); ++iter)
     {
-        if (*iter != end() && ptr1 == **iter)
+        if (*iter != end() && stack == **iter)
-            return false;
+        {
-        if (*iter != end() && ptr2 == **iter)
+            bool stackWhenEquipped = MWWorld::Class::get(**iter).getEquipmentSlots(**iter).second;
+            if (!stackWhenEquipped)
                 return false;
         }
+    }
 
     return true;
 }

apps/openmw/mwworld/inventorystore.hpp

@@ -104,9 +104,9 @@ namespace MWWorld
             ///< \attention This function is internal to the world model and should not be called from
             /// outside.
 
-            virtual bool stacks (const Ptr& ptr1, const Ptr& ptr2);
+            virtual bool stacks (const Ptr& stack, const Ptr& item);
             ///< @return true if the two specified objects can stack with each other
-            /// @note ptr1 is the item that is already in this container
+            /// @note stack is the item that is already in this container (it may be equipped)
 
             virtual int remove(const Ptr& item, int count, const Ptr& actor);
             ///< Remove \a count item(s) designated by \a item from this inventory.