Improve equipment logic (bug #5223)

CHANGELOG.md

@@ -182,6 +182,7 @@
     Bug #5218: Crash when disabling ToggleBorders
     Bug #5220: GetLOS crashes when actor isn't loaded
     Bug #5222: Empty cell name subrecords are not saved
+    Bug #5223: Bow replacement during attack animation removes attached arrow
     Bug #5226: Reputation should be capped
     Feature #1774: Handle AvoidNode
     Feature #2229: Improve pathfinding AI

apps/openmw/mwmechanics/actors.cpp

@@ -299,10 +299,10 @@ namespace MWMechanics
 
         bool wasEquipped = currentItem != store.end() && Misc::StringUtils::ciEqual(currentItem->getCellRef().getRefId(), itemId);
 
-        store.remove(itemId, 1, actor);
-
         if (actor != MWMechanics::getPlayer())
         {
+            store.remove(itemId, 1, actor);
+
             // Equip a replacement
             if (!wasEquipped)
                 return;
@@ -329,17 +329,19 @@ namespace MWMechanics
         std::string prevItemId = player.getPreviousItem(itemId);
         player.erasePreviousItem(itemId);
 
-        if (prevItemId.empty())
-            return;
-
-        // Find previous item (or its replacement) by id.
-        // we should equip previous item only if expired bound item was equipped.
-        MWWorld::Ptr item = store.findReplacement(prevItemId);
-        if (item.isEmpty() || !wasEquipped)
-            return;
+        if (!prevItemId.empty())
+        {
+            // Find previous item (or its replacement) by id.
+            // we should equip previous item only if expired bound item was equipped.
+            MWWorld::Ptr item = store.findReplacement(prevItemId);
+            if (!item.isEmpty() && wasEquipped)
+            {
+                MWWorld::ActionEquip action(item);
+                action.execute(actor);
+            }
+        }
 
-        MWWorld::ActionEquip action(item);
-        action.execute(actor);
+        store.remove(itemId, 1, actor);
     }
 
     void Actors::updateActor (const MWWorld::Ptr& ptr, float duration)

apps/openmw/mwrender/npcanimation.cpp

@@ -598,7 +598,7 @@ void NpcAnimation::updateParts()
     };
     static const size_t slotlistsize = sizeof(slotlist)/sizeof(slotlist[0]);
 
-    bool wasArrowAttached = (mAmmunition.get() != nullptr);
+    bool wasArrowAttached = isArrowAttached();
     mAmmunition.reset();
 
     const MWWorld::InventoryStore& inv = mPtr.getClass().getInventoryStore(mPtr);

apps/openmw/mwworld/actionequip.cpp

@@ -93,7 +93,7 @@ namespace MWWorld
         {
             for (slot=slots_.first.begin();slot!=slots_.first.end(); ++slot)
             {
-                invStore.unequipSlot(*slot, actor);
+                invStore.unequipSlot(*slot, actor, false);
                 if (slot+1 != slots_.first.end())
                     invStore.equip(*slot, invStore.getSlot(*(slot+1)), actor);
                 else

apps/openmw/mwworld/inventorystore.cpp

@@ -779,7 +779,7 @@ int MWWorld::InventoryStore::remove(const Ptr& item, int count, const Ptr& actor
     return retCount;
 }
 
-MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipSlot(int slot, const MWWorld::Ptr& actor)
+MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipSlot(int slot, const MWWorld::Ptr& actor, bool fireEvent)
 {
     if (slot<0 || slot>=static_cast<int> (mSlots.size()))
         throw std::runtime_error ("slot number out of range");
@@ -811,7 +811,9 @@ MWWorld::ContainerStoreIterator MWWorld::InventoryStore::unequipSlot(int slot, c
             }
         }
 
-        fireEquipmentChangedEvent(actor);
+        if (fireEvent)
+            fireEquipmentChangedEvent(actor);
+
         updateMagicEffects(actor);
 
         return retval;

apps/openmw/mwworld/inventorystore.hpp

@@ -173,7 +173,7 @@ namespace MWWorld
             ///
             /// @return the number of items actually removed
 
-            ContainerStoreIterator unequipSlot(int slot, const Ptr& actor);
+            ContainerStoreIterator unequipSlot(int slot, const Ptr& actor, bool fireEvent=true);
             ///< Unequip \a slot.
             ///
             /// @return an iterator to the item that was previously in the slot