Kotlin 260 - Flair! I mean, Flare.

GitHub Decentralized Repo
GitHub Centralized Repo
GitHub Flat Repo

Today I plan to implement the ship’s flare. My reasons are not all good ones.

I am behind the wave on testing. I have no tests for the collision logic, and I’ve been promising to do them for days now. And this morning, I plan to start by making the ship display its intermittent flare when accelerating. Introspecting, I think I might at least consider cleaning the bathroom rather than write a test. And yet, when I write them first, I have almost no resistance. It’s just that when the code works or seems to work, my willingness to write follow-up tests goes negative.

This is an interesting side effect of the test-first, test-driven development style: I get more tests, and generally better ones. With more and better tests, I have more confidence in the program and in my changes. I wind up doing fewer ad-hoc tests, which here means that I spend less time starting up the game and looking for issues.

What should I do about this? Well, I don’t should on myself, much less anyone else, but I do think I’d go faster, with more confidence, if I’d focus more on testing before coding.

What about you? I make no claims on your decisions, but if I could get some money down, I know where I’d bet.

The Story

Nonetheless, I’m going to warm up by implementing ship flare. Maybe — no promises — I’ll start that with a test. First, what’s the story?

When the ship is accelerating (“j” key held down) display the small triangular ship flare in 1/3 of the display opportunities.

The 1/3 is there to make the flare seem to flicker.

Refresh Memory

How does this program work, anyway?

The good news is that there is a public object, Controls:

object Controls {
    var left: Boolean = false
    var right: Boolean = false
    var accelerate: Boolean = false
    var fire: Boolean = false
    var hyperspace: Boolean = false
}

The var accelerate is true when the ship is accelerating. So that’s probably useful.

The less good news is that our definitions of object shapes are static:

enum class SpaceObjectType(val points: List<Vector2>) {
    ASTEROID(asteroidPoints),
    SHIP(shipPoints),
    MISSILE(missilePoints)
}

In draw, we fetch the points and display them:

fun draw(
    spaceObject: SpaceObject,
    drawer: Drawer,
) {
    drawer.isolated {
        val scale = 4.0 *spaceObject.scale
        drawer.translate(spaceObject.x, spaceObject.y)
        drawer.scale(scale, scale)
        drawer.rotate(spaceObject.angle)
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 1.0/scale
        drawer.lineStrip(spaceObject.type.points)
    }
}

So, as things stand, the draw code doesn’t know what it’s drawing, and the points it will be given are unconditional. We do have the ship flare defined, from when we imported its shape Lo! these many articles ago:

private val shipPoints = listOf(
    Vector2(-3.0, -2.0), Vector2(-3.0, 2.0), Vector2(-5.0, 4.0),
    Vector2(7.0, 0.0), Vector2(-5.0, -4.0), Vector2(-3.0, -2.0)
)

private val shipFlare = listOf(
    Vector2(-3.0,-2.0), Vector2(-7.0,0.0), Vector2(-3.0, 2.0)
)

The Options

It seems to me that, ideally, when we ask for the points above, we’d just get the ship plus flare if it’s appropriate, i.e. we’re accelerating and the three-sided die says to display it.

We could put a method on SpaceObject, but we don’t create methods in this implementation, because we’re torturing ourselves.
We could put some kind of lambda in the enum place of the literal lists of points, but that just seems like a method by another name. Maybe we’d rationalize that it is a function pointer.
We could just check in draw and do the right thing.

The Plan

I’m going with #3, at least at first, because it is the simplest idea, if not the nicest.

The Code

fun draw(
    spaceObject: SpaceObject,
    drawer: Drawer,
) {
    drawer.isolated {
        val scale = 4.0 *spaceObject.scale
        drawer.translate(spaceObject.x, spaceObject.y)
        drawer.scale(scale, scale)
        drawer.rotate(spaceObject.angle)
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 1.0/scale
        if (spaceObject.type == SpaceObjectType.SHIP) {
            if (Controls.accelerate && Random.nextInt(1,3) == 1) {
                drawer.lineStrip(shipFlare)
            }
        }
        drawer.lineStrip(spaceObject.type.points)
    }
}

I believe this will do what I intend. And indeed it does:

ship shows flare when accelerating

Let’s at least extract that as a new function.

fun draw(
    spaceObject: SpaceObject,
    drawer: Drawer,
) {
    drawer.isolated {
        val scale = 4.0 *spaceObject.scale
        drawer.translate(spaceObject.x, spaceObject.y)
        drawer.scale(scale, scale)
        drawer.rotate(spaceObject.angle)
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 1.0/scale
        possiblyDrawFlare(spaceObject, drawer)
        drawer.lineStrip(spaceObject.type.points)
    }
}

private fun possiblyDrawFlare(spaceObject: SpaceObject, drawer: Drawer) {
    if (spaceObject.type == SpaceObjectType.SHIP) {
        if (Controls.accelerate && Random.nextInt(1, 3) == 1) {
            drawer.lineStrip(shipFlare)
        }
    }
}

That’s at least a bit better. I’ll commit: ship draws flare when accelerating.

We need to talk about this.

Reflection

In a more object-oriented version of this program, we’d probably have a unique drawing method for each object. If we were allowing implementation inheritance, most of those methods might come down to a call to return the points. We don’t have that luxury here. But we do have a single draw function for all our space objects, which is nice. We just mangled it a bit by adding in the possiblyDrawFlare. The equivalent of that code would have to go somewhere in any implementation of Asteroids. In our case, it’s packaged right with the drawing logic. If all we have is functions, maybe this isn’t a bad decision.

Furthermore, it’s quite likely the only such special case. At least I can tell myself that until we get around to the ship explosion and, possibly, splats.

I think that right now, this is the simplest thing that could possibly work, and as such, I’m going to call it OK. When all you have is functions and if statements, things will be made out of functions and if statements.

We don’t need anything more fancy, so this will do for now.

Do you disagree or have questions? Hit me up on ronjeffries at mastodon dot social and we’ll discuss it. Or bring it up in the Agile Mentoring forum if you belong to it. Or, of course, I’m still on Twitter as ronjeffries, at least as of today.

OK, Some Testing

I think we should do at least one test for collisions, because we have none, and because we’re going to be doing more work in that area, which will only increase our need for confidence. Here’s the collision code as it exists today:

private fun checkCollisions() {
    checkAllMissiles()
}

private fun checkAllMissiles() {
    for (missile in activeMissiles(spaceObjects)) {
        checkAllAsteroids(missile)
    }
}

private fun checkAllAsteroids(missile: SpaceObject) {
    for (asteroid in activeAsteroids(spaceObjects)) {
        checkOneAsteroid(asteroid, missile)
    }
}

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    val killDist = 16.0 * asteroid.scale + 1
    val dist = missile.position.distanceTo(asteroid.position)
    if (dist <= killDist) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

private fun spawnNewAsteroid(asteroid: SpaceObject) {
    val newOne: SpaceObject? = spaceObjects.firstOrNull { it.type == SpaceObjectType.ASTEROID && ! it.active }
    if (newOne != null) {
        newOne.position = asteroid.position
        newOne.scale = asteroid.scale
        newOne.active = true
        val angle = Random.nextDouble(90.0, 270.0)
        newOne.velocity = asteroid.velocity.rotate(angle)
    }
}

What would we test if we could?

An asteroid and missile far apart do not collide.
An asteroid and missile close enough together do collide.
When an asteroid and missile collide, the missile goes inactive.
When the colliding asteroid size is 1, it goes inactive.
When the colliding asteroid size exceeds 1, it gets smaller.
When the colliding asteroid size exceeds 1, a new smaller asteroid appears.

I am really pretty sure that this is going to be hard to check. Let’s put in the test functions for all of these, as a challenge to ourselves to do them.

class CollisionTests {
    @Test
    fun `asteroid and missile far apart do not collide`() {
    }

    @Test
    fun `asteroid and missile close enough do collide`() {
    }

    @Test
    fun `when asteroid and missile collide, missile goes inactive`() {
    }

    @Test
    fun `when colliding asteroid size is 1, it goes inactive`() {
    }

    @Test
    fun `when colliding asteroid size exceeds 1, it gets smaller`() {
    }

    @Test
    fun `when colliding asteroid size exceeds 1, a new asteroid appears`() {
    }

    @Test
    fun `new asteroids get new velocity`() {
    }
}

I thought of a new one, the last one there. And I suspect that the “do collide” and “missile goes inactive” may want to be combined, but we’ll see.

There is no way to test whether a missile and asteroid are colliding. Current code is this:

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    val killDist = 16.0 * asteroid.scale + 1
    val dist = missile.position.distanceTo(asteroid.position)
    if (dist <= killDist) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

We need a function to answer whether a missile and an asteroid are colliding. Or, perhaps, whether any two spaceObjects are colliding. We’ll refactor to get one. First, extract variable:

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    val killDist = 16.0 * asteroid.scale + 1
    val dist = missile.position.distanceTo(asteroid.position)
    val colliding = dist <= killDist
    if (colliding) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

Inline the other variables?

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    val colliding = missile.position.distanceTo(asteroid.position) <= 16.0 * asteroid.scale + 1
    if (colliding) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

Hm. Inline that into the if:

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    if (missile.position.distanceTo(asteroid.position) <= 16.0 * asteroid.scale + 1) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

Extract method:

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    if (colliding(missile, asteroid)) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

private fun colliding(missile: SpaceObject, asteroid: SpaceObject) =
    missile.position.distanceTo(asteroid.position) <= 16.0 * asteroid.scale + 1

Nice. Could have done that in one fewer steps by inlining everything into the if. Nice enough, it was all machine refactorings. Now we have collide and can use it. In addition, we can make it stronger, which will be needed.

    @Test
    fun `asteroid and missile far apart do not collide`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(200.0,200.0)
        assertThat(colliding(missile, asteroid)).isEqualTo(false)
    }

I had to make some functions public to get that to compile. I think it should run green. It does. Shall we commit: first collision test runs.

The next one should be easy enough:

    @Test
    fun `asteroid and missile close enough do collide`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        assertThat(colliding(missile, asteroid)).isEqualTo(true)
    }

Also green. Commit: second collision test runs.

The next one is this:

    @Test
    fun `when asteroid and missile collide, missile goes inactive`() {
    }

Can we just call the outer collision method to check this?

private fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    if (colliding(missile, asteroid)) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

We can … but we’re going to be in trouble for scale > 1. But for now, we’ll give it a go. And I’ll change the tests to say scale rather than size.

    @Test
    fun `when asteroid and missile collide, missile goes inactive`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 1
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        checkOneAsteroid(missile,asteroid)
        assertThat(missile.active).isEqualTo(false)
    }

I expect green here. I get a compile error because scale is apparently a double. How odd. After that, green. Commit: test missile goes inactive on collision.

Next test is this:

    @Test
    fun `when colliding asteroid scale is 1, it goes inactive`() {
    }

Much like the preceding one. We won’t combine them in a fit of tiny test syndrome.

    @Test
    fun `when colliding asteroid scale is 1, it goes inactive`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 1.0
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        checkOneAsteroid(missile,asteroid)
        assertThat(asteroid.active).isEqualTo(false)
    }

Now for these next ones, we have an issue.

    @Test
    fun `when colliding asteroid scale exceeds 1, it gets smaller`() {
    }

    @Test
    fun `when colliding asteroid scale exceeds 1, a new asteroid appears`() {
    }

    @Test
    fun `new asteroids get new velocity`() {
    }

Let’s code the first test and see what happens. The issue is the spawn in checkOneAsteroid.

    @Test
    fun `when colliding asteroid scale exceeds 1, it gets smaller`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 2.0
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        checkOneAsteroid(missile,asteroid)
        assertThat(asteroid.scale).isEqualTo(1.0)
    }

I don’t know what this will do. Run it.

expected: 1.0
 but was: 2.0

I definitely did not expect that. Oh! The asteroid has to be first in the call. I was confused because colliding is the other way.

Let redo that signature.

fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    if (colliding(asteroid, missile)) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

fun colliding(asteroid: SpaceObject, missile: SpaceObject) =
    missile.position.distanceTo(asteroid.position) <= 16.0 * asteroid.scale + 1

Now let me check all my senders and put the asteroid first. All fixed. I still don’t know what this test will do.

lateinit property spaceObjects has not been initialized

Makes sense, we don’t have any space objects at all. Let’s see. To make it right, we really only need two asteroid slots. Let’s try that.

    @Test
    fun `when colliding asteroid scale exceeds 1, it gets smaller`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 2.0
        val availableAsteroid = newAsteroid()
        spaceObjects = arrayOf(asteroid,availableAsteroid)
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        checkOneAsteroid(asteroid,missile)
        assertThat(asteroid.scale).isEqualTo(1.0)
    }

Green. Commit: test that asteroids get smaller on collision.

Now I think we can make the next test work readily.

    @Test
    fun `when colliding asteroid scale exceeds 1, a new asteroid appears`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 2.0
        val availableAsteroid = newAsteroid()
        availableAsteroid.scale = 4.0
        spaceObjects = arrayOf(asteroid,availableAsteroid)
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        checkOneAsteroid(asteroid,missile)
        assertThat(availableAsteroid.scale).isEqualTo(1.0)
        assertThat(availableAsteroid.active).isEqualTo(true)
    }

I expect green. I get it. Commit: test missile-asteroid collision gets new asteroid.

Just one more test in the queue, the new velocity check.

    @Test
    fun `new asteroids get new velocity`() {
        val asteroid = newAsteroid()
        asteroid.position = Vector2(100.0, 100.0)
        asteroid.active = true
        asteroid.scale = 2.0
        val availableAsteroid = newAsteroid()
        availableAsteroid.scale = 4.0
        spaceObjects = arrayOf(asteroid,availableAsteroid)
        val missile :SpaceObject = newMissile()
        missile.position = Vector2(110.0,100.0)
        missile.active = true
        val asteroidVelocity = asteroid.velocity
        val availableAsteroidVelocity = availableAsteroid.velocity
        checkOneAsteroid(asteroid,missile)
        assertThat(availableAsteroid.velocity).isNotEqualTo(availableAsteroidVelocity)
        assertThat(asteroid.velocity).isNotEqualTo(asteroidVelocity)
    }

Green. Commit. test new asteroids get new velocities.

Super. Let’s sum up.

Summary

Well, that’s a charming way to wrap up. Seven new tests for collision, and a little bit of refactoring to make some of the tests quite easy, and none of them are particularly difficult. There’s some duplication in the tests that could be removed, but we’ll leave that for the future, possibly never.

I do still think we could benefit from better creation and initialization functions for our objects.

Did you notice that bug in the test, where I had the missile and asteroid reversed in the calling sequence? If our objects were of different classes, that couldn’t have happened. It was a direct consequence of my decision to go with a very flat version of the code, with all the space objects only distinguished by the type field. Type checking FTW, and I am a dedicated duck-typing fan. The tests did find the bug, though, so testing FTW also.

The bug arose because I had this code:

fun checkOneAsteroid(
    asteroid: SpaceObject,
    missile: SpaceObject
) {
    if (colliding(missile, asteroid)) {
        if (asteroid.scale > 1) {
            asteroid.scale /= 2
            asteroid.velocity = randomVelocity()
            spawnNewAsteroid(asteroid)
        } else deactivate(asteroid)
        deactivate(missile)
    }
}

Note that checkOneAsteroid and colliding have their arguments reversed. And my eyes caught the missile, asteroid in colliding and used it in checkOneAsteroid. Similar calling sequences should be similar all the way, lest silly mistakes like that one occur. Of course, type checking would also find the problem but we do not have that luxury.

And let’s not forget that we now have that nice flare showing up when the ship accelerates. The conditional code for that is called from our single draw function, so while it is a hack, it is a very small one. I don’t see a better solution. If you do, hit me up.

A good morning. See you next time!