more work on list and tests

Author: ghzdude

src/main/java/gregtech/api/capability/impl/ItemHandlerList.java

@@ -1,7 +1,5 @@
 package gregtech.api.capability.impl;
 
-import it.unimi.dsi.fastutil.objects.Object2IntMap;
-
 import net.minecraft.item.ItemStack;
 import net.minecraftforge.items.IItemHandler;
 import net.minecraftforge.items.IItemHandlerModifiable;
@@ -10,6 +8,7 @@
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
 import it.unimi.dsi.fastutil.objects.Object2IntArrayMap;
 import it.unimi.dsi.fastutil.objects.Object2IntMap;
+import it.unimi.dsi.fastutil.objects.ObjectIterator;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.*;
@@ -22,19 +21,24 @@ public class ItemHandlerList extends AbstractList<IItemHandler> implements IItem
     private final Int2ObjectMap<IItemHandler> handlerBySlotIndex = new Int2ObjectOpenHashMap<>();
     private final Object2IntMap<IItemHandler> baseIndexOffset = new Object2IntArrayMap<>();
 
-    private final List<IItemHandler> handlerList = new ArrayList<>();
+    public ItemHandlerList() {}
 
-    public ItemHandlerList(List<? extends IItemHandler> itemHandlerList) {
+    public ItemHandlerList(Collection<? extends IItemHandler> itemHandlerList) {
         addAll(itemHandlerList);
     }
 
+    public ItemHandlerList(ItemHandlerList parent, IItemHandler... additional) {
+        addAll(parent.getBackingHandlers());
+        Collections.addAll(this, additional);
+    }
+
     public int getIndexOffset(IItemHandler handler) {
         return baseIndexOffset.getOrDefault(handler, -1);
     }
 
     @Override
     public int getSlots() {
-        return size();
+        return handlerBySlotIndex.size();
     }
 
     @Override
@@ -81,14 +85,19 @@ public ItemStack extractItem(int slot, int amount, boolean simulate) {
         return itemHandler.extractItem(slot - baseIndexOffset.getInt(itemHandler), amount, simulate);
     }
 
+    private boolean invalidSlot(int slot) {
+        if (handlerBySlotIndex.isEmpty()) return false;
+        return slot < 0 || slot >= handlerBySlotIndex.size();
+    }
+
     @NotNull
     public Collection<IItemHandler> getBackingHandlers() {
-        return Collections.unmodifiableCollection(handlerList);
+        return Collections.unmodifiableCollection(baseIndexOffset.keySet());
     }
 
     @Override
     public int size() {
-        return handlerList.size();
+        return baseIndexOffset.size();
     }
 
     @Override
@@ -99,12 +108,18 @@ public boolean add(IItemHandler handler) {
     }
 
     @Override
-    public void add(int index, IItemHandler element) {
-//        Objects.checkIndex(index, size());
-        if (handlerList.contains(element)) {
+    public void add(int unused, IItemHandler element) {
+        Objects.requireNonNull(element);
+        if (baseIndexOffset.containsKey(element)) {
             throw new IllegalArgumentException("Attempted to add item handler " + element + " twice");
         }
-        handlerList.add(index, element);
+        if (element instanceof ItemHandlerList list) {
+            // possible infinite recursion
+            // throw instead?
+            addAll(list);
+            return;
+        }
+
         int offset = handlerBySlotIndex.size();
         baseIndexOffset.put(element, offset);
         for (int slotIndex = 0; slotIndex < element.getSlots(); slotIndex++) {
@@ -114,33 +129,48 @@ public void add(int index, IItemHandler element) {
 
     @Override
     public IItemHandler get(int index) {
-        return handlerList.get(index);
+        if (invalidIndex(index)) throw new IndexOutOfBoundsException();
+        ObjectIterator<IItemHandler> itr = baseIndexOffset.keySet().iterator();
+        itr.skip(index); // skip n-1 elements
+        return itr.next(); // get nth element
     }
 
     @Override
     public IItemHandler remove(int index) {
-//        Objects.checkIndex(index, size());
-        var handler2 = get(index);
-        int offset2 = baseIndexOffset.getInt(handler2);
-
-        for (int i = index; i < size(); i++) {
-            int offset = baseIndexOffset.removeInt(get(i));
-            for (int j = 0; j < get(i).getSlots(); j++) {
-                handlerBySlotIndex.remove(offset + j);
-            }
+        if (invalidIndex(index)) {
+            throw new IndexOutOfBoundsException();
         }
 
-        var removed = handlerList.remove(index);
-        for (var handler : handlerList) {
-            if (baseIndexOffset.containsKey(handler))
-                continue;
+        IItemHandler handler = get(index);
 
-            int offset = handlerBySlotIndex.size();
-            baseIndexOffset.put(handler, offset);
-            for (int i = 0; i < handler.getSlots(); i++) {
-                handlerBySlotIndex.put(offset + i, handler);
+        // remove handler
+        int lower = baseIndexOffset.removeInt(handler);
+
+        // remove slot indices
+        int upper = lower + handler.getSlots();
+        for (int i = lower; i < upper; i++) {
+            handlerBySlotIndex.remove(i);
+        }
+
+        // update slot indices ahead of the removed handler
+        for (int slot = upper; slot < getSlots() + handler.getSlots(); slot++) {
+            IItemHandler remove = handlerBySlotIndex.remove(slot);
+            handlerBySlotIndex.put(slot - upper, remove);
+        }
+
+        // update handlers ahead of the removed handler
+        for (IItemHandler h : baseIndexOffset.keySet()) {
+            int offset = baseIndexOffset.getInt(h);
+            if (offset > lower) {
+                baseIndexOffset.put(h, offset - handler.getSlots());
             }
         }
-        return removed;
+
+        return handler;
+    }
+
+    public boolean invalidIndex(int index) {
+        if (baseIndexOffset.isEmpty()) return false;
+        return index < 0 || index >= baseIndexOffset.size();
     }
 }

src/test/java/gregtech/api/capability/impl/ItemHandlerListTest.java

@@ -2,13 +2,14 @@
 
 import gregtech.Bootstrap;
 
+import net.minecraftforge.items.IItemHandler;
 import net.minecraftforge.items.ItemStackHandler;
 
 import org.hamcrest.MatcherAssert;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
-import java.util.Collections;
+import java.util.Objects;
 
 public class ItemHandlerListTest {
 
@@ -19,18 +20,24 @@ public static void bootstrap() {
 
     @Test
     public void testListOperations() {
-        var list = new ItemHandlerList(Collections.emptyList());
+        var list = new ItemHandlerList();
+        ItemStackHandler firstHandler = new ItemStackHandler(16);
+        ItemStackHandler secondHandler = new ItemStackHandler(16);
 
         // test add
-        list.add(new ItemStackHandler(16));
-//        MatcherAssert.assertThat("size is wrong", list.getSlots() == 16);
-        list.add(new ItemStackHandler(16));
-//        MatcherAssert.assertThat("size is wrong", list.getSlots() == 32);
-//        MatcherAssert.assertThat("size is wrong", list.size() == 2);
+        list.add(firstHandler);
+        MatcherAssert.assertThat("wrong number of slots!", list.getSlots() == 16);
+        MatcherAssert.assertThat("wrong number of handlers!", list.size() == 1);
+        list.add(secondHandler);
+        MatcherAssert.assertThat("wrong number of slots!", list.getSlots() == 32);
+        MatcherAssert.assertThat("wrong number of handlers!", list.size() == 2);
 
         // test removal
-        list.remove(0);
-//        MatcherAssert.assertThat("size is wrong", list.getSlots() == 16);
-//        MatcherAssert.assertThat("size is wrong", list.size() == 1);
+        IItemHandler removed = list.remove(0);
+        MatcherAssert.assertThat("wrong number of slots!", list.getSlots() == 16);
+        MatcherAssert.assertThat("wrong number of handlers!", list.size() == 1);
+        MatcherAssert.assertThat("removed handler is not the first handler!", Objects.equals(removed, firstHandler));
+        int newIndex = list.getIndexOffset(secondHandler);
+        MatcherAssert.assertThat("second handler was not updated!", newIndex == 0);
     }
 }