From 327dfe79bbc6634d064838a87d8aae0666c12bb6 Mon Sep 17 00:00:00 2001 From: Daan Vanden Bosch Date: Thu, 27 May 2021 15:00:19 +0200 Subject: [PATCH] Observables will now always see a consistent view of their dependencies when they change. --- .../phantasmal/core/unsafe/UnsafeCast.kt | 12 +- .../phantasmal/core/unsafe/UnsafeCast.kt | 4 +- .../phantasmal/core/unsafe/UnsafeCast.kt | 2 +- .../world/phantasmal/lib/asm/BytecodeIr.kt | 2 +- .../observable/AbstractDependency.kt | 13 + .../phantasmal/observable/CallbackObserver.kt | 28 +++ .../world/phantasmal/observable/Dependency.kt | 14 ++ .../world/phantasmal/observable/Dependent.kt | 21 ++ .../world/phantasmal/observable/Observable.kt | 2 +- .../world/phantasmal/observable/Observer.kt | 2 +- .../phantasmal/observable/SimpleEmitter.kt | 20 +- .../observable/cell/AbstractCell.kt | 30 ++- .../observable/cell/AbstractDependentCell.kt | 78 ++---- .../world/phantasmal/observable/cell/Cell.kt | 2 +- .../observable/cell/CellExtensions.kt | 3 + .../observable/cell/DelegatingCell.kt | 7 +- .../observable/cell/DependentCell.kt | 55 ++++- .../cell/FlatteningDependentCell.kt | 98 +++++--- .../phantasmal/observable/cell/SimpleCell.kt | 7 +- .../phantasmal/observable/cell/StaticCell.kt | 3 +- .../cell/list/AbstractDependentListCell.kt | 141 +++-------- .../observable/cell/list/AbstractListCell.kt | 136 ++--------- .../observable/cell/list/DependentListCell.kt | 34 ++- .../observable/cell/list/FilteredListCell.kt | 220 +++++++---------- .../cell/list/FlatteningDependentListCell.kt | 77 ++++-- .../observable/cell/list/FoldedCell.kt | 49 ---- .../observable/cell/list/ListCell.kt | 14 +- .../observable/cell/list/ListCellCreation.kt | 5 +- .../observable/cell/list/ListObserver.kt | 37 +-- .../observable/cell/list/MutableListCell.kt | 2 +- .../observable/cell/list/SimpleListCell.kt | 224 +++++++++++++++--- .../observable/cell/list/StaticListCell.kt | 16 +- .../phantasmal/observable/ObservableTests.kt | 4 +- .../phantasmal/observable/cell/CellTests.kt | 21 ++ .../cell/CellWithDependenciesTests.kt | 53 +++++ .../observable/cell/DelegatingCellTests.kt | 2 +- .../observable/cell/DependentCellTests.kt | 19 +- ...ndentCellTransitiveDependencyEmitsTests.kt | 24 +- .../observable/cell/RegularCellTests.kt | 2 - .../cell/list/DependentListCellTests.kt | 16 +- .../cell/list/FilteredListCellTests.kt | 91 ++++--- ...ndentListCellDirectDependencyEmitsTests.kt | 4 +- ...tListCellTransitiveDependencyEmitsTests.kt | 21 +- .../observable/cell/list/ListCellTests.kt | 29 ++- .../cell/list/MutableListCellTests.kt | 90 ++++--- .../cell/list/SimpleListCellTests.kt | 154 +++++++++++- .../phantasmal/web/core/undo/UndoManager.kt | 6 +- .../stores/HuntOptimizerStore.kt | 2 +- .../phantasmal/web/questEditor/QuestEditor.kt | 41 ++-- .../questEditor/loading/AreaAssetLoader.kt | 2 - .../questEditor/rendering/QuestMeshManager.kt | 21 +- .../questEditor/stores/QuestEditorStore.kt | 5 +- .../controllers/EventsControllerTests.kt | 77 ++++++ .../phantasmal/web/test/TestComponents.kt | 2 +- .../world/phantasmal/web/test/TestModels.kt | 12 +- .../kotlin/world/phantasmal/webui/dom/Dom.kt | 33 +-- .../webui/dom/HTMLElementSizeCell.kt | 56 ++--- 57 files changed, 1339 insertions(+), 806 deletions(-) create mode 100644 observable/src/commonMain/kotlin/world/phantasmal/observable/AbstractDependency.kt create mode 100644 observable/src/commonMain/kotlin/world/phantasmal/observable/CallbackObserver.kt create mode 100644 observable/src/commonMain/kotlin/world/phantasmal/observable/Dependency.kt create mode 100644 observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt delete mode 100644 observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FoldedCell.kt create mode 100644 observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt create mode 100644 web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt diff --git a/core/src/commonMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt b/core/src/commonMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt index bdfc5f4e..f896c041 100644 --- a/core/src/commonMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt +++ b/core/src/commonMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt @@ -1,14 +1,14 @@ package world.phantasmal.core.unsafe /** - * Asserts that receiver is of type T. No runtime check happens in KJS. Should only be used when - * absolutely certain that receiver is indeed a T. + * Asserts that [value] is of type T. No runtime check happens in KJS. Should only be used when it's + * absolutely certain that [value] is indeed a T. */ -expect inline fun Any?.unsafeCast(): T +expect inline fun unsafeCast(value: Any?): T /** - * Asserts that T is not null. No runtime check happens in KJS. Should only be used when absolutely - * certain that T is indeed not null. + * Asserts that [value] is not null. No runtime check happens in KJS. Should only be used when it's + * absolutely certain that [value] is indeed not null. */ @Suppress("NOTHING_TO_INLINE") -inline fun T?.unsafeAssertNotNull(): T = unsafeCast() +inline fun unsafeAssertNotNull(value: T?): T = unsafeCast(value) diff --git a/core/src/jsMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt b/core/src/jsMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt index 7c05752f..45bfe108 100644 --- a/core/src/jsMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt +++ b/core/src/jsMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt @@ -1,6 +1,4 @@ package world.phantasmal.core.unsafe -import kotlin.js.unsafeCast as kotlinUnsafeCast - @Suppress("NOTHING_TO_INLINE") -actual inline fun Any?.unsafeCast(): T = kotlinUnsafeCast() +actual inline fun unsafeCast(value: Any?): T = value.unsafeCast() diff --git a/core/src/jvmMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt b/core/src/jvmMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt index 81e3a373..8e3ed233 100644 --- a/core/src/jvmMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt +++ b/core/src/jvmMain/kotlin/world/phantasmal/core/unsafe/UnsafeCast.kt @@ -3,4 +3,4 @@ package world.phantasmal.core.unsafe @Suppress("UNCHECKED_CAST", "NOTHING_TO_INLINE") -actual inline fun Any?.unsafeCast(): T = this as T +actual inline fun unsafeCast(value: Any?): T = value as T diff --git a/lib/src/commonMain/kotlin/world/phantasmal/lib/asm/BytecodeIr.kt b/lib/src/commonMain/kotlin/world/phantasmal/lib/asm/BytecodeIr.kt index e3802424..7d5d60bd 100644 --- a/lib/src/commonMain/kotlin/world/phantasmal/lib/asm/BytecodeIr.kt +++ b/lib/src/commonMain/kotlin/world/phantasmal/lib/asm/BytecodeIr.kt @@ -138,7 +138,7 @@ class Instruction( } } - return paramToArgs.unsafeAssertNotNull()[paramIndex] + return unsafeAssertNotNull(paramToArgs)[paramIndex] } /** diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/AbstractDependency.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/AbstractDependency.kt new file mode 100644 index 00000000..579c2854 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/AbstractDependency.kt @@ -0,0 +1,13 @@ +package world.phantasmal.observable + +abstract class AbstractDependency : Dependency { + protected val dependents: MutableList = mutableListOf() + + override fun addDependent(dependent: Dependent) { + dependents.add(dependent) + } + + override fun removeDependent(dependent: Dependent) { + dependents.remove(dependent) + } +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/CallbackObserver.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/CallbackObserver.kt new file mode 100644 index 00000000..63f20f27 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/CallbackObserver.kt @@ -0,0 +1,28 @@ +package world.phantasmal.observable + +import world.phantasmal.core.disposable.TrackedDisposable +import world.phantasmal.core.unsafe.unsafeCast + +class CallbackObserver>( + private val dependency: Dependency, + private val callback: (E) -> Unit, +) : TrackedDisposable(), Dependent { + init { + dependency.addDependent(this) + } + + override fun dispose() { + dependency.removeDependent(this) + super.dispose() + } + + override fun dependencyMightChange() { + // Do nothing. + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (event != null) { + callback(unsafeCast(event)) + } + } +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependency.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependency.kt new file mode 100644 index 00000000..06220dd1 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependency.kt @@ -0,0 +1,14 @@ +package world.phantasmal.observable + +interface Dependency { + /** + * This method is not meant to be called from typical application code. Usually you'll want to + * use [Observable.observe]. + */ + fun addDependent(dependent: Dependent) + + /** + * This method is not meant to be called from typical application code. + */ + fun removeDependent(dependent: Dependent) +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt new file mode 100644 index 00000000..9c8943c0 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt @@ -0,0 +1,21 @@ +package world.phantasmal.observable + +interface Dependent { + /** + * This method is not meant to be called from typical application code. + * + * Called whenever a dependency of this dependent might change. Sometimes a dependency doesn't + * know that it will actually change, just that it might change. Always call [dependencyChanged] + * after calling this method. + * + * E.g. C depends on B and B depends on A. A is about to change, so it calls [dependencyMightChange] on B. At this point B doesn't know whether it will actually change since the new value of A doesn't necessarily result in a new value for B (e.g. B = A % 2 and A changes from 0 to 2). So B then calls [dependencyMightChange] on C. + */ + fun dependencyMightChange() + + /** + * This method is not meant to be called from typical application code. + * + * Always call [dependencyMightChange] before calling this method. + */ + fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/Observable.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/Observable.kt index 5965b601..3568aee5 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/Observable.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/Observable.kt @@ -2,6 +2,6 @@ package world.phantasmal.observable import world.phantasmal.core.disposable.Disposable -interface Observable { +interface Observable : Dependency { fun observe(observer: Observer): Disposable } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/Observer.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/Observer.kt index 0c7c71bd..ac24aef3 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/Observer.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/Observer.kt @@ -4,4 +4,4 @@ open class ChangeEvent(val value: T) { operator fun component1() = value } -typealias Observer = (event: ChangeEvent) -> Unit +typealias Observer = (ChangeEvent) -> Unit diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/SimpleEmitter.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/SimpleEmitter.kt index 7dd9fb30..dd1e750c 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/SimpleEmitter.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/SimpleEmitter.kt @@ -1,20 +1,18 @@ package world.phantasmal.observable import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable -class SimpleEmitter : Emitter { - private val observers = mutableListOf>() +class SimpleEmitter : AbstractDependency(), Emitter { + override fun emit(event: ChangeEvent) { + for (dependent in dependents) { + dependent.dependencyMightChange() + } - override fun observe(observer: Observer): Disposable { - observers.add(observer) - - return disposable { - observers.remove(observer) + for (dependent in dependents) { + dependent.dependencyChanged(this, event) } } - override fun emit(event: ChangeEvent) { - observers.forEach { it(event) } - } + override fun observe(observer: Observer): Disposable = + CallbackObserver(this, observer) } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt index a80d8eb1..6ba8db07 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt @@ -1,30 +1,42 @@ package world.phantasmal.observable.cell import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable +import world.phantasmal.observable.AbstractDependency +import world.phantasmal.observable.CallbackObserver import world.phantasmal.observable.ChangeEvent import world.phantasmal.observable.Observer -abstract class AbstractCell : Cell { - protected val observers: MutableList> = mutableListOf() +abstract class AbstractCell : AbstractDependency(), Cell { + private var mightChangeEmitted = false final override fun observe(observer: Observer): Disposable = observe(callNow = false, observer) override fun observe(callNow: Boolean, observer: Observer): Disposable { - observers.add(observer) + val observingCell = CallbackObserver(this, observer) if (callNow) { observer(ChangeEvent(value)) } - return disposable { - observers.remove(observer) + return observingCell + } + + protected fun emitMightChange() { + if (!mightChangeEmitted) { + mightChangeEmitted = true + + for (dependent in dependents) { + dependent.dependencyMightChange() + } } } - protected fun emit() { - val event = ChangeEvent(value) - observers.forEach { it(event) } + protected fun emitChanged(event: ChangeEvent?) { + mightChangeEmitted = false + + for (dependent in dependents) { + dependent.dependencyChanged(this, event) + } } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt index 6327b944..f928f75e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt @@ -1,71 +1,33 @@ package world.phantasmal.observable.cell -import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable -import world.phantasmal.core.unsafe.unsafeCast -import world.phantasmal.observable.Observer +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent -/** - * Starts observing its dependencies when the first observer on this cell is registered. Stops - * observing its dependencies when the last observer ov this cell is disposed. This way no extra - * disposables need to be managed when e.g. [map] is used. - */ -abstract class AbstractDependentCell( - private vararg val dependencies: Cell<*>, -) : AbstractCell() { - /** - * Is either empty or has a disposable per dependency. - */ - private val dependencyObservers = mutableListOf() +abstract class AbstractDependentCell : AbstractCell(), Dependent { + private var changingDependencies = 0 + private var dependenciesActuallyChanged = false - /** - * Set to true right before actual observers are added. - */ - protected var hasObservers = false + override fun dependencyMightChange() { + changingDependencies++ + emitMightChange() + } - protected var _value: T? = null - - override val value: T - get() { - if (!hasObservers) { - _value = computeValue() - } - - return _value.unsafeCast() + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (event != null) { + dependenciesActuallyChanged = true } - override fun observe(callNow: Boolean, observer: Observer): Disposable { - if (dependencyObservers.isEmpty()) { - hasObservers = true + if (--changingDependencies == 0) { + if (dependenciesActuallyChanged) { + dependenciesActuallyChanged = false - dependencies.forEach { dependency -> - dependencyObservers.add( - dependency.observe { - val oldValue = _value - _value = computeValue() - - if (_value != oldValue) { - emit() - } - } - ) - } - - _value = computeValue() - } - - val superDisposable = super.observe(callNow, observer) - - return disposable { - superDisposable.dispose() - - if (observers.isEmpty()) { - hasObservers = false - dependencyObservers.forEach { it.dispose() } - dependencyObservers.clear() + dependenciesChanged() + } else { + emitChanged(null) } } } - protected abstract fun computeValue(): T + abstract fun dependenciesChanged() } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/Cell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/Cell.kt index 51acad57..4fd280c1 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/Cell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/Cell.kt @@ -16,7 +16,7 @@ interface Cell : Observable { operator fun getValue(thisRef: Any?, property: KProperty<*>): T = value /** - * @param callNow Call [observer] immediately with the current [mutableCell]. + * @param callNow Call [observer] immediately with the current [value]. */ fun observe(callNow: Boolean = false, observer: Observer): Disposable diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellExtensions.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellExtensions.kt index 6106be22..03dc87da 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellExtensions.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellExtensions.kt @@ -59,3 +59,6 @@ fun Cell.isBlank(): Cell = fun Cell.isNotBlank(): Cell = map { it.isNotBlank() } + +fun Cell>.flatten(): Cell = + FlatteningDependentCell(this) { this.value } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt index fed1d1f1..36fa4bb5 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt @@ -1,5 +1,7 @@ package world.phantasmal.observable.cell +import world.phantasmal.observable.ChangeEvent + class DelegatingCell( private val getter: () -> T, private val setter: (T) -> Unit, @@ -10,8 +12,11 @@ class DelegatingCell( val oldValue = getter() if (value != oldValue) { + emitMightChange() + setter(value) - emit() + + emitChanged(ChangeEvent(value)) } } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt index c5248387..4c6bb395 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt @@ -1,11 +1,58 @@ package world.phantasmal.observable.cell +import world.phantasmal.core.unsafe.unsafeCast +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent + /** * Cell of which the value depends on 0 or more other cells. */ class DependentCell( - vararg dependencies: Cell<*>, - private val compute: () -> T, -) : AbstractDependentCell(*dependencies) { - override fun computeValue(): T = compute() + private vararg val dependencies: Dependency, + private val compute: () -> T +) : AbstractDependentCell() { + + private var _value: T? = null + override val value: T + get() { + if (dependents.isEmpty()) { + _value = compute() + } + + return unsafeCast(_value) + } + + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + _value = compute() + + for (dependency in dependencies) { + dependency.addDependent(this) + } + } + + super.addDependent(dependent) + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + for (dependency in dependencies) { + dependency.removeDependent(this) + } + } + } + + override fun dependenciesChanged() { + val newValue = compute() + + if (newValue != _value) { + _value = newValue + emitChanged(ChangeEvent(newValue)) + } else { + emitChanged(null) + } + } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt index d2b8c637..960e395e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt @@ -1,56 +1,90 @@ package world.phantasmal.observable.cell -import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable import world.phantasmal.core.unsafe.unsafeAssertNotNull -import world.phantasmal.observable.Observer +import world.phantasmal.core.unsafe.unsafeCast +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent /** * Similar to [DependentCell], except that this cell's [compute] returns a cell. */ class FlatteningDependentCell( - vararg dependencies: Cell<*>, - private val compute: () -> Cell, -) : AbstractDependentCell(*dependencies) { - private var computedCell: Cell? = null - private var computedCellObserver: Disposable? = null + private vararg val dependencies: Dependency, + private val compute: () -> Cell +) : AbstractDependentCell() { + private var computedCell: Cell? = null + private var computedInDeps = false + private var shouldRecompute = false + + private var _value: T? = null override val value: T get() { - return if (hasObservers) { - computedCell.unsafeAssertNotNull().value - } else { - super.value + if (dependents.isEmpty()) { + _value = compute().value + } + + return unsafeCast(_value) + } + + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + for (dependency in dependencies) { + dependency.addDependent(this) + } + + computedCell = compute().also { computedCell -> + computedCell.addDependent(this) + computedInDeps = dependencies.any { it === computedCell } + _value = computedCell.value } } - override fun observe(callNow: Boolean, observer: Observer): Disposable { - val superDisposable = super.observe(callNow, observer) + super.addDependent(dependent) + } - return disposable { - superDisposable.dispose() + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) - if (!hasObservers) { - computedCellObserver?.dispose() - computedCellObserver = null - computedCell = null + if (dependents.isEmpty()) { + computedCell?.removeDependent(this) + computedCell = null + computedInDeps = false + + for (dependency in dependencies) { + dependency.removeDependent(this) } } } - override fun computeValue(): T { - val computedCell = compute() - this.computedCell = computedCell - - computedCellObserver?.dispose() - - if (hasObservers) { - computedCellObserver = computedCell.observe { (value) -> - _value = value - emit() - } + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if ((dependency !== computedCell || computedInDeps) && event != null) { + shouldRecompute = true } - return computedCell.value + super.dependencyChanged(dependency, event) + } + + override fun dependenciesChanged() { + if (shouldRecompute) { + computedCell?.removeDependent(this) + + computedCell = compute().also { computedCell -> + computedCell.addDependent(this) + computedInDeps = dependencies.any { it === computedCell } + } + + shouldRecompute = false + } + + val newValue = unsafeAssertNotNull(computedCell).value + + if (newValue != _value) { + _value = newValue + emitChanged(ChangeEvent(newValue)) + } else { + emitChanged(null) + } } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt index 8b57f019..4d71655d 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt @@ -1,11 +1,16 @@ package world.phantasmal.observable.cell +import world.phantasmal.observable.ChangeEvent + class SimpleCell(value: T) : AbstractCell(), MutableCell { override var value: T = value set(value) { if (value != field) { + emitMightChange() + field = value - emit() + + emitChanged(ChangeEvent(value)) } } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/StaticCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/StaticCell.kt index 1a626828..8b64a27e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/StaticCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/StaticCell.kt @@ -2,10 +2,11 @@ package world.phantasmal.observable.cell import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.nopDisposable +import world.phantasmal.observable.AbstractDependency import world.phantasmal.observable.ChangeEvent import world.phantasmal.observable.Observer -class StaticCell(override val value: T) : Cell { +class StaticCell(override val value: T) : AbstractDependency(), Cell { override fun observe(callNow: Boolean, observer: Observer): Disposable { if (callNow) { observer(ChangeEvent(value)) diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt index d36b5261..7f66f7de 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractDependentListCell.kt @@ -1,131 +1,66 @@ package world.phantasmal.observable.cell.list import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable +import world.phantasmal.observable.CallbackObserver +import world.phantasmal.observable.Dependent import world.phantasmal.observable.Observer -import world.phantasmal.observable.cell.AbstractCell +import world.phantasmal.observable.cell.AbstractDependentCell import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.DependentCell +import world.phantasmal.observable.cell.not -/** - * Starts observing its dependencies when the first observer on this cell is registered. Stops - * observing its dependencies when the last observer on this cell is disposed. This way no extra - * disposables need to be managed when e.g. [map] is used. - */ -abstract class AbstractDependentListCell( - private vararg val dependencies: Cell<*>, -) : AbstractListCell(extractObservables = null) { - private val _size = SizeCell() - - /** - * Is either empty or has a disposable per dependency. - */ - private val dependencyObservers = mutableListOf() +abstract class AbstractDependentListCell : + AbstractDependentCell>(), + ListCell, + Dependent { protected abstract val elements: List - /** - * Set to true right before actual observers are added. - */ - protected var hasObservers = false - override val value: List get() { - if (!hasObservers) { + if (dependents.isEmpty()) { computeElements() } return elements } - override val size: Cell = _size + @Suppress("LeakingThis") + final override val size: Cell = DependentCell(this) { elements.size } - override fun observe(callNow: Boolean, observer: Observer>): Disposable { - initDependencyObservers() + final override val empty: Cell = size.map { it == 0 } - val superDisposable = super.observe(callNow, observer) + final override val notEmpty: Cell = !empty - return disposable { - superDisposable.dispose() - disposeDependencyObservers() - } - } + final override fun observe(callNow: Boolean, observer: Observer>): Disposable = + observeList(callNow, observer as ListObserver) override fun observeList(callNow: Boolean, observer: ListObserver): Disposable { - initDependencyObservers() + val observingCell = CallbackObserver(this, observer) - val superDisposable = super.observeList(callNow, observer) - - return disposable { - superDisposable.dispose() - disposeDependencyObservers() + if (callNow) { + observer( + ListChangeEvent( + value, + listOf( + ListChange.Structural(index = 0, removed = emptyList(), inserted = value), + ), + ) + ) } + + return observingCell + } + + final override fun dependenciesChanged() { + val oldElements = value + + computeElements() + + emitChanged( + ListChangeEvent(elements, listOf(ListChange.Structural(0, oldElements, elements))) + ) } protected abstract fun computeElements() - - protected open fun lastObserverRemoved() { - dependencyObservers.forEach { it.dispose() } - dependencyObservers.clear() - } - - private fun initDependencyObservers() { - if (dependencyObservers.isEmpty()) { - hasObservers = true - - dependencies.forEach { dependency -> - dependencyObservers.add( - dependency.observe { - val removed = elements - computeElements() - finalizeUpdate(ListChangeEvent.Change(0, removed, elements)) - } - ) - } - - computeElements() - } - } - - private fun disposeDependencyObservers() { - if (observers.isEmpty() && listObservers.isEmpty() && _size.publicObservers.isEmpty()) { - hasObservers = false - lastObserverRemoved() - } - } - - override fun finalizeUpdate(event: ListChangeEvent) { - if (event is ListChangeEvent.Change && event.removed.size != event.inserted.size) { - _size.publicEmit() - } - - super.finalizeUpdate(event) - } - - private inner class SizeCell : AbstractCell() { - override val value: Int - get() { - if (!hasObservers) { - computeElements() - } - - return elements.size - } - - val publicObservers = super.observers - - override fun observe(callNow: Boolean, observer: Observer): Disposable { - initDependencyObservers() - - val superDisposable = super.observe(callNow, observer) - - return disposable { - superDisposable.dispose() - disposeDependencyObservers() - } - } - - fun publicEmit() { - super.emit() - } - } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt index 0f34da4d..9dff2b33 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt @@ -1,140 +1,38 @@ package world.phantasmal.observable.cell.list import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable -import world.phantasmal.core.unsafe.unsafeAssertNotNull -import world.phantasmal.observable.ChangeEvent -import world.phantasmal.observable.Observable +import world.phantasmal.observable.CallbackObserver import world.phantasmal.observable.Observer import world.phantasmal.observable.cell.AbstractCell import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.DependentCell import world.phantasmal.observable.cell.not -abstract class AbstractListCell( - private val extractObservables: ObservablesExtractor?, -) : AbstractCell>(), ListCell { - /** - * Internal observers which observe observables related to this list's elements so that their - * changes can be propagated via ElementChange events. - */ - private val elementObservers = mutableListOf() +abstract class AbstractListCell : AbstractCell>(), ListCell { + @Suppress("LeakingThis") + final override val size: Cell = DependentCell(this) { value.size } - /** - * External list observers which are observing this list. - */ - protected val listObservers = mutableListOf>() + final override val empty: Cell = size.map { it == 0 } - override val empty: Cell by lazy { size.map { it == 0 } } + final override val notEmpty: Cell = !empty - override val notEmpty: Cell by lazy { !empty } - - override fun get(index: Int): E = - value[index] - - override fun observe(callNow: Boolean, observer: Observer>): Disposable { - if (elementObservers.isEmpty() && extractObservables != null) { - replaceElementObservers(0, elementObservers.size, value) - } - - observers.add(observer) - - if (callNow) { - observer(ChangeEvent(value)) - } - - return disposable { - observers.remove(observer) - disposeElementObserversIfNecessary() - } - } + final override fun observe(callNow: Boolean, observer: Observer>): Disposable = + observeList(callNow, observer as ListObserver) override fun observeList(callNow: Boolean, observer: ListObserver): Disposable { - if (elementObservers.isEmpty() && extractObservables != null) { - replaceElementObservers(0, elementObservers.size, value) - } - - listObservers.add(observer) + val observingCell = CallbackObserver(this, observer) if (callNow) { - observer(ListChangeEvent.Change(0, emptyList(), value)) - } - - return disposable { - listObservers.remove(observer) - disposeElementObserversIfNecessary() - } - } - - override fun firstOrNull(): Cell = - DependentCell(this) { value.firstOrNull() } - - /** - * Does the following in the given order: - * - Updates element observers - * - Emits ListChangeEvent - * - Emits ChangeEvent - */ - protected open fun finalizeUpdate(event: ListChangeEvent) { - if ( - (listObservers.isNotEmpty() || observers.isNotEmpty()) && - extractObservables != null && - event is ListChangeEvent.Change - ) { - replaceElementObservers(event.index, event.removed.size, event.inserted) - } - - listObservers.forEach { observer: ListObserver -> - observer(event) - } - - emit() - } - - private fun replaceElementObservers(from: Int, amountRemoved: Int, insertedElements: List) { - repeat(amountRemoved) { - elementObservers.removeAt(from).observers.forEach { it.dispose() } - } - - var index = from - - elementObservers.addAll( - from, - insertedElements.map { element -> - ElementObserver( - index++, - element, - extractObservables.unsafeAssertNotNull()(element) + observer( + ListChangeEvent( + value, + listOf( + ListChange.Structural(index = 0, removed = emptyList(), inserted = value), + ), ) - } - ) - - val shift = insertedElements.size - amountRemoved - - while (index < elementObservers.size) { - elementObservers[index++].index += shift + ) } - } - private fun disposeElementObserversIfNecessary() { - if (listObservers.isEmpty() && observers.isEmpty()) { - elementObservers.forEach { elementObserver: ElementObserver -> - elementObserver.observers.forEach { it.dispose() } - } - - elementObservers.clear() - } - } - - private inner class ElementObserver( - var index: Int, - element: E, - observables: Array>, - ) { - val observers: Array = Array(observables.size) { - observables[it].observe { - finalizeUpdate(ListChangeEvent.ElementChange(index, element)) - } - } + return observingCell } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DependentListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DependentListCell.kt index 648eb437..e6062646 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DependentListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DependentListCell.kt @@ -1,20 +1,42 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.core.unsafe.unsafeAssertNotNull +import world.phantasmal.observable.Dependent import world.phantasmal.observable.cell.Cell /** * ListCell of which the value depends on 0 or more other cells. */ class DependentListCell( - vararg dependencies: Cell<*>, + private vararg val dependencies: Cell<*>, private val computeElements: () -> List, -) : AbstractDependentListCell(*dependencies) { - private var _elements: List? = null +) : AbstractDependentListCell() { - override val elements: List get() = _elements.unsafeAssertNotNull() + override var elements: List = emptyList() + private set + + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + computeElements() + + for (dependency in dependencies) { + dependency.addDependent(this) + } + } + + super.addDependent(dependent) + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + for (dependency in dependencies) { + dependency.removeDependent(this) + } + } + } override fun computeElements() { - _elements = computeElements.invoke() + elements = computeElements.invoke() } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt index 31dda2c7..ab637839 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt @@ -1,28 +1,16 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable -import world.phantasmal.observable.Observer -import world.phantasmal.observable.cell.AbstractCell -import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent -// TODO: This class shares 95% of its code with AbstractDependentListCell. class FilteredListCell( private val dependency: ListCell, private val predicate: (E) -> Boolean, -) : AbstractListCell(extractObservables = null) { - private val _size = SizeCell() - - /** - * Set to true right before actual observers are added. - */ - private var hasObservers = false - - private var dependencyObserver: Disposable? = null - +) : AbstractListCell(), Dependent { /** * Maps the dependency's indices to this list's indices. When an element of the dependency list - * doesn't pass the predicate, it's index in this mapping is set to -1. + * doesn't pass the predicate, its index in this mapping is set to -1. */ private val indexMap = mutableListOf() @@ -30,66 +18,52 @@ class FilteredListCell( override val value: List get() { - if (!hasObservers) { + if (dependents.isEmpty()) { recompute() } return elements } - override val size: Cell = _size + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + dependency.addDependent(this) + recompute() + } - override fun observe(callNow: Boolean, observer: Observer>): Disposable { - initDependencyObservers() + super.addDependent(dependent) + } - val superDisposable = super.observe(callNow, observer) + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) - return disposable { - superDisposable.dispose() - disposeDependencyObservers() + if (dependents.isEmpty()) { + dependency.removeDependent(this) } } - override fun observeList(callNow: Boolean, observer: ListObserver): Disposable { - initDependencyObservers() - - val superDisposable = super.observeList(callNow, observer) - - return disposable { - superDisposable.dispose() - disposeDependencyObservers() - } + override fun dependencyMightChange() { + emitMightChange() } - private fun recompute() { - elements = ListWrapper(mutableListOf()) - indexMap.clear() + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (event is ListChangeEvent<*>) { + val filteredChanges = mutableListOf>() - dependency.value.forEach { element -> - if (predicate(element)) { - elements.mutate { add(element) } - indexMap.add(elements.lastIndex) - } else { - indexMap.add(-1) - } - } - } - - private fun initDependencyObservers() { - if (dependencyObserver == null) { - hasObservers = true - - dependencyObserver = dependency.observeList { event -> - when (event) { - is ListChangeEvent.Change -> { + for (change in event.changes) { + when (change) { + is ListChange.Structural -> { // Figure out which elements should be removed from this list, then simply // recompute the entire filtered list and finally figure out which elements - // have been added. Emit a Change event if something actually changed. + // have been added. Emit a change event if something actually changed. + @Suppress("UNCHECKED_CAST") + change as ListChange.Structural + val removed = mutableListOf() var eventIndex = -1 - event.removed.forEachIndexed { i, element -> - val index = indexMap[event.index + i] + change.removed.forEachIndexed { i, element -> + val index = indexMap[change.index + i] if (index != -1) { removed.add(element) @@ -104,8 +78,8 @@ class FilteredListCell( val inserted = mutableListOf() - event.inserted.forEachIndexed { i, element -> - val index = indexMap[event.index + i] + change.inserted.forEachIndexed { i, element -> + val index = indexMap[change.index + i] if (index != -1) { inserted.add(element) @@ -118,23 +92,31 @@ class FilteredListCell( if (removed.isNotEmpty() || inserted.isNotEmpty()) { check(eventIndex != -1) - finalizeUpdate(ListChangeEvent.Change(eventIndex, removed, inserted)) + filteredChanges.add( + ListChange.Structural( + eventIndex, + removed, + inserted + ) + ) } } - - is ListChangeEvent.ElementChange -> { - // Emit a Change or ElementChange event based on whether the updated element + is ListChange.Element -> { + // Emit a structural or element change based on whether the updated element // passes the predicate test and whether it was already in the elements list // (i.e. whether it passed the predicate test before the update). - val index = indexMap[event.index] + @Suppress("UNCHECKED_CAST") + change as ListChange.Element - if (predicate(event.updated)) { + val index = indexMap[change.index] + + if (predicate(change.updated)) { if (index == -1) { // If the element now passed the test and previously didn't pass, // insert it and emit a Change event. var insertIndex = elements.size - for (depIdx in (event.index + 1)..indexMap.lastIndex) { + for (depIdx in (change.index + 1)..indexMap.lastIndex) { val thisIdx = indexMap[depIdx] if (thisIdx != -1) { @@ -143,10 +125,10 @@ class FilteredListCell( } } - elements = elements.mutate { add(insertIndex, event.updated) } - indexMap[event.index] = insertIndex + elements = elements.mutate { add(insertIndex, change.updated) } + indexMap[change.index] = insertIndex - for (depIdx in (event.index + 1)..indexMap.lastIndex) { + for (depIdx in (change.index + 1)..indexMap.lastIndex) { val thisIdx = indexMap[depIdx] if (thisIdx != -1) { @@ -154,25 +136,25 @@ class FilteredListCell( } } - finalizeUpdate(ListChangeEvent.Change( - insertIndex, - emptyList(), - listOf(event.updated), - )) - } else { - // Otherwise just propagate the ElementChange event. - finalizeUpdate( - ListChangeEvent.ElementChange(index, event.updated) + filteredChanges.add( + ListChange.Structural( + insertIndex, + removed = emptyList(), + inserted = listOf(change.updated), + ) ) + } else { + // Otherwise just propagate the element change. + filteredChanges.add(ListChange.Element(index, change.updated)) } } else { if (index != -1) { // If the element now doesn't pass the test and it previously did - // pass, remove it and emit a Change event. + // pass, remove it and emit a structural change. elements = elements.mutate { removeAt(index) } - indexMap[event.index] = -1 + indexMap[change.index] = -1 - for (depIdx in (event.index + 1)..indexMap.lastIndex) { + for (depIdx in (change.index + 1)..indexMap.lastIndex) { val thisIdx = indexMap[depIdx] if (thisIdx != -1) { @@ -180,67 +162,45 @@ class FilteredListCell( } } - finalizeUpdate(ListChangeEvent.Change( - index, - listOf(event.updated), - emptyList(), - )) - } else { - // Otherwise just propagate the ElementChange event. - finalizeUpdate( - ListChangeEvent.ElementChange(index, event.updated) + filteredChanges.add( + ListChange.Structural( + index, + removed = listOf(change.updated), + inserted = emptyList(), + ) ) + } else { + // Otherwise just propagate the element change. + filteredChanges.add(ListChange.Element(index, change.updated)) } } } } } - recompute() - } - } - - private fun disposeDependencyObservers() { - if (observers.isEmpty() && listObservers.isEmpty() && _size.publicObservers.isEmpty()) { - hasObservers = false - dependencyObserver?.dispose() - dependencyObserver = null - } - } - - override fun finalizeUpdate(event: ListChangeEvent) { - if (event is ListChangeEvent.Change && event.removed.size != event.inserted.size) { - _size.publicEmit() - } - - super.finalizeUpdate(event) - } - - private inner class SizeCell : AbstractCell() { - override val value: Int - get() { - if (!hasObservers) { - recompute() - } - - return elements.size + if (filteredChanges.isEmpty()) { + emitChanged(null) + } else { + emitChanged(ListChangeEvent(elements, filteredChanges)) } + } else { + emitChanged(null) + } + } - val publicObservers = super.observers + private fun recompute() { + val newElements = mutableListOf() + indexMap.clear() - override fun observe(callNow: Boolean, observer: Observer): Disposable { - initDependencyObservers() - - val superDisposable = super.observe(callNow, observer) - - return disposable { - superDisposable.dispose() - disposeDependencyObservers() + dependency.value.forEach { element -> + if (predicate(element)) { + newElements.add(element) + indexMap.add(newElements.lastIndex) + } else { + indexMap.add(-1) } } - fun publicEmit() { - super.emit() - } + elements = ListWrapper(newElements) } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCell.kt index c8770ba0..54758186 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCell.kt @@ -1,38 +1,75 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.unsafe.unsafeAssertNotNull -import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent /** * Similar to [DependentListCell], except that this cell's [computeElements] returns a [ListCell]. */ class FlatteningDependentListCell( - vararg dependencies: Cell<*>, + private vararg val dependencies: Dependency, private val computeElements: () -> ListCell, -) : AbstractDependentListCell(*dependencies) { - private var computedCell: ListCell? = null - private var computedCellObserver: Disposable? = null +) : AbstractDependentListCell() { - override val elements: List get() = computedCell.unsafeAssertNotNull().value + private var computedCell: ListCell? = null + private var computedInDeps = false + private var shouldRecompute = false + + override var elements: List = emptyList() + private set + + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + for (dependency in dependencies) { + dependency.addDependent(this) + } + + computedCell = computeElements.invoke().also { computedCell -> + computedCell.addDependent(this) + computedInDeps = dependencies.any { it === computedCell } + elements = computedCell.value.toList() + } + } + + super.addDependent(dependent) + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + computedCell?.removeDependent(this) + computedCell = null + computedInDeps = false + + for (dependency in dependencies) { + dependency.removeDependent(this) + } + } + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if ((dependency !== computedCell || computedInDeps) && event != null) { + shouldRecompute = true + } + + super.dependencyChanged(dependency, event) + } override fun computeElements() { - computedCell = computeElements.invoke() + if (shouldRecompute || dependents.isEmpty()) { + computedCell?.removeDependent(this) - computedCellObserver?.dispose() - - computedCellObserver = - if (hasObservers) { - computedCell.unsafeAssertNotNull().observeList(observer = ::finalizeUpdate) - } else { - null + computedCell = computeElements.invoke().also { computedCell -> + computedCell.addDependent(this) + computedInDeps = dependencies.any { it === computedCell } } - } - override fun lastObserverRemoved() { - super.lastObserverRemoved() + shouldRecompute = false + } - computedCellObserver?.dispose() - computedCellObserver = null + elements = unsafeAssertNotNull(computedCell).value.toList() } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FoldedCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FoldedCell.kt deleted file mode 100644 index 5d3b252d..00000000 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FoldedCell.kt +++ /dev/null @@ -1,49 +0,0 @@ -package world.phantasmal.observable.cell.list - -import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable -import world.phantasmal.core.unsafe.unsafeCast -import world.phantasmal.observable.Observer -import world.phantasmal.observable.cell.AbstractCell - -class FoldedCell( - private val dependency: ListCell, - private val initial: R, - private val operation: (R, T) -> R, -) : AbstractCell() { - private var dependencyDisposable: Disposable? = null - private var _value: R? = null - - override val value: R - get() { - return if (dependencyDisposable == null) { - computeValue() - } else { - _value.unsafeCast() - } - } - - override fun observe(callNow: Boolean, observer: Observer): Disposable { - val superDisposable = super.observe(callNow, observer) - - if (dependencyDisposable == null) { - _value = computeValue() - - dependencyDisposable = dependency.observe { - _value = computeValue() - emit() - } - } - - return disposable { - superDisposable.dispose() - - if (observers.isEmpty()) { - dependencyDisposable?.dispose() - dependencyDisposable = null - } - } - } - - private fun computeValue(): R = dependency.value.fold(initial, operation) -} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCell.kt index 3145394f..ef06bfea 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCell.kt @@ -2,6 +2,7 @@ package world.phantasmal.observable.cell.list import world.phantasmal.core.disposable.Disposable import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.DependentCell interface ListCell : Cell> { /** @@ -16,23 +17,24 @@ interface ListCell : Cell> { val notEmpty: Cell - operator fun get(index: Int): E + operator fun get(index: Int): E = value[index] fun observeList(callNow: Boolean = false, observer: ListObserver): Disposable fun fold(initialValue: R, operation: (R, E) -> R): Cell = - FoldedCell(this, initialValue, operation) + DependentCell(this) { value.fold(initialValue, operation) } fun all(predicate: (E) -> Boolean): Cell = - fold(true) { acc, el -> acc && predicate(el) } + DependentCell(this) { value.all(predicate) } - fun sumBy(selector: (E) -> Int): Cell = - fold(0) { acc, el -> acc + selector(el) } + fun sumOf(selector: (E) -> Int): Cell = + DependentCell(this) { value.sumOf(selector) } fun filtered(predicate: (E) -> Boolean): ListCell = FilteredListCell(this, predicate) - fun firstOrNull(): Cell + fun firstOrNull(): Cell = + DependentCell(this) { value.firstOrNull() } operator fun contains(element: @UnsafeVariance E): Boolean = element in value } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellCreation.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellCreation.kt index e1f6a1f7..f4e678a2 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellCreation.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellCreation.kt @@ -10,8 +10,9 @@ fun emptyListCell(): ListCell = EMPTY_LIST_CELL fun mutableListCell( vararg elements: E, - extractObservables: ObservablesExtractor? = null, -): MutableListCell = SimpleListCell(mutableListOf(*elements), extractObservables) + extractDependencies: DependenciesExtractor? = null, +): MutableListCell = + SimpleListCell(mutableListOf(*elements), extractDependencies) fun flatMapToList( c1: Cell, diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt index 5f60c7f2..37584fd5 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListObserver.kt @@ -1,37 +1,42 @@ package world.phantasmal.observable.cell.list -sealed class ListChangeEvent { - abstract val index: Int +import world.phantasmal.observable.ChangeEvent +class ListChangeEvent( + value: List, + val changes: List>, +) : ChangeEvent>(value) + +sealed class ListChange { /** - * Represents a structural change to the list. E.g. an element is inserted or removed. + * Represents a structural change to a list cell. E.g. an element is inserted or removed. */ - class Change( - override val index: Int, + class Structural( + val index: Int, /** * The elements that were removed from the list at [index]. * - * Do not keep long-lived references to a [Change]'s [removed] list, it may or may not be - * mutated when the originating [ListCell] is mutated. + * Do not keep long-lived references to a [ChangeEvent]'s [removed] list, it may or may not + * be mutated when the originating [ListCell] is mutated. */ val removed: List, /** * The elements that were inserted into the list at [index]. * - * Do not keep long-lived references to a [Change]'s [inserted] list, it may or may not be - * mutated when the originating [ListCell] is mutated. + * Do not keep long-lived references to a [ChangeEvent]'s [inserted] list, it may or may not + * be mutated when the originating [ListCell] is mutated. */ val inserted: List, - ) : ListChangeEvent() + ) : ListChange() /** - * Represents a change to an element in the list. Will only be emitted if the list is configured - * to do so. + * Represents a change to an element in a list cell. Will only be emitted if the list is + * configured to do so. */ - class ElementChange( - override val index: Int, + class Element( + val index: Int, val updated: E, - ) : ListChangeEvent() + ) : ListChange() } -typealias ListObserver = (change: ListChangeEvent) -> Unit +typealias ListObserver = (ListChangeEvent) -> Unit diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/MutableListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/MutableListCell.kt index 7b78cfee..1e883391 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/MutableListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/MutableListCell.kt @@ -17,7 +17,7 @@ interface MutableListCell : ListCell, MutableCell> { fun replaceAll(elements: Sequence) - fun splice(from: Int, removeCount: Int, newElement: E) + fun splice(fromIndex: Int, removeCount: Int, newElement: E) fun clear() diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt index 9fe63b7a..3afadc9e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt @@ -1,23 +1,33 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.observable.Observable -import world.phantasmal.observable.cell.Cell -import world.phantasmal.observable.cell.MutableCell -import world.phantasmal.observable.cell.mutableCell +import world.phantasmal.core.disposable.Disposable +import world.phantasmal.core.unsafe.unsafeAssertNotNull +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent -typealias ObservablesExtractor = (element: E) -> Array> +typealias DependenciesExtractor = (element: E) -> Array /** - * @param elements The backing list for this [ListCell] - * @param extractObservables Extractor function called on each element in this list, changes to the - * returned observables will be propagated via ElementChange events + * @param elements The backing list for this [ListCell]. + * @param extractDependencies Extractor function called on each element in this list, changes to + * the returned dependencies will be propagated via [ListChange.Element]s in a [ListChangeEvent] + * event. */ class SimpleListCell( elements: MutableList, - extractObservables: ObservablesExtractor? = null, -) : AbstractListCell(extractObservables), MutableListCell { + private val extractDependencies: DependenciesExtractor? = null, +) : AbstractListCell(), MutableListCell { + private var elements = ListWrapper(elements) - private val _size: MutableCell = mutableCell(elements.size) + + /** + * Dependents of dependencies related to this list's elements. Allows us to propagate changes to + * elements via [ListChangeEvent]s. + */ + private val elementDependents = mutableListOf() + private var changingElements = 0 + private var elementListChanges = mutableListOf>() override var value: List get() = elements @@ -25,27 +35,47 @@ class SimpleListCell( replaceAll(value) } - override val size: Cell = _size - override operator fun get(index: Int): E = elements[index] override operator fun set(index: Int, element: E): E { + checkIndex(index, elements.lastIndex) + emitMightChange() + val removed: E elements = elements.mutate { removed = set(index, element) } - finalizeUpdate(ListChangeEvent.Change(index, listOf(removed), listOf(element))) + + if (extractDependencies != null) { + elementDependents[index].dispose() + elementDependents[index] = ElementDependent(index, element) + } + + emitChanged( + ListChangeEvent( + elements, + listOf(ListChange.Structural(index, listOf(removed), listOf(element))), + ), + ) + return removed } override fun add(element: E) { + emitMightChange() + val index = elements.size elements = elements.mutate { add(index, element) } - finalizeUpdate(ListChangeEvent.Change(index, emptyList(), listOf(element))) + + finalizeStructuralChange(index, emptyList(), listOf(element)) } override fun add(index: Int, element: E) { + checkIndex(index, elements.size) + emitMightChange() + elements = elements.mutate { add(index, element) } - finalizeUpdate(ListChangeEvent.Change(index, emptyList(), listOf(element))) + + finalizeStructuralChange(index, emptyList(), listOf(element)) } override fun remove(element: E): Boolean { @@ -60,46 +90,180 @@ class SimpleListCell( } override fun removeAt(index: Int): E { + checkIndex(index, elements.lastIndex) + emitMightChange() + val removed: E elements = elements.mutate { removed = removeAt(index) } - finalizeUpdate(ListChangeEvent.Change(index, listOf(removed), emptyList())) + + finalizeStructuralChange(index, listOf(removed), emptyList()) return removed } override fun replaceAll(elements: Iterable) { + emitMightChange() + val removed = this.elements this.elements = ListWrapper(elements.toMutableList()) - finalizeUpdate(ListChangeEvent.Change(0, removed, this.elements)) + + finalizeStructuralChange(0, removed, this.elements) } override fun replaceAll(elements: Sequence) { + emitMightChange() + val removed = this.elements this.elements = ListWrapper(elements.toMutableList()) - finalizeUpdate(ListChangeEvent.Change(0, removed, this.elements)) + + finalizeStructuralChange(0, removed, this.elements) } - override fun splice(from: Int, removeCount: Int, newElement: E) { - val removed = ArrayList(elements.subList(from, from + removeCount)) + override fun splice(fromIndex: Int, removeCount: Int, newElement: E) { + val removed = ArrayList(elements.subList(fromIndex, fromIndex + removeCount)) + + emitMightChange() + elements = elements.mutate { - repeat(removeCount) { removeAt(from) } - add(from, newElement) + repeat(removeCount) { removeAt(fromIndex) } + add(fromIndex, newElement) } - finalizeUpdate(ListChangeEvent.Change(from, removed, listOf(newElement))) + + finalizeStructuralChange(fromIndex, removed, listOf(newElement)) } override fun clear() { + emitMightChange() + val removed = elements elements = ListWrapper(mutableListOf()) - finalizeUpdate(ListChangeEvent.Change(0, removed, emptyList())) + + finalizeStructuralChange(0, removed, emptyList()) } override fun sortWith(comparator: Comparator) { - elements = elements.mutate { sortWith(comparator) } - finalizeUpdate(ListChangeEvent.Change(0, elements, elements)) + emitMightChange() + + var throwable: Throwable? = null + + try { + elements = elements.mutate { sortWith(comparator) } + } catch (e: Throwable) { + throwable = e + } + + finalizeStructuralChange(0, elements, elements) + + if (throwable != null) { + throw throwable + } } - override fun finalizeUpdate(event: ListChangeEvent) { - _size.value = elements.size - super.finalizeUpdate(event) + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty() && extractDependencies != null) { + for ((index, element) in elements.withIndex()) { + elementDependents.add(ElementDependent(index, element)) + } + } + + super.addDependent(dependent) + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + for (elementDependent in elementDependents) { + elementDependent.dispose() + } + + elementDependents.clear() + } + } + + private fun checkIndex(index: Int, maxIndex: Int) { + if (index !in 0..maxIndex) { + throw IndexOutOfBoundsException( + "Index $index out of bounds for length ${elements.size}", + ) + } + } + + private fun finalizeStructuralChange(index: Int, removed: List, inserted: List) { + if (extractDependencies != null) { + repeat(removed.size) { + elementDependents.removeAt(index).dispose() + } + + for ((i, element) in inserted.withIndex()) { + val elementIdx = index + i + elementDependents.add(elementIdx, ElementDependent(elementIdx, element)) + } + + val shift = inserted.size - removed.size + + for (i in (index + inserted.size)..elementDependents.lastIndex) { + elementDependents[i].index += shift + } + } + + emitChanged( + ListChangeEvent( + elements, + listOf(ListChange.Structural(index, removed, inserted)), + ), + ) + } + + private inner class ElementDependent( + var index: Int, + private val element: E, + ) : Dependent, Disposable { + private val dependencies = unsafeAssertNotNull(extractDependencies)(element) + private var changingDependencies = 0 + private var dependenciesActuallyChanged = false + + init { + for (dependency in dependencies) { + dependency.addDependent(this) + } + } + + override fun dispose() { + for (dependency in dependencies) { + dependency.removeDependent(this) + } + } + + override fun dependencyMightChange() { + if (changingDependencies++ == 0) { + changingElements++ + emitMightChange() + } + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (event != null) { + dependenciesActuallyChanged = true + } + + if (--changingDependencies == 0) { + if (dependenciesActuallyChanged) { + dependenciesActuallyChanged = false + elementListChanges.add(ListChange.Element(index, element)) + } + + if (--changingElements == 0) { + try { + if (elementListChanges.isNotEmpty()) { + emitChanged(ListChangeEvent(value, elementListChanges)) + } else { + emitChanged(null) + } + } finally { + elementListChanges = mutableListOf() + } + } + } + } } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/StaticListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/StaticListCell.kt index b96c6b8c..fd7d9259 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/StaticListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/StaticListCell.kt @@ -2,12 +2,14 @@ package world.phantasmal.observable.cell.list import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.nopDisposable +import world.phantasmal.core.unsafe.unsafeAssertNotNull +import world.phantasmal.observable.AbstractDependency import world.phantasmal.observable.ChangeEvent import world.phantasmal.observable.Observer import world.phantasmal.observable.cell.* -class StaticListCell(private val elements: List) : ListCell { - private val firstOrNull = StaticCell(elements.firstOrNull()) +class StaticListCell(private val elements: List) : AbstractDependency(), ListCell { + private var firstOrNull: Cell? = null override val size: Cell = cell(elements.size) override val empty: Cell = if (elements.isEmpty()) trueCell() else falseCell() @@ -30,11 +32,17 @@ class StaticListCell(private val elements: List) : ListCell { override fun observeList(callNow: Boolean, observer: ListObserver): Disposable { if (callNow) { - observer(ListChangeEvent.Change(0, emptyList(), value)) + observer(ListChangeEvent(value, listOf(ListChange.Structural(0, emptyList(), value)))) } return nopDisposable() } - override fun firstOrNull(): Cell = firstOrNull + override fun firstOrNull(): Cell { + if (firstOrNull == null) { + firstOrNull = StaticCell(elements.firstOrNull()) + } + + return unsafeAssertNotNull(firstOrNull) + } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/ObservableTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/ObservableTests.kt index a5ca3f61..d8d4b409 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/ObservableTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/ObservableTests.kt @@ -12,7 +12,7 @@ interface ObservableTests : ObservableTestSuite { fun createProvider(): Provider @Test - fun observable_calls_observers_when_events_are_emitted() = test { + fun calls_observers_when_events_are_emitted() = test { val p = createProvider() var changes = 0 @@ -34,7 +34,7 @@ interface ObservableTests : ObservableTestSuite { } @Test - fun observable_does_not_call_observers_after_they_are_disposed() = test { + fun does_not_call_observers_after_they_are_disposed() = test { val p = createProvider() var changes = 0 diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt index 65b739d7..eebf87f8 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt @@ -11,6 +11,27 @@ import kotlin.test.* interface CellTests : ObservableTests { override fun createProvider(): Provider + @Test + fun value_is_accessible_without_observers() = test { + val p = createProvider() + + assertNotNull(p.observable.value) + } + + @Test + fun value_is_accessible_with_observers() = test { + val p = createProvider() + + var observedValue: Any? = null + + disposer.add(p.observable.observe(callNow = true) { + observedValue = it.value + }) + + assertNotNull(observedValue) + assertNotNull(p.observable.value) + } + @Test fun propagates_changes_to_mapped_cell() = test { val p = createProvider() diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt new file mode 100644 index 00000000..9a1760d7 --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt @@ -0,0 +1,53 @@ +package world.phantasmal.observable.cell + +import world.phantasmal.observable.Dependent +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +interface CellWithDependenciesTests : CellTests { + override fun createProvider(): Provider + + @Test + fun is_recomputed_once_even_when_many_dependencies_change() = test { + val p = createProvider() + + val root = SimpleCell(5) + val branch1 = root.map { it * 2 } + val branch2 = root.map { it * 4 } + val leaf = p.createWithDependencies(branch1, branch2) + + var observedChanges = 0 + + disposer.add(leaf.observe { observedChanges++ }) + + // Change root, which results in both branches changing and thus two dependencies of leaf + // changing. + root.value = 7 + + assertEquals(1, observedChanges) + } + + @Test + fun doesnt_register_as_dependent_of_its_dependencies_until_it_has_dependents_itself() = test { + val p = createProvider() + + val dependency = object : AbstractCell() { + val publicDependents: List = dependents + + override val value: Int = 5 + } + + val cell = p.createWithDependencies(dependency) + + assertTrue(dependency.publicDependents.isEmpty()) + + disposer.add(cell.observe { }) + + assertEquals(1, dependency.publicDependents.size) + } + + interface Provider : CellTests.Provider { + fun createWithDependencies(vararg dependencies: Cell): Cell + } +} diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DelegatingCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DelegatingCellTests.kt index 14439071..1d37bbd1 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DelegatingCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DelegatingCellTests.kt @@ -2,7 +2,7 @@ package world.phantasmal.observable.cell class DelegatingCellTests : RegularCellTests, MutableCellTests { override fun createProvider() = object : MutableCellTests.Provider { - private var v = 0 + private var v = 17 override val observable = DelegatingCell({ v }, { v = it }) diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt index 3dd98d11..d4630c12 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt @@ -1,18 +1,23 @@ package world.phantasmal.observable.cell -class DependentCellTests : RegularCellTests { - override fun createProvider() = object : CellTests.Provider { - val dependency = SimpleCell(0) +class DependentCellTests : RegularCellTests, CellWithDependenciesTests { + override fun createProvider() = Provider() + + override fun createWithValue(value: T): DependentCell { + val dependency = SimpleCell(value) + return DependentCell(dependency) { dependency.value } + } + + class Provider : CellTests.Provider, CellWithDependenciesTests.Provider { + private val dependency = SimpleCell(1) override val observable = DependentCell(dependency) { 2 * dependency.value } override fun emit() { dependency.value += 2 } - } - override fun createWithValue(value: T): DependentCell { - val dependency = SimpleCell(value) - return DependentCell(dependency) { dependency.value } + override fun createWithDependencies(vararg dependencies: Cell) = + DependentCell(*dependencies) { dependencies.sumOf { it.value } } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt index 96f4b367..d401fcd5 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt @@ -3,13 +3,23 @@ package world.phantasmal.observable.cell /** * In these tests the dependency of the [FlatteningDependentCell]'s direct dependency changes. */ -class FlatteningDependentCellTransitiveDependencyEmitsTests : RegularCellTests { - override fun createProvider() = object : CellTests.Provider { +class FlatteningDependentCellTransitiveDependencyEmitsTests : + RegularCellTests, + CellWithDependenciesTests { + + override fun createProvider() = Provider() + + override fun createWithValue(value: T): FlatteningDependentCell { + val dependency = StaticCell(StaticCell(value)) + return FlatteningDependentCell(dependency) { dependency.value } + } + + class Provider : CellTests.Provider, CellWithDependenciesTests.Provider { // The transitive dependency can change. - val transitiveDependency = SimpleCell(5) + private val transitiveDependency = SimpleCell(5) // The direct dependency of the cell under test can't change. - val directDependency = StaticCell(transitiveDependency) + private val directDependency = StaticCell(transitiveDependency) override val observable = FlatteningDependentCell(directDependency) { directDependency.value } @@ -18,10 +28,8 @@ class FlatteningDependentCellTransitiveDependencyEmitsTests : RegularCellTests { // Update the transitive dependency. transitiveDependency.value += 5 } - } - override fun createWithValue(value: T): FlatteningDependentCell { - val dependency = StaticCell(StaticCell(value)) - return FlatteningDependentCell(dependency) { dependency.value } + override fun createWithDependencies(vararg dependencies: Cell): Cell = + FlatteningDependentCell(*dependencies) { StaticCell(dependencies.sumOf { it.value }) } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/RegularCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/RegularCellTests.kt index 505d7137..68fce6cb 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/RegularCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/RegularCellTests.kt @@ -32,8 +32,6 @@ interface RegularCellTests : CellTests { // Value should not change when emit hasn't been called since the last access. assertEquals(new, p.observable.value) - - old = new } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/DependentListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/DependentListCellTests.kt index 48f8212c..d06c89c5 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/DependentListCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/DependentListCellTests.kt @@ -1,13 +1,23 @@ package world.phantasmal.observable.cell.list -class DependentListCellTests : ListCellTests { - override fun createProvider() = object : ListCellTests.Provider { - private val dependency = SimpleListCell(mutableListOf()) +import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.CellWithDependenciesTests + +class DependentListCellTests : ListCellTests, CellWithDependenciesTests { + override fun createProvider() = createListProvider(empty = true) + + override fun createListProvider(empty: Boolean) = Provider(empty) + + class Provider(empty: Boolean) : ListCellTests.Provider, CellWithDependenciesTests.Provider { + private val dependency = SimpleListCell(if (empty) mutableListOf() else mutableListOf(5)) override val observable = DependentListCell(dependency) { dependency.value.map { 2 * it } } override fun addElement() { dependency.add(4) } + + override fun createWithDependencies(vararg dependencies: Cell): Cell = + DependentListCell(*dependencies) { dependencies.map { it.value } } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellTests.kt index 6af76f75..ddf398ae 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellTests.kt @@ -1,13 +1,12 @@ package world.phantasmal.observable.cell.list import world.phantasmal.observable.cell.SimpleCell -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertNull +import kotlin.test.* class FilteredListCellTests : ListCellTests { - override fun createProvider() = object : ListCellTests.Provider { - private val dependency = SimpleListCell(mutableListOf()) + override fun createListProvider(empty: Boolean) = object : ListCellTests.Provider { + private val dependency = + SimpleListCell(if (empty) mutableListOf(5) else mutableListOf(5, 10)) override val observable = FilteredListCell(dependency, predicate = { it % 2 == 0 }) @@ -82,37 +81,49 @@ class FilteredListCellTests : ListCellTests { dep.replaceAll(listOf(1, 2, 3, 4, 5)) - (event as ListChangeEvent.Change).let { e -> - assertEquals(0, e.index) - assertEquals(0, e.removed.size) - assertEquals(2, e.inserted.size) - assertEquals(2, e.inserted[0]) - assertEquals(4, e.inserted[1]) + run { + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertTrue(c is ListChange.Structural) + assertEquals(0, c.index) + assertEquals(0, c.removed.size) + assertEquals(2, c.inserted.size) + assertEquals(2, c.inserted[0]) + assertEquals(4, c.inserted[1]) } event = null dep.splice(2, 2, 10) - (event as ListChangeEvent.Change).let { e -> - assertEquals(1, e.index) - assertEquals(1, e.removed.size) - assertEquals(4, e.removed[0]) - assertEquals(1, e.inserted.size) - assertEquals(10, e.inserted[0]) + run { + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertTrue(c is ListChange.Structural) + assertEquals(1, c.index) + assertEquals(1, c.removed.size) + assertEquals(4, c.removed[0]) + assertEquals(1, c.inserted.size) + assertEquals(10, c.inserted[0]) } } /** - * When the dependency of a [FilteredListCell] emits ElementChange events, the - * [FilteredListCell] should emit either Change events or ElementChange events, depending on - * whether the predicate result has changed. + * When the dependency of a [FilteredListCell] emits [ListChange.Element] changes, the + * [FilteredListCell] should emit either [ListChange.Structural] or [ListChange.Element] + * changes, depending on whether the predicate result has changed. */ @Test - fun emits_correct_events_when_dependency_emits_ElementChange_events() = test { + fun emits_correct_events_when_dependency_emits_element_changes() = test { val dep = SimpleListCell( mutableListOf(SimpleCell(1), SimpleCell(2), SimpleCell(3), SimpleCell(4)), - extractObservables = { arrayOf(it) }, + extractDependencies = { arrayOf(it) }, ) val list = FilteredListCell(dep, predicate = { it.value % 2 == 0 }) var event: ListChangeEvent>? = null @@ -125,20 +136,25 @@ class FilteredListCellTests : ListCellTests { for (i in 0 until dep.size.value) { event = null - // Make an even number odd or an odd number even. List should emit a Change event. + // Make an even number odd or an odd number even. List should emit a structural change. val newValue = dep[i].value + 1 dep[i].value = newValue - (event as ListChangeEvent.Change).let { e -> - if (newValue % 2 == 0) { - assertEquals(0, e.removed.size) - assertEquals(1, e.inserted.size) - assertEquals(newValue, e.inserted[0].value) - } else { - assertEquals(1, e.removed.size) - assertEquals(0, e.inserted.size) - assertEquals(newValue, e.removed[0].value) - } + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertTrue(c is ListChange.Structural) + + if (newValue % 2 == 0) { + assertEquals(0, c.removed.size) + assertEquals(1, c.inserted.size) + assertEquals(newValue, c.inserted[0].value) + } else { + assertEquals(1, c.removed.size) + assertEquals(0, c.inserted.size) + assertEquals(newValue, c.removed[0].value) } } @@ -150,7 +166,14 @@ class FilteredListCellTests : ListCellTests { val newValue = dep[i].value + 2 dep[i].value = newValue - assertEquals(newValue, (event as ListChangeEvent.ElementChange).updated.value) + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertTrue(c is ListChange.Element) + + assertEquals(newValue, c.updated.value) } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellDirectDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellDirectDependencyEmitsTests.kt index ba65a6de..fe50a07b 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellDirectDependencyEmitsTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellDirectDependencyEmitsTests.kt @@ -6,9 +6,9 @@ import world.phantasmal.observable.cell.SimpleCell * In these tests the direct dependency of the [FlatteningDependentListCell] changes. */ class FlatteningDependentListCellDirectDependencyEmitsTests : ListCellTests { - override fun createProvider() = object : ListCellTests.Provider { + override fun createListProvider(empty: Boolean) = object : ListCellTests.Provider { // The transitive dependency can't change. - private val transitiveDependency = StaticListCell(emptyList()) + private val transitiveDependency = StaticListCell(if (empty) emptyList() else listOf(7)) // The direct dependency of the list under test can change. private val dependency = SimpleCell>(transitiveDependency) diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt index 501571fc..aebce5a6 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt @@ -1,14 +1,24 @@ package world.phantasmal.observable.cell.list +import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.CellWithDependenciesTests import world.phantasmal.observable.cell.StaticCell /** * In these tests the dependency of the [FlatteningDependentListCell]'s direct dependency changes. */ -class FlatteningDependentListCellTransitiveDependencyEmitsTests : ListCellTests { - override fun createProvider() = object : ListCellTests.Provider { +class FlatteningDependentListCellTransitiveDependencyEmitsTests : + ListCellTests, + CellWithDependenciesTests { + + override fun createProvider() = createListProvider(empty = true) + + override fun createListProvider(empty: Boolean) = Provider(empty) + + class Provider(empty: Boolean) : ListCellTests.Provider, CellWithDependenciesTests.Provider { // The transitive dependency can change. - private val transitiveDependency = SimpleListCell(mutableListOf()) + private val transitiveDependency = + SimpleListCell(if (empty) mutableListOf() else mutableListOf(7)) // The direct dependency of the list under test can't change. private val dependency = StaticCell>(transitiveDependency) @@ -20,5 +30,10 @@ class FlatteningDependentListCellTransitiveDependencyEmitsTests : ListCellTests // Update the transitive dependency. transitiveDependency.add(4) } + + override fun createWithDependencies(vararg dependencies: Cell): Cell = + FlatteningDependentListCell(*dependencies) { + StaticListCell(dependencies.map { it.value }) + } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListCellTests.kt index ead1085d..03092688 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListCellTests.kt @@ -8,7 +8,30 @@ import kotlin.test.* * [ListCell] implementation. */ interface ListCellTests : CellTests { - override fun createProvider(): Provider + override fun createProvider(): Provider = createListProvider(empty = true) + + fun createListProvider(empty: Boolean): Provider + + @Test + fun list_value_is_accessible_without_observers() = test { + val p = createListProvider(empty = false) + + assertTrue(p.observable.value.isNotEmpty()) + } + + @Test + fun list_value_is_accessible_with_observers() = test { + val p = createListProvider(empty = false) + + var observedValue: List<*>? = null + + disposer.add(p.observable.observe(callNow = true) { + observedValue = it.value + }) + + assertTrue(observedValue!!.isNotEmpty()) + assertTrue(p.observable.value.isNotEmpty()) + } @Test fun calls_list_observers_when_changed() = test { @@ -28,7 +51,7 @@ interface ListCellTests : CellTests { p.addElement() - assertTrue(event is ListChangeEvent.Change<*>) + assertNotNull(event) } } @@ -100,7 +123,7 @@ interface ListCellTests : CellTests { fun sumBy() = test { val p = createProvider() - val sum = p.observable.sumBy { 1 } + val sum = p.observable.sumOf { 1 } var observedValue: Int? = null diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/MutableListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/MutableListCellTests.kt index 8fc5cd4c..3e80e739 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/MutableListCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/MutableListCellTests.kt @@ -1,10 +1,7 @@ package world.phantasmal.observable.cell.list import world.phantasmal.observable.cell.MutableCellTests -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertNull -import kotlin.test.assertTrue +import kotlin.test.* /** * Test suite for all [MutableListCell] implementations. There is a subclass of this suite for every @@ -17,58 +14,79 @@ interface MutableListCellTests : ListCellTests, MutableCellTests? = null + var changeEvent: ListChangeEvent? = null disposer.add(p.observable.observeList { - assertNull(change) - change = it + assertNull(changeEvent) + changeEvent = it }) // Insert once. val v1 = p.createElement() p.observable.add(v1) - assertEquals(1, p.observable.size.value) - assertEquals(v1, p.observable[0]) - val c1 = change - assertTrue(c1 is ListChangeEvent.Change) - assertEquals(0, c1.index) - assertTrue(c1.removed.isEmpty()) - assertEquals(1, c1.inserted.size) - assertEquals(v1, c1.inserted[0]) + run { + assertEquals(1, p.observable.size.value) + assertEquals(v1, p.observable[0]) + + val e = changeEvent + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c0 = e.changes[0] + assertTrue(c0 is ListChange.Structural) + assertEquals(0, c0.index) + assertTrue(c0.removed.isEmpty()) + assertEquals(1, c0.inserted.size) + assertEquals(v1, c0.inserted[0]) + } // Insert a second time. - change = null + changeEvent = null val v2 = p.createElement() p.observable.add(v2) - assertEquals(2, p.observable.size.value) - assertEquals(v1, p.observable[0]) - assertEquals(v2, p.observable[1]) - val c2 = change - assertTrue(c2 is ListChangeEvent.Change) - assertEquals(1, c2.index) - assertTrue(c2.removed.isEmpty()) - assertEquals(1, c2.inserted.size) - assertEquals(v2, c2.inserted[0]) + run { + assertEquals(2, p.observable.size.value) + assertEquals(v1, p.observable[0]) + assertEquals(v2, p.observable[1]) + + val e = changeEvent + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c0 = e.changes[0] + assertTrue(c0 is ListChange.Structural) + assertEquals(1, c0.index) + assertTrue(c0.removed.isEmpty()) + assertEquals(1, c0.inserted.size) + assertEquals(v2, c0.inserted[0]) + } // Insert at index. - change = null + changeEvent = null val v3 = p.createElement() p.observable.add(1, v3) - assertEquals(3, p.observable.size.value) - assertEquals(v1, p.observable[0]) - assertEquals(v3, p.observable[1]) - assertEquals(v2, p.observable[2]) - val c3 = change - assertTrue(c3 is ListChangeEvent.Change) - assertEquals(1, c3.index) - assertTrue(c3.removed.isEmpty()) - assertEquals(1, c3.inserted.size) - assertEquals(v3, c3.inserted[0]) + run { + assertEquals(3, p.observable.size.value) + assertEquals(v1, p.observable[0]) + assertEquals(v3, p.observable[1]) + assertEquals(v2, p.observable[2]) + + val e = changeEvent + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c0 = e.changes[0] + assertTrue(c0 is ListChange.Structural) + assertEquals(1, c0.index) + assertTrue(c0.removed.isEmpty()) + assertEquals(1, c0.inserted.size) + assertEquals(v3, c0.inserted[0]) + } } interface Provider : ListCellTests.Provider, MutableCellTests.Provider> { diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleListCellTests.kt index eb825e8e..c69eefcf 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleListCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleListCellTests.kt @@ -1,13 +1,16 @@ package world.phantasmal.observable.cell.list -import kotlin.test.Test -import kotlin.test.assertEquals +import world.phantasmal.observable.cell.SimpleCell +import world.phantasmal.testUtils.TestContext +import kotlin.test.* class SimpleListCellTests : MutableListCellTests { - override fun createProvider() = object : MutableListCellTests.Provider { + override fun createProvider() = createListProvider(empty = true) + + override fun createListProvider(empty: Boolean) = object : MutableListCellTests.Provider { private var nextElement = 0 - override val observable = SimpleListCell(mutableListOf()) + override val observable = SimpleListCell(if (empty) mutableListOf() else mutableListOf(-13)) override fun addElement() { observable.add(createElement()) @@ -28,4 +31,147 @@ class SimpleListCellTests : MutableListCellTests { assertEquals(2, list[1]) assertEquals(3, list[2]) } + + @Test + fun add_with_index() = test { + val list = SimpleListCell(mutableListOf()) + + list.add(0, "b") + list.add(1, "c") + list.add(0, "a") + + assertEquals(3, list.size.value) + assertEquals("a", list[0]) + assertEquals("b", list[1]) + assertEquals("c", list[2]) + } + + @Test + fun element_changes_propagate_correctly_after_set() = test { + testElementChangePropagation { + val old = it[1] + it[1] = SimpleCell("new") + listOf(old) + } + } + + @Test + fun element_changes_propagate_correctly_after_add() = test { + testElementChangePropagation { + it.add(SimpleCell("new")) + emptyList() + } + } + + @Test + fun element_changes_propagate_correctly_after_add_with_index() = test { + testElementChangePropagation { + it.add(1, SimpleCell("new")) + emptyList() + } + } + + @Test + fun element_changes_propagate_correctly_after_remove() = test { + testElementChangePropagation { + val removed = it[1] + it.remove(removed) + listOf(removed) + } + } + + @Test + fun element_changes_propagate_correctly_after_removeAt() = test { + testElementChangePropagation { + listOf(it.removeAt(2)) + } + } + + @Test + fun element_changes_propagate_correctly_after_replaceAll() = test { + testElementChangePropagation { + val removed = it.value.toList() + it.replaceAll(listOf(SimpleCell("new a"), SimpleCell("new b"))) + removed + } + } + + @Test + fun element_changes_propagate_correctly_after_replaceAll_with_sequence() = test { + testElementChangePropagation { + val removed = it.value.toList() + it.replaceAll(sequenceOf(SimpleCell("new a"), SimpleCell("new b"))) + removed + } + } + + @Test + fun element_changes_propagate_correctly_after_splice() = test { + testElementChangePropagation { + val removed = it.value.toList().drop(1) + it.splice(1, 2, SimpleCell("new")) + removed + } + } + + @Test + fun element_changes_propagate_correctly_after_clear() = test { + testElementChangePropagation { + val removed = it.value.toList() + it.clear() + removed + } + } + + /** + * Creates a list with 3 SimpleCells as elements, calls [updateList] with this list and then + * checks that changes to old elements don't affect the list and changes to new elements do + * affect the list. + * + * @param updateList Function that changes the list and returns the removed elements if any. + */ + private fun TestContext.testElementChangePropagation( + updateList: (SimpleListCell>) -> List> + ) { + val list = SimpleListCell( + mutableListOf( + SimpleCell("a"), + SimpleCell("b"), + SimpleCell("c") + ) + ) { arrayOf(it) } + + var event: ListChangeEvent>? = null + + disposer.add(list.observeList { + assertNull(event) + event = it + }) + + val removed = updateList(list) + + event = null + + // The list should not emit events when an old element is changed. + for (element in removed) { + element.value += "-1" + + assertNull(event) + } + + // The list should emit events when any of the current elements are changed. + for ((index, element) in list.value.withIndex()) { + event = null + + element.value += "-2" + + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + val c = e.changes.first() + assertTrue(c is ListChange.Element) + assertEquals(index, c.index) + assertEquals(element, c.updated) + } + } } diff --git a/web/src/main/kotlin/world/phantasmal/web/core/undo/UndoManager.kt b/web/src/main/kotlin/world/phantasmal/web/core/undo/UndoManager.kt index 1dbc2395..c30df861 100644 --- a/web/src/main/kotlin/world/phantasmal/web/core/undo/UndoManager.kt +++ b/web/src/main/kotlin/world/phantasmal/web/core/undo/UndoManager.kt @@ -5,7 +5,7 @@ import world.phantasmal.observable.cell.list.mutableListCell import world.phantasmal.web.core.actions.Action class UndoManager { - private val undos = mutableListCell(NopUndo) { arrayOf(it.atSavePoint) } + private val undos = mutableListCell(NopUndo) private val _current = mutableCell(NopUndo) val current: Cell = _current @@ -19,7 +19,9 @@ class UndoManager { * True if all undos are at the most recent save point. I.e., true if there are no changes to * save. */ - val allAtSavePoint: Cell = undos.all { it.atSavePoint.value } + // TODO: Optimize this once ListCell supports more performant method for this use-case. + val allAtSavePoint: Cell = + undos.fold(trueCell()) { acc, undo -> acc and undo.atSavePoint }.flatten() fun addUndo(undo: Undo) { undos.add(undo) diff --git a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntOptimizerStore.kt b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntOptimizerStore.kt index 8920b76a..f65dc57c 100644 --- a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntOptimizerStore.kt +++ b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntOptimizerStore.kt @@ -100,7 +100,7 @@ class HuntOptimizerStore( val wantedItems = wantedItemPersister.loadWantedItems(server) withContext(Dispatchers.Main) { - _wantedItems.value = wantedItems + _wantedItems.replaceAll(wantedItems) } } } diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/QuestEditor.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/QuestEditor.kt index 91d0d0c1..52225461 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/QuestEditor.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/QuestEditor.kt @@ -45,21 +45,26 @@ class QuestEditor( // Stores val areaStore = addDisposable(AreaStore(areaAssetLoader)) - val questEditorStore = addDisposable(QuestEditorStore( - questLoader, - uiStore, - areaStore, - undoManager, - )) + val questEditorStore = addDisposable( + QuestEditorStore( + questLoader, + uiStore, + areaStore, + undoManager, + initializeNewQuest = true, + ) + ) val asmStore = addDisposable(AsmStore(questEditorStore, undoManager)) // Controllers val questEditorController = addDisposable(QuestEditorController(questEditorUiPersister)) - val toolbarController = addDisposable(QuestEditorToolbarController( - uiStore, - areaStore, - questEditorStore, - )) + val toolbarController = addDisposable( + QuestEditorToolbarController( + uiStore, + areaStore, + questEditorStore, + ) + ) val questInfoController = addDisposable(QuestInfoController(questEditorStore)) val npcCountsController = addDisposable(NpcCountsController(questEditorStore)) val entityInfoController = addDisposable(EntityInfoController(areaStore, questEditorStore)) @@ -70,12 +75,14 @@ class QuestEditor( val eventsController = addDisposable(EventsController(questEditorStore)) // Rendering - val renderer = addDisposable(QuestRenderer( - areaAssetLoader, - entityAssetLoader, - questEditorStore, - createThreeRenderer, - )) + val renderer = addDisposable( + QuestRenderer( + areaAssetLoader, + entityAssetLoader, + questEditorStore, + createThreeRenderer, + ) + ) val entityImageRenderer = addDisposable(EntityImageRenderer(entityAssetLoader, createThreeRenderer)) diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/loading/AreaAssetLoader.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/loading/AreaAssetLoader.kt index b7dea69f..341d3deb 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/loading/AreaAssetLoader.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/loading/AreaAssetLoader.kt @@ -53,8 +53,6 @@ class AreaAssetLoader(private val assetLoader: AssetLoader) : DisposableContaine addSectionsToCollisionGeometry(collisionObj3d, renderObj3d) -// cullRenderGeometry(collisionObj3d, renderObj3d) - Geom(sections, renderObj3d, collisionObj3d) }, { geom -> diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestMeshManager.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestMeshManager.kt index 5f0ce0e2..63bb9c6b 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestMeshManager.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestMeshManager.kt @@ -7,6 +7,7 @@ import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.DisposableSupervisedScope import world.phantasmal.lib.Episode import world.phantasmal.observable.cell.list.ListCell +import world.phantasmal.observable.cell.list.ListChange import world.phantasmal.observable.cell.list.ListChangeEvent import world.phantasmal.web.questEditor.loading.AreaAssetLoader import world.phantasmal.web.questEditor.loading.EntityAssetLoader @@ -68,17 +69,21 @@ abstract class QuestMeshManager protected constructor( } } - private fun npcsChanged(change: ListChangeEvent) { - if (change is ListChangeEvent.Change) { - change.removed.forEach(npcMeshManager::remove) - change.inserted.forEach(npcMeshManager::add) + private fun npcsChanged(event: ListChangeEvent) { + for (change in event.changes) { + if (change is ListChange.Structural) { + change.removed.forEach(npcMeshManager::remove) + change.inserted.forEach(npcMeshManager::add) + } } } - private fun objectsChanged(change: ListChangeEvent) { - if (change is ListChangeEvent.Change) { - change.removed.forEach(objectMeshManager::remove) - change.inserted.forEach(objectMeshManager::add) + private fun objectsChanged(event: ListChangeEvent) { + for (change in event.changes) { + if (change is ListChange.Structural) { + change.removed.forEach(objectMeshManager::remove) + change.inserted.forEach(objectMeshManager::add) + } } } } diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/stores/QuestEditorStore.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/stores/QuestEditorStore.kt index 2fdb5962..72be095e 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/stores/QuestEditorStore.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/stores/QuestEditorStore.kt @@ -22,6 +22,7 @@ class QuestEditorStore( uiStore: UiStore, private val areaStore: AreaStore, private val undoManager: UndoManager, + initializeNewQuest: Boolean, ) : Store() { private val _devMode = mutableCell(false) private val _currentQuest = mutableCell(null) @@ -102,7 +103,9 @@ class QuestEditorStore( } } - scope.launch { setCurrentQuest(getDefaultQuest(Episode.I)) } + if (initializeNewQuest) { + scope.launch { setCurrentQuest(getDefaultQuest(Episode.I)) } + } } override fun dispose() { diff --git a/web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt b/web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt new file mode 100644 index 00000000..a0025565 --- /dev/null +++ b/web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt @@ -0,0 +1,77 @@ +package world.phantasmal.web.questEditor.controllers + +import world.phantasmal.web.questEditor.models.QuestEventActionModel +import world.phantasmal.web.test.WebTestSuite +import world.phantasmal.web.test.createQuestModel +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class EventsControllerTests : WebTestSuite { + @Test + fun addEvent() = testAsync { + // Setup. + val store = components.questEditorStore + val quest = createQuestModel(mapDesignations = mapOf(1 to 0)) + store.setCurrentQuest(quest) + store.setCurrentArea(quest.areaVariants.value.first().area) + store.makeMainUndoCurrent() + + val ctrl = disposer.add(EventsController(store)) + + // Add an event. + ctrl.addEvent() + + assertEquals(1, quest.events.value.size) + + // Undo. + assertTrue(store.canUndo.value) + assertFalse(store.canRedo.value) + + store.undo() + + assertTrue(quest.events.value.isEmpty()) + + // Redo. + assertFalse(store.canUndo.value) + assertTrue(store.canRedo.value) + + store.redo() + + assertEquals(1, quest.events.value.size) + } + + @Test + fun addAction() = testAsync { + // Setup. + val store = components.questEditorStore + val quest = createQuestModel(mapDesignations = mapOf(1 to 0)) + store.setCurrentQuest(quest) + store.setCurrentArea(quest.areaVariants.value.first().area) + store.makeMainUndoCurrent() + + val ctrl = disposer.add(EventsController(store)) + + // Add an event and an action. + ctrl.addEvent() + val event = ctrl.events.value.first() + ctrl.addAction(event, QuestEventActionModel.Door.Unlock.SHORT_NAME) + + // Undo. + assertTrue(store.canUndo.value) + assertFalse(store.canRedo.value) + + store.undo() + + assertTrue(event.actions.value.isEmpty()) + + // Redo. + assertTrue(store.canUndo.value) // Can still undo event creation at this point. + assertTrue(store.canRedo.value) + + store.redo() + + assertEquals(1, event.actions.value.size) + } +} 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 56465daa..d8ef572f 100644 --- a/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt +++ b/web/src/test/kotlin/world/phantasmal/web/test/TestComponents.kt @@ -63,7 +63,7 @@ class TestComponents(private val ctx: TestContext) { var areaStore: AreaStore by default { AreaStore(areaAssetLoader) } var questEditorStore: QuestEditorStore by default { - QuestEditorStore(questLoader, uiStore, areaStore, undoManager) + QuestEditorStore(questLoader, uiStore, areaStore, undoManager, initializeNewQuest = false) } // Rendering diff --git a/web/src/test/kotlin/world/phantasmal/web/test/TestModels.kt b/web/src/test/kotlin/world/phantasmal/web/test/TestModels.kt index de4f8ffd..0d325fef 100644 --- a/web/src/test/kotlin/world/phantasmal/web/test/TestModels.kt +++ b/web/src/test/kotlin/world/phantasmal/web/test/TestModels.kt @@ -4,18 +4,21 @@ import world.phantasmal.lib.Episode import world.phantasmal.lib.asm.BytecodeIr import world.phantasmal.lib.fileFormats.quest.NpcType import world.phantasmal.lib.fileFormats.quest.QuestNpc +import world.phantasmal.web.questEditor.models.QuestEventModel import world.phantasmal.web.questEditor.models.QuestModel import world.phantasmal.web.questEditor.models.QuestNpcModel import world.phantasmal.web.questEditor.models.QuestObjectModel -fun createQuestModel( +fun WebTestContext.createQuestModel( id: Int = 1, name: String = "Test", shortDescription: String = name, longDescription: String = name, episode: Episode = Episode.I, + mapDesignations: Map = emptyMap(), npcs: List = emptyList(), objects: List = emptyList(), + events: List = emptyList(), bytecodeIr: BytecodeIr = BytecodeIr(emptyList()), ): QuestModel = QuestModel( @@ -25,14 +28,15 @@ fun createQuestModel( shortDescription, longDescription, episode, - emptyMap(), + mapDesignations, npcs.toMutableList(), objects.toMutableList(), - events = mutableListOf(), + events.toMutableList(), datUnknowns = emptyList(), bytecodeIr, UIntArray(0), - ) { _, _, _ -> null } + components.areaStore::getVariant, + ) fun createQuestNpcModel(type: NpcType, episode: Episode): QuestNpcModel = QuestNpcModel( diff --git a/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt b/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt index 172fa22c..cb2862d3 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt @@ -12,6 +12,7 @@ import world.phantasmal.core.disposable.Disposer import world.phantasmal.core.disposable.disposable import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.list.ListCell +import world.phantasmal.observable.cell.list.ListChange import world.phantasmal.observable.cell.list.ListChangeEvent fun EventTarget.disposableListener( @@ -255,26 +256,28 @@ private fun bindChildrenTo( childrenRemoved: (index: Int, count: Int) -> Unit, after: (ListChangeEvent) -> Unit, ): Disposable = - list.observeList(callNow = true) { change: ListChangeEvent -> - if (change is ListChangeEvent.Change) { - repeat(change.removed.size) { - parent.removeChild(parent.childNodes[change.index].unsafeCast()) - } + list.observeList(callNow = true) { event: ListChangeEvent -> + for (change in event.changes) { + if (change is ListChange.Structural) { + repeat(change.removed.size) { + parent.removeChild(parent.childNodes[change.index].unsafeCast()) + } - childrenRemoved(change.index, change.removed.size) + childrenRemoved(change.index, change.removed.size) - val frag = document.createDocumentFragment() + val frag = document.createDocumentFragment() - change.inserted.forEachIndexed { i, value -> - frag.appendChild(frag.createChild(value, change.index + i)) - } + change.inserted.forEachIndexed { i, value -> + frag.appendChild(frag.createChild(value, change.index + i)) + } - if (change.index >= parent.childNodes.length) { - parent.appendChild(frag) - } else { - parent.insertBefore(frag, parent.childNodes[change.index]) + if (change.index >= parent.childNodes.length) { + parent.appendChild(frag) + } else { + parent.insertBefore(frag, parent.childNodes[change.index]) + } } } - after(change) + after(event) } diff --git a/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt b/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt index 7975aaa6..ed67240b 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt @@ -1,10 +1,9 @@ package world.phantasmal.webui.dom import org.w3c.dom.HTMLElement -import world.phantasmal.core.disposable.Disposable -import world.phantasmal.core.disposable.disposable import world.phantasmal.core.unsafe.unsafeAssertNotNull -import world.phantasmal.observable.Observer +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependent import world.phantasmal.observable.cell.AbstractCell data class Size(val width: Double, val height: Double) @@ -12,14 +11,9 @@ data class Size(val width: Double, val height: Double) class HTMLElementSizeCell(element: HTMLElement? = null) : AbstractCell() { private var resizeObserver: dynamic = null - /** - * Set to true right before actual observers are added. - */ - private var hasObservers = false - private var _value: Size? = null - var element: HTMLElement? = null + var element: HTMLElement? = element set(element) { if (resizeObserver != null) { if (field != null) { @@ -34,24 +28,17 @@ class HTMLElementSizeCell(element: HTMLElement? = null) : AbstractCell() { field = element } - init { - // Ensure we call the setter with element. - this.element = element - } - override val value: Size get() { - if (!hasObservers) { + if (dependents.isEmpty()) { _value = getSize() } - return _value.unsafeAssertNotNull() + return unsafeAssertNotNull(_value) } - override fun observe(callNow: Boolean, observer: Observer): Disposable { - if (!hasObservers) { - hasObservers = true - + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { if (resizeObserver == null) { @Suppress("UNUSED_VARIABLE") val resize = ::resizeCallback @@ -65,15 +52,14 @@ class HTMLElementSizeCell(element: HTMLElement? = null) : AbstractCell() { _value = getSize() } - val superDisposable = super.observe(callNow, observer) + super.addDependent(dependent) + } - return disposable { - superDisposable.dispose() + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) - if (observers.isEmpty()) { - hasObservers = false - resizeObserver.disconnect() - } + if (dependents.isEmpty()) { + resizeObserver.disconnect() } } @@ -83,12 +69,16 @@ class HTMLElementSizeCell(element: HTMLElement? = null) : AbstractCell() { ?: Size(0.0, 0.0) private fun resizeCallback(entries: Array) { - entries.forEach { entry -> - _value = Size( - entry.contentRect.width.unsafeCast(), - entry.contentRect.height.unsafeCast() - ) - emit() + val entry = entries.first() + val newValue = Size( + entry.contentRect.width.unsafeCast(), + entry.contentRect.height.unsafeCast(), + ) + + if (newValue != _value) { + emitMightChange() + _value = newValue + emitChanged(ChangeEvent(newValue)) } } }