fix: Correctly handle onClose per inventory

refactor: Reopen guis automatically, but call exit by default

Author: 0ffz

src/main/kotlin/com/mineinabyss/guiy/components/canvases/Anvil.kt

@@ -6,6 +6,7 @@ import com.mineinabyss.guiy.components.rememberMiniMsg
 import com.mineinabyss.guiy.components.state.IntCoordinates
 import com.mineinabyss.guiy.guiyPlugin
 import com.mineinabyss.guiy.inventory.CurrentPlayer
+import com.mineinabyss.guiy.inventory.GuiyInventory
 import com.mineinabyss.guiy.inventory.GuiyInventoryHolder
 import com.mineinabyss.guiy.inventory.InventoryCloseScope
 import com.mineinabyss.guiy.layout.Box
@@ -17,15 +18,14 @@ import com.mineinabyss.idofront.textcomponents.toPlainText
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.filterNotNull
 import org.bukkit.Bukkit
-import org.bukkit.entity.Player
 import org.bukkit.event.inventory.InventoryType
 import org.bukkit.inventory.Inventory
 
 @Composable
 fun Anvil(
     title: String,
     onTextChanged: (String) -> Unit,
-    onClose: (InventoryCloseScope.(player: Player) -> Unit) = {},
+    onClose: InventoryCloseScope.() -> Unit = { exit() },
     onSubmit: (String) -> Unit = {},
     inputLeft: @Composable () -> Unit = { InvisibleItem() },
     inputRight: @Composable () -> Unit = { InvisibleItem() },
@@ -34,10 +34,12 @@ fun Anvil(
     val holder: GuiyInventoryHolder = LocalInventoryHolder.current
     val player = CurrentPlayer
     val titleMM = rememberMiniMsg(title)
-    val inventory: Inventory =
-        remember(holder) { Bukkit.getServer().createInventory(holder, InventoryType.ANVIL, titleMM) }
-    holder.setActiveInventory(inventory)
+    val inventory: Inventory = remember(holder) {
+        Bukkit.getServer().createInventory(holder, InventoryType.ANVIL, titleMM)
+    }
+    holder.setActiveInventory(GuiyInventory(inventory, onClose))
 
+    // Track updates to anvil text via packets
     var playerViewText by remember(inventory) { mutableStateOf("") }
     LaunchedEffect(player) {
         guiyPlugin.anvilPacketFlow.filter { it?.second == player }.filterNotNull().collect {
@@ -48,7 +50,7 @@ fun Anvil(
         }
     }
 
-    val constrainedModifier = Modifier.Companion.size(width = 3, height = 1)
+    val constrainedModifier = Modifier.size(width = 3, height = 1)
 
     Inventory(
         inventory,
@@ -57,13 +59,13 @@ fun Anvil(
         gridToInventoryIndex = { it.x },
         inventoryIndexToGrid = { IntCoordinates(it, 0) }) {
         Row {
-            Box(Modifier.Companion.size(1)) {
+            Box(Modifier.size(1)) {
                 inputLeft()
             }
-            Box(Modifier.Companion.size(1)) {
+            Box(Modifier.size(1)) {
                 inputRight()
             }
-            Box(Modifier.Companion.size(1).clickable { onSubmit(playerViewText) }) {
+            Box(Modifier.size(1).clickable { onSubmit(playerViewText) }) {
                 output()
             }
         }

src/main/kotlin/com/mineinabyss/guiy/components/canvases/Chest.kt

@@ -3,18 +3,17 @@ package com.mineinabyss.guiy.components.canvases
 import androidx.compose.runtime.*
 import com.mineinabyss.guiy.components.rememberMiniMsg
 import com.mineinabyss.guiy.components.state.IntCoordinates
+import com.mineinabyss.guiy.inventory.GuiyInventory
 import com.mineinabyss.guiy.inventory.GuiyInventoryHolder
-import com.mineinabyss.guiy.inventory.LocalGuiyOwner
+import com.mineinabyss.guiy.inventory.InventoryCloseScope
 import com.mineinabyss.guiy.layout.Layout
 import com.mineinabyss.guiy.layout.Size
+import com.mineinabyss.guiy.layout.StaticMeasurePolicy
 import com.mineinabyss.guiy.modifiers.Modifier
 import com.mineinabyss.guiy.modifiers.onSizeChanged
 import com.mineinabyss.guiy.modifiers.sizeIn
-import com.mineinabyss.guiy.inventory.InventoryCloseScope
-import com.mineinabyss.guiy.layout.StaticMeasurePolicy
 import net.kyori.adventure.text.Component
 import org.bukkit.Bukkit
-import org.bukkit.entity.Player
 import org.bukkit.inventory.Inventory
 
 const val CHEST_WIDTH = 9
@@ -30,7 +29,7 @@ const val MAX_CHEST_HEIGHT = 6
 fun Chest(
     title: String,
     modifier: Modifier = Modifier,
-    onClose: (InventoryCloseScope.(player: Player) -> Unit) = { exit() },
+    onClose: InventoryCloseScope.() -> Unit = { exit() },
     content: @Composable () -> Unit,
 ) {
     val titleMM = rememberMiniMsg(title)
@@ -50,7 +49,7 @@ fun Chest(
 fun Chest(
     title: Component,
     modifier: Modifier = Modifier,
-    onClose: (InventoryCloseScope.(player: Player) -> Unit) = {},
+    onClose: InventoryCloseScope.() -> Unit = { exit() },
     content: @Composable () -> Unit,
 ) {
     val holder: GuiyInventoryHolder = LocalInventoryHolder.current
@@ -59,10 +58,6 @@ fun Chest(
         Modifier.sizeIn(CHEST_WIDTH, CHEST_WIDTH, MIN_CHEST_HEIGHT, MAX_CHEST_HEIGHT).then(modifier)
             .onSizeChanged { if (size != it) size = it }
 
-    val viewers by LocalGuiyOwner.current.viewers.collectAsState()
-
-    // Create new inventory when any appropriate value changes
-
     // Draw nothing if empty
     if (size == Size()) {
         Layout(
@@ -74,11 +69,10 @@ fun Chest(
 
     val inventory: Inventory = remember(size) {
         Bukkit.createInventory(holder, CHEST_WIDTH * size.height, title).also {
-            holder.setActiveInventory(it)
         }
     }
+    holder.setActiveInventory(GuiyInventory(inventory, onClose))
 
-    //TODO handle sending correct title when player list changes
     Inventory(
         inventory = inventory,
         title = title,

src/main/kotlin/com/mineinabyss/guiy/components/canvases/Inventory.kt

@@ -1,7 +1,6 @@
 package com.mineinabyss.guiy.components.canvases
 
 import androidx.compose.runtime.*
-import com.github.shynixn.mccoroutine.bukkit.launch
 import com.github.shynixn.mccoroutine.bukkit.minecraftDispatcher
 import com.mineinabyss.guiy.components.state.IntCoordinates
 import com.mineinabyss.guiy.guiyPlugin
@@ -11,13 +10,12 @@ import com.mineinabyss.guiy.inventory.LocalGuiyOwner
 import com.mineinabyss.guiy.inventory.MapBackedGuiyCanvas
 import com.mineinabyss.guiy.layout.Layout
 import com.mineinabyss.guiy.layout.Renderer
+import com.mineinabyss.guiy.layout.StaticMeasurePolicy
 import com.mineinabyss.guiy.modifiers.Modifier
 import com.mineinabyss.guiy.nodes.GuiyNode
-import com.mineinabyss.guiy.layout.StaticMeasurePolicy
 import com.mineinabyss.idofront.entities.title
 import kotlinx.coroutines.withContext
 import net.kyori.adventure.text.Component
-import org.bukkit.entity.Player
 import org.bukkit.event.inventory.InventoryCloseEvent
 import org.bukkit.inventory.Inventory
 
@@ -42,8 +40,8 @@ fun Inventory(
 ) {
     val viewers by LocalGuiyOwner.current.viewers.collectAsState()
 
-    // This just sends a packet, doesn't need to be on sync thread
-    LaunchedEffect(title) {
+    // Update title
+    LaunchedEffect(title, viewers) {
         if (title != null) inventory.viewers.forEach { it.openInventory.title(title) }
     }
 
@@ -64,6 +62,7 @@ fun Inventory(
             }
         }
     }
+
     val canvas = remember { MapBackedGuiyCanvas() }
 
     CompositionLocalProvider(

src/main/kotlin/com/mineinabyss/guiy/components/canvases/rememberInventoryHolder.kt

@@ -33,13 +33,11 @@ fun ProvideInventoryHolder(content: @Composable () -> Unit) {
 }
 
 @Composable
-fun rememberInventoryHolder(
-    onClose: InventoryCloseScope.(Player) -> Unit = {},
-): GuiyInventoryHolder {
+fun rememberInventoryHolder(): GuiyInventoryHolder {
     val clickHandler = LocalClickHandler.current
     val owner = LocalGuiyOwner.current
     val viewers by owner.viewers.collectAsState()
-    return remember(clickHandler, onClose) {
+    return remember(clickHandler, viewers) {
         object : GuiyInventoryHolder() {
             override fun processClick(scope: ClickScope, event: Cancellable) {
                 clickHandler.processClick(scope)
@@ -50,22 +48,20 @@ fun rememberInventoryHolder(
             }
 
             override fun onClose(player: Player) {
+                val inventory = activeInventory.value ?: return
                 val scope = object : InventoryCloseScope {
-                    override fun reopen() {
-                        guiyPlugin.launch {
-                            delay(1.ticks)
-                            //TODO don't think this reference updates properly in the remember block
-                            viewers.filter { it.openInventory.topInventory != inventory }
-                                .forEach { it.openInventory(inventory) }
-                        }
-                    }
+                    override val player = player
 
                     override fun exit() {
                         owner.exit()
                     }
                 }
-                onClose.invoke(scope, player)
-                closeIfNoLongerViewing(player)
+                inventory.onClose.invoke(scope)
+                if (!owner.exitScheduled) guiyPlugin.launch {
+                    delay(1.ticks)
+                    viewers.filter { it.openInventory.topInventory != inventory }
+                        .forEach { it.openInventory(inventory.inventory) }
+                }
             }
 
             override fun closeIfNoLongerViewing(player: Player) {

src/main/kotlin/com/mineinabyss/guiy/components/items/Text.kt

@@ -12,7 +12,7 @@ import net.kyori.adventure.text.Component
  * Text with invisible item, use [LocalItemTheme] to choose the invisible item based on your resourcepack.
  */
 @Composable
-fun Text(name: String, vararg lore: String, hideTooltip: Boolean = false, modifier: Modifier = Modifier.Companion) {
+fun Text(name: String, vararg lore: String, hideTooltip: Boolean = false, modifier: Modifier = Modifier) {
     val mmName = rememberMiniMsg(name)
     val mmLore = rememberMiniMsg(*lore).toTypedArray()
     Text(mmName, lore = mmLore, hideTooltip, modifier)

src/main/kotlin/com/mineinabyss/guiy/inventory/GuiyInventory.kt

@@ -0,0 +1,10 @@
+package com.mineinabyss.guiy.inventory
+
+import androidx.compose.runtime.Immutable
+import org.bukkit.inventory.Inventory
+
+@Immutable
+data class GuiyInventory(
+    val inventory: Inventory,
+    val onClose: InventoryCloseScope.() -> Unit,
+)

src/main/kotlin/com/mineinabyss/guiy/inventory/GuiyInventoryHolder.kt

@@ -1,6 +1,5 @@
 package com.mineinabyss.guiy.inventory
 
-import androidx.compose.runtime.Immutable
 import com.mineinabyss.guiy.modifiers.click.ClickScope
 import com.mineinabyss.guiy.modifiers.drag.DragScope
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -12,20 +11,14 @@ import org.bukkit.event.inventory.InventoryCloseEvent
 import org.bukkit.inventory.Inventory
 import org.bukkit.inventory.InventoryHolder
 
-@Immutable
-data class GuiyInventory(
-    val inventory: Inventory,
-    val onClose: () -> Unit,
-)
-
 abstract class GuiyInventoryHolder : InventoryHolder {
-    private val _activeInventory = MutableStateFlow<Inventory?>(null)
+    private val _activeInventory = MutableStateFlow<GuiyInventory?>(null)
     val activeInventory = _activeInventory.asStateFlow()
 
     override fun getInventory(): Inventory =
-        activeInventory.value ?: error("Guiy inventory is used in bukkit but has not been rendered yet.")
+        activeInventory.value?.inventory ?: error("Guiy inventory is used in bukkit but has not been rendered yet.")
 
-    fun setActiveInventory(inventory: Inventory) = _activeInventory.update { inventory }
+    fun setActiveInventory(inventory: GuiyInventory) = _activeInventory.update { inventory }
 
     abstract fun processClick(scope: ClickScope, event: Cancellable)
     abstract fun processDrag(scope: DragScope)

src/main/kotlin/com/mineinabyss/guiy/inventory/InventoryCloseScope.kt

@@ -1,9 +1,10 @@
 package com.mineinabyss.guiy.inventory
 
 import androidx.compose.runtime.Stable
+import org.bukkit.entity.Player
 
 @Stable
 interface InventoryCloseScope {
-    fun reopen()
+    val player: Player
     fun exit()
 }