Custom Inventories

I’ve managed to setup a custom inventory and have actions executed when an item is clicked successfully but I have a few issues:
- The item that is clicked (sometimes) ends up in the users inventory or is dropped.
- If the action includes clearing the custom inventory, the clicked item is still there.

I’m presuming most of this is due to my Listener so here it is, if you need any more code let me know.

public class InventoryEventListener extends OCListener {

    public InventoryEventListener(OblivionCore plugin) {
        super(plugin);
    }

    @Listener
    @Exclude({ClickInventoryEvent.Open.class, ClickInventoryEvent.Close.class, ClickInventoryEvent.Primary.class})
    public void inventoryEventListener(ClickInventoryEvent event) {
        if (!(event instanceof Cancellable)) return;

        Cancellable cancellableEvent = event;
        boolean cancelled = cancellableEvent.isCancelled();

        for (Inventory slot : event.getTargetInventory().slots()) {
            if (!slot.peek().isPresent()) return;
            if (!slot.peek().get().get(MenuButtonIDData.class).isPresent()) return;

            if (!event.getTargetInventory().getClass().getSimpleName().equals("CustomInventory")) {
                slot.peek().get().setQuantity(0);
            }

            cancelled = true;
            break;
        }

        cancellableEvent.setCancelled(cancelled);
    }

    @Listener
    public void inventoryClickListener(ClickInventoryEvent.Primary event) {
        ItemStackSnapshot clickedItem = event.getCursorTransaction().getFinal();

        if (!(event.getCause().containsType(Player.class) && clickedItem.get(ImmutableMenuButtonIDData.class).isPresent())) {
            return;
        }

        Player player = event.getCause().first(Player.class).get();
        UUID buttonID = clickedItem.get(ImmutableMenuButtonIDData.class).get().getButtonID();
        Optional<MenuItem> optionalMenuItem = plugin.getCompositionRoot().getMenuService().getMenuItem(buttonID);
        if (!optionalMenuItem.isPresent()) {
            event.getTransactions().forEach(slotTransaction -> slotTransaction.setValid(false));
            event.getCursorTransaction().setValid(false);
            event.setCancelled(true);
            return;
        }

        Optional<MenuSession> oMenuSession = plugin.getCompositionRoot().getMenuService().getMenuSession(player.getUniqueId());
        if (!oMenuSession.isPresent()) {
            if (!player.getOpenInventory().isPresent()) return;

            //TODO Handle non-existant menu sessions

            event.getTransactions().forEach(slotTransaction -> slotTransaction.setValid(false));
            event.getCursorTransaction().setValid(false);
            event.setCancelled(true);
            return;
        }

        Optional<Menu> oMenu = plugin.getCompositionRoot().getMenuService().getMenu(oMenuSession.get().getActiveMenuClass());
        if (oMenu.isPresent()) {
            optionalMenuItem.get().handle(new MenuItemClickEventArgs(optionalMenuItem.get(), oMenu.get(), player));

            event.getTransactions().forEach(slotTransaction -> slotTransaction.setValid(false));
            event.getCursorTransaction().setValid(false);
            event.setCancelled(true);
            return;
        }
    }

    @Listener
    public void inventoryCloseListener(InteractInventoryEvent.Close event) {
        if (!event.getCause().first(Player.class).isPresent()) {
            return;
        }

        Player player = event.getCause().first(Player.class).get();
        if (plugin.getCompositionRoot().getMenuService().getMenuSession(player.getUniqueId()).isPresent()) {
            plugin.getCompositionRoot().getMenuService().removeMenuSession(player.getUniqueId());
        }
    }

    @Listener
    public void clientDisconnectListener(ClientConnectionEvent.Disconnect event) {
        Optional<Player> opPlayer = event.getCause().first(Player.class);
        if (!opPlayer.isPresent()) {
            return;
        }
        UUID uuid = opPlayer.get().getUniqueId();

        plugin.getCompositionRoot().getMenuService().removeMenuSession(uuid);
    }

Well, for starters, your listeners don’t get the event if it’s cancelled.
Second, I’m not sure why you store the cancelled state in a boolean if you never use it and then reassign it later (in fact, your IDE probably gave you a warning if you haven’t disabled that yet).
Third, you can use a filter argument in your listeners (i.e. @First Player p).

And what you’re describing is likely a component of the implementation, not your code. I’ve observed this bug before; if I’m right, you’ll notice this behavior even with a simple listener that cancels all inventory click events that are cancellable.

The only listener that needs to act on the events is the second ‘inventoryClickListener’. It listens to ‘ClickInventoryEvent.Primary’ which is excluded from the first listener that cancels all events if the inventory is a custom inventory (To prevent unintended interaction) created by my plugin, so that is not an issue as my listeners do get the event.

The cancelled variable is used, its initially set to the current cancelled state and if the inventory contains menu data the cancelled variable is set to true, on the final line of that event the cancelled state of the event is set based on the cancelled variable. I guess I could just set it in the loop instead but the outcome is essentially the same.

Thanks for telling me about the filters, I always forget they exist and go about it by checking the cause instead.

Is there an open issue if its an issue with the implementation?

Not that I know of.