pushItems and pullItems return wrong transfer count

[runi95]

Minecraft Version

1.20.x

Version

1.111.0

Details

I'm not sure if it matters, but I'm running this on Fabric which requires that I have the "Fabric API" mod installed alongside with the CC: Tweaked mod.

latest.log

Reproduction steps:

1. Place down Chest
2. Put 48 Dirt into slot 1 of Chest
3. Place down Advanced Turtle facing the Chest
4. Right click the Advanced Turtle to open the turtle interface
5. > lua
6. lua> peripheral.call("front", "pushItems", "front", 1, nil, nil)

The peripheral call returns 48 (transferred items), but the chest now has 16 dirt in slot 1 and 32 dirt in slot 2.
If you set the "toSlot" argument to 2 then all 48 Dirt transfers correctly with the correct transfer count (at least in this very basic scenario). It seems like it counts the Dirt in slot 1 + slot 2? This however does not match the count that has actually been moved (32).

Original Discovery

I discovered this bug when I got an absurd transfer count of 218 and 266 (which is completely impossible).
The interesting part about my initial discovery is that 0 items are transferred yet if I run the command again I'll get different transfer counts of 218, 218, 266, 266, 218, (and then it loops around again). So without actually moving any items around I get a different count.
However I've been struggling to reproduce this original finding on a new world with only CC: Tweaked installed so not sure if it's actually related or if it's a bug with some of the other mods I have loaded.
Another interesting thing to note in this original discovery is that setting the "toSlot" argument makes the item transfer correctly, but always gives me 2x the correct transfer count in response.

[SquidDev]

Thank you for the report, especially with the reproduction steps. That extra detail was incredibly useful <3.

This is a nasty one. So when mods try to move items between inventories, they typically do something like the following:

1. Count how many items are in the slot.
2. Try to insert those items into the inventory.
3. Then finally remove the actual number of items that were transferred.

If we trace out what's going on here, it looks like so:

1. CC:T sees there's 48 items to transfer.
2. It puts 16 items in the first slot.
3. It puts the remaining 32 items in the next slot.
4. It removes the 48 transferred items from the first slot (leaving 16).

So strictly speaking, the currently returned number is correct — 48 items have been moved after all! But I definitely agree that it's not helpful.

That said, I'm not sure what the best way to fix this is. Inventory transfers within the same inventory are really gnarly :/.

[zyxkad]

maybe try this:

1. remove items from the inventory
2. add items to the inventory
3. add left items to the source slot

[SquidDev]

Sadly that's not really possible due to how modded inventory APIs work. Just because you can extract an item, doesn't mean you can insert it again (think the furnace output slot).

This wouldn't entirely solve the core problem either (though would make it more consistent) — it'd say 48 items have been transferred, even if none have been visibly moved!

[zyxkad]

Sadly that's not really possible due to how modded inventory APIs work. Just because you can extract an item, doesn't mean you can insert it again (think the furnace output slot).

I see this is a problem

This wouldn't entirely solve the core problem either (though would make it more consistent) — it'd say 48 items have been transferred, even if none have been visibly moved!

This is expected and should be 48, because the items actually moved by invoking setItem, and some mod's inventory may do some logic based on that, even player usually will not notice it.

[zyxkad]

Another logic I can think of is continues try to transfer the items to the target slot until none left in the source slot, or move the target to the next slot if the first target slot is full

e.g.

for (int targetSlot = xxx; targetSlot < targetSize; ) {
  ItemStack item = source.removeItem(sourceSlot, limit, true)
  if item.isEmpty() { break }
  int transferred = target.addItem(item, targetSlot)
  if transferred == 0 {
    targetSlot++
  }
  source.removeItem(sourceSlot, transferred, false)
}

[runi95]

If we trace out what's going on here, it looks like so:

1. CC:T sees there's 48 items to transfer.

2. It puts 16 items in the first slot.

3. It puts the remaining 32 items in the next slot.

4. It removes the 48 transferred items from the first slot (leaving 16).

So strictly speaking, the currently returned number is correct — 48 items have been moved after all! But I definitely agree that it's not helpful.

That said, I'm not sure what the best way to fix this is. Inventory transfers within the same inventory are really gnarly :/.

Hm... so what you're saying is that without setting a "toSlot" argument the items can actually be moved / scattered to any number of slots? Personally I'd expect it to just find a valid slot for me and then run the exact same code as it would run with a non-nil "toSlot" argument.

I'll also look a bit more into the original findings tomorrow to see which mod actually causes it to return 2x the transfer count, but there's also a bit more to it since the inventory originally contained a lot more items. I just wanted to keep things as basic as I could for now.

[runi95]

I checked the original findings out again and it seems I only get 2x transfer count when I have the UnlimitedPeripheralWorks mod enabled so I assume that part is actually a bug with that specific mod.

That being said I still find it strange how CC: Tweaked splits the items on pushItems and pullItems to the same inventory without the "toSlot" argument. You then have to do an extra "list" call to check whether or not all items actually moved.

What I was actually looking to do was to pull items from a chest without a connected modem. If the 1st item in the chest is not the item I'm looking for I need to move that item to a different slot and I don't care which slot it is as long as the entire stack is moved to a different slot. I can then move the item I'm interested in to slot 1 so that I can suck the item out of the chest.
For now I'll solve it by manually iterating through the inventory to find an empty slot that I can provide as the "toSlot" argument which should work as long as this bug cannot occur when the "toSlot" argument is provided.