Getting the source inventory on `InventoryMoveItemEvent` the moved item size is always one.

[katekerllenevich]

Expected behavior

event.getSource().getItem(0).getAmount() on InventoryMoveItemEvent should return the amount of items that are actually in slot 0 in the inventory.

Observed/Actual behavior

The above method will always return one, where as it should only be that event.getItem().getAmount() should return one.

Steps/models to reproduce

package dev.omega24.test;

public class TestPlugin extends JavaPlugin, implements Listener {

    @Override
    public void onEnable() {
        getServer().getPluginManager().registerEvents(this, this);
    }

@EventHandler
    public void onInventoryMoveItem(InventoryMoveItemEvent event) {
        if (!(event.getSource().getHolder(false) instanceof Hopper hopper)) {
            return;
        }

        System.out.println("item: " + event.getItem().getAmount());
            System.out.println("source: " + event.getSource().getItem(0).getAmount());
    }
}

Plugin and Datapack List

[17:36:40 INFO]: Plugins (1): HopperSort

Paper version

[17:35:41 INFO]: This server is running Paper version git-Paper-43 (MC: 1.19) (Implementing API version 1.19-R0.1-SNAPSHOT) (Git: 9d0650e)
You are running the latest version
Previous version: git-Paper-42 (MC: 1.19)

Other

The problem stems from the HopperBlockEntity.hopperPush function.

What happens is that on the eight line of the function the original item stack is copied to a new variable, called itemstack (ItemStack itemstack = origItemStack;). This is done so that event.getItem() always has an amount of one, but because of Java's "pass by reference" nature, this always makes all changes to origItemStack happen to itemstack.

The problem line is origItemStack.setCount(moved);. On this line the original item stack is set to the count to be moved (this is where the number one comes from), which allows event.getItem() to have the correct amount. However this also makes it for a split second that the Hopper inventory's first (or whatever slot origItemStack is in) slot has a stack size of one.

To fix this there needs to be two changes:

1. ItemStack itemstack = origItemStack; needs to turn into ItemStack itemstack = origItemStack.clone();
2. origItemStack.setCount(moved); needs to turn into itemstack.setCount(moved);

This does bring up performance questions on how badly .clone() will affect the server performance, but if thats a concern why is there a whole new variable being created just to reference the original?

[Doc94]

its not better use copy() than clone in the origItemStack?

based in this i created a PR with the changes #8087.