search

pmmp / PocketMine-MP

A server software for Minecraft: Bedrock Edition in PHP
https://pmmp.io
GNU Lesser General Public License v3.0
3.27k stars 1.54k forks source link

ConcretePowder: Inconsistently calling BlockFormEvent #5228

Closed ColinHDev closed 2 years ago

ColinHDev commented 2 years ago

Issue description

While working on #5226, I noticed that the BlockFormEvent is not always called when ConcretePowder forms into regular Concrete after touching Water. It is only called if a neighbouring block changed: https://github.com/pmmp/PocketMine-MP/blob/stable/src/block/ConcretePowder.php#L46 But not if the powder block fell from a higher position and touched water while landing (or falling mid-air): https://github.com/pmmp/PocketMine-MP/blob/stable/src/block/ConcretePowder.php#L56-L58

Steps to reproduce the issue

  1. Register an event listener for the BlockFormEvent, so you can actually observe this.
  2. Build a small tower, place water beneath and a concrete powder block above it.
  3. Watch the concrete powder block turn into a FallingBlock entity.
  4. The entity turns into a regular concrete block without the event being called.

OS and versions

Plugins

---

Crashdump, backtrace or other files

---

dktapps commented 2 years ago

I'm not sure this actually qualifies as a bug, since there isn't actually a causing block in this case. Instead, EntityBlockChangeEvent is fired for this.

ColinHDev commented 2 years ago

Yes, there is a causing block. This is not about a FallingBlock turning into a regular block, but specifically about a falling ConcretePowder (entity) turning into a Concrete block, due to nearby water. The tickFalling() method, which gets called here only returns a regular Concrete, if a water block is nearby. Because of this, a falling ConcretePowder can turn into a regular one without touching the ground just because it fell next to a water block.

dktapps commented 2 years ago

The point is that this is an entity-block interaction, not a block-block interaction. The falling concrete powder block doesn't actually exist within the world grid, so BlockFormEvent can't really be applied.

ColinHDev commented 2 years ago

My idea was, that due to there being a block instance, tickFalling() could call the BlockFormEvent and return null if it got cancelled. But I understand your point that calling it would be wrong because the block does not exist. Yes, EntityBlockChangeEvent exists but using it for this particular use case... well, we end up right at the motivation for #5226. But feel free to close this, if you think it's fine as it is.

dktapps commented 2 years ago

If we were to use BlockFormEvent for this, you would see a water block (old state) randomly form into concrete (new state) for no obvious reason, with a water block as the cause, since there's no way to connect the falling block to BlockFormEvent. The behaviour would already differ from the behaviour of water touching concrete powder. I'm unconvinced that this would be any better.

dktapps commented 2 years ago

Closing this since it's impractical and already covered by other events.