Block unrealistic interactions to avoid getting items from gui

[Tuchan]

Currently, if we use any interaction not including LEFT and RIGHT click, we can essentially get items from the inventory. This probably isn't an intended behavior, but it would be good to add that as a boolean nonetheless. https://github.com/SamJakob/SpiGUI/blob/b57ffc09cdda54433c2dcf520655dc80ae88cfbd/src/main/java/com/samjakob/spigui/SpiGUI.java#L41 Such as: blockUnrealisticInteractions(?), which defaults to true, since 99% of interactions will be handled with either left or right click.

Than it's probably just a case of:

if(clickedGui.areUnrealisticInteractionsBlocked()){
    event.setCancelled(true);
}

You can already achieve the same behavior without that change, but it would be helpful nonetheless:

...
).withListener((InventoryClickEvent e) -> {
    if(e.getClick() == ClickType.LEFT){
        //left click stuff
    } else if (e.getClick() == ClickType.RIGHT) {
        //right click stuff
    } else {
        e.setCancelled(true);
    }
})

Would've made a PR but I would rather give that someone who actually knows what they're doing lmao

[h00z3x]

I think I must add that: you can put items into the gui by just shift-clicking them. (tested on paper version 1.8) I didn't have time to make a pull request or sth (because im lazy and its 12:41 A.M. here) I think something like if (shouldIgnoreInventoryEvent(event.getClickedInventory())) return; and if (!event.isShiftClick()) return; on InventoryClickEvent would work?

I dont know why I didn't create another issue... I hope it helped. Have a good night 🌃

[SamJakob]

@Tuchan - is this in the case of setting setCancelled(false) in the event handler? By default, all InventoryClickEvent or InventoryDragEvent interactions should be blocked, so that shouldn't be possible, if I'm not mistaken?

Note that you needn't set blockDefaultInteraction false to just add click handlers.

@h00z3x - same as above, the event should be cancelled regardless of isShiftClick, so is this with default interactions blocked or allowed?

---

The idea is that blockDefaultInteraction should block unrealistic interactions but you could still manually enable them within the listener after you've checked that they should be enabled (which is expected to be rare, and something most plugins probably wouldn't do). Is there something I'm missing here?

[h00z3x]

@Tuchan - is this in the case of setting setCancelled(false) in the event handler? By default, all InventoryClickEvent or InventoryDragEvent interactions should be blocked, so that shouldn't be possible, if I'm not mistaken?

Note that you needn't set blockDefaultInteraction false to just add click handlers.

@h00z3x - same as above, the event should be cancelled regardless of isShiftClick, so is this with default interactions blocked or allowed?

The idea is that blockDefaultInteraction should block unrealistic interactions but you could still manually enable them within the listener after you've checked that they should be enabled (which is expected to be rare, and something most plugins probably wouldn't do). Is there something I'm missing here?

First of all, Thanks for your response I really appreciate it! Second, It looks like I didn't provide much Information. Also I thought It wasn't possible, when I saw your response... So I took a look at SGMenuListener.java: It turns out the inventory given to shouldIgnoreInventoryEvent (at line 88) is the clicked inventory (which was the player's inventory in my case) it would ignore the event and return while the menu open is event.getinventory.

To fix it : just change the https://github.com/SamJakob/SpiGUI/blob/ed41ea1a99ede7df4ffffb3720c784d84f6eed51/src/main/java/com/samjakob/spigui/menu/SGMenuListener.java#L88

to if (shouldIgnoreInventoryEvent(event.Inventory())) return;

and also i think you might want to change the line 91 https://github.com/SamJakob/SpiGUI/blob/ed41ea1a99ede7df4ffffb3720c784d84f6eed51/src/main/java/com/samjakob/spigui/menu/SGMenuListener.java#L91

to SGMenu clickedGui = (SGMenu) event.getInventory().getHolder();

Note: this would also mean that the player cant move items of his own when the GUI is open! (Sorry for responsing so late)

[SamJakob]

Okay I’m following what you’re saying now.

I suspect what @Tuchan is suggesting is possible is double clicking in the player inventory to steal the items from the other (SpiGUI) inventory (as that could be possible as a result of the same issue) so I’ll work under the assumption that both of these are about the lower inventory (unless this is also an issue with events in just the upper inventory - in which case if someone could post a video to clarify).

With respect to the problem you’ve outlined, it looks like this might have been introduced in #19 which fixed the problem it intended to fix but introduced this one (which was trying to stop click events in the player’s inventory from registering as being the SpiGUI inventory). (Honestly the way the Bukkit/Spigot API does inventories seems weird).

So, I think what needs to be introduced is one of the following:

• a new boolean for blocking events in the player inventory (as some plugins could reasonably need or want to allow the player to rearrange items in their own inventory)
• an enum to set the ‘level’ to which things should be blocked (say something like, blockEverything, blockPlayerInventory, allowEverything) (might be overdoing it but depends on how much flexibility is needed)
• automatically block cross-inventory interactions (might be difficult to implement correctly because some events could be missed and could maybe be a pain to override if that functionality is required)
• automatically block player inventory events if the existing block default interactions is set (probably most ideal and plugins can just override it themselves for additional functionality anyhow)

I’m happy to hear your thoughts otherwise I’ll probably just go with the last one.

[Tuchan]

Tbh I don't remember much other than that I was able to get the items from the inventory. For example, instead of left clicking I did a shift click and the action obviously went through but the item stayed in my inventory. I think the item would disappear when you interacted with it (placing a block etc), but that was easily bypassed by just dropping and picking it back.

99% of the time an item inside a GUI will be used as a button and not as an item that you can grab and put in your inventory. So yeah, the last option is probably the best one.

[SamJakob]

Okay. It's probably a 'phantom/ghost item' in most cases (e.g., the client might make it look like the user obtained the item but the event was cancelled on the server so it's not actually there) but I guess there are some workarounds possible.

I'll try and find some time to just try and break it in every way I can think of and implement the above changes.