DropItemEvent.Destruct.Pre not honouring getDroppedItems() for blocks

[clienthax]

I am currently running

• SpongeForge version: spongeforge-1.12.2-2586-7.1.0-BETA-2890
• SpongeVanilla version:
• Java version: 1.8
• Operating System: Win10

• Plugins/Mods:

  Mods (7): Minecraft, Minecraft Coder Pack, Forge Mod Loader, Minecraft Forge, SpongeAPI, SpongeForge, mercurius_updater

  Issue Description

Attempt to mine a diamond block with the following listener active. The dropped items list only contains a diamond ore stack after the event yet a diamond is still dropped.

[01:13:24] [Server thread/INFO] [STDOUT]: [moe.clienthax.winterfell.eventhandlers.jobs.MinerHandler:onBlockItemDrop:77]: minecraft:diamond
[01:13:24] [Server thread/INFO] [STDOUT]: [moe.clienthax.winterfell.eventhandlers.jobs.MinerHandler:onBlockItemDrop:79]: changing drop to diamond ore
[01:13:24] [Server thread/INFO] [STDOUT]: [moe.clienthax.winterfell.eventhandlers.jobs.MinerHandler:onBlockItemDrop:87]: [SpongeItemStackSnapshot{itemType=minecraft:diamond_ore, quantity=1}]

    @Listener
    public void onBlockItemDrop(DropItemEvent.Destruct.Pre event, @First Player player, @First BlockSnapshot blockSnapshot) {
        BlockType type = blockSnapshot.getState().getType();

        //Make diamond ore drop ore instead of diamonds
        if(type == BlockTypes.DIAMOND_ORE) {
            List<ItemStackSnapshot> newDrops = new ArrayList<>();
            for (ItemStackSnapshot oldDrop : event.getDroppedItems()) {
                System.out.println(oldDrop.getType().getName());
                if(oldDrop.getType() == ItemTypes.DIAMOND) {
                    System.out.println("changing drop to diamond ore");
                    newDrops.add(ItemStack.builder().itemType(ItemTypes.DIAMOND_ORE).quantity(oldDrop.getQuantity()).build().createSnapshot());
                } else {
                    newDrops.add(oldDrop);
                }
            }
            event.getDroppedItems().clear();
            event.getDroppedItems().addAll(newDrops);
            System.out.println(event.getDroppedItems());
        }

    }

[Zidane]

@RedNesto Want to look into this one?

[RedNesto]

Yeah I'll look into it.

In fact I was already interested in this issue :3

[RedNesto]

I think this will need more work than a simple fix @Zidane .

First, I can't find a DropItemEvent.Destruct.Pre implementation, they are all DropItemEvent.Destruct or DropItemEvent.Pre (at least, it is not generated in the SpongeEventFactory). I don't even know if there should be such events.

Then we have two choices:

• Implement events such as DropItemEvent.Destruct.Pre, and make genEventImpl generate them.
• Keep the implementation as-is and do magic to not have to deal with this type of events in the API.

For now, it seems DropItemEvent.Pre is fired (in this case at least) and only the getEntities list is processed, so we do not actually take care of getDroppedItems. We could make getDroppedItems immutable (getEntities cannot be immutable since it comes from SpawnEntityEvent and would break the contract of this event) or make some hacks (but I actually have no idea how) to fuse the getDroppedItems into getEntities to only have to process this last list.

For now, a workaround for @clienthax would have a Listener like this:

    @Listener
    public void onBlockItemDrop(DropItemEvent.Destruct event, @First Player player, @First BlockSnapshot blockSnapshot) {
        BlockType type = blockSnapshot.getState().getType();

        if (type == BlockTypes.DIAMOND_ORE) {
            List<Entity> toAdd = new ArrayList<>();
            for (Iterator<Entity> iterator = event.getEntities().iterator(); iterator.hasNext(); ) {
                Entity itemEntity = iterator.next();
                Optional<Location<World>> maybeLocation = blockSnapshot.getLocation();
                if (itemEntity.getType() == EntityTypes.ITEM && maybeLocation.isPresent()) {
                    Optional<ItemStackSnapshot> maybeSnapshot = itemEntity.get(Keys.REPRESENTED_ITEM);
                    if (maybeSnapshot.isPresent() && maybeSnapshot.get().getType() == ItemTypes.DIAMOND) {
                        iterator.remove();
                        toAdd.add(convertToDroppedItem(maybeLocation.get().getExtent(), maybeLocation.get().getPosition(),
                                ItemStack.builder().itemType(ItemTypes.DIAMOND_ORE).quantity(maybeSnapshot.get().getQuantity()).build().createSnapshot()));
                    }
                }
            }
            event.getEntities().addAll(toAdd);
        }
    }

This is quite messy but it works.

[clienthax]

@RedNesto convertToDroppedItem ?