Fix replenisher storage notifications and byte accounting (#1621)

src/functionalTest/java/appeng/test/tile/misc/TileSuperMEReplenisherFunctionalTest.java

@@ -0,0 +1,213 @@
+package appeng.test.tile.misc;
+
+import static appeng.util.item.AEItemStackType.ITEM_STACK_TYPE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.List;
+
+import net.minecraft.init.Blocks;
+import net.minecraft.item.ItemStack;
+import net.minecraft.nbt.NBTTagCompound;
+import net.minecraftforge.fluids.Fluid;
+import net.minecraftforge.fluids.FluidRegistry;
+import net.minecraftforge.fluids.FluidStack;
+
+import org.junit.jupiter.api.Test;
+
+import appeng.api.AEApi;
+import appeng.api.config.Actionable;
+import appeng.api.networking.security.BaseActionSource;
+import appeng.api.networking.storage.IBaseMonitor;
+import appeng.api.storage.IMEMonitor;
+import appeng.api.storage.IMEMonitorHandlerReceiver;
+import appeng.api.storage.data.IAEFluidStack;
+import appeng.api.storage.data.IAEItemStack;
+import appeng.api.storage.data.IAEStack;
+import appeng.tile.misc.TileSuperMEReplenisher;
+import appeng.util.Platform;
+import appeng.util.item.AEFluidStack;
+import appeng.util.item.AEItemStack;
+import appeng.util.item.IAEStackList;
+
+public class TileSuperMEReplenisherFunctionalTest {
+
+    private static final Field STORAGE;
+    private static final Method INJECT_ITEMS;
+    private static final Method EXTRACT_ITEMS;
+
+    static {
+        try {
+            STORAGE = TileSuperMEReplenisher.class.getDeclaredField("storage");
+            INJECT_ITEMS = TileSuperMEReplenisher.class.getDeclaredMethod(
+                    "injectItems",
+                    IAEStack.class,
+                    Actionable.class,
+                    IAEStackList.class,
+                    BaseActionSource.class);
+            EXTRACT_ITEMS = TileSuperMEReplenisher.class.getDeclaredMethod(
+                    "extractItems",
+                    IAEStack.class,
+                    Actionable.class,
+                    IAEStackList.class,
+                    BaseActionSource.class);
+            STORAGE.setAccessible(true);
+            INJECT_ITEMS.setAccessible(true);
+            EXTRACT_ITEMS.setAccessible(true);
+        } catch (ReflectiveOperationException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
+
+    @Test
+    public void publishesVisibleChangesExactlyOnce() throws ReflectiveOperationException {
+        final TileSuperMEReplenisher tile = replenisherWithCells(1);
+        final IMEMonitor<IAEItemStack> monitor = itemMonitor(tile);
+        final RecordingListener listener = new RecordingListener();
+        final BaseActionSource source = new BaseActionSource();
+        final IAEItemStack cobblestone = itemStack(64);
+
+        monitor.addListener(listener, listener);
+        assertSame(monitor, tile.getMonitor(ITEM_STACK_TYPE));
+        assertEquals(0, storedAmount(monitor, cobblestone));
+
+        assertNull(inject(tile, cobblestone, Actionable.MODULATE, storage(tile), source));
+        listener.assertSingleChange(64, source);
+        assertEquals(64, storedAmount(monitor, cobblestone));
+
+        listener.clear();
+        assertNull(inject(tile, itemStack(8), Actionable.SIMULATE, storage(tile), source));
+        assertEquals(8, monitor.extractItems(itemStack(8), Actionable.SIMULATE, source).getStackSize());
+        assertEquals(0, listener.changes.size());
+        assertEquals(64, storedAmount(monitor, cobblestone));
+
+        assertNull(tile.injectItems(itemStack(8), Actionable.MODULATE, source));
+        assertEquals(0, listener.changes.size());
+        assertEquals(64, storedAmount(monitor, cobblestone));
+
+        assertEquals(16, monitor.extractItems(itemStack(16), Actionable.MODULATE, source).getStackSize());
+        listener.assertSingleChange(-16, source);
+        assertEquals(48, storedAmount(monitor, cobblestone));
+
+        listener.clear();
+        assertEquals(8, extract(tile, itemStack(8), Actionable.MODULATE, storage(tile), source).getStackSize());
+        listener.assertSingleChange(-8, source);
+        assertEquals(40, storedAmount(monitor, cobblestone));
+    }
+
+    @Test
+    public void recountsMultipleHugeStacksWithoutOverflow() {
+        final IAEStackList storage = new IAEStackList(true);
+        storage.add(fluidStack(FluidRegistry.WATER, 1L << 62));
+        storage.add(fluidStack(FluidRegistry.LAVA, 1L << 62));
+
+        final NBTTagCompound data = new NBTTagCompound();
+        data.setTag("storage", Platform.writeAEStackListNBT(storage));
+
+        final TileSuperMEReplenisher tile = new TileSuperMEReplenisher();
+        tile.readFromNBTEvent(data);
+
+        assertEquals(1L << 52, tile.getUsedBytes());
+    }
+
+    @Test
+    public void rejectsInsertionWhileOverCapacity() throws ReflectiveOperationException {
+        final TileSuperMEReplenisher tile = replenisherWithCells(2);
+        final IAEStackList storage = storage(tile);
+        final BaseActionSource source = new BaseActionSource();
+
+        assertNull(inject(tile, itemStack(16_384), Actionable.MODULATE, storage, source));
+        tile.getCellInventory().setInventorySlotContents(1, null);
+
+        assertEquals(1_024, tile.getTotalBytes());
+        assertEquals(2_048, tile.getUsedBytes());
+
+        final IAEItemStack simulated = itemStack(1);
+        assertSame(simulated, inject(tile, simulated, Actionable.SIMULATE, storage, source));
+        final IAEItemStack modulated = itemStack(1);
+        assertSame(modulated, inject(tile, modulated, Actionable.MODULATE, storage, source));
+        assertEquals(16_384, storage.findPrecise(itemStack(1)).getStackSize());
+        assertEquals(2_048, tile.getUsedBytes());
+    }
+
+    private static TileSuperMEReplenisher replenisherWithCells(final int count) {
+        final TileSuperMEReplenisher tile = new TileSuperMEReplenisher();
+        for (int i = 0; i < count; i++) {
+            tile.getCellInventory()
+                    .setInventorySlotContents(i, AEApi.instance().definitions().items().cell1k().maybeStack(1).get());
+        }
+        return tile;
+    }
+
+    @SuppressWarnings("unchecked")
+    private static IMEMonitor<IAEItemStack> itemMonitor(final TileSuperMEReplenisher tile) {
+        return (IMEMonitor<IAEItemStack>) tile.getMonitor(ITEM_STACK_TYPE);
+    }
+
+    private static IAEStackList storage(final TileSuperMEReplenisher tile) throws IllegalAccessException {
+        return (IAEStackList) STORAGE.get(tile);
+    }
+
+    private static IAEStack<?> inject(final TileSuperMEReplenisher tile, final IAEStack<?> stack, final Actionable mode,
+            final IAEStackList target, final BaseActionSource source) throws ReflectiveOperationException {
+        return (IAEStack<?>) INJECT_ITEMS.invoke(tile, stack, mode, target, source);
+    }
+
+    private static IAEStack<?> extract(final TileSuperMEReplenisher tile, final IAEStack<?> stack,
+            final Actionable mode, final IAEStackList target, final BaseActionSource source)
+            throws ReflectiveOperationException {
+        return (IAEStack<?>) EXTRACT_ITEMS.invoke(tile, stack, mode, target, source);
+    }
+
+    private static IAEItemStack itemStack(final long amount) {
+        return AEItemStack.create(new ItemStack(Blocks.cobblestone)).setStackSize(amount);
+    }
+
+    private static IAEFluidStack fluidStack(final Fluid fluid, final long amount) {
+        return AEFluidStack.create(new FluidStack(fluid, 1)).setStackSize(amount);
+    }
+
+    private static long storedAmount(final IMEMonitor<IAEItemStack> monitor, final IAEItemStack template) {
+        final IAEItemStack stored = monitor.getStorageList().findPrecise(template);
+        return stored == null ? 0 : stored.getStackSize();
+    }
+
+    private static final class RecordingListener implements IMEMonitorHandlerReceiver<IAEItemStack> {
+
+        private final List<Long> changes = new ArrayList<>();
+        private final List<BaseActionSource> sources = new ArrayList<>();
+
+        @Override
+        public boolean isValid(final Object verificationToken) {
+            return verificationToken == this;
+        }
+
+        @Override
+        public void postChange(final IBaseMonitor<IAEItemStack> monitor, final Iterable<IAEItemStack> changes,
+                final BaseActionSource source) {
+            for (final IAEItemStack change : changes) {
+                this.changes.add(change.getStackSize());
+                this.sources.add(source);
+            }
+        }
+
+        @Override
+        public void onListUpdate() {}
+
+        private void assertSingleChange(final long amount, final BaseActionSource source) {
+            assertEquals(1, this.changes.size());
+            assertEquals(amount, this.changes.get(0).longValue());
+            assertEquals(1, this.sources.size());
+            assertSame(source, this.sources.get(0));
+        }
+
+        private void clear() {
+            this.changes.clear();
+            this.sources.clear();
+        }
+    }
+}

src/main/java/appeng/me/storage/AEExternalHandler.java

@@ -45,7 +45,7 @@ public class AEExternalHandler implements IExternalStorageHandler {
 
         if (te instanceof ITileStorageMonitorable iface) {
             return iface.getMonitorable(d, src).getMEMonitor(type);
-        } else if (te instanceof TileSuperMEReplenisher tSMR) return new MEInventoryWrapper(tSMR, type);
+        } else if (te instanceof TileSuperMEReplenisher tSMR) return tSMR.getMonitor(type);
 
         return null;
     }

src/main/java/appeng/tile/misc/TileSuperMEReplenisher.java

@@ -1,6 +1,9 @@
 package appeng.tile.misc;
 
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Optional;
 import java.util.function.Predicate;
 
@@ -29,14 +32,17 @@ import appeng.api.networking.ticking.IGridTickable;
 import appeng.api.networking.ticking.TickRateModulation;
 import appeng.api.networking.ticking.TickingRequest;
 import appeng.api.storage.IMEInventory;
+import appeng.api.storage.IMEInventoryHandler;
 import appeng.api.storage.IMEMonitor;
+import appeng.api.storage.MEMonitorHandler;
 import appeng.api.storage.StorageName;
-import appeng.api.storage.data.AEStackTypeRegistry;
 import appeng.api.storage.data.IAEStack;
 import appeng.api.storage.data.IAEStackType;
 import appeng.api.storage.data.IItemList;
 import appeng.container.implementations.ContainerSuperMEReplenisher;
 import appeng.me.GridAccessException;
+import appeng.me.storage.MEInventoryHandler;
+import appeng.me.storage.MEInventoryWrapper;
 import appeng.tile.TileEvent;
 import appeng.tile.events.TileEventType;
 import appeng.tile.grid.AENetworkTile;
@@ -46,7 +52,6 @@ import appeng.tile.inventory.IIAEStackInventory;
 import appeng.util.Platform;
 import appeng.util.item.IAEStackList;
 import io.netty.buffer.ByteBuf;
-import it.unimi.dsi.fastutil.objects.Object2DoubleOpenHashMap;
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 
 public class TileSuperMEReplenisher extends AENetworkTile
@@ -65,6 +70,7 @@ public class TileSuperMEReplenisher extends AENetworkTile
     };
 
     private final Object2IntOpenHashMap<IAEStackType<?>> unusedCount = new Object2IntOpenHashMap<>();
+    private final Map<IAEStackType<?>, ReplenisherMonitor<?>> monitors = new HashMap<>();
 
     private long totalBytes = 0;
     private long usedBytes = 0;
@@ -262,7 +268,7 @@ public class TileSuperMEReplenisher extends AENetworkTile
     private void toRequest(final IAEStack<?> aes, final IStorageGrid storage) {
         final IMEMonitor monitor = storage.getMEMonitor(aes.getStackType());
         if (monitor == null) return;
-        final IAEStack<?> notAllowed = this.injectItems(aes, Actionable.SIMULATE, this.storage);
+        final IAEStack<?> notAllowed = this.injectItems(aes, Actionable.SIMULATE, this.storage, this.src);
 
         if (notAllowed != null) {
             final long requestSize = aes.getStackSize();
@@ -271,16 +277,16 @@ public class TileSuperMEReplenisher extends AENetworkTile
         }
 
         final IAEStack<?> extracted = monitor.extractItems(aes, Actionable.MODULATE, this.src);
-        this.injectItems(extracted, Actionable.MODULATE, this.storage);
+        this.injectItems(extracted, Actionable.MODULATE, this.storage, this.src);
     }
 
     private void toReturn(final IAEStack<?> aes, final IStorageGrid storage, final IAEStackList target) {
         final IMEMonitor monitor = storage.getMEMonitor(aes.getStackType());
         if (monitor == null) return;
-        final IAEStack<?> allowed = this.extractItems(aes, Actionable.MODULATE, target);
+        final IAEStack<?> allowed = this.extractItems(aes, Actionable.MODULATE, target, this.src);
         if (allowed == null) return;
         final IAEStack<?> notInjected = monitor.injectItems(allowed, Actionable.MODULATE, this.src);
-        this.injectItems(notInjected, Actionable.MODULATE, target);
+        this.injectItems(notInjected, Actionable.MODULATE, target, this.src);
     }
 
     private void refund(final IAEStackList fList) {
@@ -290,10 +296,10 @@ public class TileSuperMEReplenisher extends AENetworkTile
                 final IMEMonitor monitor = storage.getMEMonitor(listItem.getStackType());
                 if (monitor != null) {
                     final IAEStack<?> leftOver = monitor.injectItems(
-                            this.extractItems(listItem, Actionable.MODULATE, fList),
+                            this.extractItems(listItem, Actionable.MODULATE, fList, this.src),
                             Actionable.MODULATE,
                             this.src);
-                    if (leftOver != null) this.injectItems(leftOver, Actionable.MODULATE, fList);
+                    if (leftOver != null) this.injectItems(leftOver, Actionable.MODULATE, fList, this.src);
                 }
             });
         } catch (final GridAccessException ignored) {}
@@ -381,10 +387,11 @@ public class TileSuperMEReplenisher extends AENetworkTile
 
     @Override
     public IAEStack<?> injectItems(IAEStack<?> input, Actionable type, BaseActionSource src) {
-        return this.injectItems(input, type, this.out);
+        return this.injectItems(input, type, this.out, src);
     }
 
-    private IAEStack<?> injectItems(final IAEStack<?> input, final Actionable type, final IAEStackList target) {
+    private IAEStack<?> injectItems(final IAEStack<?> input, final Actionable type, final IAEStackList target,
+            final BaseActionSource source) {
         if (input == null) return null;
 
         final long freeBytes = this.totalBytes - this.usedBytes;
@@ -394,7 +401,7 @@ public class TileSuperMEReplenisher extends AENetworkTile
         final long stackSize = input.getStackSize();
         final int unusedCount = this.unusedCount.getOrDefault(stackType, 0);
 
-        if (freeBytes == 0 && unusedCount == 0) return input;
+        if (freeBytes < 0 || freeBytes == 0 && unusedCount == 0) return input;
 
         final int freeUnusedCount = unusedCount == 0 ? 0 : typeWeight - unusedCount;
 
@@ -402,8 +409,8 @@ public class TileSuperMEReplenisher extends AENetworkTile
         final int newUnusedCount;
         if (stackSize > freeUnusedCount) {
             final long toCountSize = stackSize - freeUnusedCount;
-            needBytes = (long) Math.ceil((double) toCountSize / typeWeight);
-            newUnusedCount = (int) toCountSize % typeWeight;
+            needBytes = bytesFor(toCountSize, typeWeight);
+            newUnusedCount = remainderInLastByte(toCountSize, typeWeight);
         } else {
             needBytes = 0;
             newUnusedCount = (int) ((unusedCount + stackSize) % typeWeight);
@@ -427,6 +434,7 @@ public class TileSuperMEReplenisher extends AENetworkTile
                 }
 
                 target.add(input);
+                if (target == this.storage) this.postStorageChange(input, input.getStackSize(), source);
                 return null;
             } else {
                 final IAEStack<?> notAllowed = input.copy();
@@ -442,6 +450,7 @@ public class TileSuperMEReplenisher extends AENetworkTile
                 }
 
                 target.add(allowed);
+                if (target == this.storage) this.postStorageChange(allowed, allowed.getStackSize(), source);
                 return notAllowed;
             }
         }
@@ -449,10 +458,11 @@ public class TileSuperMEReplenisher extends AENetworkTile
 
     @Override
     public IAEStack<?> extractItems(IAEStack<?> request, Actionable mode, BaseActionSource src) {
-        return this.extractItems(request, mode, this.storage);
+        return this.extractItems(request, mode, this.storage, src);
     }
 
-    private IAEStack<?> extractItems(final IAEStack<?> request, final Actionable mode, final IAEStackList target) {
+    private IAEStack<?> extractItems(final IAEStack<?> request, final Actionable mode, final IAEStackList target,
+            final BaseActionSource source) {
         if (request == null) return null;
 
         final IAEStackType<?> stackType = request.getStackType();
@@ -480,13 +490,16 @@ public class TileSuperMEReplenisher extends AENetworkTile
                 final int unusedCount = this.unusedCount.getOrDefault(stackType, 0);
 
                 final int freeInPartialByte = unusedCount == 0 ? 0 : typeWeight - unusedCount;
-                final long freedBytes = (freeInPartialByte + requestSize) / typeWeight;
-                final int newFreeInPartialByte = (int) ((freeInPartialByte + requestSize) % typeWeight);
+                final long partialByte = freeInPartialByte + requestSize % typeWeight;
+                final long freedBytes = requestSize / typeWeight + partialByte / typeWeight;
+                final int newFreeInPartialByte = (int) (partialByte % typeWeight);
                 final int newUnusedCount = newFreeInPartialByte == 0 ? 0 : typeWeight - newFreeInPartialByte;
 
                 this.usedBytes -= freedBytes;
                 this.unusedCount.put(stackType, newUnusedCount);
             }
+
+            if (target == this.storage) this.postStorageChange(ret, -requestSize, source);
         }
 
         return ret;
@@ -494,23 +507,70 @@ public class TileSuperMEReplenisher extends AENetworkTile
 
     private void countUsedBytes() {
         this.usedBytes = 0;
+        this.unusedCount.clear();
 
-        final Object2DoubleOpenHashMap<IAEStackType<?>> unusedCount = new Object2DoubleOpenHashMap<>();
-        this.storage.forEach(aes -> {
-            final IAEStackType<?> stackType = aes.getStackType();
-            unusedCount.put(stackType, unusedCount.getOrDefault(stackType, 0) + aes.getStackSize());
-        });
+        this.storage.forEach(this::countUsedBytes);
+        this.out.forEach(this::countUsedBytes);
 
-        this.out.forEach(aes -> {
-            final IAEStackType<?> stackType = aes.getStackType();
-            unusedCount.put(stackType, unusedCount.getOrDefault(stackType, 0) + aes.getStackSize());
-        });
+        for (final int remainder : this.unusedCount.values()) {
+            if (remainder != 0) this.usedBytes++;
+        }
+    }
 
-        for (IAEStackType<?> stackType : AEStackTypeRegistry.getAllTypes()) {
-            final int typeWeight = stackType.getAmountPerByte();
-            final double count = unusedCount.getOrDefault(stackType, 0);
-            this.unusedCount.put(stackType, (int) count % typeWeight);
-            this.usedBytes += (long) Math.ceil(count / typeWeight);
+    private void countUsedBytes(final IAEStack<?> stack) {
+        final IAEStackType<?> stackType = stack.getStackType();
+        final int typeWeight = stackType.getAmountPerByte();
+        final long count = stack.getStackSize();
+        final long remainder = this.unusedCount.getOrDefault(stackType, 0) + count % typeWeight;
+
+        this.usedBytes += count / typeWeight + remainder / typeWeight;
+        this.unusedCount.put(stackType, remainderInLastByte(remainder, typeWeight));
+    }
+
+    static long bytesFor(final long amount, final int amountPerByte) {
+        return amount / amountPerByte + (amount % amountPerByte == 0 ? 0 : 1);
+    }
+
+    static int remainderInLastByte(final long amount, final int amountPerByte) {
+        return (int) (amount % amountPerByte);
+    }
+
+    @SuppressWarnings({ "rawtypes", "unchecked" })
+    public IMEMonitor<?> getMonitor(final IAEStackType<?> type) {
+        return this.monitors.computeIfAbsent(
+                type,
+                stackType -> new ReplenisherMonitor(
+                        new MEInventoryHandler(new MEInventoryWrapper(this, stackType), stackType)));
+    }
+
+    private void postStorageChange(final IAEStack<?> stack, final long amount, final BaseActionSource source) {
+        final ReplenisherMonitor<?> monitor = this.monitors.get(stack.getStackType());
+        if (monitor == null) return;
+
+        final IAEStack<?> change = stack.copy();
+        change.setStackSize(amount);
+        monitor.postChange(change, source);
+    }
+
+    private static final class ReplenisherMonitor<T extends IAEStack<T>> extends MEMonitorHandler<T> {
+
+        private ReplenisherMonitor(final IMEInventoryHandler<T> handler) {
+            super(handler);
+        }
+
+        @Override
+        public T injectItems(final T input, final Actionable mode, final BaseActionSource source) {
+            // Bypass superclass notifications: the tile posts changes from its shared mutation paths.
+            return this.getHandler().injectItems(input, mode, source);
+        }
+
+        @Override
+        public T extractItems(final T request, final Actionable mode, final BaseActionSource source) {
+            return this.getHandler().extractItems(request, mode, source);
+        }
+
+        private void postChange(final IAEStack<?> change, final BaseActionSource source) {
+            this.postChangesToListeners(Collections.singletonList(change), source);
         }
     }
 

src/test/java/appeng/tile/misc/TileSuperMEReplenisherTest.java

@@ -0,0 +1,17 @@
+package appeng.tile.misc;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+public class TileSuperMEReplenisherTest {
+
+    @Test
+    public void countsLargeAmountsWithoutNarrowing() {
+        final long amount = (long) Integer.MAX_VALUE + 2;
+
+        assertEquals(1_048_577, TileSuperMEReplenisher.bytesFor(amount, 2_048));
+        assertEquals(1, TileSuperMEReplenisher.remainderInLastByte(amount, 2_048));
+        assertEquals(0, TileSuperMEReplenisher.bytesFor(0, 2_048));
+    }
+}