Make round robin work with blocked paths. Fixes #64

src/main/java/dev/technici4n/moderndynamics/attachment/attached/ItemAttachedIo.java

@@ -345,9 +345,13 @@ public int getRoundRobinIndex(int maxValue) {
         return roundRobinIndex;
     }
 
-    public void incrementRoundRobin() {
+    /**
+     * Ideally the increment size should correspond to the number of paths that were iterated through this time around.
+     * This will ensure uniform distribution even if some paths are blocked.
+     */
+    public void incrementRoundRobin(int incrementSize) {
         if (getRoutingMode() == RoutingMode.ROUND_ROBIN) {
-            roundRobinIndex++;
+            roundRobinIndex += incrementSize;
             setChangedCallback.run();
         }
     }

src/main/java/dev/technici4n/moderndynamics/network/item/ItemCache.java

@@ -27,6 +27,7 @@
 import net.fabricmc.fabric.api.transfer.v1.storage.StoragePreconditions;
 import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
 import net.minecraft.server.level.ServerLevel;
+import org.jetbrains.annotations.Nullable;
 
 public class ItemCache extends NetworkCache<ItemHost, ItemCache> {
     private boolean inserting = false;
@@ -51,8 +52,11 @@ protected void doTick() {
         }
     }
 
+    /**
+     * @param checkedPathsConsumer Accepts how many paths were evaluated if not null. Ignored if null.
+     */
     protected long insertList(NetworkNode<ItemHost, ItemCache> startingPoint, Iterable<ItemPath> paths, ItemVariant variant,
-            long maxAmount, TransactionContext transaction, double speedMultiplier) {
+            long maxAmount, TransactionContext transaction, double speedMultiplier, @Nullable MaxParticipant checkedPathsConsumer) {
         StoragePreconditions.notBlankNotNegative(variant, maxAmount);
         Preconditions.checkArgument(startingPoint.getNetworkCache() == this, "Tried to insert into another network!");
 
@@ -63,7 +67,10 @@ protected long insertList(NetworkNode<ItemHost, ItemCache> startingPoint, Iterab
         inserting = true;
         try {
             long totalInserted = 0;
+            int nextPathIndex = 0;
             for (var path : paths) {
+                nextPathIndex++;
+
                 // Check possible filter at the endpoint.
                 if (!path.getEndFilter(level).test(variant)) {
                     continue;
@@ -80,9 +87,14 @@ protected long insertList(NetworkNode<ItemHost, ItemCache> startingPoint, Iterab
                     startingPoint.getHost().addTravelingItem(travelingItem);
                 });
                 if (totalInserted == maxAmount) {
-                    return totalInserted;
+                    break;
                 }
             }
+
+            if (checkedPathsConsumer != null) {
+                checkedPathsConsumer.addEntry(nextPathIndex, transaction);
+            }
+
             return totalInserted;
         } finally {
             inserting = false;

src/main/java/dev/technici4n/moderndynamics/network/item/ItemHost.java

@@ -41,6 +41,7 @@
 import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
 import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
 import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil;
+import net.fabricmc.fabric.api.transfer.v1.storage.base.InsertionOnlyStorage;
 import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
 import net.minecraft.core.Direction;
 import net.minecraft.nbt.CompoundTag;
@@ -97,15 +98,15 @@ private boolean allowItemConnection(Direction side) {
     /**
      * Storage used for external injections (e.g. via hoppers), does not respect routing mode.
      */
-    private Storage<ItemVariant> buildExternalNetworkInjectStorage(Direction side) {
+    private InsertionOnlyStorage<ItemVariant> buildExternalNetworkInjectStorage(Direction side) {
         double speedupFactor = getAttachment(side) instanceof ItemAttachedIo io ? io.getItemSpeedupFactor() : 1;
-        return (InsertStorage) (resource, maxAmount, transaction) -> {
+        return (resource, maxAmount, transaction) -> {
             NetworkNode<ItemHost, ItemCache> node = findNode();
             if (node != null) {
                 var cache = node.getNetworkCache();
                 var paths = cache.pathCache.getPaths(node, side.getOpposite());
                 // The node can be null if the pipe was just placed, and not initialized yet.
-                return cache.insertList(node, paths, resource, maxAmount, transaction, speedupFactor);
+                return cache.insertList(node, paths, resource, maxAmount, transaction, speedupFactor, null);
             } else {
                 return 0;
             }
@@ -115,14 +116,14 @@ private Storage<ItemVariant> buildExternalNetworkInjectStorage(Direction side) {
     /**
      * Storage used by extractors. It respects routing mode.
      */
-    private Storage<ItemVariant> buildExtractorNetworkInjectStorage(Direction side, ItemAttachedIo extractor) {
+    private InsertionOnlyStorage<ItemVariant> buildExtractorNetworkInjectStorage(Direction side, ItemAttachedIo extractor,
+            @Nullable MaxParticipant maxIndexParticipant) {
         double speedupFactor = extractor.getItemSpeedupFactor();
         NetworkNode<ItemHost, ItemCache> node = findNode();
         var cache = node.getNetworkCache();
         var paths = rearrangePaths(cache.pathCache.getPaths(node, side.getOpposite()), extractor);
-        return (InsertStorage) (resource, maxAmount, transaction) -> {
-            // The node can be null if the pipe was just placed, and not initialized yet.
-            return cache.insertList(node, paths, resource, maxAmount, transaction, speedupFactor);
+        return (resource, maxAmount, transaction) -> {
+            return cache.insertList(node, paths, resource, maxAmount, transaction, speedupFactor, maxIndexParticipant);
         };
     }
 
@@ -181,9 +182,11 @@ public void tickAttachments() {
     private void tickExtractor(Direction side, ItemAttachedIo extractor) {
         if (extractor.isStuffed()) {
             // Move from stuffed items to network
-            if (extractor.moveStuffedToStorage(buildExtractorNetworkInjectStorage(side, extractor),
+            var maxParticipant = new MaxParticipant();
+
+            if (extractor.moveStuffedToStorage(buildExtractorNetworkInjectStorage(side, extractor, maxParticipant),
                     extractor.getMaxItemsExtracted()) > 0) {
-                extractor.incrementRoundRobin();
+                extractor.incrementRoundRobin(maxParticipant.getMax());
                 pipe.setChanged();
                 if (!extractor.isStuffed()) {
                     pipe.sync();
@@ -193,13 +196,16 @@ private void tickExtractor(Direction side, ItemAttachedIo extractor) {
             var adjStorage = getAdjacentStorage(side, false);
             if (adjStorage == null)
                 return;
+
+            var maxParticipant = new MaxParticipant();
+
             if (StorageUtil.move(
                     adjStorage,
-                    buildExtractorNetworkInjectStorage(side, extractor),
+                    buildExtractorNetworkInjectStorage(side, extractor, maxParticipant),
                     extractor::matchesItemFilter,
                     extractor.getMaxItemsExtracted(),
                     null) > 0) {
-                extractor.incrementRoundRobin();
+                extractor.incrementRoundRobin(maxParticipant.getMax());
             }
         }
     }
@@ -230,7 +236,10 @@ public void tickAttractor(Direction side, ItemAttachedIo attractor) {
             long maxTransfer = attractor.getMaxItemsExtracted();
             long toTransfer = maxTransfer;
 
+            int nextPathIndex = 0;
             for (var path : paths) {
+                nextPathIndex++;
+
                 // Don't allow attractors to pull from other attractors
                 if (path.getEndAttachment(cache.level) instanceof ItemAttachedIo io && io.getType() == IoAttachmentType.ATTRACTOR) {
                     continue;
@@ -241,7 +250,7 @@ public void tickAttractor(Direction side, ItemAttachedIo attractor) {
                     // Make sure to check the filter at the endpoint.
                     var endpointFilter = path.getEndFilter(cache.level);
 
-                    var insertStorage = (InsertStorage) (variant, maxAmount, tx) -> {
+                    InsertionOnlyStorage<ItemVariant> insertStorage = (variant, maxAmount, tx) -> {
                         return insertTarget.insert(variant, maxAmount, tx, (v, a) -> {
                             var reversedPath = path.reversed();
                             var travelingItem = reversedPath.makeTravelingItem(v, a, attractor.getItemSpeedupFactor());
@@ -260,7 +269,7 @@ public void tickAttractor(Direction side, ItemAttachedIo attractor) {
             }
 
             if (toTransfer < maxTransfer) {
-                attractor.incrementRoundRobin();
+                attractor.incrementRoundRobin(nextPathIndex);
             }
         }
     }

src/main/java/dev/technici4n/moderndynamics/network/item/InsertStorage.java → src/main/java/dev/technici4n/moderndynamics/network/item/MaxParticipant.java

@@ -18,15 +18,30 @@
  */
 package dev.technici4n.moderndynamics.network.item;
 
-import java.util.Collections;
-import java.util.Iterator;
-import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
-import net.fabricmc.fabric.api.transfer.v1.storage.StorageView;
-import net.fabricmc.fabric.api.transfer.v1.storage.base.InsertionOnlyStorage;
+import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
+import net.fabricmc.fabric.api.transfer.v1.transaction.base.SnapshotParticipant;
+
+public class MaxParticipant extends SnapshotParticipant<Integer> {
+    private int max = 0;
+
+    public void addEntry(int amount, TransactionContext transaction) {
+        if (amount > max) {
+            updateSnapshots(transaction);
+            max = amount;
+        }
+    }
+
+    public int getMax() {
+        return max;
+    }
+
+    @Override
+    protected Integer createSnapshot() {
+        return max;
+    }
 
-public interface InsertStorage extends InsertionOnlyStorage<ItemVariant> {
     @Override
-    default Iterator<StorageView<ItemVariant>> iterator() {
-        return Collections.emptyIterator();
+    protected void readSnapshot(Integer snapshot) {
+        max = snapshot;
     }
 }

src/main/java/dev/technici4n/moderndynamics/test/ItemDistributionTest.java

@@ -0,0 +1,77 @@
+/*
+ * Modern Dynamics
+ * Copyright (C) 2021 shartte & Technici4n
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package dev.technici4n.moderndynamics.test;
+
+import dev.technici4n.moderndynamics.attachment.settings.RedstoneMode;
+import dev.technici4n.moderndynamics.attachment.settings.RoutingMode;
+import dev.technici4n.moderndynamics.init.MdBlocks;
+import dev.technici4n.moderndynamics.init.MdItems;
+import dev.technici4n.moderndynamics.test.framework.MdGameTest;
+import dev.technici4n.moderndynamics.test.framework.MdGameTestHelper;
+import net.fabricmc.fabric.api.gametest.v1.FabricGameTest;
+import net.minecraft.core.BlockPos;
+import net.minecraft.core.Direction;
+import net.minecraft.gametest.framework.GameTest;
+import net.minecraft.world.item.ItemStack;
+import net.minecraft.world.item.Items;
+import net.minecraft.world.level.block.Blocks;
+import net.minecraft.world.level.block.entity.ChestBlockEntity;
+
+public class ItemDistributionTest extends MdGameTest {
+    @GameTest(template = FabricGameTest.EMPTY_STRUCTURE, timeoutTicks = 200)
+    public void testRoundRobinInvalidTarget(MdGameTestHelper helper) {
+        var sourceChest = new BlockPos(0, 1, 0);
+        helper.setBlock(sourceChest, Blocks.CHEST);
+
+        var targetChest1 = new BlockPos(2, 1, 0);
+        helper.setBlock(targetChest1, Blocks.CHEST);
+        var targetChest2 = new BlockPos(2, 1, 1);
+        helper.setBlock(targetChest2, Blocks.CHEST);
+        var targetChest3 = new BlockPos(2, 1, 2);
+        helper.setBlock(targetChest3, Blocks.CHEST);
+
+        var pipePos = new BlockPos(1, 1, 0);
+        helper.pipe(pipePos, MdBlocks.ITEM_PIPE)
+                .attachment(Direction.WEST, MdItems.EXTRACTOR)
+                .configureItemIo(Direction.WEST, io -> {
+                    io.setUpgrade(0, Items.COMPARATOR.getDefaultInstance());
+                    io.setUpgrade(1, new ItemStack(Items.REPEATER, 3));
+                    io.setUpgrade(2, Items.STICKY_PISTON.getDefaultInstance());
+                    io.setMaxItemsExtracted(1);
+                    io.setRoutingMode(RoutingMode.ROUND_ROBIN);
+                });
+        helper.pipe(new BlockPos(1, 1, 1), MdBlocks.ITEM_PIPE)
+                .attachment(Direction.EAST, MdItems.FILTER)
+                .configureItemIo(Direction.EAST, io -> {
+                    io.setRedstoneMode(RedstoneMode.REQUIRES_HIGH);
+                });
+        helper.pipe(new BlockPos(1, 1, 2), MdBlocks.ITEM_PIPE);
+
+        ((ChestBlockEntity) helper.getBlockEntity(sourceChest)).setItem(0, new ItemStack(Items.DIAMOND, 6));
+
+        helper.startSequence()
+                .thenWaitUntil(() -> {
+                    helper.assertContainerEmpty(sourceChest);
+                    helper.checkItem(targetChest1, Items.DIAMOND, 3);
+                    helper.checkItem(targetChest2, Items.DIAMOND, 0);
+                    helper.checkItem(targetChest3, Items.DIAMOND, 3);
+                })
+                .thenSucceed();
+    }
+}

src/main/java/dev/technici4n/moderndynamics/test/framework/MdGameTestHelper.java

@@ -23,10 +23,13 @@
 import java.util.Objects;
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidStorage;
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.item.ItemStorage;
+import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
 import net.minecraft.core.BlockPos;
 import net.minecraft.core.Direction;
 import net.minecraft.gametest.framework.GameTestHelper;
 import net.minecraft.gametest.framework.GameTestInfo;
+import net.minecraft.world.item.Item;
 import net.minecraft.world.level.material.Fluid;
 
 public class MdGameTestHelper extends GameTestHelper {
@@ -60,6 +63,23 @@ public void checkFluid(BlockPos pos, Fluid fluid, long minAmount) {
             }
         }
 
-        fail("Fluid not found");
+        fail("Fluid not found", pos);
+    }
+
+    /**
+     * Throw exception unless target block pos (relative) has at least some amount of some item.
+     */
+    public void checkItem(BlockPos pos, Item item, long minAmount) {
+        var storage = ItemStorage.SIDED.find(getLevel(), absolutePos(pos), Direction.UP);
+
+        if (storage != null) {
+            var storedAmount = storage.simulateExtract(ItemVariant.of(item), Long.MAX_VALUE, null);
+
+            if (storedAmount >= minAmount) {
+                return;
+            }
+        }
+
+        fail("Item not found", pos);
     }
 }

src/main/java/dev/technici4n/moderndynamics/test/framework/PipeBuilder.java

@@ -20,6 +20,7 @@
 
 import dev.technici4n.moderndynamics.attachment.AttachmentItem;
 import dev.technici4n.moderndynamics.attachment.attached.FluidAttachedIo;
+import dev.technici4n.moderndynamics.attachment.attached.ItemAttachedIo;
 import dev.technici4n.moderndynamics.pipe.PipeBlockEntity;
 import java.util.function.Consumer;
 import net.minecraft.core.Direction;
@@ -66,4 +67,14 @@ public PipeBuilder configureFluidIo(Direction direction, Consumer<FluidAttachedI
         helper.fail("Failed to find fluid io from pipe", pipe.getBlockPos());
         throw new UnsupportedOperationException();
     }
+
+    public PipeBuilder configureItemIo(Direction direction, Consumer<ItemAttachedIo> config) {
+        if (pipe.getAttachment(direction) instanceof ItemAttachedIo itemIo) {
+            config.accept(itemIo);
+            return this;
+        }
+
+        helper.fail("Failed to find item io from pipe", pipe.getBlockPos());
+        throw new UnsupportedOperationException();
+    }
 }

src/main/resources/fabric.mod.json

@@ -36,7 +36,8 @@
       "dev.technici4n.moderndynamics.client.compat.emi.MdEmiPlugin"
     ],
     "fabric-gametest": [
-      "dev.technici4n.moderndynamics.test.FluidTransferTest"
+      "dev.technici4n.moderndynamics.test.FluidTransferTest",
+      "dev.technici4n.moderndynamics.test.ItemDistributionTest"
     ]
   },
   "mixins": [