Router always calls entity place event with air as placed block

[Darkere]

Describe the bug

https://github.com/desht/ModularRouters/blob/ded38f8ac94a6f354d560a0dec3c363f492ff2f5/src/main/java/me/desht/modularrouters/util/BlockUtil.java#L116

The event always gets called with air instead of the with the block being placed??

How to reproduce the bug

use any block place module

Expected behavior

call with the block to place so other mods can evaluate if the event should be cancelled

Additional details

No response

Which Minecraft version are you using?

1.16

Crash log

No response

[Darkere]

After further research, I believe you are being a bit too logical in your version of the code.... The forge code is much more... frustrating. It gets the blocksnapshot from the world (where it is already placed) and reverts it if the event fails.

Your code unfortunately doesn't add which block is being placed to the event.

[desht]

Actually, the AIR blockstate there isn't the replacing blockstate, but the block being placed against:

public EntityPlaceEvent(@Nonnull BlockSnapshot blockSnapshot, @Nonnull BlockState placedAgainst, @Nullable Entity entity)
...

The replaced and new blockstates are supposed to come from the blockSnapshot parameter, but I think there's a problem in how that's set up, and I think that problem is in the Forge event. Looking at some vanilla code where Forge has patched in the event call (e.g. FrostWalkerEnchantment#onEntityMoved()), BlockSnapshot.create() is called to set up the snapshot parameter to the event construction.

But if you look at how the EntityPlaceEvent is constructed, it sets up placedBlock immediately from blockSnapshot.getCurrentBlock(), and that's always going to be AIR (at least in my case) because the block hasn't been placed yet.

In other words, I think this is a bug with the Forge event, although I'd welcome a second opinion on that...

[Darkere]

From what I understood forge reads the blocksnapshots from the world, in order to support placement of multiple blocks at once? They then revert the blocksnapshots if the event is cancelled.

Although none of that explains why the event isn't called before the placement instead. I'll ask around.

[desht]

Yeah, that's my understanding too. But the way the place event is init'd from the snapshot just looks wrong, at least for the example I looked at (Frostwalker ice placement). The to-be-placed blockstate field in the event is read from snapshot before the block is placed, and therefore will always be incorrect. It seems like the event could use a constructor with another parameter, to specify the intended new blockstate.

A potential workaround would be a subclass of EntityPlaceEvent with an overridden getPlacedBlock() method which returns a blockstate that is explicitly passed to the custom event. Such an event is caught by a listener listening for EntityPlaceEvent (verified that experimentally).

public class CustomEntityPlaceEvent extends BlockEvent.EntityPlaceEvent {
    private final BlockState newState;

    public CustomEntityPlaceEvent(@NotNull BlockSnapshot blockSnapshot, @NotNull BlockState placedAgainst, @Nullable Entity entity, @NotNull BlockState newState) {
        super(blockSnapshot, placedAgainst, entity);
        this.newState = newState;
    }

    @Override
    public BlockState getPlacedBlock() {
        return newState;
    }
}

[Darkere]

So the reason for the event being so weird, is annoyingly logical. With mods doing all sorts of weird things to place blocks in the world, there is no way for the event to be certain about what block will actually be placed until the block is actually in world. Obviously not the case for you.

I think your solution would work.

[desht]

Yep, just implemented it in my dev world (1.18, but the same principle should apply in 1.16). Seems to work quite nicely, and a test event listener can prevent placement of blocks by type as well as by position.

[desht]

A known issue right now: https://github.com/MinecraftForge/MinecraftForge/issues/8461