GitHub Decentralized Repo
GitHub Centralized Repo
GitHub Flat Repo

My “Jira” is some sticky notes on my keyboard tray. Maybe yours should be as well. Anyway there are some things we might do.

keyboard tray with stick notes

No, seriously. There are 20 notes on my tray right now, all about Asteroids, things that I “ought” to do or “should” do or “just don’t want to forget in case I run out of ideas”. We can be quite certain that all 20 of those will not be done, and that probably 20 things that are not on the tray will be done. Your Jira or equivalent, with 10 times, or 100 times, or save us 1000 times as many items is a waste of computer and human time.

But you’re not in charge of that, are you? So be it, it should be the worst thing that ever happens to any of us.

I do have some tests that can be revised and/or removed, and I have an idea that isn’t in my Jira. (See, I told you there’d be things done that aren’t even in there. And, unlike you, I don’t have to put them on the keyboard tray to be allowed to do them, or get credit for doing them. I just have to write a long article about them. (Wait, maybe you have a better deal after all!)) But I digress.

The idea is something like this:

  • We have two timer components now, Timer and SaucerTimer.
  • Timer deactivates its entity when it times out;
  • Saucer timer deactivates its entity if active, deactivates it otherwise.
  • If we just had ActivationTimer and DeactivationTimer, we could use the two for the saucer and probably the ship, and just the one for the missiles!

There is the tiny detail that activation is different for each object, but I’m confident that with the cat’s help and some bending of the rules, we can get past that.

So I think that would be neat. So I want to do it. But first

These Tests

    fun `start saucer after seven seconds`() {
    fun `saucer switches direction`() {
    fun `saucer clock resets on death`() {

All three of those are wrong now. They refer to old code that is no longer used in the actual game, and the starting and switching tests are covered, I believe, by yesterday’s tests of the SaucerTimer.

We do need to test all of those ideas, however.

Let’s review the new test, which covers some of the same ground. It’s rather nasty. I think I need a better idea.

    @Test
    fun `saucer timer`() {
        val saucer = newSaucer()
        saucer.position = Vector2(100.0, 100.0)
        saucer.velocity = Vector2(0.0, 0.0)
        val oldSpeed = saucerSpeed
        assertThat(saucer.active).isEqualTo(false)
        val timer = SaucerTimer(saucer)
        updateSaucerTimer(timer, 0.5)
        assertThat(saucer.active).isEqualTo(false)
        updateSaucerTimer(timer, U.SaucerDelay)
        assertThat(saucer.active).describedAs("should start by now").isEqualTo(true)
        assertThat(saucer.x).describedAs("should start on y axis").isEqualTo(0.0)
        assertThat(saucer.dx).describedAs("should start at oldSpeed").isEqualTo(oldSpeed)
        assertThat(saucerSpeed)
        	.describedAs("should negate saucerSpeed").isEqualTo(-oldSpeed)
    }

Let’s see what this does and then rename it appropriately. It checks:

  • saucer starts out inactive,
  • does not activate immediately,
  • activates after U.SaucerDelay seconds
  • starts on the Y axis,
  • starts at saucerSpeed
  • negates saucerSpeed

Kind of breaks the “one assert per test” rule, doesn’t it? Some more descriptions might help a bit.

    @Test
    fun `saucer timer`() {
        val saucer = newSaucer()
        saucer.position = Vector2(100.0, 100.0)
        saucer.velocity = Vector2(0.0, 0.0)
        val oldSpeed = saucerSpeed
        assertThat(saucer.active)
            .describedAs("should start inactive").isEqualTo(false)
        val timer = SaucerTimer(saucer)
        updateSaucerTimer(timer, 0.5)
        assertThat(saucer.active)
            .describedAs("should not activate immediately").isEqualTo(false)
        updateSaucerTimer(timer, U.SaucerDelay)
        assertThat(saucer.active)
            .describedAs("should start by now").isEqualTo(true)
        assertThat(saucer.x)
            .describedAs("should start on y axis").isEqualTo(0.0)
        assertThat(saucer.dx)
            .describedAs("should start at oldSpeed").isEqualTo(oldSpeed)
        assertThat(saucerSpeed)
            .describedAs("should negate saucerSpeed").isEqualTo(-oldSpeed)
    }

One Assertion or Many?

Why do smart people say that there should only be one assertion per test, and why do I ignore that advice so consistently? There are two reasons that I can think of.

Theory: Tests are for reading. We used to believe that tests formed good documentation of how the system works and that people would read them. If that’s the case, then having each test embed one little bit of information is arguably a good idea.

Theory: When a test breaks, you should immediately understand, from the test’s name, what is broken. A good theory as well, but the describedAs clause here provides the same information as the test name, maybe even more.

Theory: We should implement just one tiny bit of functionality at a time and thus one test at a time. Again a good theory, but setting up a whole test for each step of verifying, say, saucer activation, adds a lot of overhead.

Theory: High overhead in setting up a test is a sign that something needs improvement. Well, you’ve got me there. Let’s drill down on that for a moment, thinking about what we need to test with this component and its use.

Drill Down on SaucerTimer

When does it trigger?
We’d like to test that it does its action only after its designated time has elapsed, and that it resets its time such that when it’s used another time, it starts over.

Furthermore, as things stand now, the actual timer action is separate from the timer. That’s simply terrible:

    for (component in spaceObject.components) update(component, deltaTime)

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

fun updateSaucerTimer(timer: SaucerTimer, deltaTime: Double) {
    with(timer) {
        time -= deltaTime
        if (time <= 0.0) {
            time = U.SaucerDelay
            if (entity.active) {
                entity.active = false
            } else {
                entity.active = true
                entity.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
                entity.velocity = Vector2(saucerSpeed, 0.0)
                saucerSpeed *= -1.0
            }
        }
    }
}

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

The components themselves just have a time variable:

interface Component { val entity: SpaceObject }

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

data class SaucerTimer(override val entity: SpaceObject): Component {
    var time = U.SaucerDelay
}

I’m really hampered here by the rule I’ve imposed on this design, which is not to create any objects with methods. I’ve bent the rule a bit, by allowing use of built-in objects with methods, like Vector2, and by implementing properties on SpaceObject that translate Vector2 back and forth between x and y and the vector quantity.

If I had methods on my components, I think I’d give them a built-in update instead of updateTimer, which would mean that we’d just call update on each of them here:

    for (component in spaceObject.components) component.update(deltaTime)

And they’d do the rest.

Push: Danger, Will Robinson

I’m about three or four levels down in thinking and speculation about this. It’s getting to the point where I need to clear my stack and sort out a plan. But not yet.

Pop: Back to Drilling on SaucerTimer

As the test is written, we call updateSaucerTimer multiple times and then see if it has done the right thing. Remember, it’s kind of complicated:

fun updateSaucerTimer(timer: SaucerTimer, deltaTime: Double) {
    with(timer) {
        time -= deltaTime
        if (time <= 0.0) {
            time = U.SaucerDelay
            if (entity.active) {
                entity.active = false
            } else {
                entity.active = true
                entity.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
                entity.velocity = Vector2(saucerSpeed, 0.0)
                saucerSpeed *= -1.0
            }
        }
    }
}

It does a time check; it deactivates; it activates. Activation is tricky.

Pop Pop Pop …

Let’s pop all that and step back a bit. We won’t forget what we thought, but we won’t try to hold on to all the details. We’ll try to re-frame.

What if …

What if a Timer had a time that it counted down, and when the time elapsed, the Timer just reset the time and called an action? And what if the Timer had a way to be told to reset itself. (Maybe this is a method. We allow thinking about them and they’re just rules anyway.)

Timer would be easy to test. What about actions?

We’d really want to test the actions. If the action is to deactivate a saucer, we want to test that it does all the steps. So the action has to be a function or block that we can test.

We’d have to wind up with a function like activateSaucer to test and it would look like those four lines in updateSaucerTimer.

Wait! We could extract those now and test them. Let’s do that.

Test and Extract activateSaucer

    @Test
    fun `saucer activates properly`() {
        val saucer = newSaucer()
        val oldSpeed = saucerSpeed
        saucer.position = Vector2(100.0,200.0)
        saucer.velocity = Vector2(999.0,999.0)
        assertThat(saucer.active)
            .describedAs("starts inactive").isEqualTo(false)
        activateSaucer(saucer)
        assertThat(saucer.active)
            .describedAs("should be active").isEqualTo(true)
        assertThat(saucer.x)
            .describedAs("should start on y axis").isEqualTo(0.0)
        assertThat(saucer.dx)
            .describedAs("speed should be oldSpeed").isEqualTo(oldSpeed)
        assertThat(saucerSpeed)
            .describedAs("should negate saucerSpeed").isEqualTo(-oldSpeed)
    }

This isn’t so different from the other one but it directly tests activation. Extract that method:

fun updateSaucerTimer(timer: SaucerTimer, deltaTime: Double) {
    with(timer) {
        time -= deltaTime
        if (time <= 0.0) {
            time = U.SaucerDelay
            if (entity.active) {
                entity.active = false
            } else {
                activateSaucer()
            }
        }
    }
}

fun SaucerTimer.activateSaucer() {
    entity.active = true
    entity.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
    entity.velocity = Vector2(saucerSpeed, 0.0)
    saucerSpeed *= -1.0
}

I think my test should run. Wait. What is entity in that code? How did that even work? Oh, it extracted an extension. I don’t know why. I’ll fix it manually:

fun updateSaucerTimer(timer: SaucerTimer, deltaTime: Double) {
    with(timer) {
        time -= deltaTime
        if (time <= 0.0) {
            time = U.SaucerDelay
            if (entity.active) {
                entity.active = false
            } else {
                activateSaucer(entity)
            }
        }
    }
}

fun activateSaucer(entity: SpaceObject) {
    entity.active = true
    entity.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
    entity.velocity = Vector2(saucerSpeed, 0.0)
    saucerSpeed *= -1.0
}

Test. One of my old tests fails, the ones testing checkIfSaucerNeeded, which is no longer used. Remove them. We’re green. Commit: extract and test activateSaucer from updateSaucerTimer.

This has taken me a long time and a lot of thinking (and writing). Let’s reflect on what has actually happened, and think about what to do next.

Thinking …

We built the SaucerTimer yesterday and installed it, making checkIfSaucerNeeeded redundant. It was only referenced by tests at that point. Today, finally, we removed those tests. We wrote a new test yesterday and another new one today. Today’s was pointed directly at testing activateSaucer, a new method extracted from updateSaucerTimer.

We now have two tests that are more than a little redundant:

    @Test
    fun `saucer timer`() {
        val saucer = newSaucer()
        saucer.position = Vector2(100.0, 100.0)
        saucer.velocity = Vector2(0.0, 0.0)
        val oldSpeed = saucerSpeed
        assertThat(saucer.active)
            .describedAs("should start inactive").isEqualTo(false)
        val timer = SaucerTimer(saucer)
        updateSaucerTimer(timer, 0.5)
        assertThat(saucer.active)
            .describedAs("should not activate immediately").isEqualTo(false)
        updateSaucerTimer(timer, U.SaucerDelay)
        assertThat(saucer.active)
            .describedAs("should start by now").isEqualTo(true)
        assertThat(saucer.x)
            .describedAs("should start on y axis").isEqualTo(0.0)
        assertThat(saucer.dx)
            .describedAs("should start at oldSpeed").isEqualTo(oldSpeed)
        assertThat(saucerSpeed)
            .describedAs("should negate saucerSpeed").isEqualTo(-oldSpeed)
    }

    @Test
    fun `saucer activates properly`() {
        val saucer = newSaucer()
        val oldSpeed = saucerSpeed
        saucer.position = Vector2(100.0,200.0)
        saucer.velocity = Vector2(999.0,999.0)
        assertThat(saucer.active)
            .describedAs("starts inactive").isEqualTo(false)
        activateSaucer(saucer)
        assertThat(saucer.active)
            .describedAs("should be active").isEqualTo(true)
        assertThat(saucer.x)
            .describedAs("should start on y axis").isEqualTo(0.0)
        assertThat(saucer.dx)
            .describedAs("speed should be oldSpeed").isEqualTo(oldSpeed)
        assertThat(saucerSpeed)
            .describedAs("should negate saucerSpeed").isEqualTo(-oldSpeed)
    }

The second one tests activation. The first one tests triggering plus activation. Let’s strip it down to just enough checking to know that the saucer got activated.

    @Test
    fun `saucer timer activates after SaucerDelay`() {
        val saucer = newSaucer()
        val timer = SaucerTimer(saucer)
        assertThat(saucer.active)
            .describedAs("should start inactive").isEqualTo(false)
        updateSaucerTimer(timer, 0.5)
        assertThat(saucer.active)
            .describedAs("should not activate immediately").isEqualTo(false)
        updateSaucerTimer(timer, U.SaucerDelay)
        assertThat(saucer.active)
            .describedAs("should start by now").isEqualTo(true)
    }

I think that’s better, but not great. The issue is one of design (or, if you prefer, a detail of implementation).

In an object-oriented scheme, the Timer would have methods to call, so that instead of referring to updateSaucerTimer we’d have a call to timer.update(). I think that would be more clear, but we’d still have to check the result, where it would be nicer, I think, if we could just test “did it fire”. We’d need a timer with pluggable behavior for that.

I think but am not certain that in a true Entity-Component-System design, we’d have either a TimingSystem or a SaucerTimingSystem that would spin over all the timers, tick them, and execute the appropriate action if it was time. That would be silly here, where we just have one saucer, but in a space game with hundreds of appearing saucers, it could make more sense.

An Idea Forms …

It really doesn’t make sense to go to the System level here, in my strong albeit somewhat inexperienced opinion. But I think we could benefit from a kind of pluggable timer that we could test for triggering, and then for which we could test the actions separately, as in the test above.

Let’s TDD that one.

    var executed = false
    @Test
    fun `pluggable timer triggers`() {
        val entity = newAsteroid()
        val timer = PluggableTimer(entity, 5.0) {executed = true}
        timer.update(4.0)
        assertThat(executed)
            .describedAs("not time yet")
            .isEqualTo(false)
        timer.update(1.0)
        assertThat(executed)
            .describedAs("should have triggered")
            .isEqualTo(true)
    }

THe Pluggable timer takes an entity (irrelevant here?), a delay time, and a block to be executed. We should pass the timer as a parameter to the block. Fix the test.

    var executed = false
    @Test
    fun `pluggable timer triggers`() {
        val entity = newAsteroid()
        val timer = PluggableTimer(entity, 5.0) {timer: PluggableTimer -> executed = true}
        timer.update(4.0)
        assertThat(executed)
            .describedAs("not time yet")
            .isEqualTo(false)
        timer.update(1.0)
        assertThat(executed)
            .describedAs("should have triggered")
            .isEqualTo(true)
        assertThat(timer.time)
            .describedAs("should reset timer")
            .isEqualTo(5.0)
    }

I think that’s close to what I want. Create the class.

class PluggableTimer(
    override val entity: SpaceObject,
    private val startTime: Double,
    val action: (PluggableTimer) -> Unit
): Component {
    var time = startTime
    fun update(deltaTime: Double) {
        time -= deltaTime
        if (time <= 0.0 ) {
            action(this)
            time = startTime
        }
    }
}

I rather expect that to work. Let’s find out why I’m wrong. Test. Ha! It does in fact work.

It violates my “no methods” rule, but I’ll worry about that sin later. For now, it’s an interesting object that we’re not using. But it has one delicious property, which is that when it triggers, its action is pluggable and therefore directly able to be checked. I like that and think we’ll find it useful, even if we decide not to let it have update as a method.

For now, that’s enough. The system is a little bit better and better tested and we have an interesting new idea, just forming, that looks valuable. More than enough for a Sunday morning.

See you next time!