From f10a4ebe6c68596371d0c331f545b1398a77f539 Mon Sep 17 00:00:00 2001 From: Daan Vanden Bosch Date: Thu, 2 Dec 2021 22:08:26 +0100 Subject: [PATCH] Improved LoadingStatusCell and its usage in Table, TableController and HuntMethodStore. Added a unit test for MethodsForEpisodeController. --- .../phantasmal/web/core/models/Server.kt | 9 ++- .../web/core/persistence/Persister.kt | 26 ++++--- .../MethodsForEpisodeController.kt | 76 +++++++++---------- .../huntOptimizer/stores/HuntMethodStore.kt | 15 ++-- .../MethodsForEpisodeControllerTests.kt | 39 ++++++++++ .../phantasmal/web/test/TestComponents.kt | 39 +++++++--- .../phantasmal/webui/LoadingStatusCell.kt | 54 +++++++------ .../webui/controllers/TableController.kt | 6 +- .../world/phantasmal/webui/widgets/Table.kt | 28 ++++--- 9 files changed, 184 insertions(+), 108 deletions(-) create mode 100644 web/src/test/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeControllerTests.kt diff --git a/web/src/main/kotlin/world/phantasmal/web/core/models/Server.kt b/web/src/main/kotlin/world/phantasmal/web/core/models/Server.kt index 35cccb1e..83c5bfdf 100644 --- a/web/src/main/kotlin/world/phantasmal/web/core/models/Server.kt +++ b/web/src/main/kotlin/world/phantasmal/web/core/models/Server.kt @@ -3,6 +3,11 @@ package world.phantasmal.web.core.models /** * Represents a PSO private server. */ -enum class Server(val uiName: String, val slug: String) { - Ephinea("Ephinea", "ephinea") +enum class Server( + /** Display name shown to the user. */ + val uiName: String, + /** Used in URLs, do not change these. */ + val slug: String, +) { + Ephinea(uiName = "Ephinea", slug = "ephinea") } diff --git a/web/src/main/kotlin/world/phantasmal/web/core/persistence/Persister.kt b/web/src/main/kotlin/world/phantasmal/web/core/persistence/Persister.kt index 720396e8..31c001fe 100644 --- a/web/src/main/kotlin/world/phantasmal/web/core/persistence/Persister.kt +++ b/web/src/main/kotlin/world/phantasmal/web/core/persistence/Persister.kt @@ -1,5 +1,7 @@ package world.phantasmal.web.core.persistence +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext import kotlinx.serialization.KSerializer import kotlinx.serialization.json.Json import kotlinx.serialization.serializer @@ -21,10 +23,12 @@ abstract class Persister(private val store: KeyValueStore) { // Method suspends so we can use async storage in the future. @Suppress("RedundantSuspendModifier") protected suspend fun persist(key: String, data: T, serializer: KSerializer) { - try { - store.put(key, format.encodeToString(serializer, data)) - } catch (e: Throwable) { - logger.error(e) { "Couldn't persist ${key}." } + withContext(Dispatchers.Default) { + try { + store.put(key, format.encodeToString(serializer, data)) + } catch (e: Throwable) { + logger.error(e) { "Couldn't persist ${key}." } + } } } @@ -42,12 +46,14 @@ abstract class Persister(private val store: KeyValueStore) { // Method suspends so we can use async storage in the future. @Suppress("RedundantSuspendModifier") protected suspend fun load(key: String, serializer: KSerializer): T? = - try { - val json = store.get(key) - json?.let { format.decodeFromString(serializer, it) } - } catch (e: Throwable) { - logger.error(e) { "Couldn't load ${key}." } - null + withContext(Dispatchers.Default) { + try { + val json = store.get(key) + json?.let { format.decodeFromString(serializer, it) } + } catch (e: Throwable) { + logger.error(e) { "Couldn't load ${key}." } + null + } } protected suspend inline fun loadForServer(server: Server, key: String): T? = diff --git a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt index 479dc5c9..c2fb6472 100644 --- a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt +++ b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt @@ -21,11 +21,47 @@ class MethodsForEpisodeController( private val methods = mutableListCell() private val enemies: List = NpcType.VALUES.filter { it.enemy && it.episode == episode } + private var sortColumns: List> = emptyList() + + private val comparator: Comparator = + Comparator { a, b -> + for (sortColumn in sortColumns) { + val cmp = when (sortColumn.column.key) { + METHOD_COL_KEY -> + a.name.asDynamic().localeCompare(b.name).unsafeCast() + + TIME_COL_KEY -> a.time.value.compareTo(b.time.value) + + else -> { + val type = NpcType.valueOf(sortColumn.column.key) + (a.enemyCounts[type] ?: 0) - (b.enemyCounts[type] ?: 0) + } + } + + if (cmp != 0) { + return@Comparator if (sortColumn.direction == SortDirection.Asc) cmp else -cmp + } + } + + 0 + } + override val fixedColumns = 2 - override val values: ListCell = methods + override val values: ListCell by lazy { + // TODO: Use ListCell.sortedWith when this is available. + observe(huntMethodStore.methods) { allMethods -> + methods.value = allMethods + .asSequence() + .filter { it.episode == episode } + .sortedWith(comparator) + .toList() + } - override val valuesStatus: LoadingStatusCell = huntMethodStore.methodsStatus + methods + } + + override val loadingStatus: LoadingStatusCell = huntMethodStore.methodsStatus override val columns: ListCell> = listCell( Column( @@ -60,42 +96,6 @@ class MethodsForEpisodeController( }.toTypedArray() ) - private var sortColumns: List> = emptyList() - - private val comparator: Comparator = - Comparator { a, b -> - for (sortColumn in sortColumns) { - val cmp = when (sortColumn.column.key) { - METHOD_COL_KEY -> - a.name.asDynamic().localeCompare(b.name).unsafeCast() - - TIME_COL_KEY -> a.time.value.compareTo(b.time.value) - - else -> { - val type = NpcType.valueOf(sortColumn.column.key) - (a.enemyCounts[type] ?: 0) - (b.enemyCounts[type] ?: 0) - } - } - - if (cmp != 0) { - return@Comparator if (sortColumn.direction == SortDirection.Asc) cmp else -cmp - } - } - - 0 - } - - init { - // TODO: Use ListCell.sortedWith when this is available. - observe(huntMethodStore.methods) { allMethods -> - methods.value = allMethods - .asSequence() - .filter { it.episode == episode } - .sortedWith(comparator) - .toList() - } - } - override fun sort(sortColumns: List>) { this.sortColumns = sortColumns methods.sortWith(comparator) diff --git a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt index 467c2fd3..ad549917 100644 --- a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt +++ b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt @@ -7,8 +7,8 @@ import world.phantasmal.observable.cell.list.mutableListCell import world.phantasmal.psolib.Episode import world.phantasmal.psolib.fileFormats.quest.NpcType import world.phantasmal.web.core.loading.AssetLoader -import world.phantasmal.web.core.models.Server import world.phantasmal.web.core.stores.UiStore +import world.phantasmal.web.huntOptimizer.HuntOptimizerUrls.methods import world.phantasmal.web.huntOptimizer.models.HuntMethodModel import world.phantasmal.web.huntOptimizer.models.SimpleQuestModel import world.phantasmal.web.huntOptimizer.persistence.HuntMethodPersister @@ -27,22 +27,27 @@ class HuntMethodStore( private val huntMethodPersister: HuntMethodPersister, ) : Store() { private val _methods = mutableListCell { arrayOf(it.time) } + private val _methodsStatus = LoadingStatusCellImpl(scope, "methods", ::loadMethods) + /** Hunting methods supported by the current server. */ val methods: ListCell by lazy { - observe(uiStore.server) { loadMethods(it) } + observe(uiStore.server) { _methodsStatus.load() } _methods } - private val _methodsStatus = LoadingStatusCellImpl("methods") + /** Loading status of [methods]. */ val methodsStatus: LoadingStatusCell = _methodsStatus suspend fun setMethodTime(method: HuntMethodModel, time: Duration) { method.setUserTime(time) + huntMethodPersister.persistMethodUserTimes(methods.value, uiStore.server.value) } - private fun loadMethods(server: Server) { - _methodsStatus.load(scope) { + private suspend fun loadMethods() { + val server = uiStore.server.value + + withContext(Dispatchers.Default) { val quests = assetLoader.load>("/quests.${server.slug}.json") val methods = quests diff --git a/web/src/test/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeControllerTests.kt b/web/src/test/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeControllerTests.kt new file mode 100644 index 00000000..18bdfe24 --- /dev/null +++ b/web/src/test/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeControllerTests.kt @@ -0,0 +1,39 @@ +package world.phantasmal.web.huntOptimizer.controllers + +import world.phantasmal.psolib.Episode +import world.phantasmal.web.huntOptimizer.stores.HuntMethodStore +import world.phantasmal.web.test.WebTestSuite +import world.phantasmal.webui.LoadingStatus +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class MethodsForEpisodeControllerTests : WebTestSuite { + @Test + fun methods_for_the_given_episode_are_loaded_when_necessary() = testAsync { + for (episode in Episode.values()) { + val ctrl = disposer.add( + MethodsForEpisodeController( + // Create our own store each time to ensure methods is uninitialized. + disposer.add(HuntMethodStore( + components.uiStore, + components.assetLoader, + components.huntMethodPersister, + )), + episode, + ) + ) + + assertEquals(LoadingStatus.Uninitialized, ctrl.loadingStatus.value) + + // Start loading methods by accessing values. + ctrl.values + + ctrl.loadingStatus.await() + + assertEquals(LoadingStatus.Ok, ctrl.loadingStatus.value) + + assertTrue(ctrl.values.value.all { it.episode == episode }) + } + } +} diff --git a/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt b/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt index d8ef572f..a592cb14 100644 --- a/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt +++ b/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt @@ -10,15 +10,20 @@ import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.disposable import world.phantasmal.testUtils.TestContext import world.phantasmal.web.core.loading.AssetLoader +import world.phantasmal.web.core.persistence.KeyValueStore +import world.phantasmal.web.core.persistence.MemoryKeyValueStore import world.phantasmal.web.core.rendering.DisposableThreeRenderer import world.phantasmal.web.core.stores.ApplicationUrl import world.phantasmal.web.core.stores.UiStore import world.phantasmal.web.core.undo.UndoManager import world.phantasmal.web.externals.three.WebGLRenderer +import world.phantasmal.web.huntOptimizer.persistence.HuntMethodPersister +import world.phantasmal.web.huntOptimizer.stores.HuntMethodStore import world.phantasmal.web.questEditor.loading.AreaAssetLoader import world.phantasmal.web.questEditor.loading.QuestLoader import world.phantasmal.web.questEditor.stores.AreaStore import world.phantasmal.web.questEditor.stores.QuestEditorStore +import kotlin.properties.ReadWriteProperty import kotlin.reflect.KProperty /** @@ -52,6 +57,12 @@ class TestComponents(private val ctx: TestContext) { var questLoader: QuestLoader by default { QuestLoader(assetLoader) } + // Persistence + + var keyValueStore: KeyValueStore by default { MemoryKeyValueStore() } + + var huntMethodPersister: HuntMethodPersister by default { HuntMethodPersister(keyValueStore) } + // Undo var undoManager: UndoManager by default { UndoManager() } @@ -62,6 +73,10 @@ class TestComponents(private val ctx: TestContext) { var areaStore: AreaStore by default { AreaStore(areaAssetLoader) } + var huntMethodStore: HuntMethodStore by default { + HuntMethodStore(uiStore, assetLoader, huntMethodPersister) + } + var questEditorStore: QuestEditorStore by default { QuestEditorStore(questLoader, uiStore, areaStore, undoManager, initializeNewQuest = false) } @@ -78,30 +93,30 @@ class TestComponents(private val ctx: TestContext) { private fun default(defaultValue: () -> T) = LazyDefault(defaultValue) - private inner class LazyDefault(private val defaultValue: () -> T) { + private inner class LazyDefault( + private val defaultValue: () -> T, + ) : ReadWriteProperty { + private var initialized = false private var value: T? = null - operator fun getValue(thisRef: Any?, prop: KProperty<*>): T { + override operator fun getValue(thisRef: Any?, property: KProperty<*>): T { if (!initialized) { - val value = defaultValue() - - if (value is Disposable) { - ctx.disposer.add(value) - } - - this.value = value - initialized = true + setValue(defaultValue()) } return value.unsafeCast() } - operator fun setValue(thisRef: Any?, prop: KProperty<*>, value: T) { + override operator fun setValue(thisRef: Any?, property: KProperty<*>, value: T) { require(!initialized) { - "Property ${prop.name} is already initialized." + "Property ${property.name} is already initialized." } + setValue(value) + } + + private fun setValue(value: T) { if (value is Disposable) { ctx.disposer.add(value) } diff --git a/webui/src/main/kotlin/world/phantasmal/webui/LoadingStatusCell.kt b/webui/src/main/kotlin/world/phantasmal/webui/LoadingStatusCell.kt index 5c6f4a21..b17e2bb4 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/LoadingStatusCell.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/LoadingStatusCell.kt @@ -3,7 +3,6 @@ package world.phantasmal.webui import kotlinx.coroutines.* import mu.KotlinLogging import world.phantasmal.observable.cell.Cell -import world.phantasmal.observable.cell.ImmutableCell import world.phantasmal.observable.cell.SimpleCell import kotlin.time.measureTime @@ -18,58 +17,65 @@ enum class LoadingStatus { } interface LoadingStatusCell : Cell { - suspend fun awaitLoad() + /** Await the current load, if a load in ongoing. */ + suspend fun await() } -class ImmutableLoadingStatusCell(status: LoadingStatus) : - LoadingStatusCell, - Cell by ImmutableCell(status) { - - override suspend fun awaitLoad() { - // Nothing to await. - } -} - -class LoadingStatusCellImpl( - private val cellDelegate: SimpleCell, +class LoadingStatusCellImpl private constructor( + private val scope: CoroutineScope, private val dataName: String, + /** Will be called with [Dispatchers.Main] context. */ + private val loadData: suspend () -> Unit, + private val cellDelegate: SimpleCell, ) : LoadingStatusCell, Cell by cellDelegate { - constructor(dataName: String) : this(SimpleCell(LoadingStatus.Uninitialized), dataName) + constructor( + scope: CoroutineScope, + dataName: String, + loadData: suspend () -> Unit, + ) : this(scope, dataName, loadData, SimpleCell(LoadingStatus.Uninitialized)) - private var job: Job? = null + private var currentJob: Job? = null - fun load(scope: CoroutineScope, loadData: suspend () -> Unit) { + fun load() { logger.trace { "Loading $dataName." } cellDelegate.value = if (value == LoadingStatus.Uninitialized) LoadingStatus.InitialLoad else LoadingStatus.Loading - job = scope.launch { + currentJob?.cancel("New load started.") + + currentJob = scope.launch(Dispatchers.Main) { var success = false try { val duration = measureTime { - withContext(Dispatchers.Default) { - loadData() - } + loadData() } logger.trace { "Loaded $dataName in ${duration.inWholeMilliseconds}ms." } success = true + } catch (e: CancellationException) { + logger.trace(e) { "Loading $dataName was cancelled." } } catch (e: Exception) { logger.error(e) { "Error while loading $dataName." } - } finally { - job = null + } + // Only reset job and set value when a new job hasn't been started in the meantime. + if (coroutineContext.job == currentJob) { + currentJob = null cellDelegate.value = if (success) LoadingStatus.Ok else LoadingStatus.Error } } } - override suspend fun awaitLoad() { - job?.join() + override suspend fun await() { + currentJob?.let { + if (!it.isCompleted) { + it.join() + } + } } } diff --git a/webui/src/main/kotlin/world/phantasmal/webui/controllers/TableController.kt b/webui/src/main/kotlin/world/phantasmal/webui/controllers/TableController.kt index 58ea07aa..8940bdf2 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/controllers/TableController.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/controllers/TableController.kt @@ -3,8 +3,6 @@ package world.phantasmal.webui.controllers import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.list.ListCell import world.phantasmal.observable.cell.nullCell -import world.phantasmal.webui.ImmutableLoadingStatusCell -import world.phantasmal.webui.LoadingStatus import world.phantasmal.webui.LoadingStatusCell class Column( @@ -44,9 +42,7 @@ abstract class TableController : Controller() { /** Each value is represented by a row in the table. */ abstract val values: ListCell - open val valuesStatus: LoadingStatusCell = - // Assume values are already loaded by default. - ImmutableLoadingStatusCell(LoadingStatus.Ok) + open val loadingStatus: LoadingStatusCell? = null abstract val columns: ListCell> diff --git a/webui/src/main/kotlin/world/phantasmal/webui/widgets/Table.kt b/webui/src/main/kotlin/world/phantasmal/webui/widgets/Table.kt index f98abf04..49531158 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/widgets/Table.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/widgets/Table.kt @@ -29,18 +29,22 @@ class Table( div { className = "pw-table-notification" - observe(ctrl.valuesStatus) { - when (it) { - LoadingStatus.InitialLoad -> { - hidden = false - innerText = "Loading..." - } - LoadingStatus.Error -> { - hidden = false - innerText = "An error occurred while loading this table." - } - else -> { - hidden = true + ctrl.loadingStatus?.let { loadingStatus -> + observe(loadingStatus) { status -> + when (status) { + LoadingStatus.Uninitialized, + LoadingStatus.InitialLoad, + -> { + hidden = false + innerText = "Loading..." + } + LoadingStatus.Error -> { + hidden = false + innerText = "An error occurred while loading this table." + } + else -> { + hidden = true + } } } }