Paper: [Feature request] Add isSource() and getChangedBlockData() to BlockPhysicsEvent

[cakoyo]

Is your feature request related to a problem? Please describe.

• Now, commonly BlockPhysicsEvent will be called for 7 times when one block change(the "root" block). Paper's getSourceBlock() allows checking whether the block being updated is the root block itself, but it's veiled for most developers, we can simply add a method named like isSource() and simply return sourceBlock == block.

• There is a BlockData field called changed in BPE, however, we can only use getChangedType() instead of getting the block data directly, we can simply add a method like getChangedBlockData() and simply return it.

Describe the solution you'd like
Described above.

Describe alternatives you've considered
Aside from this, we can even add a new event called BlockChangedEvent for every source event, which is perfect for block change recording and monitor. (at World#notifyAndUpdatePhysics, blockdata1 is the old block)

Additional context
I have done some tests and works smoothly.

[aikar]

id rather not encourage more use of BlockPhysicsEvent, especially since its more unreliable in 1.13 (it doesn't fire for all of physics events anymore)

but a BlockChangedEvent can be nice, if someone wants to PR it.

[aikar]

https://hub.spigotmc.org/jira/browse/SPIGOT-4256 for information on the 1.13 regression of BlockPhysicsEvent

[cakoyo]

@aikar That's weird, I just wrote a test plugin to cancel all BlockPhysicsEvents, and the behaviour is the same as 1.12.2 and seems it was fixed here: https://hub.spigotmc.org/jira/browse/SPIGOT-4178

[aikar]

@cakoyo found out it's only when NewState == AIR, it no longer fires for that. In your case I have a feeling you cancelled the NEIGHBOR blocks physics event which cancelled the event for the sign itself.

Maybe this has always been the case though. I know we had issues with signs popping but maybe that issue you linked did solve it.

But ultimately I'm about to add a BlockDestroyEvent to Paper that covers the AIR case so I can personally stop listening to Physics all together.

[aikar]

@cakoyo that's my code essentially
Our code in pre 1.13.2 was:

    @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
    public void onBlockPhysics(BlockPhysicsEvent event) {
        if (Worlds.isTownWorld(event.getBlock().getWorld())) {
            return;
        }

        String[] lines = BlockUtil.getSignLines(event.getBlock());
        if (lines == null) {
            return;
        }
        if ("[ LOCKED ]".equals(lines[0])) {
            event.setCancelled(true);
        }
    }

This worked for years to prevent signs popping, but doesn't anymore

[cakoyo]

@aikar I think adding an extra event call at Block#a is the correct solution: https://gist.github.com/cakoyo/6218171b5df24b9e1d12077b490e7291#file-block-a-java-L6

It won't call six times for every 'root' block, only when a 'state update' is needed, and will only call for the block will be updated: https://gist.github.com/cakoyo/6218171b5df24b9e1d12077b490e7291#file-block-a-java-L3

My test code:

@EventHandler
public void onPhysics(BlockPhysicsEvent event) {
    Block block = event.getBlock();

    if (event.getBlock().getType() == Material.WALL_SIGN) {
        event.setCancelled(true);
    }
}

This works with the extra call added, btw, I've removed all other event calls during the test.

---

There are two BlockPhysicsEvent calls in World and the one in notifyAndUpdatePhysics is actually called before 'update source block's 6 neighbour blocks' (i.e. update neighbour), this was added through https://hub.spigotmc.org/jira/browse/SPIGOT-4178.

I personally don't think it's a good solution, because BPE is always called for every updated block (commonly 6 blocks) before 1.13.2 (but not include source itself), and this change mixed things up. I think add a Type for BlockPhysicsEvent would be fine, such as UPDATE_BY_SOURCE', 'UPDATE_NEIGHBOUR.

---

To compare codes with 1.12.2 and do some tests, I also found that World#neighborChanged is not valid at specific cases in 1.13.2 (but still call the method), such as 'break the block that wall sign/torch against', maybe we can remove these event calls to reduce spam? (since they are useless)

---

I personally suggest removing the event call at notifyAndUpdatePhysics or clarify the event type, and adding the extra call at Block#a for every 'confirmed' block update.

And since notifyAndUpdatePhysics means a block changed, we can replace that with BlockChangedEvent. setTypeAndData is also a position can be used to call event, however, if it comes from player digging, cancel the event will still cause drop item. (idk how to handle it xD)

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. If the issue still applies to the most recent supported version, please open a new issue referencing this original issue.