fix #182

Author: brachy84

src/main/java/com/cleanroommc/modularui/CommonProxy.java

@@ -8,7 +8,6 @@
 import com.cleanroommc.modularui.screen.ModularContainer;
 import com.cleanroommc.modularui.test.ItemEditorGui;
 import com.cleanroommc.modularui.test.TestBlock;
-import com.cleanroommc.modularui.value.sync.ModularSyncManager;
 
 import net.minecraft.util.Timer;
 import net.minecraftforge.common.MinecraftForge;
@@ -67,12 +66,16 @@ public void registerBlocks(RegistryEvent.Register<EntityEntry> event) {
     }
 
     @SubscribeEvent
-    public void onCloseContainer(PlayerContainerEvent.Open event) {
+    public void onOpenContainer(PlayerContainerEvent.Open event) {
         if (event.getContainer() instanceof ModularContainer container) {
-            ModularSyncManager syncManager = container.getSyncManager();
-            if (syncManager != null) {
-                syncManager.onOpen();
-            }
+            container.onModularContainerOpened();
+        }
+    }
+
+    @SubscribeEvent
+    public void onCloseContainer(PlayerContainerEvent.Close event) {
+        if (event.getContainer() instanceof ModularContainer container) {
+            container.onModularContainerClosed();
         }
     }
 

src/main/java/com/cleanroommc/modularui/screen/ClientScreenHandler.java

@@ -226,7 +226,7 @@ private static void invalidateCurrentScreen() {
         if (lastMui != null) {
             ((GuiScreenAccessor) lastMui.getGuiScreen()).setEventButton(-1);
             ((GuiScreenAccessor) lastMui.getGuiScreen()).setLastMouseEvent(-1);
-            lastMui.getScreen().getPanelManager().closeAll();
+            lastMui.getScreen().getPanelManager().closeScreen();
             lastMui = null;
         }
         currentScreen = null;

src/main/java/com/cleanroommc/modularui/screen/ModularContainer.java

@@ -13,6 +13,7 @@
 import com.cleanroommc.bogosorter.api.ISortableContainer;
 import com.cleanroommc.bogosorter.api.ISortingContextBuilder;
 
+import net.minecraft.client.gui.inventory.GuiContainer;
 import net.minecraft.entity.player.EntityPlayer;
 import net.minecraft.entity.player.InventoryPlayer;
 import net.minecraft.inventory.ClickType;
@@ -101,11 +102,21 @@ public ContainerAccessor acc() {
     }
 
     @MustBeInvokedByOverriders
-    @Override
-    public void onContainerClosed(@NotNull EntityPlayer playerIn) {
-        super.onContainerClosed(playerIn);
+    public void onModularContainerOpened() {
+        if (this.syncManager != null) {
+            this.syncManager.onOpen();
+        }
+    }
+
+    /**
+     * Called when this container closes. This is different to {@link Container#onContainerClosed(EntityPlayer)}, since that one is also
+     * called from {@link GuiContainer#onGuiClosed()}, which means it is called even when the container may still exist.
+     * This happens when a temporary client screen takes over (like JEI,NEI,etc.). This is only called when the container actually closes.
+     */
+    @MustBeInvokedByOverriders
+    public void onModularContainerClosed() {
         if (this.syncManager != null) {
-            this.syncManager.onClose();
+            this.syncManager.dispose();
         }
     }
 
@@ -119,7 +130,7 @@ public void detectAndSendChanges() {
         this.init = false;
     }
 
-    @ApiStatus.Internal
+    @MustBeInvokedByOverriders
     public void onUpdate() {
         // detectAndSendChanges is potentially called multiple times per tick, while this method is called exactly once per tick
         if (this.syncManager != null) {

src/main/java/com/cleanroommc/modularui/screen/ModularPanel.java

@@ -244,13 +244,8 @@ public void onOpen(ModularScreen screen) {
         this.state = State.OPEN;
     }
 
-    boolean reopen(boolean strict) {
-        if (this.state != State.CLOSED) {
-            if (strict) throw new IllegalStateException();
-            return false;
-        }
+    void reopen() {
         this.state = State.OPEN;
-        return true;
     }
 
     @MustBeInvokedByOverriders

src/main/java/com/cleanroommc/modularui/screen/PanelManager.java

@@ -60,7 +60,7 @@ boolean tryInit() {
                     throw new IllegalStateException("Tried to reopen closed screen, but all panels are disposed!");
                 }
                 // set all stored panels to be open
-                this.panels.forEach(p -> p.reopen(true));
+                this.panels.forEach(ModularPanel::reopen);
                 this.disposal.removeIf(this.panels::contains);
                 setState(State.REOPENED);
                 yield true;
@@ -226,8 +226,17 @@ public boolean closeAll() {
         return false;
     }
 
+    void closeScreen() {
+        // only close the screen without closing the panels
+        // this is useful when we expect the screen to reopen at some point and the sync managers are still available
+        if (this.state.isOpen) {
+            setState(State.CLOSED);
+            this.screen.onClose();
+        }
+    }
+
     private void finalizePanel(ModularPanel panel) {
-        panel.onClose();
+        if (panel.isOpen()) panel.onClose();
         if (!this.disposal.contains(panel)) {
             if (this.disposal.size() == DISPOSAL_CAPACITY) {
                 this.disposal.removeFirst().dispose();
@@ -258,6 +267,8 @@ public void dispose() {
             setState(State.WAIT_DISPOSAL);
             return;
         }
+        // make sure every panel gets closed before disposing
+        this.panels.forEach(this::finalizePanel);
         setState(State.CLOSED);
         this.disposal.forEach(ModularPanel::dispose);
         this.disposal.clear();
@@ -449,7 +460,7 @@ public enum State {
          */
         REOPENED(true),
         /**
-         * Screen is closed after it was open.
+         * Screen is closed after it was open. Panels may still be considered open in some cases.
          */
         CLOSED(false),
         /**

src/main/java/com/cleanroommc/modularui/test/TestTile.java

@@ -158,7 +158,7 @@ public ModularPanel buildUI(PosGuiData guiData, PanelSyncManager syncManager, UI
 
         Rectangle colorPickerBackground = new Rectangle().setColor(Color.RED.main);
         ModularPanel panel = new ModularPanel("test_tile");
-        IPanelHandler panelSyncHandler = syncManager.panel("other_panel", this::openSecondWindow, true);
+        IPanelHandler panelSyncHandler = syncManager.syncedPanel("other_panel", true, this::openSecondWindow);
         IPanelHandler colorPicker = IPanelHandler.simple(panel, (mainPanel, player) -> new ColorPickerDialog(colorPickerBackground::setColor, colorPickerBackground.getColor(), true)
                 .setDraggable(true)
                 .relative(panel)
@@ -502,8 +502,8 @@ public ModularPanel openSecondWindow(PanelSyncManager syncManager, IPanelHandler
         syncManager.registerSlotGroup(slotGroup);
         AtomicInteger number = new AtomicInteger(0);
         syncManager.syncValue("int_value", new IntSyncValue(number::get, number::set));
-        IPanelHandler panelSyncHandler = syncManager.panel("other_panel_2", (syncManager1, syncHandler1) ->
-                openThirdWindow(syncManager1, syncHandler1, number), true);
+        IPanelHandler panelSyncHandler = syncManager.syncedPanel("other_panel_2", true, (syncManager1, syncHandler1) ->
+                openThirdWindow(syncManager1, syncHandler1, number));
         panel.child(ButtonWidget.panelCloseButton())
                 .child(new ButtonWidget<>()
                         .size(10).top(14).right(4)

src/main/java/com/cleanroommc/modularui/test/TestTile2.java

@@ -54,7 +54,7 @@ public ModularPanel buildUI(PosGuiData data, PanelSyncManager syncManager, UISet
                     .slot(new ModularSlot(this.itemHandler, i)));
         }
         ModularPanel panel = ModularPanel.defaultPanel("test_tile_2", 176, 13 * 18 + 14 + 10 + 20);
-        IPanelHandler otherPanel = syncManager.panel("2nd panel", (syncManager1, syncHandler) -> {
+        IPanelHandler otherPanel = syncManager.syncedPanel("2nd panel", true, (syncManager1, syncHandler) -> {
             ModularPanel panel1 = new Dialog<>("Option Selection").setDisablePanelsBelow(false).setDraggable(true).size(4 * 18 + 8, 4 * 18 + 8);
             return panel1
                     .child(SlotGroupWidget.builder()
@@ -66,7 +66,7 @@ public ModularPanel buildUI(PosGuiData data, PanelSyncManager syncManager, UISet
                             .build()
                             .pos(4, 4)
                     );
-        }, true);
+        });
         return panel
                 .bindPlayerInventory()
                 .child(sw)

src/main/java/com/cleanroommc/modularui/value/sync/ModularSyncManager.java

@@ -61,8 +61,9 @@ public void detectAndSendChanges(boolean init) {
         this.panelSyncManagerMap.values().forEach(psm -> psm.detectAndSendChanges(init));
     }
 
-    public void onClose() {
+    public void dispose() {
         this.panelSyncManagerMap.values().forEach(PanelSyncManager::onClose);
+        this.panelSyncManagerMap.clear();
     }
 
     public void onOpen() {