Reuse inventories whenever possible, prevent bedrock inventory id conflicts

core/src/main/java/org/geysermc/geyser/GeyserLogger.java

@@ -125,7 +125,7 @@ public interface GeyserLogger extends GeyserCommandSource {
      */
     default void debug(GeyserSession session, String message, Object... arguments) {
         if (isDebug()) {
-            debug("( " + session.bedrockUsername() + " ) " + message, arguments);
+            debug("(" + session.bedrockUsername() + ") " + message, arguments);
         }
     }
 

core/src/main/java/org/geysermc/geyser/command/defaults/AdvancedTooltipsCommand.java

@@ -51,6 +51,6 @@ public class AdvancedTooltipsCommand extends GeyserCommand {
             + MinecraftLocale.getLocaleString("debug.prefix", session.locale())
             + " " + ChatColor.RESET
             + MinecraftLocale.getLocaleString("debug.advanced_tooltips." + onOrOff, session.locale()));
-        session.getPlayerInventory().updateInventory(session);
+        session.getPlayerInventory().updateInventory();
     }
 }

core/src/main/java/org/geysermc/geyser/inventory/AnvilContainer.java

@@ -63,8 +63,8 @@ public class AnvilContainer extends Container {
 
     private int lastTargetSlot = -1;
 
-    public AnvilContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public AnvilContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     /**

core/src/main/java/org/geysermc/geyser/inventory/BeaconContainer.java

@@ -25,6 +25,7 @@
 
 package org.geysermc.geyser.inventory;
 
+import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 import lombok.Getter;
@@ -36,7 +37,7 @@ public class BeaconContainer extends Container {
     private int primaryId;
     private int secondaryId;
 
-    public BeaconContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public BeaconContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 }

core/src/main/java/org/geysermc/geyser/inventory/CartographyContainer.java

@@ -25,11 +25,12 @@
 
 package org.geysermc.geyser.inventory;
 
+import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 
 public class CartographyContainer extends Container {
-    public CartographyContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public CartographyContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 }

core/src/main/java/org/geysermc/geyser/inventory/Container.java

@@ -26,7 +26,6 @@
 package org.geysermc.geyser.inventory;
 
 import lombok.Getter;
-import lombok.Setter;
 import org.checkerframework.checker.nullness.qual.NonNull;
 import org.geysermc.geyser.level.block.type.Block;
 import org.geysermc.geyser.session.GeyserSession;
@@ -47,15 +46,8 @@ public class Container extends Inventory {
      */
     private boolean isUsingRealBlock = false;
 
-    /**
-     * Used to minimize delay when switching between "same" containers.
-     * Currently unused; see {@link org.geysermc.geyser.translator.protocol.java.inventory.JavaOpenScreenTranslator} for info.
-     */
-    @Setter
-    private boolean isReusingBlock = false;
-
-    public Container(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, translator);
+    public Container(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, translator);
         this.playerInventory = playerInventory;
         this.containerSize = this.size + InventoryTranslator.PLAYER_INVENTORY_SIZE;
     }

core/src/main/java/org/geysermc/geyser/inventory/CrafterContainer.java

@@ -48,8 +48,8 @@ public class CrafterContainer extends Container {
      */
     private short disabledSlotsMask = 0;
 
-    public CrafterContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public CrafterContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/inventory/EnchantingContainer.java

@@ -25,25 +25,25 @@
 
 package org.geysermc.geyser.inventory;
 
+import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 import org.cloudburstmc.protocol.bedrock.data.inventory.EnchantOptionData;
 import lombok.Getter;
 
+@Getter
 public class EnchantingContainer extends Container {
     /**
      * A cache of what Bedrock sees
      */
-    @Getter
     private final EnchantOptionData[] enchantOptions;
     /**
      * A mutable cache of what the server sends us
      */
-    @Getter
     private final GeyserEnchantOption[] geyserEnchantOptions;
 
-    public EnchantingContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public EnchantingContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
 
         enchantOptions = new EnchantOptionData[3];
         geyserEnchantOptions = new GeyserEnchantOption[3];

core/src/main/java/org/geysermc/geyser/inventory/Generic3X3Container.java

@@ -33,17 +33,17 @@ import org.geysermc.geyser.translator.inventory.Generic3X3InventoryTranslator;
 import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 
+@Getter
 public class Generic3X3Container extends Container {
     /**
      * Whether we need to set the container type as {@link org.cloudburstmc.protocol.bedrock.data.inventory.ContainerType#DROPPER}.
      * <p>
      * Used at {@link Generic3X3InventoryTranslator#openInventory(GeyserSession, Inventory)}
      */
-    @Getter
     private boolean isDropper = false;
 
-    public Generic3X3Container(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public Generic3X3Container(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/inventory/Inventory.java

@@ -47,6 +47,10 @@ public abstract class Inventory {
     @Getter
     protected final int javaId;
 
+    @Setter
+    @Getter
+    private int bedrockId;
+
     /**
      * The Java inventory state ID from the server. As of Java Edition 1.18.1 this value has one instance per player.
      * If this is out of sync with the server when a packet containing it is handled, the server will resync items.
@@ -105,11 +109,14 @@ public abstract class Inventory {
     @Getter
     private final InventoryTranslator translator;
 
-    protected Inventory(int id, int size, ContainerType containerType, InventoryTranslator translator) {
-        this("Inventory", id, size, containerType, translator);
+    @Getter
+    private final GeyserSession session;
+
+    protected Inventory(GeyserSession session, int id, int size, ContainerType containerType, InventoryTranslator translator) {
+        this(session, "Inventory", id, size, containerType, translator);
     }
 
-    protected Inventory(String title, int javaId, int size, ContainerType containerType, InventoryTranslator translator) {
+    protected Inventory(GeyserSession session, String title, int javaId, int size, ContainerType containerType, InventoryTranslator translator) {
         this.title = title;
         this.javaId = javaId;
         this.size = size;
@@ -117,14 +124,19 @@ public abstract class Inventory {
         this.items = new GeyserItemStack[size];
         Arrays.fill(items, GeyserItemStack.EMPTY);
         this.translator = translator;
-    }
+        this.session = session;
 
-    // This is to prevent conflicts with special bedrock inventory IDs.
-    // The vanilla java server only sends an ID between 1 and 100 when opening an inventory,
-    // so this is rarely needed. (certain plugins)
-    // Example: https://github.com/GeyserMC/Geyser/issues/3254
-    public int getBedrockId() {
-        return javaId <= 100 ? javaId : (javaId % 100) + 1;
+        // This is to prevent conflicts with special bedrock inventory IDs.
+        // The vanilla java server only sends an ID between 1 and 100 when opening an inventory,
+        // so this is rarely needed. (certain plugins)
+        // Example: https://github.com/GeyserMC/Geyser/issues/3254
+        this.bedrockId = javaId <= 100 ? javaId : (javaId % 100) + 1;
+
+        // We occasionally need to re-open inventories with a delay in cases where
+        // Java wouldn't - e.g. for virtual chest menus that switch pages
+        if (session.getOpenInventory() != null && session.getOpenInventory().getBedrockId() == bedrockId) {
+            this.bedrockId += 1;
+        }
     }
 
     public GeyserItemStack getItem(int slot) {
@@ -197,15 +209,15 @@ public abstract class Inventory {
      * Helper methods to avoid using the wrong translator to update specific inventories.
      */
 
-    public void updateInventory(GeyserSession session) {
+    public void updateInventory() {
         this.translator.updateInventory(session, this);
     }
 
-    public void updateProperty(GeyserSession session, int rawProperty, int value) {
+    public void updateProperty(int rawProperty, int value) {
         this.translator.updateProperty(session, this, rawProperty, value);
     }
 
-    public void updateSlot(GeyserSession session, int slot) {
+    public void updateSlot(int slot) {
         this.translator.updateSlot(session, this, slot);
     }
 }

core/src/main/java/org/geysermc/geyser/inventory/LecternContainer.java

@@ -46,8 +46,8 @@ public class LecternContainer extends Container {
 
     private boolean isBookInPlayerInventory = false;
 
-    public LecternContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public LecternContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     /**

core/src/main/java/org/geysermc/geyser/inventory/MerchantContainer.java

@@ -35,18 +35,18 @@ import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.VillagerTrade;
 import org.geysermc.mcprotocollib.protocol.packet.ingame.clientbound.inventory.ClientboundMerchantOffersPacket;
 
+@Setter
 public class MerchantContainer extends Container {
-    @Getter @Setter
+    @Getter
     private Entity villager;
-    @Setter
     private VillagerTrade[] villagerTrades;
-    @Getter @Setter
+    @Getter
     private ClientboundMerchantOffersPacket pendingOffersPacket;
-    @Getter @Setter
+    @Getter
     private int tradeExperience;
 
-    public MerchantContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public MerchantContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     public void onTradeSelected(GeyserSession session, int slot) {

core/src/main/java/org/geysermc/geyser/inventory/PlayerInventory.java

@@ -35,21 +35,20 @@ import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.entity.player.Hand;
 import org.jetbrains.annotations.Range;
 
+@Getter
 public class PlayerInventory extends Inventory {
     /**
      * Stores the held item slot, starting at index 0.
      * Add 36 in order to get the network item slot.
      */
-    @Getter
     @Setter
     private int heldItemSlot;
 
-    @Getter
     @NonNull
     private GeyserItemStack cursor = GeyserItemStack.EMPTY;
 
-    public PlayerInventory() {
-        super(0, 46, null, InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR);
+    public PlayerInventory(GeyserSession session) {
+        super(session, 0, 46, null, InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR);
         heldItemSlot = 0;
     }
 

core/src/main/java/org/geysermc/geyser/inventory/StonecutterContainer.java

@@ -32,16 +32,16 @@ import org.geysermc.geyser.session.GeyserSession;
 import org.geysermc.geyser.translator.inventory.InventoryTranslator;
 import org.geysermc.mcprotocollib.protocol.data.game.inventory.ContainerType;
 
+@Setter
+@Getter
 public class StonecutterContainer extends Container {
     /**
      * The button that has currently been pressed Java-side
      */
-    @Getter
-    @Setter
     private int stonecutterButton = -1;
 
-    public StonecutterContainer(String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
-        super(title, id, size, containerType, playerInventory, translator);
+    public StonecutterContainer(GeyserSession session, String title, int id, int size, ContainerType containerType, PlayerInventory playerInventory, InventoryTranslator translator) {
+        super(session, title, id, size, containerType, playerInventory, translator);
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/inventory/holder/BlockInventoryHolder.java

@@ -78,25 +78,16 @@ public class BlockInventoryHolder extends InventoryHolder {
 
     @Override
     public boolean canReuseContainer(GeyserSession session, Container container, Container previous) {
-        // We already ensured that the inventories are the same type, size,
+        // We already ensured that the inventories are using the same type, size, and title
         if (canUseRealBlock(session, container)) {
-            // We can reuse the same holder position.
-            if (container.getHolderPosition() != previous.getHolderPosition()) {
-                return false;
-            } else {
-                container.setReusingBlock(true);
-                return true;
-            }
+            // Only reuse if the position matches
+            return container.getHolderPosition() == previous.getHolderPosition();
         }
 
         // Check if we'd be using the same virtual inventory position.
         Vector3i position = InventoryUtils.findAvailableWorldSpace(session);
-        if (Objects.equals(position, previous.getHolderPosition())) {
-            container.setReusingBlock(true);
-            return true;
-        }
-
-        return false;
+        // Reuse inventory if a virtual inventory position exists and matches
+        return Objects.equals(position, previous.getHolderPosition());
     }
 
     @Override

core/src/main/java/org/geysermc/geyser/session/GeyserSession.java

@@ -721,7 +721,7 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
         this.playerEntity = new SessionPlayerEntity(this);
         collisionManager.updatePlayerBoundingBox(this.playerEntity.getPosition());
 
-        this.playerInventory = new PlayerInventory();
+        this.playerInventory = new PlayerInventory(this);
         this.openInventory = null;
         this.craftingRecipes = new Int2ObjectOpenHashMap<>();
         this.javaToBedrockRecipeIds = new Int2ObjectOpenHashMap<>();

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

@@ -25,6 +25,7 @@
 
 package org.geysermc.geyser.translator.inventory;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.protocol.bedrock.data.inventory.ContainerType;
 import org.geysermc.geyser.inventory.Container;
@@ -78,11 +79,11 @@ public abstract class AbstractBlockInventoryTranslator extends BaseInventoryTran
 
     @Override
     public boolean shouldDelayInventoryOpen(GeyserSession session, Inventory inventory) {
-        return inventory instanceof Container container && !container.isUsingRealBlock() && !container.isReusingBlock();
+        return inventory instanceof Container container && !container.isUsingRealBlock();
     }
 
     @Override
-    public boolean canReuseInventory(GeyserSession session, Inventory inventory, Inventory previous) {
+    public boolean canReuseInventory(GeyserSession session, @NonNull Inventory inventory, @NonNull Inventory previous) {
         if (super.canReuseInventory(session, inventory, previous)
                 && inventory instanceof Container container
                 && previous instanceof Container previousContainer) {

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

@@ -103,8 +103,8 @@ public class AnvilInventoryTranslator extends AbstractBlockInventoryTranslator {
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new AnvilContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new AnvilContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -90,7 +90,7 @@ public abstract class BaseInventoryTranslator extends InventoryTranslator {
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new Container(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new Container(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 }

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

@@ -145,8 +145,8 @@ public class BeaconInventoryTranslator extends AbstractBlockInventoryTranslator
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new BeaconContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new BeaconContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -86,8 +86,8 @@ public class CartographyInventoryTranslator extends AbstractBlockInventoryTransl
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new CartographyContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new CartographyContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -139,9 +139,9 @@ public class CrafterInventoryTranslator extends AbstractBlockInventoryTranslator
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
         // Java sends the triggered and slot bits incrementally through properties, which we store here
-        return new CrafterContainer(name, windowId, this.size, containerType, playerInventory, this);
+        return new CrafterContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     private static void updateBlockEntity(GeyserSession session, CrafterContainer container) {

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

@@ -168,8 +168,8 @@ public class EnchantingInventoryTranslator extends AbstractBlockInventoryTransla
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new EnchantingContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new EnchantingContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -46,8 +46,8 @@ public class Generic3X3InventoryTranslator extends AbstractBlockInventoryTransla
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new Generic3X3Container(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new Generic3X3Container(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -35,6 +35,7 @@ import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
 import it.unimi.dsi.fastutil.ints.IntSet;
 import it.unimi.dsi.fastutil.ints.IntSortedSet;
 import lombok.AllArgsConstructor;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.checkerframework.checker.nullness.qual.Nullable;
 import org.cloudburstmc.protocol.bedrock.data.inventory.ContainerSlotType;
 import org.cloudburstmc.protocol.bedrock.data.inventory.FullContainerName;
@@ -143,23 +144,17 @@ public abstract class InventoryTranslator {
     /**
      * Whether a new inventory should be prepared - or if we can re-use the previous one.
      */
-    public boolean canReuseInventory(GeyserSession session, Inventory inventory, Inventory previous) {
-        // Filter for obvious mismatches that require a new inventory
-        if (previous == null
-            || inventory.getContainerType() == null
-            || previous.getContainerType() == null
-            || !Objects.equals(inventory.getContainerType(), previous.getContainerType())) {
+    public boolean canReuseInventory(GeyserSession session, @NonNull Inventory inventory, @NonNull Inventory previous) {
+        // Filter for mismatches that require a new inventory
+        if (inventory.getContainerType() == null || previous.getContainerType() == null
+            || !Objects.equals(inventory.getContainerType(), previous.getContainerType())
+            || inventory.getJavaId() != previous.getJavaId()
+        ) {
             return false;
         }
 
-        // Inventory size should also match.
-        if (inventory.getSize() != previous.getSize() || !Objects.equals(inventory.getTitle(), previous.getTitle())) {
-            return false;
-        }
-
-        // TODO can we easily set a new inventory name???
-
-        return true;
+        // Inventory size and the title should also match.
+        return inventory.getSize() == previous.getSize() && Objects.equals(inventory.getTitle(), previous.getTitle());
     }
     public abstract boolean prepareInventory(GeyserSession session, Inventory inventory);
     public abstract void openInventory(GeyserSession session, Inventory inventory);
@@ -171,7 +166,7 @@ public abstract class InventoryTranslator {
     public abstract int javaSlotToBedrock(int javaSlot);
     public abstract BedrockContainerSlot javaSlotToBedrockContainer(int javaSlot);
     public abstract SlotType getSlotType(int javaSlot);
-    public abstract Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory);
+    public abstract Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory);
 
     /**
      * Used for crafting-related transactions. Will override in PlayerInventoryTranslator and CraftingInventoryTranslator.

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

@@ -199,7 +199,7 @@ public class LecternInventoryTranslator extends AbstractBlockInventoryTranslator
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new LecternContainer(name, windowId, this.size + playerInventory.getSize(), containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new LecternContainer(session, name, windowId, this.size + playerInventory.getSize(), containerType, playerInventory, this);
     }
 }

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

@@ -195,7 +195,7 @@ public class MerchantInventoryTranslator extends BaseInventoryTranslator {
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new MerchantContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new MerchantContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 }

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

@@ -28,6 +28,7 @@ package org.geysermc.geyser.translator.inventory;
 import it.unimi.dsi.fastutil.ints.IntIterator;
 import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
 import it.unimi.dsi.fastutil.ints.IntSet;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.cloudburstmc.math.vector.Vector3i;
 import org.cloudburstmc.protocol.bedrock.data.definitions.BlockDefinition;
 import org.cloudburstmc.protocol.bedrock.data.inventory.ContainerId;
@@ -569,12 +570,12 @@ public class PlayerInventoryTranslator extends InventoryTranslator {
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
         throw new UnsupportedOperationException();
     }
 
     @Override
-    public boolean canReuseInventory(GeyserSession session, Inventory inventory, Inventory previous) {
+    public boolean canReuseInventory(GeyserSession session, @NonNull Inventory inventory, @NonNull Inventory previous) {
         return true;
     }
 

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

@@ -123,8 +123,8 @@ public class StonecutterInventoryTranslator extends AbstractBlockInventoryTransl
     }
 
     @Override
-    public Inventory createInventory(String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
-        return new StonecutterContainer(name, windowId, this.size, containerType, playerInventory, this);
+    public Inventory createInventory(GeyserSession session, String name, int windowId, ContainerType containerType, PlayerInventory playerInventory) {
+        return new StonecutterContainer(session, name, windowId, this.size, containerType, playerInventory, this);
     }
 
     @Override

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

@@ -25,6 +25,7 @@
 
 package org.geysermc.geyser.translator.inventory.chest;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.cloudburstmc.math.vector.Vector3i;
 import org.cloudburstmc.nbt.NbtMap;
 import org.cloudburstmc.nbt.NbtMapBuilder;
@@ -37,6 +38,7 @@ import org.cloudburstmc.protocol.bedrock.packet.UpdateBlockPacket;
 import org.geysermc.geyser.GeyserImpl;
 import org.geysermc.geyser.inventory.Container;
 import org.geysermc.geyser.inventory.Inventory;
+import org.geysermc.geyser.inventory.holder.BlockInventoryHolder;
 import org.geysermc.geyser.level.block.Blocks;
 import org.geysermc.geyser.level.block.property.ChestType;
 import org.geysermc.geyser.level.block.property.Properties;
@@ -61,35 +63,27 @@ public class DoubleChestInventoryTranslator extends ChestInventoryTranslator {
                 .javaId();
     }
 
+    /**
+     * Additional checks to verify that we can re-use the block inventory holder.
+     * Mirrors {@link BlockInventoryHolder#canReuseContainer(GeyserSession, Container, Container)}
+     */
     @Override
-    public boolean canReuseInventory(GeyserSession session, Inventory inventory, Inventory previous) {
-        if (!super.canReuseInventory(session, inventory, previous)) {
+    public boolean canReuseInventory(GeyserSession session, @NonNull Inventory inventory, @NonNull Inventory previous) {
+        if (!super.canReuseInventory(session, inventory, previous) ||
+            !(inventory instanceof Container container) ||
+            !(previous instanceof Container)
+        ) {
             return false;
         }
 
-        if (!(inventory instanceof Container container) || !(previous instanceof Container previousContainer)) {
-            return false;
-        }
-
-        // We already ensured that the inventories are the same type, size,
         if (canUseRealBlock(session, container)) {
             // We can reuse the same holder position.
-            if (container.getHolderPosition() != previous.getHolderPosition()) {
-                return false;
-            } else {
-                container.setReusingBlock(true);
-                return true;
-            }
+            return container.getHolderPosition() == previous.getHolderPosition();
         }
 
         // Check if we'd be using the same virtual inventory position.
         Vector3i position = InventoryUtils.findAvailableWorldSpace(session);
-        if (Objects.equals(position, previous.getHolderPosition())) {
-            container.setReusingBlock(true);
-            return true;
-        }
-
-        return false;
+        return Objects.equals(position, previous.getHolderPosition());
     }
 
     @Override
@@ -165,6 +159,8 @@ public class DoubleChestInventoryTranslator extends ChestInventoryTranslator {
 
     @Override
     public void closeInventory(GeyserSession session, Inventory inventory) {
+        // this should no longer be possible; as we're storing the translator with the inventory to avoid desyncs.
+        // TODO use generics to ensure we don't need to cast unsafely in the first place
         if (!(inventory instanceof Container container)) {
             GeyserImpl.getInstance().getLogger().warning("Tried to close a non-container inventory in a block inventory holder! Please report this error on discord.");
             GeyserImpl.getInstance().getLogger().warning("Current inventory translator: " + InventoryUtils.getInventoryTranslator(session).getClass().getSimpleName());

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockBookEditTranslator.java

@@ -132,7 +132,7 @@ public class BedrockBookEditTranslator extends PacketTranslator<BookEditPacket>
 
             // Update local copy
             session.getPlayerInventory().setItem(36 + session.getPlayerInventory().getHeldItemSlot(), GeyserItemStack.from(bookItem), session);
-            session.getPlayerInventory().updateInventory(session);
+            session.getPlayerInventory().updateInventory();
 
             String title;
             if (packet.getAction() == BookEditPacket.Action.SIGN_BOOK) {

core/src/main/java/org/geysermc/geyser/translator/protocol/bedrock/BedrockFilterTextTranslator.java

@@ -48,7 +48,7 @@ public class BedrockFilterTextTranslator extends PacketTranslator<FilterTextPack
         packet.setFromServer(true);
         if (session.getOpenInventory() instanceof AnvilContainer anvilContainer) {
             packet.setText(anvilContainer.checkForRename(session, packet.getText()));
-            anvilContainer.updateSlot(session, 1);
+            anvilContainer.updateSlot(1);
         }
         session.sendUpstreamPacket(packet);
     }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/inventory/JavaContainerSetDataTranslator.java

@@ -41,6 +41,6 @@ public class JavaContainerSetDataTranslator extends PacketTranslator<Clientbound
         if (inventory == null)
             return;
 
-        inventory.updateProperty(session, packet.getRawProperty(), packet.getValue());
+        inventory.updateProperty(packet.getRawProperty(), packet.getValue());
     }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/inventory/JavaHorseScreenOpenTranslator.java

@@ -153,6 +153,6 @@ public class JavaHorseScreenOpenTranslator extends PacketTranslator<ClientboundH
         updateEquipPacket.setTag(builder.build());
         session.sendUpstreamPacket(updateEquipPacket);
 
-        InventoryUtils.openInventory(session, new Container(entity.getNametag(), packet.getContainerId(), slotCount, null, session.getPlayerInventory(), inventoryTranslator));
+        InventoryUtils.openInventory(session, new Container(session, entity.getNametag(), packet.getContainerId(), slotCount, null, session.getPlayerInventory(), inventoryTranslator));
     }
 }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/inventory/JavaOpenBookTranslator.java

@@ -71,7 +71,7 @@ public class JavaOpenBookTranslator extends PacketTranslator<ClientboundOpenBook
 
             // Should never be null
             Objects.requireNonNull(translator, "lectern translator must exist");
-            Inventory inventory = translator.createInventory("", FAKE_LECTERN_WINDOW_ID, ContainerType.LECTERN, session.getPlayerInventory());
+            Inventory inventory = translator.createInventory(session, "", FAKE_LECTERN_WINDOW_ID, ContainerType.LECTERN, session.getPlayerInventory());
             ((LecternContainer) inventory).setFakeLecternBook(stack, session);
             InventoryUtils.openInventory(session, inventory);
         }

core/src/main/java/org/geysermc/geyser/translator/protocol/java/inventory/JavaOpenScreenTranslator.java

@@ -75,34 +75,31 @@ public class JavaOpenScreenTranslator extends PacketTranslator<ClientboundOpenSc
 
         String name = MessageTranslator.convertMessage(packet.getTitle(), session.locale());
 
-        Inventory newInventory = newTranslator.createInventory(name, packet.getContainerId(), packet.getType(), session.getPlayerInventory());
+        Inventory newInventory = newTranslator.createInventory(session, name, packet.getContainerId(), packet.getType(), session.getPlayerInventory());
         if (openInventory != null) {
-//            // TODO this would allow us to open repeating virtual inventories quite a bit faster.
-//            // Attempt to re-use existing open inventories, if possible
-//            if (newTranslator.canReuseInventory(session, newInventory, openInventory)) {
-//                // We need to handle pending virtual block inventories *slightly* different
-//                if (session.getPendingInventoryId() == openInventory.getJavaId()
-//                    && openInventory.isPending()
-//                    && openInventory instanceof Container container
-//                    && newInventory instanceof Container newContainer
-//                    && !container.isUsingRealBlock()
-//                ) {
-//                    GeyserImpl.getInstance().getLogger().info("can reuse inv!");
-//                    // Check here if these inventories are the "same". If not, we will have to interrupt
-//                    // the pending inventory.
-//                    if (openInventory.getJavaId() == newInventory.getJavaId()) {
-//                        // no changes needed, they're the same; continue opening and ignore this packet
-//                        return;
-//                    }
-//
-//                    // We'll have to close the inventory :)))
-//                    newContainer.setReusingBlock(false);
-//                } else {
-//                    // Not a virtual inventory - seem to match though, so let's just update the inventory
-//                    newTranslator.updateInventory(session, newInventory);
-//                    return;
-//                }
-//            }
+            // Attempt to re-use existing open inventories, if possible
+            if (newTranslator.canReuseInventory(session, newInventory, openInventory)) {
+                // Use the same Bedrock id. The java id is already confirmed to match
+                // in the reuse inventory check.
+                newInventory.setBedrockId(openInventory.getBedrockId());
+
+                // Also mirror other properties - in case we're e.g. dealing with a pending virtual inventory
+                boolean pending = openInventory.isPending();
+                newInventory.setDisplayed(openInventory.isDisplayed());
+                newInventory.setPending(pending);
+                newInventory.setCurrentlyDelayed(openInventory.isCurrentlyDelayed());
+                session.setOpenInventory(newInventory);
+
+                GeyserImpl.getInstance().getLogger().debug(session, "Able to reuse current inventory, matching Bedrock id (%s). Is current pending? %s",
+                    openInventory.getBedrockId(), pending);
+
+                // If the current inventory is still pending, it'll be updated once open
+                if (newInventory.isDisplayed()) {
+                    newTranslator.updateInventory(session, newInventory);
+                }
+
+                return;
+            }
 
             InventoryUtils.closeInventory(session, openInventory.getJavaId(), true);
         }

core/src/main/java/org/geysermc/geyser/util/InventoryUtils.java

@@ -104,7 +104,7 @@ public class InventoryUtils {
             // Wait for close confirmation from client before opening the new inventory.
             // Handled in BedrockContainerCloseTranslator
             // or - client hasn't yet loaded in; wait until inventory is shown
-            GeyserImpl.getInstance().getLogger().debug(session, "Inv (%s) set pending: closing inv? %s, pending inv id? %s", inventory.getJavaId(), session.isClosingInventory(), session.getPendingInventoryId());
+            GeyserImpl.getInstance().getLogger().debug(session, "Inventory (%s) set pending: closing inv? %s, pending inv id? %s", inventory.getJavaId(), session.isClosingInventory(), session.getPendingInventoryId());
             inventory.setPending(true);
             return;
         }
@@ -208,9 +208,9 @@ public class InventoryUtils {
                 session.setClosingInventory(true);
             }
             session.getBundleCache().onInventoryClose(inventory);
+            GeyserImpl.getInstance().getLogger().debug(session, "Closed inventory: (java id: %s/bedrock id: %s), waiting on confirm? %s", inventory.getJavaId(), inventory.getBedrockId(), session.isClosingInventory());
         }
 
-        GeyserImpl.getInstance().getLogger().debug(session, "Closed inventory: " + (inventory != null ? inventory.getJavaId() : "null") + " Waiting on confirm? %s", session.isClosingInventory());
         session.setOpenInventory(null);
     }