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

Fix waterlogging checking affected location #1156

Merged
merged 3 commits into from Dec 16, 2020
Merged

Fix waterlogging checking affected location #1156

merged 3 commits into from Dec 16, 2020

Conversation

nouish
Copy link

@nouish nouish commented Dec 16, 2020

For blocks that may be waterlogged, checking the relative adjacent location, is wrong. Currently non-trustees may waterlog certain blocks placed on the claim boundary, from outside said claim.

This does not affect waterloggable blocks with containers (chests).

Fixes #1155

For blocks that may be waterlogged, checking the relative adjacent location, is wrong. Currently non-trustees may waterlog certain blocks placed on the claim boundary, from outside said claim.

This does not affect waterloggable blocks with containers (chests).

Will need to be updated with upcomming waterloggable blocks in Minecraft 1.17.

Fixes #1155
Copy link
Collaborator

@Jikoo Jikoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why add a manually maintained material list when Waterlogged exists?

@nouish
Copy link
Author

nouish commented Dec 16, 2020

Why add a manually maintained material list when Waterlogged exists?

Unfamiliar with the Bukkit/Spigot API, and because not all waterlogged blocks are affected. That said, it shouldn't matter if the unaffected ones are included. Will verify.

@Jikoo
Copy link
Collaborator

Jikoo commented Dec 16, 2020

I'd guess it would actually create an edge case allowing waterlogging by users with ContainerTrust who should not be able to build for those objects.

I suggest something like this (not accurate, sorry, mobile):

Block block = event.getBlock();
if (event.getItem() != Material.WATER_BUCKET || !(block.getBlockData() instanceof Waterlogged))
{
  block = block.getRelative(event.getFace());
}

Is it possible for users without trust to also remove water from waterlogged blocks?

Co-Authored-By: Adam <Jikoo.Games@gmail.com>
@nouish
Copy link
Author

nouish commented Dec 16, 2020

Excellent suggestion!

That also lets me remove that caveat of having to add the handful of waterlogged blocks comming in 1.17 at later date.

I'd guess it would actually create an edge case allowing waterlogging by users with ContainerTrust who should not be able to build for those objects.

Fortunately not 😄 It will never use CT checks for this!

@Jikoo
Copy link
Collaborator

Jikoo commented Dec 16, 2020

Fortunately not 😄 It will never use CT checks for this!

I meant with the manually specified materials not including containers, since GP was likely only blocking the water placement because it thought the player was attempting to access the inventory.

Looks good!

Copy link
Collaborator

@Jikoo Jikoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess I have to review again to dismiss a review, weird. Any rate, need Robo's final approval on this, but you have mine.

@RoboMWM RoboMWM merged commit 329bfb5 into GriefPrevention:master Dec 16, 2020
@nouish nouish deleted the fix/issue/1155 branch December 17, 2020 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Water in the fence (with video)
3 participants