From ac3b805dd9f2ad5689aa62da43d446455f9ba7f5 Mon Sep 17 00:00:00 2001
From: Randy Heit <rheit@zdoom.fake>
Date: Fri, 9 Mar 2012 01:19:02 +0000
Subject: [PATCH] - Fixed: ACS's ClearInventory was not equipped to handle
 items that destroy other items when   destroyed. For example, if a weapon has
 a sister weapon immediately after it in the inventory   list, they would both
 be destroyed by the call to destroy the weapon, and ClearInventory   would
 fail to find anything beyond the sister weapon in the inventory list.

SVN r3413 (trunk)
---
 src/p_acs.cpp | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/p_acs.cpp b/src/p_acs.cpp
index 9fee64925..d8e45ba1a 100644
--- a/src/p_acs.cpp
+++ b/src/p_acs.cpp
@@ -406,28 +406,54 @@ void P_WriteACSVars(FILE *stdfile)
 
 static void DoClearInv (AActor *actor)
 {
-	AInventory *inv = actor->Inventory;
+	// In case destroying an inventory item causes another to be destroyed
+	// (e.g. Weapons destroy their sisters), keep track of the pointer to
+	// the next inventory item rather than the next inventory item itself.
+	// For example, if a weapon is immediately followed by its sister, the
+	// next weapon we had tracked would be to the sister, so it is now
+	// invalid and we won't be able to find the complete inventory by
+	// following it.
+	//
+	// When we destroy an item, we leave invp alone, since the destruction
+	// process will leave it pointing to the next item we want to check. If
+	// we don't destroy an item, then we move invp to point to its Inventory
+	// pointer.
+	//
+	// It should be safe to assume that an item being destroyed will only
+	// destroy items further down in the chain, because if it was going to
+	// destroy something we already processed, we've already destroyed it,
+	// so it won't have anything to destroy.
 
-	while (inv != NULL)
+	AInventory **invp = &actor->Inventory;
+
+	while (*invp != NULL)
 	{
-		AInventory *next = inv->Inventory;
+		AInventory *inv = *invp;
 		if (!(inv->ItemFlags & IF_UNDROPPABLE))
 		{
-			// Fix for undroppable weapons. Is this ok?
+			// For the sake of undroppable weapons, never remove ammo once
+			// it has been acquired; just set its amount to 0.
 			if (inv->IsKindOf(RUNTIME_CLASS(AAmmo)))
 			{
 				AAmmo *ammo = static_cast<AAmmo*>(inv);
 				ammo->Amount = 0;
+				invp = &inv->Inventory;
 			}
 			else
+			{
 				inv->Destroy ();
+			}
 		}
 		else if (inv->GetClass() == RUNTIME_CLASS(AHexenArmor))
 		{
 			AHexenArmor *harmor = static_cast<AHexenArmor *> (inv);
 			harmor->Slots[3] = harmor->Slots[2] = harmor->Slots[1] = harmor->Slots[0] = 0;
+			invp = &inv->Inventory;
+		}
+		else
+		{
+			invp = &inv->Inventory;
 		}
-		inv = next;
 	}
 	if (actor->player != NULL)
 	{