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

@@ -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();