Fixed bug related to ListCell.value by returning a lazy copy list.

observable/src/commonMain/kotlin/world/phantasmal/observable/Observer.kt

@@ -2,7 +2,7 @@ package world.phantasmal.observable
 
 open class ChangeEvent<out T>(
     /**
-     * The observable's new value
+     * The observable's new value.
      */
     val value: T,
 ) {

observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/AbstractListCell.kt

@@ -1,6 +1,7 @@
 package world.phantasmal.observable.cell.list
 
 import world.phantasmal.core.disposable.Disposable
+import world.phantasmal.core.unsafe.unsafeAssertNotNull
 import world.phantasmal.observable.CallbackObserver
 import world.phantasmal.observable.Observer
 import world.phantasmal.observable.cell.AbstractCell
@@ -9,6 +10,28 @@ import world.phantasmal.observable.cell.DependentCell
 import world.phantasmal.observable.cell.not
 
 abstract class AbstractListCell<E> : AbstractCell<List<E>>(), ListCell<E> {
+    /**
+     * When [value] is accessed and this property is null, a new wrapper is created that points to
+     * [elements]. Before changes to [elements] are made, if there's a wrapper, the current
+     * wrapper's backing list is set to a copy of [elements] and this property is set to null. This
+     * way, accessing [value] acts like accessing a snapshot without making an actual copy
+     * everytime. This is necessary because the contract is that a cell's new value is always != to
+     * its old value whenever a change event was emitted.
+     */
+    // TODO: Optimize this by using a weak reference to avoid copying when nothing references the
+    //  wrapper.
+    private var _elementsWrapper: DelegatingList<E>? = null
+    protected val elementsWrapper: DelegatingList<E>
+        get() {
+            if (_elementsWrapper == null) {
+                _elementsWrapper = DelegatingList(elements)
+            }
+
+            return unsafeAssertNotNull(_elementsWrapper)
+        }
+
+    protected abstract val elements: List<E>
+
     @Suppress("LeakingThis")
     final override val size: Cell<Int> = DependentCell(this) { value.size }
 
@@ -35,4 +58,9 @@ abstract class AbstractListCell<E> : AbstractCell<List<E>>(), ListCell<E> {
 
         return observingCell
     }
+
+    protected fun copyAndResetWrapper() {
+        _elementsWrapper?.backingList = elements.toList()
+        _elementsWrapper = null
+    }
 }

observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/DelegatingList.kt

@@ -0,0 +1,36 @@
+package world.phantasmal.observable.cell.list
+
+/**
+ * Simply delegates all methods to [backingList], even [equals], [hashCode] and [toString].
+ */
+class DelegatingList<E>(var backingList: List<E>) : List<E> {
+    override val size: Int = backingList.size
+
+    override fun contains(element: E): Boolean = backingList.contains(element)
+
+    override fun containsAll(elements: Collection<E>): Boolean = backingList.containsAll(elements)
+
+    override fun get(index: Int): E = backingList[index]
+
+    override fun indexOf(element: E): Int = backingList.indexOf(element)
+
+    override fun isEmpty(): Boolean = backingList.isEmpty()
+
+    override fun iterator(): Iterator<E> = backingList.iterator()
+
+    override fun lastIndexOf(element: E): Int = backingList.lastIndexOf(element)
+
+    override fun listIterator(): ListIterator<E> = backingList.listIterator()
+
+    override fun listIterator(index: Int): ListIterator<E> = backingList.listIterator(index)
+
+    override fun subList(fromIndex: Int, toIndex: Int): List<E> =
+        backingList.subList(fromIndex, toIndex)
+
+    @Suppress("SuspiciousEqualsCombination")
+    override fun equals(other: Any?): Boolean = this === other || other == backingList
+
+    override fun hashCode(): Int = backingList.hashCode()
+
+    override fun toString(): String = backingList.toString()
+}

observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FilteredListCell.kt

@@ -1,5 +1,6 @@
 package world.phantasmal.observable.cell.list
 
+import world.phantasmal.core.unsafe.unsafeAssertNotNull
 import world.phantasmal.observable.ChangeEvent
 import world.phantasmal.observable.Dependency
 import world.phantasmal.observable.Dependent
@@ -14,7 +15,7 @@ class FilteredListCell<E>(
      */
     private val indexMap = mutableListOf<Int>()
 
-    private val elements = mutableListOf<E>()
+    override val elements = mutableListOf<E>()
 
     override val value: List<E>
         get() {
@@ -22,7 +23,7 @@ class FilteredListCell<E>(
                 recompute()
             }
 
-            return elements
+            return elementsWrapper
         }
 
     override fun addDependent(dependent: Dependent) {
@@ -125,6 +126,7 @@ class FilteredListCell<E>(
                                     }
                                 }
 
+                                copyAndResetWrapper()
                                 elements.add(insertIndex, change.updated)
                                 indexMap[change.index] = insertIndex
 
@@ -151,6 +153,7 @@ class FilteredListCell<E>(
                             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
 
@@ -181,7 +184,7 @@ class FilteredListCell<E>(
             if (filteredChanges.isEmpty()) {
                 emitDependencyChanged(null)
             } else {
-                emitDependencyChanged(ListChangeEvent(elements, filteredChanges))
+                emitDependencyChanged(ListChangeEvent(elementsWrapper, filteredChanges))
             }
         } else {
             emitDependencyChanged(null)
@@ -195,6 +198,7 @@ class FilteredListCell<E>(
     }
 
     private fun recompute() {
+        copyAndResetWrapper()
         elements.clear()
         indexMap.clear()
 

observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/FlatteningDependentListCell.kt

@@ -29,7 +29,7 @@ class FlatteningDependentListCell<E>(
             computedCell = computeElements.invoke().also { computedCell ->
                 computedCell.addDependent(this)
                 computedInDeps = dependencies.any { it === computedCell }
-                elements = computedCell.value.toList()
+                elements = computedCell.value
             }
         }
 
@@ -70,6 +70,6 @@ class FlatteningDependentListCell<E>(
             shouldRecompute = false
         }
 
-        elements = unsafeAssertNotNull(computedCell).value.toList()
+        elements = unsafeAssertNotNull(computedCell).value
     }
 }

observable/src/commonMain/kotlin/world/phantasmal/observable/cell/list/SimpleListCell.kt

@@ -17,7 +17,7 @@ typealias DependenciesExtractor<E> = (element: E) -> Array<Dependency>
  * event.
  */
 class SimpleListCell<E>(
-    private val elements: MutableList<E>,
+    override val elements: MutableList<E>,
     private val extractDependencies: DependenciesExtractor<E>? = null,
 ) : AbstractListCell<E>(), MutableListCell<E> {
 
@@ -30,7 +30,7 @@ class SimpleListCell<E>(
     private var changes = mutableListOf<ListChange<E>>()
 
     override var value: List<E>
-        get() = elements
+        get() = elementsWrapper
         set(value) {
             replaceAll(value)
         }
@@ -42,6 +42,7 @@ class SimpleListCell<E>(
         checkIndex(index, elements.lastIndex)
         emitMightChange()
 
+        copyAndResetWrapper()
         val removed = elements.set(index, element)
 
         if (dependents.isNotEmpty() && extractDependencies != null) {
@@ -59,6 +60,7 @@ class SimpleListCell<E>(
         emitMightChange()
 
         val index = elements.size
+        copyAndResetWrapper()
         elements.add(element)
 
         finalizeStructuralChange(index, emptyList(), listOf(element))
@@ -68,6 +70,7 @@ class SimpleListCell<E>(
         checkIndex(index, elements.size)
         emitMightChange()
 
+        copyAndResetWrapper()
         elements.add(index, element)
 
         finalizeStructuralChange(index, emptyList(), listOf(element))
@@ -88,6 +91,7 @@ class SimpleListCell<E>(
         checkIndex(index, elements.lastIndex)
         emitMightChange()
 
+        copyAndResetWrapper()
         val removed = elements.removeAt(index)
 
         finalizeStructuralChange(index, listOf(removed), emptyList())
@@ -97,19 +101,21 @@ class SimpleListCell<E>(
     override fun replaceAll(elements: Iterable<E>) {
         emitMightChange()
 
-        val removed = ArrayList(this.elements)
+        val removed = elementsWrapper
+        copyAndResetWrapper()
         this.elements.replaceAll(elements)
 
-        finalizeStructuralChange(0, removed, ArrayList(this.elements))
+        finalizeStructuralChange(0, removed, elementsWrapper)
     }
 
     override fun replaceAll(elements: Sequence<E>) {
         emitMightChange()
 
-        val removed = ArrayList(this.elements)
+        val removed = elementsWrapper
+        copyAndResetWrapper()
         this.elements.replaceAll(elements)
 
-        finalizeStructuralChange(0, removed, ArrayList(this.elements))
+        finalizeStructuralChange(0, removed, elementsWrapper)
     }
 
     override fun splice(fromIndex: Int, removeCount: Int, newElement: E) {
@@ -121,6 +127,7 @@ class SimpleListCell<E>(
 
         emitMightChange()
 
+        copyAndResetWrapper()
         repeat(removeCount) { elements.removeAt(fromIndex) }
         elements.add(fromIndex, newElement)
 
@@ -130,7 +137,8 @@ class SimpleListCell<E>(
     override fun clear() {
         emitMightChange()
 
-        val removed = ArrayList(this.elements)
+        val removed = elementsWrapper
+        copyAndResetWrapper()
         elements.clear()
 
         finalizeStructuralChange(0, removed, emptyList())
@@ -139,7 +147,8 @@ class SimpleListCell<E>(
     override fun sortWith(comparator: Comparator<E>) {
         emitMightChange()
 
-        val removed = ArrayList(elements)
+        val removed = elementsWrapper
+        copyAndResetWrapper()
         var throwable: Throwable? = null
 
         try {
@@ -148,7 +157,7 @@ class SimpleListCell<E>(
             throwable = e
         }
 
-        finalizeStructuralChange(0, removed, ArrayList(elements))
+        finalizeStructuralChange(0, removed, elementsWrapper)
 
         if (throwable != null) {
             throw throwable
@@ -180,7 +189,7 @@ class SimpleListCell<E>(
     override fun emitDependencyChanged() {
         val currentChanges = changes
         changes = mutableListOf()
-        emitDependencyChanged(ListChangeEvent(elements, currentChanges))
+        emitDependencyChanged(ListChangeEvent(elementsWrapper, currentChanges))
     }
 
     private fun checkIndex(index: Int, maxIndex: Int) {

observable/src/commonTest/kotlin/world/phantasmal/observable/cell/CellTests.kt

@@ -122,6 +122,32 @@ interface CellTests : ObservableTests {
         }
     }
 
+    /**
+     * [Cell.value] should correctly reflect changes even when the [Cell] has no observers.
+     * Typically this means that the cell's value is not updated in real time, only when it is
+     * queried.
+     */
+    @Test
+    fun reflects_changes_without_observers() = test {
+        val p = createProvider()
+
+        var old: Any?
+
+        repeat(5) {
+            // Value should change after emit.
+            old = p.observable.value
+
+            p.emit()
+
+            val new = p.observable.value
+
+            assertNotEquals(old, new)
+
+            // Value should not change when emit hasn't been called since the last access.
+            assertEquals(new, p.observable.value)
+        }
+    }
+
     interface Provider : ObservableTests.Provider {
         override val observable: Cell<Any>
     }

observable/src/commonTest/kotlin/world/phantasmal/observable/cell/DependentCellWithSimpleListCellTests.kt

@@ -0,0 +1,17 @@
+package world.phantasmal.observable.cell
+
+import world.phantasmal.observable.cell.list.SimpleListCell
+
+class DependentCellWithSimpleListCellTests : CellTests {
+    override fun createProvider() = Provider()
+
+    class Provider : CellTests.Provider {
+        private val dependencyCell = SimpleListCell(mutableListOf("a", "b", "c"))
+
+        override val observable = DependentCell(dependencyCell) { dependencyCell.value }
+
+        override fun emit() {
+            dependencyCell.add("x")
+        }
+    }
+}

observable/src/commonTest/kotlin/world/phantasmal/observable/cell/FlatteningDependentCellWithSimpleListCellTests.kt

@@ -0,0 +1,17 @@
+package world.phantasmal.observable.cell
+
+import world.phantasmal.observable.cell.list.SimpleListCell
+
+class FlatteningDependentCellWithSimpleListCellTests : CellTests {
+    override fun createProvider() = Provider()
+
+    class Provider : CellTests.Provider {
+        private val dependencyCell = SimpleListCell(mutableListOf("a", "b", "c"))
+
+        override val observable = FlatteningDependentCell(dependencyCell) { dependencyCell }
+
+        override fun emit() {
+            dependencyCell.add("x")
+        }
+    }
+}

observable/src/commonTest/kotlin/world/phantasmal/observable/cell/MutableCellTests.kt

@@ -1,9 +1,6 @@
 package world.phantasmal.observable.cell
 
-import world.phantasmal.observable.ChangeEvent
-import world.phantasmal.observable.Dependency
 import world.phantasmal.observable.Dependent
-import world.phantasmal.observable.change
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertNull

observable/src/commonTest/kotlin/world/phantasmal/observable/cell/RegularCellTests.kt

@@ -9,32 +9,6 @@ import kotlin.test.*
 interface RegularCellTests : CellTests {
     fun <T> createWithValue(value: T): Cell<T>
 
-    /**
-     * [Cell.value] should correctly reflect changes even when the [Cell] has no observers.
-     * Typically this means that the cell's value is not updated in real time, only when it is
-     * queried.
-     */
-    @Test
-    fun reflects_changes_without_observers() = test {
-        val p = createProvider()
-
-        var old: Any?
-
-        repeat(5) {
-            // Value should change after emit.
-            old = p.observable.value
-
-            p.emit()
-
-            val new = p.observable.value
-
-            assertNotEquals(old, new)
-
-            // Value should not change when emit hasn't been called since the last access.
-            assertEquals(new, p.observable.value)
-        }
-    }
-
     @Test
     fun convenience_methods() = test {
         listOf(Any(), null).forEach { any ->