diff --git a/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeManager.kt b/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeManager.kt index 11ef2952..67a34b5d 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeManager.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/ChangeManager.kt @@ -4,19 +4,22 @@ object ChangeManager { private var currentChangeSet: ChangeSet? = null fun inChangeSet(block: () -> Unit) { - val existingChangeSet = currentChangeSet - val changeSet = existingChangeSet ?: ChangeSet().also { - currentChangeSet = it - } - - try { + // TODO: Figure out change set bug and enable change sets again. +// val existingChangeSet = currentChangeSet +// val changeSet = existingChangeSet ?: ChangeSet().also { +// currentChangeSet = it +// } +// +// try { block() - } finally { - if (existingChangeSet == null) { - currentChangeSet = null - changeSet.complete() - } - } +// } finally { +// if (existingChangeSet == null) { +// // Set to null so changed calls are turned into emitDependencyChanged calls +// // immediately instead of being deferred. +// currentChangeSet = null +// changeSet.complete() +// } +// } } fun changed(dependency: Dependency) { @@ -31,15 +34,24 @@ object ChangeManager { } private class ChangeSet { + private var completing = false private val changedDependencies: MutableList = mutableListOf() fun changed(dependency: Dependency) { + check(!completing) + changedDependencies.add(dependency) } fun complete() { - for (dependency in changedDependencies) { - dependency.emitDependencyChanged() + try { + completing = true + + for (dependency in changedDependencies) { + dependency.emitDependencyChanged() + } + } finally { + completing = false } } } 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 6299392f..014697f1 100644 --- a/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt +++ b/observable/src/commonMain/kotlin/world/phantasmal/observable/cell/AbstractDependentCell.kt @@ -31,8 +31,8 @@ abstract class AbstractDependentCell : AbstractCell(), Dependent { override fun emitDependencyChanged() { // Nothing to do because dependent cells emit dependencyChanged immediately. They don't - // defer this operation because they only change when there is no transaction or the current - // transaction is being committed. + // defer this operation because they only change when there is no change set or the current + // change set is being completed. } protected abstract fun dependenciesChanged() diff --git a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/MutableCellTests.kt b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/MutableCellTests.kt index 56f61f2b..72174e49 100644 --- a/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/MutableCellTests.kt +++ b/observable/src/commonTest/kotlin/world/phantasmal/observable/cell/MutableCellTests.kt @@ -29,114 +29,117 @@ interface MutableCellTests : CellTests { assertEquals(newValue, observedValue) } + // TODO: Figure out change set bug and enable change sets again. /** * Modifying mutable cells in a change set doesn't result in calls to * [Dependent.dependencyChanged] of their dependents until the change set is completed. */ - @Test - fun cell_changes_in_change_set_dont_immediately_produce_dependencyChanged_calls() = test { - val dependencies = (1..5).map { createProvider() } - - var dependencyMightChangeCount = 0 - var dependencyChangedCount = 0 - - val dependent = object : Dependent { - override fun dependencyMightChange() { - dependencyMightChangeCount++ - } - - override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { - dependencyChangedCount++ - } - } - - for (dependency in dependencies) { - dependency.observable.addDependent(dependent) - } - - change { - for (dependency in dependencies) { - dependency.observable.value = dependency.createValue() - } - - // Calls to dependencyMightChange happen immediately. - assertEquals(dependencies.size, dependencyMightChangeCount) - // Calls to dependencyChanged happen later. - assertEquals(0, dependencyChangedCount) - } - - assertEquals(dependencies.size, dependencyMightChangeCount) - assertEquals(dependencies.size, dependencyChangedCount) - } +// @Test +// fun cell_changes_in_change_set_dont_immediately_produce_dependencyChanged_calls() = test { +// val dependencies = (1..5).map { createProvider() } +// +// var dependencyMightChangeCount = 0 +// var dependencyChangedCount = 0 +// +// val dependent = object : Dependent { +// override fun dependencyMightChange() { +// dependencyMightChangeCount++ +// } +// +// override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { +// dependencyChangedCount++ +// } +// } +// +// for (dependency in dependencies) { +// dependency.observable.addDependent(dependent) +// } +// +// change { +// for (dependency in dependencies) { +// dependency.observable.value = dependency.createValue() +// } +// +// // Calls to dependencyMightChange happen immediately. +// assertEquals(dependencies.size, dependencyMightChangeCount) +// // Calls to dependencyChanged happen later. +// assertEquals(0, dependencyChangedCount) +// } +// +// assertEquals(dependencies.size, dependencyMightChangeCount) +// assertEquals(dependencies.size, dependencyChangedCount) +// } + // TODO: Figure out change set bug and enable change sets again. /** * Modifying a mutable cell multiple times in one change set results in a single call to * [Dependent.dependencyMightChange] and [Dependent.dependencyChanged]. */ - @Test - fun multiple_changes_to_one_cell_in_change_set() = test { - val dependency = createProvider() - - var dependencyMightChangeCount = 0 - var dependencyChangedCount = 0 - - val dependent = object : Dependent { - override fun dependencyMightChange() { - dependencyMightChangeCount++ - } - - override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { - dependencyChangedCount++ - } - } - - dependency.observable.addDependent(dependent) - - // Change the dependency multiple times in a transaction. - change { - repeat(5) { - dependency.observable.value = dependency.createValue() - } - - // Calls to dependencyMightChange happen immediately. - assertEquals(1, dependencyMightChangeCount) - // Calls to dependencyChanged happen later. - assertEquals(0, dependencyChangedCount) - } - - assertEquals(1, dependencyMightChangeCount) - assertEquals(1, dependencyChangedCount) - } +// @Test +// fun multiple_changes_to_one_cell_in_change_set() = test { +// val dependency = createProvider() +// +// var dependencyMightChangeCount = 0 +// var dependencyChangedCount = 0 +// +// val dependent = object : Dependent { +// override fun dependencyMightChange() { +// dependencyMightChangeCount++ +// } +// +// override fun dependencyChanged(dependency: Dependency, event: ChangeEvent<*>?) { +// dependencyChangedCount++ +// } +// } +// +// dependency.observable.addDependent(dependent) +// +// // Change the dependency multiple times in a transaction. +// change { +// repeat(5) { +// dependency.observable.value = dependency.createValue() +// } +// +// // Calls to dependencyMightChange happen immediately. +// assertEquals(1, dependencyMightChangeCount) +// // Calls to dependencyChanged happen later. +// assertEquals(0, dependencyChangedCount) +// } +// +// assertEquals(1, dependencyMightChangeCount) +// assertEquals(1, dependencyChangedCount) +// } + // TODO: Figure out change set bug and enable change sets again. /** * Modifying two mutable cells in a change set results in a single recomputation of their * dependent. */ - @Test - fun modifying_two_cells_together_results_in_one_recomputation() = test { - val dependency1 = createProvider() - val dependency2 = createProvider() - - var computeCount = 0 - - val dependent = DependentCell(dependency1.observable, dependency2.observable) { - computeCount++ - Unit - } - - // Observe dependent to ensure it gets recomputed when its dependencies change. - disposer.add(dependent.observe {}) - - // DependentCell's compute function is called once when we start observing. - assertEquals(1, computeCount) - - change { - dependency1.observable.value = dependency1.createValue() - dependency2.observable.value = dependency2.createValue() - } - - assertEquals(2, computeCount) - } +// @Test +// fun modifying_two_cells_together_results_in_one_recomputation() = test { +// val dependency1 = createProvider() +// val dependency2 = createProvider() +// +// var computeCount = 0 +// +// val dependent = DependentCell(dependency1.observable, dependency2.observable) { +// computeCount++ +// Unit +// } +// +// // Observe dependent to ensure it gets recomputed when its dependencies change. +// disposer.add(dependent.observe {}) +// +// // DependentCell's compute function is called once when we start observing. +// assertEquals(1, computeCount) +// +// change { +// dependency1.observable.value = dependency1.createValue() +// dependency2.observable.value = dependency2.createValue() +// } +// +// assertEquals(2, computeCount) +// } interface Provider : CellTests.Provider { override val observable: MutableCell