Fix regression caused by Foe item queues

Foe item queues broke certain items being removed correctly by other quest actions. This regression was introduced by commit 5c30873
Problem is caused by item cloning behaviour of queues desyncing prototype item UID from cloned item UID. When ReleaseQuestItemForReoffer() was called by various quest actions, it was unable to find cloned items because it was searching on prototype item UID only.
Refactored ReleaseQuestItemForReoffer() to search on quest UID and item Symbol instead. This is more stable than item UID and will not break by cloning.
This fixes item removal behaviour for several quest actions broken by Foe item queues:
-GetItem
-GivePc
-TakeItem
-TotingItemAndClickedNpc
These actions will no longer leave behind orphaned items if previously cloned via Foe item queue. Process remains compatible with prototype items added directly to an item collection without first being cloned by Foe item queue.
Removed CleanupPlayerQuestItems() mitigation in Quest as it's no longer required now these quest actions have correct behaviour again.
Global orphaned item cleanup remains at load time.

Assets/Scripts/Game/Entities/PlayerEntity.cs

@@ -1484,26 +1484,32 @@ void HandleStartingCrimeGuildQuests()
         /// Releases a quest item carried by player so it can be assigned back again by quest script.
         /// This ensures item is properly unequipped and optionally makes permanent.
         /// </summary>
-        /// <param name="item">Item to release.</param>
+        /// <param name="questUID">Quest UID owning Item.</param>
+        /// <param name="item">Quest Item to release.</param>
         /// <param name="makePermanent">True to make item permanent.</param>
-        public void ReleaseQuestItemForReoffer(DaggerfallUnityItem item, bool makePermanent = false)
+        public void ReleaseQuestItemForReoffer(ulong questUID, Item item, bool makePermanent = false)
         {
-            if (item == null)
+            if (item == null || item.Symbol == null)
                 return;
 
-            // Unequip item if player is wearing it
-            if (GameManager.Instance.PlayerEntity.ItemEquipTable.UnequipItem(item))
+            // Get all player held quest items matching this quest and item symbol
+            DaggerfallUnityItem[] items = GameManager.Instance.PlayerEntity.Items.ExportQuestItems(questUID, item.Symbol);
+            foreach (DaggerfallUnityItem dfitem in items)
             {
-                // If item was actually unequipped then update armour values
-                GameManager.Instance.PlayerEntity.UpdateEquippedArmorValues(item, false);
-            }
+                // Unequip item if player is wearing it
+                if (GameManager.Instance.PlayerEntity.ItemEquipTable.UnequipItem(dfitem))
+                {
+                    // If item was actually unequipped then update armour values
+                    GameManager.Instance.PlayerEntity.UpdateEquippedArmorValues(dfitem, false);
+                }
 
-            // Remove quest from inventory so it can be offered back to player
-            GameManager.Instance.PlayerEntity.Items.RemoveItem(item);
+                // Remove quest from inventory so it can be offered back to player
+                GameManager.Instance.PlayerEntity.Items.RemoveItem(dfitem);
 
-            // Optionally make permanent
-            if (makePermanent)
-                item.MakePermanent();
+                // Optionally make permanent
+                if (makePermanent)
+                    dfitem.MakePermanent();
+            }
         }
 
         public void ClearReactionMods()

Assets/Scripts/Game/Items/ItemCollection.cs

@@ -510,16 +510,20 @@ public DaggerfallUnityItem[] Export()
         }
 
         /// <summary>
-        /// Gets all quest items for a specific quest.
+        /// Gets all quest items for a specific quest and item Symbol.
         /// Ignores ex-quest items that have been made permanent.
         /// </summary>
         /// <param name="questUID">Quest UID for item search.</param>
-        public DaggerfallUnityItem[] ExportQuestItems(ulong questUID)
+        /// <param name="itemSymbol">Item Symbol for item search.</param>
+        public DaggerfallUnityItem[] ExportQuestItems(ulong questUID, Symbol itemSymbol)
         {
+            if (itemSymbol == null)
+                return null;
+
             List<DaggerfallUnityItem> results = new List<DaggerfallUnityItem>();
             foreach (DaggerfallUnityItem item in items.Values)
             {
-                if (item.IsQuestItem && item.QuestUID == questUID)
+                if (item.IsQuestItem && item.QuestUID == questUID && itemSymbol.Equals(item.QuestItemSymbol))
                     results.Add(item);
             }
 

Assets/Scripts/Game/Questing/Actions/GetItem.cs

@@ -67,7 +67,7 @@ public override void Update(Task caller)
             // Release item so we can give back to player
             // Sometimes a quest item is both carried by player then handed back to them
             // Example is Sx012 where courier hands back two items of jewellery
-            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(item.DaggerfallUnityItem);
+            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(ParentQuest.UID, item);
 
             // Give quest item to player
             if (item.DaggerfallUnityItem.IsOfTemplate(ItemGroups.Currency, (int)Currency.Gold_pieces))

Assets/Scripts/Game/Questing/Actions/GivePc.cs

@@ -161,7 +161,7 @@ void OfferToPlayerWithQuestComplete(Item item)
             // Release item so we can offer back to player
             // Sometimes a quest item is both carried by player then offered back to them
             // Example is Sx010 where "curse" is removed and player can keep item
-            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(item.DaggerfallUnityItem, true);
+            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(ParentQuest.UID, item, true);
 
             // Create a dropped loot container window for player to loot their reward
             rewardLoot = GameObjectHelper.CreateDroppedLootContainer(GameManager.Instance.PlayerObject, DaggerfallUnity.NextUID);

Assets/Scripts/Game/Questing/Actions/TakeItem.cs

@@ -1,4 +1,4 @@
-// Project:         Daggerfall Tools For Unity
+// Project:         Daggerfall Tools For Unity
 // Copyright:       Copyright (C) 2009-2020 Daggerfall Workshop
 // Web Site:        http://www.dfworkshop.net
 // License:         MIT License (http://www.opensource.org/licenses/mit-license.php)
@@ -63,7 +63,7 @@ public override void Update(Task caller)
 
             // Release item - this will clear equip state and remove item from player's inventory
             // Now item is not associated with any collections and will just be garbage collected
-            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(item.DaggerfallUnityItem);
+            GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(ParentQuest.UID, item);
 
             // "saying" popup
             if (textId != 0)

Assets/Scripts/Game/Questing/Actions/TotingItemAndClickedNpc.cs

@@ -91,7 +91,7 @@ public override bool CheckTrigger(Task caller)
 
                     // Show message popup, remove item, return true on trigger
                     ParentQuest.ShowMessagePopup(id);
-                    GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(item.DaggerfallUnityItem);
+                    GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(ParentQuest.UID, item);
                     return true;
                 }
             }

Assets/Scripts/Game/Questing/Quest.cs

@@ -285,10 +285,7 @@ public void Update()
             if (ticksToEnd > 0)
             {
                 if (--ticksToEnd == 0)
-                {
                     questComplete = true;
-                    CleanupPlayerQuestItems();
-                }
             }
 
             // Tick resources
@@ -524,30 +521,6 @@ void DropAllQuestors()
             }
         }
 
-        /// <summary>
-        /// Releases all orphaned non-permanent quest items from player inventory when quest is ended.
-        /// Item will no longer belong to any collection and will be garbage collected.
-        /// Only call this method once quest has fully completed and entered tombstone state.
-        /// </summary>
-        void CleanupPlayerQuestItems()
-        {
-            // Quest must be completed
-            if (!questComplete)
-                throw new Exception("CleanupPlayerQuestItems() called prior to quest completion.");
-
-            // Get all non-permanent quest items in player inventory linked to this quest UID
-            DaggerfallUnityItem[] items = GameManager.Instance.PlayerEntity.Items.ExportQuestItems(UID);
-            if (items == null || items.Length == 0)
-                return;
-
-            // Release these items from collection
-            // Using ReleaseQuestItemForReoffer() to ensure item is correctly unequipped before release as player may have equipped quest item
-            foreach (DaggerfallUnityItem item in items)
-            {
-                GameManager.Instance.PlayerEntity.ReleaseQuestItemForReoffer(item);
-            }
-        }
-
         #endregion
 
         #region Log Message Methods