Fabric API

Fabric API

106M Downloads

ItemPickup Event

TheCoolestPaul opened this issue ยท 5 comments

commented

I think an OnItemPickup(ItemStack itemStack) would be a nice addition.

commented

As far as in 1.19.3, the logic of ItemEntity.onPlayerCollision is:

 public void onPlayerCollision(PlayerEntity player) {
        if (!this.world.isClient) { // this always = 0 Air
            ItemStack itemStack = this.getStack();
            Item item = itemStack.getItem(); // Real item
            int i = itemStack.getCount();
            if (this.pickupDelay == 0 && (this.owner == null || this.owner.equals(player.getUuid())) && player.getInventory().insertStack(itemStack)) {
                player.sendPickup(this, i);
                if (itemStack.isEmpty()) {
                    this.discard();
                    itemStack.setCount(i);
                }

                player.increaseStat(Stats.PICKED_UP.getOrCreateStat(item), i); // The item will be directly sent into player's inventory
                player.triggerItemPickedUpByEntityCriteria(this);
            }

        }
    }

The player.sendPickup(this, i) will invoke LivingEntity.sendPickup eventually.
As I comment above, (I tested it in single-player game) whatever the player picks up, the ItemEntity that onPlayerCollision is triggered is always an ItemEntity['Air'/633...].

So if you want to get the information about picked item (in 1.19.3), set the PlayerEntity.increaseStat with if condition on Stats.PICKED_UP may be a better idea.

Edit: No, can not capture the item pick up state in PlayerEntity.increaseStat, just mixin on ItemEntity.onPlayerCollision HEAD and extract the real item inside it.

And here this the cause that I learned in Fabric Discord server:

  • There are two player entities for one real player, ClientPlayerEntity and ServerPlayerEntity.
  • As we know, in Minecraft code, the client side is responsible for visioning things like rendering, and the server side is for calculating all sorts of logic, so do these two player entities.
  • The ClientPlayerEntity will always pick up items in form of air (with LivingEntity.sendPickup invoked), and the real items will be sent to ServerPlayerEntity (also with LivingEntity.sendPickup).
  • When server and client do next synchronization, the server will inform the client about real items information (through ClientPlayPacketListener.onItemPickupAnimation and ItemPickupAnimationS2CPacket ), then everything is correct again.

In a nutshell, you can achieve this (monitor item picking up) by mixin either ClientPlayPacketListener.onItemPickupAnimation or ItemEntity.onPlayerCollision, I think the front one is better because it's more accurate. Here is my final implementation.

commented

Hello, I'm new to minecraft moding and was looking for exactly that event; I tried diferent ways including @boholder last try; but this one was on the client. I needed it on the server. I don't know if all your discussios are about implementing it on the client specificaly; but on the server I found an easy way that might help people comming here from google.

I created a Mixin on PlayerInventory.

@Mixin(PlayerInventory.class)
public abstract class PlayerInventoryMixin {

    @Inject(method = "addStack(ILnet/minecraft/item/ItemStack;)I", at = @At("TAIL"), cancellable = true)
    private void onItemPickup(int slot, ItemStack stack, CallbackInfoReturnable<Integer> cir) {
        ActionResult result = PlayerPickupItemCallback.EVENT.invoker().interact((PlayerInventory) (Object) this, slot, stack);
        if (result == ActionResult.FAIL) {
            cir.cancel();
        }
    }
}

That mixin invoke a PlayerPickupItemCallback.

public interface PlayerPickupItemCallback {
    Event<PlayerPickupItemCallback> EVENT = EventFactory.createArrayBacked(PlayerPickupItemCallback.class,
            (listeners) -> (player, entity, amount) -> {
                for (PlayerPickupItemCallback event : listeners) {
                    ActionResult result = event.interact(player, entity, amount);
                    if (result != ActionResult.PASS) return result;
                }
                return ActionResult.PASS;
            }
    );

    ActionResult interact(PlayerInventory playerPickingUpItems, int slot, ItemStack entityBeingPickedUp);
}

Then in my main class I have:

    override fun onInitialize() {
        PlayerPickupItemCallback.EVENT.register(PlayerPickupItemCallback { inventory: PlayerInventory, slot: Int, stack: ItemStack ->
            // Do some logic here
            return@PlayerPickupItemCallback ActionResult.PASS

        })
    }
commented

This definitely seems like a good fit for where fabric api can go in the future, I would agree. In the meantime, however, if you are looking for a way to do this right now it should be as simple as mixing into ItemEntity.onPlayerCollision or LivingEntity.sendPickup depending upon what your purpose is. If you'd like more help with getting a quick and dirty item pickup event working, join the fabric discord and ask for help in any of the #mod-dev-1, #mod-dev-2, or #mod-dev-3 channels.

commented

As a little history lesson for anyone trying to turn this into a PR. Addressing this feature was previously by Draylar in #261 and #449, but neither made it into production (however discussion on the implementation in those two cases should probably be considered for a future attempt as well). In terms of events standardization, it has been discussed that events should have an "allow", "before", "after", and "cancelled" callback (see #1121), which I personally think would fit well in this case. Additionally, this could probably be bundled with an item drop event. In terms of organization this would likely go in fabric-events-interaction-v0.

commented

I did a DIRTY implementation, but it works great for what I needed.
I have it here