[Solved] BlockBreakEvent calling

Question: Why would Call 2 improperly break a block in a restricted area when Call 1 obeys area restrictions?

Background:
I'm working on my plugin ToolBelt: http://dev.bukkit.org/server-mods/toolbelt/

One of the plug-in tools (PickHax) allows the player to delete a block while holding a defined material (default DIAMOND_PICKAXE). To provide the widest possible support for region protection (like WorldGuard) and logging (like HawkEye), I make and call a BlockBreakEvent. Below is the code that does this:

Code:

        protected boolean safeBreak(Block target, Player subject, boolean applyPhysics) {
                BlockBreakEvent canBreak = new BlockBreakEvent(target,subject);
                server.getPluginManager().callEvent(canBreak);
                if(!canBreak.isCancelled())
                        target.setTypeId(0, applyPhysics);
                return !canBreak.isCancelled();
        }

With the current release (0.1) of ToolBelt, this works quite well, and doesn't remove a block if the player doesn't have permission for that region of the server (WorldGuard). I simply set the target Block to the event.getClickedBlock(), the subject Player to event.getPlayer(), and the physics Boolean to true if they left-clicked, and false if they right-clicked. Here is the call in version 0.1:

Call 1

Code:

safeBreak(target,event.getPlayer(),physics);

I'm currently working on a new version that supports ranged block deletion. Depending of whether the user is crouching and has the range permission node, the target Block is set by event.getClickedBlock() or subject.getTargetBlock(null,25). This works fine except for in one case.

• case 1: If they actually clicked on the block (*_CLICK_BLOCK), they see the change, with either physics settings.

• case 2: If they acquired the block at range, and have physics to true (LEFT_CLICK_AIR), then they see the change.
• case 3: However, if they acquired the block at range, and have physics to false (RIGHT_CLICK_AIR), then they do not see a block update on the client. If they log off and back on, the change is visible, so the change does happen server side.

To alleviate this issue, I tried to use subject.sendBlockChange(), which is supposed to send a fake block change to the user, and not modify the server in any way. With this the user always sees what they have done. Thus the new code:

Call 2

Code:

if(safeBreak(target,event.getPlayer(),physics))
    subject.sendBlockChange(target.getLocation(), 0, (byte)0);

However, for some reason, the change between just running safeBreak() and running if(safeBreak()) sendBlockChange() makes it so that the region protection (WorldGuard) is not respected. The user gets a printout from WorldGuard still warning them they don't have build rights, but the bock still gets broken.

This brings me back to the original question. I can recompile the ToolBelt.jar with the only difference being Call 1 vs Call 2 and reproduce the issue reliably. If Call 1 is in place, region support is respected, but case 3 doesn't update the client. If Call 2 is in place, the client always gets the block update, but they can remove blocks in any region.

I can route around the issue by not allowing case 3 (no physics removal at range) and using Call 1, however I would like not to compromise the tools ability. Does anyone have any idea why Call 2 doesn't work with regions when Call 1 does without issue.

 

mayby you chhanced some code from safebreak()?

 

With Call 2 after re-logging, is the change still there?

 

I have tested this by compiling the jar with the only two lines changed being Case 1 and Case 2. No other changes at all. The issue still persists.

Yes, if Case 2 is compiled an run, and you click in a region you don't have access to, you get a warning about not having permission, but the block still disappears. If you log out and log back in, the block you removed is still gone. Even after closing your client and then starting back up, the block is still gone, showing that the server did actually record the block being removed.

 

I have a feeling that "safebreak()" always returns true. Which in turn causes "subject.sendBlockChange()" to always be triggered. And perhaps causing the actual block update server side aswell.

 

I have now tested what I call Call 2.1

Code:

if(safeBreak(target,event.getPlayer(),physics))
    subject.sendMessage("Error still present?");

If I click in a non-restricted zone, I get the message "Error still present?". However, if I click in a restricted zone, I do not get that message. This means that safeBreak is returning true if non-restricted and false if restricted. Thus .sendBlockChange() will only be triggered in a non-restricted zone

 

In Case 2.1 I take the block still disappeared?

 

Yes, the clicked block was still removed from the server (and client for that matter). Also, case 3 still showed up as a problem because of no-longer running .sendBlockChange().

 

In that case the error would be in safeBreak() as it always causes the block to be broken...

 

No. In Call 1 regions are perfectly supported. The block is not always broken, but only when you are in a region that you have support for. In Call 1, in a region you can break blocks, the block is broken. In Call 1, in a region you do not build rights for, the block remains unbroken.

 

However as soon as you use it in an "if" statement the block does get broken, even if the user is not allowed. So I think it would be in safeBreak() somewhere, OR one of those really annoying unexplainable came from out of nowhere whilst always worked properly bugs that computers sometimes throw at us

 

Ok, I modified the function so that I could receive more debugging information.

Code:

        protected boolean safeBreak(Block target, Player subject, boolean applyPhysics) {
                BlockBreakEvent canBreak = new BlockBreakEvent(target,subject);
                server.getPluginManager().callEvent(canBreak);
                if(!canBreak.isCancelled())
                        subject.sendMessage("I break block!");
                return !canBreak.isCancelled();
        }

and then ran it with Call 1 and Call 2.1.

With Call 1 I got the "I break block!" message outside of restricted areas, and nothing inside restricted areas (no blocks actually broken because of no .setTypeID(0))

With Call 2.1 I got the "I break block!" and "Error still present?" messages outside of restricted areas, and no messages inside of the restricted area. Outside restricted areas no blocks broken, Inside restricted area blocks were broken. So somehow something is breaking the block for me inside the restricted area. I have no idea what could possibly be causing the block break.

 

The only thing I can think of is that somehow WorldGuard does not actually cancel the event. Or another plugin is interfering and setting the blockBreak to allowed again...

 

The issue was at a level higher then I posted. Sorry for not including enough data. The original Case 1 call was:

Code:

if(isUseEvent())
    safeBreak(target,event.getPlayer(),physics);
else {
    target.setTypeId(0,physics);
    subject.sendBlockChange(target.getLocation(), 0, (byte)0);
}

The Case 2 call was

Code:

if(isUseEvent())
    if(safeBreak(target,event.getPlayer(),physics))
        subject.sendBlockChange(target.getLocation(),0,(byte)0);
else{
    target.setTypeId(0,physics);
    subject.sendBlockChange(target.getLocation(),0,(byte)0);
}

However, because I didn't wrap the if(safeBreak()) doStuff; in {}, it meant that the else lined up with the innermost if() statement. Thus proper formatting would show:

Code:

if(isUseEvent())
    if(safeBreak(target,event.getPlayer(),physics))
        subject.sendBlockChange(target.getLocation(),0,(byte)0);
    else{
        target.setTypeId(0,physics);
        subject.sendBlockChange(target.getLocation(),0,(byte)0);
    }

This can be simply resolved by not taking a stupid shortcut and properly wrapping my if() {doStuff;}

Code:

if(isUseEvent()){
    if(safeBreak(target,event.getPlayer(),physics)){
        subject.sendBlockChange(target.getLocation(),0,(byte)0);
    }
}else{
    target.setTypeId(0,physics);
    subject.sendBlockChange(target.getLocation(),0,(byte)0);
}

 

Haha, yea that's the niceness of coding! and that is also why it is best practice (IMHO) to use brackets as soon as you need an else.

 

I can understand why the client might see the update (I assume it isn't perfectly synchronous?) but the server should definitely not be jumping the gun.

EDIT:

Wait really? Not using curly braces for an else caused this? Never seen that be a problem in another language before, although I do make a habit of always using them.

I see, that should still not be a problem, no curly brace if finished with a ;, and then an else with curly braces, it should definitely know what is going on.