fixed recipes blocking each other by making dependencies not auto-deliver items

closes #232

src/main/java/de/ellpeck/prettypipes/network/ActiveCraft.java

@@ -1,5 +1,6 @@
 package de.ellpeck.prettypipes.network;
 
+import com.mojang.datafixers.util.Either;
 import de.ellpeck.prettypipes.Utility;
 import de.ellpeck.prettypipes.misc.ItemEquality;
 import net.minecraft.core.BlockPos;
@@ -10,14 +11,13 @@ import net.minecraft.world.item.ItemStack;
 import net.neoforged.neoforge.common.util.INBTSerializable;
 import org.jetbrains.annotations.UnknownNullability;
 
-import java.util.ArrayList;
 import java.util.List;
 
 public class ActiveCraft implements INBTSerializable<CompoundTag> {
 
     public BlockPos pipe;
     public int moduleSlot;
-    public List<NetworkLock> ingredientsToRequest;
+    public List<Either<NetworkLock, ItemStack>> ingredientsToRequest;
     public List<ItemStack> travelingIngredients;
     public BlockPos resultDestPipe;
     public ItemStack resultStackRemain;
@@ -26,7 +26,7 @@ public class ActiveCraft implements INBTSerializable<CompoundTag> {
     // we only remove canceled requests from the queue once their items are fully delivered to the crafting location, so that unfinished recipes don't get stuck in crafters etc.
     public boolean canceled;
 
-    public ActiveCraft(BlockPos pipe, int moduleSlot, List<NetworkLock> ingredientsToRequest, List<ItemStack> travelingIngredients, BlockPos resultDestPipe, ItemStack resultStackRemain) {
+    public ActiveCraft(BlockPos pipe, int moduleSlot, List<Either<NetworkLock, ItemStack>> ingredientsToRequest, List<ItemStack> travelingIngredients, BlockPos resultDestPipe, ItemStack resultStackRemain) {
         this.pipe = pipe;
         this.moduleSlot = moduleSlot;
         this.ingredientsToRequest = ingredientsToRequest;
@@ -44,7 +44,11 @@ public class ActiveCraft implements INBTSerializable<CompoundTag> {
         var ret = new CompoundTag();
         ret.putLong("pipe", this.pipe.asLong());
         ret.putInt("module_slot", this.moduleSlot);
-        ret.put("ingredients_to_request", Utility.serializeAll(this.ingredientsToRequest, n -> n.serializeNBT(provider)));
+        ret.put("ingredients_to_request", Utility.serializeAll(this.ingredientsToRequest, n -> {
+            var tag = new CompoundTag();
+            n.ifLeft(l -> tag.put("lock", l.serializeNBT(provider))).ifRight(s -> tag.put("stack", s.save(provider)));
+            return tag;
+        }));
         ret.put("traveling_ingredients", Utility.serializeAll(this.travelingIngredients, s -> (CompoundTag) s.save(provider, new CompoundTag())));
         ret.putLong("result_dest_pipe", this.resultDestPipe.asLong());
         ret.put("result_stack_remain", this.resultStackRemain.saveOptional(provider));
@@ -58,7 +62,8 @@ public class ActiveCraft implements INBTSerializable<CompoundTag> {
     public void deserializeNBT(HolderLookup.Provider provider, CompoundTag nbt) {
         this.pipe = BlockPos.of(nbt.getLong("pipe"));
         this.moduleSlot = nbt.getInt("module_slot");
-        this.ingredientsToRequest = Utility.deserializeAll(nbt.getList("ingredients_to_request", Tag.TAG_COMPOUND), t -> new NetworkLock(provider, t));
+        this.ingredientsToRequest = Utility.deserializeAll(nbt.getList("ingredients_to_request", Tag.TAG_COMPOUND), t ->
+            t.contains("lock") ? Either.left(new NetworkLock(provider, t.getCompound("lock"))) : Either.right(ItemStack.parseOptional(provider, t.getCompound("stack"))));
         this.travelingIngredients = Utility.deserializeAll(nbt.getList("traveling_ingredients", Tag.TAG_COMPOUND), t -> ItemStack.parse(provider, t).orElseThrow());
         this.resultDestPipe = BlockPos.of(nbt.getLong("result_dest_pipe"));
         this.resultStackRemain = ItemStack.parseOptional(provider, nbt.getCompound("result_stack_remain"));
@@ -92,7 +97,7 @@ public class ActiveCraft implements INBTSerializable<CompoundTag> {
     public boolean markCanceledOrResolve(PipeNetwork network, boolean force) {
         if (force || !this.inProgress) {
             for (var lock : this.ingredientsToRequest)
-                network.resolveNetworkLock(lock);
+                lock.ifLeft(network::resolveNetworkLock);
             return true;
         } else {
             this.canceled = true;

src/main/java/de/ellpeck/prettypipes/network/PipeNetwork.java

@@ -210,13 +210,10 @@ public class PipeNetwork extends SavedData implements GraphListener<BlockPos, Ne
     }
 
     public ItemStack requestItem(BlockPos destPipe, BlockPos destInventory, ItemStack stack, ItemEquality... equalityTypes) {
-        var remain = stack.copy();
         // check existing items
-        for (var location : this.getOrderedNetworkItems(destPipe)) {
+        var remain = this.requestExistingItem(destPipe, destInventory, null, stack, equalityTypes);
-            remain = this.requestExistingItem(location, destPipe, destInventory, null, remain, equalityTypes);
+        if (remain.isEmpty())
-            if (remain.isEmpty())
+            return remain;
-                return remain;
-        }
         // check craftable items
         return this.requestCraftedItem(destPipe, null, remain, new Stack<>(), equalityTypes).getLeft();
     }
@@ -272,6 +269,16 @@ public class PipeNetwork extends SavedData implements GraphListener<BlockPos, Ne
         return Pair.of(stack, crafts);
     }
 
+    public ItemStack requestExistingItem(BlockPos destPipe, BlockPos destInventory, NetworkLock ignoredLock, ItemStack stack, ItemEquality... equalityTypes) {
+        var remain = stack.copy();
+        for (var location : this.getOrderedNetworkItems(destPipe)) {
+            remain = this.requestExistingItem(location, destPipe, destInventory, ignoredLock, remain, equalityTypes);
+            if (remain.isEmpty())
+                return remain;
+        }
+        return remain;
+    }
+
     public ItemStack requestExistingItem(NetworkLocation location, BlockPos destPipe, BlockPos destInventory, NetworkLock ignoredLock, ItemStack stack, ItemEquality... equalityTypes) {
         return this.requestExistingItem(location, destPipe, destInventory, ignoredLock, PipeItem::new, stack, equalityTypes);
     }
@@ -403,11 +410,11 @@ public class PipeNetwork extends SavedData implements GraphListener<BlockPos, Ne
     }
 
     public void createNetworkLock(NetworkLock lock) {
-        this.networkLocks.put(lock.location.getPos(), lock);
+        this.networkLocks.put(lock.location != null ? lock.location.getPos() : null, lock);
     }
 
     public void resolveNetworkLock(NetworkLock lock) {
-        this.networkLocks.remove(lock.location.getPos(), lock);
+        this.networkLocks.remove(lock.location != null ? lock.location.getPos() : null, lock);
     }
 
     public List<NetworkLock> getNetworkLocks(BlockPos pos) {
@@ -415,7 +422,7 @@ public class PipeNetwork extends SavedData implements GraphListener<BlockPos, Ne
     }
 
     public int getLockedAmount(BlockPos pos, ItemStack stack, NetworkLock ignoredLock, ItemEquality... equalityTypes) {
-        return this.getNetworkLocks(pos).stream()
+        return Streams.concat(this.getNetworkLocks(pos).stream(), this.getNetworkLocks(null).stream())
             .filter(l -> !l.equals(ignoredLock) && ItemEquality.compareItems(l.stack, stack, equalityTypes))
             .mapToInt(l -> l.stack.getCount()).sum();
     }

src/main/java/de/ellpeck/prettypipes/pipe/PipeBlock.java

@@ -271,7 +271,7 @@ public class PipeBlock extends BaseEntityBlock implements SimpleWaterloggedBlock
                     pipe.removeCover();
                 for (var craft : pipe.getActiveCrafts()) {
                     for (var lock : craft.ingredientsToRequest)
-                        network.resolveNetworkLock(lock);
+                        lock.ifLeft(network::resolveNetworkLock);
                 }
             }
             super.onRemove(state, worldIn, pos, newState, isMoving);

src/main/java/de/ellpeck/prettypipes/pipe/modules/craft/CraftingModuleItem.java

@@ -1,5 +1,6 @@
 package de.ellpeck.prettypipes.pipe.modules.craft;
 
+import com.mojang.datafixers.util.Either;
 import com.mojang.serialization.Codec;
 import com.mojang.serialization.codecs.RecordCodecBuilder;
 import de.ellpeck.prettypipes.Registry;
@@ -81,23 +82,38 @@ public class CraftingModuleItem extends ModuleItem {
                 if (!craft.ingredientsToRequest.isEmpty()) {
                     if (craft.moduleSlot == slot) {
                         network.startProfile("crafting_ingredients");
-                        var lock = craft.ingredientsToRequest.getFirst();
+                        var ingredient = craft.ingredientsToRequest.getFirst();
-                        var equalityTypes = ItemFilter.getEqualityTypes(tile);
+                        var toRequest = ingredient.map(l -> l.stack, s -> s).copy();
-                        var dest = tile.getAvailableDestination(Direction.values(), lock.stack, true, true);
+                        var dest = tile.getAvailableDestination(Direction.values(), toRequest, true, true);
                         if (dest != null) {
                             // if we're ensuring the correct item order and the item is already on the way, don't do anything yet
                             if (!module.get(Contents.TYPE).ensureItemOrder || craft.travelingIngredients.isEmpty()) {
-                                network.requestExistingItem(lock.location, tile.getBlockPos(), dest.getLeft(), lock, dest.getRight(), equalityTypes);
+                                var equalityTypes = ItemFilter.getEqualityTypes(tile);
-                                network.resolveNetworkLock(lock);
+                                var requested = ingredient.map(l -> {
-                                craft.ingredientsToRequest.remove(lock);
+                                    // we can ignore the return value here since we're using a lock, so we know that the item is already waiting for us there
-                                craft.travelingIngredients.add(lock.stack.copy());
+                                    network.requestExistingItem(l.location, tile.getBlockPos(), dest.getLeft(), l, dest.getRight(), equalityTypes);
-                                craft.inProgress = true;
+                                    network.resolveNetworkLock(l);
+                                    return toRequest;
+                                }, s -> {
+                                    var remain = network.requestExistingItem(tile.getBlockPos(), dest.getLeft(), null, dest.getRight(), equalityTypes);
+                                    var ret = s.copyWithCount(s.getCount() - remain.getCount());
+                                    s.setCount(remain.getCount());
+                                    return ret;
+                                });
+                                if (!requested.isEmpty()) {
+                                    if (toRequest.getCount() - requested.getCount() <= 0)
+                                        craft.ingredientsToRequest.remove(ingredient);
+                                    craft.travelingIngredients.add(requested);
+                                    craft.inProgress = true;
+                                }
                             }
                         }
                         network.endProfile();
                     }
                     foundMainCraft = true;
-                } else if (!craft.resultFound && craft.travelingIngredients.isEmpty()) {
+                } else if (!craft.travelingIngredients.isEmpty()) {
+                    foundMainCraft = true;
+                } else if (!craft.resultFound) {
                     if (craft.moduleSlot == slot) {
                         // check whether the crafting results have arrived in storage
                         if (craft.resultStackRemain.isEmpty()) {
@@ -125,20 +141,17 @@ public class CraftingModuleItem extends ModuleItem {
 
             // pull requested crafting results from the network once they are stored
             if (craft.resultFound && craft.moduleSlot == slot) {
-                var items = network.getOrderedNetworkItems(tile.getBlockPos());
-                var equalityTypes = ItemFilter.getEqualityTypes(tile);
                 network.startProfile("pull_crafting_results");
                 var destPipe = network.getPipe(craft.resultDestPipe);
                 if (destPipe != null) {
                     var dest = destPipe.getAvailableDestinationOrConnectable(craft.resultStackRemain, true, true);
                     if (dest != null) {
-                        for (var item : items) {
+                        var equalityTypes = ItemFilter.getEqualityTypes(tile);
-                            var requestRemain = network.requestExistingItem(item, craft.resultDestPipe, dest.getLeft(), null, dest.getRight(), equalityTypes);
+                        var requestRemain = network.requestExistingItem(craft.resultDestPipe, dest.getLeft(), null, dest.getRight(), equalityTypes);
-                            craft.resultStackRemain.shrink(dest.getRight().getCount() - requestRemain.getCount());
+                        craft.resultStackRemain.shrink(dest.getRight().getCount() - requestRemain.getCount());
-                            if (craft.resultStackRemain.isEmpty()) {
+                        if (craft.resultStackRemain.isEmpty()) {
-                                crafts.remove();
+                            crafts.remove();
-                                break;
+                            break;
-                            }
                         }
                     }
                 }
@@ -202,8 +215,7 @@ public class CraftingModuleItem extends ModuleItem {
         var allCrafts = new ArrayList<ActiveCraft>();
         // if we're ensuring item order, all items for a single recipe should be sent in order first before starting on the next one!
         for (var c = contents.ensureItemOrder ? toCraft : 1; c > 0; c--) {
-            var crafts = new ArrayList<ItemStack>();
+            var toRequest = new ArrayList<Either<NetworkLock, ItemStack>>();
-            var locks = new ArrayList<NetworkLock>();
             for (var i = 0; i < contents.input.getSlots(); i++) {
                 var in = contents.input.getStackInSlot(i);
                 if (in.isEmpty())
@@ -212,19 +224,24 @@ public class CraftingModuleItem extends ModuleItem {
                 if (!contents.ensureItemOrder)
                     request.setCount(in.getCount() * toCraft);
                 var ret = network.requestLocksAndStartCrafting(tile.getBlockPos(), items, unavailableConsumer, request, CraftingModuleItem.addDependency(dependencyChain, module), equalityTypes);
-                // set crafting dependencies as in progress immediately so that, when canceling, they don't leave behind half-crafted inbetween dependencies
+                for (var lock : ret.getLeft())
-                // TODO to be more optimal, we should really do this when setting the main craft as in progress, but that would require storing references to all of the dependencies
+                    toRequest.add(Either.left(lock));
-                ret.getRight().forEach(a -> a.inProgress = true);
+                for (var dep : ret.getRight()) {
-                locks.addAll(ret.getLeft());
+                    // if the dependency doesn't have a result stack, it means it's an extraneous craft and we don't need to mark it as a dependency!
-                allCrafts.addAll(ret.getRight());
+                    if (dep.resultStackRemain.isEmpty())
-                // the items we started crafting are the ones we didn't request normally (ie ones we didn't create locks for)
+                        continue;
-                var startedCrafting = request.copyWithCount(request.getCount() - ret.getLeft().stream().mapToInt(l -> l.stack.getCount()).sum());
+                    // set crafting dependencies as in progress immediately so that, when canceling, they don't leave behind half-crafted intermediate dependencies
-                if (!startedCrafting.isEmpty())
+                    // TODO to be more optimal, we should really do this when setting the main craft as in progress, but that would require storing references to all of the dependencies
-                    crafts.add(startedCrafting);
+                    dep.inProgress = true;
+                    // we don't want dependencies to send their crafted items to us automatically (instead, we request them ourselves to maintain ordering)
+                    toRequest.add(Either.right(dep.resultStackRemain));
+                    dep.resultStackRemain = ItemStack.EMPTY;
+                    allCrafts.add(dep);
+                }
             }
             var crafted = contents.ensureItemOrder ? resultAmount : resultAmount * toCraft;
             // items we started craft dependencies for are ones that will be sent to us (so we're waiting for them immediately!)
-            var activeCraft = new ActiveCraft(tile.getBlockPos(), slot, locks, crafts, destPipe, stack.copyWithCount(Math.min(crafted, leftOfRequest)));
+            var activeCraft = new ActiveCraft(tile.getBlockPos(), slot, toRequest, new ArrayList<>(), destPipe, stack.copyWithCount(Math.min(crafted, leftOfRequest)));
             tile.getActiveCrafts().add(activeCraft);
             allCrafts.add(activeCraft);
             leftOfRequest -= crafted;