Feature: Fixup form closing (#5872)

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

@@ -110,6 +110,11 @@ public class PlayerInventory extends Inventory {
         items[36 + heldItemSlot] = item;
     }
 
+    @Override
+    public boolean shouldConfirmContainerClose() {
+        return false;
+    }
+
     public GeyserItemStack getOffhand() {
         return items[45];
     }

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

@@ -84,7 +84,6 @@ import org.cloudburstmc.protocol.bedrock.packet.BedrockPacket;
 import org.cloudburstmc.protocol.bedrock.packet.BiomeDefinitionListPacket;
 import org.cloudburstmc.protocol.bedrock.packet.CameraPresetsPacket;
 import org.cloudburstmc.protocol.bedrock.packet.ChunkRadiusUpdatedPacket;
-import org.cloudburstmc.protocol.bedrock.packet.ClientboundCloseFormPacket;
 import org.cloudburstmc.protocol.bedrock.packet.CreativeContentPacket;
 import org.cloudburstmc.protocol.bedrock.packet.DimensionDataPacket;
 import org.cloudburstmc.protocol.bedrock.packet.EmoteListPacket;
@@ -1674,16 +1673,29 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
     public boolean sendForm(@NonNull Form form) {
         // First close any dialogs that are open. This won't execute the dialog's closing action.
         dialogManager.close();
+        // Also close all currently open forms.
+        if (formCache.hasFormOpen()) {
+            closeForm();
+        }
+
+        // Cache this form, let's see whether we can open it immediately
+        formCache.addForm(form);
+
         // Also close current inventories, otherwise the form will not show
         if (inventoryHolder != null) {
             // We'll open the form when the client confirms current inventory being closed
-            formCache.addForm(form);
             InventoryUtils.sendJavaContainerClose(inventoryHolder);
             InventoryUtils.closeInventory(this, inventoryHolder, true);
-            return true;
-        } else {
-            return doSendForm(form);
         }
+
+        // Open the current form, unless we're in the process of closing another
+        // If we're waiting, the form will be sent when Bedrock confirms closing
+        // If we don't wait, the client rejects the form as it is busy
+        if (!isClosingInventory()) {
+            formCache.resendAllForms();
+        }
+
+        return true;
     }
 
     /**
@@ -2406,7 +2418,7 @@ public class GeyserSession implements GeyserConnection, GeyserCommandSource {
 
     @Override
     public void closeForm() {
-        sendUpstreamPacket(new ClientboundCloseFormPacket());
+        formCache.closeForms();
     }
 
     public void addCommandEnum(String name, String enums) {

core/src/main/java/org/geysermc/geyser/session/cache/FormCache.java

@@ -27,7 +27,10 @@ package org.geysermc.geyser.session.cache;
 
 import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
+import it.unimi.dsi.fastutil.ints.IntArrayList;
+import it.unimi.dsi.fastutil.ints.IntList;
 import lombok.RequiredArgsConstructor;
+import org.cloudburstmc.protocol.bedrock.packet.ClientboundCloseFormPacket;
 import org.cloudburstmc.protocol.bedrock.packet.ModalFormRequestPacket;
 import org.cloudburstmc.protocol.bedrock.packet.ModalFormResponsePacket;
 import org.cloudburstmc.protocol.bedrock.packet.NetworkStackLatencyPacket;
@@ -51,10 +54,15 @@ public class FormCache {
     private final FormDefinitions formDefinitions = FormDefinitions.instance();
     private final AtomicInteger formIdCounter = new AtomicInteger(0);
     private final Int2ObjectMap<Form> forms = new Int2ObjectOpenHashMap<>();
+    private final IntList sentFormIds = new IntArrayList();
     private final GeyserSession session;
 
     public boolean hasFormOpen() {
-        return !forms.isEmpty();
+        // If forms is empty it implies that there are no forms to show
+        // so technically this returns "has forms to show" or "has open"
+        // Forms are only queued in specific circumstances, such as waiting on
+        // previous inventories to close
+        return !forms.isEmpty() && !sentFormIds.isEmpty();
     }
 
     public int addForm(Form form) {
@@ -74,6 +82,9 @@ public class FormCache {
     private void sendForm(int formId, Form form) {
         String jsonData = formDefinitions.codecFor(form).jsonData(form);
 
+        // Store that this form has been sent
+        sentFormIds.add(formId);
+
         ModalFormRequestPacket formRequestPacket = new ModalFormRequestPacket();
         formRequestPacket.setFormId(formId);
         formRequestPacket.setFormData(jsonData);
@@ -99,6 +110,7 @@ public class FormCache {
 
     public void handleResponse(ModalFormResponsePacket response) {
         Form form = forms.remove(response.getFormId());
+        this.sentFormIds.rem(response.getFormId());
         if (form == null) {
             return;
         }
@@ -110,4 +122,18 @@ public class FormCache {
             GeyserImpl.getInstance().getLogger().error("Error while processing form response!", e);
         }
     }
+
+    public void closeForms() {
+        if (!forms.isEmpty()) {
+            // Check if there are any forms that have not been sent to the client yet
+            for (Int2ObjectMap.Entry<Form> entry : forms.int2ObjectEntrySet()) {
+                if (!sentFormIds.contains(entry.getIntKey())) {
+                    // This will send the form, but close it instantly with the packet later
+                    // ...thereby clearing our list!
+                    sendForm(entry.getIntKey(), entry.getValue());
+                }
+            }
+            session.sendUpstreamPacket(new ClientboundCloseFormPacket());
+        }
+    }
 }