Add BlockDataChangeEvent

[PSJahn]

Currently, when wanting to prevent or modify all or certain BlockData changes, you have to listen to numerous different BlockEvents to try and catch every one, and even then you don't catch cases like changes made by commands.

This PR adds a BlockDataChangeEvent which gets called whenever a BlockState gets changed in net.minecraft.world.level.Level#setBlock. To stay more consistent with Bukkit's Naming Scheme, BlockData has been chosen instead of BlockState for the Event Name.

Another benefit of this method is that commands like setblock return a sensible message that the block changes could not be made, instead of not notifying the user like with other workarounds.

[lynxplay]

Welcome to paper 🥳
Thank you for this really well crafted PR!

The general "worry" with such events has always been
a) Yea, incredibly hot code path, but len checks like you added should balance that ™️
b) more importantly, context. The event gets to carry effectively 0 context of why this change is happening and as such, it is useful for a rather niche use-case, specifically trying to protect a specific block state at all cost. For nearly all other use-cases, it is pretty nonsensical addition. The fact that plugins will still just ignore this when they skip straight to palette modifications because they are placing a crap ton of blocks does not help it either.

We'll definitely need some wider discussions on this.

[davidmayr]

b) more importantly, context. The event gets to carry effectively 0 context of why this change is happening and as such, it is useful for a rather niche use-case, specifically trying to protect a specific block state at all cost. For nearly all other use-cases, it is pretty nonsensical addition. The fact that plugins will still just ignore this when they skip straight to palette modifications because they are placing a crap ton of blocks does not help it either.

Having some state attached to a block somewhere in memory and wanting to clean that up when the block changes is the biggest usecase for me tbh. And I don't think this is that rare of a situation? The palette modification thing is something paper cannot do anything about, but it would be nice to have at least some API for when the world changes by different plugins, commands, vanilla features etc. that use the normal api. Listening to every possible event and still missing something is annoying.

More context is necessary for most other things, and it would be very tedious to properly integrate (and update that in the future) that with this event, I get that. But for those that can make use of the event in its current state, why not?

I'd be much in favor of such an event, and I was thinking of adding it in my private fork as well for a while (I just was too lazy to actually do that). Would be even better if this would be in paper directly.

[electronicboy]

Highly low-level events are a pain because they're completely out of the context of anything that traditionally interacts with this stuff and are in an area which can get hot stupidly fast; definitely not fond of an event with zero contexts fired after everything else is cancellable or even modifiable this far down the chain;

[lynxplay]

Yea, having data attached to blocks is a fine usecase, until the block is pushed by a piston and now the event did not carry enough context for you to properly move the data. Or the block was destroyed and you want to carry over data to the drops.

As to the "why not" part, the event chilling where it is has downsides (at least as long as the thing is cancelllable).
Plugin API for example is not supposed to be cancellable outside of specific calls that denote that.
This addition would retroactively make all block modification API calls cancellable with 0 proper way of the calling plugin to react to such a change.
Stuff also breaks around the concept of MONITOR priority in other existing events, specifically "At MONITOR priority, the EntityChangeBlockEvent is fine, lemme record that" being completely undone as the event now cancels, idk, an Enderman placing their block but without the entity maintaining it.
The missing context isn't just a plugin problem, it is also a server state preservation problem.

If we wanted to do something like this, it might be an idea to have some form of marker parent type for BlockEvents that modify their block state? Not to listen to but at least to make handling on callers as easy as handle(io/papermc/paper/event/block/BlockModifyingEvent)

[PSJahn]

As to the "why not" part, the event chilling where it is has downsides (at least as long as the thing is cancelllable). Plugin API for example is not supposed to be cancellable outside of specific calls that denote that.

This could be solved by setting a bitfield in flags for the API Level#setBlock calls and checking if it is present before calling the Event

[PSJahn]

36af0b8 Stops API methods from calling the BlockDataChangeEvent, similar to the UPDATE_SKIP_ON_PLACE flag. I understand that the context issue still remains, however I am adamant that this event can still be very useful. Further discussion is appreciated!

[lynxplay]

To avoid having this PR linger too much, I haven't been able to find a team member that is in favour of pulling this.
I'll be closing this PR due to such low interest on our side to expose an event at this low context.

I want to add that
a) this is purely due to the nature of the event. The PR is really well crafted, everyone in here was nothing but great at discussing this.
b) I (and I think the team) generally agree with the annoyance that causes such an idea to exist, which is "there is so many events, I don't wanna listen to all of those". I don't know how a solution to such an issue could look rn, but there might be one that we can discuss/come up with in a feature request issue or something. If we can find a way to e.g. ease registration for event listeners for a set of events, that might be something (just throwing out ideas tho).

Again, thank you for the super well crafted PR and super nice discussion + fast feedback implementation.
I hope the points above at least are enough to communicate where we are coming from, even if they do not convince everyone reading them.

[davidmayr]

I believe this event still has a use case for monitoring purposes. It doesn't need to be cancellable

In some cases there are events missing, and in others you just want to observe any block change. Some things I could think of + examples from where I wished to have such an event:

• Fully storing all changes to e.g. an arena
• Making a replay system and storing plugin changes without explicitly adding support in every single plugin you use
• I do a flood fill to check if the player is in a room and cache those results, and I am invalidating those cached states on any possible block change. Having an easy event instead of, like, 20 would be nice
• I have a few systems where I use the Chunk contains API (and use a bit of NMS magic to check for individual sections to avoid that many block state checks) to check for certain blocks and then iterate through those chunks to cache occurrences of specific blocks. When their state changes, I obviously need to replace their stored state. I've had a few things where I needed to use weird workarounds to get all the updates I needed (and there were still events missing...). Also, the individual events that are currently available are annoying to work with (some don't return the new block state, so you have to artificially create what it would look like/run the code delayed by one tick etc.) making the listening process even more tedious only to miss many events in the process.

The list might not be that exhaustive, but I'd argue there are less useful events in the API. The valid concern (which is also why this is not yet in my private fork) is the massive amount of executions this might hit. I'd argue the BlockPhysicsEvent is way worse, but this probably won't be much better, but I haven't had the opportunity to try how bad this would be in a real environment.