Repository: sirendii/advancedperipherals
Issue: Inventory Manager api, slot argument behaviour is assumedly incorrect.

### Description

The Inventory Manager api is dumb/broken.

addItemToPlayer slot designation does not choose where to place an item and instead chooses from which slot to take the item in the originating inventory/chest.
This functionality is stupid because in the alternative function, removeItemFromPlayer, the slot designation chooses the slot in the inventory to take the item from. Thus, I assume the intended behaviour is for addItemToPlayer to also designate a slot in the player inventory to place it.

The api wiki also seems to suggest this is the case, so I assume something went horribly wrong and the behaviour was implemented incorrectly. please fix.

### Steps to reproduce

1) place down computer
2) make lua script to place an item in player inventory via inventory manager.
`manager.addItemToPlayer("UP", 1, 4, "minecraft:dirt")`
3) notice that instead of placing the dirt in slot 4, (in the hotbar according to removeItemFromPlayer), it instead takes the dirt from slot 4 from the chest, and places it in the first empty slot in your inventory.

-- > correct behaviour should be to take from any random slot with the item, or anything if no item designated, and to place said item into slot 4 of the inventory, or return an error if said slot is full/occupied. -1 slot designation should place it in the first empty slot in your inventory.

### Multiplayer?

Yes

### Version

0.7.2r (Latest)

### Minecraft, Forge and maybe other related mods versions

Forge 36.2.6, Minecraft 1.16.5

### Screenshots or Videos

_No response_

### Crashlog/log

_No response_

Changed files:
src/main/java/de/srendi/advancedperipherals/common/addons/computercraft/peripheral/InventoryManagerPeripheral.java

diff --git a/src/main/java/de/srendi/advancedperipherals/common/addons/computercraft/peripheral/InventoryManagerPeripheral.java b/src/main/java/de/srendi/advancedperipherals/common/addons/computercraft/peripheral/InventoryManagerPeripheral.java
index 8e3c7db9..4f35eea8 100644
--- a/src/main/java/de/srendi/advancedperipherals/common/addons/computercraft/peripheral/InventoryManagerPeripheral.java
+++ b/src/main/java/de/srendi/advancedperipherals/common/addons/computercraft/peripheral/InventoryManagerPeripheral.java
@@ -52,11 +52,6 @@ public class InventoryManagerPeripheral extends BasePeripheral<TileEntityPeriphe
             Item item1 = ItemUtil.getRegistryEntry(item.get(), ForgeRegistries.ITEMS);
             stack = new ItemStack(item1, count);
         }
-        //With this, we can use the item parameter without need to use the slot parameter. If we don't want to use
-        //the slot parameter, we can use -1
-        int invSlot = -1;
-        if (slot.isPresent() && slot.get() > 0)
-            invSlot = slot.get();
 
         Direction direction = validateSide(invDirection);
 
@@ -65,6 +60,13 @@ public class InventoryManagerPeripheral extends BasePeripheral<TileEntityPeriphe
                 .getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, direction).resolve().orElse(null) : null;
         PlayerInventory inventoryTo = getOwnerPlayer().inventory;
 
+
+
+        int invSlot = slot.orElse(0);
+
+        if (invSlot > inventoryTo.getContainerSize() || invSlot < 0)
+            throw new LuaException("Inventory out of bounds " + invSlot + " (max: " + (inventoryTo.getContainerSize()-1) + ")");
+
         //inventoryTo is checked via ensurePlayerIsLinked()
         if (inventoryFrom == null)
             return 0;
@@ -72,71 +74,33 @@ public class InventoryManagerPeripheral extends BasePeripheral<TileEntityPeriphe
         int amount = count;
         int transferableAmount = 0;
 
-        if (invSlot == -1)
-            for (int i = 0; i < inventoryFrom.getSlots(); i++) {
-                if (!stack.isEmpty())
-                    if (inventoryFrom.getStackInSlot(i).sameItem(stack)) {
-                        if (inventoryFrom.getStackInSlot(i).getCount() >= amount) {
-                            if (inventoryTo.add(inventoryFrom.extractItem(i, amount, true))) {
-                                inventoryFrom.extractItem(i, amount, false);
-                                transferableAmount += amount;
-                            }
-                            break;
-                        } else {
-                            int subcount = inventoryFrom.getStackInSlot(i).getCount();
-                            if (inventoryTo.add(inventoryFrom.extractItem(i, subcount, true))) {
-                                inventoryFrom.extractItem(i, subcount, false);
-                                amount = count - subcount;
-                                transferableAmount += subcount;
-                            }
-                        }
-                    }
+        for (int i = 0; i < inventoryFrom.getSlots() && amount > 0; i++) {
+            if (stack.isEmpty()) {
+                stack = inventoryFrom.getStackInSlot(i).copy();
                 if (stack.isEmpty())
-                    if (inventoryFrom.getStackInSlot(i).getCount() >= amount) {
-                        if (inventoryTo.add(inventoryFrom.extractItem(i, amount, true))) {
-                            inventoryFrom.extractItem(i, amount, false);
-                            transferableAmount += amount;
-                        }
-                        break;
-                    } else {
-                        int subcount = inventoryFrom.getStackInSlot(i).getCount();
-                        if (inventoryTo.add(inventoryFrom.extractItem(i, subcount, true))) {
-                            inventoryFrom.extractItem(i, subcount, false);
-                            amount = count - subcount;
-                            transferableAmount += subcount;
-                        }
-                    }
+                    continue;
             }
-        if (invSlot != -1) {
-            if (stack.isEmpty())
-                if (inventoryFrom.getStackInSlot(slot.get()).getCount() >= amount) {
-                    if (inventoryTo.add(inventoryFrom.extractItem(slot.get(), amount, true))) {
-                        inventoryFrom.extractItem(slot.get(), amount, false);
-                        transferableAmount += amount;
+
+            if (inventoryFrom.getStackInSlot(i).sameItem(stack)) {
+                ItemStack invSlotItem = inventoryTo.getItem(invSlot);
+                int subcount = Math.min( inventoryFrom.getStackInSlot(i).getCount(), amount);
+                if (!invSlotItem.sameItem(stack) || invSlotItem.getCount() == invSlotItem.getMaxStackSize()) {
+                    if (inventoryTo.add(inventoryFrom.extractItem(i, subcount, true))) {
+                        transferableAmount += subcount;
+                        amount -= subcount;
+                        inventoryFrom.extractItem(i, subcount, false);
                     }
                 } else {
-                    int subcount = inventoryFrom.getStackInSlot(slot.get()).getCount();
-                    if (inventoryTo.add(inventoryFrom.extractItem(slot.get(), subcount, true))) {
-                        inventoryFrom.extractItem(slot.get(), subcount, false);
+                    subcount = Math.min(subcount, stack.getMaxStackSize() - invSlotItem.getCount());
+                    if (inventoryTo.add(invSlot, inventoryFrom.extractItem(i, subcount, true))) {
+                        inventoryFrom.extractItem(i, subcount, false);
+                        amount -= subcount;
                         transferableAmount += subcount;
                     }
                 }
-            if (!stack.isEmpty())
-                if (inventoryFrom.getStackInSlot(slot.get()).sameItem(stack)) {
-                    if (inventoryFrom.getStackInSlot(slot.get()).getCount() >= amount) {
-                        if (inventoryTo.add(inventoryFrom.extractItem(slot.get(), amount, true))) {
-                            inventoryFrom.extractItem(slot.get(), amount, false);
-                            transferableAmount += amount;
-                        }
-                    } else {
-                        int subcount = inventoryFrom.getStackInSlot(slot.get()).getCount();
-                        if (inventoryTo.add(inventoryFrom.extractItem(slot.get(), subcount, true))) {
-                            inventoryFrom.extractItem(slot.get(), subcount, false);
-                            transferableAmount += subcount;
-                        }
-                    }
-                }
+            }
         }
+
         return transferableAmount;
     }