From 36e814c2b3ff89b32848ba9c0ac548980e1a99fd Mon Sep 17 00:00:00 2001 From: Distractic Date: Sat, 16 Dec 2023 14:36:35 +0100 Subject: [PATCH 01/24] feat: Apply loading animation --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 122 ++++++++++++++---- .../github/rushyverse/api/gui/LocaleGUI.kt | 9 +- .../github/rushyverse/api/gui/PlayerGUI.kt | 5 + 3 files changed, 113 insertions(+), 23 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index 164341ba..028588f3 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -1,10 +1,19 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.player.Client +import java.util.* +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.async +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import org.bukkit.Material import org.bukkit.entity.HumanEntity import org.bukkit.inventory.Inventory +import org.bukkit.inventory.ItemStack /** * GUI where a new inventory is created for each key used. @@ -38,32 +47,24 @@ public abstract class DedicatedGUI : GUI() { return mutex.withLock { inventories[key] ?: createInventory(key).also { inventories[key] = it - fill(key, it) + + val coroutineScopeLoading = coroutineScopeFill(key) + coroutineScopeLoading.launch { + val fillJob = coroutineScopeLoading.async { + createInventoryContents(key, it) + } + + val loadingAnimationJob = loadingAnimation(key, it) + // Set the inventory contents only when the fill is done. + // This will erase the loading animation. + it.contents = fillJob.await().also { + loadingAnimationJob.cancel() + } + } } } } - /** - * Get the key linked to the client to interact with the GUI. - * @param client Client to get the key for. - * @return The key. - */ - protected abstract suspend fun getKey(client: Client): T - - /** - * Create the inventory for the key. - * @param key Key to create the inventory for. - * @return New created inventory. - */ - protected abstract suspend fun createInventory(key: T): Inventory - - /** - * Fill the inventory for the key. - * @param key Key to fill the inventory for. - * @param inventory Inventory to fill. - */ - protected abstract suspend fun fill(key: T, inventory: Inventory) - override suspend fun hasInventory(inventory: Inventory): Boolean { return mutex.withLock { inventories.values.contains(inventory) @@ -109,4 +110,81 @@ public abstract class DedicatedGUI : GUI() { inventories.clear() } } + + /** + * Get the items to use for the loading animation. + * @return Sequence of ItemStack to use for the loading animation. + */ + protected open fun loadingItems(key: T): Sequence { + return sequence { + yield(ItemStack(Material.LIGHT_BLUE_STAINED_GLASS_PANE)) + yield(ItemStack(Material.BLUE_STAINED_GLASS_PANE)) + yield(ItemStack(Material.PURPLE_STAINED_GLASS_PANE)) + + val blackGlassPaneItem = ItemStack(Material.BLACK_STAINED_GLASS_PANE) + yieldAll(generateSequence { blackGlassPaneItem }) + } + } + + /** + * Animate the inventory while it is being filled by another coroutine. + * @receiver Scope to launch the animation in. + * @param inventory Inventory to animate. + * @return Job of the animation. + */ + protected open fun CoroutineScope.loadingAnimation(key: T, inventory: Inventory): Job { + return launch { + val size = inventory.size + val contents = arrayOfNulls(size) + loadingItems(key).take(size).forEachIndexed { index, item -> + contents[index] = item + } + + val contentList = contents.toMutableList() + while (isActive) { + delay(100) + Collections.rotate(contentList, 1) + inventory.contents = contentList.toTypedArray() + } + } + } + + /** + * Create a new array of ItemStack to fill the inventory later. + * This function is used to avoid conflicts when filling the inventory and the loading animation. + * @param inventory Inventory to fill. + * @param key Key to fill the inventory for. + * @return New created array of ItemStack that will be set to the inventory. + */ + private suspend fun createInventoryContents(key: T, inventory: Inventory): Array { + return arrayOfNulls(inventory.size).apply { fill(key, this) } + } + + /** + * Get the key linked to the client to interact with the GUI. + * @param client Client to get the key for. + * @return The key. + */ + protected abstract suspend fun getKey(client: Client): T + + /** + * Create the inventory for the key. + * @param key Key to create the inventory for. + * @return New created inventory. + */ + protected abstract suspend fun createInventory(key: T): Inventory + + /** + * Fill the inventory for the key. + * @param key Key to fill the inventory for. + * @param inventory Inventory to fill. + */ + protected abstract suspend fun fill(key: T, inventory: Array) + + /** + * Get the coroutine scope to fill the inventory and the loading animation. + * @param key Key to get the coroutine scope for. + * @return The coroutine scope. + */ + protected abstract suspend fun coroutineScopeFill(key: T): CoroutineScope } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index 9c004504..13f52d14 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -1,7 +1,10 @@ package com.github.rushyverse.api.gui +import com.github.rushyverse.api.Plugin import com.github.rushyverse.api.player.Client +import com.github.shynixn.mccoroutine.bukkit.scope import java.util.* +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent @@ -12,7 +15,7 @@ import org.bukkit.event.inventory.InventoryCloseEvent * For example, if two players have the same language, they will share the same inventory. * If one of them changes their language, he will have another inventory dedicated to his new language. */ -public abstract class LocaleGUI : DedicatedGUI() { +public abstract class LocaleGUI(private val plugin: Plugin) : DedicatedGUI() { override suspend fun getKey(client: Client): Locale { return client.lang().locale @@ -30,4 +33,8 @@ public abstract class LocaleGUI : DedicatedGUI() { } else false } } + + override suspend fun coroutineScopeFill(key: Locale): CoroutineScope { + return plugin.scope + } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index cd3524b9..5b374d7e 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -1,6 +1,7 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.player.Client +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.inventory.Inventory @@ -41,6 +42,10 @@ public abstract class PlayerGUI : DedicatedGUI() { return inventories.containsKey(client) } + override suspend fun coroutineScopeFill(key: Client): CoroutineScope { + return key + } + override suspend fun close(client: Client, closeInventory: Boolean): Boolean { return mutex.withLock { inventories.remove(client) }?.run { if (closeInventory) { From 33457a54190fe09f97ad91bafcbff72c86615ed9 Mon Sep 17 00:00:00 2001 From: Distractic Date: Sat, 16 Dec 2023 20:40:36 +0100 Subject: [PATCH 02/24] fix: Avoid delay before first animation --- src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index 028588f3..ce0902fe 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -142,9 +142,9 @@ public abstract class DedicatedGUI : GUI() { val contentList = contents.toMutableList() while (isActive) { + inventory.contents = contentList.toTypedArray() delay(100) Collections.rotate(contentList, 1) - inventory.contents = contentList.toTypedArray() } } } From 6686c706a23c402f539950d0f34e761529c5404f Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 01:01:16 +0100 Subject: [PATCH 03/24] fix: Extract animation loading & avoid dead lock --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 132 +++++++++--------- .../com/github/rushyverse/api/gui/GUI.kt | 9 +- .../github/rushyverse/api/gui/LocaleGUI.kt | 18 ++- .../github/rushyverse/api/gui/PlayerGUI.kt | 19 ++- .../api/gui/load/InventoryLoadingAnimation.kt | 19 +++ .../load/ShiftInventoryLoadingAnimation.kt | 39 ++++++ 6 files changed, 156 insertions(+), 80 deletions(-) create mode 100644 src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt create mode 100644 src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index ce0902fe..be2259cf 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -1,29 +1,39 @@ package com.github.rushyverse.api.gui +import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client -import java.util.* import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job import kotlinx.coroutines.async -import kotlinx.coroutines.delay -import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock -import org.bukkit.Material import org.bukkit.entity.HumanEntity import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack +/** + * Data class to store the inventory and the loading job. + * Can be used to cancel the loading job if the inventory is closed. + * @property inventory Inventory created. + * @property job Loading job to fill & animate the loading of the inventory. + */ +public data class InventoryData( + val inventory: Inventory, + val job: Job, +) + /** * GUI where a new inventory is created for each key used. * @param T Type of the key. * @property inventories Map of inventories for each key. * @property mutex Mutex to process thread-safe operations. */ -public abstract class DedicatedGUI : GUI() { +public abstract class DedicatedGUI( + private val loadingAnimation: InventoryLoadingAnimation? = null, +) : GUI() { - protected var inventories: MutableMap = mutableMapOf() + protected var inventories: MutableMap = mutableMapOf() protected val mutex: Mutex = Mutex() @@ -31,8 +41,15 @@ public abstract class DedicatedGUI : GUI() { val key = getKey(client) val inventory = getOrCreateInventory(key) - val player = client.requirePlayer() - player.openInventory(inventory) + val player = client.player + // We open the inventory out of the mutex to avoid blocking operation from registered Listener. + if (player?.openInventory(inventory) == null) { + // If the opening was cancelled (null returned), + // We need to unregister the client from the GUI + // and maybe close the inventory if it is individual. + close(client, true) + return false + } return true } @@ -45,29 +62,44 @@ public abstract class DedicatedGUI : GUI() { */ private suspend fun getOrCreateInventory(key: T): Inventory { return mutex.withLock { - inventories[key] ?: createInventory(key).also { - inventories[key] = it - - val coroutineScopeLoading = coroutineScopeFill(key) - coroutineScopeLoading.launch { - val fillJob = coroutineScopeLoading.async { - createInventoryContents(key, it) - } - - val loadingAnimationJob = loadingAnimation(key, it) - // Set the inventory contents only when the fill is done. - // This will erase the loading animation. - it.contents = fillJob.await().also { - loadingAnimationJob.cancel() - } - } + val loadedInventory = inventories[key] + if (loadedInventory != null) { + return@withLock loadedInventory.inventory + } + + val inventory = createInventory(key) + // Start the fill asynchronously to avoid blocking the other inventory creation with the mutex. + val loadingJob = startLoadingInventory(key, inventory) + inventories[key] = InventoryData(inventory, loadingJob) + + inventory + } + } + + /** + * Start the asynchronous loading animation and fill the inventory. + * @param key Key to create the inventory for. + * @param inventory Inventory to fill and animate. + * @return The job that can be cancelled to stop the loading animation. + */ + private suspend fun startLoadingInventory(key: T, inventory: Inventory): Job { + return loadingScope(key).launch { + val fillJob = async { + createInventoryContents(key, inventory) + } + + val loadingAnimationJob = loadingAnimation?.loading(this, key, inventory) + // Set the inventory contents only when the fill is done. + // This will erase the loading animation. + inventory.contents = fillJob.await().also { + loadingAnimationJob?.cancel() } } } override suspend fun hasInventory(inventory: Inventory): Boolean { return mutex.withLock { - inventories.values.contains(inventory) + inventories.values.any { it.inventory == inventory } } } @@ -83,7 +115,7 @@ public abstract class DedicatedGUI : GUI() { * @return The viewers of the inventory. */ protected open fun unsafeViewers(): List { - return inventories.values.flatMap(Inventory::getViewers) + return inventories.values.asSequence().map { it.inventory }.flatMap(Inventory::getViewers).toList() } override suspend fun contains(client: Client): Boolean { @@ -100,55 +132,21 @@ public abstract class DedicatedGUI : GUI() { */ protected open fun unsafeContains(client: Client): Boolean { val player = client.player ?: return false - return inventories.values.any { it.viewers.contains(player) } + return inventories.values.any { it.inventory.viewers.contains(player) } } override suspend fun close() { super.close() mutex.withLock { - inventories.values.forEach(Inventory::close) + inventories.values.asSequence() + .forEach { + it.job.cancel(GUIClosedException("The GUI is closing")) + it.inventory.close() + } inventories.clear() } } - /** - * Get the items to use for the loading animation. - * @return Sequence of ItemStack to use for the loading animation. - */ - protected open fun loadingItems(key: T): Sequence { - return sequence { - yield(ItemStack(Material.LIGHT_BLUE_STAINED_GLASS_PANE)) - yield(ItemStack(Material.BLUE_STAINED_GLASS_PANE)) - yield(ItemStack(Material.PURPLE_STAINED_GLASS_PANE)) - - val blackGlassPaneItem = ItemStack(Material.BLACK_STAINED_GLASS_PANE) - yieldAll(generateSequence { blackGlassPaneItem }) - } - } - - /** - * Animate the inventory while it is being filled by another coroutine. - * @receiver Scope to launch the animation in. - * @param inventory Inventory to animate. - * @return Job of the animation. - */ - protected open fun CoroutineScope.loadingAnimation(key: T, inventory: Inventory): Job { - return launch { - val size = inventory.size - val contents = arrayOfNulls(size) - loadingItems(key).take(size).forEachIndexed { index, item -> - contents[index] = item - } - - val contentList = contents.toMutableList() - while (isActive) { - inventory.contents = contentList.toTypedArray() - delay(100) - Collections.rotate(contentList, 1) - } - } - } - /** * Create a new array of ItemStack to fill the inventory later. * This function is used to avoid conflicts when filling the inventory and the loading animation. @@ -186,5 +184,5 @@ public abstract class DedicatedGUI : GUI() { * @param key Key to get the coroutine scope for. * @return The coroutine scope. */ - protected abstract suspend fun coroutineScopeFill(key: T): CoroutineScope + protected abstract suspend fun loadingScope(key: T): CoroutineScope } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index ced3136c..5ce99deb 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -2,6 +2,7 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.koin.inject import com.github.rushyverse.api.player.Client +import kotlinx.coroutines.CancellationException import mu.KotlinLogging import org.bukkit.Server import org.bukkit.entity.HumanEntity @@ -14,13 +15,19 @@ private val logger = KotlinLogging.logger {} /** * Exception concerning the GUI. */ -public open class GUIException(message: String) : RuntimeException(message) +public open class GUIException(message: String) : CancellationException(message) /** * Exception thrown when the GUI is closed. */ public class GUIClosedException(message: String) : GUIException(message) +/** + * Exception thrown when the GUI is closed for a specific client. + */ +public class GUIClosedForClientException(public val client: Client) : + GUIException("GUI closed for client ${client.playerUUID}") + /** * GUI that can be shared by multiple players. * Only one inventory is created for all the viewers. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index 13f52d14..914ee793 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -1,10 +1,14 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.Plugin +import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client import com.github.shynixn.mccoroutine.bukkit.scope import java.util.* import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.job +import kotlinx.coroutines.plus import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent @@ -15,12 +19,20 @@ import org.bukkit.event.inventory.InventoryCloseEvent * For example, if two players have the same language, they will share the same inventory. * If one of them changes their language, he will have another inventory dedicated to his new language. */ -public abstract class LocaleGUI(private val plugin: Plugin) : DedicatedGUI() { +public abstract class LocaleGUI( + private val plugin: Plugin, + loadingAnimation: InventoryLoadingAnimation? = null +) : DedicatedGUI(loadingAnimation) { override suspend fun getKey(client: Client): Locale { return client.lang().locale } + override suspend fun loadingScope(key: Locale): CoroutineScope { + val scope = plugin.scope + return scope + SupervisorJob(scope.coroutineContext.job) + } + override suspend fun close(client: Client, closeInventory: Boolean): Boolean { if (!closeInventory) { return false @@ -33,8 +45,4 @@ public abstract class LocaleGUI(private val plugin: Plugin) : DedicatedGUI() { +public abstract class PlayerGUI(loadingAnimation: InventoryLoadingAnimation? = null) : + DedicatedGUI(loadingAnimation) { override suspend fun getKey(client: Client): Client { return client } + override suspend fun loadingScope(key: Client): CoroutineScope { + return key + SupervisorJob(key.coroutineContext.job) + } + /** * Create the inventory for the client. * Will translate the title and fill the inventory. @@ -42,14 +50,11 @@ public abstract class PlayerGUI : DedicatedGUI() { return inventories.containsKey(client) } - override suspend fun coroutineScopeFill(key: Client): CoroutineScope { - return key - } - override suspend fun close(client: Client, closeInventory: Boolean): Boolean { return mutex.withLock { inventories.remove(client) }?.run { + job.cancel(GUIClosedForClientException(client)) if (closeInventory) { - client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) + inventory.close() } true } == true diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt b/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt new file mode 100644 index 00000000..0ea4897c --- /dev/null +++ b/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt @@ -0,0 +1,19 @@ +package com.github.rushyverse.api.gui.load + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import org.bukkit.inventory.Inventory + +/** + * Animate an inventory while it is being loaded in the background. + * @param T Type of the key. + */ +public fun interface InventoryLoadingAnimation { + + /** + * Animate the inventory while the real inventory is being loaded in the background. + * @param key Key to animate the inventory for. + * @return A job that can be cancelled to stop the animation. + */ + public fun loading(scope: CoroutineScope, key: T, inventory: Inventory): Job +} diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt new file mode 100644 index 00000000..2a6eaf65 --- /dev/null +++ b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt @@ -0,0 +1,39 @@ +package com.github.rushyverse.api.gui.load + +import java.util.* +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.launch +import org.bukkit.inventory.Inventory +import org.bukkit.inventory.ItemStack + +public class ShiftInventoryLoadingAnimation( + private val initialize: (T) -> Sequence, + private val shift: Int = 1, + private val delay: Duration = 100.milliseconds, +) : InventoryLoadingAnimation { + + override fun loading(scope: CoroutineScope, key: T, inventory: Inventory): Job { + return scope.launch { + val size = inventory.size + val contents = arrayOfNulls(size) + // Fill the inventory with the initial items. + // If the sequence is too short, it will be filled with null items. + // If the sequence is too long, the overflowing items will be ignored. + initialize(key).take(size).forEachIndexed { index, item -> + contents[index] = item + } + + val contentList = contents.toMutableList() + while (isActive) { + inventory.contents = contentList.toTypedArray() + delay(delay) + Collections.rotate(contentList, shift) + } + } + } +} From 14777640ff88aafbbd2ec65ee86e1ba7a9da5d1c Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 10:30:59 +0100 Subject: [PATCH 04/24] fix: Add doc & is loading state for inventory --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 27 +++++++++++--- .../com/github/rushyverse/api/gui/GUI.kt | 37 +++++++++++-------- .../github/rushyverse/api/gui/GUIListener.kt | 5 ++- .../github/rushyverse/api/gui/PlayerGUI.kt | 5 ++- 4 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index be2259cf..86e7b12c 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -17,11 +17,16 @@ import org.bukkit.inventory.ItemStack * Can be used to cancel the loading job if the inventory is closed. * @property inventory Inventory created. * @property job Loading job to fill & animate the loading of the inventory. + * @property isLoading If true, the inventory is loading; otherwise it is filled or cancelled. */ public data class InventoryData( val inventory: Inventory, val job: Job, -) +) { + + val isLoading: Boolean get() = job.isActive + +} /** * GUI where a new inventory is created for each key used. @@ -103,6 +108,17 @@ public abstract class DedicatedGUI( } } + /** + * Check if the inventory is loading. + * @param inventory Inventory to check. + * @return True if the inventory is loading, false otherwise. + */ + protected suspend fun isInventoryLoading(inventory: Inventory): Boolean { + return mutex.withLock { + inventories.values.firstOrNull { it.inventory == inventory }?.isLoading == true + } + } + override suspend fun viewers(): List { return mutex.withLock { unsafeViewers() @@ -138,11 +154,10 @@ public abstract class DedicatedGUI( override suspend fun close() { super.close() mutex.withLock { - inventories.values.asSequence() - .forEach { - it.job.cancel(GUIClosedException("The GUI is closing")) - it.inventory.close() - } + inventories.values.forEach { + it.job.cancel(GUIClosedException("The GUI is closing")) + it.inventory.close() + } inventories.clear() } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index 5ce99deb..e79671f3 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -4,6 +4,7 @@ import com.github.rushyverse.api.koin.inject import com.github.rushyverse.api.player.Client import kotlinx.coroutines.CancellationException import mu.KotlinLogging +import org.bukkit.Material import org.bukkit.Server import org.bukkit.entity.HumanEntity import org.bukkit.event.inventory.InventoryClickEvent @@ -85,10 +86,16 @@ public abstract class GUI { /** * Action to do when the client clicks on an item in the inventory. * @param client Client who clicked. - * @param clickedItem Item clicked by the client. + * @param clickedItem Item clicked by the client cannot be null or [AIR][Material.AIR] + * @param clickedInventory Inventory where the click was detected. * @param event Event of the click. */ - public abstract suspend fun onClick(client: Client, clickedItem: ItemStack, event: InventoryClickEvent) + public abstract suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) /** * Remove the client has a viewer of the GUI. @@ -105,6 +112,19 @@ public abstract class GUI { */ public abstract suspend fun hasInventory(inventory: Inventory): Boolean + /** + * Get the viewers of the GUI. + * @return List of viewers. + */ + public abstract suspend fun viewers(): List + + /** + * Check if the GUI contains the player. + * @param client Client to check. + * @return True if the GUI contains the player, false otherwise. + */ + public abstract suspend fun contains(client: Client): Boolean + /** * Close the inventory. * The inventory will be closed for all the viewers. @@ -123,19 +143,6 @@ public abstract class GUI { if (isClosed) throw GUIClosedException("Cannot use a closed GUI") } - /** - * Get the viewers of the GUI. - * @return List of viewers. - */ - public abstract suspend fun viewers(): List - - /** - * Check if the GUI contains the player. - * @param client Client to check. - * @return True if the GUI contains the player, false otherwise. - */ - public abstract suspend fun contains(client: Client): Boolean - /** * Register the GUI to the listener. * @return True if the GUI was registered, false otherwise. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt index af3cebc9..0eb8bd35 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt @@ -42,8 +42,10 @@ public class GUIListener(private val plugin: Plugin) : Listener { if (event.isCancelled) return val item = event.currentItem + // If the item is null or air, we should ignore the click if (item == null || item.type == Material.AIR) return + // If the click is not in an inventory, this is not a GUI click val clickedInventory = event.clickedInventory ?: return val player = event.whoClicked @@ -75,7 +77,7 @@ public class GUIListener(private val plugin: Plugin) : Listener { // The item in a GUI is not supposed to be moved event.cancel() - gui.onClick(client, item, event) + gui.onClick(client, clickedInventory, item, event) } /** @@ -137,6 +139,7 @@ public class GUIListener(private val plugin: Plugin) : Listener { val client = clients.getClientOrNull(player) val gui = client?.gui() ?: return // We don't close the inventory because it is closing due to event. + // That avoid a infinite loop of events and consequently a stack overflow. gui.close(client, false) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index d6202288..ce54dabf 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -14,8 +14,9 @@ import org.bukkit.inventory.InventoryHolder * GUI where a new inventory is created for each player. * An inventory is created when the player opens the GUI and he is not sharing the GUI with another player. */ -public abstract class PlayerGUI(loadingAnimation: InventoryLoadingAnimation? = null) : - DedicatedGUI(loadingAnimation) { +public abstract class PlayerGUI( + loadingAnimation: InventoryLoadingAnimation? = null +) : DedicatedGUI(loadingAnimation) { override suspend fun getKey(client: Client): Client { return client From f02c14cd24ac2ed996a44155f92be818a7bbafcb Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 10:49:27 +0100 Subject: [PATCH 05/24] doc: Add missing documentation --- src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt | 1 + .../api/gui/load/ShiftInventoryLoadingAnimation.kt | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index e79671f3..fab6fb54 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -25,6 +25,7 @@ public class GUIClosedException(message: String) : GUIException(message) /** * Exception thrown when the GUI is closed for a specific client. + * @property client Client for which the GUI is closed. */ public class GUIClosedForClientException(public val client: Client) : GUIException("GUI closed for client ${client.playerUUID}") diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt index 2a6eaf65..4e9b4763 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt @@ -11,6 +11,17 @@ import kotlinx.coroutines.launch import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack +/** + * Animation that shifts the items in the inventory. + * The items are shifted by [shift] slots every [delay]. + * The items are placed in the inventory by calling [initialize]. + * If too few items are returned by [initialize], the remaining slots will be filled with null items. + * If too many items are returned by [initialize], the overflowing items will be ignored. + * @param T Type of the key. + * @property initialize Function that returns the sequence of items to place in the inventory. + * @property shift Number of slots to shift the items by. + * @property delay Delay between each shift. + */ public class ShiftInventoryLoadingAnimation( private val initialize: (T) -> Sequence, private val shift: Int = 1, From e940da1907558ec8a957786b9dad32f80f23ab28 Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 10:49:55 +0100 Subject: [PATCH 06/24] test: Adapt tests for new signature --- .../rushyverse/api/gui/GUIListenerTest.kt | 12 +++++----- .../rushyverse/api/gui/PlayerGUITest.kt | 22 ++++++++++++++----- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt index 54900bb5..54fb3139 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt @@ -80,7 +80,7 @@ class GUIListenerTest : AbstractKoinTest() { } callEvent(true, player, ItemStack { type = Material.DIRT }, player.inventory) - coVerify(exactly = 0) { gui.onClick(any(), any(), any()) } + coVerify(exactly = 0) { gui.onClick(any(), any(), any(), any()) } } @Test @@ -92,7 +92,7 @@ class GUIListenerTest : AbstractKoinTest() { suspend fun callEvent(item: ItemStack?) { val event = callEvent(false, player, item, player.inventory) - coVerify(exactly = 0) { gui.onClick(any(), any(), any()) } + coVerify(exactly = 0) { gui.onClick(any(), any(), any(), any()) } verify(exactly = 0) { event.cancel() } } @@ -114,7 +114,7 @@ class GUIListenerTest : AbstractKoinTest() { val item = ItemStack { type = Material.DIRT } callEvent(false, player, item, mockk()) - coVerify(exactly = 0) { gui.onClick(any(), any(), any()) } + coVerify(exactly = 0) { gui.onClick(any(), any(), any(), any()) } verify(exactly = 0) { pluginManager.callSuspendingEvent(any(), plugin) } } @@ -136,7 +136,7 @@ class GUIListenerTest : AbstractKoinTest() { val item = ItemStack { type = Material.DIRT } callEvent(false, player, item, player.inventory) - coVerify(exactly = 0) { gui.onClick(any(), any(), any()) } + coVerify(exactly = 0) { gui.onClick(any(), any(), any(), any()) } verify(exactly = 1) { pluginManager.callSuspendingEvent(any(), plugin) } jobs.forEach { it.isCompleted shouldBe true } @@ -153,13 +153,13 @@ class GUIListenerTest : AbstractKoinTest() { val inventory = mockk() val gui = registerGUI { coEvery { contains(client) } returns true - coEvery { onClick(client, any(), any()) } returns Unit + coEvery { onClick(client, any(), any(), any()) } returns Unit coEvery { hasInventory(inventory) } returns true } val item = ItemStack { type = Material.DIRT } val event = callEvent(false, player, item, inventory) - coVerify(exactly = 1) { gui.onClick(client, item, event) } + coVerify(exactly = 1) { gui.onClick(client, inventory, item, event) } verify(exactly = 1) { event.cancel() } } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 949d4d15..0fd1f734 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -284,11 +284,16 @@ private class TestGUI(val serverMock: ServerMock, val type: InventoryType = Inve return serverMock.createInventory(owner, type) } - override suspend fun fill(client: Client, inventory: Inventory) { + override suspend fun fill(key: Client, inventory: Array) { // Do nothing } - override suspend fun onClick(client: Client, clickedItem: ItemStack, event: InventoryClickEvent) { + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { error("Should not be called") } } @@ -298,12 +303,17 @@ private class TestFilledGUI(val serverMock: ServerMock) : PlayerGUI() { return serverMock.createInventory(owner, InventoryType.CHEST) } - override suspend fun fill(client: Client, inventory: Inventory) { - inventory.setItem(0, ItemStack { type = Material.DIAMOND_ORE }) - inventory.setItem(1, ItemStack { type = Material.STICK }) + override suspend fun fill(key: Client, inventory: Array) { + inventory[0] = ItemStack { type = Material.DIAMOND_ORE } + inventory[1] = ItemStack { type = Material.STICK } } - override suspend fun onClick(client: Client, clickedItem: ItemStack, event: InventoryClickEvent) { + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { error("Should not be called") } } From 9e2a19a587e8b4bb68a3fb74b56c9accca0d3be5 Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 19:58:31 +0100 Subject: [PATCH 07/24] fix: Animation using flow --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 60 +++++++++++-------- .../api/gui/load/InventoryLoadingAnimation.kt | 5 +- .../load/ShiftInventoryLoadingAnimation.kt | 8 +-- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index 86e7b12c..b6147e5c 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -4,7 +4,10 @@ import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.async +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.cancellable +import kotlinx.coroutines.flow.onCompletion import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock @@ -12,6 +15,11 @@ import org.bukkit.entity.HumanEntity import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack +/** + * Pair of an index and an ItemStack. + */ +public typealias ItemStackIndex = Pair + /** * Data class to store the inventory and the loading job. * Can be used to cancel the loading job if the inventory is closed. @@ -35,7 +43,7 @@ public data class InventoryData( * @property mutex Mutex to process thread-safe operations. */ public abstract class DedicatedGUI( - private val loadingAnimation: InventoryLoadingAnimation? = null, + private val inventoryLoadingAnimation: InventoryLoadingAnimation? = null, ) : GUI() { protected var inventories: MutableMap = mutableMapOf() @@ -89,15 +97,29 @@ public abstract class DedicatedGUI( */ private suspend fun startLoadingInventory(key: T, inventory: Inventory): Job { return loadingScope(key).launch { - val fillJob = async { - createInventoryContents(key, inventory) - } - - val loadingAnimationJob = loadingAnimation?.loading(this, key, inventory) - // Set the inventory contents only when the fill is done. - // This will erase the loading animation. - inventory.contents = fillJob.await().also { - loadingAnimationJob?.cancel() + val size = inventory.size + val inventoryFlowItems = getItemStacks(key, size).cancellable() + + if (inventoryLoadingAnimation == null) { + // Will fill the inventory bit by bit. + inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } + } else { + val loadingAnimationJob = launch { inventoryLoadingAnimation.loading(key, inventory) } + + // To avoid conflicts with the loading animation, + // we need to store the items in a temporary inventory + val temporaryInventory = arrayOfNulls(size) + + inventoryFlowItems + .onCompletion { exception -> + // When the flow is finished, we cancel the loading animation. + loadingAnimationJob.cancelAndJoin() + + // If the flow was completed successfully, we fill the inventory with the temporary inventory. + if (exception != null) { + inventory.contents = temporaryInventory + } + }.collect { (index, item) -> temporaryInventory[index] = item } } } } @@ -162,17 +184,6 @@ public abstract class DedicatedGUI( } } - /** - * Create a new array of ItemStack to fill the inventory later. - * This function is used to avoid conflicts when filling the inventory and the loading animation. - * @param inventory Inventory to fill. - * @param key Key to fill the inventory for. - * @return New created array of ItemStack that will be set to the inventory. - */ - private suspend fun createInventoryContents(key: T, inventory: Inventory): Array { - return arrayOfNulls(inventory.size).apply { fill(key, this) } - } - /** * Get the key linked to the client to interact with the GUI. * @param client Client to get the key for. @@ -190,9 +201,10 @@ public abstract class DedicatedGUI( /** * Fill the inventory for the key. * @param key Key to fill the inventory for. - * @param inventory Inventory to fill. + * @param size Size of the inventory. + * @return Flow of ItemStack to fill the inventory with. */ - protected abstract suspend fun fill(key: T, inventory: Array) + protected abstract fun getItemStacks(key: T, size: Int): Flow /** * Get the coroutine scope to fill the inventory and the loading animation. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt b/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt index 0ea4897c..a6179f74 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/load/InventoryLoadingAnimation.kt @@ -1,7 +1,5 @@ package com.github.rushyverse.api.gui.load -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job import org.bukkit.inventory.Inventory /** @@ -13,7 +11,8 @@ public fun interface InventoryLoadingAnimation { /** * Animate the inventory while the real inventory is being loaded in the background. * @param key Key to animate the inventory for. + * @param inventory Inventory to animate. * @return A job that can be cancelled to stop the animation. */ - public fun loading(scope: CoroutineScope, key: T, inventory: Inventory): Job + public suspend fun loading(key: T, inventory: Inventory) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt index 4e9b4763..14f95f3e 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/load/ShiftInventoryLoadingAnimation.kt @@ -3,11 +3,9 @@ package com.github.rushyverse.api.gui.load import java.util.* import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.isActive -import kotlinx.coroutines.launch import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack @@ -28,8 +26,8 @@ public class ShiftInventoryLoadingAnimation( private val delay: Duration = 100.milliseconds, ) : InventoryLoadingAnimation { - override fun loading(scope: CoroutineScope, key: T, inventory: Inventory): Job { - return scope.launch { + override suspend fun loading(key: T, inventory: Inventory) { + coroutineScope { val size = inventory.size val contents = arrayOfNulls(size) // Fill the inventory with the initial items. From 0401967e10e4339df724953a6670248b2d6b1ee6 Mon Sep 17 00:00:00 2001 From: Distractic Date: Sun, 17 Dec 2023 23:00:59 +0100 Subject: [PATCH 08/24] fix: Fill inventory if not cancelled --- src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index b6147e5c..590f37a8 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -116,7 +116,7 @@ public abstract class DedicatedGUI( loadingAnimationJob.cancelAndJoin() // If the flow was completed successfully, we fill the inventory with the temporary inventory. - if (exception != null) { + if (exception == null) { inventory.contents = temporaryInventory } }.collect { (index, item) -> temporaryInventory[index] = item } From ce8aa9fd51c4cd093eae3f050a1c81f231a3039f Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 07:48:48 +0100 Subject: [PATCH 09/24] fix: Register thread safe & doc --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 6 ++- .../com/github/rushyverse/api/gui/GUI.kt | 10 ++-- .../github/rushyverse/api/gui/GUIListener.kt | 23 +-------- .../github/rushyverse/api/gui/GUIManager.kt | 19 +++++-- .../github/rushyverse/api/gui/LocaleGUI.kt | 2 +- .../github/rushyverse/api/gui/PlayerGUI.kt | 1 + .../rushyverse/api/gui/GUIListenerTest.kt | 31 ++---------- .../rushyverse/api/gui/GUIManagerTest.kt | 12 ++--- .../rushyverse/api/gui/PlayerGUITest.kt | 50 ++++++++++++++++--- 9 files changed, 82 insertions(+), 72 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index 590f37a8..8aa20334 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -100,6 +100,7 @@ public abstract class DedicatedGUI( val size = inventory.size val inventoryFlowItems = getItemStacks(key, size).cancellable() + // If no suspend operation is used in the flow, the fill will be done in the same tick. if (inventoryLoadingAnimation == null) { // Will fill the inventory bit by bit. inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } @@ -177,7 +178,10 @@ public abstract class DedicatedGUI( super.close() mutex.withLock { inventories.values.forEach { - it.job.cancel(GUIClosedException("The GUI is closing")) + it.job.apply { + cancel(GUIClosedException("The GUI is closing")) + join() + } it.inventory.close() } inventories.clear() diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index fab6fb54..ed53f910 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -46,10 +46,6 @@ public abstract class GUI { public var isClosed: Boolean = false protected set - init { - register() - } - /** * Open the GUI for the client only if the GUI is not closed. * If the client has another GUI opened, close it. @@ -148,15 +144,17 @@ public abstract class GUI { * Register the GUI to the listener. * @return True if the GUI was registered, false otherwise. */ - protected fun register(): Boolean { + public suspend fun register(): Boolean { + requireOpen() return manager.add(this) } /** * Unregister the GUI from the listener. + * Should be called when the GUI is closed with [close]. * @return True if the GUI was unregistered, false otherwise. */ - protected fun unregister(): Boolean { + protected suspend fun unregister(): Boolean { return manager.remove(this) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt index 0eb8bd35..f78786ec 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt @@ -17,7 +17,6 @@ import org.bukkit.event.block.Action import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.event.player.PlayerInteractEvent -import org.bukkit.event.player.PlayerQuitEvent import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack import org.bukkit.inventory.PlayerInventory @@ -118,28 +117,10 @@ public class GUIListener(private val plugin: Plugin) : Listener { */ @EventHandler public suspend fun onInventoryClose(event: InventoryCloseEvent) { - quitOpenedGUI(event.player) - } - - /** - * Called when a player quits the server. - * If the player has a GUI opened, the GUI is notified that it is closed for this player. - * @param event Event of the quit. - */ - @EventHandler - public suspend fun onPlayerQuit(event: PlayerQuitEvent) { - quitOpenedGUI(event.player) - } - - /** - * Quit the opened GUI for the player. - * @param player Player to quit the GUI for. - */ - private suspend fun quitOpenedGUI(player: HumanEntity) { - val client = clients.getClientOrNull(player) + val client = clients.getClientOrNull(event.player) val gui = client?.gui() ?: return // We don't close the inventory because it is closing due to event. - // That avoid a infinite loop of events and consequently a stack overflow. + // That avoid an infinite loop of events and consequently a stack overflow. gui.close(client, false) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt index f110d828..bb7fcb3f 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt @@ -1,6 +1,8 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.player.Client +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock /** * Manages the GUIs for players within the game. @@ -8,6 +10,11 @@ import com.github.rushyverse.api.player.Client */ public class GUIManager { + /** + * Mutex used to ensure thread-safe operations. + */ + private val mutex = Mutex() + /** * Private mutable set storing GUIs. */ @@ -26,7 +33,9 @@ public class GUIManager { * @return The language associated with the player. */ public suspend fun get(client: Client): GUI? { - return _guis.firstOrNull { it.contains(client) } + return mutex.withLock { + _guis.firstOrNull { it.contains(client) } + } } /** @@ -34,8 +43,8 @@ public class GUIManager { * @param gui GUI to add. * @return True if the GUI was added, false otherwise. */ - public fun add(gui: GUI): Boolean { - return _guis.add(gui) + public suspend fun add(gui: GUI): Boolean { + return mutex.withLock { _guis.add(gui) } } /** @@ -43,7 +52,7 @@ public class GUIManager { * @param gui GUI to remove. * @return True if the GUI was removed, false otherwise. */ - public fun remove(gui: GUI): Boolean { - return _guis.remove(gui) + public suspend fun remove(gui: GUI): Boolean { + return mutex.withLock { _guis.remove(gui) } } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index 914ee793..91e3dd52 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -1,6 +1,5 @@ package com.github.rushyverse.api.gui -import com.github.rushyverse.api.Plugin import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client import com.github.shynixn.mccoroutine.bukkit.scope @@ -11,6 +10,7 @@ import kotlinx.coroutines.job import kotlinx.coroutines.plus import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent +import org.bukkit.plugin.Plugin /** * GUI where a new inventory is created for each [locale][Locale]. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index ce54dabf..6078943f 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -54,6 +54,7 @@ public abstract class PlayerGUI( override suspend fun close(client: Client, closeInventory: Boolean): Boolean { return mutex.withLock { inventories.remove(client) }?.run { job.cancel(GUIClosedForClientException(client)) + job.join() if (closeInventory) { inventory.close() } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt index 54fb3139..704bfd50 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt @@ -27,14 +27,12 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.async import kotlinx.coroutines.delay import kotlinx.coroutines.test.runTest -import net.kyori.adventure.text.Component import org.bukkit.Material import org.bukkit.entity.Player import org.bukkit.event.block.Action import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.event.player.PlayerInteractEvent -import org.bukkit.event.player.PlayerQuitEvent import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack import org.junit.jupiter.api.Nested @@ -183,7 +181,8 @@ class GUIListenerTest : AbstractKoinTest() { } - abstract inner class CloseGUIDuringEvent { + @Nested + inner class OnInventoryClose { @Test fun `should do nothing if client doesn't have a GUI opened`() = runTest { @@ -192,9 +191,7 @@ class GUIListenerTest : AbstractKoinTest() { coEvery { contains(client) } returns false } - val event = PlayerQuitEvent(player, Component.empty(), PlayerQuitEvent.QuitReason.DISCONNECTED) - listener.onPlayerQuit(event) - + callEvent(player) coVerify(exactly = 0) { gui.close(client, any()) } } @@ -216,14 +213,7 @@ class GUIListenerTest : AbstractKoinTest() { coVerify(exactly = 0) { gui2.close(client, any()) } } - protected abstract suspend fun callEvent(player: Player) - - } - - @Nested - inner class OnInventoryClose : CloseGUIDuringEvent() { - - override suspend fun callEvent(player: Player) { + private suspend fun callEvent(player: Player) { val event = mockk { every { getPlayer() } returns player } @@ -231,17 +221,6 @@ class GUIListenerTest : AbstractKoinTest() { } } - @Nested - inner class OnPlayerQuit : CloseGUIDuringEvent() { - - override suspend fun callEvent(player: Player) { - val event = mockk { - every { getPlayer() } returns player - } - listener.onPlayerQuit(event) - } - } - private suspend fun registerPlayer(): Pair { val player = serverMock.addPlayer() val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) @@ -249,7 +228,7 @@ class GUIListenerTest : AbstractKoinTest() { return player to client } - private inline fun registerGUI(block: GUI.() -> Unit): GUI { + private suspend inline fun registerGUI(block: GUI.() -> Unit): GUI { val gui = mockk(block = block) guiManager.add(gui) return gui diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt index e861e595..fc7aaab8 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt @@ -66,7 +66,7 @@ class GUIManagerTest { inner class Add { @Test - fun `should add non registered GUI`() { + fun `should add non registered GUI`() = runTest { val gui = mockk() manager.add(gui) shouldBe true manager.guis.contains(gui) shouldBe true @@ -74,7 +74,7 @@ class GUIManagerTest { } @Test - fun `should not add registered GUI`() { + fun `should not add registered GUI`() = runTest { val gui = mockk() manager.add(gui) shouldBe true manager.add(gui) shouldBe false @@ -83,7 +83,7 @@ class GUIManagerTest { } @Test - fun `should add multiple GUIs`() { + fun `should add multiple GUIs`() = runTest { val gui1 = mockk() val gui2 = mockk() manager.add(gui1) shouldBe true @@ -99,7 +99,7 @@ class GUIManagerTest { inner class Remove { @Test - fun `should remove registered GUI`() { + fun `should remove registered GUI`() = runTest { val gui = mockk() manager.add(gui) shouldBe true manager.remove(gui) shouldBe true @@ -108,7 +108,7 @@ class GUIManagerTest { } @Test - fun `should not remove non registered GUI`() { + fun `should not remove non registered GUI`() = runTest { val gui = mockk() manager.remove(gui) shouldBe false manager.guis.contains(gui) shouldBe false @@ -116,7 +116,7 @@ class GUIManagerTest { } @Test - fun `should remove one GUI`() { + fun `should remove one GUI`() = runTest { val gui1 = mockk() val gui2 = mockk() manager.add(gui1) shouldBe true diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 0fd1f734..73f60584 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -19,6 +19,11 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.time.Duration.Companion.minutes import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.bukkit.Material import org.bukkit.event.inventory.InventoryClickEvent @@ -54,6 +59,32 @@ class PlayerGUITest : AbstractKoinTest() { MockBukkit.unmock() } + @Nested + inner class Register { + + @Test + fun `should register if not already registered`() = runTest { + val gui = TestGUI(serverMock) + gui.register() shouldBe true + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should not register if already registered`() = runTest { + val gui = TestGUI(serverMock) + gui.register() shouldBe true + gui.register() shouldBe false + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should throw exception if GUI is closed`() = runTest { + val gui = TestGUI(serverMock) + gui.close() + shouldThrow { gui.register() } + } + } + @Nested inner class Open { @@ -71,6 +102,7 @@ class PlayerGUITest : AbstractKoinTest() { @Test fun `should do nothing if the client has the same GUI opened`() = runTest { val gui = TestGUI(serverMock) + gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true @@ -83,6 +115,7 @@ class PlayerGUITest : AbstractKoinTest() { @Test fun `should close the previous GUI if the client has one opened`() = runTest { val gui = TestGUI(serverMock, InventoryType.ENDER_CHEST) + gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true @@ -123,8 +156,9 @@ class PlayerGUITest : AbstractKoinTest() { } @Test - fun `should fill the inventory`() = runTest { + fun `should fill the inventory`() = runBlocking { val gui = TestFilledGUI(serverMock) + gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true @@ -235,6 +269,7 @@ class PlayerGUITest : AbstractKoinTest() { @Test fun `should close all inventories and remove all viewers`() = runTest(timeout = 1.minutes) { val gui = TestGUI(serverMock, InventoryType.BREWING) + gui.register() val playerClients = List(5) { registerPlayer() } val initialInventoryViewType = playerClients.first().first.openInventory.type @@ -264,6 +299,7 @@ class PlayerGUITest : AbstractKoinTest() { @Test fun `should unregister the GUI`() = runTest { val gui = TestGUI(serverMock) + gui.register() guiManager.guis shouldContainAll listOf(gui) gui.close() guiManager.guis shouldContainAll listOf() @@ -284,8 +320,8 @@ private class TestGUI(val serverMock: ServerMock, val type: InventoryType = Inve return serverMock.createInventory(owner, type) } - override suspend fun fill(key: Client, inventory: Array) { - // Do nothing + override fun getItemStacks(key: Client, size: Int): Flow { + return emptyFlow() } override suspend fun onClick( @@ -303,9 +339,11 @@ private class TestFilledGUI(val serverMock: ServerMock) : PlayerGUI() { return serverMock.createInventory(owner, InventoryType.CHEST) } - override suspend fun fill(key: Client, inventory: Array) { - inventory[0] = ItemStack { type = Material.DIAMOND_ORE } - inventory[1] = ItemStack { type = Material.STICK } + override fun getItemStacks(key: Client, size: Int): Flow { + return flow { + emit(0 to ItemStack { type = Material.DIAMOND_ORE }) + emit(1 to ItemStack { type = Material.STICK }) + } } override suspend fun onClick( From 9bab6b0d6dc199344ee18917ea8a80840967cae3 Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 07:55:12 +0100 Subject: [PATCH 10/24] feat: Function to check the load state of inventory --- .../kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt | 7 +------ src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt | 7 +++++++ src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt | 5 +++++ .../kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt | 6 +++++- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt index 8aa20334..68a40a5c 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt @@ -131,12 +131,7 @@ public abstract class DedicatedGUI( } } - /** - * Check if the inventory is loading. - * @param inventory Inventory to check. - * @return True if the inventory is loading, false otherwise. - */ - protected suspend fun isInventoryLoading(inventory: Inventory): Boolean { + public override suspend fun isInventoryLoading(inventory: Inventory): Boolean { return mutex.withLock { inventories.values.firstOrNull { it.inventory == inventory }?.isLoading == true } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index ed53f910..d059b537 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -109,6 +109,13 @@ public abstract class GUI { */ public abstract suspend fun hasInventory(inventory: Inventory): Boolean + /** + * Check if the inventory is loading. + * @param inventory Inventory to check. + * @return True if the inventory is loading, false otherwise. + */ + public abstract suspend fun isInventoryLoading(inventory: Inventory): Boolean + /** * Get the viewers of the GUI. * @return List of viewers. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index 167c74d5..ea55a3a9 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -66,6 +66,11 @@ public abstract class SingleGUI : GUI() { return mutex.withLock { this.inventory } == inventory } + override suspend fun isInventoryLoading(inventory: Inventory): Boolean { + // TODO + return false + } + override suspend fun close() { super.close() mutex.withLock { diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 73f60584..0997c6d9 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -156,7 +156,7 @@ class PlayerGUITest : AbstractKoinTest() { } @Test - fun `should fill the inventory`() = runBlocking { + fun `should fill the inventory`() = runTest { val gui = TestFilledGUI(serverMock) gui.register() val (player, client) = registerPlayer() @@ -165,6 +165,10 @@ class PlayerGUITest : AbstractKoinTest() { player.assertInventoryView(InventoryType.CHEST) val inventory = player.openInventory.topInventory + while (gui.isInventoryLoading(inventory)) { + delay(100) + } + val content = inventory.contents println(content.contentToString()) content[0]!!.type shouldBe Material.DIAMOND_ORE From 7ddf1065fe92556c565ed4bfd8d6075c84deed3d Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 09:38:19 +0100 Subject: [PATCH 11/24] fix: Harmonize GUI implementation --- .../github/rushyverse/api/gui/DedicatedGUI.kt | 214 --------------- .../com/github/rushyverse/api/gui/GUI.kt | 249 +++++++++++++++--- .../github/rushyverse/api/gui/GUIListener.kt | 2 +- .../github/rushyverse/api/gui/GUIManager.kt | 12 +- .../github/rushyverse/api/gui/LocaleGUI.kt | 4 +- .../github/rushyverse/api/gui/PlayerGUI.kt | 2 +- .../github/rushyverse/api/gui/SingleGUI.kt | 101 +++---- .../github/rushyverse/api/player/Client.kt | 2 +- .../rushyverse/api/gui/GUIListenerTest.kt | 4 +- .../rushyverse/api/gui/GUIManagerTest.kt | 22 +- .../rushyverse/api/gui/PlayerGUITest.kt | 3 +- .../rushyverse/api/player/ClientTest.kt | 16 +- 12 files changed, 280 insertions(+), 351 deletions(-) delete mode 100644 src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt deleted file mode 100644 index 68a40a5c..00000000 --- a/src/main/kotlin/com/github/rushyverse/api/gui/DedicatedGUI.kt +++ /dev/null @@ -1,214 +0,0 @@ -package com.github.rushyverse.api.gui - -import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation -import com.github.rushyverse.api.player.Client -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Job -import kotlinx.coroutines.cancelAndJoin -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.cancellable -import kotlinx.coroutines.flow.onCompletion -import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock -import org.bukkit.entity.HumanEntity -import org.bukkit.inventory.Inventory -import org.bukkit.inventory.ItemStack - -/** - * Pair of an index and an ItemStack. - */ -public typealias ItemStackIndex = Pair - -/** - * Data class to store the inventory and the loading job. - * Can be used to cancel the loading job if the inventory is closed. - * @property inventory Inventory created. - * @property job Loading job to fill & animate the loading of the inventory. - * @property isLoading If true, the inventory is loading; otherwise it is filled or cancelled. - */ -public data class InventoryData( - val inventory: Inventory, - val job: Job, -) { - - val isLoading: Boolean get() = job.isActive - -} - -/** - * GUI where a new inventory is created for each key used. - * @param T Type of the key. - * @property inventories Map of inventories for each key. - * @property mutex Mutex to process thread-safe operations. - */ -public abstract class DedicatedGUI( - private val inventoryLoadingAnimation: InventoryLoadingAnimation? = null, -) : GUI() { - - protected var inventories: MutableMap = mutableMapOf() - - protected val mutex: Mutex = Mutex() - - override suspend fun openGUI(client: Client): Boolean { - val key = getKey(client) - val inventory = getOrCreateInventory(key) - - val player = client.player - // We open the inventory out of the mutex to avoid blocking operation from registered Listener. - if (player?.openInventory(inventory) == null) { - // If the opening was cancelled (null returned), - // We need to unregister the client from the GUI - // and maybe close the inventory if it is individual. - close(client, true) - return false - } - - return true - } - - /** - * Get the inventory for the key. - * If the inventory does not exist, create it. - * @param key Key to get the inventory for. - * @return The inventory for the key. - */ - private suspend fun getOrCreateInventory(key: T): Inventory { - return mutex.withLock { - val loadedInventory = inventories[key] - if (loadedInventory != null) { - return@withLock loadedInventory.inventory - } - - val inventory = createInventory(key) - // Start the fill asynchronously to avoid blocking the other inventory creation with the mutex. - val loadingJob = startLoadingInventory(key, inventory) - inventories[key] = InventoryData(inventory, loadingJob) - - inventory - } - } - - /** - * Start the asynchronous loading animation and fill the inventory. - * @param key Key to create the inventory for. - * @param inventory Inventory to fill and animate. - * @return The job that can be cancelled to stop the loading animation. - */ - private suspend fun startLoadingInventory(key: T, inventory: Inventory): Job { - return loadingScope(key).launch { - val size = inventory.size - val inventoryFlowItems = getItemStacks(key, size).cancellable() - - // If no suspend operation is used in the flow, the fill will be done in the same tick. - if (inventoryLoadingAnimation == null) { - // Will fill the inventory bit by bit. - inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } - } else { - val loadingAnimationJob = launch { inventoryLoadingAnimation.loading(key, inventory) } - - // To avoid conflicts with the loading animation, - // we need to store the items in a temporary inventory - val temporaryInventory = arrayOfNulls(size) - - inventoryFlowItems - .onCompletion { exception -> - // When the flow is finished, we cancel the loading animation. - loadingAnimationJob.cancelAndJoin() - - // If the flow was completed successfully, we fill the inventory with the temporary inventory. - if (exception == null) { - inventory.contents = temporaryInventory - } - }.collect { (index, item) -> temporaryInventory[index] = item } - } - } - } - - override suspend fun hasInventory(inventory: Inventory): Boolean { - return mutex.withLock { - inventories.values.any { it.inventory == inventory } - } - } - - public override suspend fun isInventoryLoading(inventory: Inventory): Boolean { - return mutex.withLock { - inventories.values.firstOrNull { it.inventory == inventory }?.isLoading == true - } - } - - override suspend fun viewers(): List { - return mutex.withLock { - unsafeViewers() - } - } - - /** - * Get the viewers of the inventory. - * This function is not thread-safe. - * @return The viewers of the inventory. - */ - protected open fun unsafeViewers(): List { - return inventories.values.asSequence().map { it.inventory }.flatMap(Inventory::getViewers).toList() - } - - override suspend fun contains(client: Client): Boolean { - return mutex.withLock { - unsafeContains(client) - } - } - - /** - * Check if the GUI contains the client. - * This function is not thread-safe. - * @param client Client to check. - * @return True if the GUI contains the client, false otherwise. - */ - protected open fun unsafeContains(client: Client): Boolean { - val player = client.player ?: return false - return inventories.values.any { it.inventory.viewers.contains(player) } - } - - override suspend fun close() { - super.close() - mutex.withLock { - inventories.values.forEach { - it.job.apply { - cancel(GUIClosedException("The GUI is closing")) - join() - } - it.inventory.close() - } - inventories.clear() - } - } - - /** - * Get the key linked to the client to interact with the GUI. - * @param client Client to get the key for. - * @return The key. - */ - protected abstract suspend fun getKey(client: Client): T - - /** - * Create the inventory for the key. - * @param key Key to create the inventory for. - * @return New created inventory. - */ - protected abstract suspend fun createInventory(key: T): Inventory - - /** - * Fill the inventory for the key. - * @param key Key to fill the inventory for. - * @param size Size of the inventory. - * @return Flow of ItemStack to fill the inventory with. - */ - protected abstract fun getItemStacks(key: T, size: Int): Flow - - /** - * Get the coroutine scope to fill the inventory and the loading animation. - * @param key Key to get the coroutine scope for. - * @return The coroutine scope. - */ - protected abstract suspend fun loadingScope(key: T): CoroutineScope -} diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index d059b537..8b4fdda5 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -1,8 +1,18 @@ package com.github.rushyverse.api.gui +import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.koin.inject import com.github.rushyverse.api.player.Client import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.cancellable +import kotlinx.coroutines.flow.onCompletion +import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import mu.KotlinLogging import org.bukkit.Material import org.bukkit.Server @@ -11,6 +21,27 @@ import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack +/** + * Pair of an index and an ItemStack. + */ +public typealias ItemStackIndex = Pair + +/** + * Data class to store the inventory and the loading job. + * Can be used to cancel the loading job if the inventory is closed. + * @property inventory Inventory created. + * @property job Loading job to fill & animate the loading of the inventory. + * @property isLoading If true, the inventory is loading; otherwise it is filled or cancelled. + */ +public data class InventoryData( + val inventory: Inventory, + val job: Job, +) { + + val isLoading: Boolean get() = job.isActive + +} + private val logger = KotlinLogging.logger {} /** @@ -37,15 +68,35 @@ public class GUIClosedForClientException(public val client: Client) : * @property manager Manager to register or unregister the GUI. * @property isClosed If true, the GUI is closed; otherwise it is open. */ -public abstract class GUI { +public abstract class GUI( + private val inventoryLoadingAnimation: InventoryLoadingAnimation? = null, +) { protected val server: Server by inject() - private val manager: GUIManager by inject() + protected val manager: GUIManager by inject() public var isClosed: Boolean = false protected set + protected var inventories: MutableMap = mutableMapOf() + + protected val mutex: Mutex = Mutex() + + /** + * Get the key linked to the client to interact with the GUI. + * @param client Client to get the key for. + * @return The key. + */ + protected abstract suspend fun getKey(client: Client): T + + /** + * Get the coroutine scope to fill the inventory and the loading animation. + * @param key Key to get the coroutine scope for. + * @return The coroutine scope. + */ + protected abstract suspend fun loadingScope(key: T): CoroutineScope + /** * Open the GUI for the client only if the GUI is not closed. * If the client has another GUI opened, close it. @@ -56,11 +107,6 @@ public abstract class GUI { public suspend fun open(client: Client): Boolean { requireOpen() - val gui = client.gui() - if (gui === this) return false - // If the client has another GUI opened, close it. - gui?.close(client, true) - val player = client.player if (player === null) { logger.warn { "Cannot open inventory for player ${client.playerUUID}: player is null" } @@ -69,65 +115,164 @@ public abstract class GUI { // If the player is dead, do not open the GUI because the interface cannot be shown to the player. if (player.isDead) return false - return openGUI(client) + val gui = client.gui() + if (gui === this) return false + + // Here we don't need + // to force to close the GUI because the GUI is closed when the player opens another inventory + // (if not cancelled). + + val key = getKey(client) + val inventory = getOrCreateInventory(key) + + // We open the inventory out of the mutex to avoid blocking operation from registered Listener. + if (player.openInventory(inventory) == null) { + // If the opening was cancelled (null returned), + // We need to unregister the client from the GUI + // and maybe close the inventory if it is individual. + close(client, false) + return false + } + + return true } /** - * Open the GUI for the client. - * Called by [open] after all the checks. - * @param client Client to open the GUI for. - * @return True if the GUI was opened, false otherwise. + * Get the inventory for the key. + * If the inventory does not exist, create it. + * @param key Key to get the inventory for. + * @return The inventory for the key. */ - protected abstract suspend fun openGUI(client: Client): Boolean + private suspend fun getOrCreateInventory(key: T): Inventory { + return mutex.withLock { + val loadedInventory = inventories[key] + if (loadedInventory != null) { + return@withLock loadedInventory.inventory + } + + val inventory = createInventory(key) + // Start the fill asynchronously to avoid blocking the other inventory creation with the mutex. + val loadingJob = startLoadingInventory(key, inventory) + inventories[key] = InventoryData(inventory, loadingJob) + + inventory + } + } /** - * Action to do when the client clicks on an item in the inventory. - * @param client Client who clicked. - * @param clickedItem Item clicked by the client cannot be null or [AIR][Material.AIR] - * @param clickedInventory Inventory where the click was detected. - * @param event Event of the click. + * Start the asynchronous loading animation and fill the inventory. + * @param key Key to create the inventory for. + * @param inventory Inventory to fill and animate. + * @return The job that can be cancelled to stop the loading animation. */ - public abstract suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) + private suspend fun startLoadingInventory(key: T, inventory: Inventory): Job { + return loadingScope(key).launch { + val size = inventory.size + val inventoryFlowItems = getItemStacks(key, size).cancellable() + + // If no suspend operation is used in the flow, the fill will be done in the same tick. + if (inventoryLoadingAnimation == null) { + // Will fill the inventory bit by bit. + inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } + } else { + val loadingAnimationJob = launch { inventoryLoadingAnimation.loading(key, inventory) } + + // To avoid conflicts with the loading animation, + // we need to store the items in a temporary inventory + val temporaryInventory = arrayOfNulls(size) + + inventoryFlowItems + .onCompletion { exception -> + // When the flow is finished, we cancel the loading animation. + loadingAnimationJob.cancelAndJoin() + + // If the flow was completed successfully, we fill the inventory with the temporary inventory. + if (exception == null) { + inventory.contents = temporaryInventory + } + }.collect { (index, item) -> temporaryInventory[index] = item } + } + } + } /** - * Remove the client has a viewer of the GUI. - * @param client Client to close the GUI for. - * @param closeInventory If true, the interface will be closed, otherwise it will be kept open. - * @return True if the inventory was closed, false otherwise. + * Create the inventory for the key. + * @param key Key to create the inventory for. + * @return New created inventory. */ - public abstract suspend fun close(client: Client, closeInventory: Boolean = true): Boolean + protected abstract suspend fun createInventory(key: T): Inventory + + /** + * Fill the inventory for the key. + * @param key Key to fill the inventory for. + * @param size Size of the inventory. + * @return Flow of ItemStack to fill the inventory with. + */ + protected abstract fun getItemStacks(key: T, size: Int): Flow /** * Check if the GUI contains the inventory. * @param inventory Inventory to check. * @return True if the GUI contains the inventory, false otherwise. */ - public abstract suspend fun hasInventory(inventory: Inventory): Boolean + public open suspend fun hasInventory(inventory: Inventory): Boolean { + return mutex.withLock { + inventories.values.any { it.inventory == inventory } + } + } /** * Check if the inventory is loading. * @param inventory Inventory to check. - * @return True if the inventory is loading, false otherwise. + * @return True if the inventory is loading (all the items are not loaded), + * false if the inventory is loaded or not present in the GUI. */ - public abstract suspend fun isInventoryLoading(inventory: Inventory): Boolean + public open suspend fun isInventoryLoading(inventory: Inventory): Boolean { + return mutex.withLock { + inventories.values.firstOrNull { it.inventory == inventory }?.isLoading == true + } + } /** * Get the viewers of the GUI. * @return List of viewers. */ - public abstract suspend fun viewers(): List + public open suspend fun viewers(): List { + return mutex.withLock { + unsafeViewers() + } + } + + /** + * Get the viewers of the inventory. + * This function is not thread-safe. + * @return The viewers of the inventory. + */ + protected open fun unsafeViewers(): List { + return inventories.values.asSequence().map { it.inventory }.flatMap(Inventory::getViewers).toList() + } /** * Check if the GUI contains the player. * @param client Client to check. * @return True if the GUI contains the player, false otherwise. */ - public abstract suspend fun contains(client: Client): Boolean + public open suspend fun contains(client: Client): Boolean { + return mutex.withLock { + unsafeContains(client) + } + } + + /** + * Check if the GUI contains the client. + * This function is not thread-safe. + * @param client Client to check. + * @return True if the GUI contains the client, false otherwise. + */ + protected open fun unsafeContains(client: Client): Boolean { + val player = client.player ?: return false + return inventories.values.any { it.inventory.viewers.contains(player) } + } /** * Close the inventory. @@ -137,8 +282,27 @@ public abstract class GUI { public open suspend fun close() { isClosed = true unregister() + + mutex.withLock { + inventories.values.forEach { + it.job.apply { + cancel(GUIClosedException("The GUI is closing")) + join() + } + it.inventory.close() + } + inventories.clear() + } } + /** + * Remove the client has a viewer of the GUI. + * @param client Client to close the GUI for. + * @param closeInventory If true, the interface will be closed, otherwise it will be kept open. + * @return True if the inventory was closed, false otherwise. + */ + public abstract suspend fun close(client: Client, closeInventory: Boolean = true): Boolean + /** * Verify that the GUI is open. * If the GUI is closed, throw an exception. @@ -151,7 +315,7 @@ public abstract class GUI { * Register the GUI to the listener. * @return True if the GUI was registered, false otherwise. */ - public suspend fun register(): Boolean { + public open suspend fun register(): Boolean { requireOpen() return manager.add(this) } @@ -161,8 +325,21 @@ public abstract class GUI { * Should be called when the GUI is closed with [close]. * @return True if the GUI was unregistered, false otherwise. */ - protected suspend fun unregister(): Boolean { + protected open suspend fun unregister(): Boolean { return manager.remove(this) } + /** + * Action to do when the client clicks on an item in the inventory. + * @param client Client who clicked. + * @param clickedItem Item clicked by the client cannot be null or [AIR][Material.AIR] + * @param clickedInventory Inventory where the click was detected. + * @param event Event of the click. + */ + public abstract suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt index f78786ec..8053552f 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUIListener.kt @@ -120,7 +120,7 @@ public class GUIListener(private val plugin: Plugin) : Listener { val client = clients.getClientOrNull(event.player) val gui = client?.gui() ?: return // We don't close the inventory because it is closing due to event. - // That avoid an infinite loop of events and consequently a stack overflow. + // That avoids an infinite loop of events and consequently a stack overflow. gui.close(client, false) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt index bb7fcb3f..fc720077 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUIManager.kt @@ -18,12 +18,12 @@ public class GUIManager { /** * Private mutable set storing GUIs. */ - private val _guis = mutableSetOf() + private val _guis = mutableSetOf>() /** * Immutable view of the GUIs set. */ - public val guis: Collection get() = _guis + public val guis: Collection> get() = _guis /** * Retrieves the GUI for the specified player. @@ -32,9 +32,9 @@ public class GUIManager { * @param client The player for whom the GUI is to be retrieved or created. * @return The language associated with the player. */ - public suspend fun get(client: Client): GUI? { + public suspend fun get(client: Client): GUI<*>? { return mutex.withLock { - _guis.firstOrNull { it.contains(client) } + guis.firstOrNull { it.contains(client) } } } @@ -43,7 +43,7 @@ public class GUIManager { * @param gui GUI to add. * @return True if the GUI was added, false otherwise. */ - public suspend fun add(gui: GUI): Boolean { + public suspend fun add(gui: GUI<*>): Boolean { return mutex.withLock { _guis.add(gui) } } @@ -52,7 +52,7 @@ public class GUIManager { * @param gui GUI to remove. * @return True if the GUI was removed, false otherwise. */ - public suspend fun remove(gui: GUI): Boolean { + public suspend fun remove(gui: GUI<*>): Boolean { return mutex.withLock { _guis.remove(gui) } } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index 91e3dd52..e8c1d125 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -20,9 +20,9 @@ import org.bukkit.plugin.Plugin * If one of them changes their language, he will have another inventory dedicated to his new language. */ public abstract class LocaleGUI( - private val plugin: Plugin, + protected val plugin: Plugin, loadingAnimation: InventoryLoadingAnimation? = null -) : DedicatedGUI(loadingAnimation) { +) : GUI(loadingAnimation) { override suspend fun getKey(client: Client): Locale { return client.lang().locale diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index 6078943f..48ab967e 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -16,7 +16,7 @@ import org.bukkit.inventory.InventoryHolder */ public abstract class PlayerGUI( loadingAnimation: InventoryLoadingAnimation? = null -) : DedicatedGUI(loadingAnimation) { +) : GUI(loadingAnimation) { override suspend fun getKey(client: Client): Client { return client diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index ea55a3a9..bf1fa7c1 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -1,91 +1,54 @@ package com.github.rushyverse.api.gui +import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client -import kotlinx.coroutines.sync.Mutex +import com.github.shynixn.mccoroutine.bukkit.scope +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.job +import kotlinx.coroutines.plus import kotlinx.coroutines.sync.withLock -import org.bukkit.entity.HumanEntity -import org.bukkit.inventory.Inventory +import org.bukkit.event.inventory.InventoryCloseEvent +import org.bukkit.plugin.Plugin /** * GUI that can be shared by multiple players. * Only one inventory is created for all the viewers. * @property server Server. * @property viewers List of viewers. - * @property inventory Inventory shared by all the viewers. */ -public abstract class SingleGUI : GUI() { +public abstract class SingleGUI( + protected val plugin: Plugin, + loadingAnimation: InventoryLoadingAnimation? = null +) : GUI(loadingAnimation) { - private var inventory: Inventory? = null - - private val mutex = Mutex() - - override suspend fun openGUI(client: Client): Boolean { - val player = client.requirePlayer() - val inventory = getOrCreateInventory() - player.openInventory(inventory) - return true + public companion object { + /** + * Unique key for the GUI. + * This GUI is shared by all the players, so the key is the same for all of them. + */ + private val KEY = Any() } - /** - * Get the inventory of the GUI. - * If the inventory is not created, create it. - * @return The inventory of the GUI. - */ - private suspend fun getOrCreateInventory(): Inventory { - return mutex.withLock { - inventory ?: createInventory().also { - inventory = it - fill(it) - } - } + override suspend fun loadingScope(key: Any): CoroutineScope { + val scope = plugin.scope + return scope + SupervisorJob(scope.coroutineContext.job) } - /** - * Create the inventory of the GUI. - * This function is called only once when the inventory is created. - * @return A new inventory. - */ - protected abstract fun createInventory(): Inventory - override suspend fun close(client: Client, closeInventory: Boolean): Boolean { - return if (closeInventory && contains(client)) { - client.player?.closeInventory() - true - } else false - } - - override suspend fun viewers(): List { - return mutex.withLock { inventory?.viewers } ?: emptyList() - } - - override suspend fun contains(client: Client): Boolean { - return client.player?.let { it in viewers() } == true - } - - override suspend fun hasInventory(inventory: Inventory): Boolean { - return mutex.withLock { this.inventory } == inventory - } - - override suspend fun isInventoryLoading(inventory: Inventory): Boolean { - // TODO - return false - } + if (!closeInventory) { + return false + } - override suspend fun close() { - super.close() - mutex.withLock { - val inventory = inventory - if (inventory != null) { - inventory.close() - this.inventory = null - } + return mutex.withLock { + if (unsafeContains(client)) { + client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) + true + } else false } } - /** - * Fill the inventory with items for the client. - * This function is called when the inventory is created. - * @param inventory The inventory to fill. - */ - protected abstract suspend fun fill(inventory: Inventory) + override suspend fun getKey(client: Client): Any { + return KEY + } } diff --git a/src/main/kotlin/com/github/rushyverse/api/player/Client.kt b/src/main/kotlin/com/github/rushyverse/api/player/Client.kt index bb6ff06f..22c9cefe 100644 --- a/src/main/kotlin/com/github/rushyverse/api/player/Client.kt +++ b/src/main/kotlin/com/github/rushyverse/api/player/Client.kt @@ -81,6 +81,6 @@ public open class Client( * Get the opened GUI of the player. * @return The opened GUI of the player. */ - public suspend fun gui(): GUI? = guiManager.get(this) + public suspend fun gui(): GUI<*>? = guiManager.get(this) } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt index 704bfd50..6d9f6e27 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/GUIListenerTest.kt @@ -228,8 +228,8 @@ class GUIListenerTest : AbstractKoinTest() { return player to client } - private suspend inline fun registerGUI(block: GUI.() -> Unit): GUI { - val gui = mockk(block = block) + private suspend inline fun registerGUI(block: GUI<*>.() -> Unit): GUI<*> { + val gui = mockk>(block = block) guiManager.add(gui) return gui } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt index fc7aaab8..04c2c6b0 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/GUIManagerTest.kt @@ -30,7 +30,7 @@ class GUIManagerTest { @Test fun `should returns null if no GUI contains the client`() = runTest { val client = mockk() - val gui = mockk { + val gui = mockk> { coEvery { contains(any()) } returns false } manager.add(gui) @@ -40,7 +40,7 @@ class GUIManagerTest { @Test fun `should returns GUI if contains the client`() = runTest { val client = mockk() - val gui = mockk { + val gui = mockk> { coEvery { contains(client) } returns true } manager.add(gui) @@ -51,7 +51,7 @@ class GUIManagerTest { fun `should returns GUI if contains the asked client`() = runTest { val client = mockk() val client2 = mockk() - val gui = mockk { + val gui = mockk> { coEvery { contains(client) } returns true coEvery { contains(client2) } returns false } @@ -67,7 +67,7 @@ class GUIManagerTest { @Test fun `should add non registered GUI`() = runTest { - val gui = mockk() + val gui = mockk>() manager.add(gui) shouldBe true manager.guis.contains(gui) shouldBe true manager.guis.size shouldBe 1 @@ -75,7 +75,7 @@ class GUIManagerTest { @Test fun `should not add registered GUI`() = runTest { - val gui = mockk() + val gui = mockk>() manager.add(gui) shouldBe true manager.add(gui) shouldBe false manager.guis.contains(gui) shouldBe true @@ -84,8 +84,8 @@ class GUIManagerTest { @Test fun `should add multiple GUIs`() = runTest { - val gui1 = mockk() - val gui2 = mockk() + val gui1 = mockk>() + val gui2 = mockk>() manager.add(gui1) shouldBe true manager.add(gui2) shouldBe true manager.guis.contains(gui1) shouldBe true @@ -100,7 +100,7 @@ class GUIManagerTest { @Test fun `should remove registered GUI`() = runTest { - val gui = mockk() + val gui = mockk>() manager.add(gui) shouldBe true manager.remove(gui) shouldBe true manager.guis.contains(gui) shouldBe false @@ -109,7 +109,7 @@ class GUIManagerTest { @Test fun `should not remove non registered GUI`() = runTest { - val gui = mockk() + val gui = mockk>() manager.remove(gui) shouldBe false manager.guis.contains(gui) shouldBe false manager.guis.size shouldBe 0 @@ -117,8 +117,8 @@ class GUIManagerTest { @Test fun `should remove one GUI`() = runTest { - val gui1 = mockk() - val gui2 = mockk() + val gui1 = mockk>() + val gui2 = mockk>() manager.add(gui1) shouldBe true manager.add(gui2) shouldBe true manager.remove(gui1) shouldBe true diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 0997c6d9..54e355ba 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -23,7 +23,6 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.bukkit.Material import org.bukkit.event.inventory.InventoryClickEvent @@ -166,7 +165,7 @@ class PlayerGUITest : AbstractKoinTest() { val inventory = player.openInventory.topInventory while (gui.isInventoryLoading(inventory)) { - delay(100) + delay(10) } val content = inventory.contents diff --git a/src/test/kotlin/com/github/rushyverse/api/player/ClientTest.kt b/src/test/kotlin/com/github/rushyverse/api/player/ClientTest.kt index 7d356af7..205eb4cc 100644 --- a/src/test/kotlin/com/github/rushyverse/api/player/ClientTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/player/ClientTest.kt @@ -10,13 +10,17 @@ import com.github.rushyverse.api.player.exception.PlayerNotFoundException import io.kotest.matchers.shouldBe import io.mockk.coEvery import io.mockk.mockk -import kotlinx.coroutines.CoroutineScope -import org.junit.jupiter.api.assertThrows import java.util.* import kotlin.coroutines.EmptyCoroutineContext -import kotlin.test.* +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.assertThrows class ClientTest : AbstractKoinTest() { @@ -75,7 +79,7 @@ class ClientTest : AbstractKoinTest() { } @Nested - inner class GetGUI { + inner class GetGUI { @Test fun `get GUI returns null if no GUI is registered`() = runTest { @@ -86,7 +90,7 @@ class ClientTest : AbstractKoinTest() { @Test fun `get GUI returns null if no GUI contains the client`() = runTest { val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - val gui = mockk { + val gui = mockk> { coEvery { contains(client) } returns false } guiManager.add(gui) @@ -96,7 +100,7 @@ class ClientTest : AbstractKoinTest() { @Test fun `get GUI returns GUI if contains the client`() = runTest { val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - val gui = mockk { + val gui = mockk> { coEvery { contains(client) } returns true } guiManager.add(gui) From 0ec4a67d7125726f79e1906372c1a11305d6252d Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 09:44:27 +0100 Subject: [PATCH 12/24] test: remove print --- src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 54e355ba..d7186b8a 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -169,7 +169,6 @@ class PlayerGUITest : AbstractKoinTest() { } val content = inventory.contents - println(content.contentToString()) content[0]!!.type shouldBe Material.DIAMOND_ORE content[1]!!.type shouldBe Material.STICK From 4cbf9492dbd99d674ad5e112eb5d7de6f897ac7e Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 10:27:00 +0100 Subject: [PATCH 13/24] fix: Fill inventory in same called thread --- src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt | 8 +++++--- .../kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt | 2 +- .../kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt | 2 +- .../kotlin/com/github/rushyverse/api/gui/SingleGUI.kt | 2 +- .../kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt | 3 --- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index 8b4fdda5..3abe1d43 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -5,6 +5,7 @@ import com.github.rushyverse.api.koin.inject import com.github.rushyverse.api.player.Client import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.flow.Flow @@ -95,7 +96,7 @@ public abstract class GUI( * @param key Key to get the coroutine scope for. * @return The coroutine scope. */ - protected abstract suspend fun loadingScope(key: T): CoroutineScope + protected abstract suspend fun fillScope(key: T): CoroutineScope /** * Open the GUI for the client only if the GUI is not closed. @@ -166,11 +167,12 @@ public abstract class GUI( * @return The job that can be cancelled to stop the loading animation. */ private suspend fun startLoadingInventory(key: T, inventory: Inventory): Job { - return loadingScope(key).launch { + // If no suspend operation is used in the flow, the fill will be done in the same thread & tick. + // That's why we start with unconfined dispatcher. + return fillScope(key).launch(Dispatchers.Unconfined) { val size = inventory.size val inventoryFlowItems = getItemStacks(key, size).cancellable() - // If no suspend operation is used in the flow, the fill will be done in the same tick. if (inventoryLoadingAnimation == null) { // Will fill the inventory bit by bit. inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index e8c1d125..f0b6f624 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -28,7 +28,7 @@ public abstract class LocaleGUI( return client.lang().locale } - override suspend fun loadingScope(key: Locale): CoroutineScope { + override suspend fun fillScope(key: Locale): CoroutineScope { val scope = plugin.scope return scope + SupervisorJob(scope.coroutineContext.job) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index 48ab967e..398ab485 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -22,7 +22,7 @@ public abstract class PlayerGUI( return client } - override suspend fun loadingScope(key: Client): CoroutineScope { + override suspend fun fillScope(key: Client): CoroutineScope { return key + SupervisorJob(key.coroutineContext.job) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index bf1fa7c1..a240bb0b 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -30,7 +30,7 @@ public abstract class SingleGUI( private val KEY = Any() } - override suspend fun loadingScope(key: Any): CoroutineScope { + override suspend fun fillScope(key: Any): CoroutineScope { val scope = plugin.scope return scope + SupervisorJob(scope.coroutineContext.job) } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index d7186b8a..d5d5c90b 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -164,9 +164,6 @@ class PlayerGUITest : AbstractKoinTest() { player.assertInventoryView(InventoryType.CHEST) val inventory = player.openInventory.topInventory - while (gui.isInventoryLoading(inventory)) { - delay(10) - } val content = inventory.contents content[0]!!.type shouldBe Material.DIAMOND_ORE From f19a05adcccdcc86bf23065fefbb244ac096e88a Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 11:34:45 +0100 Subject: [PATCH 14/24] test: Add test about suspending operation during load --- .../rushyverse/api/gui/PlayerGUITest.kt | 91 ++++++++++++++----- 1 file changed, 68 insertions(+), 23 deletions(-) diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index d5d5c90b..1e592ced 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -17,12 +17,15 @@ import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.bukkit.Material import org.bukkit.event.inventory.InventoryClickEvent @@ -111,22 +114,6 @@ class PlayerGUITest : AbstractKoinTest() { player.assertInventoryView(gui.type) } - @Test - fun `should close the previous GUI if the client has one opened`() = runTest { - val gui = TestGUI(serverMock, InventoryType.ENDER_CHEST) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - - val gui2 = TestGUI(serverMock, InventoryType.CHEST) - gui2.open(client) shouldBe true - player.assertInventoryView(gui2.type) - gui.contains(client) shouldBe false - gui2.contains(client) shouldBe true - } - @Test fun `should do nothing if the player is dead`() = runTest { val gui = TestGUI(serverMock) @@ -155,7 +142,9 @@ class PlayerGUITest : AbstractKoinTest() { } @Test - fun `should fill the inventory`() = runTest { + fun `should fill the inventory in the same thread if no suspend operation`() = runTest { + val currentThread = Thread.currentThread() + val gui = TestFilledGUI(serverMock) gui.register() val (player, client) = registerPlayer() @@ -164,14 +153,52 @@ class PlayerGUITest : AbstractKoinTest() { player.assertInventoryView(InventoryType.CHEST) val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe false + + val content = inventory.contents + TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } + + gui.calledThread shouldBe currentThread + gui.newThread shouldBe currentThread + } + + @Test + fun `should fill the inventory in the other thread after suspend operation`(): Unit = runBlocking { + val currentThread = Thread.currentThread() + + val delay = 100.milliseconds + val gui = TestFilledGUI(serverMock, delay) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) + + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true val content = inventory.contents - content[0]!!.type shouldBe Material.DIAMOND_ORE - content[1]!!.type shouldBe Material.STICK + content.forEach { it shouldBe null } - for (i in 2 until content.size) { + delay(delay + 10.milliseconds) + gui.isInventoryLoading(inventory) shouldBe false + + TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { content[i] shouldBe null } + + gui.calledThread shouldBe currentThread + gui.newThread shouldNotBe currentThread } } @@ -333,15 +360,33 @@ private class TestGUI(val serverMock: ServerMock, val type: InventoryType = Inve } } -private class TestFilledGUI(val serverMock: ServerMock) : PlayerGUI() { +private class TestFilledGUI( + val serverMock: ServerMock, + val delay: Duration? = null, +) : PlayerGUI() { + + companion object { + val EXPECTED_INV = arrayOf( + ItemStack { type = Material.DIAMOND_ORE }, + ItemStack { type = Material.STICK }, + ) + } + + var calledThread: Thread? = null + + var newThread: Thread? = null + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { return serverMock.createInventory(owner, InventoryType.CHEST) } override fun getItemStacks(key: Client, size: Int): Flow { + calledThread = Thread.currentThread() return flow { - emit(0 to ItemStack { type = Material.DIAMOND_ORE }) - emit(1 to ItemStack { type = Material.STICK }) + delay?.let { delay(it) } + emit(0 to EXPECTED_INV[0]) + newThread = Thread.currentThread() + emit(1 to EXPECTED_INV[1]) } } From d2552bb9eab56b1e185dc9dcaaaf7c110ce2605b Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 11:38:46 +0100 Subject: [PATCH 15/24] fix: Detekt --- .../rushyverse/api/gui/PlayerGUITest.kt | 341 +++++++++--------- 1 file changed, 171 insertions(+), 170 deletions(-) diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 1e592ced..66045249 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -169,233 +169,234 @@ class PlayerGUITest : AbstractKoinTest() { } @Test - fun `should fill the inventory in the other thread after suspend operation`(): Unit = runBlocking { - val currentThread = Thread.currentThread() + fun `should fill the inventory in the other thread after suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() - val delay = 100.milliseconds - val gui = TestFilledGUI(serverMock, delay) - gui.register() - val (player, client) = registerPlayer() + val delay = 100.milliseconds + val gui = TestFilledGUI(serverMock, delay) + gui.register() + val (player, client) = registerPlayer() - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe true + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true - val content = inventory.contents - content.forEach { it shouldBe null } + val content = inventory.contents + content.forEach { it shouldBe null } - delay(delay + 10.milliseconds) - gui.isInventoryLoading(inventory) shouldBe false + delay(delay + 10.milliseconds) + gui.isInventoryLoading(inventory) shouldBe false - TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } + TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } - for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } + for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } - gui.calledThread shouldBe currentThread - gui.newThread shouldNotBe currentThread + gui.calledThread shouldBe currentThread + gui.newThread shouldNotBe currentThread + } } - } + @Nested + inner class Viewers { - @Nested - inner class Viewers { + @Test + fun `should return empty list if no client is viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + gui.viewers() shouldBe emptyList() + } - @Test - fun `should return empty list if no client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.viewers() shouldBe emptyList() - } + @Test + fun `should return the list of clients viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val playerClients = List(5) { registerPlayer() } - @Test - fun `should return the list of clients viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val playerClients = List(5) { registerPlayer() } + playerClients.forEach { (_, client) -> + gui.open(client) shouldBe true + } - playerClients.forEach { (_, client) -> - gui.open(client) shouldBe true + gui.viewers() shouldContainExactlyInAnyOrder playerClients.map { it.first } } - gui.viewers() shouldContainExactlyInAnyOrder playerClients.map { it.first } } - } + @Nested + inner class Contains { - @Nested - inner class Contains { + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (_, client) = registerPlayer() + gui.contains(client) shouldBe false + } - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.contains(client) shouldBe false - } + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (_, client) = registerPlayer() + gui.open(client) shouldBe true + gui.contains(client) shouldBe true + } - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.open(client) shouldBe true - gui.contains(client) shouldBe true } - } - - @Nested - inner class CloseForClient { - - @Test - fun `should return false if the client is not viewing the GUI`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() + @Nested + inner class CloseForClient { - val initialInventoryViewType = player.openInventory.type - - player.assertInventoryView(initialInventoryViewType) - gui.close(client, true) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } - - @Test - fun `should close the inventory if the client is viewing the GUI`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() + @Test + fun `should return false if the client is not viewing the GUI`() = runTest(timeout = 1.minutes) { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() - val initialInventoryViewType = player.openInventory.type + val initialInventoryViewType = player.openInventory.type - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - gui.close(client, true) shouldBe true - player.assertInventoryView(initialInventoryViewType) - } + player.assertInventoryView(initialInventoryViewType) + gui.close(client, true) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } - @Test - fun `should remove client inventory without closing it if closeInventory is false`() = - runTest(timeout = 1.minutes) { + @Test + fun `should close the inventory if the client is viewing the GUI`() = runTest(timeout = 1.minutes) { val gui = TestGUI(serverMock) val (player, client) = registerPlayer() + val initialInventoryViewType = player.openInventory.type + gui.open(client) shouldBe true player.assertInventoryView(gui.type) - gui.close(client, false) shouldBe true - player.assertInventoryView(gui.type) - gui.contains(client) shouldBe false + gui.close(client, true) shouldBe true + player.assertInventoryView(initialInventoryViewType) } - } + @Test + fun `should remove client inventory without closing it if closeInventory is false`() = + runTest(timeout = 1.minutes) { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() - @Nested - inner class Close { + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + gui.close(client, false) shouldBe true + player.assertInventoryView(gui.type) + gui.contains(client) shouldBe false + } - @Test - fun `should close all inventories and remove all viewers`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock, InventoryType.BREWING) - gui.register() + } - val playerClients = List(5) { registerPlayer() } - val initialInventoryViewType = playerClients.first().first.openInventory.type + @Nested + inner class Close { + + @Test + fun `should close all inventories and remove all viewers`() = runTest(timeout = 1.minutes) { + val gui = TestGUI(serverMock, InventoryType.BREWING) + gui.register() + + val playerClients = List(5) { registerPlayer() } + val initialInventoryViewType = playerClients.first().first.openInventory.type + + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + client.gui() shouldBe gui + } + + gui.close() + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + client.gui() shouldBe null + } + } - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - client.gui() shouldBe gui + @Test + fun `should set isClosed to true`() = runTest { + val gui = TestGUI(serverMock) + gui.isClosed shouldBe false + gui.close() + gui.isClosed shouldBe true } - gui.close() - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - client.gui() shouldBe null + @Test + fun `should unregister the GUI`() = runTest { + val gui = TestGUI(serverMock) + gui.register() + guiManager.guis shouldContainAll listOf(gui) + gui.close() + guiManager.guis shouldContainAll listOf() } - } - @Test - fun `should set isClosed to true`() = runTest { - val gui = TestGUI(serverMock) - gui.isClosed shouldBe false - gui.close() - gui.isClosed shouldBe true } - @Test - fun `should unregister the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.register() - guiManager.guis shouldContainAll listOf(gui) - gui.close() - guiManager.guis shouldContainAll listOf() + private suspend fun registerPlayer(): Pair { + val player = serverMock.addPlayer() + val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) + clientManager.put(player, client) + return player to client } - - } - - private suspend fun registerPlayer(): Pair { - val player = serverMock.addPlayer() - val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - clientManager.put(player, client) - return player to client } -} -private class TestGUI(val serverMock: ServerMock, val type: InventoryType = InventoryType.HOPPER) : PlayerGUI() { - override fun createInventory(owner: InventoryHolder, client: Client): Inventory { - return serverMock.createInventory(owner, type) - } + private class TestGUI(val serverMock: ServerMock, val type: InventoryType = InventoryType.HOPPER) : PlayerGUI() { + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { + return serverMock.createInventory(owner, type) + } - override fun getItemStacks(key: Client, size: Int): Flow { - return emptyFlow() - } + override fun getItemStacks(key: Client, size: Int): Flow { + return emptyFlow() + } - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } } -} -private class TestFilledGUI( - val serverMock: ServerMock, - val delay: Duration? = null, -) : PlayerGUI() { + private class TestFilledGUI( + val serverMock: ServerMock, + val delay: Duration? = null, + ) : PlayerGUI() { - companion object { - val EXPECTED_INV = arrayOf( - ItemStack { type = Material.DIAMOND_ORE }, - ItemStack { type = Material.STICK }, - ) - } + companion object { + val EXPECTED_INV = arrayOf( + ItemStack { type = Material.DIAMOND_ORE }, + ItemStack { type = Material.STICK }, + ) + } - var calledThread: Thread? = null + var calledThread: Thread? = null - var newThread: Thread? = null + var newThread: Thread? = null - override fun createInventory(owner: InventoryHolder, client: Client): Inventory { - return serverMock.createInventory(owner, InventoryType.CHEST) - } + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { + return serverMock.createInventory(owner, InventoryType.CHEST) + } - override fun getItemStacks(key: Client, size: Int): Flow { - calledThread = Thread.currentThread() - return flow { - delay?.let { delay(it) } - emit(0 to EXPECTED_INV[0]) - newThread = Thread.currentThread() - emit(1 to EXPECTED_INV[1]) + override fun getItemStacks(key: Client, size: Int): Flow { + calledThread = Thread.currentThread() + return flow { + delay?.let { delay(it) } + emit(0 to EXPECTED_INV[0]) + newThread = Thread.currentThread() + emit(1 to EXPECTED_INV[1]) + } } - } - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } } } From f744554747d17eb3a79150d78ed5230926df062a Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 14:18:26 +0100 Subject: [PATCH 16/24] fix: Memory optimization to store inventory --- .../com/github/rushyverse/api/gui/GUI.kt | 17 +- .../github/rushyverse/api/gui/LocaleGUI.kt | 9 +- .../github/rushyverse/api/gui/PlayerGUI.kt | 2 +- .../github/rushyverse/api/gui/SingleGUI.kt | 6 +- .../rushyverse/api/gui/PlayerGUITest.kt | 397 +++++++++++------- 5 files changed, 257 insertions(+), 174 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index 3abe1d43..ab5bb48c 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -70,7 +70,8 @@ public class GUIClosedForClientException(public val client: Client) : * @property isClosed If true, the GUI is closed; otherwise it is open. */ public abstract class GUI( - private val inventoryLoadingAnimation: InventoryLoadingAnimation? = null, + private val loadingAnimation: InventoryLoadingAnimation? = null, + initialNumberInventories: Int = 16, ) { protected val server: Server by inject() @@ -80,7 +81,7 @@ public abstract class GUI( public var isClosed: Boolean = false protected set - protected var inventories: MutableMap = mutableMapOf() + protected var inventories: MutableMap = HashMap(initialNumberInventories) protected val mutex: Mutex = Mutex() @@ -173,11 +174,11 @@ public abstract class GUI( val size = inventory.size val inventoryFlowItems = getItemStacks(key, size).cancellable() - if (inventoryLoadingAnimation == null) { + if (loadingAnimation == null) { // Will fill the inventory bit by bit. inventoryFlowItems.collect { (index, item) -> inventory.setItem(index, item) } } else { - val loadingAnimationJob = launch { inventoryLoadingAnimation.loading(key, inventory) } + val loadingAnimationJob = launch { loadingAnimation.loading(key, inventory) } // To avoid conflicts with the loading animation, // we need to store the items in a temporary inventory @@ -239,7 +240,7 @@ public abstract class GUI( * Get the viewers of the GUI. * @return List of viewers. */ - public open suspend fun viewers(): List { + public open suspend fun viewers(): Sequence { return mutex.withLock { unsafeViewers() } @@ -250,8 +251,8 @@ public abstract class GUI( * This function is not thread-safe. * @return The viewers of the inventory. */ - protected open fun unsafeViewers(): List { - return inventories.values.asSequence().map { it.inventory }.flatMap(Inventory::getViewers).toList() + protected open fun unsafeViewers(): Sequence { + return inventories.values.asSequence().map { it.inventory }.flatMap(Inventory::getViewers) } /** @@ -273,7 +274,7 @@ public abstract class GUI( */ protected open fun unsafeContains(client: Client): Boolean { val player = client.player ?: return false - return inventories.values.any { it.inventory.viewers.contains(player) } + return unsafeViewers().any { it == player } } /** diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index f0b6f624..afe27e1b 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -2,6 +2,7 @@ package com.github.rushyverse.api.gui import com.github.rushyverse.api.gui.load.InventoryLoadingAnimation import com.github.rushyverse.api.player.Client +import com.github.rushyverse.api.translation.SupportedLanguage import com.github.shynixn.mccoroutine.bukkit.scope import java.util.* import kotlinx.coroutines.CoroutineScope @@ -21,8 +22,12 @@ import org.bukkit.plugin.Plugin */ public abstract class LocaleGUI( protected val plugin: Plugin, - loadingAnimation: InventoryLoadingAnimation? = null -) : GUI(loadingAnimation) { + loadingAnimation: InventoryLoadingAnimation? = null, + initialNumberInventories: Int = SupportedLanguage.entries.size +) : GUI( + loadingAnimation = loadingAnimation, + initialNumberInventories = initialNumberInventories +) { override suspend fun getKey(client: Client): Locale { return client.lang().locale diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index 398ab485..7dc5568e 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -16,7 +16,7 @@ import org.bukkit.inventory.InventoryHolder */ public abstract class PlayerGUI( loadingAnimation: InventoryLoadingAnimation? = null -) : GUI(loadingAnimation) { +) : GUI(loadingAnimation = loadingAnimation) { override suspend fun getKey(client: Client): Client { return client diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index a240bb0b..2a965b1f 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -20,12 +20,16 @@ import org.bukkit.plugin.Plugin public abstract class SingleGUI( protected val plugin: Plugin, loadingAnimation: InventoryLoadingAnimation? = null -) : GUI(loadingAnimation) { +) : GUI( + loadingAnimation = loadingAnimation, + initialNumberInventories = 1 +) { public companion object { /** * Unique key for the GUI. * This GUI is shared by all the players, so the key is the same for all of them. + * That allows creating a unique inventory. */ private val KEY = Any() } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 66045249..ee7aba62 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -34,6 +34,8 @@ import org.bukkit.inventory.Inventory import org.bukkit.inventory.InventoryHolder import org.bukkit.inventory.ItemStack import org.junit.jupiter.api.Nested +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource class PlayerGUITest : AbstractKoinTest() { @@ -87,6 +89,49 @@ class PlayerGUITest : AbstractKoinTest() { } } + @Nested + inner class Viewers { + + @Test + fun `should return empty list if no client is viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + gui.viewers().toList() shouldBe emptyList() + } + + @Test + fun `should return the list of clients viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val playerClients = List(5) { registerPlayer() } + + playerClients.forEach { (_, client) -> + gui.open(client) shouldBe true + } + + gui.viewers().toList() shouldContainExactlyInAnyOrder playerClients.map { it.first } + } + + } + + @Nested + inner class Contains { + + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (_, client) = registerPlayer() + gui.contains(client) shouldBe false + } + + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (_, client) = registerPlayer() + gui.open(client) shouldBe true + gui.contains(client) shouldBe true + } + + } + @Nested inner class Open { @@ -142,30 +187,32 @@ class PlayerGUITest : AbstractKoinTest() { } @Test - fun `should fill the inventory in the same thread if no suspend operation`() = runTest { - val currentThread = Thread.currentThread() + fun `should fill the inventory in the same thread if no suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() - val gui = TestFilledGUI(serverMock) - gui.register() - val (player, client) = registerPlayer() + val gui = TestFilledGUI(serverMock) + gui.register() + val (player, client) = registerPlayer() - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe false + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe false - val content = inventory.contents - TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } + val content = inventory.contents + TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } - for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } + for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } - gui.calledThread shouldBe currentThread - gui.newThread shouldBe currentThread + gui.calledThread shouldBe currentThread + gui.newThread shouldBe currentThread + } } @Test @@ -187,7 +234,7 @@ class PlayerGUITest : AbstractKoinTest() { val content = inventory.contents content.forEach { it shouldBe null } - delay(delay + 10.milliseconds) + delay(delay * 2) gui.isInventoryLoading(inventory) shouldBe false TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> @@ -202,201 +249,227 @@ class PlayerGUITest : AbstractKoinTest() { gui.newThread shouldNotBe currentThread } } + } - @Nested - inner class Viewers { + @Nested + inner class Close { - @Test - fun `should return empty list if no client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.viewers() shouldBe emptyList() - } + @Test + fun `should close all inventories and remove all viewers`() = runTest { + val gui = TestGUI(serverMock, InventoryType.BREWING) + gui.register() - @Test - fun `should return the list of clients viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val playerClients = List(5) { registerPlayer() } + val playerClients = List(5) { registerPlayer() } + val initialInventoryViewType = playerClients.first().first.openInventory.type - playerClients.forEach { (_, client) -> - gui.open(client) shouldBe true - } + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + client.gui() shouldBe gui + } - gui.viewers() shouldContainExactlyInAnyOrder playerClients.map { it.first } + gui.close() + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + client.gui() shouldBe null } + } + @Test + fun `should set isClosed to true`() = runTest { + val gui = TestGUI(serverMock) + gui.isClosed shouldBe false + gui.close() + gui.isClosed shouldBe true } - @Nested - inner class Contains { + @Test + fun `should unregister the GUI`() = runTest { + val gui = TestGUI(serverMock) + gui.register() + guiManager.guis shouldContainAll listOf(gui) + gui.close() + guiManager.guis shouldContainAll listOf() + } - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.contains(client) shouldBe false - } + @Test + fun `should not be able to open the GUI after closing it`() = runTest { + val gui = TestGUI(serverMock) + gui.register() + val (_, client) = registerPlayer() + gui.close() - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.open(client) shouldBe true - gui.contains(client) shouldBe true + shouldThrow { + gui.open(client) } + } + @Test + fun `should not be able to register the GUI after closing it`() = runTest { + val gui = TestGUI(serverMock) + gui.close() + shouldThrow { + gui.register() + } } + } - @Nested - inner class CloseForClient { + @Nested + inner class CloseForClient { - @Test - fun `should return false if the client is not viewing the GUI`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() - val initialInventoryViewType = player.openInventory.type + val initialInventoryViewType = player.openInventory.type - player.assertInventoryView(initialInventoryViewType) - gui.close(client, true) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } + player.assertInventoryView(initialInventoryViewType) + gui.close(client, true) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } - @Test - fun `should close the inventory if the client is viewing the GUI`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock) + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + gui.close(client, true) shouldBe true + player.assertInventoryView(initialInventoryViewType) + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `should stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { + runBlocking { + val gui = TestFilledGUI(serverMock, 10.minutes, InventoryType.ENDER_CHEST) + gui.register() val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type gui.open(client) shouldBe true player.assertInventoryView(gui.type) - gui.close(client, true) shouldBe true - player.assertInventoryView(initialInventoryViewType) - } - @Test - fun `should remove client inventory without closing it if closeInventory is false`() = - runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() + val openInventory = player.openInventory + val inventory = openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - gui.close(client, false) shouldBe true + gui.close(client, closeInventory) shouldBe true + gui.isInventoryLoading(inventory) shouldBe false + + if (closeInventory) { + player.assertInventoryView(initialInventoryViewType) + } else { player.assertInventoryView(gui.type) - gui.contains(client) shouldBe false } - + } } - @Nested - inner class Close { - - @Test - fun `should close all inventories and remove all viewers`() = runTest(timeout = 1.minutes) { - val gui = TestGUI(serverMock, InventoryType.BREWING) - gui.register() + @Test + fun `should remove client inventory without closing it if closeInventory is false`() = + runTest { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() - val playerClients = List(5) { registerPlayer() } - val initialInventoryViewType = playerClients.first().first.openInventory.type + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - client.gui() shouldBe gui - } + gui.close(client, false) shouldBe true + player.assertInventoryView(gui.type) - gui.close() - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - client.gui() shouldBe null - } + gui.contains(client) shouldBe false } - @Test - fun `should set isClosed to true`() = runTest { - val gui = TestGUI(serverMock) - gui.isClosed shouldBe false - gui.close() - gui.isClosed shouldBe true - } + @Test + fun `should not close for other clients`() = runTest { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + val initialInventoryViewType = player2.openInventory.type - @Test - fun `should unregister the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.register() - guiManager.guis shouldContainAll listOf(gui) - gui.close() - guiManager.guis shouldContainAll listOf() - } + gui.open(client) shouldBe true + gui.open(client2) shouldBe true - } + player.assertInventoryView(gui.type) + player2.assertInventoryView(gui.type) - private suspend fun registerPlayer(): Pair { - val player = serverMock.addPlayer() - val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - clientManager.put(player, client) - return player to client + gui.close(client2, true) shouldBe true + player.assertInventoryView(gui.type) + player2.assertInventoryView(initialInventoryViewType) } } - private class TestGUI(val serverMock: ServerMock, val type: InventoryType = InventoryType.HOPPER) : PlayerGUI() { - override fun createInventory(owner: InventoryHolder, client: Client): Inventory { - return serverMock.createInventory(owner, type) - } + private suspend fun registerPlayer(): Pair { + val player = serverMock.addPlayer() + val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) + clientManager.put(player, client) + return player to client + } +} - override fun getItemStacks(key: Client, size: Int): Flow { - return emptyFlow() - } +private class TestGUI(val serverMock: ServerMock, val type: InventoryType = InventoryType.HOPPER) : + PlayerGUI() { + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { + return serverMock.createInventory(owner, type) + } - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") - } + override fun getItemStacks(key: Client, size: Int): Flow { + return emptyFlow() } - private class TestFilledGUI( - val serverMock: ServerMock, - val delay: Duration? = null, - ) : PlayerGUI() { + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} - companion object { - val EXPECTED_INV = arrayOf( - ItemStack { type = Material.DIAMOND_ORE }, - ItemStack { type = Material.STICK }, - ) - } +private class TestFilledGUI( + val serverMock: ServerMock, + val delay: Duration? = null, + val type: InventoryType = InventoryType.CHEST +) : PlayerGUI() { + + companion object { + val EXPECTED_INV = arrayOf( + ItemStack { type = Material.DIAMOND_ORE }, + ItemStack { type = Material.STICK }, + ) + } - var calledThread: Thread? = null + var calledThread: Thread? = null - var newThread: Thread? = null + var newThread: Thread? = null - override fun createInventory(owner: InventoryHolder, client: Client): Inventory { - return serverMock.createInventory(owner, InventoryType.CHEST) - } + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { + return serverMock.createInventory(owner, type) + } - override fun getItemStacks(key: Client, size: Int): Flow { - calledThread = Thread.currentThread() - return flow { - delay?.let { delay(it) } - emit(0 to EXPECTED_INV[0]) - newThread = Thread.currentThread() - emit(1 to EXPECTED_INV[1]) - } + override fun getItemStacks(key: Client, size: Int): Flow { + calledThread = Thread.currentThread() + return flow { + delay?.let { delay(it) } + emit(0 to EXPECTED_INV[0]) + newThread = Thread.currentThread() + emit(1 to EXPECTED_INV[1]) } + } - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") - } + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") } } From 9bad4112229ed0acbfa078668a11c4928fc629e7 Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 14:55:30 +0100 Subject: [PATCH 17/24] test: Verify new creation inventory --- .../github/rushyverse/api/gui/PlayerGUITest.kt | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index ee7aba62..f6c5aab5 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -186,6 +186,22 @@ class PlayerGUITest : AbstractKoinTest() { player.openInventory.topInventory shouldNotBe player2.openInventory.topInventory } + @Test + fun `should create a new inventory for the same client if previous is closed before`() = runTest { + val gui = TestGUI(serverMock) + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + val firstInventory = player.openInventory.topInventory + + gui.close(client, true) shouldBe true + + gui.open(client) shouldBe true + player.openInventory.topInventory shouldNotBe firstInventory + + player.assertInventoryView(gui.type) + } + @Test fun `should fill the inventory in the same thread if no suspend operation`() { runBlocking { From 782840929a941eb64fe29f0a5ab83ddc72889c72 Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 16:23:51 +0100 Subject: [PATCH 18/24] fix: Avoid slowing down mutex when close --- .../github/rushyverse/api/gui/LocaleGUI.kt | 15 ++++----------- .../github/rushyverse/api/gui/PlayerGUI.kt | 19 +++++++++++-------- .../github/rushyverse/api/gui/SingleGUI.kt | 15 ++++----------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt index afe27e1b..4eaf1977 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/LocaleGUI.kt @@ -9,7 +9,6 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.job import kotlinx.coroutines.plus -import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.plugin.Plugin @@ -39,15 +38,9 @@ public abstract class LocaleGUI( } override suspend fun close(client: Client, closeInventory: Boolean): Boolean { - if (!closeInventory) { - return false - } - - return mutex.withLock { - if (unsafeContains(client)) { - client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) - true - } else false - } + return if (closeInventory && contains(client)) { + client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) + true + } else false } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index 7dc5568e..61b9e726 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -52,13 +52,16 @@ public abstract class PlayerGUI( } override suspend fun close(client: Client, closeInventory: Boolean): Boolean { - return mutex.withLock { inventories.remove(client) }?.run { - job.cancel(GUIClosedForClientException(client)) - job.join() - if (closeInventory) { - inventory.close() - } - true - } == true + val (inventory, job) = mutex.withLock { inventories.remove(client) } ?: return false + + job.cancel(GUIClosedForClientException(client)) + job.join() + + if (closeInventory) { + // Call out of the lock to avoid slowing down the mutex. + inventory.close() + } + + return true } } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index 2a965b1f..28468f81 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -7,7 +7,6 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.job import kotlinx.coroutines.plus -import kotlinx.coroutines.sync.withLock import org.bukkit.event.inventory.InventoryCloseEvent import org.bukkit.plugin.Plugin @@ -40,16 +39,10 @@ public abstract class SingleGUI( } override suspend fun close(client: Client, closeInventory: Boolean): Boolean { - if (!closeInventory) { - return false - } - - return mutex.withLock { - if (unsafeContains(client)) { - client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) - true - } else false - } + return if (closeInventory && contains(client)) { + client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) + true + } else false } override suspend fun getKey(client: Client): Any { From 92b63debd41bf7acda05eb7f23e95aad0325a36e Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 17:51:09 +0100 Subject: [PATCH 19/24] chore: Change method name & documentation --- .../kotlin/com/github/rushyverse/api/gui/GUI.kt | 17 +++++++++++++---- .../github/rushyverse/api/gui/PlayerGUITest.kt | 4 ++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index ab5bb48c..1ac5f626 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -172,7 +172,7 @@ public abstract class GUI( // That's why we start with unconfined dispatcher. return fillScope(key).launch(Dispatchers.Unconfined) { val size = inventory.size - val inventoryFlowItems = getItemStacks(key, size).cancellable() + val inventoryFlowItems = getItems(key, size).cancellable() if (loadingAnimation == null) { // Will fill the inventory bit by bit. @@ -206,12 +206,21 @@ public abstract class GUI( protected abstract suspend fun createInventory(key: T): Inventory /** - * Fill the inventory for the key. + * Create a new flow of ItemStack to fill the inventory with. + * ```kotlin + * flow { + * emit(0 to ItemStack(Material.STONE)) + * delay(1.seconds) // simulate a suspend operation + * emit(1 to ItemStack(Material.DIRT)) + * } + * ``` + * If the flow doesn't suspend the coroutine, + * the inventory will be filled in the same tick & thread than during the creation of the inventory. * @param key Key to fill the inventory for. * @param size Size of the inventory. - * @return Flow of ItemStack to fill the inventory with. + * @return Flow of [Item][ItemStack] with index. */ - protected abstract fun getItemStacks(key: T, size: Int): Flow + protected abstract fun getItems(key: T, size: Int): Flow /** * Check if the GUI contains the inventory. diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index f6c5aab5..4193e0dc 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -435,7 +435,7 @@ private class TestGUI(val serverMock: ServerMock, val type: InventoryType = Inve return serverMock.createInventory(owner, type) } - override fun getItemStacks(key: Client, size: Int): Flow { + override fun getItems(key: Client, size: Int): Flow { return emptyFlow() } @@ -470,7 +470,7 @@ private class TestFilledGUI( return serverMock.createInventory(owner, type) } - override fun getItemStacks(key: Client, size: Int): Flow { + override fun getItems(key: Client, size: Int): Flow { calledThread = Thread.currentThread() return flow { delay?.let { delay(it) } From e4ede857f638960a8c2d9496f4199b6a3c8639f6 Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 17:51:34 +0100 Subject: [PATCH 20/24] doc: Change ItemStack to Item --- src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index 1ac5f626..6e5d972c 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -206,7 +206,7 @@ public abstract class GUI( protected abstract suspend fun createInventory(key: T): Inventory /** - * Create a new flow of ItemStack to fill the inventory with. + * Create a new flow of [Item][ItemStack] to fill the inventory with. * ```kotlin * flow { * emit(0 to ItemStack(Material.STONE)) From b0d0b10150a56e2c947e5a8dde7fa2da57e7e93c Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 18:18:13 +0100 Subject: [PATCH 21/24] fix: Create abstract function without unnecessary params --- .../com/github/rushyverse/api/gui/GUI.kt | 2 +- .../github/rushyverse/api/gui/PlayerGUI.kt | 2 +- .../github/rushyverse/api/gui/SingleGUI.kt | 24 +++++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt index 6e5d972c..9f037538 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/GUI.kt @@ -203,7 +203,7 @@ public abstract class GUI( * @param key Key to create the inventory for. * @return New created inventory. */ - protected abstract suspend fun createInventory(key: T): Inventory + protected abstract fun createInventory(key: T): Inventory /** * Create a new flow of [Item][ItemStack] to fill the inventory with. diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt index 61b9e726..d5a096fc 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/PlayerGUI.kt @@ -32,7 +32,7 @@ public abstract class PlayerGUI( * @param key The client to create the inventory for. * @return The inventory for the client. */ - override suspend fun createInventory(key: Client): Inventory { + override fun createInventory(key: Client): Inventory { val player = key.requirePlayer() return createInventory(player, key) } diff --git a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt index 28468f81..3ec24c71 100644 --- a/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt +++ b/src/main/kotlin/com/github/rushyverse/api/gui/SingleGUI.kt @@ -5,9 +5,11 @@ import com.github.rushyverse.api.player.Client import com.github.shynixn.mccoroutine.bukkit.scope import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.job import kotlinx.coroutines.plus import org.bukkit.event.inventory.InventoryCloseEvent +import org.bukkit.inventory.Inventory import org.bukkit.plugin.Plugin /** @@ -33,11 +35,24 @@ public abstract class SingleGUI( private val KEY = Any() } + override suspend fun getKey(client: Client): Any { + return KEY + } + override suspend fun fillScope(key: Any): CoroutineScope { val scope = plugin.scope return scope + SupervisorJob(scope.coroutineContext.job) } + override fun createInventory(key: Any): Inventory { + return createInventory() + } + + /** + * @see createInventory(key) + */ + protected abstract fun createInventory(): Inventory + override suspend fun close(client: Client, closeInventory: Boolean): Boolean { return if (closeInventory && contains(client)) { client.player?.closeInventory(InventoryCloseEvent.Reason.PLUGIN) @@ -45,7 +60,12 @@ public abstract class SingleGUI( } else false } - override suspend fun getKey(client: Client): Any { - return KEY + override fun getItems(key: Any, size: Int): Flow { + return getItems(size) } + + /** + * @see getItems(key, size) + */ + protected abstract fun getItems(size: Int): Flow } From b06bff52324a28e97778bb84f9f0bd83b537d6de Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 21:21:15 +0100 Subject: [PATCH 22/24] test: Add tests for LocaleGUI --- .../rushyverse/api/gui/LocaleGUITest.kt | 516 ++++++++++++++++++ 1 file changed, 516 insertions(+) create mode 100644 src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt new file mode 100644 index 00000000..8186fe82 --- /dev/null +++ b/src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt @@ -0,0 +1,516 @@ +package com.github.rushyverse.api.gui + +import be.seeseemelk.mockbukkit.MockBukkit +import be.seeseemelk.mockbukkit.ServerMock +import be.seeseemelk.mockbukkit.entity.PlayerMock +import com.github.rushyverse.api.AbstractKoinTest +import com.github.rushyverse.api.extension.ItemStack +import com.github.rushyverse.api.player.Client +import com.github.rushyverse.api.player.ClientManager +import com.github.rushyverse.api.player.ClientManagerImpl +import com.github.rushyverse.api.player.language.LanguageManager +import com.github.rushyverse.api.translation.SupportedLanguage +import com.github.shynixn.mccoroutine.bukkit.scope +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.collections.shouldContainAll +import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import java.util.* +import kotlin.coroutines.EmptyCoroutineContext +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.bukkit.Material +import org.bukkit.event.inventory.InventoryClickEvent +import org.bukkit.event.inventory.InventoryType +import org.bukkit.inventory.Inventory +import org.bukkit.inventory.ItemStack +import org.bukkit.plugin.Plugin +import org.junit.jupiter.api.Nested +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +class LocaleGUITest : AbstractKoinTest() { + + private lateinit var guiManager: GUIManager + private lateinit var languageManager: LanguageManager + private lateinit var clientManager: ClientManager + private lateinit var serverMock: ServerMock + + @BeforeTest + override fun onBefore() { + super.onBefore() + guiManager = GUIManager() + languageManager = LanguageManager() + clientManager = ClientManagerImpl() + + loadApiTestModule { + single { guiManager } + single { clientManager } + single { languageManager } + } + + serverMock = MockBukkit.mock() + + mockkStatic("com.github.shynixn.mccoroutine.bukkit.MCCoroutineKt") + every { plugin.scope } returns CoroutineScope(EmptyCoroutineContext) + } + + @AfterTest + override fun onAfter() { + super.onAfter() + MockBukkit.unmock() + unmockkAll() + } + + @Nested + inner class Register { + + @Test + fun `should register if not already registered`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.register() shouldBe true + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should not register if already registered`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.register() shouldBe true + gui.register() shouldBe false + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should throw exception if GUI is closed`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.close() + shouldThrow { gui.register() } + } + } + + @Nested + inner class Viewers { + + @Test + fun `should return empty list if no client is viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.viewers().toList() shouldBe emptyList() + } + + @Test + fun `should return the list of clients viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val playerClients = List(5) { registerPlayer() } + + playerClients.forEach { (_, client) -> + gui.open(client) shouldBe true + } + + gui.viewers().toList() shouldContainExactlyInAnyOrder playerClients.map { it.first } + } + + } + + @Nested + inner class Contains { + + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (_, client) = registerPlayer() + gui.contains(client) shouldBe false + } + + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (_, client) = registerPlayer() + gui.open(client) shouldBe true + gui.contains(client) shouldBe true + } + + } + + @Nested + inner class Open { + + @Test + fun `should throw exception if GUI is closed`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.close() + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + shouldThrow { gui.open(client) } + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should do nothing if the client has the same GUI opened`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + + gui.open(client) shouldBe false + player.assertInventoryView(gui.type) + } + + @Test + fun `should do nothing if the player is dead`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + player.damage(Double.MAX_VALUE) + gui.open(client) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should create a new inventory according to the language client`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + languageManager.set(player, SupportedLanguage.ENGLISH) + languageManager.set(player2, SupportedLanguage.FRENCH) + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(gui.type) + player2.assertInventoryView(gui.type) + + player.openInventory.topInventory shouldNotBe player2.openInventory.topInventory + } + + @Test + fun `should use the same inventory according to the language client`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + languageManager.set(player, SupportedLanguage.FRENCH) + languageManager.set(player2, SupportedLanguage.FRENCH) + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(gui.type) + player2.assertInventoryView(gui.type) + + player.openInventory.topInventory shouldBe player2.openInventory.topInventory + } + + @Test + fun `should not create a new inventory for the same client if previously closed`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + val firstInventory = player.openInventory.topInventory + + gui.close(client, true) shouldBe true + + gui.open(client) shouldBe true + player.openInventory.topInventory shouldBe firstInventory + + player.assertInventoryView(gui.type) + } + + @Test + fun `should fill the inventory in the same thread if no suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() + + val gui = LocaleFillGUI(plugin, serverMock) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) + + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe false + + val content = inventory.contents + LocaleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in LocaleFillGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } + + gui.calledThread shouldBe currentThread + gui.newThread shouldBe currentThread + } + } + + @Test + fun `should fill the inventory in the other thread after suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() + + val delay = 100.milliseconds + val gui = LocaleFillGUI(plugin, serverMock, delay) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) + + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true + + val content = inventory.contents + content.forEach { it shouldBe null } + + delay(delay * 2) + gui.isInventoryLoading(inventory) shouldBe false + + LocaleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in LocaleFillGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } + + gui.calledThread shouldBe currentThread + gui.newThread shouldNotBe currentThread + } + } + } + + @Nested + inner class Close { + + @Test + fun `should close all inventories and remove all viewers`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock, InventoryType.BREWING) + gui.register() + + val playerClients = List(5) { registerPlayer() } + val initialInventoryViewType = playerClients.first().first.openInventory.type + + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + client.gui() shouldBe gui + } + + gui.close() + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + client.gui() shouldBe null + } + } + + @Test + fun `should set isClosed to true`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.isClosed shouldBe false + gui.close() + gui.isClosed shouldBe true + } + + @Test + fun `should unregister the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.register() + guiManager.guis shouldContainAll listOf(gui) + gui.close() + guiManager.guis shouldContainAll listOf() + } + + @Test + fun `should not be able to open the GUI after closing it`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.register() + val (_, client) = registerPlayer() + gui.close() + + shouldThrow { + gui.open(client) + } + } + + @Test + fun `should not be able to register the GUI after closing it`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + gui.close() + shouldThrow { + gui.register() + } + } + } + + @Nested + inner class CloseForClient { + + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + player.assertInventoryView(initialInventoryViewType) + gui.close(client, true) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + gui.close(client, true) shouldBe true + player.assertInventoryView(initialInventoryViewType) + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `should not stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { + runBlocking { + val gui = LocaleFillGUI(plugin, serverMock, 10.minutes, InventoryType.ENDER_CHEST) + gui.register() + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + + val openInventory = player.openInventory + val inventory = openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true + + gui.close(client, closeInventory) shouldBe closeInventory + gui.isInventoryLoading(inventory) shouldBe true + + if (closeInventory) { + player.assertInventoryView(initialInventoryViewType) + gui.contains(client) shouldBe false + } else { + player.assertInventoryView(gui.type) + gui.contains(client) shouldBe true + } + } + } + + @Test + fun `should not close for other clients`() = runTest { + val gui = LocaleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + val initialInventoryViewType = player2.openInventory.type + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(gui.type) + player2.assertInventoryView(gui.type) + + gui.close(client2, true) shouldBe true + player.assertInventoryView(gui.type) + player2.assertInventoryView(initialInventoryViewType) + } + } + + private suspend fun registerPlayer(): Pair { + val player = serverMock.addPlayer() + val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) + clientManager.put(player, client) + return player to client + } +} + +class LocaleNonFillGUI( + plugin: Plugin, + val serverMock: ServerMock, + val type: InventoryType = InventoryType.HOPPER +) : LocaleGUI(plugin) { + + override fun createInventory(key: Locale): Inventory { + return serverMock.createInventory(null, type) + } + + override fun getItems(key: Locale, size: Int): Flow { + return emptyFlow() + } + + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} + +class LocaleFillGUI( + plugin: Plugin, + val serverMock: ServerMock, + val delay: Duration? = null, + val type: InventoryType = InventoryType.CHEST +) : LocaleGUI(plugin) { + + companion object { + val EXPECTED_INV = arrayOf( + ItemStack { type = Material.DIAMOND_ORE }, + ItemStack { type = Material.STICK }, + ) + } + + var calledThread: Thread? = null + + var newThread: Thread? = null + + override fun createInventory(key: Locale): Inventory { + return serverMock.createInventory(null, type) + } + + override fun getItems(key: Locale, size: Int): Flow { + calledThread = Thread.currentThread() + return flow { + delay?.let { delay(it) } + emit(0 to EXPECTED_INV[0]) + newThread = Thread.currentThread() + emit(1 to EXPECTED_INV[1]) + } + } + + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} From cd35b89f4bd878324b6fd9a30975ab52abb0d728 Mon Sep 17 00:00:00 2001 From: Distractic Date: Mon, 18 Dec 2023 22:03:06 +0100 Subject: [PATCH 23/24] test: Add tests for SingleGUI --- .../rushyverse/api/gui/SingleGUITest.kt | 490 ++++++++++++++++++ 1 file changed, 490 insertions(+) create mode 100644 src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt new file mode 100644 index 00000000..a7cab9cf --- /dev/null +++ b/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt @@ -0,0 +1,490 @@ +package com.github.rushyverse.api.gui + +import be.seeseemelk.mockbukkit.MockBukkit +import be.seeseemelk.mockbukkit.ServerMock +import be.seeseemelk.mockbukkit.entity.PlayerMock +import com.github.rushyverse.api.AbstractKoinTest +import com.github.rushyverse.api.extension.ItemStack +import com.github.rushyverse.api.player.Client +import com.github.rushyverse.api.player.ClientManager +import com.github.rushyverse.api.player.ClientManagerImpl +import com.github.shynixn.mccoroutine.bukkit.scope +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.collections.shouldContainAll +import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import kotlin.coroutines.EmptyCoroutineContext +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.minutes +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.bukkit.Material +import org.bukkit.event.inventory.InventoryClickEvent +import org.bukkit.event.inventory.InventoryType +import org.bukkit.inventory.Inventory +import org.bukkit.inventory.ItemStack +import org.bukkit.plugin.Plugin +import org.junit.jupiter.api.Nested +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +class SingleGUITest : AbstractKoinTest() { + + private lateinit var guiManager: GUIManager + private lateinit var clientManager: ClientManager + private lateinit var serverMock: ServerMock + + @BeforeTest + override fun onBefore() { + super.onBefore() + guiManager = GUIManager() + clientManager = ClientManagerImpl() + + loadApiTestModule { + single { guiManager } + single { clientManager } + } + + serverMock = MockBukkit.mock() + + mockkStatic("com.github.shynixn.mccoroutine.bukkit.MCCoroutineKt") + every { plugin.scope } returns CoroutineScope(EmptyCoroutineContext) + } + + @AfterTest + override fun onAfter() { + super.onAfter() + MockBukkit.unmock() + unmockkAll() + } + + @Nested + inner class Register { + + @Test + fun `should register if not already registered`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.register() shouldBe true + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should not register if already registered`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.register() shouldBe true + gui.register() shouldBe false + guiManager.guis shouldContainAll listOf(gui) + } + + @Test + fun `should throw exception if GUI is closed`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.close() + shouldThrow { gui.register() } + } + } + + @Nested + inner class Viewers { + + @Test + fun `should return empty list if no client is viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.viewers().toList() shouldBe emptyList() + } + + @Test + fun `should return the list of clients viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val playerClients = List(5) { registerPlayer() } + + playerClients.forEach { (_, client) -> + gui.open(client) shouldBe true + } + + gui.viewers().toList() shouldContainExactlyInAnyOrder playerClients.map { it.first } + } + + } + + @Nested + inner class Contains { + + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (_, client) = registerPlayer() + gui.contains(client) shouldBe false + } + + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (_, client) = registerPlayer() + gui.open(client) shouldBe true + gui.contains(client) shouldBe true + } + + } + + @Nested + inner class Open { + + @Test + fun `should throw exception if GUI is closed`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.close() + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + shouldThrow { gui.open(client) } + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should do nothing if the client has the same GUI opened`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + + gui.open(client) shouldBe false + player.assertInventoryView(gui.type) + } + + @Test + fun `should do nothing if the player is dead`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + player.damage(Double.MAX_VALUE) + gui.open(client) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should use the same inventory for all clients`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val inventories = List(5) { + val (player, client) = registerPlayer() + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + + player.openInventory.topInventory + } + + inventories.all { it === inventories.first() } shouldBe true + } + + @Test + fun `should not create a new inventory for the same client if previously closed`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + val firstInventory = player.openInventory.topInventory + + gui.close(client, true) shouldBe true + + gui.open(client) shouldBe true + player.openInventory.topInventory shouldBe firstInventory + + player.assertInventoryView(gui.type) + } + + @Test + fun `should fill the inventory in the same thread if no suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() + + val gui = SingleFillGUI(plugin, serverMock) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) + + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe false + + val content = inventory.contents + SingleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in SingleFillGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } + + gui.calledThread shouldBe currentThread + gui.newThread shouldBe currentThread + } + } + + @Test + fun `should fill the inventory in the other thread after suspend operation`() { + runBlocking { + val currentThread = Thread.currentThread() + + val delay = 100.milliseconds + val gui = SingleFillGUI(plugin, serverMock, delay) + gui.register() + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + player.assertInventoryView(InventoryType.CHEST) + + val inventory = player.openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true + + val content = inventory.contents + content.forEach { it shouldBe null } + + delay(delay * 2) + gui.isInventoryLoading(inventory) shouldBe false + + SingleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + content[index] shouldBe item + } + + for (i in SingleFillGUI.EXPECTED_INV.size until content.size) { + content[i] shouldBe null + } + + gui.calledThread shouldBe currentThread + gui.newThread shouldNotBe currentThread + } + } + } + + @Nested + inner class Close { + + @Test + fun `should close all inventories and remove all viewers`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock, InventoryType.BREWING) + gui.register() + + val playerClients = List(5) { registerPlayer() } + val initialInventoryViewType = playerClients.first().first.openInventory.type + + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + client.gui() shouldBe gui + } + + gui.close() + playerClients.forEach { (player, client) -> + player.assertInventoryView(initialInventoryViewType) + client.gui() shouldBe null + } + } + + @Test + fun `should set isClosed to true`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.isClosed shouldBe false + gui.close() + gui.isClosed shouldBe true + } + + @Test + fun `should unregister the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.register() + guiManager.guis shouldContainAll listOf(gui) + gui.close() + guiManager.guis shouldContainAll listOf() + } + + @Test + fun `should not be able to open the GUI after closing it`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.register() + val (_, client) = registerPlayer() + gui.close() + + shouldThrow { + gui.open(client) + } + } + + @Test + fun `should not be able to register the GUI after closing it`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + gui.close() + shouldThrow { + gui.register() + } + } + } + + @Nested + inner class CloseForClient { + + @Test + fun `should return false if the client is not viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + player.assertInventoryView(initialInventoryViewType) + gui.close(client, true) shouldBe false + player.assertInventoryView(initialInventoryViewType) + } + + @Test + fun `should return true if the client is viewing the GUI`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + gui.close(client, true) shouldBe true + player.assertInventoryView(initialInventoryViewType) + } + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `should not stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { + runBlocking { + val gui = SingleFillGUI(plugin, serverMock, 10.minutes, InventoryType.ENDER_CHEST) + gui.register() + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(gui.type) + + val openInventory = player.openInventory + val inventory = openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true + + gui.close(client, closeInventory) shouldBe closeInventory + gui.isInventoryLoading(inventory) shouldBe true + + if (closeInventory) { + player.assertInventoryView(initialInventoryViewType) + gui.contains(client) shouldBe false + } else { + player.assertInventoryView(gui.type) + gui.contains(client) shouldBe true + } + } + } + + @Test + fun `should not close for other clients`() = runTest { + val gui = SingleNonFillGUI(plugin, serverMock) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + val initialInventoryViewType = player2.openInventory.type + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(gui.type) + player2.assertInventoryView(gui.type) + + gui.close(client2, true) shouldBe true + player.assertInventoryView(gui.type) + player2.assertInventoryView(initialInventoryViewType) + } + } + + private suspend fun registerPlayer(): Pair { + val player = serverMock.addPlayer() + val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) + clientManager.put(player, client) + return player to client + } +} + +class SingleNonFillGUI( + plugin: Plugin, + val serverMock: ServerMock, + val type: InventoryType = InventoryType.HOPPER +) : SingleGUI(plugin) { + + override fun createInventory(): Inventory { + return serverMock.createInventory(null, type) + } + + override fun getItems(size: Int): Flow { + return emptyFlow() + } + + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} + +class SingleFillGUI( + plugin: Plugin, + val serverMock: ServerMock, + val delay: Duration? = null, + val type: InventoryType = InventoryType.CHEST +) : SingleGUI(plugin) { + + companion object { + val EXPECTED_INV = Array(2) { + ItemStack { type = Material.entries.filter { it != Material.AIR }.random() } + } + } + + var calledThread: Thread? = null + + var newThread: Thread? = null + + override fun createInventory(): Inventory { + return serverMock.createInventory(null, type) + } + + override fun getItems(size: Int): Flow { + calledThread = Thread.currentThread() + return flow { + delay?.let { delay(it) } + EXPECTED_INV.forEachIndexed { index, item -> + emit(index to item) + } + newThread = Thread.currentThread() + } + } + + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} From a174f799a3b67bd4db5abbb72863daa1fbd5a6b7 Mon Sep 17 00:00:00 2001 From: Distractic Date: Tue, 19 Dec 2023 09:40:08 +0100 Subject: [PATCH 24/24] test: Unify GUI tests --- .../github/rushyverse/api/AbstractKoinTest.kt | 2 + .../{LocaleGUITest.kt => AbstractGUITest.kt} | 301 ++++--------- .../rushyverse/api/gui/LocalePlayerGUITest.kt | 226 ++++++++++ .../rushyverse/api/gui/PlayerGUITest.kt | 409 +++--------------- .../rushyverse/api/gui/SingleGUITest.kt | 391 ++--------------- 5 files changed, 410 insertions(+), 919 deletions(-) rename src/test/kotlin/com/github/rushyverse/api/gui/{LocaleGUITest.kt => AbstractGUITest.kt} (50%) create mode 100644 src/test/kotlin/com/github/rushyverse/api/gui/LocalePlayerGUITest.kt diff --git a/src/test/kotlin/com/github/rushyverse/api/AbstractKoinTest.kt b/src/test/kotlin/com/github/rushyverse/api/AbstractKoinTest.kt index df562752..fef9664c 100644 --- a/src/test/kotlin/com/github/rushyverse/api/AbstractKoinTest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/AbstractKoinTest.kt @@ -5,6 +5,7 @@ import com.github.rushyverse.api.koin.loadModule import com.github.rushyverse.api.utils.randomString import io.mockk.every import io.mockk.mockk +import io.mockk.unmockkAll import kotlin.test.AfterTest import kotlin.test.BeforeTest import org.bukkit.Server @@ -47,6 +48,7 @@ open class AbstractKoinTest { open fun onAfter() { CraftContext.stopKoin(pluginId) CraftContext.stopKoin(APIPlugin.ID_API) + unmockkAll() } fun loadTestModule(moduleDeclaration: ModuleDeclaration): Module = diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/AbstractGUITest.kt similarity index 50% rename from src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt rename to src/test/kotlin/com/github/rushyverse/api/gui/AbstractGUITest.kt index 8186fe82..abf9ff48 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/LocaleGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/AbstractGUITest.kt @@ -8,88 +8,63 @@ import com.github.rushyverse.api.extension.ItemStack import com.github.rushyverse.api.player.Client import com.github.rushyverse.api.player.ClientManager import com.github.rushyverse.api.player.ClientManagerImpl -import com.github.rushyverse.api.player.language.LanguageManager -import com.github.rushyverse.api.translation.SupportedLanguage -import com.github.shynixn.mccoroutine.bukkit.scope import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.collections.shouldContainAll import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe -import io.mockk.every -import io.mockk.mockkStatic -import io.mockk.unmockkAll -import java.util.* import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds -import kotlin.time.Duration.Companion.minutes import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emptyFlow -import kotlinx.coroutines.flow.flow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.bukkit.Material -import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryType -import org.bukkit.inventory.Inventory import org.bukkit.inventory.ItemStack -import org.bukkit.plugin.Plugin -import org.junit.jupiter.api.Nested -import org.junit.jupiter.params.ParameterizedTest -import org.junit.jupiter.params.provider.ValueSource -class LocaleGUITest : AbstractKoinTest() { +abstract class AbstractGUITest : AbstractKoinTest() { - private lateinit var guiManager: GUIManager - private lateinit var languageManager: LanguageManager - private lateinit var clientManager: ClientManager - private lateinit var serverMock: ServerMock + protected lateinit var guiManager: GUIManager + protected lateinit var clientManager: ClientManager + protected lateinit var serverMock: ServerMock @BeforeTest override fun onBefore() { super.onBefore() guiManager = GUIManager() - languageManager = LanguageManager() clientManager = ClientManagerImpl() loadApiTestModule { single { guiManager } single { clientManager } - single { languageManager } } serverMock = MockBukkit.mock() - - mockkStatic("com.github.shynixn.mccoroutine.bukkit.MCCoroutineKt") - every { plugin.scope } returns CoroutineScope(EmptyCoroutineContext) } @AfterTest override fun onAfter() { super.onAfter() MockBukkit.unmock() - unmockkAll() } - @Nested - inner class Register { + abstract inner class Register { @Test fun `should register if not already registered`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.register() shouldBe true guiManager.guis shouldContainAll listOf(gui) } @Test fun `should not register if already registered`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.register() shouldBe true gui.register() shouldBe false guiManager.guis shouldContainAll listOf(gui) @@ -97,24 +72,23 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should throw exception if GUI is closed`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.close() shouldThrow { gui.register() } } } - @Nested - inner class Viewers { + abstract inner class Viewers { @Test fun `should return empty list if no client is viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.viewers().toList() shouldBe emptyList() } @Test fun `should return the list of clients viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() val playerClients = List(5) { registerPlayer() } playerClients.forEach { (_, client) -> @@ -126,19 +100,18 @@ class LocaleGUITest : AbstractKoinTest() { } - @Nested - inner class Contains { + abstract inner class Contains { @Test fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() val (_, client) = registerPlayer() gui.contains(client) shouldBe false } @Test fun `should return true if the client is viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() val (_, client) = registerPlayer() gui.open(client) shouldBe true gui.contains(client) shouldBe true @@ -146,12 +119,11 @@ class LocaleGUITest : AbstractKoinTest() { } - @Nested - inner class Open { + abstract inner class Open { @Test fun `should throw exception if GUI is closed`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.close() val (player, client) = registerPlayer() @@ -162,120 +134,85 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should do nothing if the client has the same GUI opened`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val type = InventoryType.HOPPER + val gui = createNonFillGUI(inventoryType = type) gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) gui.open(client) shouldBe false - player.assertInventoryView(gui.type) + player.assertInventoryView(type) } @Test fun `should do nothing if the player is dead`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type - player.damage(Double.MAX_VALUE) + player.health = 0.0 gui.open(client) shouldBe false player.assertInventoryView(initialInventoryViewType) } @Test - fun `should create a new inventory according to the language client`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - val (player2, client2) = registerPlayer() - languageManager.set(player, SupportedLanguage.ENGLISH) - languageManager.set(player2, SupportedLanguage.FRENCH) - - gui.open(client) shouldBe true - gui.open(client2) shouldBe true - - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) - - player.openInventory.topInventory shouldNotBe player2.openInventory.topInventory - } - - @Test - fun `should use the same inventory according to the language client`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - val (player2, client2) = registerPlayer() - languageManager.set(player, SupportedLanguage.FRENCH) - languageManager.set(player2, SupportedLanguage.FRENCH) - - gui.open(client) shouldBe true - gui.open(client2) shouldBe true - - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) - - player.openInventory.topInventory shouldBe player2.openInventory.topInventory - } - - @Test - fun `should not create a new inventory for the same client if previously closed`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - val firstInventory = player.openInventory.topInventory - - gui.close(client, true) shouldBe true - - gui.open(client) shouldBe true - player.openInventory.topInventory shouldBe firstInventory + fun `should fill the inventory in the same thread if no suspend operation`() { - player.assertInventoryView(gui.type) - } + val items: Array = arrayOf( + ItemStack { type = Material.DIAMOND_ORE }, + ItemStack { type = Material.STICK }, + ) - @Test - fun `should fill the inventory in the same thread if no suspend operation`() { runBlocking { val currentThread = Thread.currentThread() - val gui = LocaleFillGUI(plugin, serverMock) + val type = InventoryType.ENDER_CHEST + val gui = createFillGUI(items, delay = null, inventoryType = type) gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) + player.assertInventoryView(type) val inventory = player.openInventory.topInventory gui.isInventoryLoading(inventory) shouldBe false val content = inventory.contents - LocaleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + items.forEachIndexed { index, item -> content[index] shouldBe item } - for (i in LocaleFillGUI.EXPECTED_INV.size until content.size) { + for (i in items.size until content.size) { content[i] shouldBe null } - gui.calledThread shouldBe currentThread - gui.newThread shouldBe currentThread + getFillThreadBeforeSuspend(gui) shouldBe currentThread + getFillThreadAfterSuspend(gui) shouldBe currentThread } } @Test fun `should fill the inventory in the other thread after suspend operation`() { + + val items: Array = arrayOf( + ItemStack { type = Material.DIAMOND_AXE }, + ItemStack { type = Material.ACACIA_LEAVES }, + ) + runBlocking { val currentThread = Thread.currentThread() + val type = InventoryType.ENDER_CHEST val delay = 100.milliseconds - val gui = LocaleFillGUI(plugin, serverMock, delay) + val gui = createFillGUI(items = items, delay = delay, inventoryType = type) gui.register() val (player, client) = registerPlayer() gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) + player.assertInventoryView(type) val inventory = player.openInventory.topInventory gui.isInventoryLoading(inventory) shouldBe true @@ -286,26 +223,27 @@ class LocaleGUITest : AbstractKoinTest() { delay(delay * 2) gui.isInventoryLoading(inventory) shouldBe false - LocaleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> + items.forEachIndexed { index, item -> content[index] shouldBe item } - for (i in LocaleFillGUI.EXPECTED_INV.size until content.size) { + for (i in items.size until content.size) { content[i] shouldBe null } - gui.calledThread shouldBe currentThread - gui.newThread shouldNotBe currentThread + getFillThreadBeforeSuspend(gui) shouldBe currentThread + getFillThreadAfterSuspend(gui) shouldNotBe currentThread } } + } - @Nested - inner class Close { + abstract inner class Close { @Test fun `should close all inventories and remove all viewers`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock, InventoryType.BREWING) + val type = InventoryType.BREWING + val gui = createNonFillGUI(type) gui.register() val playerClients = List(5) { registerPlayer() } @@ -314,7 +252,7 @@ class LocaleGUITest : AbstractKoinTest() { playerClients.forEach { (player, client) -> player.assertInventoryView(initialInventoryViewType) gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) client.gui() shouldBe gui } @@ -327,7 +265,7 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should set isClosed to true`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.isClosed shouldBe false gui.close() gui.isClosed shouldBe true @@ -335,7 +273,7 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should unregister the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.register() guiManager.guis shouldContainAll listOf(gui) gui.close() @@ -344,7 +282,7 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should not be able to open the GUI after closing it`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.register() val (_, client) = registerPlayer() gui.close() @@ -356,7 +294,7 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should not be able to register the GUI after closing it`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() gui.close() shouldThrow { gui.register() @@ -364,12 +302,11 @@ class LocaleGUITest : AbstractKoinTest() { } } - @Nested - inner class CloseForClient { + abstract inner class CloseForClient { @Test fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val gui = createNonFillGUI() val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type @@ -381,50 +318,22 @@ class LocaleGUITest : AbstractKoinTest() { @Test fun `should return true if the client is viewing the GUI`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val type = InventoryType.DISPENSER + val gui = createNonFillGUI(type) val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) gui.close(client, true) shouldBe true player.assertInventoryView(initialInventoryViewType) } - @ParameterizedTest - @ValueSource(booleans = [true, false]) - fun `should not stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { - runBlocking { - val gui = LocaleFillGUI(plugin, serverMock, 10.minutes, InventoryType.ENDER_CHEST) - gui.register() - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - - val openInventory = player.openInventory - val inventory = openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe true - - gui.close(client, closeInventory) shouldBe closeInventory - gui.isInventoryLoading(inventory) shouldBe true - - if (closeInventory) { - player.assertInventoryView(initialInventoryViewType) - gui.contains(client) shouldBe false - } else { - player.assertInventoryView(gui.type) - gui.contains(client) shouldBe true - } - } - } - @Test fun `should not close for other clients`() = runTest { - val gui = LocaleNonFillGUI(plugin, serverMock) + val type = InventoryType.HOPPER + val gui = createNonFillGUI(type) val (player, client) = registerPlayer() val (player2, client2) = registerPlayer() val initialInventoryViewType = player2.openInventory.type @@ -432,85 +341,33 @@ class LocaleGUITest : AbstractKoinTest() { gui.open(client) shouldBe true gui.open(client2) shouldBe true - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) + player.assertInventoryView(type) + player2.assertInventoryView(type) gui.close(client2, true) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) player2.assertInventoryView(initialInventoryViewType) } - } - - private suspend fun registerPlayer(): Pair { - val player = serverMock.addPlayer() - val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - clientManager.put(player, client) - return player to client - } -} - -class LocaleNonFillGUI( - plugin: Plugin, - val serverMock: ServerMock, - val type: InventoryType = InventoryType.HOPPER -) : LocaleGUI(plugin) { - - override fun createInventory(key: Locale): Inventory { - return serverMock.createInventory(null, type) - } - - override fun getItems(key: Locale, size: Int): Flow { - return emptyFlow() - } - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") - } -} -class LocaleFillGUI( - plugin: Plugin, - val serverMock: ServerMock, - val delay: Duration? = null, - val type: InventoryType = InventoryType.CHEST -) : LocaleGUI(plugin) { - - companion object { - val EXPECTED_INV = arrayOf( - ItemStack { type = Material.DIAMOND_ORE }, - ItemStack { type = Material.STICK }, - ) } - var calledThread: Thread? = null + abstract fun createNonFillGUI(inventoryType: InventoryType = InventoryType.HOPPER): GUI<*> - var newThread: Thread? = null + abstract fun createFillGUI( + items: Array, + inventoryType: InventoryType = InventoryType.HOPPER, + delay: Duration? = null + ): GUI<*> - override fun createInventory(key: Locale): Inventory { - return serverMock.createInventory(null, type) - } + abstract fun getFillThreadBeforeSuspend(gui: GUI<*>): Thread? - override fun getItems(key: Locale, size: Int): Flow { - calledThread = Thread.currentThread() - return flow { - delay?.let { delay(it) } - emit(0 to EXPECTED_INV[0]) - newThread = Thread.currentThread() - emit(1 to EXPECTED_INV[1]) - } - } + abstract fun getFillThreadAfterSuspend(gui: GUI<*>): Thread? - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") + protected suspend fun registerPlayer(): Pair { + val player = serverMock.addPlayer() + val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) + clientManager.put(player, client) + return player to client } } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/LocalePlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/LocalePlayerGUITest.kt new file mode 100644 index 00000000..3e0fdce6 --- /dev/null +++ b/src/test/kotlin/com/github/rushyverse/api/gui/LocalePlayerGUITest.kt @@ -0,0 +1,226 @@ +package com.github.rushyverse.api.gui + +import be.seeseemelk.mockbukkit.ServerMock +import com.github.rushyverse.api.player.Client +import com.github.rushyverse.api.player.language.LanguageManager +import com.github.rushyverse.api.translation.SupportedLanguage +import com.github.shynixn.mccoroutine.bukkit.scope +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockkStatic +import java.util.* +import kotlin.coroutines.EmptyCoroutineContext +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runTest +import org.bukkit.event.inventory.InventoryClickEvent +import org.bukkit.event.inventory.InventoryType +import org.bukkit.inventory.Inventory +import org.bukkit.inventory.ItemStack +import org.bukkit.plugin.Plugin +import org.junit.jupiter.api.Nested +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ValueSource + +class LocalePlayerGUITest : AbstractGUITest() { + + private lateinit var languageManager: LanguageManager + + @BeforeTest + override fun onBefore() { + super.onBefore() + languageManager = LanguageManager() + + loadApiTestModule { + single { languageManager } + } + + mockkStatic("com.github.shynixn.mccoroutine.bukkit.MCCoroutineKt") + every { plugin.scope } returns CoroutineScope(EmptyCoroutineContext) + } + + override fun createFillGUI(items: Array, inventoryType: InventoryType, delay: Duration?): GUI<*> { + return LocaleFillGUI(plugin, serverMock, inventoryType, items, delay) + } + + override fun createNonFillGUI(inventoryType: InventoryType): GUI<*> { + return LocaleNonFillGUI(plugin, serverMock, inventoryType) + } + + override fun getFillThreadAfterSuspend(gui: GUI<*>): Thread? { + return (gui as LocaleFillGUI).newThread + } + + override fun getFillThreadBeforeSuspend(gui: GUI<*>): Thread? { + return (gui as LocaleFillGUI).calledThread + } + + @Nested + inner class Register : AbstractGUITest.Register() + + @Nested + inner class Viewers : AbstractGUITest.Viewers() + + @Nested + inner class Contains : AbstractGUITest.Contains() + + @Nested + inner class Open : AbstractGUITest.Open() { + + @Test + fun `should create a new inventory according to the language client`() = runTest { + val type = InventoryType.HOPPER + val gui = createNonFillGUI(type) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + languageManager.set(player, SupportedLanguage.ENGLISH) + languageManager.set(player2, SupportedLanguage.FRENCH) + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(type) + player2.assertInventoryView(type) + + player.openInventory.topInventory shouldNotBe player2.openInventory.topInventory + } + + @Test + fun `should use the same inventory according to the language client`() = runTest { + val type = InventoryType.DISPENSER + val gui = createNonFillGUI(type) + val (player, client) = registerPlayer() + val (player2, client2) = registerPlayer() + languageManager.set(player, SupportedLanguage.FRENCH) + languageManager.set(player2, SupportedLanguage.FRENCH) + + gui.open(client) shouldBe true + gui.open(client2) shouldBe true + + player.assertInventoryView(type) + player2.assertInventoryView(type) + + player.openInventory.topInventory shouldBe player2.openInventory.topInventory + } + + @Test + fun `should not create a new inventory for the same client if previously closed`() = runTest { + val type = InventoryType.BREWING + val gui = createNonFillGUI(type) + val (player, client) = registerPlayer() + + gui.open(client) shouldBe true + val firstInventory = player.openInventory.topInventory + + gui.close(client, true) shouldBe true + + gui.open(client) shouldBe true + player.openInventory.topInventory shouldBe firstInventory + + player.assertInventoryView(type) + } + + } + + @Nested + inner class Close : AbstractGUITest.Close() + + @Nested + inner class CloseForClient : AbstractGUITest.CloseForClient() { + + @ParameterizedTest + @ValueSource(booleans = [true, false]) + fun `should not stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { + runBlocking { + val type = InventoryType.HOPPER + val gui = createFillGUI(emptyArray(), delay = 10.minutes, inventoryType = type) + gui.register() + val (player, client) = registerPlayer() + + val initialInventoryViewType = player.openInventory.type + + gui.open(client) shouldBe true + player.assertInventoryView(type) + + val openInventory = player.openInventory + val inventory = openInventory.topInventory + gui.isInventoryLoading(inventory) shouldBe true + + gui.close(client, closeInventory) shouldBe closeInventory + gui.isInventoryLoading(inventory) shouldBe true + + if (closeInventory) { + player.assertInventoryView(initialInventoryViewType) + gui.contains(client) shouldBe false + } else { + player.assertInventoryView(type) + gui.contains(client) shouldBe true + } + } + } + } +} + +private abstract class AbstractLocaleGUITest( + plugin: Plugin, + val serverMock: ServerMock, + val type: InventoryType = InventoryType.HOPPER +) : LocaleGUI(plugin) { + + override fun createInventory(key: Locale): Inventory { + return serverMock.createInventory(null, type) + } + + override suspend fun onClick( + client: Client, + clickedInventory: Inventory, + clickedItem: ItemStack, + event: InventoryClickEvent + ) { + error("Should not be called") + } +} + +private class LocaleNonFillGUI( + plugin: Plugin, + serverMock: ServerMock, + type: InventoryType +) : AbstractLocaleGUITest(plugin, serverMock, type) { + + override fun getItems(key: Locale, size: Int): Flow { + return emptyFlow() + } +} + +private class LocaleFillGUI( + plugin: Plugin, + serverMock: ServerMock, + type: InventoryType, + val items: Array, + val delay: Duration? +) : AbstractLocaleGUITest(plugin, serverMock, type) { + + var calledThread: Thread? = null + + var newThread: Thread? = null + + override fun getItems(key: Locale, size: Int): Flow { + calledThread = Thread.currentThread() + return flow { + delay?.let { delay(it) } + items.forEachIndexed { index, item -> + emit(index to item) + } + newThread = Thread.currentThread() + } + } +} diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt index 4193e0dc..6f5877d4 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/PlayerGUITest.kt @@ -1,33 +1,18 @@ package com.github.rushyverse.api.gui -import be.seeseemelk.mockbukkit.MockBukkit import be.seeseemelk.mockbukkit.ServerMock -import be.seeseemelk.mockbukkit.entity.PlayerMock -import com.github.rushyverse.api.AbstractKoinTest -import com.github.rushyverse.api.extension.ItemStack import com.github.rushyverse.api.player.Client -import com.github.rushyverse.api.player.ClientManager -import com.github.rushyverse.api.player.ClientManagerImpl -import io.kotest.assertions.throwables.shouldThrow -import io.kotest.matchers.collections.shouldContainAll -import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe -import kotlin.coroutines.EmptyCoroutineContext -import kotlin.test.AfterTest -import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.time.Duration -import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes -import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest -import org.bukkit.Material import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryType import org.bukkit.inventory.Inventory @@ -37,158 +22,40 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource -class PlayerGUITest : AbstractKoinTest() { - - private lateinit var guiManager: GUIManager - private lateinit var clientManager: ClientManager - private lateinit var serverMock: ServerMock - - @BeforeTest - override fun onBefore() { - super.onBefore() - guiManager = GUIManager() - clientManager = ClientManagerImpl() - - loadApiTestModule { - single { guiManager } - single { clientManager } - } - - serverMock = MockBukkit.mock() - } - - @AfterTest - override fun onAfter() { - super.onAfter() - MockBukkit.unmock() - } +class PlayerGUITest : AbstractGUITest() { @Nested - inner class Register { - - @Test - fun `should register if not already registered`() = runTest { - val gui = TestGUI(serverMock) - gui.register() shouldBe true - guiManager.guis shouldContainAll listOf(gui) - } - - @Test - fun `should not register if already registered`() = runTest { - val gui = TestGUI(serverMock) - gui.register() shouldBe true - gui.register() shouldBe false - guiManager.guis shouldContainAll listOf(gui) - } - - @Test - fun `should throw exception if GUI is closed`() = runTest { - val gui = TestGUI(serverMock) - gui.close() - shouldThrow { gui.register() } - } - } + inner class Register : AbstractGUITest.Register() @Nested - inner class Viewers { - - @Test - fun `should return empty list if no client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.viewers().toList() shouldBe emptyList() - } - - @Test - fun `should return the list of clients viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val playerClients = List(5) { registerPlayer() } - - playerClients.forEach { (_, client) -> - gui.open(client) shouldBe true - } - - gui.viewers().toList() shouldContainExactlyInAnyOrder playerClients.map { it.first } - } - - } + inner class Viewers : AbstractGUITest.Viewers() @Nested - inner class Contains { - - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.contains(client) shouldBe false - } - - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (_, client) = registerPlayer() - gui.open(client) shouldBe true - gui.contains(client) shouldBe true - } - - } + inner class Contains : AbstractGUITest.Contains() @Nested - inner class Open { - - @Test - fun `should throw exception if GUI is closed`() = runTest { - val gui = TestGUI(serverMock) - gui.close() - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - shouldThrow { gui.open(client) } - player.assertInventoryView(initialInventoryViewType) - } - - @Test - fun `should do nothing if the client has the same GUI opened`() = runTest { - val gui = TestGUI(serverMock) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - - gui.open(client) shouldBe false - player.assertInventoryView(gui.type) - } - - @Test - fun `should do nothing if the player is dead`() = runTest { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - player.damage(Double.MAX_VALUE) - gui.open(client) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } + inner class Open : AbstractGUITest.Open() { @Test fun `should create a new inventory for the client`() = runTest { - val gui = TestGUI(serverMock) + val type = InventoryType.ENDER_CHEST + val gui = createNonFillGUI(type) val (player, client) = registerPlayer() val (player2, client2) = registerPlayer() gui.open(client) shouldBe true gui.open(client2) shouldBe true - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) + player.assertInventoryView(type) + player2.assertInventoryView(type) player.openInventory.topInventory shouldNotBe player2.openInventory.topInventory } @Test fun `should create a new inventory for the same client if previous is closed before`() = runTest { - val gui = TestGUI(serverMock) + val type = InventoryType.BREWING + val gui = createNonFillGUI(type) val (player, client) = registerPlayer() gui.open(client) shouldBe true @@ -199,178 +66,29 @@ class PlayerGUITest : AbstractKoinTest() { gui.open(client) shouldBe true player.openInventory.topInventory shouldNotBe firstInventory - player.assertInventoryView(gui.type) - } - - @Test - fun `should fill the inventory in the same thread if no suspend operation`() { - runBlocking { - val currentThread = Thread.currentThread() - - val gui = TestFilledGUI(serverMock) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) - - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe false - - val content = inventory.contents - TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } - - for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } - - gui.calledThread shouldBe currentThread - gui.newThread shouldBe currentThread - } - } - - @Test - fun `should fill the inventory in the other thread after suspend operation`() { - runBlocking { - val currentThread = Thread.currentThread() - - val delay = 100.milliseconds - val gui = TestFilledGUI(serverMock, delay) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) - - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe true - - val content = inventory.contents - content.forEach { it shouldBe null } - - delay(delay * 2) - gui.isInventoryLoading(inventory) shouldBe false - - TestFilledGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } - - for (i in TestFilledGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } - - gui.calledThread shouldBe currentThread - gui.newThread shouldNotBe currentThread - } + player.assertInventoryView(type) } } @Nested - inner class Close { - - @Test - fun `should close all inventories and remove all viewers`() = runTest { - val gui = TestGUI(serverMock, InventoryType.BREWING) - gui.register() - - val playerClients = List(5) { registerPlayer() } - val initialInventoryViewType = playerClients.first().first.openInventory.type - - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - client.gui() shouldBe gui - } - - gui.close() - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - client.gui() shouldBe null - } - } - - @Test - fun `should set isClosed to true`() = runTest { - val gui = TestGUI(serverMock) - gui.isClosed shouldBe false - gui.close() - gui.isClosed shouldBe true - } - - @Test - fun `should unregister the GUI`() = runTest { - val gui = TestGUI(serverMock) - gui.register() - guiManager.guis shouldContainAll listOf(gui) - gui.close() - guiManager.guis shouldContainAll listOf() - } - - @Test - fun `should not be able to open the GUI after closing it`() = runTest { - val gui = TestGUI(serverMock) - gui.register() - val (_, client) = registerPlayer() - gui.close() - - shouldThrow { - gui.open(client) - } - } - - @Test - fun `should not be able to register the GUI after closing it`() = runTest { - val gui = TestGUI(serverMock) - gui.close() - shouldThrow { - gui.register() - } - } - } + inner class Close : AbstractGUITest.Close() @Nested - inner class CloseForClient { - - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - player.assertInventoryView(initialInventoryViewType) - gui.close(client, true) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } - - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - gui.close(client, true) shouldBe true - player.assertInventoryView(initialInventoryViewType) - } + inner class CloseForClient : AbstractGUITest.CloseForClient() { @ParameterizedTest @ValueSource(booleans = [true, false]) fun `should stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { runBlocking { - val gui = TestFilledGUI(serverMock, 10.minutes, InventoryType.ENDER_CHEST) + val type = InventoryType.DROPPER + val gui = createFillGUI(items = emptyArray(), inventoryType = type, delay = 10.minutes) gui.register() val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) val openInventory = player.openInventory val inventory = openInventory.topInventory @@ -382,7 +100,7 @@ class PlayerGUITest : AbstractKoinTest() { if (closeInventory) { player.assertInventoryView(initialInventoryViewType) } else { - player.assertInventoryView(gui.type) + player.assertInventoryView(type) } } } @@ -390,55 +108,46 @@ class PlayerGUITest : AbstractKoinTest() { @Test fun `should remove client inventory without closing it if closeInventory is false`() = runTest { - val gui = TestGUI(serverMock) + val type = InventoryType.ENDER_CHEST + val gui = NonFillGUI(serverMock, type = type) val (player, client) = registerPlayer() gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) gui.close(client, false) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) gui.contains(client) shouldBe false } + } - @Test - fun `should not close for other clients`() = runTest { - val gui = TestGUI(serverMock) - val (player, client) = registerPlayer() - val (player2, client2) = registerPlayer() - val initialInventoryViewType = player2.openInventory.type - - gui.open(client) shouldBe true - gui.open(client2) shouldBe true + override fun createNonFillGUI(inventoryType: InventoryType): GUI<*> { + return NonFillGUI(serverMock, inventoryType) + } - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) + override fun createFillGUI(items: Array, inventoryType: InventoryType, delay: Duration?): GUI<*> { + return FillGUI(serverMock, inventoryType, items, delay) + } - gui.close(client2, true) shouldBe true - player.assertInventoryView(gui.type) - player2.assertInventoryView(initialInventoryViewType) - } + override fun getFillThreadBeforeSuspend(gui: GUI<*>): Thread? { + return (gui as FillGUI).calledThread } - private suspend fun registerPlayer(): Pair { - val player = serverMock.addPlayer() - val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - clientManager.put(player, client) - return player to client + override fun getFillThreadAfterSuspend(gui: GUI<*>): Thread? { + return (gui as FillGUI).newThread } } -private class TestGUI(val serverMock: ServerMock, val type: InventoryType = InventoryType.HOPPER) : - PlayerGUI() { +private abstract class AbstractPlayerGUITest( + val serverMock: ServerMock, + val type: InventoryType +) : PlayerGUI() { + override fun createInventory(owner: InventoryHolder, client: Client): Inventory { return serverMock.createInventory(owner, type) } - override fun getItems(key: Client, size: Int): Flow { - return emptyFlow() - } - override suspend fun onClick( client: Client, clickedInventory: Inventory, @@ -449,43 +158,35 @@ private class TestGUI(val serverMock: ServerMock, val type: InventoryType = Inve } } -private class TestFilledGUI( - val serverMock: ServerMock, - val delay: Duration? = null, - val type: InventoryType = InventoryType.CHEST -) : PlayerGUI() { +private class NonFillGUI( + serverMock: ServerMock, + type: InventoryType +) : AbstractPlayerGUITest(serverMock, type) { - companion object { - val EXPECTED_INV = arrayOf( - ItemStack { type = Material.DIAMOND_ORE }, - ItemStack { type = Material.STICK }, - ) + override fun getItems(key: Client, size: Int): Flow { + return emptyFlow() } +} + +private class FillGUI( + serverMock: ServerMock, + type: InventoryType, + val items: Array, + val delay: Duration? +) : AbstractPlayerGUITest(serverMock, type) { var calledThread: Thread? = null var newThread: Thread? = null - override fun createInventory(owner: InventoryHolder, client: Client): Inventory { - return serverMock.createInventory(owner, type) - } - override fun getItems(key: Client, size: Int): Flow { calledThread = Thread.currentThread() return flow { delay?.let { delay(it) } - emit(0 to EXPECTED_INV[0]) + items.forEachIndexed { index, item -> + emit(index to item) + } newThread = Thread.currentThread() - emit(1 to EXPECTED_INV[1]) } } - - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") - } } diff --git a/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt b/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt index a7cab9cf..0cd3c26f 100644 --- a/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt +++ b/src/test/kotlin/com/github/rushyverse/api/gui/SingleGUITest.kt @@ -1,28 +1,15 @@ package com.github.rushyverse.api.gui -import be.seeseemelk.mockbukkit.MockBukkit import be.seeseemelk.mockbukkit.ServerMock -import be.seeseemelk.mockbukkit.entity.PlayerMock -import com.github.rushyverse.api.AbstractKoinTest -import com.github.rushyverse.api.extension.ItemStack import com.github.rushyverse.api.player.Client -import com.github.rushyverse.api.player.ClientManager -import com.github.rushyverse.api.player.ClientManagerImpl import com.github.shynixn.mccoroutine.bukkit.scope -import io.kotest.assertions.throwables.shouldThrow -import io.kotest.matchers.collections.shouldContainAll -import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder import io.kotest.matchers.shouldBe -import io.kotest.matchers.shouldNotBe import io.mockk.every import io.mockk.mockkStatic -import io.mockk.unmockkAll import kotlin.coroutines.EmptyCoroutineContext -import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.time.Duration -import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.minutes import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.delay @@ -31,7 +18,6 @@ import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest -import org.bukkit.Material import org.bukkit.event.inventory.InventoryClickEvent import org.bukkit.event.inventory.InventoryType import org.bukkit.inventory.Inventory @@ -41,151 +27,51 @@ import org.junit.jupiter.api.Nested import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.ValueSource -class SingleGUITest : AbstractKoinTest() { - - private lateinit var guiManager: GUIManager - private lateinit var clientManager: ClientManager - private lateinit var serverMock: ServerMock +class SingleGUITest : AbstractGUITest() { @BeforeTest override fun onBefore() { super.onBefore() - guiManager = GUIManager() - clientManager = ClientManagerImpl() - - loadApiTestModule { - single { guiManager } - single { clientManager } - } - - serverMock = MockBukkit.mock() - mockkStatic("com.github.shynixn.mccoroutine.bukkit.MCCoroutineKt") every { plugin.scope } returns CoroutineScope(EmptyCoroutineContext) } - @AfterTest - override fun onAfter() { - super.onAfter() - MockBukkit.unmock() - unmockkAll() + override fun createNonFillGUI(inventoryType: InventoryType): GUI<*> { + return SingleNonFillGUI(plugin, serverMock, inventoryType) } - @Nested - inner class Register { - - @Test - fun `should register if not already registered`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.register() shouldBe true - guiManager.guis shouldContainAll listOf(gui) - } - - @Test - fun `should not register if already registered`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.register() shouldBe true - gui.register() shouldBe false - guiManager.guis shouldContainAll listOf(gui) - } - - @Test - fun `should throw exception if GUI is closed`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.close() - shouldThrow { gui.register() } - } + override fun createFillGUI(items: Array, inventoryType: InventoryType, delay: Duration?): GUI<*> { + return SingleFillGUI(plugin, serverMock, inventoryType, items, delay) } - @Nested - inner class Viewers { - - @Test - fun `should return empty list if no client is viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.viewers().toList() shouldBe emptyList() - } - - @Test - fun `should return the list of clients viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val playerClients = List(5) { registerPlayer() } - - playerClients.forEach { (_, client) -> - gui.open(client) shouldBe true - } - - gui.viewers().toList() shouldContainExactlyInAnyOrder playerClients.map { it.first } - } - + override fun getFillThreadBeforeSuspend(gui: GUI<*>): Thread? { + return (gui as SingleFillGUI).calledThread } - @Nested - inner class Contains { - - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (_, client) = registerPlayer() - gui.contains(client) shouldBe false - } - - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (_, client) = registerPlayer() - gui.open(client) shouldBe true - gui.contains(client) shouldBe true - } - + override fun getFillThreadAfterSuspend(gui: GUI<*>): Thread? { + return (gui as SingleFillGUI).newThread } @Nested - inner class Open { - - @Test - fun `should throw exception if GUI is closed`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.close() - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - shouldThrow { gui.open(client) } - player.assertInventoryView(initialInventoryViewType) - } + inner class Register : AbstractGUITest.Register() - @Test - fun `should do nothing if the client has the same GUI opened`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - - gui.open(client) shouldBe false - player.assertInventoryView(gui.type) - } - - @Test - fun `should do nothing if the player is dead`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() + @Nested + inner class Viewers : AbstractGUITest.Viewers() - val initialInventoryViewType = player.openInventory.type + @Nested + inner class Contains : AbstractGUITest.Contains() - player.damage(Double.MAX_VALUE) - gui.open(client) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } + @Nested + inner class Open : AbstractGUITest.Open() { @Test fun `should use the same inventory for all clients`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) + val type = InventoryType.ENDER_CHEST + val gui = createNonFillGUI(type) val inventories = List(5) { val (player, client) = registerPlayer() gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) player.openInventory.topInventory } @@ -195,7 +81,8 @@ class SingleGUITest : AbstractKoinTest() { @Test fun `should not create a new inventory for the same client if previously closed`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) + val type = InventoryType.BREWING + val gui = createNonFillGUI(type) val (player, client) = registerPlayer() gui.open(client) shouldBe true @@ -206,178 +93,29 @@ class SingleGUITest : AbstractKoinTest() { gui.open(client) shouldBe true player.openInventory.topInventory shouldBe firstInventory - player.assertInventoryView(gui.type) - } - - @Test - fun `should fill the inventory in the same thread if no suspend operation`() { - runBlocking { - val currentThread = Thread.currentThread() - - val gui = SingleFillGUI(plugin, serverMock) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) - - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe false - - val content = inventory.contents - SingleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } - - for (i in SingleFillGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } - - gui.calledThread shouldBe currentThread - gui.newThread shouldBe currentThread - } - } - - @Test - fun `should fill the inventory in the other thread after suspend operation`() { - runBlocking { - val currentThread = Thread.currentThread() - - val delay = 100.milliseconds - val gui = SingleFillGUI(plugin, serverMock, delay) - gui.register() - val (player, client) = registerPlayer() - - gui.open(client) shouldBe true - player.assertInventoryView(InventoryType.CHEST) - - val inventory = player.openInventory.topInventory - gui.isInventoryLoading(inventory) shouldBe true - - val content = inventory.contents - content.forEach { it shouldBe null } - - delay(delay * 2) - gui.isInventoryLoading(inventory) shouldBe false - - SingleFillGUI.EXPECTED_INV.forEachIndexed { index, item -> - content[index] shouldBe item - } - - for (i in SingleFillGUI.EXPECTED_INV.size until content.size) { - content[i] shouldBe null - } - - gui.calledThread shouldBe currentThread - gui.newThread shouldNotBe currentThread - } + player.assertInventoryView(type) } } @Nested - inner class Close { - - @Test - fun `should close all inventories and remove all viewers`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock, InventoryType.BREWING) - gui.register() - - val playerClients = List(5) { registerPlayer() } - val initialInventoryViewType = playerClients.first().first.openInventory.type - - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - client.gui() shouldBe gui - } - - gui.close() - playerClients.forEach { (player, client) -> - player.assertInventoryView(initialInventoryViewType) - client.gui() shouldBe null - } - } - - @Test - fun `should set isClosed to true`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.isClosed shouldBe false - gui.close() - gui.isClosed shouldBe true - } - - @Test - fun `should unregister the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.register() - guiManager.guis shouldContainAll listOf(gui) - gui.close() - guiManager.guis shouldContainAll listOf() - } - - @Test - fun `should not be able to open the GUI after closing it`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.register() - val (_, client) = registerPlayer() - gui.close() - - shouldThrow { - gui.open(client) - } - } - - @Test - fun `should not be able to register the GUI after closing it`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - gui.close() - shouldThrow { - gui.register() - } - } - } + inner class Close : AbstractGUITest.Close() @Nested - inner class CloseForClient { - - @Test - fun `should return false if the client is not viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - player.assertInventoryView(initialInventoryViewType) - gui.close(client, true) shouldBe false - player.assertInventoryView(initialInventoryViewType) - } - - @Test - fun `should return true if the client is viewing the GUI`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - - val initialInventoryViewType = player.openInventory.type - - gui.open(client) shouldBe true - player.assertInventoryView(gui.type) - gui.close(client, true) shouldBe true - player.assertInventoryView(initialInventoryViewType) - } + inner class CloseForClient : AbstractGUITest.CloseForClient() { @ParameterizedTest @ValueSource(booleans = [true, false]) fun `should not stop loading the inventory if the client is viewing the GUI`(closeInventory: Boolean) { runBlocking { - val gui = SingleFillGUI(plugin, serverMock, 10.minutes, InventoryType.ENDER_CHEST) + val type = InventoryType.DROPPER + val gui = createFillGUI(emptyArray(), delay = 10.minutes, inventoryType = type) gui.register() val (player, client) = registerPlayer() val initialInventoryViewType = player.openInventory.type gui.open(client) shouldBe true - player.assertInventoryView(gui.type) + player.assertInventoryView(type) val openInventory = player.openInventory val inventory = openInventory.topInventory @@ -390,53 +128,25 @@ class SingleGUITest : AbstractKoinTest() { player.assertInventoryView(initialInventoryViewType) gui.contains(client) shouldBe false } else { - player.assertInventoryView(gui.type) + player.assertInventoryView(type) gui.contains(client) shouldBe true } } } - @Test - fun `should not close for other clients`() = runTest { - val gui = SingleNonFillGUI(plugin, serverMock) - val (player, client) = registerPlayer() - val (player2, client2) = registerPlayer() - val initialInventoryViewType = player2.openInventory.type - - gui.open(client) shouldBe true - gui.open(client2) shouldBe true - - player.assertInventoryView(gui.type) - player2.assertInventoryView(gui.type) - - gui.close(client2, true) shouldBe true - player.assertInventoryView(gui.type) - player2.assertInventoryView(initialInventoryViewType) - } - } - - private suspend fun registerPlayer(): Pair { - val player = serverMock.addPlayer() - val client = Client(player.uniqueId, CoroutineScope(EmptyCoroutineContext)) - clientManager.put(player, client) - return player to client } } -class SingleNonFillGUI( +private abstract class AbstractSingleGUITest( plugin: Plugin, val serverMock: ServerMock, - val type: InventoryType = InventoryType.HOPPER + val type: InventoryType ) : SingleGUI(plugin) { override fun createInventory(): Inventory { return serverMock.createInventory(null, type) } - override fun getItems(size: Int): Flow { - return emptyFlow() - } - override suspend fun onClick( client: Client, clickedInventory: Inventory, @@ -445,46 +155,41 @@ class SingleNonFillGUI( ) { error("Should not be called") } + } -class SingleFillGUI( +private class SingleNonFillGUI( plugin: Plugin, - val serverMock: ServerMock, - val delay: Duration? = null, - val type: InventoryType = InventoryType.CHEST -) : SingleGUI(plugin) { + serverMock: ServerMock, + type: InventoryType +) : AbstractSingleGUITest(plugin, serverMock, type) { - companion object { - val EXPECTED_INV = Array(2) { - ItemStack { type = Material.entries.filter { it != Material.AIR }.random() } - } + override fun getItems(size: Int): Flow { + return emptyFlow() } +} + +private class SingleFillGUI( + plugin: Plugin, + serverMock: ServerMock, + type: InventoryType, + val items: Array, + val delay: Duration? +) : AbstractSingleGUITest(plugin, serverMock, type) { + var calledThread: Thread? = null var newThread: Thread? = null - override fun createInventory(): Inventory { - return serverMock.createInventory(null, type) - } - override fun getItems(size: Int): Flow { calledThread = Thread.currentThread() return flow { delay?.let { delay(it) } - EXPECTED_INV.forEachIndexed { index, item -> + items.forEachIndexed { index, item -> emit(index to item) } newThread = Thread.currentThread() } } - - override suspend fun onClick( - client: Client, - clickedInventory: Inventory, - clickedItem: ItemStack, - event: InventoryClickEvent - ) { - error("Should not be called") - } }