Little TakeInventory refactoring.

Introduce AActor::TakeInventory, which unifies DoTakeInv from ACS and DoTakeInventory from Decorate, and AInventory::DepleteOrDestroy, which is extracted from the DoTakeInv core function, and use both where they're needed.
I don't know if the differences between DoTakeInv and DoTakeInventory were intentional, so I kept both behaviors.

src/actor.h

@@ -677,6 +677,11 @@ public:
 	// Removes the item from the inventory list.
 	virtual void RemoveInventory (AInventory *item);
 
+	// Take the amount value of an item from the inventory list.
+	// If nothing is left, the item may be destroyed.
+	// Returns true if the initial item count is positive.
+	virtual bool TakeInventory (const PClass *itemclass, int amount, bool fromdecorate = false, bool notakeinfinite = false);
+
 	// Uses an item and removes it from the inventory.
 	virtual bool UseInventory (AInventory *item);
 

src/g_shared/a_morph.cpp

@@ -137,14 +137,9 @@ bool P_MorphPlayer (player_t *activator, player_t *p, const PClass *spawntype, i
 				hxarmor->Slots[3] = 0;
 				hxarmor->Slots[4] = spawntype->Meta.GetMetaFixed (APMETA_Hexenarmor0);
 			}
-			else if (item->ItemFlags & IF_KEEPDEPLETED)
-			{
-				// Set depletable armor to 0 (this includes BasicArmor).
-				item->Amount = 0;
-			}
 			else
 			{
-				item->Destroy ();
+				item->DepleteOrDestroy();
 			}
 		}
 		item = next;

src/g_shared/a_pickups.cpp

@@ -1131,6 +1131,32 @@ void AInventory::Destroy ()
 	if (SendItemDrop == this) SendItemDrop = NULL;
 }
 
+//===========================================================================
+//
+// AInventory :: DepleteOrDestroy
+//
+// If the item is depleted, just change its amount to 0, otherwise it's destroyed.
+//
+//===========================================================================
+
+void AInventory::DepleteOrDestroy ()
+{
+	// If it's not ammo or an internal armor, destroy it.
+	// Ammo needs to stick around, even when it's zero for the benefit
+	// of the weapons that use it and to maintain the maximum ammo
+	// amounts a backpack might have given.
+	// Armor shouldn't be removed because they only work properly when
+	// they are the last items in the inventory.
+	if (ItemFlags & IF_KEEPDEPLETED)
+	{
+		Amount = 0;
+	}
+	else
+	{
+		Destroy();
+	}
+}
+
 //===========================================================================
 //
 // AInventory :: GetBlend

src/g_shared/a_pickups.h

@@ -151,6 +151,7 @@ public:
 	virtual void MarkPrecacheSounds() const;
 	virtual void BeginPlay ();
 	virtual void Destroy ();
+	virtual void DepleteOrDestroy ();
 	virtual void Tick ();
 	virtual bool ShouldRespawn ();
 	virtual bool ShouldStay ();

src/m_cheat.cpp

@@ -1052,24 +1052,7 @@ void cht_Take (player_t *player, const char *name, int amount)
 	}
 	else
 	{
-		AInventory *inventory = player->mo->FindInventory (type);
+		player->mo->TakeInventory(type, amount ? amount : 1);
-
-		if (inventory != NULL)
-		{
-			inventory->Amount -= amount ? amount : 1;
-
-			if (inventory->Amount <= 0)
-			{
-				if (inventory->ItemFlags & IF_KEEPDEPLETED)
-				{
-					inventory->Amount = 0;
-				}
-				else
-				{
-					inventory->Destroy ();
-				}
-			}
-		}
 	}
 	return;
 }

src/p_acs.cpp

@@ -1131,40 +1131,6 @@ static void GiveInventory (AActor *activator, const char *type, int amount)
 	}
 }
 
-//============================================================================
-//
-// DoTakeInv
-//
-// Takes an item from a single actor.
-//
-//============================================================================
-
-static void DoTakeInv (AActor *actor, const PClass *info, int amount)
-{
-	AInventory *item = actor->FindInventory (info);
-	if (item != NULL)
-	{
-		item->Amount -= amount;
-		if (item->Amount <= 0)
-		{
-			// If it's not ammo or an internal armor, destroy it.
-			// Ammo needs to stick around, even when it's zero for the benefit
-			// of the weapons that use it and to maintain the maximum ammo
-			// amounts a backpack might have given.
-			// Armor shouldn't be removed because they only work properly when
-			// they are the last items in the inventory.
-			if (item->ItemFlags & IF_KEEPDEPLETED)
-			{
-				item->Amount = 0;
-			}
-			else
-			{
-				item->Destroy ();
-			}
-		}
-	}
-}
-
 //============================================================================
 //
 // TakeInventory
@@ -1199,12 +1165,12 @@ static void TakeInventory (AActor *activator, const char *type, int amount)
 		for (int i = 0; i < MAXPLAYERS; ++i)
 		{
 			if (playeringame[i])
-				DoTakeInv (players[i].mo, info, amount);
+				players[i].mo->TakeInventory(info, amount);
 		}
 	}
 	else
 	{
-		DoTakeInv (activator, info, amount);
+		activator->TakeInventory(info, amount);
 	}
 }
 

src/p_conversation.cpp

@@ -649,22 +649,7 @@ static void TakeStrifeItem (player_t *player, const PClass *itemtype, int amount
 	if (itemtype == RUNTIME_CLASS(ASigil))
 		return;
 
-	AInventory *item = player->mo->FindInventory (itemtype);
+	player->mo->TakeInventory(itemtype, amount);
-	if (item != NULL)
-	{
-		item->Amount -= amount;
-		if (item->Amount <= 0)
-		{
-			if (item->ItemFlags & IF_KEEPDEPLETED)
-			{
-				item->Amount = 0;
-			}
-			else
-			{
-				item->Destroy ();
-			}
-		}
-	}
 }
 
 CUSTOM_CVAR(Float, dlg_musicvolume, 1.0f, CVAR_ARCHIVE)

src/p_interaction.cpp

@@ -804,10 +804,7 @@ static int UseHealthItems(TArray<AInventory *> &Items, int &saveHealth)
 			saveHealth -= maxhealth;
 			if (--Items[index]->Amount == 0)
 			{
-				if (!(Items[index]->ItemFlags & IF_KEEPDEPLETED))
+				Items[index]->DepleteOrDestroy ();
-				{
-					Items[index]->Destroy ();
-				}
 				Items.Delete(index);
 				break;
 			}

src/p_mobj.cpp

@@ -591,6 +591,57 @@ void AActor::RemoveInventory(AInventory *item)
 	}
 }
 
+//============================================================================
+//
+// AActor :: TakeInventory
+//
+//============================================================================
+
+bool AActor::TakeInventory(const PClass *itemclass, int amount, bool fromdecorate, bool notakeinfinite)
+{
+	AInventory *item = FindInventory(itemclass);
+
+	if (item == NULL)
+		return false;
+
+	if (!fromdecorate)
+	{
+		item->Amount -= amount;
+		if (item->Amount <= 0)
+		{
+			item->DepleteOrDestroy();
+		}
+		// It won't be used in non-decorate context, so return false here
+		return false;
+	}
+
+	bool result = false;
+	if (item->Amount > 0)
+	{
+		result = true;
+	}
+
+	if (item->IsKindOf(RUNTIME_CLASS(AHexenArmor)))
+		return false;
+
+	// Do not take ammo if the "no take infinite/take as ammo depletion" flag is set
+	// and infinite ammo is on
+	if (notakeinfinite &&
+	((dmflags & DF_INFINITE_AMMO) || (player && player->cheats & CF_INFINITEAMMO)) &&
+		item->IsKindOf(RUNTIME_CLASS(AAmmo)))
+	{
+		// Nothing to do here, except maybe res = false;? Would it make sense?
+	}
+	else if (!amount || amount>=item->Amount)
+	{
+		item->DepleteOrDestroy();
+	}
+	else item->Amount-=amount;
+
+	return result;
+}
+
+
 //============================================================================
 //
 // AActor :: DestroyAllInventory
@@ -658,9 +709,9 @@ bool AActor::UseInventory (AInventory *item)
 	if (dmflags2 & DF2_INFINITE_INVENTORY)
 		return true;
 
-	if (--item->Amount <= 0 && !(item->ItemFlags & IF_KEEPDEPLETED))
+	if (--item->Amount <= 0)
 	{
-		item->Destroy ();
+		item->DepleteOrDestroy ();
 	}
 	return true;
 }

src/thingdef/thingdef_codeptr.cpp

@@ -1783,31 +1783,7 @@ void DoTakeInventory(AActor * receiver, bool use_aaptr, DECLARE_PARAMINFO)
 		COPY_AAPTR_NOT_NULL(receiver, receiver, setreceiver);
 	}
 
-	bool res = false;
+	bool res = receiver->TakeInventory(item, amount, true, (flags & TIF_NOTAKEINFINITE) != 0);
-
-	AInventory * inv = receiver->FindInventory(item);
-
-	if (inv && !inv->IsKindOf(RUNTIME_CLASS(AHexenArmor)))
-	{
-		if (inv->Amount > 0)
-		{
-			res = true;
-		}
-		// Do not take ammo if the "no take infinite/take as ammo depletion" flag is set
-		// and infinite ammo is on
-		if (flags & TIF_NOTAKEINFINITE &&
-			((dmflags & DF_INFINITE_AMMO) || (receiver->player->cheats & CF_INFINITEAMMO)) &&
-			inv->IsKindOf(RUNTIME_CLASS(AAmmo)))
-		{
-			// Nothing to do here, except maybe res = false;? Would it make sense?
-		}
-		else if (!amount || amount>=inv->Amount) 
-		{
-			if (inv->ItemFlags&IF_KEEPDEPLETED) inv->Amount=0;
-			else inv->Destroy();
-		}
-		else inv->Amount-=amount;
-	}
 	ACTION_SET_RESULT(res);
 }