InventorySlot fixes, Workbench Dupe fix

src/main/java/blusunrize/immersiveengineering/common/gui/IEContainerMenu.java

@@ -37,6 +37,7 @@
 import javax.annotation.Nonnull;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.IntBinaryOperator;
 import java.util.function.Predicate;
 
 @EventBusSubscriber(modid = Lib.MODID, bus = Bus.GAME)
@@ -144,7 +145,7 @@ public ItemStack quickMoveStack(Player player, int slot)
 				if(!this.moveItemStackTo(itemstack1, ownSlotCount, this.slots.size(), true))
 					return ItemStack.EMPTY;
 			}
-			else if(!this.moveItemStackToWithMayPlace(itemstack1, 0, ownSlotCount))
+			else if(!this.moveItemStackToWithMayPlace(itemstack1, 0, ownSlotCount, false))
 				return ItemStack.EMPTY;
 
 			if(itemstack1.isEmpty())
@@ -156,34 +157,82 @@ else if(!this.moveItemStackToWithMayPlace(itemstack1, 0, ownSlotCount))
 		return itemstack;
 	}
 
-	protected boolean moveItemStackToWithMayPlace(ItemStack pStack, int pStartIndex, int pEndIndex)
+	protected boolean moveItemStackToWithMayPlace(ItemStack pStack, int pStartIndex, int pEndIndex, boolean reverseDirection)
 	{
-		return moveItemStackToWithMayPlace(slots, this::moveItemStackTo, pStack, pStartIndex, pEndIndex);
+		return moveItemStackToWithMayPlace(slots, this::moveItemStackTo, pStack, pStartIndex, pEndIndex, reverseDirection);
 	}
 
+	/**
+	 * This function moves the ItemStack pStack to the target container index.
+	 * Unlike the regular AbstractContainerMenu::moveItemStackTo it will check mayPlace even when finding existing
+	 * stacks of the same item, so players can't place items into the ghost item slot this way.
+	 *
+	 * @param pStack      The ItemStack being moved
+	 * @param pStartIndex the start index of the targeted container slots
+	 * @param pEndIndex   the end index of the targeted container slots (excluded)
+	 * @return true, if any movement has taken place, false, otherwise
+	 */
 	public static boolean moveItemStackToWithMayPlace(
-			List<Slot> slots, MoveItemsFunc move, ItemStack pStack, int pStartIndex, int pEndIndex
+			List<Slot> slots, MoveItemsFunc move, ItemStack pStack, int pStartIndex, int pEndIndex, boolean reverseDirection
 	)
 	{
-		boolean inAllowedRange = true;
-		int allowedStart = pStartIndex;
-		// TODO understand
-		for(int i = pStartIndex; i < pEndIndex; i++)
+		/* Bunch of extra handling for reverseDirection */
+		int i;
+		final byte step;
+		Predicate<Integer> continueLoop;
+		IntBinaryOperator innerMoveStart;
+		IntBinaryOperator innerMoveEnd;
+		if(reverseDirection)
+		{
+			i = pEndIndex-1;
+			step = -1;
+			continueLoop = (a) -> a >= pStartIndex;
+			innerMoveStart = (a, b) -> b;
+			/* +1, because the range is exclusive and unlike i for reverseDirection == false,
+			 * allowedStart does not increment at the end of the loop body */
+			innerMoveEnd = (a, b) -> a+1;
+		}
+		else
+		{
+			i = pStartIndex;
+			step = 1;
+			continueLoop = (a) -> a < pEndIndex;
+			innerMoveStart = (a, b) -> a;
+			innerMoveEnd = (a, b) -> b;
+		}
+		boolean inAllowedRange = false;
+		int allowedStart = i;
+		boolean returnValue = false;
+
+		/* check if there are disallowed slots in the given range.
+		 * if a disallowed slot is encountered, use super.moveItemStackTo from the last allowedStart index
+		 * up until that slot (might result in a 0 range, which is fine and handled)
+		 * then, continue looking through the rest of the indices and see if another allowed slot is encountered,
+		 * to start a new range of allowed slots, which will be moved with super.moveItemStackTo */
+		while(continueLoop.test(i))
 		{
 			boolean mayplace = slots.get(i).mayPlace(pStack);
 			if(inAllowedRange&&!mayplace)
 			{
-				if(move.moveItemStackTo(pStack, allowedStart, i, false))
-					return true;
+				if(move.moveItemStackTo(pStack, innerMoveStart.applyAsInt(allowedStart, i), innerMoveEnd.applyAsInt(allowedStart, i), reverseDirection))
+				{
+					/* only return if the stack is empty, if not, continue trying to find other allowed ranges to place the remainder */
+					if (pStack.isEmpty())
+						return true;
+					returnValue = true;
+				}
 				inAllowedRange = false;
 			}
 			else if(!inAllowedRange&&mayplace)
 			{
 				allowedStart = i;
 				inAllowedRange = true;
 			}
+			i += step;
 		}
-		return inAllowedRange&&move.moveItemStackTo(pStack, allowedStart, pEndIndex, false);
+
+		returnValue |= inAllowedRange&&move.moveItemStackTo(pStack, innerMoveStart.applyAsInt(allowedStart, i), innerMoveEnd.applyAsInt(allowedStart, i), reverseDirection);
+		return returnValue;
 	}
 
 	/**
@@ -192,9 +241,9 @@ else if(!inAllowedRange&&mayplace)
 	 * immediately before the matching slot.
 	 * This logic is currently used by the storage shelf.
 	 *
-	 * @param stack the stack to be inserted
+	 * @param stack      the stack to be inserted
 	 * @param startIndex the first slot to check
-	 * @param endIndex the final slot to check
+	 * @param endIndex   the final slot to check
 	 * @return true if the stack was fully consumed
 	 */
 	public boolean moveToMatchingSlotOrAdjacent(ItemStack stack, int startIndex, int endIndex)
@@ -348,7 +397,7 @@ protected record MenuContext(
 	}
 
 	public record MultiblockMenuContext<S extends IMultiblockState>(IMultiblockContext<S> mbContext,
-																	BlockPos clickedPos)
+	                                                                BlockPos clickedPos)
 	{
 	}
 

src/main/java/blusunrize/immersiveengineering/common/gui/IESlot.java

@@ -35,6 +35,7 @@
 import net.neoforged.neoforge.capabilities.Capabilities.FluidHandler;
 import net.neoforged.neoforge.fluids.capability.IFluidHandlerItem;
 import net.neoforged.neoforge.items.IItemHandler;
+import net.neoforged.neoforge.items.ItemHandlerCopySlot;
 import net.neoforged.neoforge.items.SlotItemHandler;
 import org.jetbrains.annotations.NotNull;
 
@@ -253,7 +254,7 @@ public void setChanged()
 		}
 	}
 
-	public static class Upgrades extends SlotItemHandlerIE
+	public static class Upgrades extends ISISlotItemHandlerIE
 	{
 		final ItemStack toolStack;
 		private final IUpgradeableTool upgradeableTool;
@@ -264,7 +265,7 @@ public static class Upgrades extends SlotItemHandlerIE
 		private final Supplier<Player> getPlayer;
 
 		public Upgrades(AbstractContainerMenu container, IItemHandler inv, int id, int x, int y, String type, ItemStack toolStack,
-						boolean preventDoubles, Level world, Supplier<Player> getPlayer)
+		                boolean preventDoubles, Level world, Supplier<Player> getPlayer)
 		{
 			super(inv, id, x, y);
 			this.container = container;
@@ -295,13 +296,6 @@ public int getMaxStackSize()
 			return 64;
 		}
 
-		@Nonnull
-		@Override
-		public ItemStack getItem()
-		{
-			return upgradeableTool.getUpgradeAfterRemoval(toolStack, super.getItem());
-		}
-
 		@Override
 		public void onTake(Player thePlayer, ItemStack stack)
 		{
@@ -311,9 +305,9 @@ public void onTake(Player thePlayer, ItemStack stack)
 		}
 
 		@Override
-		public void setChanged()
+		protected void setStackCopy(ItemStack stack)
 		{
-			super.setChanged();
+			super.setStackCopy(stack);
 			if(!world.isClientSide)
 			{
 				upgradeableTool.recalculateUpgrades(toolStack, world, getPlayer.get());
@@ -726,6 +720,29 @@ public boolean mayPickup(Player player)
 		}
 	}
 
+	public static class ISIContainerCallback extends ISISlotItemHandlerIE
+	{
+		ICallbackContainer container;
+
+		public ISIContainerCallback(ICallbackContainer container, IItemHandler inv, int id, int x, int y)
+		{
+			super(inv, id, x, y);
+			this.container = container;
+		}
+
+		@Override
+		public boolean mayPlace(ItemStack itemStack)
+		{
+			return this.container.canInsert(itemStack, getSlotIndex(), this);
+		}
+
+		@Override
+		public boolean mayPickup(Player player)
+		{
+			return this.container.canTake(this.getItem(), getSlotIndex(), this);
+		}
+	}
+
 	public static class LogicCircuit extends SlotItemHandlerIE
 	{
 		public LogicCircuit(IItemHandler inv, int id, int x, int y)
@@ -760,6 +777,31 @@ public int getMaxStackSize(@NotNull ItemStack stack)
 		}
 	}
 
+	/**
+	 * This is for use with Inventory Storage Items (ISI), since inventory items use ComponentItemHandler, which returns copies of the item stacks,
+	 * and ItemHandlerCopySlot compensates for it.
+	 */
+	private static class ISISlotItemHandlerIE extends ItemHandlerCopySlot
+	{
+		public ISISlotItemHandlerIE(IItemHandler itemHandler, int index, int xPosition, int yPosition)
+		{
+			super(itemHandler, index, xPosition, yPosition);
+		}
+
+		@Override
+		public int getMaxStackSize(@NotNull ItemStack stack)
+		{
+			return Math.min(Math.min(this.getMaxStackSize(), stack.getMaxStackSize()), super.getMaxStackSize(stack));
+		}
+
+		@Override
+		public int getSlotIndex()
+		{
+			// TODO: this is a filthy workaround, because ItemHandlerCopySlot always returns 0 otherwise. Lets hope neoforge fixes that in the future.
+			return index;
+		}
+	}
+
 	public interface ICallbackContainer
 	{
 		boolean canInsert(ItemStack stack, int slotNumer, Slot slotObject);

src/main/java/blusunrize/immersiveengineering/common/gui/ModWorkbenchContainer.java

@@ -16,6 +16,7 @@
 import blusunrize.immersiveengineering.api.tool.IConfigurableTool;
 import blusunrize.immersiveengineering.api.tool.upgrade.IUpgradeableTool;
 import blusunrize.immersiveengineering.common.blocks.wooden.ModWorkbenchBlockEntity;
+import blusunrize.immersiveengineering.common.gui.IESlot.BlueprintOutput;
 import blusunrize.immersiveengineering.common.items.EngineersBlueprintItem;
 import blusunrize.immersiveengineering.mixin.accessors.ContainerAccess;
 import net.minecraft.world.entity.player.Inventory;
@@ -173,14 +174,17 @@ public ItemStack quickMoveStack(Player player, int slot)
 		ItemStack stackInSlot = slotObject.getItem();
 		resultStack = stackInSlot.copy();
 
-		if(slot < ownSlotCount)
+		boolean moveDirectionToPlayer = slot < ownSlotCount;
+		if(moveDirectionToPlayer)
 		{
-			if(!this.moveItemStackTo(stackInSlot, ownSlotCount, (ownSlotCount+36), true))
+			/* Player slots can take a simple moveItemStackTo */
+			if(!super.moveItemStackTo(stackInSlot, ownSlotCount, (ownSlotCount+36), true))
 				return ItemStack.EMPTY;
 		}
 		else if(!stackInSlot.isEmpty())
 		{
 			boolean singleSlot = ownSlotCount==1;
+
 			if(stackInSlot.getItem() instanceof EngineersBlueprintItem
 					||(stackInSlot.getItem() instanceof IUpgradeableTool uTool&&uTool.canModify(stackInSlot))
 					||(stackInSlot.getItem() instanceof IConfigurableTool cTool&&cTool.canConfigure(stackInSlot)))
@@ -196,16 +200,37 @@ else if(!stackInSlot.isEmpty())
 			}
 		}
 
-		slotObject.setChanged();
-		// TODO hack
-		slotObject.set(stackInSlot);
+		if(stackInSlot.isEmpty())
+		{
+			slotObject.setByPlayer(ItemStack.EMPTY);
+		}
+		else
+		{
+			slotObject.setChanged();
+		}
+
+		/* handle movement from crafting slots to player */
+		boolean isCrafting = slotObject instanceof BlueprintOutput;
+		if(!isCrafting)
+		{
+			resultStack = stackInSlot; /* resultStack must contain the remainder for regular quick moves */
+		}
+		/* handle movement from crafting slots to player */
+		else if(moveDirectionToPlayer) // moveDirection probably always is true, but checking doesn't hurt
+		{
+			/* if the slot still has items , it couldn't deposit them all in the player's inventory.
+			 * drop the remainder into the world */
+			if(slotObject.hasItem())
+			{
+				player.drop(slotObject.getItem(), false);
+			}
+			/* this triggers consuming the crafting components, but also resets the workbench crafting
+			 * ghost-item to the original stack size. if this stops being the case, this needs an extra
+			 * slotObject.set(resultsStack), or the ghost-item amount might differ from the intended
+			 * crafting quantity */
+			slotObject.onTake(player, resultStack);
+		}
 
-		// TODO that looks weird as well
-		if(stackInSlot.getCount()==resultStack.getCount())
-			resultStack = ItemStack.EMPTY;
-		slotObject.onTake(player, resultStack);
-		if(slotObject.hasItem())
-			player.drop(slotObject.getItem(), false);
 		return resultStack;
 	}
 
@@ -214,7 +239,8 @@ protected boolean moveItemStackTo(ItemStack stack, int startIndex, int endIndex,
 	{
 		// TODO I think super::moveItemStackTo just assumes it can mutate the stacks in slots and it will propagate
 		//  correctly?
-		return IEContainerMenu.moveItemStackToWithMayPlace(slots, super::moveItemStackTo, stack, startIndex, endIndex);
+		//  Answer: Yes. And this does fail for certain IItemHandler implementations (e.g., ComponentItemHandler).
+		return IEContainerMenu.moveItemStackToWithMayPlace(slots, super::moveItemStackTo, stack, startIndex, endIndex, reverseDirection);
 	}
 
 	@Override

src/main/java/blusunrize/immersiveengineering/common/gui/ToolboxMenu.java

@@ -69,22 +69,22 @@ public ToolboxMenu(MenuContext ctx, Inventory inventoryPlayer, IItemHandler inv)
 	{
 		super(ctx);
 		ownSlotCount = 0;
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 48, 24));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 30, 42));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 48, 42));
-
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 75, 24));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 93, 24));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 111, 24));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 75, 42));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 93, 42));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 111, 42));
-		addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 129, 42));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 48, 24));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 30, 42));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 48, 42));
+
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 75, 24));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 93, 24));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 111, 24));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 75, 42));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 93, 42));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 111, 42));
+		addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 129, 42));
 
 		for(int j = 0; j < 6; j++)
-			addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 35+j*18, 77));
+			addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 35+j*18, 77));
 		for(int j = 0; j < 7; j++)
-			addSlot(new IESlot.ContainerCallback(this, inv, ownSlotCount++, 26+j*18, 112));
+			addSlot(new IESlot.ISIContainerCallback(this, inv, ownSlotCount++, 26+j*18, 112));
 
 		for(int i = 0; i < 3; i++)
 			for(int j = 0; j < 9; j++)