fix: Reduce times player interfaces are drawn on re-opening

Previously a background open player interface would be seen as open causing draws that got overwritten

Author: Aeltumn

examples/src/main/kotlin/com/noxcrew/interfaces/example/ExamplePlugin.kt

@@ -45,7 +45,7 @@ public class ExamplePlugin : JavaPlugin(), Listener {
 
                 suspendingHandler {
                     val player = it.sender() as Player
-                    InterfacesListeners.INSTANCE.getOpenInterface(player.uniqueId)?.close()
+                    InterfacesListeners.INSTANCE.getOpenPlayerInterface(player.uniqueId)?.close()
                     player.inventory.clear()
                 }
             }

interfaces/src/main/kotlin/com/noxcrew/interfaces/InterfacesListeners.kt

@@ -150,44 +150,73 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
     private val queries: Cache<UUID, ChatQuery> = Caffeine.newBuilder()
         .build()
 
-    /** A cache of open player interface views, with weak values. */
+    /** A cache of player interfaces that should be opened again in the future. */
+    private val backgroundPlayerInterfaceViews: Cache<UUID, PlayerInterfaceView> = Caffeine.newBuilder()
+        .weakValues()
+        .build()
+
+    /** A cache of actively open player interfaces. */
     private val openPlayerInterfaceViews: Cache<UUID, PlayerInterfaceView> = Caffeine.newBuilder()
         .weakValues()
         .build()
 
-    /** Re-opens the current open interface of [player]. */
+    /** Re-opens the current background interface of [player]. */
     public fun reopenInventory(player: Player) {
-        getOpenInterface(player.uniqueId)?.also {
+        getBackgroundPlayerInterface(player.uniqueId)?.also {
             SCOPE.launch {
                 it.open()
             }
         }
     }
 
-    /** Returns the currently open interface for [playerId]. */
-    public fun getOpenInterface(playerId: UUID): PlayerInterfaceView? {
+    /**
+     * Returns the background interface for [playerId]. This is the last
+     * player interface that was opened, which should be re-opened once
+     * we no longer have anything else showing.
+     */
+    public fun getBackgroundPlayerInterface(playerId: UUID): PlayerInterfaceView? {
         // Check if the menu is definitely still meant to be open
+        val result = backgroundPlayerInterfaceViews.getIfPresent(playerId) ?: return null
+        if (result.shouldStillBeOpened) return result
+        backgroundPlayerInterfaceViews.invalidate(playerId)
+        return null
+    }
+
+    /**
+     * Returns the currently open player interface for [playerId].
+     */
+    public fun getOpenPlayerInterface(playerId: UUID): PlayerInterfaceView? {
         val result = openPlayerInterfaceViews.getIfPresent(playerId) ?: return null
-        if (result.shouldStillBeOpened) {
-            return result
-        }
+        if (result.shouldStillBeOpened) return result
         openPlayerInterfaceViews.invalidate(playerId)
         return null
     }
 
-    /** Updates the currently open interface for [playerId] to [view]. */
-    public fun setOpenInterface(playerId: UUID, view: PlayerInterfaceView?) {
+    /** Marks the given [view] as the opened player interface. */
+    public fun openPlayerInterface(playerId: UUID, view: PlayerInterfaceView) {
+        backgroundPlayerInterfaceViews.invalidate(playerId)
+        openPlayerInterfaceViews.put(playerId, view)
+    }
+
+    /** Closes the given [view] of a player interface. */
+    public fun closePlayerInterface(playerId: UUID, view: PlayerInterfaceView?) {
         // Save the contents of their currently shown inventory
         val bukkitPlayer = Bukkit.getPlayer(playerId)
         if (bukkitPlayer != null) {
             saveInventoryContentsIfOpened(bukkitPlayer)
         }
 
+        abortQuery(playerId, view)
         if (view == null) {
+            backgroundPlayerInterfaceViews.invalidate(playerId)
             openPlayerInterfaceViews.invalidate(playerId)
         } else {
-            abortQuery(playerId, null)
-            openPlayerInterfaceViews.put(playerId, view)
+            if (backgroundPlayerInterfaceViews.getIfPresent(playerId) === view) {
+                backgroundPlayerInterfaceViews.invalidate(playerId)
+            }
+            if (openPlayerInterfaceViews.getIfPresent(playerId) === view) {
+                openPlayerInterfaceViews.invalidate(playerId)
+            }
         }
     }
 
@@ -212,6 +241,13 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
         val holder = event.inventory.holder
         val view = convertHolderToInterfaceView(holder) ?: return
 
+        // Move the current open inventory to the background to indicate
+        // it is no longer the actually opened inventory!
+        openPlayerInterfaceViews.getIfPresent(event.player.uniqueId)?.also {
+            backgroundPlayerInterfaceViews.put(event.player.uniqueId, it)
+            openPlayerInterfaceViews.invalidate(event.player.uniqueId)
+        }
+
         // Abort any previous query the player had
         abortQuery(event.player.uniqueId, null)
         view.onOpen()
@@ -231,15 +267,15 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
 
         SCOPE.launch {
             // Determine if we can re-open a previous interface
-            val openInterface = getOpenInterface(event.player.uniqueId)
-            val shouldReopen = reason in REOPEN_REASONS && !event.player.isDead && openInterface != null
+            val backgroundInterface = getBackgroundPlayerInterface(event.player.uniqueId)
+            val shouldReopen = reason in REOPEN_REASONS && !event.player.isDead && backgroundInterface != null
 
             // Mark the current view as closed properly
             view.markClosed(reason)
 
             // If possible, open back up a previous interface
             if (shouldReopen) {
-                requireNotNull(openInterface).open()
+                requireNotNull(backgroundInterface).open()
             }
         }
     }
@@ -375,7 +411,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
     @EventHandler
     public fun onPlayerQuit(event: PlayerQuitEvent) {
         abortQuery(event.player.uniqueId, null)
-        setOpenInterface(event.player.uniqueId, null)
+        closePlayerInterface(event.player.uniqueId, null)
     }
 
     /** Returns whether [block] will trigger some interaction if clicked with [item]. */
@@ -388,7 +424,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
         if (event.useItemInHand() == Event.Result.DENY) return
 
         val player = event.player
-        val view = getOpenInterface(player.uniqueId) ?: return
+        val view = getOpenPlayerInterface(player.uniqueId) ?: return
 
         // If we are prioritizing block interactions we assure they are not happening first
         if (view.builder.prioritiseBlockInteractions) {
@@ -428,7 +464,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
     @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true)
     public fun onDropItem(event: PlayerDropItemEvent) {
         val player = event.player
-        val view = getOpenInterface(player.uniqueId) ?: return
+        val view = getOpenPlayerInterface(player.uniqueId) ?: return
         val slot = player.inventory.heldItemSlot
         val droppedSlot = GridPoint.at(3, slot)
 
@@ -441,7 +477,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
     @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true)
     public fun onSwapHands(event: PlayerSwapHandItemsEvent) {
         val player = event.player
-        val view = getOpenInterface(player.uniqueId) ?: return
+        val view = getOpenPlayerInterface(player.uniqueId) ?: return
         val slot = player.inventory.heldItemSlot
         val interactedSlot1 = GridPoint.at(3, slot)
         val interactedSlot2 = GridPoint.at(4, 4)
@@ -527,7 +563,7 @@ public class InterfacesListeners private constructor(private val plugin: Plugin)
         if (holder is AbstractInterfaceView<*, *, *>) return holder
 
         // If it's the player's own inventory use the held one
-        if (holder is HumanEntity) return getOpenInterface(holder.uniqueId)
+        if (holder is HumanEntity) return getOpenPlayerInterface(holder.uniqueId)
 
         return null
     }

interfaces/src/main/kotlin/com/noxcrew/interfaces/view/AbstractInterfaceView.kt

@@ -412,15 +412,21 @@ public abstract class AbstractInterfaceView<I : InterfacesInventory, T : Interfa
             drawPaneToInventory(drawNormalInventory = true, drawPlayerInventory = isOpen)
             callback(createdInventory)
 
-            if ((openIfClosed.get() && !isOpen) || createdInventory) {
-                InterfacesListeners.INSTANCE.viewBeingOpened = this
-                if (player.isConnected) openInventory()
-                if (InterfacesListeners.INSTANCE.viewBeingOpened == this) {
-                    InterfacesListeners.INSTANCE.viewBeingOpened = null
+            if (this is PlayerInterfaceView) {
+                // If this is a player inventory we can't update the inventory without
+                // opening it, so we trigger opening it properly.
+                if (!isOpen && player.isConnected) openInventory()
+            } else {
+                if ((openIfClosed.get() && !isOpen) || createdInventory) {
+                    InterfacesListeners.INSTANCE.viewBeingOpened = this
+                    if (player.isConnected) openInventory()
+                    if (InterfacesListeners.INSTANCE.viewBeingOpened == this) {
+                        InterfacesListeners.INSTANCE.viewBeingOpened = null
+                    }
                 }
-                openIfClosed.set(false)
-                firstPaint = false
             }
+            openIfClosed.set(false)
+            firstPaint = false
         }
     }
 

interfaces/src/main/kotlin/com/noxcrew/interfaces/view/PlayerInterfaceView.kt

@@ -34,29 +34,29 @@ public class PlayerInterfaceView internal constructor(
         // Close whatever inventory the player has open so they can look at their normal inventory!
         // This will only continue if the menu hasn't been closed yet.
         if (!isOpen()) {
-            // First we close then we set the interface so we don't double open!
-            InterfacesListeners.INSTANCE.setOpenInterface(player.uniqueId, null)
+            // Remove this inventory from the background interface before closing so it
+            // doesn't automatically re-open!
+            InterfacesListeners.INSTANCE.closePlayerInterface(player.uniqueId, this)
             player.closeInventory()
-            InterfacesListeners.INSTANCE.setOpenInterface(player.uniqueId, this)
         }
 
-        // Double-check that this inventory is open now!
-        if (isOpen()) {
-            if (!builder.inheritExistingItems) {
-                // Clear the player's inventory!
-                player.inventory.clear()
-                if (player.openInventory.topInventory.type == InventoryType.CRAFTING ||
-                    player.openInventory.topInventory.type == InventoryType.CREATIVE
-                ) {
-                    player.openInventory.topInventory.clear()
-                }
-                player.openInventory.setCursor(null)
-            }
+        // Open this player interface for the player
+        InterfacesListeners.INSTANCE.openPlayerInterface(player.uniqueId, this)
 
-            // Trigger onOpen manually because there is no real inventory being opened,
-            // this will also re-draw the player inventory parts!
-            onOpen()
+        if (!builder.inheritExistingItems) {
+            // Clear the player's inventory!
+            player.inventory.clear()
+            if (player.openInventory.topInventory.type == InventoryType.CRAFTING ||
+                player.openInventory.topInventory.type == InventoryType.CREATIVE
+            ) {
+                player.openInventory.topInventory.clear()
+            }
+            player.openInventory.setCursor(null)
         }
+
+        // Trigger onOpen manually because there is no real inventory being opened,
+        // this will also re-draw the player inventory parts!
+        onOpen()
     }
 
     override suspend fun close(reason: InventoryCloseEvent.Reason, changingView: Boolean) {
@@ -65,14 +65,10 @@ public class PlayerInterfaceView internal constructor(
         // Ensure we update the interface state in the main thread!
         // Even if the menu is not currently on the screen.
         InterfacesListeners.INSTANCE.runSync {
-            InterfacesListeners.INSTANCE.setOpenInterface(player.uniqueId, null)
+            InterfacesListeners.INSTANCE.closePlayerInterface(player.uniqueId, this)
         }
     }
 
     override fun isOpen(): Boolean =
-        (
-            player.openInventory.type == InventoryType.CRAFTING ||
-                player.openInventory.type == InventoryType.CREATIVE
-            ) &&
-            InterfacesListeners.INSTANCE.getOpenInterface(player.uniqueId) == this
+        InterfacesListeners.INSTANCE.getOpenPlayerInterface(player.uniqueId) == this
 }