diff --git a/core/src/commonMain/kotlin/world/phantasmal/core/Assertions.kt b/core/src/commonMain/kotlin/world/phantasmal/core/Assertions.kt new file mode 100644 index 00000000..928234e9 --- /dev/null +++ b/core/src/commonMain/kotlin/world/phantasmal/core/Assertions.kt @@ -0,0 +1,7 @@ +package world.phantasmal.core + +inline fun assert(value: () -> Boolean) { + assert(value) { "An assertion failed." } +} + +expect inline fun assert(value: () -> Boolean, lazyMessage: () -> Any) diff --git a/core/src/jsMain/kotlin/world/phantasmal/core/Assertions.kt b/core/src/jsMain/kotlin/world/phantasmal/core/Assertions.kt new file mode 100644 index 00000000..958c1d98 --- /dev/null +++ b/core/src/jsMain/kotlin/world/phantasmal/core/Assertions.kt @@ -0,0 +1,5 @@ +package world.phantasmal.core + +actual inline fun assert(value: () -> Boolean, lazyMessage: () -> Any) { + // TODO: Figure out a sensible way to do dev assertions in JS. +} diff --git a/core/src/jvmMain/kotlin/world/phantasmal/core/Assertions.kt b/core/src/jvmMain/kotlin/world/phantasmal/core/Assertions.kt new file mode 100644 index 00000000..a8670852 --- /dev/null +++ b/core/src/jvmMain/kotlin/world/phantasmal/core/Assertions.kt @@ -0,0 +1,11 @@ +@file:JvmName("AssertionsJvm") + +package world.phantasmal.core + +val ASSERTIONS_ENABLED: Boolean = {}.javaClass.desiredAssertionStatus() + +actual inline fun assert(value: () -> Boolean, lazyMessage: () -> Any) { + if (ASSERTIONS_ENABLED && !value()) { + throw AssertionError(lazyMessage()) + } +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeObserver.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeObserver.kt index 58c06706..fdfae8f3 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeObserver.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeObserver.kt @@ -3,9 +3,7 @@ package world.phantasmal.observable typealias ChangeObserver = (ChangeEvent) -> Unit open class ChangeEvent( - /** - * The observable's new value. - */ + /** The observable's new value. */ val value: T, ) { operator fun component1() = value diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt index 9c8943c0..4292a81e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/Dependent.kt @@ -8,7 +8,10 @@ interface Dependent { * 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. + * 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() 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 132aeff4..9ba19d8e 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractCell.kt @@ -22,7 +22,7 @@ abstract class AbstractCell : AbstractDependency(), Cell { } } - protected fun emitDependencyChanged(event: ChangeEvent<*>?) { + protected fun emitDependencyChangedEvent(event: ChangeEvent<*>?) { if (mightChangeEmitted) { mightChangeEmitted = false @@ -31,4 +31,6 @@ abstract class AbstractCell : AbstractDependency(), Cell { } } } + + override fun toString(): String = "${this::class.simpleName}[$value]" } 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 014697f1..9d005ffd 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt @@ -18,13 +18,15 @@ abstract class AbstractDependentCell : AbstractCell(), Dependent { dependenciesActuallyChanged = true } - if (--changingDependencies == 0) { + changingDependencies-- + + if (changingDependencies == 0) { if (dependenciesActuallyChanged) { dependenciesActuallyChanged = false - dependenciesChanged() + dependenciesFinishedChanging() } else { - emitDependencyChanged(null) + emitDependencyChangedEvent(null) } } } @@ -35,5 +37,9 @@ abstract class AbstractDependentCell : AbstractCell(), Dependent { // change set is being completed. } - protected abstract fun dependenciesChanged() + /** + * Called after a wave of dependencyMightChange notifications followed by an equal amount of + * dependencyChanged notifications of which at least one signified an actual change. + */ + protected abstract fun dependenciesFinishedChanging() } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellUtils.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellUtils.kt index 2668fc33..109d793b 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellUtils.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/CellUtils.kt @@ -193,18 +193,24 @@ infix fun > Cell.lt(value: T): Cell = infix fun > Cell.lt(other: Cell): Cell = map(this, other) { a, b -> a < b } -fun and(vararg cells: Cell): Cell = - DependentCell(*cells) { cells.all { it.value } } - infix fun Cell.and(other: Cell): Cell = map(this, other) { a, b -> a && b } infix fun Cell.and(other: Boolean): Cell = if (other) this else falseCell() +infix fun Boolean.and(other: Cell): Cell = + if (this) other else falseCell() + infix fun Cell.or(other: Cell): Cell = map(this, other) { a, b -> a || b } +infix fun Cell.or(other: Boolean): Cell = + if (other) trueCell() else this + +infix fun Boolean.or(other: Cell): Cell = + if (this) trueCell() else other + infix fun Cell.xor(other: Cell): Cell = // Use != because of https://youtrack.jetbrains.com/issue/KT-31277. map(this, other) { a, b -> a != b } 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 88a6966a..b0b44610 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DelegatingCell.kt @@ -22,6 +22,6 @@ class DelegatingCell( } override fun emitDependencyChanged() { - emitDependencyChanged(ChangeEvent(value)) + emitDependencyChangedEvent(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 d5a2b07a..976fb601 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/DependentCell.kt @@ -6,16 +6,19 @@ import world.phantasmal.observable.Dependency import world.phantasmal.observable.Dependent /** - * Cell of which the value depends on 0 or more other cells. + * Cell of which the value depends on 0 or more dependencies. */ class DependentCell( private vararg val dependencies: Dependency, - private val compute: () -> T + private val compute: () -> T, ) : AbstractDependentCell() { private var _value: T? = null override val value: T get() { + // Recompute value every time when we have no dependents. At this point we're not yet a + // dependent of our own dependencies, and thus we won't automatically recompute our + // value when they change. if (dependents.isEmpty()) { _value = compute() } @@ -25,6 +28,9 @@ class DependentCell( override fun addDependent(dependent: Dependent) { if (dependents.isEmpty()) { + // Start actually depending on or dependencies when we get our first dependent. + // Make sure value is up-to-date here, because from now on `compute` will only be called + // when our dependencies change. _value = compute() for (dependency in dependencies) { @@ -39,20 +45,16 @@ class DependentCell( super.removeDependent(dependent) if (dependents.isEmpty()) { + // Stop actually depending on our dependencies when we no longer have any dependents. for (dependency in dependencies) { dependency.removeDependent(this) } } } - override fun dependenciesChanged() { + override fun dependenciesFinishedChanging() { val newValue = compute() - - if (newValue != _value) { - _value = newValue - emitDependencyChanged(ChangeEvent(newValue)) - } else { - emitDependencyChanged(null) - } + _value = newValue + emitDependencyChangedEvent(ChangeEvent(newValue)) } } 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 3c618cc4..cae0858b 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/FlatteningDependentCell.kt @@ -11,7 +11,7 @@ import world.phantasmal.observable.Dependent */ class FlatteningDependentCell( private vararg val dependencies: Dependency, - private val compute: () -> Cell + private val compute: () -> Cell, ) : AbstractDependentCell() { private var computedCell: Cell? = null @@ -66,7 +66,7 @@ class FlatteningDependentCell( super.dependencyChanged(dependency, event) } - override fun dependenciesChanged() { + override fun dependenciesFinishedChanging() { if (shouldRecompute) { computedCell?.removeDependent(this) @@ -79,12 +79,7 @@ class FlatteningDependentCell( } val newValue = unsafeAssertNotNull(computedCell).value - - if (newValue != _value) { - _value = newValue - emitDependencyChanged(ChangeEvent(newValue)) - } else { - emitDependencyChanged(null) - } + _value = newValue + emitDependencyChangedEvent(ChangeEvent(newValue)) } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/ImmutableCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/ImmutableCell.kt index 5045621c..34d76eb7 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/ImmutableCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/ImmutableCell.kt @@ -2,10 +2,19 @@ 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.ChangeObserver +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent + +class ImmutableCell(override val value: T) : Dependency, Cell { + override fun addDependent(dependent: Dependent) { + // We don't remember our dependents because we never need to notify them of changes. + } + + override fun removeDependent(dependent: Dependent) { + // Nothing to remove because we don't remember our dependents. + } -class ImmutableCell(override val value: T) : AbstractDependency(), Cell { override fun observeChange(observer: ChangeObserver): Disposable = nopDisposable() override fun emitDependencyChanged() { 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 20ba550e..a9e863b3 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/SimpleCell.kt @@ -16,6 +16,6 @@ class SimpleCell(value: T) : AbstractCell(), MutableCell { } override fun emitDependencyChanged() { - emitDependencyChanged(ChangeEvent(value)) + emitDependencyChangedEvent(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 00fcc0dd..5f2198db 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 @@ -61,15 +61,15 @@ abstract class AbstractDependentListCell : override fun observeListChange(observer: ListChangeObserver): Disposable = CallbackChangeObserver(this, observer) - final override fun dependenciesChanged() { + final override fun dependenciesFinishedChanging() { val oldElements = value computeElements() - emitDependencyChanged( + emitDependencyChangedEvent( ListChangeEvent( elements, - listOf(ListChange.Structural( + listOf(ListChange( index = 0, prevSize = oldElements.size, removed = oldElements, diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCell.kt new file mode 100644 index 00000000..24027c64 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCell.kt @@ -0,0 +1,218 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.core.unsafe.unsafeCast +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent + +abstract class AbstractFilteredListCell( + protected val list: ListCell, +) : AbstractListCell(), Dependent { + + /** Keeps track of number of changing dependencies during a change wave. */ + private var changingDependencies = 0 + + /** Set during a change wave when [list] changes. */ + private var listChangeEvent: ListChangeEvent? = null + + /** Set during a change wave when [predicateDependency] changes. */ + private var predicateChanged = false + + override val elements = mutableListOf() + + protected abstract val predicateDependency: Dependency + + override val value: List + get() { + if (dependents.isEmpty()) { + recompute() + } + + return elementsWrapper + } + + override fun addDependent(dependent: Dependent) { + val wasEmpty = dependents.isEmpty() + + super.addDependent(dependent) + + if (wasEmpty) { + list.addDependent(this) + predicateDependency.addDependent(this) + recompute() + } + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + predicateDependency.removeDependent(this) + list.removeDependent(this) + } + } + + override fun dependencyMightChange() { + changingDependencies++ + emitMightChange() + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (dependency === list) { + listChangeEvent = unsafeCast(event) + } else if (dependency === predicateDependency) { + predicateChanged = event != null + } else if (event != null) { + otherDependencyChanged(dependency) + } + + changingDependencies-- + + if (changingDependencies == 0) { + if (predicateChanged) { + // Simply assume the entire list changes and recompute. + val removed = elementsWrapper + + ignoreOtherChanges() + recompute() + + emitDependencyChangedEvent( + ListChangeEvent( + elementsWrapper, + listOf(ListChange(0, removed.size, removed, elementsWrapper)), + ) + ) + } else { + // TODO: Conditionally copyAndResetWrapper? + copyAndResetWrapper() + val filteredChanges = mutableListOf>() + + val listChangeEvent = this.listChangeEvent + + if (listChangeEvent != null) { + for (change in listChangeEvent.changes) { + val prevSize = elements.size + // Map the incoming change index to an index into our own elements list. + // TODO: Avoid this loop by storing the index where an element "would" be + // if it passed the predicate. + var eventIndex = prevSize + + for (index in change.index..maxDepIndex()) { + val i = mapIndex(index) + + if (i != -1) { + eventIndex = i + break + } + } + + // Process removals. + val removed = mutableListOf() + + for (element in change.removed) { + val index = removeIndexMapping(change.index) + + if (index != -1) { + elements.removeAt(eventIndex) + removed.add(element) + } + } + + // Process insertions. + val inserted = mutableListOf() + var insertionIndex = eventIndex + + for ((i, element) in change.inserted.withIndex()) { + if (applyPredicate(element)) { + insertIndexMapping(change.index + i, insertionIndex, element) + elements.add(insertionIndex, element) + inserted.add(element) + insertionIndex++ + } else { + insertIndexMapping(change.index + i, -1, element) + } + } + + // Shift mapped indices by a certain amount. This amount can be positive, + // negative or zero. + val diff = inserted.size - removed.size + + if (diff != 0) { + // Indices before the change index stay the same. Newly inserted indices + // are already correct. So we only need to shift everything after the + // new indices. + for (index in (change.index + change.inserted.size)..maxDepIndex()) { + shiftIndexMapping(index, diff) + } + } + + // Add a list change if something actually changed. + if (removed.isNotEmpty() || inserted.isNotEmpty()) { + filteredChanges.add( + ListChange( + eventIndex, + prevSize, + removed, + inserted, + ) + ) + } + } + } + + processOtherChanges(filteredChanges) + + if (filteredChanges.isEmpty()) { + emitDependencyChangedEvent(null) + } else { + emitDependencyChangedEvent( + ListChangeEvent(elementsWrapper, filteredChanges) + ) + } + } + + // Reset for next change wave. + listChangeEvent = null + predicateChanged = false + resetChangeWaveData() + } + } + + /** Called when a dependency that's neither [list] nor [predicateDependency] has changed. */ + protected abstract fun otherDependencyChanged(dependency: Dependency) + + protected abstract fun ignoreOtherChanges() + + protected abstract fun processOtherChanges(filteredChanges: MutableList>) + + override fun emitDependencyChanged() { + // Nothing to do because AbstractFilteredListCell emits dependencyChanged immediately. We + // don't defer this operation because AbstractFilteredListCell only changes when there is no + // change set or the current change set is being completed. + } + + protected abstract fun applyPredicate(element: E): Boolean + + protected abstract fun maxDepIndex(): Int + + /** + * Maps and index into [list] to an index into this list. Returns -1 if the given index does not + * point to an element that passes the predicate, i.e. the element is not in this list. + */ + protected abstract fun mapIndex(index: Int): Int + + /** + * Removes the element at the given index into [list] from our mapping. Returns the previous + * index into our list. + */ + protected abstract fun removeIndexMapping(index: Int): Int + + protected abstract fun insertIndexMapping(depIndex: Int, localIndex: Int, element: E) + + /** Adds [shift] to the local index at [depIndex] if it's not -1. */ + protected abstract fun shiftIndexMapping(depIndex: Int, shift: Int) + + protected abstract fun recompute() + + protected abstract fun resetChangeWaveData() +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DelegatingList.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DelegatingList.kt index 2f99899f..0aa7c136 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DelegatingList.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DelegatingList.kt @@ -27,8 +27,7 @@ class DelegatingList(var backingList: List) : List { override fun subList(fromIndex: Int, toIndex: Int): List = backingList.subList(fromIndex, toIndex) - @Suppress("SuspiciousEqualsCombination") - override fun equals(other: Any?): Boolean = this === other || other == backingList + override fun equals(other: Any?): Boolean = other == backingList override fun hashCode(): Int = backingList.hashCode() 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 a2c2c1fc..971fa114 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,217 +1,212 @@ package world.phantasmal.observable.cell.list +import world.phantasmal.core.assert +import world.phantasmal.core.unsafe.unsafeCast import world.phantasmal.observable.ChangeEvent import world.phantasmal.observable.Dependency import world.phantasmal.observable.Dependent +import world.phantasmal.observable.cell.Cell class FilteredListCell( - private val dependency: ListCell, - private val predicate: (E) -> Boolean, -) : AbstractListCell(), Dependent { + list: ListCell, + private val predicate: Cell<(E) -> Cell>, +) : AbstractFilteredListCell(list) { /** - * Maps the dependency's indices to this list's indices. When an element of the dependency list + * Maps the dependency's indices to the corresponding index into this list and the result of the + * predicate applied to the element at that index. When an element of the dependency list * doesn't pass the predicate, its index in this mapping is set to -1. */ - private val indexMap = mutableListOf() + private val indexMap = mutableListOf() - override val elements = mutableListOf() + private val changedPredicateResults = mutableListOf() - override val value: List - get() { - if (dependents.isEmpty()) { - recompute() - } - - return elementsWrapper - } - - override fun addDependent(dependent: Dependent) { - if (dependents.isEmpty()) { - dependency.addDependent(this) - recompute() - } - - super.addDependent(dependent) - } + override val predicateDependency: Dependency + get() = predicate override fun removeDependent(dependent: Dependent) { super.removeDependent(dependent) if (dependents.isEmpty()) { - dependency.removeDependent(this) + for (mapping in indexMap) { + mapping.removeDependent(this) + } } } - override fun dependencyMightChange() { - emitMightChange() + override fun otherDependencyChanged(dependency: Dependency) { + assert { dependency is FilteredListCell<*>.Mapping } + + changedPredicateResults.add(unsafeCast(dependency)) } - override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { - if (event is ListChangeEvent<*>) { - val prevSize = elements.size - val filteredChanges = mutableListOf>() + override fun ignoreOtherChanges() { + changedPredicateResults.clear() + } - 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. - @Suppress("UNCHECKED_CAST") - change as ListChange.Structural + override fun processOtherChanges(filteredChanges: MutableList>) { + var shift = 0 - val removed = mutableListOf() - var eventIndex = -1 + for ((dependencyIndex, mapping) in indexMap.withIndex()) { + if (changedPredicateResults.isEmpty()) { + break + } - change.removed.forEachIndexed { i, element -> - val index = indexMap[change.index + i] + if (changedPredicateResults.remove(mapping)) { + val result = mapping.predicateResult.value + val oldResult = mapping.index != -1 - if (index != -1) { - removed.add(element) + if (result != oldResult) { + val prevSize = elements.size - if (eventIndex == -1) { - eventIndex = index - } + if (result) { + // TODO: Avoid this loop by storing the index where an element "would" be + // if it passed the predicate. + var insertionIndex = elements.size + + for (index in (dependencyIndex + 1)..indexMap.lastIndex) { + val i = indexMap[index].index + + if (i != -1) { + insertionIndex = i + shift + break } } - recompute() + val element = list.value[dependencyIndex] + elements.add(insertionIndex, element) + mapping.index = insertionIndex + shift++ - val inserted = mutableListOf() + filteredChanges.add(ListChange( + insertionIndex, + prevSize, + removed = emptyList(), + inserted = listOf(element), + )) + } else { + val index = mapping.index + shift + val element = elements.removeAt(index) + mapping.index = -1 + shift-- - change.inserted.forEachIndexed { i, element -> - val index = indexMap[change.index + i] - - if (index != -1) { - inserted.add(element) - - if (eventIndex == -1) { - eventIndex = index - } - } - } - - if (removed.isNotEmpty() || inserted.isNotEmpty()) { - check(eventIndex != -1) - filteredChanges.add( - ListChange.Structural( - eventIndex, - prevSize, - removed, - inserted - ) - ) - } - } - 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). - @Suppress("UNCHECKED_CAST") - change as ListChange.Element - - 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 (change.index + 1)..indexMap.lastIndex) { - val thisIdx = indexMap[depIdx] - - if (thisIdx != -1) { - insertIndex = thisIdx - break - } - } - - copyAndResetWrapper() - elements.add(insertIndex, change.updated) - indexMap[change.index] = insertIndex - - for (depIdx in (change.index + 1)..indexMap.lastIndex) { - val thisIdx = indexMap[depIdx] - - if (thisIdx != -1) { - indexMap[depIdx]++ - } - } - - filteredChanges.add( - ListChange.Structural( - insertIndex, - prevSize, - 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 structural change. - copyAndResetWrapper() - elements.removeAt(index) - indexMap[change.index] = -1 - - for (depIdx in (change.index + 1)..indexMap.lastIndex) { - val thisIdx = indexMap[depIdx] - - if (thisIdx != -1) { - indexMap[depIdx]-- - } - } - - filteredChanges.add( - ListChange.Structural( - index, - prevSize, - removed = listOf(change.updated), - inserted = emptyList(), - ) - ) - } else { - // Otherwise just propagate the element change. - filteredChanges.add(ListChange.Element(index, change.updated)) - } - } + filteredChanges.add(ListChange( + index, + prevSize, + removed = listOf(element), + inserted = emptyList(), + )) } + } else if (oldResult) { + mapping.index += shift } - } - - if (filteredChanges.isEmpty()) { - emitDependencyChanged(null) } else { - emitDependencyChanged(ListChangeEvent(elementsWrapper, filteredChanges)) + mapping.index += shift } - } else { - emitDependencyChanged(null) + } + + // Can still contain changed mappings at this point if e.g. an element was removed after its + // predicate result changed. + changedPredicateResults.clear() + } + + override fun applyPredicate(element: E): Boolean = + predicate.value(element).value + + override fun maxDepIndex(): Int = + indexMap.lastIndex + + override fun mapIndex(index: Int): Int = + indexMap[index].index + + override fun removeIndexMapping(index: Int): Int { + val mapping = indexMap.removeAt(index) + mapping.removeDependent(this) + return mapping.index + } + + override fun insertIndexMapping(depIndex: Int, localIndex: Int, element: E) { + val mapping = Mapping(predicate.value(element), localIndex) + mapping.addDependent(this) + indexMap.add(depIndex, mapping) + } + + override fun shiftIndexMapping(depIndex: Int, shift: Int) { + val mapping = indexMap[depIndex] + + if (mapping.index != -1) { + mapping.index += shift } } - override fun emitDependencyChanged() { - // Nothing to do because FilteredListCell emits dependencyChanged immediately. We don't - // defer this operation because FilteredListCell only changes when there is no transaction - // or the current transaction is being committed. - } - - private fun recompute() { + override fun recompute() { copyAndResetWrapper() elements.clear() + + for (mapping in indexMap) { + mapping.removeDependent(this) + } + indexMap.clear() - dependency.value.forEach { element -> - if (predicate(element)) { - elements.add(element) - indexMap.add(elements.lastIndex) - } else { - indexMap.add(-1) + // Cache value here to facilitate loop unswitching. + val hasDependents = dependents.isNotEmpty() + val pred = predicate.value + + for (element in list.value) { + val predicateResult = pred(element) + + val index = + if (predicateResult.value) { + elements.add(element) + elements.lastIndex + } else { + -1 + } + + if (hasDependents) { + val mapping = Mapping(predicateResult, index) + mapping.addDependent(this) + indexMap.add(mapping) } } } + + override fun resetChangeWaveData() { + changedPredicateResults.clear() + } + + private inner class Mapping( + val predicateResult: Cell, + /** + * The index into [elements] if the element passes the predicate. -1 If the element does not + * pass the predicate. + */ + var index: Int, + ) : Dependent, Dependency { + override fun dependencyMightChange() { + this@FilteredListCell.dependencyMightChange() + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + assert { dependency === predicateResult } + + this@FilteredListCell.dependencyChanged(this, event) + } + + override fun addDependent(dependent: Dependent) { + assert { dependent === this@FilteredListCell } + + predicateResult.addDependent(this) + } + + override fun removeDependent(dependent: Dependent) { + assert { dependent === this@FilteredListCell } + + predicateResult.removeDependent(this) + } + + override fun emitDependencyChanged() { + // Nothing to do. + } + } } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt index b26400d2..154e89ec 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ImmutableListCell.kt @@ -2,20 +2,29 @@ package world.phantasmal.observable.cell.list import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.nopDisposable -import world.phantasmal.observable.AbstractDependency import world.phantasmal.observable.ChangeObserver +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.cell import world.phantasmal.observable.cell.falseCell import world.phantasmal.observable.cell.trueCell -class ImmutableListCell(private val elements: List) : AbstractDependency(), ListCell { +class ImmutableListCell(private val elements: List) : Dependency, ListCell { override val size: Cell = cell(elements.size) override val empty: Cell = if (elements.isEmpty()) trueCell() else falseCell() override val notEmpty: Cell = if (elements.isNotEmpty()) trueCell() else falseCell() override val value: List = elements + override fun addDependent(dependent: Dependent) { + // We don't remember our dependents because we never need to notify them of changes. + } + + override fun removeDependent(dependent: Dependent) { + // Nothing to remove because we don't remember our dependents. + } + override fun get(index: Int): E = elements[index] diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellUtils.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellUtils.kt index 31d5b792..d74ade41 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellUtils.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListCellUtils.kt @@ -1,7 +1,9 @@ package world.phantasmal.observable.cell.list +import world.phantasmal.observable.Observable import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.DependentCell +import world.phantasmal.observable.cell.ImmutableCell private val EMPTY_LIST_CELL = ImmutableListCell(emptyList()) @@ -12,11 +14,20 @@ fun listCell(vararg elements: E): ListCell = ImmutableListCell(elements.t fun emptyListCell(): ListCell = EMPTY_LIST_CELL /** Returns a mutable list cell containing [elements]. */ -fun mutableListCell( - vararg elements: E, - extractDependencies: DependenciesExtractor? = null, -): MutableListCell = - SimpleListCell(mutableListOf(*elements), extractDependencies) +fun mutableListCell(vararg elements: E): MutableListCell = + SimpleListCell(mutableListOf(*elements)) + +/** + * Returns a cell that changes whenever this list cell is structurally changed or when its + * individual elements change. + * + * @param extractObservables Called on each element to determine which element changes should be + * observed. + */ +fun ListCell.dependingOnElements( + extractObservables: (element: E) -> Array>, +): Cell> = + ListElementsDependentCell(this, extractObservables) fun ListCell.listMap(transform: (E) -> R): ListCell = DependentListCell(this) { value.map(transform) } @@ -31,13 +42,16 @@ fun ListCell.sumOf(selector: (E) -> Int): Cell = DependentCell(this) { value.sumOf(selector) } fun ListCell.filtered(predicate: (E) -> Boolean): ListCell = - FilteredListCell(this, predicate) + SimpleFilteredListCell(this, ImmutableCell(predicate)) fun ListCell.filtered(predicate: Cell<(E) -> Boolean>): ListCell = - DependentListCell(this, predicate) { value.filter(predicate.value) } + SimpleFilteredListCell(this, predicate) -fun ListCell.sortedWith(comparator: Cell>): ListCell = - DependentListCell(this, comparator) { value.sortedWith(comparator.value) } +fun ListCell.filteredCell(predicate: (E) -> Cell): ListCell = + FilteredListCell(this, ImmutableCell(predicate)) + +fun ListCell.filteredCell(predicate: Cell<(E) -> Cell>): ListCell = + FilteredListCell(this, predicate) fun ListCell.firstOrNull(): Cell = DependentCell(this) { value.firstOrNull() } diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListChangeObserver.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListChangeObserver.kt index 83985b2b..f47b3db2 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListChangeObserver.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListChangeObserver.kt @@ -7,39 +7,19 @@ class ListChangeEvent( val changes: List>, ) : ChangeEvent>(value) -sealed class ListChange { - /** - * Represents a structural change to a list cell. E.g. an element is inserted or removed. - */ - class Structural( - val index: Int, - val prevSize: Int, - /** - * The elements that were removed from the list at [index]. - * - * 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 [ChangeEvent]'s [inserted] list, it may or may not - * be mutated when the originating [ListCell] is mutated. - */ - val inserted: List, - ) : ListChange() { - val allRemoved: Boolean get() = removed.size == prevSize - } - - /** - * Represents a change to an element in a list cell. Will only be emitted if the list is - * configured to do so. - */ - class Element( - val index: Int, - val updated: E, - ) : ListChange() +/** + * Represents a structural change to a list cell. E.g. an element is inserted or removed. + */ +class ListChange( + val index: Int, + val prevSize: Int, + /** The elements that were removed from the list at [index]. */ + val removed: List, + /** The elements that were inserted into the list at [index]. */ + val inserted: List, +) { + /** True when this change resulted in the removal of all elements from the list. */ + val allRemoved: Boolean get() = removed.size == prevSize } typealias ListChangeObserver = (ListChangeEvent) -> Unit diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCell.kt new file mode 100644 index 00000000..c59ad59f --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCell.kt @@ -0,0 +1,140 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.core.splice +import world.phantasmal.core.unsafe.unsafeCast +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.Dependent +import world.phantasmal.observable.Observable +import world.phantasmal.observable.cell.AbstractCell + +/** + * Depends on a [ListCell] and zero or more [Observable]s per element in the list. + */ +class ListElementsDependentCell( + private val list: ListCell, + private val extractObservables: (element: E) -> Array>, +) : AbstractCell>(), Dependent { + /** An array of dependencies per [list] element, extracted by [extractObservables]. */ + private val elementDependencies = mutableListOf>() + + /** Keeps track of how many of our dependencies are about to (maybe) change. */ + private var changingDependencies = 0 + + /** + * Set to true once one of our dependencies has actually changed. Reset to false whenever + * [changingDependencies] hits 0 again. + */ + private var dependenciesActuallyChanged = false + + private var listChangeEvent: ListChangeEvent? = null + + override val value: List + get() = list.value + + override fun addDependent(dependent: Dependent) { + if (dependents.isEmpty()) { + // Once we have our first dependent, we start depending on our own dependencies. + list.addDependent(this) + + for (element in list.value) { + val dependencies = extractObservables(element) + + for (dependency in dependencies) { + dependency.addDependent(this) + } + + elementDependencies.add(dependencies) + } + } + + super.addDependent(dependent) + } + + override fun removeDependent(dependent: Dependent) { + super.removeDependent(dependent) + + if (dependents.isEmpty()) { + // At this point we have no more dependents, so we can stop depending on our own + // dependencies. + for (dependencies in elementDependencies) { + for (dependency in dependencies) { + dependency.removeDependent(this) + } + } + + elementDependencies.clear() + list.removeDependent(this) + } + } + + override fun dependencyMightChange() { + changingDependencies++ + emitMightChange() + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + if (event != null) { + dependenciesActuallyChanged = true + + // Simply store all list changes when the changing dependency is our list dependency. We + // don't update our dependencies yet to avoid receiving dependencyChanged notifications + // from newly inserted dependencies for which we haven't received any + // dependencyMightChange notifications and to avoid *NOT* receiving dependencyChanged + // notifications from removed dependencies for which we *HAVE* received + // dependencyMightChange notifications. + if (dependency === list) { + listChangeEvent = unsafeCast(event) + } + } + + changingDependencies-- + + if (changingDependencies == 0) { + // All of our dependencies have finished changing. + + // At this point we can remove this dependent from the removed elements' dependencies + // and add it to the newly inserted elements' dependencies. + listChangeEvent?.let { listChangeEvent -> + for (change in listChangeEvent.changes) { + for (i in change.index until (change.index + change.removed.size)) { + for (elementDependency in elementDependencies[i]) { + elementDependency.removeDependent(this) + } + } + + val inserted = change.inserted.map(extractObservables) + + elementDependencies.splice( + startIndex = change.index, + amount = change.removed.size, + elements = inserted, + ) + + for (elementDependencies in inserted) { + for (elementDependency in elementDependencies) { + elementDependency.addDependent(this) + } + } + } + } + + // Reset for the next change wave. + listChangeEvent = null + + if (dependenciesActuallyChanged) { + dependenciesActuallyChanged = false + + emitDependencyChangedEvent(ChangeEvent(list.value)) + } else { + emitDependencyChangedEvent(null) + } + } + } + + override fun emitDependencyChanged() { + // Nothing to do because ListElementsDependentCell emits dependencyChanged immediately. We + // don't defer this operation because ListElementsDependentCell only changes when there is + // no transaction or the current transaction is being committed. + } +} diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCell.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCell.kt new file mode 100644 index 00000000..c0963854 --- /dev/null +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCell.kt @@ -0,0 +1,80 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.Dependency +import world.phantasmal.observable.cell.Cell + +class SimpleFilteredListCell( + list: ListCell, + private val predicate: Cell<(E) -> Boolean>, +) : AbstractFilteredListCell(list) { + /** + * Maps the dependency's indices to this list's indices. When an element of the dependency list + * doesn't pass the predicate, its index in this mapping is set to -1. + * + * This is not a performance improvement but a requirement. We can't determine an element's + * index into our own [elements] list by using e.g. indexOf because a list can contain the same + * element multiple times. + */ + private val indexMap = mutableListOf() + + override val predicateDependency: Dependency + get() = predicate + + override fun otherDependencyChanged(dependency: Dependency) { + // Unreachable code path. + error("Unexpected dependency.") + } + + override fun ignoreOtherChanges() { + // Nothing to ignore. + } + + override fun processOtherChanges(filteredChanges: MutableList>) { + // Nothing to process. + } + + override fun applyPredicate(element: E): Boolean = + predicate.value(element) + + override fun maxDepIndex(): Int = + indexMap.lastIndex + + override fun mapIndex(index: Int): Int = + indexMap[index] + + override fun removeIndexMapping(index: Int): Int = + indexMap.removeAt(index) + + override fun insertIndexMapping(depIndex: Int, localIndex: Int, element: E) { + indexMap.add(depIndex, localIndex) + } + + override fun shiftIndexMapping(depIndex: Int, shift: Int) { + val i = indexMap[depIndex] + + if (i != -1) { + indexMap[depIndex] = i + shift + } + } + + override fun recompute() { + copyAndResetWrapper() + elements.clear() + indexMap.clear() + + val pred = predicate.value + + for (element in list.value) { + if (pred(element)) { + elements.add(element) + indexMap.add(elements.lastIndex) + } else { + indexMap.add(-1) + } + } + } + + override fun resetChangeWaveData() { + // Nothing to reset. + } +} 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 85b974be..e04af30c 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,32 +1,15 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.replaceAll -import world.phantasmal.core.unsafe.unsafeAssertNotNull -import world.phantasmal.observable.ChangeEvent import world.phantasmal.observable.ChangeManager -import world.phantasmal.observable.Dependency -import world.phantasmal.observable.Dependent - -typealias DependenciesExtractor = (element: E) -> Array /** * @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( override val elements: MutableList, - private val extractDependencies: DependenciesExtractor? = null, ) : AbstractListCell(), MutableListCell { - /** - * 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 changes = mutableListOf>() override var value: List @@ -45,18 +28,12 @@ class SimpleListCell( copyAndResetWrapper() val removed = elements.set(index, element) - if (dependents.isNotEmpty() && extractDependencies != null) { - elementDependents[index].dispose() - elementDependents[index] = ElementDependent(index, element) - } - - changes.add(ListChange.Structural( + finalizeChange( index, prevSize = elements.size, removed = listOf(removed), inserted = listOf(element), - )) - ChangeManager.changed(this) + ) return removed } @@ -68,7 +45,7 @@ class SimpleListCell( copyAndResetWrapper() elements.add(element) - finalizeStructuralChange( + finalizeChange( index, prevSize = index, removed = emptyList(), @@ -84,7 +61,7 @@ class SimpleListCell( copyAndResetWrapper() elements.add(index, element) - finalizeStructuralChange(index, prevSize, removed = emptyList(), inserted = listOf(element)) + finalizeChange(index, prevSize, removed = emptyList(), inserted = listOf(element)) } override fun remove(element: E): Boolean { @@ -107,7 +84,7 @@ class SimpleListCell( copyAndResetWrapper() val removed = elements.removeAt(index) - finalizeStructuralChange(index, prevSize, removed = listOf(removed), inserted = emptyList()) + finalizeChange(index, prevSize, removed = listOf(removed), inserted = emptyList()) return removed } @@ -120,7 +97,7 @@ class SimpleListCell( copyAndResetWrapper() this.elements.replaceAll(elements) - finalizeStructuralChange(index = 0, prevSize, removed, inserted = elementsWrapper) + finalizeChange(index = 0, prevSize, removed, inserted = elementsWrapper) } override fun replaceAll(elements: Sequence) { @@ -132,7 +109,7 @@ class SimpleListCell( copyAndResetWrapper() this.elements.replaceAll(elements) - finalizeStructuralChange(index = 0, prevSize, removed, inserted = elementsWrapper) + finalizeChange(index = 0, prevSize, removed, inserted = elementsWrapper) } override fun splice(fromIndex: Int, removeCount: Int, newElement: E) { @@ -149,7 +126,7 @@ class SimpleListCell( repeat(removeCount) { elements.removeAt(fromIndex) } elements.add(fromIndex, newElement) - finalizeStructuralChange(fromIndex, prevSize, removed, inserted = listOf(newElement)) + finalizeChange(fromIndex, prevSize, removed, inserted = listOf(newElement)) } override fun clear() { @@ -161,7 +138,7 @@ class SimpleListCell( copyAndResetWrapper() elements.clear() - finalizeStructuralChange(index = 0, prevSize, removed, inserted = emptyList()) + finalizeChange(index = 0, prevSize, removed, inserted = emptyList()) } override fun sortWith(comparator: Comparator) { @@ -177,7 +154,7 @@ class SimpleListCell( throwable = e } - finalizeStructuralChange( + finalizeChange( index = 0, prevSize = elements.size, removed, @@ -189,32 +166,10 @@ class SimpleListCell( } } - 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() - } - } - override fun emitDependencyChanged() { val currentChanges = changes changes = mutableListOf() - emitDependencyChanged(ListChangeEvent(elementsWrapper, currentChanges)) + emitDependencyChangedEvent(ListChangeEvent(elementsWrapper, currentChanges)) } private fun checkIndex(index: Int, maxIndex: Int) { @@ -225,75 +180,13 @@ class SimpleListCell( } } - private fun finalizeStructuralChange( + private fun finalizeChange( index: Int, prevSize: Int, removed: List, inserted: List, ) { - if (dependents.isNotEmpty() && 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 - } - } - - changes.add(ListChange.Structural(index, prevSize, removed, inserted)) + changes.add(ListChange(index, prevSize, removed, inserted)) ChangeManager.changed(this) } - - 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 - changes.add(ListChange.Element(index, element)) - } - - if (--changingElements == 0) { - ChangeManager.changed(this@SimpleListCell) - } - } - } - } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/DependencyTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/DependencyTests.kt index fcd1c993..733b7d59 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/DependencyTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/DependencyTests.kt @@ -42,6 +42,10 @@ interface DependencyTests : ObservableTestSuite { interface Provider { val dependency: Dependency + /** + * Makes [dependency] emit [Dependent.dependencyMightChange] followed by + * [Dependent.dependencyChanged] with a non-null event. + */ fun emit() } } 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 3d8dc3b8..125c0e2d 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt @@ -51,12 +51,12 @@ interface CellTests : ObservableTests { fun emits_correct_value_in_change_events() = test { val p = createProvider() - var prevValue: Any? - var observedValue: Any? = null + var prevValue: Snapshot? + var observedValue: Snapshot? = null disposer.add(p.observable.observeChange { changeEvent -> assertNull(observedValue) - observedValue = changeEvent.value + observedValue = changeEvent.value.snapshot() }) repeat(3) { @@ -69,7 +69,7 @@ interface CellTests : ObservableTests { // it should be equal to the cell's current value. assertNotNull(observedValue) assertNotEquals(prevValue, observedValue) - assertEquals(p.observable.value, observedValue) + assertEquals(p.observable.value.snapshot(), observedValue) } } @@ -82,20 +82,20 @@ interface CellTests : ObservableTests { fun reflects_changes_without_observers() = test { val p = createProvider() - var old: Any? + var old: Snapshot? repeat(5) { // Value should change after emit. - old = p.observable.value + old = p.observable.value.snapshot() p.emit() - val new = p.observable.value + val new = p.observable.value.snapshot() assertNotEquals(old, new) // Value should not change when emit hasn't been called since the last access. - assertEquals(new, p.observable.value) + assertEquals(new, p.observable.value.snapshot()) } } @@ -120,10 +120,10 @@ interface CellTests : ObservableTests { @Test fun propagates_changes_to_mapped_cell() = test { val p = createProvider() - val mapped = p.observable.map { it.hashCode() } + val mapped = p.observable.map { it.snapshot() } val initialValue = mapped.value - var observedValue: Int? = null + var observedValue: Snapshot? = null disposer.add(mapped.observeChange { changeEvent -> assertNull(observedValue) @@ -140,10 +140,10 @@ interface CellTests : ObservableTests { fun propagates_changes_to_flat_mapped_cell() = test { val p = createProvider() - val mapped = p.observable.flatMap { ImmutableCell(it.hashCode()) } + val mapped = p.observable.flatMap { ImmutableCell(it.snapshot()) } val initialValue = mapped.value - var observedValue: Int? = null + var observedValue: Snapshot? = null disposer.add(mapped.observeChange { assertNull(observedValue) @@ -160,3 +160,16 @@ interface CellTests : ObservableTests { override val observable: Cell } } + +/** See [snapshot]. */ +private typealias Snapshot = String + +/** + * We use toString to create "snapshots" of values throughout the tests. Most of the time cells will + * actually have a new value after emitting a change event, but this is not always the case with + * more complex cells or cells that point to complex values. So instead of keeping references to + * values and comparing them with == (or using e.g. assertEquals), we compare snapshots. + * + * This of course assumes that all values have sensible toString implementations. + */ +private fun Any?.snapshot(): Snapshot = toString() diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt index 8efa6df4..c74cd3dc 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellWithDependenciesTests.kt @@ -1,21 +1,61 @@ package world.phantasmal.observable.cell +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.Dependency import world.phantasmal.observable.Dependent -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertTrue +import kotlin.test.* interface CellWithDependenciesTests : CellTests { - override fun createProvider(): Provider + fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ): Cell + + @Test + fun emits_precisely_once_when_all_of_its_dependencies_emit() = test { + val root = SimpleCell(5) + val branch1 = DependentCell(root) { root.value * 2 } + val branch2 = DependentCell(root) { root.value * 3 } + val branch3 = DependentCell(root) { root.value * 4 } + val leaf = createWithDependencies(branch1, branch2, branch3) + var dependencyMightChangeCalled = false + var dependencyChangedCalled = false + + leaf.addDependent(object : Dependent { + override fun dependencyMightChange() { + assertFalse(dependencyMightChangeCalled) + assertFalse(dependencyChangedCalled) + dependencyMightChangeCalled = true + } + + override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { + assertTrue(dependencyMightChangeCalled) + assertFalse(dependencyChangedCalled) + assertEquals(leaf, dependency) + assertNotNull(event) + dependencyChangedCalled = true + } + }) + + repeat(5) { index -> + dependencyMightChangeCalled = false + dependencyChangedCalled = false + + root.value += 1 + + assertTrue(dependencyMightChangeCalled, "repetition $index") + assertTrue(dependencyChangedCalled, "repetition $index") + } + } @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) + val branch1 = DependentCell(root) { root.value * 2 } + val branch2 = DependentCell(root) { root.value * 3 } + val branch3 = DependentCell(root) { root.value * 4 } + val leaf = createWithDependencies(branch1, branch2, branch3) var observedChanges = 0 @@ -30,29 +70,31 @@ interface CellWithDependenciesTests : CellTests { @Test fun doesnt_register_as_dependent_of_its_dependencies_until_it_has_dependents_itself() = test { - val p = createProvider() + val dependency1 = TestCell() + val dependency2 = TestCell() + val dependency3 = TestCell() - val dependency = object : AbstractCell() { - val publicDependents: List = dependents + val cell = createWithDependencies(dependency1, dependency2, dependency3) - override val value: Int = 5 - - override fun emitDependencyChanged() { - // Not going to change. - throw NotImplementedError() - } - } - - val cell = p.createWithDependencies(dependency) - - assertTrue(dependency.publicDependents.isEmpty()) + assertTrue(dependency1.publicDependents.isEmpty()) + assertTrue(dependency2.publicDependents.isEmpty()) + assertTrue(dependency3.publicDependents.isEmpty()) disposer.add(cell.observeChange { }) - assertEquals(1, dependency.publicDependents.size) + assertEquals(1, dependency1.publicDependents.size) + assertEquals(1, dependency2.publicDependents.size) + assertEquals(1, dependency3.publicDependents.size) } - interface Provider : CellTests.Provider { - fun createWithDependencies(vararg dependencies: Cell): Cell + private class TestCell : AbstractCell() { + val publicDependents: List = dependents + + override val value: Int = 5 + + override fun emitDependencyChanged() { + // Not going to change. + throw NotImplementedError() + } } } 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 d48da842..1e87eb45 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellTests.kt @@ -8,7 +8,16 @@ class DependentCellTests : RegularCellTests, CellWithDependenciesTests { return DependentCell(dependency) { dependency.value } } - class Provider : CellTests.Provider, CellWithDependenciesTests.Provider { + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + DependentCell(dependency1, dependency2, dependency3) { + dependency1.value + dependency2.value + dependency3.value + } + + class Provider : CellTests.Provider { private val dependencyCell = SimpleCell(1) override val observable = DependentCell(dependencyCell) { 2 * dependencyCell.value } @@ -16,8 +25,5 @@ class DependentCellTests : RegularCellTests, CellWithDependenciesTests { override fun emit() { dependencyCell.value += 2 } - - 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 e559f561..6312e9f6 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellTransitiveDependencyEmitsTests.kt @@ -14,7 +14,16 @@ class FlatteningDependentCellTransitiveDependencyEmitsTests : return FlatteningDependentCell(dependency) { dependency.value } } - class Provider : CellTests.Provider, CellWithDependenciesTests.Provider { + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + FlatteningDependentCell(dependency1, dependency2, dependency3) { + ImmutableCell(dependency1.value + dependency2.value + dependency3.value) + } + + class Provider : CellTests.Provider { // The transitive dependency can change. private val transitiveDependency = SimpleCell(5) @@ -28,8 +37,5 @@ class FlatteningDependentCellTransitiveDependencyEmitsTests : // Update the transitive dependency. transitiveDependency.value += 5 } - - override fun createWithDependencies(vararg dependencies: Cell): Cell = - FlatteningDependentCell(*dependencies) { ImmutableCell(dependencies.sumOf { it.value }) } } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCellTests.kt new file mode 100644 index 00000000..7c85307d --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/AbstractFilteredListCellTests.kt @@ -0,0 +1,306 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.ChangeManager +import world.phantasmal.observable.cell.CellWithDependenciesTests +import world.phantasmal.observable.cell.ImmutableCell +import world.phantasmal.observable.cell.SimpleCell +import kotlin.test.* + +interface AbstractFilteredListCellTests : ListCellTests, CellWithDependenciesTests { + @Test + fun contains_only_values_that_match_the_predicate() = test { + val dep = SimpleListCell(mutableListOf("a", "b")) + val list = SimpleFilteredListCell(dep, predicate = ImmutableCell { 'a' in it }) + + assertEquals(1, list.value.size) + assertEquals("a", list.value[0]) + + dep.add("foo") + dep.add("bar") + + assertEquals(2, list.value.size) + assertEquals("a", list.value[0]) + assertEquals("bar", list.value[1]) + + dep.add("quux") + dep.add("qaax") + + assertEquals(3, list.value.size) + assertEquals("a", list.value[0]) + assertEquals("bar", list.value[1]) + assertEquals("qaax", list.value[2]) + } + + @Test + fun only_emits_when_necessary() = test { + val dep = SimpleListCell(mutableListOf()) + val list = SimpleFilteredListCell(dep, predicate = ImmutableCell { it % 2 == 0 }) + var changes = 0 + var listChanges = 0 + + disposer.add(list.observeChange { + changes++ + }) + disposer.add(list.observeListChange { + listChanges++ + }) + + dep.add(1) + dep.add(3) + dep.add(5) + + assertEquals(0, changes) + assertEquals(0, listChanges) + + dep.add(0) + dep.add(2) + dep.add(4) + + assertEquals(3, changes) + assertEquals(3, listChanges) + } + + @Test + fun emits_correct_change_events() = test { + val dep = SimpleListCell(mutableListOf()) + val list = SimpleFilteredListCell(dep, predicate = ImmutableCell { it % 2 == 0 }) + var event: ListChangeEvent? = null + + disposer.add(list.observeListChange { + assertNull(event) + event = it + }) + + run { + dep.replaceAll(listOf(1, 2, 3, 4, 5)) + + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + 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 + + run { + dep.splice(2, 2, 10) + + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(1, c.index) + assertEquals(1, c.removed.size) + assertEquals(4, c.removed[0]) + assertEquals(1, c.inserted.size) + assertEquals(10, c.inserted[0]) + } + } + + @Test + fun value_changes_and_emits_when_predicate_changes() = test { + val predicate: SimpleCell<(Int) -> Boolean> = SimpleCell { it % 2 == 0 } + val list = SimpleFilteredListCell(ImmutableListCell(listOf(1, 2, 3, 4, 5)), predicate) + var event: ListChangeEvent? = null + + disposer.add(list.observeListChange { + assertNull(event) + event = it + }) + + run { + // Change predicate. + predicate.value = { it % 2 == 1 } + + // Value changes. + assertEquals(listOf(1, 3, 5), list.value) + + // An event was emitted. + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(0, c.index) + assertEquals(listOf(2, 4), c.removed) + assertEquals(listOf(1, 3, 5), c.inserted) + } + + event = null + + run { + // Change predicate. + predicate.value = { it % 2 == 0 } + + // Value changes. + assertEquals(listOf(2, 4), list.value) + + // An event was emitted. + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(0, c.index) + assertEquals(listOf(1, 3, 5), c.removed) + assertEquals(listOf(2, 4), c.inserted) + } + } + + @Test + fun emits_correctly_when_multiple_changes_happen_at_once() = test { + val dependency = object : AbstractListCell() { + private val changes: MutableList> = mutableListOf() + override val elements: MutableList = mutableListOf() + override val value = elements + + override fun emitDependencyChanged() { + emitDependencyChangedEvent(ListChangeEvent( + elementsWrapper, + changes.map { (index, newElement) -> + ListChange( + index = index, + prevSize = index, + removed = emptyList(), + inserted = listOf(newElement), + ) + } + )) + changes.clear() + } + + fun makeChanges(newElements: List) { + emitMightChange() + + for (newElement in newElements) { + changes.add(Pair(elements.size, newElement)) + elements.add(newElement) + } + + ChangeManager.changed(this) + } + } + + val list = SimpleFilteredListCell(dependency, ImmutableCell { true }) + var event: ListChangeEvent? = null + + disposer.add(list.observeListChange { + assertNull(event) + event = it + }) + + for (i in 1..3) { + event = null + + // Make two changes at once everytime. + val change0 = i * 13 + val change1 = i * 17 + val changes = listOf(change0, change1) + val oldList = list.value.toList() + + dependency.makeChanges(changes) + + // These checks are very implementation-specific. At some point the filtered list might, + // for example, emit an event with a single change instead of two changes and then this + // test will incorrectly fail. + val e = event + assertNotNull(e) + assertEquals(oldList + changes, e.value) + assertEquals(2, e.changes.size) + + val lc0 = e.changes[0] + assertEquals(oldList.size, lc0.index) + assertEquals(oldList.size, lc0.prevSize) + assertTrue(lc0.removed.isEmpty()) + assertEquals(listOf(change0), lc0.inserted) + + val lc1 = e.changes[1] + assertEquals(oldList.size + 1, lc1.index) + assertEquals(oldList.size + 1, lc1.prevSize) + assertTrue(lc1.removed.isEmpty()) + assertEquals(listOf(change1), lc1.inserted) + } + } + + @Test + fun emits_correctly_when_dependency_contains_same_element_twice() = test { + val x = "x" + val y = "y" + val z = "z" + val dependency = SimpleListCell(mutableListOf(x, y, z, x, y, z)) + val list = SimpleFilteredListCell(dependency, SimpleCell { it != y }) + var event: ListChangeEvent? = null + + disposer.add(list.observeListChange { + assertNull(event) + event = it + }) + + assertEquals(listOf(x, z, x, z), list.value) + + run { + // Remove second x element. + dependency.removeAt(3) + + // Value changes. + assertEquals(listOf(x, z, z), list.value) + + // An event was emitted. + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(2, c.index) + assertEquals(listOf(x), c.removed) + assertTrue(c.inserted.isEmpty()) + } + + event = null + + run { + // Remove first x element. + dependency.removeAt(0) + + // Value changes. + assertEquals(listOf(z, z), list.value) + + // An event was emitted. + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(0, c.index) + assertEquals(listOf(x), c.removed) + assertTrue(c.inserted.isEmpty()) + } + + event = null + + run { + // Remove second z element. + dependency.removeAt(3) + + // Value changes. + assertEquals(listOf(z), list.value) + + // An event was emitted. + val e = event + assertNotNull(e) + assertEquals(1, e.changes.size) + + val c = e.changes.first() + assertEquals(1, c.index) + assertEquals(listOf(z), c.removed) + assertTrue(c.inserted.isEmpty()) + } + } +} 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 56dc3a6b..7030d239 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 @@ -8,7 +8,16 @@ class DependentListCellTests : ListCellTests, CellWithDependenciesTests { override fun createListProvider(empty: Boolean) = Provider(empty) - class Provider(empty: Boolean) : ListCellTests.Provider, CellWithDependenciesTests.Provider { + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + DependentListCell(dependency1, dependency2, dependency3) { + listOf(dependency1.value, dependency2.value, dependency3.value) + } + + class Provider(empty: Boolean) : ListCellTests.Provider { private val dependencyCell = SimpleListCell(if (empty) mutableListOf() else mutableListOf(5)) @@ -18,8 +27,5 @@ class DependentListCellTests : ListCellTests, CellWithDependenciesTests { override fun addElement() { dependencyCell.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/FilteredListCellListDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellListDependencyEmitsTests.kt new file mode 100644 index 00000000..f07ab204 --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellListDependencyEmitsTests.kt @@ -0,0 +1,41 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.DependentCell +import world.phantasmal.observable.cell.ImmutableCell + +// TODO: A test suite that tests FilteredListCell while its predicate dependency is changing. +// TODO: A test suite that tests FilteredListCell while the predicate results are changing. +class FilteredListCellListDependencyEmitsTests : AbstractFilteredListCellTests { + override fun createListProvider(empty: Boolean) = object : ListCellTests.Provider { + private val dependencyCell = + SimpleListCell(if (empty) mutableListOf(5) else mutableListOf(5, 10)) + + override val observable = + FilteredListCell( + list = dependencyCell, + predicate = ImmutableCell { ImmutableCell(it % 2 == 0) }, + ) + + override fun addElement() { + dependencyCell.add(4) + } + } + + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + FilteredListCell( + list = DependentListCell(dependency1, computeElements = { + listOf(dependency1.value) + }), + predicate = DependentCell(dependency2, compute = { + fun predicate(element: Int) = + DependentCell(dependency3, compute = { element < dependency2.value }) + + ::predicate + }), + ) +} 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 deleted file mode 100644 index 6d2405e6..00000000 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FilteredListCellTests.kt +++ /dev/null @@ -1,179 +0,0 @@ -package world.phantasmal.observable.cell.list - -import world.phantasmal.observable.cell.SimpleCell -import kotlin.test.* - -class FilteredListCellTests : ListCellTests { - override fun createListProvider(empty: Boolean) = object : ListCellTests.Provider { - private val dependencyCell = - SimpleListCell(if (empty) mutableListOf(5) else mutableListOf(5, 10)) - - override val observable = FilteredListCell(dependencyCell, predicate = { it % 2 == 0 }) - - override fun addElement() { - dependencyCell.add(4) - } - } - - @Test - fun contains_only_values_that_match_the_predicate() = test { - val dep = SimpleListCell(mutableListOf("a", "b")) - val list = FilteredListCell(dep, predicate = { 'a' in it }) - - assertEquals(1, list.value.size) - assertEquals("a", list.value[0]) - - dep.add("foo") - dep.add("bar") - - assertEquals(2, list.value.size) - assertEquals("a", list.value[0]) - assertEquals("bar", list.value[1]) - - dep.add("quux") - dep.add("qaax") - - assertEquals(3, list.value.size) - assertEquals("a", list.value[0]) - assertEquals("bar", list.value[1]) - assertEquals("qaax", list.value[2]) - } - - @Test - fun only_emits_when_necessary() = test { - val dep = SimpleListCell(mutableListOf()) - val list = FilteredListCell(dep, predicate = { it % 2 == 0 }) - var changes = 0 - var listChanges = 0 - - disposer.add(list.observeChange { - changes++ - }) - disposer.add(list.observeListChange { - listChanges++ - }) - - dep.add(1) - dep.add(3) - dep.add(5) - - assertEquals(0, changes) - assertEquals(0, listChanges) - - dep.add(0) - dep.add(2) - dep.add(4) - - assertEquals(3, changes) - assertEquals(3, listChanges) - } - - @Test - fun emits_correct_change_events() = test { - val dep = SimpleListCell(mutableListOf()) - val list = FilteredListCell(dep, predicate = { it % 2 == 0 }) - var event: ListChangeEvent? = null - - disposer.add(list.observeListChange { - assertNull(event) - event = it - }) - - dep.replaceAll(listOf(1, 2, 3, 4, 5)) - - 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) - - 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 [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_element_changes() = test { - val dep = SimpleListCell( - mutableListOf(SimpleCell(1), SimpleCell(2), SimpleCell(3), SimpleCell(4)), - extractDependencies = { arrayOf(it) }, - ) - val list = FilteredListCell(dep, predicate = { it.value % 2 == 0 }) - var event: ListChangeEvent>? = null - - disposer.add(list.observeListChange { - assertNull(event) - event = it - }) - - for (i in 0 until dep.size.value) { - event = null - - // 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 - - 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) - } - } - - for (i in 0 until dep.size.value) { - event = null - - // Change a value, but keep even numbers even and odd numbers odd. List should emit an - // ElementChange event. - val newValue = dep[i].value + 2 - dep[i].value = newValue - - 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/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCellTransitiveDependencyEmitsTests.kt index 97a41eec..5ce3da89 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,6 +1,7 @@ package world.phantasmal.observable.cell.list import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.CellTests import world.phantasmal.observable.cell.CellWithDependenciesTests import world.phantasmal.observable.cell.ImmutableCell @@ -15,7 +16,16 @@ class FlatteningDependentListCellTransitiveDependencyEmitsTests : override fun createListProvider(empty: Boolean) = Provider(empty) - class Provider(empty: Boolean) : ListCellTests.Provider, CellWithDependenciesTests.Provider { + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + FlatteningDependentListCell(dependency1, dependency2, dependency3) { + ImmutableListCell(listOf(dependency1.value, dependency2.value, dependency3.value)) + } + + class Provider(empty: Boolean) : ListCellTests.Provider, CellTests.Provider { // The transitive dependency can change. private val transitiveDependency = SimpleListCell(if (empty) mutableListOf() else mutableListOf(7)) @@ -30,10 +40,5 @@ class FlatteningDependentListCellTransitiveDependencyEmitsTests : // Update the transitive dependency. transitiveDependency.add(4) } - - override fun createWithDependencies(vararg dependencies: Cell): Cell = - FlatteningDependentListCell(*dependencies) { - ImmutableListCell(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 243b1560..ed207f58 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 @@ -211,8 +211,8 @@ interface ListCellTests : CellTests { p.addElement() assertNotNull(firstOrNull.value) - // Observer should not be called when adding elements at the end of the list. - assertNull(observedValue) + // Observer may or may not be called when adding elements at the end of the list. + assertTrue(observedValue == null || observedValue == firstOrNull.value) } } diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellElementEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellElementEmitsTests.kt new file mode 100644 index 00000000..f4a68e1f --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellElementEmitsTests.kt @@ -0,0 +1,43 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.cell.* + +/** + * In these tests, the direct list cell dependency of the [ListElementsDependentCell] doesn't + * change, but its elements do change. + */ +class ListElementsDependentCellElementEmitsTests : CellWithDependenciesTests { + + override fun createProvider() = object : CellTests.Provider { + // One transitive dependency can change. + private val transitiveDependency = Element(2) + + // The direct dependency of the list under test can't change. + private val directDependency: ListCell = + ImmutableListCell(listOf(Element(1), transitiveDependency, Element(3))) + + override val observable = + ListElementsDependentCell(directDependency) { arrayOf(it.int, it.double, it.string) } + + override fun emit() { + transitiveDependency.int.value++ + } + } + + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + ListElementsDependentCell( + ImmutableListCell(listOf(dependency1, dependency2, dependency3)) + ) { arrayOf(it) } + + private class Element(value: Int) { + val int: MutableCell = SimpleCell(value) + val double: Cell = DependentCell(int) { int.value.toDouble() } + val string: Cell = DependentCell(int) { int.value.toString() } + + override fun toString(): String = "Element[int=$int, double=$double, string=$string]" + } +} diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellListCellEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellListCellEmitsTests.kt new file mode 100644 index 00000000..3d9565d0 --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellListCellEmitsTests.kt @@ -0,0 +1,25 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.CellTests +import world.phantasmal.observable.cell.ImmutableCell + +/** + * In these tests, the direct list cell dependency of the [ListElementsDependentCell] changes, while + * its elements don't change. + */ +class ListElementsDependentCellListCellEmitsTests : CellTests { + + override fun createProvider() = object : CellTests.Provider { + // The direct dependency of the list under test changes, its elements are immutable. + private val directDependency: SimpleListCell> = + SimpleListCell(mutableListOf(ImmutableCell(1), ImmutableCell(2), ImmutableCell(3))) + + override val observable = + ListElementsDependentCell(directDependency) { arrayOf(it) } + + override fun emit() { + directDependency.add(ImmutableCell(directDependency.value.size + 1)) + } + } +} diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellTests.kt new file mode 100644 index 00000000..7ea2f5c9 --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/ListElementsDependentCellTests.kt @@ -0,0 +1,56 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.ChangeEvent +import world.phantasmal.observable.cell.SimpleCell +import world.phantasmal.observable.test.ObservableTestSuite +import kotlin.test.Test +import kotlin.test.assertNotNull +import kotlin.test.assertNull + +/** + * Standard tests are done by [ListElementsDependentCellElementEmitsTests] and + * [ListElementsDependentCellListCellEmitsTests]. + */ +class ListElementsDependentCellTests : ObservableTestSuite { + @Test + fun element_changes_are_correctly_propagated() = test { + val list = SimpleListCell( + mutableListOf( + SimpleCell("a"), + SimpleCell("b"), + SimpleCell("c") + ) + ) + + val cell = ListElementsDependentCell(list) { arrayOf(it) } + + var event: ChangeEvent<*>? = null + + disposer.add(cell.observeChange { + assertNull(event) + event = it + }) + + // The cell should not emit events when an old element is changed. + run { + val removed = list.removeAt(1) + event = null + + removed.value += "-1" + + assertNull(event) + } + + // The cell should emit events when any of the current elements are changed. + list.add(SimpleCell("d")) + + for (element in list.value) { + event = null + + element.value += "-2" + + val e = event + assertNotNull(e) + } + } +} 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 5a0aab62..09b99b1d 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 @@ -34,7 +34,6 @@ interface MutableListCellTests : ListCellTests, MutableCellTests : ListCellTests, MutableCellTests : ListCellTests, MutableCellTests, + dependency2: Cell, + dependency3: Cell, + ) = + SimpleFilteredListCell( + list = DependentListCell(dependency1, dependency2) { + listOf(dependency1.value, dependency2.value) + }, + predicate = DependentCell(dependency3) { + { it < dependency3.value } + }, + ) +} diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCellPredicateDependencyEmitsTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCellPredicateDependencyEmitsTests.kt new file mode 100644 index 00000000..6fb8ee1f --- /dev/null +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/list/SimpleFilteredListCellPredicateDependencyEmitsTests.kt @@ -0,0 +1,41 @@ +package world.phantasmal.observable.cell.list + +import world.phantasmal.observable.cell.Cell +import world.phantasmal.observable.cell.DependentCell +import world.phantasmal.observable.cell.SimpleCell + +/** + * In these tests the predicate dependency of the [SimpleListCell] changes and the list dependency + * does not. + */ +class SimpleFilteredListCellPredicateDependencyEmitsTests : AbstractFilteredListCellTests { + override fun createListProvider(empty: Boolean) = object : ListCellTests.Provider { + private var maxValue = if (empty) 0 else 1 + private val predicateCell = SimpleCell<(Int) -> Boolean> { it <= maxValue } + + override val observable = SimpleFilteredListCell( + list = ImmutableListCell((1..20).toList()), + predicate = predicateCell, + ) + + override fun addElement() { + maxValue++ + val max = maxValue + predicateCell.value = { it <= max } + } + } + + override fun createWithDependencies( + dependency1: Cell, + dependency2: Cell, + dependency3: Cell, + ) = + SimpleFilteredListCell( + list = DependentListCell(dependency1, dependency2) { + listOf(dependency1.value, dependency2.value) + }, + predicate = DependentCell(dependency3) { + { it < dependency3.value } + }, + ) +} 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 e4ddfb87..ee9dd12f 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,9 +1,10 @@ package world.phantasmal.observable.cell.list -import world.phantasmal.observable.cell.SimpleCell import world.phantasmal.observable.test.assertListCellEquals -import world.phantasmal.testUtils.TestContext -import kotlin.test.* +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue class SimpleListCellTests : MutableListCellTests { override fun createProvider() = createListProvider(empty = true) @@ -31,15 +32,8 @@ class SimpleListCellTests : MutableListCellTests { @Test fun set() = test { - testSet(SimpleListCell(mutableListOf("a", "b", "c"))) - } + val list = SimpleListCell(mutableListOf("a", "b", "c")) - @Test - fun set_with_extractDependencies() = test { - testSet(SimpleListCell(mutableListOf("a", "b", "c")) { arrayOf() }) - } - - private fun testSet(list: SimpleListCell) { list[1] = "test" list[2] = "test2" assertFailsWith { @@ -147,133 +141,4 @@ class SimpleListCellTests : MutableListCellTests { // List should remain unchanged after invalid calls. assertListCellEquals(listOf(101, 0, 1, 2, 100, 8, 9, 102), list) } - - @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.observeListChange { - 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/huntOptimizer/controllers/MethodsForEpisodeController.kt b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt index a749b4af..e1df8fc1 100644 --- a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt +++ b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/controllers/MethodsForEpisodeController.kt @@ -1,9 +1,6 @@ package world.phantasmal.web.huntOptimizer.controllers -import world.phantasmal.observable.cell.list.ListCell -import world.phantasmal.observable.cell.list.filtered -import world.phantasmal.observable.cell.list.listCell -import world.phantasmal.observable.cell.list.sortedWith +import world.phantasmal.observable.cell.list.* import world.phantasmal.observable.cell.mutableCell import world.phantasmal.psolib.Episode import world.phantasmal.psolib.fileFormats.quest.NpcType @@ -27,9 +24,13 @@ class MethodsForEpisodeController( override val fixedColumns = 2 override val values: ListCell by lazy { - huntMethodStore.methods + val methodsForEpisode = huntMethodStore.methods .filtered { it.episode == episode } - .sortedWith(comparator) + .dependingOnElements { arrayOf(it.time) } + + mapToList(methodsForEpisode, comparator) { list, cmp -> + list.sortedWith(cmp) + } } override val loadingStatus: LoadingStatusCell = huntMethodStore.methodsStatus diff --git a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt index 89401d9a..9b1c0876 100644 --- a/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt +++ b/web/src/main/kotlin/world/phantasmal/web/huntOptimizer/stores/HuntMethodStore.kt @@ -26,7 +26,7 @@ class HuntMethodStore( private val assetLoader: AssetLoader, private val huntMethodPersister: HuntMethodPersister, ) : Store() { - private val _methods = mutableListCell { arrayOf(it.time) } + private val _methods = mutableListCell() private val _methodsStatus = LoadingStatusCellImpl(scope, "methods", ::loadMethods) /** Hunting methods supported by the current server. */ 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 42ab0af6..6eadbe18 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 @@ -12,8 +12,10 @@ import world.phantasmal.core.unsafe.UnsafeMap import world.phantasmal.core.unsafe.UnsafeSet import world.phantasmal.observable.cell.Cell import world.phantasmal.observable.cell.list.ListCell +import world.phantasmal.observable.cell.list.dependingOnElements import world.phantasmal.observable.cell.list.mutableListCell import world.phantasmal.observable.cell.mutableCell +import world.phantasmal.observable.observe import world.phantasmal.psolib.fileFormats.quest.NpcType import world.phantasmal.web.core.models.Server import world.phantasmal.web.core.stores.EnemyDropTable @@ -50,7 +52,7 @@ class HuntOptimizerStore( private val itemDropStore: ItemDropStore, ) : Store() { private val _huntableItems = mutableListCell() - private val _wantedItems = mutableListCell { arrayOf(it.amount) } + private val _wantedItems = mutableListCell() private val _optimizationResult = mutableCell(OptimizationResultModel(emptyList(), emptyList())) private var wantedItemsPersistenceObserver: Disposable? = null @@ -58,6 +60,7 @@ class HuntOptimizerStore( observeNow(uiStore.server) { server -> _huntableItems.clear() + // There's a race condition here. scope.launch { val dropTable = itemDropStore.getEnemyDropTable(server) @@ -76,9 +79,18 @@ class HuntOptimizerStore( } val optimizationResult: Cell by lazy { - observeNow(wantedItems, huntMethodStore.methods) { wantedItems, huntMethods -> + observeNow( + _wantedItems.dependingOnElements { arrayOf(it.amount) }, + huntMethodStore.methods.dependingOnElements { arrayOf(it.time) }, + ) { wantedItems, huntMethods -> + // There's a race condition here. scope.launch(Dispatchers.Default) { - _optimizationResult.value = optimize(wantedItems, huntMethods) + val dropTable = itemDropStore.getEnemyDropTable(uiStore.server.value) + val result = optimize(wantedItems, huntMethods, dropTable) + + withContext(Dispatchers.Main) { + _optimizationResult.value = result + } } } @@ -91,7 +103,7 @@ class HuntOptimizerStore( } fun addWantedItem(itemType: ItemType) { - if (wantedItems.value.none { it.itemType == itemType }) { + if (_wantedItems.value.none { it.itemType == itemType }) { _wantedItems.add(WantedItemModel(itemType, 1)) } } @@ -114,20 +126,20 @@ class HuntOptimizerStore( _wantedItems.replaceAll(wantedItems) // Wanted items are loaded, start observing them and persist whenever they change. - wantedItemsPersistenceObserver = _wantedItems.observeChange { - val items = it.value - - scope.launch(Dispatchers.Main) { - wantedItemPersister.persistWantedItems(items, server) + wantedItemsPersistenceObserver = + _wantedItems.dependingOnElements { arrayOf(it.amount) }.observe { items -> + scope.launch(Dispatchers.Main) { + wantedItemPersister.persistWantedItems(items, server) + } } - } } } } - private suspend fun optimize( + private fun optimize( wantedItems: List, methods: List, + dropTable: EnemyDropTable, ): OptimizationResultModel { logger.debug { "Optimization start." } @@ -139,8 +151,6 @@ class HuntOptimizerStore( return OptimizationResultModel(emptyList(), emptyList()) } - val dropTable = itemDropStore.getEnemyDropTable(uiStore.server.value) - // Add a constraint per wanted item. val constraints: dynamic = obj {} diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/models/QuestModel.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/models/QuestModel.kt index 3d4e5afa..80282de9 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/models/QuestModel.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/models/QuestModel.kt @@ -36,9 +36,9 @@ class QuestModel( private val _shortDescription = mutableCell("") private val _longDescription = mutableCell("") private val _mapDesignations = mutableCell(mapDesignations) - private val _npcs = SimpleListCell(npcs) { arrayOf(it.sectionInitialized, it.wave) } - private val _objects = SimpleListCell(objects) { arrayOf(it.sectionInitialized) } - private val _events = SimpleListCell(events) { arrayOf(it.id) } + private val _npcs = SimpleListCell(npcs) + private val _objects = SimpleListCell(objects) + private val _events = SimpleListCell(events) val id: Cell = _id val language: Cell = _language diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestEditorMeshManager.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestEditorMeshManager.kt index d0cf1d3c..714d1ee0 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestEditorMeshManager.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestEditorMeshManager.kt @@ -1,8 +1,11 @@ package world.phantasmal.web.questEditor.rendering +import world.phantasmal.observable.cell.and +import world.phantasmal.observable.cell.eq import world.phantasmal.observable.cell.flatMapNull import world.phantasmal.observable.cell.list.emptyListCell -import world.phantasmal.observable.cell.list.filtered +import world.phantasmal.observable.cell.list.filteredCell +import world.phantasmal.observable.cell.or import world.phantasmal.web.questEditor.loading.AreaAssetLoader import world.phantasmal.web.questEditor.loading.EntityAssetLoader import world.phantasmal.web.questEditor.stores.QuestEditorStore @@ -28,10 +31,10 @@ class QuestEditorMeshManager( ) { quest, area, wave -> loadNpcMeshes( if (quest != null && area != null) { - quest.npcs.filtered { - it.sectionInitialized.value && - it.areaId == area.id && - (wave == null || it.wave.value == wave) + quest.npcs.filteredCell { + it.sectionInitialized and + (it.areaId == area.id) and + ((wave == null) or (it.wave eq wave)) } } else { emptyListCell() @@ -45,8 +48,8 @@ class QuestEditorMeshManager( ) { quest, area -> loadObjectMeshes( if (quest != null && area != null) { - quest.objects.filtered { - it.sectionInitialized.value && it.areaId == area.id + quest.objects.filteredCell { + it.sectionInitialized and (it.areaId == area.id) } } else { emptyListCell() 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 632acac4..1f4c9225 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 @@ -6,7 +6,6 @@ import kotlinx.coroutines.launch import world.phantasmal.core.disposable.Disposable import world.phantasmal.core.disposable.DisposableSupervisedScope import world.phantasmal.observable.cell.list.ListCell -import world.phantasmal.observable.cell.list.ListChange import world.phantasmal.observable.cell.list.ListChangeEvent import world.phantasmal.psolib.Episode import world.phantasmal.web.questEditor.loading.AreaAssetLoader @@ -75,19 +74,15 @@ abstract class QuestMeshManager protected constructor( 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) - } + change.removed.forEach(npcMeshManager::remove) + change.inserted.forEach(npcMeshManager::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) - } + change.removed.forEach(objectMeshManager::remove) + change.inserted.forEach(objectMeshManager::add) } } } diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestRenderer.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestRenderer.kt index 99a1b06d..407533ed 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestRenderer.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/QuestRenderer.kt @@ -39,7 +39,16 @@ class QuestRenderer( ), ) - observeNow(questEditorStore.currentQuest) { inputManager.resetCamera() } - observeNow(questEditorStore.currentAreaVariant) { inputManager.resetCamera() } + var prevQuest = questEditorStore.currentQuest.value + var prevAreaVariant = questEditorStore.currentAreaVariant.value + + observeNow(questEditorStore.currentQuest, questEditorStore.currentAreaVariant) { q, av -> + if (q !== prevQuest || av !== prevAreaVariant) { + inputManager.resetCamera() + + prevQuest = q + prevAreaVariant = av + } + } } } diff --git a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/input/state/IdleState.kt b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/input/state/IdleState.kt index eb2ef5f0..cd4fa178 100644 --- a/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/input/state/IdleState.kt +++ b/web/src/main/kotlin/world/phantasmal/web/questEditor/rendering/input/state/IdleState.kt @@ -209,14 +209,14 @@ class IdleState( val entity: QuestEntityModel<*, *>, /** - * Vector that points from the grabbing point (somewhere on the model's surface) to the entity's - * origin. + * Vector that points from the grabbing point (somewhere on the model's surface) to the + * entity's origin. */ val grabOffset: Vector3, /** - * Vector that points from the grabbing point to the terrain point directly under the entity's - * origin. + * Vector that points from the grabbing point to the terrain point directly under the + * entity's origin. */ val dragAdjust: Vector3, ) 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 9a4d26c1..bad37a7f 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 @@ -158,8 +158,8 @@ class QuestEditorStore( updateQuestEntitySections(quest) // Ensure all entities have their section initialized. - quest.npcs.value.forEach { it.setSectionInitialized() } - quest.objects.value.forEach { it.setSectionInitialized() } + quest.npcs.value.forEach(QuestNpcModel::setSectionInitialized) + quest.objects.value.forEach(QuestObjectModel::setSectionInitialized) } } 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 index 51051944..04e7c5db 100644 --- a/web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt +++ b/web/src/test/kotlin/world/phantasmal/web/questEditor/controllers/EventsControllerTests.kt @@ -1,11 +1,13 @@ package world.phantasmal.web.questEditor.controllers -import world.phantasmal.observable.cell.observeNow import world.phantasmal.web.questEditor.models.QuestEventActionModel import world.phantasmal.web.questEditor.models.QuestEventModel import world.phantasmal.web.test.WebTestSuite import world.phantasmal.web.test.createQuestModel -import kotlin.test.* +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue class EventsControllerTests : WebTestSuite { @Test @@ -113,38 +115,27 @@ class EventsControllerTests : WebTestSuite { (ctrl.events[0].actions[0] as QuestEventActionModel.TriggerEvent).eventId ) - // We test the observed value instead of the cell's value property. - var canGoToEventValue: Boolean? = null - - disposer.add(canGoToEvent.observeNow { - assertNull(canGoToEventValue) - canGoToEventValue = it - }) - - assertEquals(true, canGoToEventValue) + assertEquals(true, canGoToEvent.value) // Let event 100 point to nonexistent event 102. - canGoToEventValue = null ctrl.setActionEventId( ctrl.events[0], ctrl.events[0].actions[0] as QuestEventActionModel.TriggerEvent, 102, ) - assertEquals(false, canGoToEventValue) + assertEquals(false, canGoToEvent.value) // Add event 102. - canGoToEventValue = null ctrl.selectEvent(null) // Deselect so the next event will be added at the end of the list. ctrl.addEvent() ctrl.setId(ctrl.events.value.last(), 102) - assertEquals(true, canGoToEventValue) + assertEquals(true, canGoToEvent.value) // Remove event 102. - canGoToEventValue = null ctrl.removeEvent(ctrl.events.value.last()) - assertEquals(false, canGoToEventValue) + assertEquals(false, canGoToEvent.value) } } 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 4866ebc7..9165079a 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/dom/Dom.kt @@ -14,7 +14,6 @@ import world.phantasmal.core.unsafe.UnsafeMap import world.phantasmal.core.unsafe.getOrPut 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 import world.phantasmal.webui.externals.fontawesome.IconDefinition import world.phantasmal.webui.externals.fontawesome.freeBrandsSvgIcons.faGithub @@ -288,28 +287,26 @@ private fun bindChildrenTo( return list.observeListChange { event: ListChangeEvent -> for (change in event.changes) { - if (change is ListChange.Structural) { - if (change.allRemoved) { - parent.innerHTML = "" - } else { - repeat(change.removed.size) { - parent.removeChild(parent.childNodes[change.index].unsafeCast()) - } + if (change.allRemoved) { + parent.innerHTML = "" + } else { + 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]) } } 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 01853f34..e9182272 100644 --- a/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt +++ b/webui/src/main/kotlin/world/phantasmal/webui/dom/HTMLElementSizeCell.kt @@ -72,7 +72,7 @@ class HTMLElementSizeCell(element: HTMLElement? = null) : AbstractCell() { if (newValue != _value) { emitMightChange() _value = newValue - emitDependencyChanged(ChangeEvent(newValue)) + emitDependencyChangedEvent(ChangeEvent(newValue)) } } }