Right click on door without item in hand

Hello everyone,
I recently try to make a plugin for claim doors by Rightclicking.
But the problem is : The door can open when the player haven’t got item in hand. I tryed all I can but nothing works.
Please help
Here is the code :

String owners = "";

@Listener
public void onRightClickOnBlock(InteractBlockEvent.Secondary event) {
	Player player = event.getCause().last(Player.class).get();
	BlockSnapshot block = event.getTargetBlock();
Optional<EconomyService> serviceOpt = Sponge.getServiceManager().provide(EconomyService.class);
	EconomyService economyService = serviceOpt.get();
	
	// Système de claim des portes    	
	if(block.toString().contains("blockState=minecraft:dark_oak_door")) {
		final int x = block.getPosition().getX();
		final int z = block.getPosition().getZ();
		
		if(owners.contains("| " + x + " " + z + " " + player.getName() + " |")) {
        	event.setCancelled(false);
		} else {
    		if(isLockPick(player.getItemInHand(HandTypes.MAIN_HAND).orElse(null))) {
        		event.setCancelled(false);
        		player.sendMessage(Text.of(TextColors.GREEN, "Vous avez ouvert la porte avec un Lock Pick !"));
    		} else {
    			event.setCancelled(true);
        		player.sendMessage(Text.of(TextColors.RED, "Cette porte ne vous appartient pas !"));
    		}
		}
		
		if(player.getItemInHand(HandTypes.MAIN_HAND).equals(null)) {
    		if(owners.contains("| " + x + " " + z + " " + player.getName() + " |")) {
            	event.setCancelled(false);
    		} else {
    			event.setCancelled(true);
    		}
		}
		
		if(isIdCard(player.getItemInHand(HandTypes.MAIN_HAND).orElse(null))) {
    		if(owners.contains("| " + x + " " + z + " " + player.getName() + " |")) {
            	event.setCancelled(false);
    		}
            else if(owners.contains("| " + x + " " + z + " ")) {
            	event.setCancelled(true);
            	player.sendMessage(Text.of(TextColors.RED, "Cette porte ne vous appartient pas !"));
            } else {
    			Optional<UniqueAccount> uOpt = economyService.getOrCreateAccount(player.getUniqueId());
				if (uOpt.isPresent()) {
				    UniqueAccount acc = uOpt.get();
				    BigDecimal balance = acc.getBalance(economyService.getDefaultCurrency());
				    
					if(balance.intValue() >= 200) {
						Sponge.getCommandManager().process(Sponge.getServer().getConsole(), "econ remove " + player.getName() + " 1000");
						player.sendMessage(Text.of(TextColors.GREEN, "Vous avez acheté cette propriété pour 1000 euros ! Elle vous appartient désormais !"));
						owners = owners + "| " + x + " " + z + " " + player.getName() + " |";
					}
					else {
						player.sendMessage(Text.of(TextColors.RED, "Vous n'avez pas assez d'argent pour acheter cette propriété !"));
					}
				}
                          }
		           }
	}
}


public boolean isLockPick(ItemStack stack)
{
	ItemType type = stack.getItem();
	return type.equals(ItemTypes.FLINT);
}

public boolean isIdCard(ItemStack stack)
{
	ItemType type = stack.getItem();
	return type.equals(ItemTypes.STRING);
}

Try debugging the program you may find out why its not working

System.out.println("\n code is running here");

There’s a lot of other problems in your code that may be masking the actual issue or even causing it.

  • InteractBlockEvent.Secondary fires once for each HandType. If you only want the main hand, use InteractBlockEvent.Secondary.MainHand.
  • Getting the Player from the cause is much easier done using event filters.
  • Obtaining the last Player may not always be the player you want. @First should be your default choice, or @Root if you know the Player is the main cause.
  • You save the EconomyService as an optional, but then immediately #get it. If the server doesn’t have an economy plugin installed this will throw a NSEE.
  • Relying on the #toString implementation is really bad. You should be using Keys to check for a door with a dark oak variant. More info on block data.
  • Likewise, having owners be a String is not helping you. You can save data to the block directory by creating your own Key (preferable), or maintain some type of registry on your own.
  • Additionally, player.getName() will change if the player’s name changes. For any type of persistent data with players, use player.getUniqueId().
  • Calling event.setCancelled(false) may overwrite a plugin that has already canceled the event for another reason, such as a protection plugin.
  • If the player is not holding an item in their main hand, #isLockPick will throw an NPE from ItemStack#getItem You can check if the item is present first or use .orElse(ItemStack.empty()). For something a bit more advanced, you can also use #map.
  • As above, don’t un-cancel the event here
  • Player#getItemInHand never returns null, it returns an Optional.
  • Once again, avoid strings and un-canceling the event. Last I’ll mention of these two ^^
  • If you’re going to make multiple checks to the players held it, store it.
  • Same with #isLockPick, #isIdCardwill throw aNPE` if there’s no item.
  • Review the docs for the Economy API, especially the best practices. Don’t handle the calculations of the balance yourself and leave it up to the implementing economy plugin.
  • Also, BigDecimal#intValue() can do a lot of freaky things if the value is out of bounds for an int and can even give negative results.
  • Processing a command is really only appropriate when you’re working with user input. This is dependent on the economy plugin and version as the command syntax may change.
  • For both of your custom methods, this can easily be done with stack.getItem() == ItemTypes.FLINT as there should only be a single registry for each type.

For future notes, always include log printouts wherever possible as things like the event’s data and any exceptions throws can easily narrow down the issue.

2 Likes

The problem is ItemStack.empty() don’t exist in API 5.

private static final ItemStack EMPTY = ItemStack.of(ItemTypes.NONE, 0);

Though as mentioned, there are many other ways to go about doing that. One way is just to check the optional, which should already be saved anyways.

if (optItem.isPresent() && isLockPick(optItem.get())

I personally would just Optional#map which handles some of that logic for me.

if (optItem.map(this::isLockPick).orElse(false))

//Basically, if optItem is present it will return an Optional<Boolean>
//of the result of isLockPick(optItem.get()). Then orElse(false) returns
//the result of #isLockPick or else false.

Just because one option isn’t available doesn’t mean there isn’t others that could be used instead. In this case, I prefer both of these variants to using ItemStack.empty() anyways.

Got an error when server starts, this is the cause :
Caused by: java.lang.IllegalArgumentException: Quantity must be greater than 0

There’s a difference in the implementation between API 5 and API 7 with relation to the quantity that I was unaware of. If you really want to use that method, ItemStack.builder().itemType(ItemTypes.NONE).build() should work fine - it sets the quantity to 0 by default and avoids the precondition. However, I still prefer one of the other methods.

False, actually. Because by default this listener won’t be called if the event was already cancelled. Although it still doesn’t do anything, so it should still be removed.

1 Like

I tryed :

private static final ItemStack EMPTY = ItemStack.builder().itemType(ItemTypes.NONE).build();

@Listener
public void onRightClickOnBlock(InteractBlockEvent.Secondary.MainHand event) {
    Player player = event.getCause().last(Player.class).get();
    BlockSnapshot block = event.getTargetBlock();

    if(isThisItem(player.getItemInHand(HandTypes.MAIN_HAND).orElse(EMPTY))) {
            // do stuff
    } else {
           event.setCancelled(true);
    }
}

But it’s not working too, please help

That’s not nearly enough to go on. Show logs, full code segments, a print of the event, and demonstrate what’s happening and what you expect to happen.

@pie_flavor Good to know - I’ll have to remember that for the future.

I tried this for cancel interact with the block only there are no item in hand of player with the tip you give me :
private static final ItemStack EMPTY = ItemStack.builder().itemType(ItemTypes.NONE).build();
but don’t detect

I’m not understanding what you’re saying. The use of an empty ItemStack has absolutely no effect on whether the player is holding an item or not. Add some log messages into your code to see what’s going on and print out information about the event where necessary.

I just need to find how to do something when the player rightclick with no item in hand.

If they don’t have an item in hand, then player.getItemInHand(HandTypes.MAIN_HAND).isPresent() will be false.

Hmm that’s really weird, I’ve try to player.sendMessage(Text.of(player.getItemInHand(HandTypes.MAIN_HAND).isPresent()))
It’s send me true when I have an item in hand but when I haven’t an item, don’t send any message, and this is the same for Gravity Gun (item from a mod).

When I try t sendMessage just after the event declaration, the message is sent only if I rightclick in air

So ?

Blockquote

OK, the only problem I’ve now is for the hand without item : it’s not working.
For the Gravity Gun I’ve find myself.

As I’ve said, if you cannot show the code and log output of your program there is no way I can help you. Repeating generic ‘it doesn’t work’ or ‘help me’ statements is not going to address the problem or help in the slightest.

Just before, have you an idea on how to change if(block.toString().contains("blockState=minecraft:dark_oak_door")) by something best.