GitHub Decentralized Repo
GitHub Centralized Repo
GitHub Flat Repo

Well, of course I’ve made a mistake, rather good at it in fact. But have I broken the saucer timing?

I was gently browsing the code this morning, wondering what I might work on and write about, and I happened upon this code:

var dropScale = U.ShipDropInScale
private var shipGoneFor = 0.0
fun checkIfShipNeeded(deltaTime: Double) {
    if ( ! Ship.active ) {
        shipGoneFor += deltaTime
        if (shipGoneFor > U.ShipDelay) {
            dropScale = U.ShipDropInScale
            Ship.position = Vector2(U.ScreenWidth/2.0, U.ScreenHeight/2.0)
            Ship.velocity = Vector2(0.0,0.0)
            Ship.angle = 0.0
            Ship.active = true
            shipGoneFor = 0.0
        }
    } else {
        dropScale = max(dropScale - U.ShipDropInScale*deltaTime, 1.0)
    }
}

The code there checks the shipGoneFor clock and that got me thinking about time, and that got me thinking that yesterday, when we implemented shooting down the saucer, I don’t remember doing anything about the saucer’s time code. Maybe I did and don’t remember. Maybe nothing was needed. And maybe I’ve messed up the saucer’s timing.

Let’s find out.

private var saucerSpeed = U.SaucerSpeed
private var saucerGoneFor = 0.0
fun checkIfSaucerNeeded(deltaTime: Double) {
    saucerGoneFor += deltaTime
    if (saucerGoneFor > U.SaucerDelay) {
        saucerGoneFor = 0.0
        if (!Saucer.active) {
            Saucer.active = true
            Saucer.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
            Saucer.velocity = Vector2(saucerSpeed, 0.0)
            saucerSpeed *= -1.0
        } else {
            Saucer.active = false
        }
    }
}

We see that here, when, say, the saucer is inactive, after U.SaucerDelay seconds, we’ll zero the saucerGoneFor and activate it. That many seconds later, we’ll zero the timer and deactivate it. But what if we shoot it down?

If we don’t reset the saucerGoneFor time at that point, it will continue to tick from there and a shorter time later, we’ll time out, zero the counter, and reactivate it. The effect would be that after you shoot down the saucer, it would appear sooner than U.SaucerDelay seconds later. A little game play convinces me that it does.

I am now also a bit concerned about the ship and its timing. Does it have the same bug?

I think I’d like to have a test. We’ll do the saucer first, because we know it’s broken.

I was going to make that flag public but I think instead I’ll ask for a little function. We’ll see. I have some saucer-related tests in the GameTests file, so I’ll put a new one in there.

I write this much:

@Test
    fun `saucer clock resets on death`() {
        createGame(U.MissileCount, U.AsteroidCount)
        startGame(U.ScreenWidth, U.ScreenHeight)
        checkIfSaucerNeeded(U.SaucerDelay + 0.1)
        assertThat(Saucer.active).isEqualTo(true)
        val missile = missiles(SpaceObjects).first()!!
        missile.active = true
        missile.position = Saucer.position
        checkSaucerVsMissile(Saucer, missile)
        assertThat(Saucer.active).isEqualTo(false)
    }

A bit messy, but I need an active missile to kill the saucer etc etc. This test runs so far … but it breaks another test somehow. This one:

@Test
    fun `saucer switches direction`() {
        createGame(U.MissileCount, U.AsteroidCount)
        startGame(U.ScreenWidth, U.ScreenHeight)
        checkIfSaucerNeeded(U.SaucerDelay + 0.1)
        assertThat(Saucer.active).isEqualTo(true)
        assertThat(Saucer.dx).isGreaterThan(0.0)
        checkIfSaucerNeeded(U.SaucerDelay + 0.1)
        assertThat(Saucer.active).isEqualTo(false)
        checkIfSaucerNeeded(U.SaucerDelay + 0.1)
        assertThat(Saucer.active).isEqualTo(true)
        assertThat(Saucer.dx).describedAs("reverse").isLessThan(0.0)
    }

The failure is that the direction is -150 and should be greater than zero. How can this be? Clearly we’re not clearing the saucer direction, wherever that is. Ah:

private var saucerSpeed = U.SaucerSpeed
private var saucerGoneFor = 0.0
fun checkIfSaucerNeeded(deltaTime: Double) {
    saucerGoneFor += deltaTime
    if (saucerGoneFor > U.SaucerDelay) {
        saucerGoneFor = 0.0
        if (!Saucer.active) {
            Saucer.active = true
            Saucer.position = Vector2(0.0, Random.nextDouble(U.ScreenHeight.toDouble()))
            Saucer.velocity = Vector2(saucerSpeed, 0.0)
            saucerSpeed *= -1.0
        } else {
            Saucer.active = false
        }
    }
}

saucerSpeed needs to be initialized in startGame.

Oh this is not good at all:

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

fun startGame(width: Int, height: Int) {
    Ship.active = true
    Ship.position = Vector2(width/2.0, height/2.0)
    activateAsteroids(4)
}

There are surely lots of constants, including the timers, that need to be initialized here. And we’d do well not to rely on createGame’s individual values here. It would be better to be explicit, I think.

Let’s start with these:

fun startGame(width: Int, height: Int) {
    saucerGoneFor = 0.0
    saucerSpeed = U.SaucerSpeed
    shipGoneFor = 0.0
    Ship.active = true
    Ship.position = Vector2(width/2.0, height/2.0)
    Saucer.active = false
    activateAsteroids(4)
}

Let’s see if the tests run now. Yes, so I was right about the init. Let’s commit those fixes and then think a moment. Commit: Fix startGame to init necessary individual vars.

Reflection

Global initialization is always an important matter, and in a small game like this one, with fairly many global variables, we’re always in danger of missing some. We might do well to create a Game data object that holds all these things, and to create a new one for each game. Why would this be better? It would be better because all the game information would be there right in front of us and we could specify the initial values once and for all, and be pretty confident that we’d done it.

I don’t want to divert to do that, but I think we’ll put it on the agenda.

For now, we have a bug in timing to find and fix. Back to that test:

    @Test
    fun `saucer clock resets on death`() {
        createGame(U.MissileCount, U.AsteroidCount)
        startGame(U.ScreenWidth, U.ScreenHeight)
        checkIfSaucerNeeded(U.SaucerDelay + 0.1)
        assertThat(Saucer.active).isEqualTo(true)
        checkIfSaucerNeeded(U.SaucerDelay - 1.0)
        val missile = missiles(SpaceObjects).first()!!
        missile.active = true
        missile.position = Saucer.position
        checkSaucerVsMissile(Saucer, missile)
        assertThat(Saucer.active).isEqualTo(false)
        checkIfSaucerNeeded(3.0)
        assertThat(Saucer.active).describedAs("too soon").isEqualTo(false)
        checkIfSaucerNeeded(U.SaucerDelay)
        assertThat(Saucer.active).isEqualTo(true)
    }

This is long. The gist is, we run until the saucer is spawned, then almost until it should time out, and then we shoot it down. If the timer is reset, then three seconds later, the saucer should still be inactive. If it is active, it’s “too soon”. Then, finally, it’ll come back to life.

I expect this to fail saying “too soon”.

org.opentest4j.AssertionFailedError: [too soon] 
expected: false
 but was: true

Perfect. Now how do we fix it? We have a simple deactivate function that we use and it doesn’t know about timers:

fun checkSaucerVsMissile(saucer: SpaceObject, missile: SpaceObject) {
    if (colliding(saucer, missile)) {
        Score += U.SaucerScore
        deactivate(saucer)
        deactivate(missile)
    }
}

fun deactivate(entity: SpaceObject) {
    entity.active = false
    for (component in entity.components) {
        when (component) {
            is Timer -> {
                component.time = component.startTime
            }
        }
    }
}

That timer isn’t the creation timer, it’s the timer that causes missiles to time out.

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

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

We could do something truly awful, and I think that in fact we will.

fun deactivate(entity: SpaceObject) {
    entity.active = false
    for (component in entity.components) {
        when (component) {
            is Timer -> {
                component.time = component.startTime
            }
        }
    }
    if (entity.type == SpaceObjectType.SHIP) shipGoneFor = 0.0
    if (entity.type == SpaceObjectType.SAUCER) saucerGoneFor = 0.0
}

Nasty. But I’d rather try to have the timers reset in one place than have to sprinkle changes all over. Let’s see if the test runs. It does. This does fix the bug.

Let’s commit and then reflect upon our sins. Commit: Fix issue with ship and saucer GoneFor timers not resetting on deactivation.

Serious Reflection

That’s really pretty nasty code, checking the data types of our objects. It is, however, quite common when we don’t have object methods to handle the breakout for us. If we had a ship class and a saucer class, their deactivate methods would be separate and we could do what’s needed in each one. Here, we cannot.

Hey! What about Components?

However, the code above reminds us that we do have Components. Could we devise a new kind of timer that when it counts down, it inverts the activation of its entity? If so, we could use that as a component of the saucer and it would toggle on and off all by itself. That would actually be rather nifty.

What about the Ship? It might have a different kind of timer, one that notices when the ship has been gone for a while, and restarts it when it times out.

Those might well be in the spirit of our mini-ECS, which so far just has the one missile-killing Timer.

I think we should do that, if only to see how it feels. But not today. So now I have to make two yellow sticky Jira notes:

  • Game data object
  • ECS timers for saucer and ship

We’ll deal with those in future issues. For now, we’ve patched the leak in the saucer and ship and we’ll call it good enough.

Nasty code, though. Nasty. Blech!

See you next time!