Partially revert previous commit and implement hotbar swap actions

1.16.5 does not cooperate well when an action is invalid, and this breaks hard when crafting.

core/src/main/java/org/geysermc/geyser/inventory/click/Click.java

@@ -38,7 +38,16 @@ public enum Click {
     DROP_ONE(ContainerActionType.DROP_ITEM, DropItemAction.DROP_FROM_SELECTED),
     DROP_ALL(ContainerActionType.DROP_ITEM, DropItemAction.DROP_SELECTED_STACK),
     LEFT_OUTSIDE(ContainerActionType.CLICK_ITEM, ClickItemAction.LEFT_CLICK),
-    RIGHT_OUTSIDE(ContainerActionType.CLICK_ITEM, ClickItemAction.RIGHT_CLICK);
+    RIGHT_OUTSIDE(ContainerActionType.CLICK_ITEM, ClickItemAction.RIGHT_CLICK),
+    SWAP_TO_HOTBAR_1(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_1),
+    SWAP_TO_HOTBAR_2(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_2),
+    SWAP_TO_HOTBAR_3(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_3),
+    SWAP_TO_HOTBAR_4(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_4),
+    SWAP_TO_HOTBAR_5(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_5),
+    SWAP_TO_HOTBAR_6(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_6),
+    SWAP_TO_HOTBAR_7(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_7),
+    SWAP_TO_HOTBAR_8(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_8),
+    SWAP_TO_HOTBAR_9(ContainerActionType.MOVE_TO_HOTBAR_SLOT, MoveToHotbarAction.SLOT_9);
 
     public static final int OUTSIDE_SLOT = -999;
 

core/src/main/java/org/geysermc/geyser/inventory/click/ClickPlan.java

@@ -28,6 +28,7 @@ package org.geysermc.geyser.inventory.click;
 import com.github.steveice10.mc.protocol.data.game.entity.metadata.ItemStack;
 import com.github.steveice10.mc.protocol.data.game.inventory.ContainerActionType;
 import com.github.steveice10.mc.protocol.data.game.inventory.ContainerType;
+import com.github.steveice10.mc.protocol.data.game.inventory.MoveToHotbarAction;
 import com.github.steveice10.mc.protocol.packet.ingame.serverbound.inventory.ServerboundContainerClickPacket;
 import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
@@ -107,12 +108,13 @@ public class ClickPlan {
             ClickAction action = planIter.next();
 
             if (action.slot != Click.OUTSIDE_SLOT && translator.getSlotType(action.slot) != SlotType.NORMAL) {
+                // Needed with Paper 1.16.5
                 refresh = true;
             }
 
-            int stateId = stateIdHack(action);
+            //int stateId = stateIdHack(action);
 
-            simulateAction(action);
+            //simulateAction(action);
 
             ItemStack clickedItemStack;
             if (!planIter.hasNext() && refresh) {
@@ -120,13 +122,14 @@ public class ClickPlan {
             } else if (action.click.actionType == ContainerActionType.DROP_ITEM || action.slot == Click.OUTSIDE_SLOT) {
                 clickedItemStack = null;
             } else {
-                // The action must be simulated first as Java expects the new contents of the cursor (as of 1.18.1)
+                //// The action must be simulated first as Java expects the new contents of the cursor (as of 1.18.1)
-                clickedItemStack = simulatedCursor.getItemStack();
+                //clickedItemStack = simulatedCursor.getItemStack(); TODO fix - this is the proper behavior but it terribly breaks 1.16.5
+                clickedItemStack = getItem(action.slot).getItemStack();
             }
 
             ServerboundContainerClickPacket clickPacket = new ServerboundContainerClickPacket(
                     inventory.getId(),
-                    stateId,
+                    inventory.getStateId(),
                     action.slot,
                     action.click.actionType,
                     action.click.action,
@@ -134,6 +137,8 @@ public class ClickPlan {
                     Collections.emptyMap() // Anything else we change, at this time, should have a packet sent to address
             );
 
+            simulateAction(action);
+
             session.sendDownstreamPacket(clickPacket);
         }
 
@@ -228,6 +233,33 @@ public class ClickPlan {
                         clicked.add(1);
                     }
                     break;
+                case SWAP_TO_HOTBAR_1:
+                    swap(action.slot, 36, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_2:
+                    swap(action.slot, 37, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_3:
+                    swap(action.slot, 38, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_4:
+                    swap(action.slot, 39, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_5:
+                    swap(action.slot, 40, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_6:
+                    swap(action.slot, 41, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_7:
+                    swap(action.slot, 42, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_8:
+                    swap(action.slot, 43, clicked);
+                    break;
+                case SWAP_TO_HOTBAR_9:
+                    swap(action.slot, 44, clicked);
+                    break;
                 case LEFT_SHIFT:
                     //TODO
                     break;
@@ -243,6 +275,15 @@ public class ClickPlan {
         }
     }
 
+    /**
+     * Swap between two inventory slots without a cursor. This should only be used with {@link ContainerActionType#MOVE_TO_HOTBAR_SLOT}
+     */
+    private void swap(int sourceSlot, int destSlot, GeyserItemStack sourceItem) {
+        GeyserItemStack destinationItem = simulating ? getItem(destSlot) : inventory.getItem(destSlot);
+        setItem(sourceSlot, destinationItem);
+        setItem(destSlot, sourceItem);
+    }
+
     private int stateIdHack(ClickAction action) {
         int stateId;
         if (inventory.getNextStateId() != -1) {
@@ -297,6 +338,9 @@ public class ClickPlan {
                 stateIdIncrements = 2;
             }
             inventory.incrementStateId(stateIdIncrements);
+        } else if (action.click.action instanceof MoveToHotbarAction) {
+            // Two slot changes sent
+            inventory.incrementStateId(2);
         } else {
             inventory.incrementStateId(1);
         }

core/src/main/java/org/geysermc/geyser/translator/inventory/CartographyInventoryTranslator.java

@@ -42,7 +42,7 @@ public class CartographyInventoryTranslator extends AbstractBlockInventoryTransl
     }
 
     @Override
-    public boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
+    protected boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
                                          int javaSourceSlot, ContainerSlotType bedrockDestinationContainer, int javaDestinationSlot) {
         if (javaDestinationSlot == 0) {
             // Bedrock Edition can use paper or an empty map in slot 0

core/src/main/java/org/geysermc/geyser/translator/inventory/InventoryTranslator.java

@@ -127,7 +127,7 @@ public abstract class InventoryTranslator {
      *
      * @return true if this transfer should be rejected
      */
-    public boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
+    protected boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
                                          int javaSourceSlot, ContainerSlotType bedrockDestinationContainer, int javaDestinationSlot) {
         return false;
     }
@@ -288,26 +288,49 @@ public abstract class InventoryTranslator {
                 }
                 case SWAP: {
                     SwapStackRequestActionData swapAction = (SwapStackRequestActionData) action;
-                    if (!(checkNetId(session, inventory, swapAction.getSource()) && checkNetId(session, inventory, swapAction.getDestination()))) {
+                    StackRequestSlotInfoData source = swapAction.getSource();
+                    StackRequestSlotInfoData destination = swapAction.getDestination();
+
+                    if (!(checkNetId(session, inventory, source) && checkNetId(session, inventory, destination))) {
                         if (session.getGeyser().getConfig().isDebugMode()) {
                             session.getGeyser().getLogger().error("DEBUG: About to reject SWAP request made by " + session.name());
-                            dumpStackRequestDetails(session, inventory, swapAction.getSource(), swapAction.getDestination());
+                            dumpStackRequestDetails(session, inventory, source, destination);
                         }
                         return rejectRequest(request);
                     }
 
-                    int sourceSlot = bedrockSlotToJava(swapAction.getSource());
+                    int sourceSlot = bedrockSlotToJava(source);
-                    int destSlot = bedrockSlotToJava(swapAction.getDestination());
+                    int destSlot = bedrockSlotToJava(destination);
-                    boolean isSourceCursor = isCursor(swapAction.getSource());
+                    boolean isSourceCursor = isCursor(source);
-                    boolean isDestCursor = isCursor(swapAction.getDestination());
+                    boolean isDestCursor = isCursor(destination);
 
-                    if (shouldRejectItemPlace(session, inventory, swapAction.getSource().getContainer(),
+                    if (shouldRejectItemPlace(session, inventory, source.getContainer(),
                             isSourceCursor ? -1 : sourceSlot,
-                            swapAction.getDestination().getContainer(), isDestCursor ? -1 : destSlot)) {
+                            destination.getContainer(), isDestCursor ? -1 : destSlot)) {
                         // This item would not be here in Java
                         return rejectRequest(request, false);
                     }
 
+                    if (!isSourceCursor && destination.getContainer() == ContainerSlotType.HOTBAR || destination.getContainer() == ContainerSlotType.HOTBAR_AND_INVENTORY) {
+                        // Tell the server we're pressing one of the hotbar keys to save clicks
+                        Click click = switch (destination.getSlot()) {
+                            case 0 -> Click.SWAP_TO_HOTBAR_1;
+                            case 1 -> Click.SWAP_TO_HOTBAR_2;
+                            case 2 -> Click.SWAP_TO_HOTBAR_3;
+                            case 3 -> Click.SWAP_TO_HOTBAR_4;
+                            case 4 -> Click.SWAP_TO_HOTBAR_5;
+                            case 5 -> Click.SWAP_TO_HOTBAR_6;
+                            case 6 -> Click.SWAP_TO_HOTBAR_7;
+                            case 7 -> Click.SWAP_TO_HOTBAR_8;
+                            case 8 -> Click.SWAP_TO_HOTBAR_9;
+                            default -> null;
+                        };
+                        if (click != null) {
+                            plan.add(click, sourceSlot);
+                            break;
+                        }
+                    }
+
                     if (isSourceCursor && isDestCursor) { //???
                         return rejectRequest(request);
                     } else if (isSourceCursor) { //swap cursor

core/src/main/java/org/geysermc/geyser/translator/inventory/LoomInventoryTranslator.java

@@ -102,7 +102,7 @@ public class LoomInventoryTranslator extends AbstractBlockInventoryTranslator {
     }
 
     @Override
-    public boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
+    protected boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
                                          int javaSourceSlot, ContainerSlotType bedrockDestinationContainer, int javaDestinationSlot) {
         if (javaDestinationSlot != 1) {
             return false;

core/src/main/java/org/geysermc/geyser/translator/inventory/chest/ChestInventoryTranslator.java

@@ -42,7 +42,7 @@ public abstract class ChestInventoryTranslator extends BaseInventoryTranslator {
     }
 
     @Override
-    public boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
+    protected boolean shouldRejectItemPlace(GeyserSession session, Inventory inventory, ContainerSlotType bedrockSourceContainer,
                                          int javaSourceSlot, ContainerSlotType bedrockDestinationContainer, int javaDestinationSlot) {
         // Reject any item placements that occur in the unusable inventory space
         if (bedrockSourceContainer == ContainerSlotType.CONTAINER && javaSourceSlot >= this.size) {