From 9e6402c279850e67cec31a66b53c59516d8b62c7 Mon Sep 17 00:00:00 2001
From: AJ Ferguson <AJ-Ferguson@users.noreply.github.com>
Date: Wed, 24 Feb 2021 20:14:01 -0900
Subject: [PATCH] Fixes

---
 .../connector/inventory/Inventory.java        |  4 ++
 .../network/session/GeyserSession.java        |  2 +
 .../BedrockContainerCloseTranslator.java      | 25 ++++++-----
 .../BedrockLecternUpdateTranslator.java       |  2 +-
 .../LecternInventoryTranslator.java           |  4 +-
 .../chest/DoubleChestInventoryTranslator.java |  6 +++
 .../java/JavaRespawnTranslator.java           |  1 +
 .../window/JavaCloseWindowTranslator.java     |  2 +-
 .../java/window/JavaOpenWindowTranslator.java |  4 +-
 .../connector/utils/InventoryUtils.java       | 42 ++++++++++++-------
 10 files changed, 59 insertions(+), 33 deletions(-)

diff --git a/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java b/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java
index 535a9077a..179801825 100644
--- a/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java
+++ b/connector/src/main/java/org/geysermc/connector/inventory/Inventory.java
@@ -61,6 +61,10 @@ public class Inventory {
     @Getter
     protected short transactionId = 0;
 
+    @Getter
+    @Setter
+    private boolean pending = false;
+
     protected Inventory(int id, int size) {
         this("Inventory", id, size);
     }
diff --git a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java
index 41269ebf1..f2fa98ba4 100644
--- a/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java
+++ b/connector/src/main/java/org/geysermc/connector/network/session/GeyserSession.java
@@ -136,6 +136,8 @@ public class GeyserSession implements CommandSender {
     private final PlayerInventory playerInventory;
     @Setter
     private Inventory openInventory;
+    @Setter
+    private boolean closingInventory;
 
     @Setter
     private InventoryTranslator inventoryTranslator = InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR;
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java
index 6f0f877c8..d46a51ef1 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockContainerCloseTranslator.java
@@ -31,6 +31,7 @@ import org.geysermc.connector.inventory.Inventory;
 import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.PacketTranslator;
 import org.geysermc.connector.network.translators.Translator;
+import org.geysermc.connector.network.translators.inventory.InventoryTranslator;
 import org.geysermc.connector.utils.InventoryUtils;
 
 @Translator(packet = ContainerClosePacket.class)
@@ -42,19 +43,21 @@ public class BedrockContainerCloseTranslator extends PacketTranslator<ContainerC
             session.setLastWindowCloseTime(0);
             byte windowId = packet.getId();
 
-            if (windowId == -1 && session.getOpenInventory() != null) {
-                windowId = (byte) session.getOpenInventory().getId();
-            }
-
-            Inventory openInventory = session.getOpenInventory();
-            if (openInventory != null && windowId == openInventory.getId()) {
-                ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(windowId);
-                session.sendDownstreamPacket(closeWindowPacket);
-                InventoryUtils.closeInventory(session, windowId);
-            }
-
             //Client wants close confirmation
             session.sendUpstreamPacket(packet);
+            session.setClosingInventory(false);
+
+            Inventory openInventory = session.getOpenInventory();
+            if (openInventory != null) {
+                if (windowId == openInventory.getId()) {
+                    ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(windowId);
+                    session.sendDownstreamPacket(closeWindowPacket);
+                    InventoryUtils.closeInventory(session, windowId, false);
+                } else if (openInventory.isPending()) {
+                    InventoryUtils.displayInventory(session, openInventory);
+                    openInventory.setPending(false);
+                }
+            }
         });
     }
 }
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockLecternUpdateTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockLecternUpdateTranslator.java
index c4f50c6da..99dcebed9 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockLecternUpdateTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/bedrock/BedrockLecternUpdateTranslator.java
@@ -70,7 +70,7 @@ public class BedrockLecternUpdateTranslator extends PacketTranslator<LecternUpda
                 // The same page means Bedrock is closing the window
                 ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(lecternContainer.getId());
                 session.sendDownstreamPacket(closeWindowPacket);
-                InventoryUtils.closeInventory(session, lecternContainer.getId());
+                InventoryUtils.closeInventory(session, lecternContainer.getId(), false);
             } else {
                 // Each "page" Bedrock gives to us actually represents two pages (think opening a book and seeing two pages)
                 // Each "page" on Java is just one page (think a spiral notebook folded back to only show one page)
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/LecternInventoryTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/LecternInventoryTranslator.java
index 157f63164..05529da3e 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/LecternInventoryTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/LecternInventoryTranslator.java
@@ -91,7 +91,7 @@ public class LecternInventoryTranslator extends BaseInventoryTranslator {
                 ClientClickWindowButtonPacket packet = new ClientClickWindowButtonPacket(inventory.getId(), 3);
                 session.sendDownstreamPacket(packet);
                 session.setDroppingLecternBook(false);
-                InventoryUtils.closeInventory(session, inventory.getId());
+                InventoryUtils.closeInventory(session, inventory.getId(), false);
             } else if (lecternContainer.getBlockEntityTag() == null) {
                 // If the method returns true, this is already handled for us
                 GeyserItemStack geyserItemStack = inventory.getItem(0);
@@ -123,7 +123,7 @@ public class LecternInventoryTranslator extends BaseInventoryTranslator {
                         // Close the window - we will reopen it once the client has this data synced
                         ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(lecternContainer.getId());
                         session.sendDownstreamPacket(closeWindowPacket);
-                        InventoryUtils.closeInventory(session, inventory.getId());
+                        InventoryUtils.closeInventory(session, inventory.getId(), false);
                     }
                 }
             }
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/DoubleChestInventoryTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/DoubleChestInventoryTranslator.java
index 5a5c8209d..66766e1b9 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/DoubleChestInventoryTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/inventory/translators/chest/DoubleChestInventoryTranslator.java
@@ -31,6 +31,7 @@ import com.nukkitx.nbt.NbtMap;
 import com.nukkitx.nbt.NbtMapBuilder;
 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;
@@ -142,6 +143,11 @@ public class DoubleChestInventoryTranslator extends ChestInventoryTranslator {
     public void closeInventory(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;
         }
 
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/JavaRespawnTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/JavaRespawnTranslator.java
index 55fdcacd1..7c8cd0583 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/java/JavaRespawnTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/JavaRespawnTranslator.java
@@ -52,6 +52,7 @@ public class JavaRespawnTranslator extends PacketTranslator<ServerRespawnPacket>
         session.addInventoryTask(() -> {
             session.setInventoryTranslator(InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR);
             session.setOpenInventory(null);
+            session.setClosingInventory(false);
         });
 
         SetPlayerGameTypePacket playerGameTypePacket = new SetPlayerGameTypePacket();
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java
index b78dc32f2..639996203 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaCloseWindowTranslator.java
@@ -36,6 +36,6 @@ public class JavaCloseWindowTranslator extends PacketTranslator<ServerCloseWindo
 
     @Override
     public void translate(ServerCloseWindowPacket packet, GeyserSession session) {
-        session.addInventoryTask(() -> InventoryUtils.closeInventory(session, packet.getWindowId()));
+        session.addInventoryTask(() -> InventoryUtils.closeInventory(session, packet.getWindowId(), true));
     }
 }
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java
index 2d1f77317..db20dfcd2 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/window/JavaOpenWindowTranslator.java
@@ -51,7 +51,7 @@ public class JavaOpenWindowTranslator extends PacketTranslator<ServerOpenWindowP
             //No translator exists for this window type. Close all windows and return.
             if (newTranslator == null) {
                 if (openInventory != null) {
-                    InventoryUtils.closeInventory(session, openInventory.getId());
+                    InventoryUtils.closeInventory(session, openInventory.getId(), true);
                 }
                 ClientCloseWindowPacket closeWindowPacket = new ClientCloseWindowPacket(packet.getWindowId());
                 session.sendDownstreamPacket(closeWindowPacket);
@@ -63,7 +63,7 @@ public class JavaOpenWindowTranslator extends PacketTranslator<ServerOpenWindowP
 
             Inventory newInventory = newTranslator.createInventory(name, packet.getWindowId(), packet.getType(), session.getPlayerInventory());
             if (openInventory != null) {
-                InventoryUtils.closeInventory(session, openInventory.getId());
+                InventoryUtils.closeInventory(session, openInventory.getId(), true);
             }
 
             session.setInventoryTranslator(newTranslator);
diff --git a/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java b/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java
index 81f59cf3f..cf235b4c7 100644
--- a/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java
+++ b/connector/src/main/java/org/geysermc/connector/utils/InventoryUtils.java
@@ -39,11 +39,10 @@ import com.nukkitx.protocol.bedrock.packet.InventorySlotPacket;
 import com.nukkitx.protocol.bedrock.packet.PlayerHotbarPacket;
 import org.geysermc.connector.GeyserConnector;
 import org.geysermc.connector.common.ChatColor;
-import org.geysermc.connector.inventory.GeyserItemStack;
-import org.geysermc.connector.inventory.Inventory;
-import org.geysermc.connector.inventory.PlayerInventory;
+import org.geysermc.connector.inventory.*;
 import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.inventory.InventoryTranslator;
+import org.geysermc.connector.network.translators.inventory.translators.LecternInventoryTranslator;
 import org.geysermc.connector.network.translators.inventory.translators.chest.DoubleChestInventoryTranslator;
 import org.geysermc.connector.network.translators.item.ItemEntry;
 import org.geysermc.connector.network.translators.item.ItemRegistry;
@@ -56,21 +55,30 @@ public class InventoryUtils {
     public static final ItemStack REFRESH_ITEM = new ItemStack(1, 127, new CompoundTag(""));
 
     public static void openInventory(GeyserSession session, Inventory inventory) {
+        session.setOpenInventory(inventory);
+        if (session.isClosingInventory()) {
+            //Wait for close confirmation from client before opening the new inventory.
+            //Handled in BedrockContainerCloseTranslator
+            inventory.setPending(true);
+            return;
+        }
+        displayInventory(session, inventory);
+    }
+
+    public static void displayInventory(GeyserSession session, Inventory inventory) {
         InventoryTranslator translator = session.getInventoryTranslator();
         if (translator != null) {
-            session.setOpenInventory(inventory);
             translator.prepareInventory(session, inventory);
-            //Ensure at least half a second passes between closing and opening a new window
-            //The client will not open the new window if it is still closing the old one
-            long delay = 700 - (System.currentTimeMillis() - session.getLastWindowCloseTime());
-            if (translator instanceof DoubleChestInventoryTranslator) {
-                delay = Math.max(delay, 200);
-            }
-            if (delay > 0) {
+            if (translator instanceof DoubleChestInventoryTranslator && !((Container) inventory).isUsingRealBlock()) {
                 GeyserConnector.getInstance().getGeneralThreadPool().schedule(() -> {
-                    translator.openInventory(session, inventory);
-                    translator.updateInventory(session, inventory);
-                }, delay, TimeUnit.MILLISECONDS);
+                    session.addInventoryTask(() -> {
+                        Inventory openInv = session.getOpenInventory();
+                        if (openInv != null && openInv.getId() == inventory.getId()) {
+                            translator.openInventory(session, inventory);
+                            translator.updateInventory(session, inventory);
+                        }
+                    });
+                }, 200, TimeUnit.MILLISECONDS);
             } else {
                 translator.openInventory(session, inventory);
                 translator.updateInventory(session, inventory);
@@ -78,7 +86,7 @@ public class InventoryUtils {
         }
     }
 
-    public static void closeInventory(GeyserSession session, int windowId) {
+    public static void closeInventory(GeyserSession session, int windowId, boolean confirm) {
         session.getPlayerInventory().setCursor(GeyserItemStack.EMPTY, session);
         updateCursor(session);
 
@@ -86,7 +94,9 @@ public class InventoryUtils {
         if (inventory != null) {
             InventoryTranslator translator = session.getInventoryTranslator();
             translator.closeInventory(session, inventory);
-            session.setLastWindowCloseTime(System.currentTimeMillis());
+            if (confirm && !inventory.isPending() && !(translator instanceof LecternInventoryTranslator)) {
+                session.setClosingInventory(true);
+            }
         }
         session.setInventoryTranslator(InventoryTranslator.PLAYER_INVENTORY_TRANSLATOR);
         session.setOpenInventory(null);