VanillaDev/Vanilla — pull request #454 by Hidendra (2012-12-29 08:43:05+00:00)
Title: Fix two issues encountered when cancelling a PlayerBreakCause event (via BlockChangeEvent)

This is cross-referenced with the PR on [https://github.com/SpoutDev/Spout/issues/2154](Spout)

There are currently two problems within Spout and Vanilla related to cancelling block break events. One is minor and the second is subtle yet a problem. These are listed below.

* Spout: Currently there are two main stages during the life cycle of a block being being broken. `onDestroy()` and `onPostDestroy()`. In Spout, `onDestroy()` will destroy the block. In Vanilla, `onPostDestroy()` generates drops. These two stages are entirely independent of each other so even if the block break is cancelled in `onDestroy()` the method `onPostDestroy()` is still called and thus drops are still generated. This was doubly confirmed by *always* cancelling a `BlockChangeEvent` with a `PlayerBreakCause` cause and then breaking a diamond block. The block (albeit destruction cancelled) constantly dropped diamonds, many diamonds.
* Vanilla: When a block break is cancelled (via `BlockChangeEvent`) the block is not sent back to the client, so it will appear broken until it is forcibly updated by the server or you log out and in.

I was able to fix both of these without making any API breaking changes by moving the creation/call of the `BlockChangeEvent` from `SpoutChunk#setBlockMaterial` to the digging packet handler in Vanilla. This fixes both issues:

* Spout: `onDestroy()` and thus `onPostDestroy()` are only ever reached if the `BlockChangeEvent` is not cancelled.
* Vanilla: Vanilla is required to send a `BlockChangeMessage`. This allows the block to be updated on the client with the correct one.

I have not throughly investigated all possible implications of these changes however do not foresee issues from a change so easy to test.

Likely this could be implemented in a better way by modifying the API itself but this is certainly one way :-)