AnvilGUI icon indicating copy to clipboard operation
AnvilGUI copied to clipboard

Bypass Item Canel

Open RootException opened this issue 1 year ago • 8 comments

Its possible to take out the items by clicking on it with an not null item cursor.

interactableSlots() none Only async click used.

Version 1.20.4

Video of the problem :

https://github.com/user-attachments/assets/8e488d06-f5e2-4979-831f-514522e267fb

RootException avatar Jul 14 '24 16:07 RootException

Can you state a minimal code example for reproduction? Also the exact server version + mc client version.

mastercake10 avatar Jul 19 '24 00:07 mastercake10

Sure!

`new AnvilGUI.Builder() .onClickAsync((slot, stateSnapshot) -> CompletableFuture.supplyAsync(() -> { if (slot == AnvilGUI.Slot.OUTPUT) { bukkitPlayer.removeMetaData("selling");

                    bukkitPlayer.addEco("DEFAULT", price, "ITEM-SELL");
                    ScoreboardService.getInstance().updateMoney(bukkitPlayer);
                    bukkitPlayer.sendLangMessage("cb_messages", "cb.sell.sold",
                            new Replacement("%DOLLAR%", "" + price));
                    bukkitPlayer.playSound(Sound.ENTITY_PLAYER_LEVELUP, 1, 5);
                }
                return List.of(AnvilGUI.ResponseAction.close());
            }))
            .onClose(stateSnapshot -> {
                if(bukkitPlayer.hasMetaData("selling")) {
                    bukkitPlayer.removeMetaData("selling");
                    acceptedItems.forEach(bukkitPlayer::addItem);
                }
            })
            .text("Our Offer: " + price + "$")
            .itemRight(this.cancelItem)
            .itemLeft(this.invisibleItem)
            .itemOutput(this.finishItem)
            .title(":offset_-70::input-dollar-gui:")
            .plugin(CityBuild.getInstance())
            .open(bukkitPlayer.toBukkitPlayer());`

RootException avatar Jul 19 '24 07:07 RootException

bump

Server: 1.20.4 Client: 1.20.4 & 1.21

RootException avatar Jul 27 '24 15:07 RootException

Please try again with the latest version of AnvilGUI, where #339 has been fixed. I'm not sure if it's related to this issue, but it doesn't hurt to try it and see. I tried to reproduce the bug you're describing but was unable to, probably because I can't recreate the same environment. But I do have a few questions that might help us investigate the problem:

  • Why are you using onClickAsync instead of onClick here? The Bukkit API generally does not support asynchronous usage from other threads, and it doesn't look like you're doing anything that would benefit from running async. Can you try switching to onClick instead? That's probably a good idea regardless of whether it solves this problem or not.
  • Can you confirm whether the close listener gets called or not, when the problem occurs? For example, using a debugger or by logging something to the console.
  • It's unclear to me what the exact steps to reproduce the bug are when clicking around in the GUI. Can you provide a step-by-step list? It's hard to tell from the video if the order is significant, if the stack size is significant, whether you're shift- or drag-clicking etc.
  • Is the complete source code available somewhere? Right now we only have a part of it, but more context may be needed.

0dinD avatar Aug 18 '24 16:08 0dinD

This is the current code. Its possible to shift-left click the items. Then you receive them in the inventory. This is the only implementation of anvil gui. There is not matter if onClick or onClickAsync.

Step by Step list:

  1. Player wants to rename a value
  2. Anvil Gui opens
  3. Normal click will be canceled 3.5) Shift Left click on item right or item left will not cancled
  4. on Click Action is executed
  5. The player receive the item in the inventory if he shit clicks

onClose is called only once. I use the newest version.

new AnvilGUI.Builder() .onClose(stateSnapshot -> inventory.get().open(clickPlayer))

                    .onClick((slot, stateSnapshot) -> {
                        if(slot != AnvilGUI.Slot.OUTPUT) {
                            return Collections.emptyList();
                        }
                        String homeId = stateSnapshot.getText();
                        if(homeId.isEmpty()) {
                            bukkitPlayer.playSound(Sound.ENTITY_BAT_TAKEOFF, 1, 5);
                            return List.of(AnvilGUI.ResponseAction.replaceInputText("Enter homeId"));
                        }
                        if(homeId.length() > 255) {
                            bukkitPlayer.playSound(Sound.ENTITY_BAT_TAKEOFF, 1, 5);
                            return List.of(AnvilGUI.ResponseAction.replaceInputText("Too long"));
                        }
                        CoreAPI.threadService.execute(() ->
                                SetHomeCommand.setHome(clickPlayer, homeId));
                        return List.of(AnvilGUI.ResponseAction.close());
                    })
                    .itemRight(this.invisibleItem)
                    .itemLeft(this.invisibleItem)
                    .itemOutput(this.finishItem)
                    .title(":offset_-70::input-gui:")
                    .plugin(CityBuild.getInstance())
                    .open(bukkitPlayer.toBukkitPlayer());

RootException avatar Sep 06 '24 17:09 RootException

#bump

RootException avatar Sep 29 '24 19:09 RootException

I was meaning to take a look at this but got busy, will get back to you when I've tried to reproduce using your instructions, hopefully in a few days.

0dinD avatar Oct 02 '24 19:10 0dinD

@RootException I have now tried to reproduce the issue using your instructions, but was unable to do so (everything worked as expected for me, I was not able to move items to my inventory, even with shift-click).

I do have a theory though, which I can't confirm because you haven't shown all of your code (please confirm it or share more code). In your code for the menu that opens AnvilGUI (the chest GUI), you must have a InventoryClickEvent listener somewhere, right? I assume that click listener might be directly calling new AnvilGUI.Builder().open(...)? If so, that would explain your issue. Opening an inventory from an InventoryClickEvent, InventoryCloseEvent etc. will cause issues, as is documented here:

  • https://hub.spigotmc.org/javadocs/spigot/org/bukkit/event/inventory/InventoryClickEvent.html
  • https://jd.papermc.io/paper/1.21.1/org/bukkit/event/inventory/InventoryCloseEvent.html

Because InventoryClickEvent occurs within a modification of the Inventory, not all Inventory related methods are safe to use.

The following should never be invoked by an EventHandler for InventoryClickEvent using the HumanEntity or InventoryView associated with this event:

To invoke one of these methods, schedule a task using BukkitScheduler.runTask(Plugin, Runnable), which will run the task on the next tick. Also be aware that this is not an exhaustive list, and other methods could potentially create issues as well.

Also note that the same issue will occur if you open any other inventory from AnvilGUI's onClick or onClose. So this line in the code that you shared will also cause problems (in addition to your InventoryClickEvent handler):

.onClose(stateSnapshot -> inventory.get().open(clickPlayer))

A similar issue to yours (with the same reason: they didn't schedule a task for opening the GUI) has been discussed in #348.

So in conclusion, whenever you open an inventory from an inventory event listener, you need to wrap it in a call to Bukkit.getScheduler().runTask. Let me know if this answers your questions.

0dinD avatar Oct 24 '24 17:10 0dinD