GitHub Decentralized Repo
GitHub Centralized Repo
GitHub Flat Repo

And on the sixth day, it came to pass that the cat1 did approach her chief servant and did loudly demand her tribute. Arising, her servant prepared a sumptuous repast, and the cat did eat and there was again peace upon the land.


Some design thinking leads the cat’s servant to think we can improve our nascent ECS component.

In yesterday’s first Entity-Component-System experiment, I added a component dynamically to a space object, and then removed it when its job was done. This required that I buffer the removal, because you dasn’t remove things from a list while iterating it. It looks like this:

fun removeComponent(entity: SpaceObject, component: Component) {
    removed.add(component)
}

    for (spaceObject in spaceObjects) {
        removed = mutableListOf()
        for (component in spaceObject.components) {
            update(component, deltaTime)
        }
        spaceObject.components.removeAll(removed)
        removed.forEach { spaceObject.components.remove(it)}
        if (spaceObject.type == SpaceObjectType.SHIP) {
            applyControls(spaceObject, deltaTime)
        }
    ...

This is especially nasty with our current scheme of not giving our objects any methods. I am tiring of that constraint, bye the way. In this case, there is shared knowledge between the game and the components and space objects, all of which know a little bit about removing components.

Let’s try a new rule: components can be added, and not removed. We’ll try to move toward a situation where an entity is created with its components in place and thereafter none are added, nor are any removed. For now, we’ll follow the no removals rule and worry about the adding in due time.

If the Timer is not to be removed, then clearly each time the missile is fired, the Timer needs to start anew. I think this means that the timer needs to know its starting value, and, as Dirk suggested yesterday, it needs to operate only if the missile is active. I think we’ll find that setting the starting value back into the timing value at the point where we inactivate the object will be just right.

I also think it is time to give the Component its own file, especially since it is actually an interface. The file gets this:

interface Component { val entity: SpaceObject }

data class Timer(override val entity: SpaceObject, val startTime: Double): Component {
    var time = startTime
}

The update is presently this:

fun update(component: Component, deltaTime: Double) {
    when (component) {
        is Timer -> {
            component.time -= deltaTime
            if (component.time <= 0.0) {
                component.entity.active = false
                removeComponent(component.entity, component)
            }
        }
    }
}

And it needs to be more like this:

fun update(component: Component, deltaTime: Double) {
    when (component) {
        is Timer -> {
            if ( ! component.entity.active) return
            component.time -= deltaTime
            if (component.time <= 0.0) {
                component.entity.active = false
                component.time = component.startTime
            }
        }
    }
}

Now I think there may be no calls to removeComponent. True. Remove it. Remove the mess in Game:

    for (spaceObject in spaceObjects) {
        for (component in spaceObject.components) {
            update(component, deltaTime)
        }
        ...

Even Homer sometimes nods.2

Did you notice up above that the code was removing the removed objects twice? Yours truly had patched in the removeAll and forgot to remove the loop. Both gone now.

Test. The tests pass, but in the game I notice something odd. Sometimes when I fire, the missile flashes onto the screen and then immediately dies. Other times they seem to work right. After a while, it seems that they don’t fire at all.

How could that happen? Let’s review the code.

fun update(component: Component, deltaTime: Double) {
    when (component) {
        is Timer -> {
            if ( ! component.entity.active) return
            component.time -= deltaTime
            if (component.time <= 0.0) {
                component.entity.active = false
                component.time = component.startTime
            }
        }
    }
}

Could we be interrupted between setting the entity inactive and resetting the time? I don’t think we can be interrupted at all, to be honest. I think the cycle runs to completion before it can possibly be called again. Let’s look at the firing logic:

fun fireMissile() {
    Controls.fire = false
    val missile: SpaceObject = availableShipMissile() ?: return
    val offset = Vector2(U.MissileOffset, 0.0).rotate(Ship.angle)
    missile.x = offset.x + Ship.x
    missile.y = offset.y + Ship.y
    val velocity = Vector2(U.MissileSpeed, 0.0).rotate(Ship.angle)
    missile.dx = velocity.x + Ship.dx
    missile.dy = velocity.y + Ship.dy
    addComponent(missile, Timer(missile, U.MissileTime))
    missile.active = true
}

fun availableShipMissile(): SpaceObject? {
    for ( i in 2..5) {
        if (!spaceObjects[i].active) return spaceObjects[i]
    }
    return null
}

Ah, the issue could be that we add the component … yes! We wind up with more than one timer. Since we don’t remove them, we should only add them once, when the missiles are created. Amusing bug.

fun createGame(missileCount: Int, asteroidCount: Int) {
    val objects = mutableListOf<SpaceObject>()
    for ( i in 1..missileCount) {
        objects.add(newMissile())
    }
    Ship = newShip()
    objects.add(Ship)
    spaceObjects = objects.toTypedArray()
}

private fun newMissile(): SpaceObject {
    return SpaceObject(SpaceObjectType.MISSILE, 0.0, 0.0, 0.0, 0.0, 0.0, false)
}

We’ll pop a timer in there when we create the missile.

private fun newMissile(): SpaceObject {
    return SpaceObject(SpaceObjectType.MISSILE, 0.0, 0.0, 0.0, 0.0, 0.0, false)
        .also { addComponent(it, Timer(it, 3.0)) }
}

Sweet. That should fix the bug. And it does. I’m not sure how I could write a test for that error. If I were at my best, I’d probably stop here and figure out a way. I’m apparently not at my best, because I’m not going to do it.

We can commit: Timer created when missile created, resets rather than removing itself.

Let’s reflect.

Reflection

So that went fairly well. The actual code was a simple change and went in without error. The bug, however, was really a bit insidious. What we should really have, to save us from ourselves, might be for the collection of components in the space object to be immutable, so that we simply can’t add to it on the fly. Instead, we’d have to create all the timers in a constructor for the specific kind of entity being created, and provide them at construction time. I’ll make a yellow sticky “Jira” note for that.

I am rather hating the rule that we only use top level functions in this version of the game, because methods on the components and other objects would be far more convenient. We’ll stick with the rule, though, because I do want to assess the whole program and compare it to the other versions.

Some of the code might be improved. Let’s try.

Improvement

Can we make this a bit nicer?

fun update(component: Component, deltaTime: Double) {
    when (component) {
        is Timer -> {
            if ( ! component.entity.active) return
            component.time -= deltaTime
            if (component.time <= 0.0) {
                component.entity.active = false
                component.time = component.startTime
            }
        }
    }
}

We could extract a function:

fun update(component: Component, deltaTime: Double) {
    when (component) {
        is Timer -> {
            updateTimer(component, deltaTime)
        }
    }
}

private fun updateTimer(component: Timer, deltaTime: Double) {
    if (!component.entity.active) return
    component.time -= deltaTime
    if (component.time <= 0.0) {
        component.entity.active = false
        component.time = component.startTime
    }
}

Let’s rename the parameter:

private fun updateTimer(timer: Timer, deltaTime: Double) {
    if (!timer.entity.active) return
    timer.time -= deltaTime
    if (timer.time <= 0.0) {
        timer.entity.active = false
        timer.time = timer.startTime
    }
}

Let’s use with:

private fun updateTimer(timer: Timer, deltaTime: Double) {
    with(timer) {
        if ( ! entity.active) return
        time -= deltaTime
        if (time <= 0.0) {
            entity.active = false
            time = timer.startTime
        }
    }
}

OK, that’s a bit less messy. Almost as good as it would look if it were a method on Timer.

Test. Green. Commit: Refactor update for clarity.

What else might we not like?

fun fireMissile() {
    Controls.fire = false
    val missile: SpaceObject = availableShipMissile() ?: return
    val offset = Vector2(U.MissileOffset, 0.0).rotate(Ship.angle)
    missile.x = offset.x + Ship.x
    missile.y = offset.y + Ship.y
    val velocity = Vector2(U.MissileSpeed, 0.0).rotate(Ship.angle)
    missile.dx = velocity.x + Ship.dx
    missile.dy = velocity.y + Ship.dy
    missile.active = true
}

This is certainly a bit nasty. It’d be better if the objects had a Vector2 instead of x and y. It’d be better if space objects could have methods. We could at least do this:

fun fireMissile() {
    Controls.fire = false
    val missile: SpaceObject = availableShipMissile() ?: return
    val offset = Vector2(U.MissileOffset, 0.0).rotate(Ship.angle)
    setPosition(missile, offset)
    val velocity = Vector2(U.MissileSpeed, 0.0).rotate(Ship.angle)
    setVelocity(missile, velocity)
    missile.active = true
}

private fun setPosition(missile: SpaceObject, offset: Vector2) {
    missile.x = offset.x + Ship.x
    missile.y = offset.y + Ship.y
}

private fun setVelocity(missile: SpaceObject, velocity: Vector2) {
    missile.dx = velocity.x + Ship.dx
    missile.dy = velocity.y + Ship.dy
}

And let’s be inlining the new values:

fun fireMissile() {
    Controls.fire = false
    val missile: SpaceObject = availableShipMissile() ?: return
    setPosition(missile, Vector2(U.MissileOffset, 0.0).rotate(Ship.angle))
    setVelocity(missile, Vector2(U.MissileSpeed, 0.0).rotate(Ship.angle))
    missile.active = true
}

So, that’s a bit better. Test. Commit: Refactor for clarity.

I have a weird idea. What if we could say this:

fun fireMissile() {
    Controls.fire = false
    withAvailableMissile { missile ->
        setPosition(missile, Vector2(U.MissileOffset, 0.0).rotate(Ship.angle))
        setVelocity(missile, Vector2(U.MissileSpeed, 0.0).rotate(Ship.angle))
        missile.active = true
    }
}

That would be pleasant. Let’s make that work:

fun withAvailableMissile(action: (SpaceObject) -> Unit) {
    for ( i in 2..5) {
        if (!spaceObjects[i].active) return action(spaceObjects[i])
    }
}

OK, I call that nice. Test, of course, then commit: Refactor to provide withAvailableMissile.

I think that’ll do (pig), so let’s sum up3.

Summary

We began with a design improvement to our ECS, avoiding removing a component, and moving in the direction of having components defined at the time an object is built and kept immutable thereafter. We’re not there yet, but we’re on the way.

There was a fairly nasty defect, when I left in the addition of the Timer component when assigning a missile, because a missile with more than one timer tends to time out instantly. I was unable to think of a test for that, or chose not to, but when we get the components immutable that defect will be impossible.

Then we did a fairly nice code improvement to the timer, using with to our benefit as well as an extracted function.

After that, a couple of extracts and then a very nice withAvailableMissile function that conditionally executes a block, resulted in a much improved fireMissile function.

I still miss having methods available but we’re getting decently clean code, and fairly compact code, so it’s not driving me mad do you hear me mad yet. Yet. Perhaps next time.

Come visit me then! And if you have questions or feedback, ping me. Thanks!


  1. We call her Kitty, or Miss Kitty. We don’t know her real name. They never tell. She is clearly a goddess in her own mind, and we assume that she is right. ↩

  2. No, not Simpson! Greek poet, wrote the Iliad and the Odyssey. That Homer. ↩

  3. Not calling you or me a pig. Calling back to the excellent movie Babe, where “That’ll do, pig, that’ll do” is the highest praise one could imagine. ↩