From 2216c33f12ef17af0b30a1945f5474e2a2d2970a Mon Sep 17 00:00:00 2001
From: Sebastian Hartte <sebastian@hartte.de>
Date: Sat, 10 Oct 2020 20:13:36 +0200
Subject: [PATCH] Be less defensive about copying item stacks in simulated
 extractions (but still catch obviously broken implementations). Also fix
 looping over the same slot while simulating, and try to heuristically guess
 the maximum extraction from the slot given what getStackInSlot returned.

---
 .../appeng/parts/misc/ItemHandlerAdapter.java | 28 +++++++++++++++----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/main/java/appeng/parts/misc/ItemHandlerAdapter.java b/src/main/java/appeng/parts/misc/ItemHandlerAdapter.java
index fbc9f7abf..23a698873 100644
--- a/src/main/java/appeng/parts/misc/ItemHandlerAdapter.java
+++ b/src/main/java/appeng/parts/misc/ItemHandlerAdapter.java
@@ -122,6 +122,15 @@ class ItemHandlerAdapter implements IMEInventory<IAEItemStack>, IBaseMonitor<IAE
             do {
                 extracted = this.itemHandler.extractItem(i, remainingCurrentSlot, simulate);
                 if (!extracted.isEmpty()) {
+                    // In order to guard against broken IItemHandler implementations, we'll
+                    // try to guess if the returned stack (especially in simulate mode) is
+                    // the same that was returned by getStackInSlot. This is obviously not a
+                    // precise science, but it would catch the previous Forge bug:
+                    // https://github.com/MinecraftForge/MinecraftForge/pull/6580
+                    if (extracted == stackInInventorySlot) {
+                        extracted = extracted.copy();
+                    }
+
                     if (extracted.getCount() > remainingCurrentSlot) {
                         // Something broke. It should never return more than we requested...
                         // We're going to silently eat the remainder
@@ -131,19 +140,28 @@ class ItemHandlerAdapter implements IMEInventory<IAEItemStack>, IBaseMonitor<IAE
                         extracted.setCount(remainingCurrentSlot);
                     }
 
+                    // Heuristic for simulation: looping in case of simulations is pointless, since
+                    // the
+                    // state of the underlying inventory does not change after a simulated
+                    // extraction
+                    // To still support inventories that report stacks that are larger than
+                    // maxStackSize,
+                    // we use this heuristic
+                    if (simulate && extracted.getCount() == extracted.getMaxStackSize()
+                            && remainingCurrentSlot > extracted.getMaxStackSize()) {
+                        extracted.setCount(remainingCurrentSlot);
+                    }
+
                     // We're just gonna use the first stack we get our hands on as the template for
                     // the rest.
-                    // In case some stupid itemhandler (aka forge) returns an internal state we have
-                    // to do a second
-                    // expensive copy again.
                     if (gathered.isEmpty()) {
-                        gathered = extracted.copy();
+                        gathered = extracted;
                     } else {
                         gathered.grow(extracted.getCount());
                     }
                     remainingCurrentSlot -= extracted.getCount();
                 }
-            } while (!extracted.isEmpty() && remainingCurrentSlot > 0);
+            } while (!simulate && !extracted.isEmpty() && remainingCurrentSlot > 0);
 
             remainingSize -= stackSizeCurrentSlot - remainingCurrentSlot;