Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

It is impossible to log water reinforcement damage events correctly #30

Open
indiv0 opened this issue Nov 2, 2013 · 1 comment
Open
Assignees
Labels

Comments

@indiv0
Copy link
Member

indiv0 commented Nov 2, 2013

As of the latest commit in feat/refactoringAndCleanup, 4f44d61, there is an issue where it is impossible to log flowing water damage to reinforced blocks without introducing a bug.

Scenario: Torches
In a previous scenario, BlockFromToEvent was previously monitored to remove reinforcements from reinforced blocks that were washed away by water (e.g. torches).

Issue #29
Previously, in issue #29, water would call a BlockFromToEvent if a redstone dust was placed on a block adjacent to the water. If this block was reinforced at the time, the listener attached to BlockFromToEvent would assume the reinforced block was being washed away and remove the reinforcement.

This would occur even if the block was not permeable to water (e.g. dirt blocks).

This is a bug because blocks should retain their reinforcement if they are not being washed away.

The reason this erroneous BlockFromToEvent gets called is because of this issue in CraftBukkit.

Solution to Issue #29
The solution to Issue #29 is to not remove reinforcements in BlockFromToEvent itself. This might seem to cause an issue with the torches scenario described above, but BlockFromToEvent gets called more than once sequentially for any water flow event.

_Torch Scenario_
When the second BTFE is called, in the torch scenario, the torches have been washed away by the first BTFE, and now in the second BTFE, in the torch position is a STATIONARY_WATER block.

This check will be executed by the second BTFE.

isReinforced() will ensure that the reinforcement is on a valid block, but as the block is now STATIONARY_WATER, the reinforcement gets removed, thereby making the torches scenario conform to expected results.

_Redstone Scenario_
In the case of the redstone bug, the first and second BTFEs will do nothing as they no longer call removeReinforcement() and the block on which the redstone dust is put is allowed to keep its (valid) reinforcement.

The Problem
In the torch scenario, removeReinforcement() is not called until the second BTFE, which means that it is impossible to know what reinforced block was damaged at the time of event logging (as the block is now STATIONARY_WATER). This makes it impossible to correctly log the Prism event at that time.

As well, the events occur out of order, as BLOCK_BREAK gets logged to Prism before BLOCK_DEINFORCE. This is because the block is broken immediately after the first BTFE, and before the second.

These two issues make proper rollbacks in this scenario impossible.

The Conflict
Removing the reinforcement during the first BTFE, as it was initially, will correct the problems stated above, but reintroduce Issue #29.

@ghost ghost assigned indiv0 and ams2990 Nov 2, 2013
@indiv0
Copy link
Member Author

indiv0 commented Aug 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants