From 97ee4bc3496d104e5c4e49458ef9e4d4851e112c Mon Sep 17 00:00:00 2001
From: Andrei Kortunov <andrei.kortunov@yandex.ru>
Date: Sat, 14 Dec 2019 21:30:46 +0400
Subject: [PATCH] Improve equipment logic (bug #5223)

---
 CHANGELOG.md                           |  1 +
 apps/openmw/mwmechanics/actors.cpp     | 26 ++++++++++++++------------
 apps/openmw/mwrender/npcanimation.cpp  |  2 +-
 apps/openmw/mwworld/actionequip.cpp    |  2 +-
 apps/openmw/mwworld/inventorystore.cpp |  6 ++++--
 apps/openmw/mwworld/inventorystore.hpp |  2 +-
 6 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d55452e42..a8ee80f31 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/apps/openmw/mwmechanics/actors.cpp b/apps/openmw/mwmechanics/actors.cpp
index 7cd17d13e..35a48710d 100644
--- a/apps/openmw/mwmechanics/actors.cpp
+++ b/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)
diff --git a/apps/openmw/mwrender/npcanimation.cpp b/apps/openmw/mwrender/npcanimation.cpp
index 063c1a60f..e9f62e25c 100644
--- a/apps/openmw/mwrender/npcanimation.cpp
+++ b/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);
diff --git a/apps/openmw/mwworld/actionequip.cpp b/apps/openmw/mwworld/actionequip.cpp
index 30f969193..4cb0dbe51 100644
--- a/apps/openmw/mwworld/actionequip.cpp
+++ b/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
diff --git a/apps/openmw/mwworld/inventorystore.cpp b/apps/openmw/mwworld/inventorystore.cpp
index 04947335f..094d1c8ec 100644
--- a/apps/openmw/mwworld/inventorystore.cpp
+++ b/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;
diff --git a/apps/openmw/mwworld/inventorystore.hpp b/apps/openmw/mwworld/inventorystore.hpp
index 2293f96af..663e7a2d3 100644
--- a/apps/openmw/mwworld/inventorystore.hpp
+++ b/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