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

Deny brushing of suspicious blocks in claims without build trust #2162

Closed
wants to merge 3 commits into from

Conversation

RoboMWM
Copy link

@RoboMWM RoboMWM commented Oct 5, 2023

Closes #2064

@RoboMWM RoboMWM added this to the 16.18.2 milestone Oct 5, 2023
@RoboMWM
Copy link
Author

RoboMWM commented Oct 5, 2023

Ideally I'd like to make branches in my own repository, but Github Desktop's UI only allows me to branch off of the top-most repo, even if mine is a fork of a fork... I'll eventually learn how to do it via cli commands

@Jikoo
Copy link
Collaborator

Jikoo commented Oct 5, 2023

Personally for simple git stuff I just use the UI built into my IDE these days - the one in Eclipse worked fine when I used it, and IntelliJ's is pretty nice too. GitHub Desktop doesn't seem to have any concept of heads that are not "origin," and as a result I only use it for pushing to the one GH org that for some unknown reason rejects my key (even though the same key works for everything else I can access).
Only thing I do seem to end up doing regularly via cli is managing tags, because no, I do not want to push every tag made ever so my most recent tag will show up, thank you very much for offering, built in UI. I guess the other "major" hitter is that every once in a while I'll get a little too crazy with a rebase and have to manually fix it.

@RoboMWM
Copy link
Author

RoboMWM commented Oct 5, 2023

Maybe git integration with IDEs has improved, but I just like having something very nice and colorful to show my diffs without the other IDE stuff in the way IMO. I'll prolly try to give it another go, but it's also, in a way, looking at the diff with "a fresh set of eyes" cuz I'm not using my editor I just stared at for an hour.

Anyways I may change to BrushableBlock instead of a switch. Guess that'll make it more future-proof too.

@RoboMWM RoboMWM marked this pull request as draft October 5, 2023 22:54
@RoboMWM
Copy link
Author

RoboMWM commented Oct 5, 2023

This does not work. PlayerInteractEvent is only fired once as long as the player is looking at a block and has not let go of right-click nor has looked away from a block (i.e. targeting air). Thus, the player can look at contigously-connected blocks and brush any block he chooses, and bukkit's events have absolutely no idea what's happening, since no event is fired when a suspicious block has been fully brushed.

@RoboMWM RoboMWM closed this Oct 5, 2023
@Jikoo
Copy link
Collaborator

Jikoo commented Oct 6, 2023

Sounds like we'd have to do PlayerAnimationEvent, assuming the arm swings for the brush. Kinda rough though, there really should be some other event for this.

@QarthO
Copy link

QarthO commented Oct 6, 2023

Sounds like we'd have to do PlayerAnimationEvent, assuming the arm swings for the brush. Kinda rough though, there really should be some other event for this.

Even though the player arm is visually swinging with the brush in hand, the PlayerAnimationEvent is not being fired.
Double checked and no event that is fired apart from the initial PlayerInteractEvent.

I do have a small work around, to tackle this problem. I tested it myself and it looks like it works

public void onPlayerInteractEvent(PlayerInteractEvent event){
    // ... initial null checks
    // ... put checks to make sure they aren't trying to open an inventory or
    //    interact with another block and they just happen to be holding a brush
       
    if(materialInHand == Material.BRUSH){
        if (!(clickedBlock.getState() instanceof BrushableBlock))
            event.setCancelled(true);
    }
}

You are able to cancel the initial PlayerInteractEvent , which prevents the user from moving to a brushable block.
We'd just need to make sure to not cancel the event if they're trying to open a chest or something like that while holding a brush.

@QarthO
Copy link

QarthO commented Oct 6, 2023

Sounds like we'd have to do PlayerAnimationEvent, assuming the arm swings for the brush. Kinda rough though, there really should be some other event for this.

Even though the player arm is visually swinging with the brush in hand, the PlayerAnimationEvent is not being fired. Double checked and no event that is fired apart from the initial PlayerInteractEvent.

I do have a small work around, to tackle this problem. I tested it myself and it looks like it works

public void onPlayerInteractEvent(PlayerInteractEvent event){
    // ... initial null checks
    // ... put checks to make sure they aren't trying to open an inventory or
    //    interact with another block and they just happen to be holding a brush
       
    if(materialInHand == Material.BRUSH){
        if (!(clickedBlock.getState() instanceof BrushableBlock))
            event.setCancelled(true);
    }
}

You are able to cancel the initial PlayerInteractEvent , which prevents the user from moving to a brushable block. We'd just need to make sure to not cancel the event if they're trying to open a chest or something like that while holding a brush.

Actually, technically this won't work. A player could place a suspicious block outside of a claim. Begin brushing it, (since it's outside of a claim we wouldn't want to cancel it) then all while never letting go of right click, move inside the claim and brush "protected" suspicious blocks.

Guess the only answer here is to wait until Spigot correctly handles this event. Or hopefully actually implement proper PlayerBrushEvents

@QarthO
Copy link

QarthO commented Oct 6, 2023

Went ahead and created the issue letting spigot know. tbh, hopefully they just add actual PlayerBrushEvents

https://hub.spigotmc.org/jira/browse/SPIGOT-7501

@RoboMWM RoboMWM deleted the deny-brushing-without-build-trust branch October 14, 2023 17:57
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.

Suspicious Sand and Gravel not protected from brushing mechanic
3 participants