From b1b72e7a40dc9fe1b23b441d7b25684808aac9e8 Mon Sep 17 00:00:00 2001 From: James Roome Date: Fri, 5 Jul 2024 10:22:34 +0530 Subject: [PATCH] Ability to add systems to the world after world creation (#145) --- .../com/github/quillraven/fleks/system.kt | 14 +- .../com/github/quillraven/fleks/world.kt | 201 +++++++++++----- .../quillraven/fleks/FamilySystemHookTest.kt | 227 +++++++++++++++++- .../com/github/quillraven/fleks/WorldTest.kt | 73 +++++- 4 files changed, 436 insertions(+), 79 deletions(-) diff --git a/src/commonMain/kotlin/com/github/quillraven/fleks/system.kt b/src/commonMain/kotlin/com/github/quillraven/fleks/system.kt index 91c0d26..10d2dc3 100644 --- a/src/commonMain/kotlin/com/github/quillraven/fleks/system.kt +++ b/src/commonMain/kotlin/com/github/quillraven/fleks/system.kt @@ -160,8 +160,18 @@ abstract class IteratingSystem( protected val comparator: EntityComparator = EMPTY_COMPARATOR, protected val sortingType: SortingType = Automatic, interval: Interval = EachFrame, - enabled: Boolean = true -) : IntervalSystem(interval, enabled) { + enabled: Boolean = true, + world: World +) : IntervalSystem(interval, enabled, world) { + + constructor( + family: Family, + comparator: EntityComparator = EMPTY_COMPARATOR, + sortingType: SortingType = Automatic, + interval: Interval = EachFrame, + enabled: Boolean = true, + ) : this(family, comparator, sortingType, interval, enabled, World.CURRENT_WORLD ?: throw FleksWrongConfigurationUsageException()) + /** * Flag that defines if sorting of [entities][Entity] will be performed the next time [onTick] is called. * diff --git a/src/commonMain/kotlin/com/github/quillraven/fleks/world.kt b/src/commonMain/kotlin/com/github/quillraven/fleks/world.kt index 6e93aaf..b0582dd 100644 --- a/src/commonMain/kotlin/com/github/quillraven/fleks/world.kt +++ b/src/commonMain/kotlin/com/github/quillraven/fleks/world.kt @@ -55,7 +55,7 @@ class InjectableConfiguration(private val world: World) { */ @WorldCfgMarker class SystemConfiguration( - internal val systems: MutableList = mutableListOf() + private val systems: MutableList ) { /** * Adds the [system] to the [world][World]. @@ -171,75 +171,25 @@ class WorldConfiguration(@PublishedApi internal val world: World) { fun configure() { injectableCfg?.invoke(InjectableConfiguration(world)) familyCfg?.invoke(FamilyConfiguration(world)) - SystemConfiguration().also { + SystemConfiguration(world.mutableSystems).also { systemCfg?.invoke(it) - // assign world systems afterward to resize the systems array only once to the correct size - // instead of resizing every time a system gets added to the configuration - world.systems = it.systems.toTypedArray() } if (world.numEntities > 0) { throw FleksWorldModificationDuringConfigurationException() } - setUpAggregatedFamilyHooks() - - world.systems.forEach { it.onInit() } - } - - /** - * Extend [Family.addHook] and [Family.removeHook] with - * other objects that needed to triggered by the hooks. - */ - private fun setUpAggregatedFamilyHooks() { - - // validate systems against illegal interfaces - world.systems.forEach { system -> - // FamilyOnAdd and FamilyOnRemove interfaces are only meant to be used by IteratingSystem - if (system !is IteratingSystem) { - - if (system is FamilyOnAdd) { - throw FleksWrongSystemInterfaceException(system::class, FamilyOnAdd::class) - } - - if (system is FamilyOnRemove) { - throw FleksWrongSystemInterfaceException(system::class, FamilyOnRemove::class) - } - } + // add all the configured family hooks to the cache + world.allFamilies.forEach { + world.worldCfgFamilyAddHooks[it] = it.addHook + world.worldCfgFamilyRemoveHooks[it] = it.removeHook } - // register family hooks for IteratingSystem.FamilyOnAdd containing systems - world.systems - .mapNotNull { if (it is IteratingSystem && it is FamilyOnAdd) it else null } - .groupBy { it.family } - .forEach { entry -> - val (family, systemList) = entry - val ownHook = family.addHook - val systemArray = systemList.toTypedArray() - family.addHook = if (ownHook != null) { entity -> - ownHook(world, entity) - systemArray.forEach { it.onAddEntity(entity) } - } else { entity -> - systemArray.forEach { it.onAddEntity(entity) } - } - } + world.initAggregatedFamilyHooks() - // register family hooks for IteratingSystem.FamilyOnRemove containing systems - world.systems - .mapNotNull { if (it is IteratingSystem && it is FamilyOnRemove) it else null } - .groupBy { it.family } - .forEach { entry -> - val (family, systemList) = entry - val ownHook = family.removeHook - val systemArray = systemList.toTypedArray() - family.removeHook = if (ownHook != null) { entity -> - systemArray.forEachReverse { it.onRemoveEntity(entity) } - ownHook(world, entity) - } else { entity -> - systemArray.forEachReverse { it.onRemoveEntity(entity) } - } - } + world.systems.forEach { it.onInit() } } + } /** @@ -326,8 +276,82 @@ class World internal constructor( /** * Returns the world's systems. */ - var systems = emptyArray() - internal set + internal val mutableSystems = arrayListOf() + val systems: List + get() = mutableSystems + + /** + * Map of add [FamilyHook] out of the [WorldConfiguration]. + * Only used if there are also aggregated system hooks for the family to remember + * its original world configuration hook (see [initAggregatedFamilyHooks] and [updateAggregatedFamilyHooks]). + */ + val worldCfgFamilyAddHooks = mutableMapOf() + + /** + * Map of remove [FamilyHook] out of the [WorldConfiguration]. + * Only used if there are also aggregated system hooks for the family to remember + * its original world configuration hook (see [initAggregatedFamilyHooks] and [updateAggregatedFamilyHooks]). + */ + val worldCfgFamilyRemoveHooks = mutableMapOf() + + /** + * Adds a new system to the world. + * + * @param index The position at which the system should be inserted in the list of systems. If null, the system is added at the end of the list. + * This parameter is optional and defaults to null. + * @param system The system to be added to the world. This should be an instance of a class that extends IntervalSystem. + * + * @throws FleksSystemAlreadyAddedException if the system was already added before. + */ + fun add(index: Int, system: IntervalSystem) { + if (systems.any { it::class == system::class }) { + throw FleksSystemAlreadyAddedException(system::class) + } + + mutableSystems.add(index, system) + + if (system is IteratingSystem && (system is FamilyOnAdd || system is FamilyOnRemove)) { + updateAggregatedFamilyHooks(system.family) + } + } + + /** + * Adds a new system to the world. + * + * @param system The system to be added to the world. This should be an instance of a class that extends IntervalSystem. + */ + fun add(system: IntervalSystem) = add(systems.size, system) + + /** + * Removes the specified system from the world. + * + * @param system The system to be removed from the world. This should be an instance of a class that extends IntervalSystem. + * @return True if the system was successfully removed, false otherwise. + */ + fun remove(system: IntervalSystem) { + mutableSystems.remove(system) + if (system is IteratingSystem && (system is FamilyOnAdd || system is FamilyOnRemove)) { + updateAggregatedFamilyHooks(system.family) + } + } + + /** + * Adds a new system to the world using the '+=' operator. + * + * @param system The system to be added to the world. This should be an instance of a class that extends IntervalSystem. + * + * @throws FleksSystemAlreadyAddedException if the system was already added before. + */ + operator fun plusAssign(system: IntervalSystem) = add(system) + + /** + * Removes the specified system from the world using the '-=' operator. + * + * @param system The system to be removed from the world. This should be an instance of a class that extends IntervalSystem. + */ + operator fun minusAssign(system: IntervalSystem) { + remove(system) + } /** * Cache of used [EntityTag] instances. Needed for snapshot functionality. @@ -620,7 +644,8 @@ class World internal constructor( */ fun update(deltaTime: Float) { this.deltaTime = deltaTime - systems.forEach { system -> + for (i in systems.indices) { + val system = systems[i] if (system.enabled) { system.onUpdate() } @@ -636,6 +661,56 @@ class World internal constructor( systems.forEachReverse { it.onDispose() } } + /** + * Extend [Family.addHook] and [Family.removeHook] for all + * [systems][IteratingSystem] that implement [FamilyOnAdd] and/or [FamilyOnRemove]. + */ + internal fun initAggregatedFamilyHooks() { + // validate systems against illegal interfaces + systems.forEach { system -> + // FamilyOnAdd and FamilyOnRemove interfaces are only meant to be used by IteratingSystem + if (system !is IteratingSystem) { + if (system is FamilyOnAdd) { + throw FleksWrongSystemInterfaceException(system::class, FamilyOnAdd::class) + } + if (system is FamilyOnRemove) { + throw FleksWrongSystemInterfaceException(system::class, FamilyOnRemove::class) + } + } + } + + allFamilies.forEach { updateAggregatedFamilyHooks(it) } + } + + /** + * Update [Family.addHook] and [Family.removeHook] for all + * [systems][IteratingSystem] that implement [FamilyOnAdd] and/or [FamilyOnRemove] + * and iterate over the given [family]. + */ + private fun updateAggregatedFamilyHooks(family: Family) { + // system validation like in initAggregatedFamilyHooks is not necessary + // because it is already validated before (in initAggregatedFamilyHooks and in add/remove system) + + // update family add hook by adding systems' onAddEntity calls after its original world cfg hook + val ownAddHook = worldCfgFamilyAddHooks[family] + val addSystems = systems.filter { it is IteratingSystem && it is FamilyOnAdd && it.family == family } + family.addHook = if (ownAddHook != null) { entity -> + ownAddHook(this, entity) + addSystems.forEach { (it as FamilyOnAdd).onAddEntity(entity) } + } else { entity -> + addSystems.forEach { (it as FamilyOnAdd).onAddEntity(entity) } + } + + // update family remove hook by adding systems' onRemoveEntity calls before its original world cfg hook + val ownRemoveHook = worldCfgFamilyRemoveHooks[family] + val removeSystems = systems.filter { it is IteratingSystem && it is FamilyOnRemove && it.family == family } + family.removeHook = if (ownRemoveHook != null) { entity -> + removeSystems.forEachReverse { (it as FamilyOnRemove).onRemoveEntity(entity) } + ownRemoveHook(this, entity) + } else { entity -> + removeSystems.forEachReverse { (it as FamilyOnRemove).onRemoveEntity(entity) } + } + } @ThreadLocal companion object { @@ -670,7 +745,7 @@ class World internal constructor( } } -private inline fun Array.forEachReverse(action: (T) -> Unit) { +private inline fun List.forEachReverse(action: (T) -> Unit) { val lastIndex = this.lastIndex for (i in lastIndex downTo 0) { action(this[i]) diff --git a/src/commonTest/kotlin/com/github/quillraven/fleks/FamilySystemHookTest.kt b/src/commonTest/kotlin/com/github/quillraven/fleks/FamilySystemHookTest.kt index a738982..0a8d314 100644 --- a/src/commonTest/kotlin/com/github/quillraven/fleks/FamilySystemHookTest.kt +++ b/src/commonTest/kotlin/com/github/quillraven/fleks/FamilySystemHookTest.kt @@ -1,36 +1,64 @@ package com.github.quillraven.fleks import com.github.quillraven.fleks.World.Companion.family -import kotlin.test.Test -import kotlin.test.assertFailsWith -import kotlin.test.assertTrue +import kotlin.test.* private class SimpleTestComponent : Component { companion object : ComponentType() override fun type() = SimpleTestComponent } -private class OnAddHookSystem : IteratingSystem( - family = family { all(SimpleTestComponent) } +private class OnAddHookSystem(world: World? = null) : IteratingSystem( + world = world ?: World.CURRENT_WORLD!!, family = world?.family { all(SimpleTestComponent) } ?: family { all(SimpleTestComponent) } ), FamilyOnAdd { - var addEntityHandled = false + val addEntityHandled + get() = addEntityHandledCount > 0 + var addEntityHandledCount = 0 override fun onAddEntity(entity: Entity) { - addEntityHandled = true + addEntityHandledCount++ } override fun onTickEntity(entity: Entity) = Unit } -private class OnRemoveHookSystem : IteratingSystem( - family = family { all(SimpleTestComponent) } +private class OnAddHookSystem2(world: World? = null) : IteratingSystem( + world = world ?: World.CURRENT_WORLD!!, family = world?.family { all(SimpleTestComponent) } ?: family { all(SimpleTestComponent) } +), FamilyOnAdd { + + var addEntityHandledCount = 0 + + override fun onAddEntity(entity: Entity) { + addEntityHandledCount++ + } + + override fun onTickEntity(entity: Entity) = Unit +} + +private class OnAddHookSystem3(world: World? = null) : IteratingSystem( + world = world ?: World.CURRENT_WORLD!!, family = world?.family { all(SimpleTestComponent) } ?: family { all(SimpleTestComponent) } +), FamilyOnAdd { + + var addEntityHandledCount = 0 + + override fun onAddEntity(entity: Entity) { + addEntityHandledCount++ + } + + override fun onTickEntity(entity: Entity) = Unit +} + +private class OnRemoveHookSystem(world: World? = null) : IteratingSystem( + world = world ?: World.CURRENT_WORLD!!, family = world?.family { all(SimpleTestComponent) } ?: family { all(SimpleTestComponent) } ), FamilyOnRemove { - var removeEntityHandled = false + val removeEntityHandled + get() = removeEntityHandledCount > 0 + var removeEntityHandledCount = 0 override fun onRemoveEntity(entity: Entity) { - removeEntityHandled = true + removeEntityHandledCount++ } override fun onTickEntity(entity: Entity) = Unit @@ -62,6 +90,34 @@ internal class FamilySystemHookTest { assertTrue { system.addEntityHandled } } + @Test + fun onAddHookSystemPostConfiguration() { + val world = configureWorld {}.also { + it.add(OnAddHookSystem(world = it)) + } + val system = world.system() + + // add an entity after the system is added, should trigger the onAddEntity hook + world.entity { it += SimpleTestComponent() } + assertEquals(1, system.addEntityHandledCount) + + // remove system + world -= system + + // add an entity after the system is removed, should not trigger the onAddEntity hook + world.entity { it += SimpleTestComponent() } + assertEquals(1, system.addEntityHandledCount) + + // add the system back, existing entities don't trigger the hook + world += system + assertNotNull(system.family.addHook) + assertEquals(1, system.addEntityHandledCount) + + // add an entity after the system is added back, should trigger the onAddEntity hook + world.entity { it += SimpleTestComponent() } + assertEquals(2, system.addEntityHandledCount) + } + @Test fun onRemoveHookSystem() { val world = configureWorld { @@ -77,6 +133,155 @@ internal class FamilySystemHookTest { assertTrue { system.removeEntityHandled } } + @Test + fun onRemoveHookSystemPostConfiguration() { + val world = configureWorld {}.also { + it.add(OnRemoveHookSystem(world = it)) + } + val system = world.system() + + val entity1 = world.entity { it += SimpleTestComponent() } + val entity2 = world.entity { it += SimpleTestComponent() } + val entity3 = world.entity { it += SimpleTestComponent() } + + // remove entity after the system is added, should trigger the onRemoveEntity hook + world -= entity1 + assertEquals(1, system.removeEntityHandledCount) + + // remove system + world -= system + + // remove an entity after the system is removed, should not trigger the onRemoveEntity hook + world -= entity2 + assertEquals(1, system.removeEntityHandledCount) + + // add the system back, existing entities don't trigger the hook + world += system + assertNotNull(system.family.removeHook) + assertEquals(1, system.removeEntityHandledCount) + + // add an entity after the system is added back, should trigger the onAddEntity hook + world -= entity3 + assertEquals(2, system.removeEntityHandledCount) + } + + + @Test + fun onHooksConfigureAndSystemHooksConfiguration() { + var onAddCount = 0 + var onRemoveCount = 0 + val world = configureWorld { + families { + // hook that reacts on entities that have a MoveComponent and do not have a DeadComponent + val family = family { all(SimpleTestComponent) } + onAdd(family) { _ -> + onAddCount++ + } + onRemove(family) { _ -> + onRemoveCount++ + } + } + }.also { + it.add(OnAddHookSystem(world = it)) + } + val systemAddedAfterConfigure = world.system() + + // add an entity after the system is added, should trigger the onAddEntity hook + world.entity { it += SimpleTestComponent() } + assertEquals(1, systemAddedAfterConfigure.addEntityHandledCount) + assertEquals(1, onAddCount) + + // remove system + world -= systemAddedAfterConfigure + + // add an entity after the system is removed, should not trigger the onAddEntity hook + val addedEntity = world.entity { it += SimpleTestComponent() } + assertEquals(1, systemAddedAfterConfigure.addEntityHandledCount) + assertEquals(2, onAddCount) + + // add the system back, existing entities don't trigger the hook + world += systemAddedAfterConfigure + assertEquals(1, systemAddedAfterConfigure.addEntityHandledCount) + assertEquals(2, onAddCount) + + // add an entity after the system is added back, should trigger the onAddEntity hook + world.entity { it += SimpleTestComponent() } + assertEquals(2, systemAddedAfterConfigure.addEntityHandledCount) + assertEquals(3, onAddCount) + + // let's test the onRemove hook + world -= addedEntity + assertEquals(1, onRemoveCount) + } + + @Test + fun testWorldAndSystemHooksScenario() { + // add a family hook during world configuration + var worldOnAddCount = 0 + val world = configureWorld { + families { + onAdd(family { all(SimpleTestComponent) }) { _ -> + worldOnAddCount++ + } + } + systems { + add(OnAddHookSystem3()) + } + } + + // add a FamilyOnAdd interface to system A that iterates over the same family of the previous step. + val systemA = OnAddHookSystem(world).also { + world += it + } + + // do the same thing again for system B and call it systemBHook + val systemB = OnAddHookSystem2(world).also { + world += it + } + + val systemC = world.system() + + // remove system A + world -= systemA + + // trigger an update and confirm that the onAdd is called for all systems except system A + world.entity { it += SimpleTestComponent() } + assertEquals(1, worldOnAddCount) + assertEquals(0, systemA.addEntityHandledCount) + assertEquals(1, systemB.addEntityHandledCount) + assertEquals(1, systemC.addEntityHandledCount) + + // put system A back in + world += systemA + + // trigger an update and confirm that the onAdd is called for all systems + world.entity { it += SimpleTestComponent() } + assertEquals(2, worldOnAddCount) + assertEquals(1, systemA.addEntityHandledCount) + assertEquals(2, systemB.addEntityHandledCount) + assertEquals(2, systemC.addEntityHandledCount) + + // remove system C + world -= systemC + + // trigger an update and confirm that the onAdd is called for all systems except system C + world.entity { it += SimpleTestComponent() } + assertEquals(3, worldOnAddCount) + assertEquals(2, systemA.addEntityHandledCount) + assertEquals(3, systemB.addEntityHandledCount) + assertEquals(2, systemC.addEntityHandledCount) + + // put system C back in + world += systemC + + // trigger an update and confirm that the onAdd is called for all systems + world.entity { it += SimpleTestComponent() } + assertEquals(4, worldOnAddCount) + assertEquals(3, systemA.addEntityHandledCount) + assertEquals(4, systemB.addEntityHandledCount) + assertEquals(3, systemC.addEntityHandledCount) + } + @Test fun illegalOnAddHookSystem() { assertFailsWith { diff --git a/src/commonTest/kotlin/com/github/quillraven/fleks/WorldTest.kt b/src/commonTest/kotlin/com/github/quillraven/fleks/WorldTest.kt index 62c37b9..52c97d3 100644 --- a/src/commonTest/kotlin/com/github/quillraven/fleks/WorldTest.kt +++ b/src/commonTest/kotlin/com/github/quillraven/fleks/WorldTest.kt @@ -36,7 +36,7 @@ private class WorldTestComponent2 : Component { companion object : ComponentType() } -private class WorldTestIntervalSystem : IntervalSystem() { +private class WorldTestIntervalSystem(world: World = World.CURRENT_WORLD!!) : IntervalSystem(world = world) { var numCalls = 0 var disposed = false @@ -50,8 +50,9 @@ private class WorldTestIntervalSystem : IntervalSystem() { } private class WorldTestIteratingSystem( - val testInject: String = inject() -) : IteratingSystem(family { all(WorldTestComponent) }) { + world: World = World.CURRENT_WORLD!!, + val testInject: String = world.inject() +) : IteratingSystem(world = world, family = world.family { all(WorldTestComponent) }) { var numCalls = 0 var numCallsEntity = 0 @@ -1026,4 +1027,70 @@ internal class WorldTest { assertEquals(1, numAdd) } } + + @Test + fun addingASystemAfterWorldConfiguration() { + val world = configureWorld { + injectables { + add("42") + } + } + world.add(WorldTestIteratingSystem(world = world)) + + assertNotNull(world.system()) + } + + @Test + fun addingASystemAtIndexAfterWorldConfiguration() { + val world = configureWorld { + injectables { + add("42") + } + + systems { + add(WorldTestIntervalSystem()) + } + } + val system = WorldTestIteratingSystem(world = world) + world.add(0, system) + + assertEquals(system, world.systems[0]) + } + + @Test + fun addingASystemPlusAssignAfterWorldConfiguration() { + val world = configureWorld { + injectables { + add("42") + } + } + world += WorldTestIteratingSystem(world = world) + + assertNotNull(world.system()) + } + + @Test + fun removingASystemAfterWorldConfiguration() { + val world = configureWorld { + injectables { + add("42") + } + } + + // add 2 systems + val system1 = WorldTestIteratingSystem(world = world) + val system2 = WorldTestIntervalSystem(world = world) + world.add(system1) + world.add(system2) + assertEquals(2, world.systems.size) + + // remove using remove function + world.remove(system2) + assertEquals(1, world.systems.size) + assertEquals(system1, world.systems[0]) + + // remove using minusAssign operator + world -= system1 + assertEquals(0, world.systems.size) + } }