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 Key
s 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 a
NPE` 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
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.