From 9a2002e2009878c0a62af3e80b5eb9409f422732 Mon Sep 17 00:00:00 2001
From: ephphatha <ephphatha@thelettereph.com>
Date: Tue, 8 Feb 2022 21:44:29 +1100
Subject: [PATCH] Get rid of RemoveEmptyLevelItems

This was a cleanup function to handle converting saves between Hellfire <-> Diablo, mainly when a hellfire specific item was dropped and should be removed from a Diablo game. This had an off by one with the way it iterated backwards over the active items list which meant it always called DeleteItem at least once (since it uses an invalid item past the end of the active items range). DeleteItem then always decremented the ActiveItems count. When saving the game after converting the items we then end up with a dropped items array containing an index to an item that is no longer considered active.

However the code in LoadDroppedItems already checks that the conversion was successful before considering it active, so this function doesn't need to be called. Instead we can just save if we converted game mode.
---
 Source/loadsave.cpp | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/Source/loadsave.cpp b/Source/loadsave.cpp
index e9a1164dd..45c62a54f 100644
--- a/Source/loadsave.cpp
+++ b/Source/loadsave.cpp
@@ -922,17 +922,6 @@ void LoadDroppedItems(LoadHelper &file, size_t savedItemCount)
 	}
 }
 
-void RemoveEmptyLevelItems()
-{
-	for (int i = ActiveItemCount; i > 0; i--) {
-		auto &item = Items[ActiveItems[i]];
-		if (item.isEmpty()) {
-			dItem[item.position.x][item.position.y] = 0;
-			DeleteItem(i);
-		}
-	}
-}
-
 void SaveItem(SaveHelper &file, const Item &item)
 {
 	auto idx = item.IDidx;
@@ -1981,7 +1970,6 @@ void LoadGame(bool firstflag)
 	gbProcessPlayers = true;
 
 	if (gbIsHellfireSaveGame != gbIsHellfire) {
-		RemoveEmptyLevelItems();
 		SaveGame();
 	}
 
@@ -2319,10 +2307,6 @@ void LoadLevel()
 		}
 	}
 
-	if (gbIsHellfireSaveGame != gbIsHellfire) {
-		RemoveEmptyLevelItems();
-	}
-
 	if (!gbSkipSync) {
 		AutomapZoomReset();
 		ResyncQuests();