2.1 family doesn't get updated

[ernespn]

Hi Simon,
I think I found a bug in the 2.1 version.
So I have a extension that check if a certain family is in a position:

fun Position.findEntitiesInPosition(family: Family): List<Entity> {
    val entities = mutableListOf<Entity>()
    family.forEach {
        if (it.has(Position)
            && it[Position].x == x
            && it[Position].y == y
        ) {
            entities.add(it)
        }
    }
    return entities
}

I use this to see if I can move to a target position

class MoveSystem(
    private val maxWidth:Int = inject("maxWidth"),
    private val maxHeight:Int = inject("maxHeight")
) : IteratingSystem(family { all(Position, Moving).none(Dead)}) {
    override fun onTickEntity(entity: Entity) {
        val originalPosition = entity[Position]
        val targetPosition = entity[Moving].target
        // position is within the limit and position is not occupied
        if (targetPosition.positionWithin(maxWidth, maxHeight)
            && targetPosition.findEntitiesInPosition(world.family { all(Position, Solid) }).isEmpty()){
            originalPosition.x = targetPosition.x
            originalPosition.y = targetPosition.y
        }

        entity.configure { it -= Moving }
    }

So for the version 2.0 this was working fine but I recently upgraded to 2.1 and now it is failing the test below:

    @Test
    fun shouldMoveIntoADeadObject(){
        val x = 5
        val y = 5
        val original = Position(x, y)
        val target = Position(8,8)
        val entity = world.entity {
            it += original
            it += Moving(target)
        }

        val toDie = world.entity{
            it += target
            it += Solid()
        }

        world.update(1f)
        // Check I cannot move
        Assertions.assertEquals(original, Position(x,y))
        Assertions.assertNotEquals(original, target)

        with(world){
            toDie.configure {
                it -= Solid
                it += Dead()
            }
            entity.configure { it += Moving(target) }
        }

        world.update(1f)
        // Check it should have moved
        Assertions.assertEquals(original, target)
    }

    

[Quillraven]

Hm I wouldn't know which change was introduced in 2.1 that could cause that 😅 weird.

Anyway, can you let me know please which assertion exactly is failing? Also, which family doesn't get updated? The one from the system or the one that you create inside the onTickEntity method?

Also, with 2.1 there is a filter function that you can use to remove your findEntitiesInPosition function. But first let's find out what is not working and why 😉

edit: please also add your component code so that I can reproduce your scenario.

[ernespn]

It is the last assertions

// Check it should have moved
Assertions.assertEquals(original, target)

The family still returns the toDie entity as it still have the Solid component but if I inspect the entity doesn't have it
I have modified the system to see the entity snapshot with:

class MoveSystem(
    private val maxWidth:Int = inject("maxWidth"),
    private val maxHeight:Int = inject("maxHeight")
) : IteratingSystem(family { all(Position, Moving).none(Dead)}) {
    override fun onTickEntity(entity: Entity) {
        val originalPosition = entity[Position]
        val targetPosition = entity[Moving].target
        // position is within the limit and position is not occupied
        val entityInTarget = targetPosition.findEntitiesInPosition(world.family { all(Position, Solid) })
        entityInTarget.forEach { println(world.snapshotOf(it)) }
        if (targetPosition.positionWithin(maxWidth, maxHeight)
            && targetPosition.findEntitiesInPosition(world.family { all(Position, Solid) }).isEmpty()){
            originalPosition.x = targetPosition.x
            originalPosition.y = targetPosition.y
        }

        entity.configure { it -= Moving }
    }

And getting:
This one before removing the Solid
[com.nestosoft.spacerl.gamelib.components.attributes.Solid@d62fe5b, Position(x=8, y=8)]
This one after removing the Solid
[com.nestosoft.spacerl.gamelib.components.attributes.Dead@539d019, Position(x=8, y=8)]

[ernespn]

Components:

data class Position(
    var x: Int,
    var y: Int,
) : Component<Position> {
    constructor(pos: Position) : this(pos.x, pos.y)

    override fun type(): ComponentType<Position> = Position
    companion object : ComponentType<Position>()

    fun equals(other: Position): Boolean {
        return x == other.x && y == other.y
    }
}
class Solid: Component<Solid> {
    override fun type(): ComponentType<Solid> = Solid
    companion object : ComponentType<Solid>()
}
class Dead: Component<Dead> {
    override fun type(): ComponentType<Dead> = Dead
    companion object : ComponentType<Dead>()

}
data class Moving(val target: Position) : Component<Moving> {
    override fun type(): ComponentType<Moving> = Moving
    companion object : ComponentType<Moving>()
}

[Quillraven]

I also need your "positionWithin" function please and maxWidth/maxHeight

[ernespn]

sure!

fun Position.positionWithin(widthLimit:Int, heightLimit:Int):Boolean {
    return x in 0 until widthLimit
            && y in 0 until heightLimit
}

class MoveSystemTest {
    private val width = 100
    private val height = 100
    private val world = world {
        systems {
            add(MoveSystem(width,height))
        }
    }

[Quillraven]

It is indeed a bug that got introduced with 2.1 but it only happens, when family iteration is happening in a nested case like you have it.

Your system is iterating over its own family and then you are also iterating over the "special" family that you retrieve from your world.

Unfortunately, I broke something in this scenario when trying to cleanup some code and avoid some code duplication. I will have a look at that within this week (I hope) and try to release a fixed version.

---

Additional info for me (and of course you if you are interested!):

• In 2.1 I introduced a lot of standard collection functionality for entities in the EntityBag/MutableEntityBag classes (before it was just an IntBag)
• Inside a family we use the bag for entity iteration and due to performance this information is stored in two separate variables
  • we have an entities BitArray which is immediately updated when an entity becomes a part of the family or gets removed. The BitArray is very fast to insert/remove entities and to check if an entity is part of the family
  • we have an entities bag which is NOT updated immediately because its main purpose is for iteration. The bag is way faster for iteration (=array like) when compare to the BitArray but is way slower for the other things mentioned above
• Therefore, it is necessary to update the entities bag "manually" when needed. We also need to be careful to not update the entities bag when an iteration is currently in progress over those entities
• in the latest version this manual update was broken when it comes to nested operations
• the important function that needs to be called is updateActiveEntities

In some cases like in forEach this method was called all the time while in other functions it was surrounded by a condition !entityService.delayRemoval || entitiesBag.isEmpty. I thought I solved that in a cleaner way and simply moved this inside the getter of the bag:

@PublishedApi
    internal val mutableEntities = MutableEntityBag()
        get() {
            if (!entityService.delayRemoval || field.isEmpty()) {
                // no iteration in process -> update entities if necessary
                updateActiveEntities(field)
            }
            return field
        }

BUT, this is not correct because e.g. in a nested forEach the updatedActiveEntities is not called and therefore the family not updated. However, it is safe to update the nested family because there is no iteration happening over that family.

What I need to do:

• write a test case for this scenario that will fail
• fix the problem (either by reverting those changes or fixing the new approach. Preferably the later one to avoid the code duplication)
• release bugfix version: most likely 2.1a

[Quillraven]

version 2.2 should be soon available on maven