Fix inventory handling

fixes #780 & #800

bukkit/src/main/java/us/myles/ViaVersion/bukkit/providers/BukkitInventoryQuickMoveProvider.java

@@ -1,8 +1,19 @@
 package us.myles.ViaVersion.bukkit.providers;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.logging.Level;
+
 import org.bukkit.entity.Player;
+import org.bukkit.event.inventory.InventoryType;
+import org.bukkit.inventory.Inventory;
 import org.bukkit.inventory.InventoryView;
 import org.bukkit.inventory.ItemStack;
+
 import us.myles.ViaVersion.api.Via;
 import us.myles.ViaVersion.api.data.UserConnection;
 import us.myles.ViaVersion.api.protocol.ProtocolRegistry;
@@ -14,13 +25,6 @@ import us.myles.ViaVersion.protocols.protocol1_12to1_11_1.providers.InventoryQui
 import us.myles.ViaVersion.protocols.protocol1_12to1_11_1.storage.ItemTransaction;
 import us.myles.ViaVersion.util.ReflectionUtil;
 
-import java.lang.reflect.Field;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.util.Map;
-import java.util.UUID;
-import java.util.concurrent.ConcurrentHashMap;
-
 public class BukkitInventoryQuickMoveProvider extends InventoryQuickMoveProvider {
 
     private static Map<UUID, BukkitInventoryUpdateTask> updateTasks = new ConcurrentHashMap<UUID, BukkitInventoryUpdateTask>();
@@ -40,13 +44,26 @@ public class BukkitInventoryQuickMoveProvider extends InventoryQuickMoveProvider
     }
 
     @Override
-    public boolean registerQuickMove(short windowId, short slotId, short actionId, UserConnection userConnection) {
+    public boolean registerQuickMoveAction(short windowId, short slotId, short actionId, UserConnection userConnection) {
         if (!supported) {
             return false;
         }
         if (slotId < 0) { // clicked out of inv slot
             return false;
         }
+        if (windowId == 0) {
+            /**
+             * windowId is always 0 for player inventory.
+             * This has almost definitely something to do with the offhand slot.
+             */
+            if (slotId >= 36 && slotId <= 44) {
+                int protocolId = ProtocolRegistry.SERVER_PROTOCOL;
+                // this seems to be working just fine.
+                if (protocolId == ProtocolVersion.v1_8.getId()) {
+                    return false;
+                }
+            }
+        }
         ProtocolInfo info = userConnection.get(ProtocolInfo.class);
         UUID uuid = info.getUuid();
         BukkitInventoryUpdateTask updateTask = updateTasks.get(uuid);
@@ -69,17 +86,33 @@ public class BukkitInventoryQuickMoveProvider extends InventoryQuickMoveProvider
         }
         InventoryView inv = p.getOpenInventory();
         short slotId = storage.getSlotId();
-        if (slotId > inv.countSlots()) {
-            return null; // wrong container open?
+        Inventory tinv = inv.getTopInventory();
+        InventoryType tinvtype = tinv == null ? null : tinv.getType(); // can this even be null?
+        if (tinvtype != null) {
+            int protocolId = ProtocolRegistry.SERVER_PROTOCOL;
+            if (protocolId == ProtocolVersion.v1_8.getId()) {
+                if (tinvtype == InventoryType.BREWING) {
+                    // 1.9 added the blaze powder slot to brewing stand fix for 1.8 servers
+                    if (slotId >= 5 && slotId <= 40) {
+                        slotId = (short) (slotId - 1);
+                    }
+                }
+            }
         }
-        ItemStack itemstack = inv.getItem(slotId);
-        if (itemstack == null) {
-            return null;
+        ItemStack itemstack = null;
+        // must be after top inventory slot check
+        if (slotId <= inv.countSlots()) {
+            itemstack = inv.getItem(slotId);
+        } else {
+            // if not true we got too many slots (version inventory slot changes)?
+            String cause = "Too many inventory slots: slotId: " + slotId + " invSlotCount: " + inv.countSlots()
+                    + " invType: " + inv.getType() + " topInvType: " + tinvtype;
+            Via.getPlatform().getLogger().severe("Failed to get an item to create a window click packet. Please report this issue to the ViaVersion Github: " + cause);
         }
         Object packet = null;
         try {
-            packet = windowClickPacketClass.newInstance();
-            Object nmsItem = nmsItemMethod.invoke(null, itemstack);
+            packet = windowClickPacketClass.getDeclaredConstructor().newInstance();
+            Object nmsItem = itemstack == null ? null : nmsItemMethod.invoke(null, itemstack);
             ReflectionUtil.set(packet, "a", (int) storage.getWindowId());
             ReflectionUtil.set(packet, "slot", (int) slotId);
             ReflectionUtil.set(packet, "button", 0); // shift + left mouse click
@@ -92,14 +125,15 @@ public class BukkitInventoryQuickMoveProvider extends InventoryQuickMoveProvider
                 ReflectionUtil.set(packet, "shift", clickTypeEnum);
             }
         } catch (Exception e) {
-            e.printStackTrace();
+            Via.getPlatform().getLogger().log(Level.SEVERE, "Failed to create a window click packet. Please report this issue to the ViaVersion Github: " + e.getMessage(), e);
         }
         return packet;
     }
 
-    public boolean sendPlayer(Player p, Object packet) {
+    public boolean sendPacketToServer(Player p, Object packet) {
         if (packet == null) {
-            return false;
+            // let the other packets pass through
+            return true;
         }
         try {
             Object entityPlayer = craftPlayerHandle.invoke(p);

bukkit/src/main/java/us/myles/ViaVersion/bukkit/tasks/protocol1_12to1_11_1/BukkitInventoryUpdateTask.java

@@ -39,7 +39,7 @@ public class BukkitInventoryUpdateTask implements Runnable {
             synchronized (items) {
                 for (ItemTransaction storage : items) {
                     Object packet = provider.buildWindowClickPacket(p, storage);
-                    boolean result = provider.sendPlayer(p, packet);
+                    boolean result = provider.sendPacketToServer(p, packet);
                     if (!result) {
                         break;
                     }

common/src/main/java/us/myles/ViaVersion/protocols/protocol1_12to1_11_1/packets/InventoryPackets.java

@@ -133,7 +133,7 @@ public class InventoryPackets {
                                     short slotId = wrapper.get(Type.SHORT, 0);
                                     short actionId = wrapper.get(Type.SHORT, 1);
                                     InventoryQuickMoveProvider provider = Via.getManager().getProviders().get(InventoryQuickMoveProvider.class);
-                                    boolean succeed = provider.registerQuickMove(windowId, slotId, actionId, wrapper.user());
+                                    boolean succeed = provider.registerQuickMoveAction(windowId, slotId, actionId, wrapper.user());
                                     if (succeed) {
                                         wrapper.cancel();
                                     }

common/src/main/java/us/myles/ViaVersion/protocols/protocol1_12to1_11_1/providers/InventoryQuickMoveProvider.java

@@ -5,7 +5,7 @@ import us.myles.ViaVersion.api.platform.providers.Provider;
 
 public class InventoryQuickMoveProvider implements Provider {
 
-    public boolean registerQuickMove(short windowId, short slotId, short actionId, UserConnection userConnection) {
+    public boolean registerQuickMoveAction(short windowId, short slotId, short actionId, UserConnection userConnection) {
         return false; // not supported :/ plays very sad violin
     }
 }