A bunch of fixes

connector/src/main/java/org/geysermc/connector/inventory/Inventory.java

@@ -36,7 +36,7 @@ import java.util.Arrays;
 public class Inventory {
 
     @Getter
-    protected int id;
+    protected final int id;
 
     @Getter
     protected final int size;
@@ -96,4 +96,17 @@ public class Inventory {
     public short getNextTransactionId() {
         return ++transactionId;
     }
+
+    @Override
+    public String toString() {
+        return "Inventory{" +
+                "id=" + id +
+                ", size=" + size +
+                ", title='" + title + '\'' +
+                ", items=" + Arrays.toString(items) +
+                ", holderPosition=" + holderPosition +
+                ", holderId=" + holderId +
+                ", transactionId=" + transactionId +
+                '}';
+    }
 }

connector/src/main/java/org/geysermc/connector/network/UpstreamPacketHandler.java

@@ -47,6 +47,7 @@ public class UpstreamPacketHandler extends LoggingPacketHandler {
     }
 
     private boolean translateAndDefault(BedrockPacket packet) {
+        System.out.println(packet.toString());
         return PacketTranslatorRegistry.BEDROCK_TRANSLATOR.translate(packet.getClass(), packet, session);
     }
 

connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java

@@ -144,7 +144,7 @@ public class GeyserSession implements CommandSender {
      * Use {@link #getNextItemNetId()} instead for consistency
      */
     @Getter(AccessLevel.NONE)
-    private final AtomicInteger itemNetId = new AtomicInteger(1);
+    private final AtomicInteger itemNetId = new AtomicInteger(2);
 
     @Getter(AccessLevel.NONE)
     private final Object inventoryLock = new Object();

connector/src/main/java/org/geysermc/connector/network/translators/inventory/InventoryTranslator.java

@@ -85,6 +85,8 @@ public abstract class InventoryTranslator {
             put(WindowType.CARTOGRAPHY, new CartographyInventoryTranslator());
             put(WindowType.CRAFTING, new CraftingInventoryTranslator());
             put(WindowType.ENCHANTMENT, new EnchantingInventoryTranslator());
+            put(WindowType.HOPPER, new HopperInventoryTranslator());
+            put(WindowType.GENERIC_3X3, new Generic3X3InventoryTranslator());
             put(WindowType.GRINDSTONE, new GrindstoneInventoryTranslator());
             put(WindowType.LOOM, new LoomInventoryTranslator());
             put(WindowType.MERCHANT, new MerchantInventoryTranslator());
@@ -92,10 +94,6 @@ public abstract class InventoryTranslator {
             put(WindowType.SMITHING, new SmithingInventoryTranslator());
             put(WindowType.STONECUTTER, new StonecutterInventoryTranslator());
 
-            /* Generics */
-            put(WindowType.GENERIC_3X3, new Generic3X3InventoryTranslator());
-            put(WindowType.HOPPER, new HopperInventoryTranslator());
-
             /* Lectern */
             put(WindowType.LECTERN, new LecternInventoryTranslator());
         }
@@ -195,9 +193,11 @@ public abstract class InventoryTranslator {
                                 transferAction.getSource().getSlot() >= 28 && transferAction.getSource().getSlot() <= 31) {
                             return rejectRequest(request, false);
                         }
-                        session.getConnector().getLogger().error("DEBUG: About to reject request made by " + session.getName());
+                        session.getConnector().getLogger().error("DEBUG: About to reject TAKE/PLACE request made by " + session.getName());
                         session.getConnector().getLogger().error("Source: " + transferAction.getSource().toString() + " Result: " + checkNetId(session, inventory, transferAction.getSource()));
                         session.getConnector().getLogger().error("Destination: " + transferAction.getDestination().toString() + " Result: " + checkNetId(session, inventory, transferAction.getDestination()));
+                        session.getConnector().getLogger().error("Geyser's record of source slot: " + inventory.getItem(bedrockSlotToJava(transferAction.getSource())));
+                        session.getConnector().getLogger().error("Geyser's record of destination slot: " + inventory.getItem(bedrockSlotToJava(transferAction.getDestination())));
                         return rejectRequest(request);
                     }
 
@@ -278,8 +278,14 @@ public abstract class InventoryTranslator {
                 }
                 case SWAP: { //TODO
                     SwapStackRequestActionData swapAction = (SwapStackRequestActionData) action;
-                    if (!(checkNetId(session, inventory, swapAction.getSource()) && checkNetId(session, inventory, swapAction.getDestination())))
+                    if (!(checkNetId(session, inventory, swapAction.getSource()) && checkNetId(session, inventory, swapAction.getDestination()))) {
+                        session.getConnector().getLogger().error("DEBUG: About to reject SWAP request made by " + session.getName());
+                        session.getConnector().getLogger().error("Source: " + swapAction.getSource().toString() + " Result: " + checkNetId(session, inventory, swapAction.getSource()));
+                        session.getConnector().getLogger().error("Destination: " + swapAction.getDestination().toString() + " Result: " + checkNetId(session, inventory, swapAction.getDestination()));
+                        session.getConnector().getLogger().error("Geyser's record of source slot: " + inventory.getItem(bedrockSlotToJava(swapAction.getSource())));
+                        session.getConnector().getLogger().error("Geyser's record of destination slot: " + inventory.getItem(bedrockSlotToJava(swapAction.getDestination())));
                         return rejectRequest(request);
+                    }
 
                     if (isCursor(swapAction.getSource()) && isCursor(swapAction.getDestination())) { //???
                         return rejectRequest(request);
@@ -361,14 +367,9 @@ public abstract class InventoryTranslator {
                     }
                     break;
                 }
-                case CRAFT_NON_IMPLEMENTED_DEPRECATED: {
+                case CRAFT_NON_IMPLEMENTED_DEPRECATED:
-                    break;
+                case CRAFT_RESULTS_DEPRECATED:
-                }
+                case CRAFT_RECIPE_OPTIONAL: { // Anvils and cartography tables will handle this
-                case CRAFT_RESULTS_DEPRECATED: {
-                    break;
-                }
-                case CRAFT_RECIPE_OPTIONAL: {
-                    // Anvils and cartography tables will handle this
                     break;
                 }
                 default:
@@ -381,9 +382,8 @@ public abstract class InventoryTranslator {
     }
     
     public ItemStackResponsePacket.Response translateCraftingRequest(GeyserSession session, Inventory inventory, ItemStackRequest request) {
-        int recipeId = 0;
         int resultSize = 0;
-        int timesCrafted = 0;
+        int timesCrafted;
         CraftState craftState = CraftState.START;
 
         int leftover = 0;
@@ -396,7 +396,6 @@ public abstract class InventoryTranslator {
                         return rejectRequest(request);
                     }
                     craftState = CraftState.RECIPE_ID;
-                    recipeId = craftAction.getRecipeNetworkId();
                     break;
                 }
                 case CRAFT_RESULTS_DEPRECATED: {
@@ -417,7 +416,6 @@ public abstract class InventoryTranslator {
                     break;
                 }
                 case CONSUME: {
-                    ConsumeStackRequestActionData consumeAction = (ConsumeStackRequestActionData) action;
                     if (craftState != CraftState.DEPRECATED && craftState != CraftState.INGREDIENTS) {
                         return rejectRequest(request);
                     }
@@ -507,9 +505,7 @@ public abstract class InventoryTranslator {
 
         Int2IntMap consumedSlots = new Int2IntOpenHashMap();
         int prioritySlot = -1;
-        int secondarySlot = -1;
+        int tempSlot;
-        int tempSlot = -1;
-        boolean intoCursor = false;
 
         int resultSize;
         int timesCrafted = 0;
@@ -626,7 +622,6 @@ public abstract class InventoryTranslator {
 
                     int javaSlot = bedrockSlotToJava(transferAction.getDestination());
                     if (isCursor(transferAction.getDestination())) { //TODO
-                        intoCursor = true;
                         if (timesCrafted > 1) {
                             tempSlot = findTempSlot(inventory, GeyserItemStack.from(output), true);
                             if (tempSlot == -1) {
@@ -735,6 +730,8 @@ public abstract class InventoryTranslator {
 
     public boolean checkNetId(GeyserSession session, Inventory inventory, StackRequestSlotInfoData slotInfoData) {
         int netId = slotInfoData.getStackNetworkId();
+        // "In my testing, sometimes the client thinks the netId of an item in the crafting grid is 1, even though we never said it was.
+        // I think it only happens when we manually set the grid but that was my quick fix"
         if (netId < 0 || netId == 1)
             return true;
 

connector/src/main/java/org/geysermc/connector/network/translators/inventory/holder/BlockInventoryHolder.java

@@ -31,6 +31,7 @@ import com.nukkitx.math.vector.Vector3i;
 import com.nukkitx.nbt.NbtMap;
 import com.nukkitx.protocol.bedrock.data.inventory.ContainerType;
 import com.nukkitx.protocol.bedrock.packet.BlockEntityDataPacket;
+import com.nukkitx.protocol.bedrock.packet.ContainerClosePacket;
 import com.nukkitx.protocol.bedrock.packet.ContainerOpenPacket;
 import com.nukkitx.protocol.bedrock.packet.UpdateBlockPacket;
 import org.geysermc.connector.inventory.Container;
@@ -76,11 +77,11 @@ public class BlockInventoryHolder extends InventoryHolder {
         if (session.getLastInteractionPlayerPosition().equals(session.getPlayerEntity().getPosition())) {
             // Then, check to see if the interacted block is valid for this inventory by ensuring the block state identifier is valid
             int javaBlockId = session.getConnector().getWorldManager().getBlockAt(session, session.getLastInteractionBlockPosition());
-            String javaBlockString = BlockTranslator.getJavaIdBlockMap().inverse().getOrDefault(javaBlockId, "minecraft:air").split("\\[")[0];
+            String[] javaBlockString = BlockTranslator.getJavaIdBlockMap().inverse().getOrDefault(javaBlockId, "minecraft:air").split("\\[");
-            if (this.validBlocks.contains(javaBlockString)) {
+            if (isValidBlock(javaBlockString)) {
                 // We can safely use this block
                 inventory.setHolderPosition(session.getLastInteractionBlockPosition());
-                ((Container) inventory).setUsingRealBlock(true, javaBlockString);
+                ((Container) inventory).setUsingRealBlock(true, javaBlockString[0]);
                 setCustomName(session, session.getLastInteractionBlockPosition(), inventory);
                 return;
             }
@@ -100,6 +101,13 @@ public class BlockInventoryHolder extends InventoryHolder {
         setCustomName(session, position, inventory);
     }
 
+    /**
+     * @return true if this Java block ID can be used for player inventory.
+     */
+    protected boolean isValidBlock(String[] javaBlockString) {
+        return this.validBlocks.contains(javaBlockString[0]);
+    }
+
     protected void setCustomName(GeyserSession session, Vector3i position, Inventory inventory) {
         NbtMap tag = NbtMap.builder()
                 .putInt("x", position.getX())
@@ -126,6 +134,11 @@ public class BlockInventoryHolder extends InventoryHolder {
     public void closeInventory(InventoryTranslator translator, GeyserSession session, Inventory inventory) {
         if (((Container) inventory).isUsingRealBlock()) {
             // No need to reset a block since we didn't change any blocks
+            // But send a container close packet because we aren't destroying the original.
+            ContainerClosePacket packet = new ContainerClosePacket();
+            packet.setId((byte) inventory.getId());
+            packet.setUnknownBool0(true); //TODO needs to be changed in Protocol to "server-side" or something
+            session.sendUpstreamPacket(packet);
             return;
         }
 

connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/SingleChestInventoryTranslator.java

@@ -37,7 +37,18 @@ public class SingleChestInventoryTranslator extends ChestInventoryTranslator {
     public SingleChestInventoryTranslator(int size) {
         super(size, 27);
         this.holder = new BlockInventoryHolder("minecraft:chest[facing=north,type=single,waterlogged=false]", ContainerType.CONTAINER,
-                "minecraft:ender_chest", "minecraft:trapped_chest");
+                "minecraft:ender_chest", "minecraft:trapped_chest") {
+            @Override
+            protected boolean isValidBlock(String[] javaBlockString) {
+                if (javaBlockString[0].equals("minecraft:ender_chest")) {
+                    // Can't have double ender chests
+                    return true;
+                }
+
+                // Add provision to ensure this isn't a double chest
+                return super.isValidBlock(javaBlockString) && (javaBlockString.length > 1 && javaBlockString[1].contains("type=single"));
+            }
+        };
     }
 
     @Override

connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java

@@ -36,6 +36,9 @@ public class JavaCloseWindowTranslator extends PacketTranslator<ServerCloseWindo
 
     @Override
     public void translate(ServerCloseWindowPacket packet, GeyserSession session) {
-        session.addInventoryTask(() -> InventoryUtils.closeInventory(session, packet.getWindowId()));
+        session.addInventoryTask(() -> {
+            session.getConnector().getLogger().info("Closing window ID " + packet.getWindowId());
+            InventoryUtils.closeInventory(session, packet.getWindowId());
+        });
     }
 }

connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java

@@ -45,6 +45,7 @@ public class JavaOpenWindowTranslator extends PacketTranslator<ServerOpenWindowP
             if (packet.getWindowId() == 0) {
                 return;
             }
+            session.getConnector().getLogger().info("Opening window ID " + packet.getWindowId());
 
             InventoryTranslator newTranslator = InventoryTranslator.INVENTORY_TRANSLATORS.get(packet.getType());
             Inventory openInventory = session.getOpenInventory();
@@ -63,10 +64,7 @@ public class JavaOpenWindowTranslator extends PacketTranslator<ServerOpenWindowP
 
             Inventory newInventory = newTranslator.createInventory(name, packet.getWindowId(), packet.getType(), session.getPlayerInventory());
             if (openInventory != null) {
-                InventoryTranslator openTranslator = session.getInventoryTranslator();
+                InventoryUtils.closeInventory(session, openInventory.getId());
-                if (!openTranslator.getClass().equals(newTranslator.getClass())) {
-                    InventoryUtils.closeInventory(session, openInventory.getId());
-                }
             }
 
             session.setInventoryTranslator(newTranslator);