Fixes related to hacky plugins that spam open screen packets

Author: onebeastchris

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

@@ -299,9 +299,10 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
     /**
      * Stores the bedrock inventory id of the pending inventory, or -1 if no inventory is pending.
+     * This id is only set when the block that should be opened exists.
      */
     @Setter
-    private int pendingInventoryId = -1;
+    private int pendingOrCurrentBedrockInventoryId = -1;
 
     /**
      * Use {@link #getNextItemNetId()} instead for consistency

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

@@ -31,6 +31,7 @@
 import org.geysermc.geyser.inventory.Inventory;
 import org.geysermc.geyser.inventory.MerchantContainer;
 import org.geysermc.geyser.session.GeyserSession;
+import org.geysermc.geyser.translator.inventory.MerchantInventoryTranslator;
 import org.geysermc.geyser.translator.protocol.PacketTranslator;
 import org.geysermc.geyser.translator.protocol.Translator;
 import org.geysermc.geyser.translator.protocol.java.inventory.JavaMerchantOffersTranslator;
@@ -56,28 +57,26 @@ public void translate(GeyserSession session, ContainerClosePacket packet) {
         Inventory openInventory = session.getOpenInventory();
         if (bedrockId == -1 && openInventory != null) {
             // 1.16.200 - window ID is always -1 sent from Bedrock for merchant containers
-            bedrockId = (byte) openInventory.getBedrockId();
+            if (openInventory.getTranslator() instanceof MerchantInventoryTranslator) {
+                bedrockId = (byte) openInventory.getBedrockId();
+            }
 
             // If virtual inventories are opened too quickly, they can be occasionally rejected
             // We just try and queue a new one.
-            if (openInventory.getTranslator().requiresOpeningDelay(session, openInventory)) {
+            if (openInventory.getBedrockId() == session.getPendingOrCurrentBedrockInventoryId()) {
+                // Before making another attempt to re-open, let's make sure we actually need this inventory open.
                 if (session.getContainerOpenAttempts() < 3) {
                     openInventory.setPending(true);
                     openInventory.setDelayed(true);
-                    session.setPendingInventoryId(openInventory.getBedrockId());
+                    session.setPendingOrCurrentBedrockInventoryId(openInventory.getBedrockId());
 
-                    byte finalBedrockId = bedrockId;
                     session.scheduleInEventLoop(() -> {
-                        if (InventoryUtils.shouldQueueRejectedInventory(session)) {
-                            NetworkStackLatencyPacket latencyPacket = new NetworkStackLatencyPacket();
-                            latencyPacket.setFromServer(true);
-                            latencyPacket.setTimestamp(MAGIC_VIRTUAL_INVENTORY_HACK);
-                            session.sendUpstreamPacket(latencyPacket);
-                            GeyserImpl.getInstance().getLogger().debug(session, "Unable to open a virtual inventory, sending another latency packet!");
-                        } else {
-                            closeCurrentOrOpenPending(session, finalBedrockId, session.getOpenInventory());
-                        }
-                    }, 200, TimeUnit.MILLISECONDS);
+                        NetworkStackLatencyPacket latencyPacket = new NetworkStackLatencyPacket();
+                        latencyPacket.setFromServer(true);
+                        latencyPacket.setTimestamp(MAGIC_VIRTUAL_INVENTORY_HACK);
+                        session.sendUpstreamPacket(latencyPacket);
+                        GeyserImpl.getInstance().getLogger().debug(session, "Unable to open a virtual inventory, sent another latency packet!");
+                    }, 100, TimeUnit.MILLISECONDS);
                     return;
                 } else {
                     GeyserImpl.getInstance().getLogger().debug(session, "Exceeded 3 attempts to open a virtual inventory!");
@@ -86,6 +85,7 @@ public void translate(GeyserSession session, ContainerClosePacket packet) {
             }
         }
 
+        session.setPendingOrCurrentBedrockInventoryId(-1);
         session.setContainerOpenAttempts(0);
         closeCurrentOrOpenPending(session, bedrockId, openInventory);
     }

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

@@ -65,7 +65,7 @@ public void translate(GeyserSession session, NetworkStackLatencyPacket packet) {
             return;
         }
 
-        if (session.getPendingInventoryId() != -1) {
+        if (session.getPendingOrCurrentBedrockInventoryId() != -1) {
             InventoryUtils.openPendingInventory(session);
         } else {
             session.scheduleInEventLoop(() -> {

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

@@ -100,12 +100,12 @@ public class InventoryUtils {
      */
     public static void openInventory(GeyserSession session, Inventory inventory) {
         session.setOpenInventory(inventory);
-        if (session.isClosingInventory() || !session.getUpstream().isInitialized() || session.getPendingInventoryId() != -1) {
+        if (session.isClosingInventory() || !session.getUpstream().isInitialized() || session.getPendingOrCurrentBedrockInventoryId() != -1) {
             // 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, "Inventory (%s) set pending: closing inv? %s, pending inv id? %s", debugInventory(inventory), session.isClosingInventory(), session.getPendingInventoryId());
             inventory.setPending(true);
+            GeyserImpl.getInstance().getLogger().debug(session, "Inventory (%s) set pending: closing inv? %s, pending inv id? %s", debugInventory(inventory), session.isClosingInventory(), session.getPendingOrCurrentBedrockInventoryId());
             return;
         }
         displayInventory(session, inventory);
@@ -119,32 +119,21 @@ public static void openInventory(GeyserSession session, Inventory inventory) {
     public static void openPendingInventory(GeyserSession session) {
         Inventory currentInventory = session.getOpenInventory();
         if (currentInventory == null || !currentInventory.isPending()) {
+            session.setPendingOrCurrentBedrockInventoryId(-1);
             GeyserImpl.getInstance().getLogger().debug(session, "No pending inventory, not opening an inventory! Current inventory: %s", debugInventory(currentInventory));
-            session.setPendingInventoryId(-1);
             return;
         }
 
         // Current inventory isn't null! Let's see if we need to open it.
-        if (currentInventory.isDelayed() && currentInventory.getBedrockId() == session.getPendingInventoryId()) {
+        if (currentInventory.isDelayed() && currentInventory.getBedrockId() == session.getPendingOrCurrentBedrockInventoryId()) {
             GeyserImpl.getInstance().getLogger().debug(session, "Attempting to open currently delayed inventory with matching bedrock id! " + currentInventory.getBedrockId());
             openAndUpdateInventory(session, currentInventory);
             return;
         }
 
         GeyserImpl.getInstance().getLogger().debug(session, "Opening any pending inventory! " + debugInventory(currentInventory));
-
-        session.setPendingInventoryId(-1);
-        openInventory(session, currentInventory);
-    }
-
-    public static boolean shouldQueueRejectedInventory(GeyserSession session) {
-        Inventory currentInventory = session.getOpenInventory();
-        if (currentInventory == null || !currentInventory.isDelayed() || currentInventory.getBedrockId() != session.getPendingInventoryId()) {
-            GeyserImpl.getInstance().getLogger().debug(session, "Aborting NetworkStackLatency hack as the inventory has changed!");
-            return false;
-        }
-
-        return true;
+        session.setPendingOrCurrentBedrockInventoryId(-1);
+        displayInventory(session, currentInventory);
     }
 
     /**
@@ -157,7 +146,7 @@ public static void displayInventory(GeyserSession session, Inventory inventory)
             if (translator.requiresOpeningDelay(session, inventory)) {
                 inventory.setPending(true);
                 inventory.setDelayed(true);
-                session.setPendingInventoryId(inventory.getBedrockId());
+                session.setPendingOrCurrentBedrockInventoryId(inventory.getBedrockId());
 
                 NetworkStackLatencyPacket latencyPacket = new NetworkStackLatencyPacket();
                 latencyPacket.setFromServer(true);
@@ -184,7 +173,6 @@ public static void openAndUpdateInventory(GeyserSession session, Inventory inven
         inventory.setDisplayed(true);
         inventory.setPending(false);
         inventory.setDelayed(false);
-        session.setPendingInventoryId(-1);
     }
 
     /**
@@ -221,6 +209,7 @@ public static void closeInventory(GeyserSession session, int javaId, boolean con
             GeyserImpl.getInstance().getLogger().debug(session, "Closed inventory: (java id: %s/bedrock id: %s), waiting on confirm? %s", inventory.getJavaId(), inventory.getBedrockId(), session.isClosingInventory());
         }
 
+        session.setPendingOrCurrentBedrockInventoryId(-1);
         session.setOpenInventory(null);
     }