Invalid SlotTransactions not properly cancelling

[BenWoodworth]

Issue

Marking a SlotTransaction as invalid is not properly cancelling the transaction.

Observed

Invalidate all the server's SlotTransactions:

@Listener
fun onAffectSlot(event: AffectSlotEvent) {
    event.transactions.forEach { it.isValid = false }
}

Open an inventory with a single item in it. If the item is left clicked, the item remains in the inventory, but the cursor will have a copy. Clicking on any slot with the item on the cursor, the item will be removed from the cursor, with the slot remaining unchanged. Clicking outside the inventory with the item on the cursor, the item is dropped in front of the player.

Expected

When clicking on a slot, both the cursor and the slot should remain unchanged; not just the slot.

Details

SpongeVanilla: 1.12-7.0.0-BETA-300 Java: 1.12-7.0.0-BETA-300 Ubuntu: 16.04

Nothing noteworthy in the server log, and no other plugins installed.

Discussed briefly in a Sponge Forum post: here

[BenWoodworth]

This issue still remains when cancelling dragged slot transactions.

I have a top inventory open which cancels all of its SlotTransactions. If I have items on my cursor, and drag with the secondary mouse button (right click, leaves one item in each dragged slot), dragging into the top inventory will cancel the transaction without putting the items back on the cursor.

E.g. if I had 3 items on the cursor, dragged 1 on the bottom inventory, 1 on the top, and left 1 on the cursor, the item in the bottom inventory will remain, the item in the top inventory will be removed, and I will be left with 1 item on the cursor.

Sponge Vanilla version: 1.12.1-7.0.0-BETA-311

[BenWoodworth]

It also appears to be present when double clicking in some situations. (Same inventory setup, and same Sponge version)

http://imgur.com/a/Hlp7x The asterisk* in the chat indicates that the top diamonds had an AffectSlotEvent cancelled.

With two diamonds in the top inventory and two in the bottom inventory (pic 1), double clicking the bottom diamonds triggers an AffectSlotEvent on the top diamond stack, and leaves two diamonds in the top inventory, and two on the cursor (pic 2). This is all expected.

With two diamonds on the top, and two stacks of 1 diamond on the bottom (pic 3). Double clicking the right diamond on the bottom triggers an AffectSlotEvent on the top diamond stack, and leaves two diamonds on the top, and 4 diamonds on the cursor (pic 4). The 2 diamonds from the top were left in their slot, and added to the cursor.

[BenWoodworth]

@Faithcaio @bloodmc I believe this issue should be re-opened, because the issue still remains when dragging and double clicking items in the inventory, as described in the two previous comments.

[bloodmc]

@BenWoodworth The code in first post was fixed. I cannot reproduce any issues when invalidating all transactions. How are you getting items onto your cursor when the code is invalidating ALL transactions? You need to open another ticket if you are going to be changing code and providing different scenarios. Also, your screenshot shows "FastCraft". I have no idea what plugin/mod this is but you need to test using a standard vanilla chest.

Also, make sure you are testing in Survival Mode as Creative is limited.

[BenWoodworth]

@bloodmc It's a plugin I'm working on, and I'm dabbling with creating a GUI. And in the dragging and double clicking examples, I'm only cancelling the AffectSlotEvents for the slots in the upper inventory.

Here's the code I'm using. Basically, the inventory is built with a carrier, and the title "FastCraft", and the AffectSlotEvent listener cancels if it's a slot in the top inventory.

Code for building the inventory in the examples:

Inventory.builder()
        .of(InventoryArchetypes.CHEST)
        .property(
                InventoryDimension.PROPERTY_NAME,
                InventoryDimension(width, height)
        )
        .property(
                InventoryTitle.PROPERTY_NAME,
                InventoryTitle(title) // 'title' is FastCraft in the examples
        )
        .withCarrier(this) // 'this' is of the type Gui
        .build(plugin)

Code for cancelling: isGuiSlot returns true only for the upper inventory slots in those examples. It's checking that the containing inventory's carrier is a Gui object.

private fun isGuiSlot(slot: Slot): Boolean {
    val parent = slot.parent() as? CarriedInventory<*> ?: return false
    val carrier = parent.carrier.orElse(null) ?: return false

    val slotIndex = slot.getProperty(SlotIndex::class.java, "slotindex")
            .map(SlotIndex::getValue)
            .orElse(null)

    return slotIndex in 0 until carrier.inventory.capacity()
}

@Sponge_Listener(order = Order.EARLY)
fun onAffectSlot(event: AffectSlotEvent) {
    event.transactions
            .filter { isGuiSlot(it.slot) } // True only for upper inventory slots in the examples
            .forEach { it.isValid = false } // Invalidate the Gui slot transactions
}

[bloodmc]

OK i'll look into this when I get more time.

[Faithcaio]

Restoring items to the cursor when invalidating slottransactions makes no sense. Events that also affect the cursor have a cursor transaction InteractInventoryEvent#getCursorTranssaction which you can modify.

Canceling the entire event will result in the cursor being restored as well.

If you only want to partially cancel dragging items into bottom/top inventory you would have to modify the cursor transaction accordingly yourself.