How can checking the return of a java function change the action the function performs?

Question

Why would the function safeBreak() perform as expected in Call 1, but not in Call 2 or Call 2.1?

I'm working on my Minecraft Bukkit plugin ToolBelt.

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

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

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 (LEFT_CLICK_BLOCK / RIGHT_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

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.

Just in-case the description for .sendBlockChange() was not accurate about it not actually changing the world in any way, I have also tried it with a "harmless" print message

Call 2.1

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

When I run Call 2.1, if I am in a region where I have permission to break blocks, I get the "Error still present?" message, but when I'm in a restricted area, I do not. This means that safeBreak() is returning the correct value for if I should have broken a block or not, but somehow is still breaking the block. Also, because it doesn't have .sendBlockChange() of Call 2, I get the case 3 issue again.

Call 3

if(safeBreak(target,event.getPlayer(),physics)) {}

Now the code has no action other then checking the return value of safeBreak(), and yet it still breaks blocks in the restricted area. I switched to Call 1 and it didn't break the blocks in the restricted area.

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 and Call 2.1 don't work with regions when Call 1 does without issue?

Documentation links of interest:

• Block : Represents a block. This is a live object, and only one Block may exist for any given location in a world.

• Player : Represents a player, connected or not

• .isCanceled() : Gets the cancellation state of this event. A canceled event will not be executed in the server, but will still pass to other plugins

• .sendBlockChange() : Send a block change. This fakes a block change packet for a user at a certain location. This will not actually change the world in any way.

Answer 1

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

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

The Case 2 call was

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:

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;}

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);
}

Answer 2

Checking an l-value has no effect on the code which generated the l-value. This means the code in the "then" section of your if statement is likely the culprit.

Depending on how the third-party code you're integrating with is designed, things like exceptions might be causing rollback-like behavior.

try using a simple statement in your if block (rather than the "harmless print message") and you should be able to validate this yourself:

if(safeBreak(target,event.getPlayer(),physics))
{
}