GitHub Repo

We replace ShipMonitor with ShipChecker and ShipMaker, about the same number of lines but far less intricate. A win.

My plan this morning is to revise the ShipMonitor in view of the new ability of an object to know when interaction is starting and when it is over. Prior to that change, we needed an odd hand-off between interaction and update, where update would set a flag one way, an individual interaction might set it the other way, and then in the next update, we could see if the flag had changed and if so or if not, take some action.

ShipMonitor’s job is to watch to see if there is no ship, and if not, bring it back to life. In the future, there will be other considerations, such as the number of times you can get a new ship, handling game over and attract mode, and so on. It’s already rather complicated.

ShipMonitor uses four states:

enum class ShipMonitorState {
    HaveSeenShip, LookingForShip, WaitingForTime, WaitingForSafety
}

Using these four states, it starts each interaction cycle having set LookingForShip, and the update cycle, if it sees the ship go by, sets HaveSeenShip. If that’s the case, update sets looking and things proceed, ping-ponging back and forth.

If ever the interaction does not see the ship, update finds the state to still be LookingForShip and it must therefore do things. First, it sets state to WaitingForTime, and, well, waits for the time to elapse, the point being to make a discernible delay between your ship being destroyed and a new one appearing. (We plan a ship explosion display: we’d have to wait that out as well.)

After we’re done waiting for the time to expire, we enter WaitingForSafety. In that state, the interaction logic considers all asteroids and their distance from the proposed point of emergence. Only if they’re all far enough away does it set safeToEmerge. When that happens, update goes back to the HaveSeenShip state and returns the ship to be inserted in the mix … and the cycle continues.

In aid of all this, the object is messy. Here it is for your scanning “pleasure”:

class ShipMonitor(val ship: SolidObject) : ISpaceObject {
    override var elapsedTime: Double = 0.0
    var state = HaveSeenShip
    var safeToEmerge = false
    var asteroidTally = 0


    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        } else if (state == WaitingForSafety){
            if (other is SolidObject && other.isAsteroid) {
                asteroidTally += 1
            }
            if (tooClose(other)) safeToEmerge = false
        }
        return emptyList() // no damage done here
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        return this.interactWith(other)
    }

    private fun shipReset(): ISpaceObject {
        ship.velocity = Velocity.ZERO
        return ship
    }

    fun startCheckingForSafeEmergence() {
        // assume we're OK. interactWith may tell us otherwise.
        safeToEmerge = true
        asteroidTally = 0
    }

    private fun tooClose(other:ISpaceObject): Boolean {
        return if (other !is SolidObject) false
        else (ship.position.distanceTo(other.position) < U.SAFE_SHIP_DISTANCE)
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        elapsedTime += deltaTime
        var toBeCreated: List<ISpaceObject> = emptyList()
        state = when (state) {
            HaveSeenShip -> LookingForShip
            LookingForShip -> {
                elapsedTime = 0.0
                WaitingForTime
            }
            WaitingForTime -> {
                if (elapsedTime < 3.0)
                    WaitingForTime
                else {
                    startCheckingForSafeEmergence()
                    WaitingForSafety
                }
            }
            WaitingForSafety -> {
                if (safeToEmerge) {
                    toBeCreated = makeEmergenceObjects()
                    HaveSeenShip
                } else {
                    startCheckingForSafeEmergence()
                    WaitingForSafety
                }
            }
        }
        return toBeCreated
    }

    private fun makeEmergenceObjects(): List<ISpaceObject> {
        return when (emergenceIsOK()) {
            true -> {
                listOf(shipReset())
            }
            false -> {
                val splat = SolidObject.splat(ship)
                val destroyer = SolidObject.shipDestroyer(ship)
                listOf(splat, destroyer, shipReset())
            }
        }
    }

    private fun emergenceIsOK() = notInHyperspace() or hyperspaceWorks()

    private fun notInHyperspace() = ship.position == U.CENTER_OF_UNIVERSE

    private fun hyperspaceWorks(): Boolean
        = !hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidCount: Int): Boolean
        = random0thru62 >= (asteroidCount + 44)
}

In this beauty, interactWith has two different modes, one looking for the ship, the other one counting asteroids (for hyperspace emergence odds) and checking to see if any are too close. In the other two states, it just returns.

The update considers all four states, with if statements in two of them.

Suffice to say, if someone asked me to change how this works, my initial reaction would be to consider early retirement. I think in the end, I could figure it out, since I wrote it, but it’s not easy to change. Since it’s a state machine, it’s quite likely that to update it you have to rewrite it, essentially creating a new machine from scratch.

In fact, arguably, it is a state machine with another state machine embedded in it, due to the if statements in there. It’s as clean as I could make it, but still far from obvious.

My hope this morning is to replace this with something discernibly better, something that wouldn’t seem three times more complicated than anything else in the whole program.

Let’s get started.

Developing a Plan

I think we should have a two-object design, as we have for the asteroid waves. One object (ShipChecker) will check for ship absence and when the ship is gone, create a ShipMaker to place the ship. I was thinking that ShipChecker would wait for the ShipMaker to disappear, but it makes more sense to me at this moment to have ShipMaker just call back to ShipChecker when it’s safe to start detecting again.

Another possibility, Plan B, is this:

  1. ShipChecker watches the ship until it disappears from the mix.
  2. ShipChecker
    • adds a ShipMaker, giving it the ship to place.
    • removes itself from the mix.
  3. ShipMaker
    • waits a while,
    • waits for safety,
    • inserts the ship,
    • inserts a new ShipChecker, giving it the ship to watch,
    • removes itself from the mix.

That’s interesting. Does it seem like a bit of a dirty trick, deleting and restoring the checker and maker? I think that if we do it, it should be quite nifty.

Let’s go with Plan B.

Let’s try building this with TDD. We’ll want to build these two collaborating objects together, I think. My plan is to build this right, but I am open to the possibility that it won’t work out, in which case we’ll roll back. I do not expect to want to do that.

Build It

I think my first test will be to create a ship and a ship checker, and run the checker’s interaction cycle twice, once with the ship appearing and once with it not. The first time through it shouldn’t return anything from the finishInteractions function. The second, it should return a ShipMaker to add and itself to remove.

I’ll go in small steps, growing the test and adding new ones. Often these tests got pretty long. I’ll try to keep these shorter, perhaps adding most specific smaller tests instead.

    @Test
    fun `ShipChecker adds maker, removes self if no ship`() {
        val ship = SolidObject.ship(U.randomPoint())
        val checker = ShipChecker(ship)
    }

This will drive out the object and anything that we have to override. IDEA builds this:

class ShipChecker(ship: SolidObject) : ISpaceObject {
    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        TODO("Not yet implemented")
    }
}

I could speculate but let’s let the test drive. Here’s a bit more of the story:

    @Test
    fun `ShipChecker adds maker, removes self if no ship`() {
        val ship = SolidObject.ship(U.randomPoint())
        val checker = ShipChecker(ship)
        checker.beginInteraction()
        val nothing = checker.interactWith(ship)
        assertThat(nothing).isEmpty()
        val emptyTransaction = checker.finishInteraction()
        assertThat(emptyTransaction.adds).isEmpty()
        assertThat(emptyTransaction.removes).isEmpty()
    }

Should I fake all this with explicit returns? OK, for fun, let’s do that, approximately the minimal code.

    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        return emptyList()
    }

This is enough to pass the test so far. OK, we’ll continue with these tiny steps. Reading the generated code, I see that we’d better cover the other interactsWith, and update. So I’ll make sure to use those in the test. I decide to end the test here and write a second one. Here’s the first one, renamed:

    @Test
    fun `ShipChecker does nothing if ship seen`() {
        val ship = SolidObject.ship(U.randomPoint())
        val checker = ShipChecker(ship)
        checker.beginInteraction()
        val nothing = checker.interactWith(ship)
        assertThat(nothing).isEmpty()
        val emptyTransaction = checker.finishInteraction()
        assertThat(emptyTransaction.adds).isEmpty()
        assertThat(emptyTransaction.removes).isEmpty()
        val alsoNothing = checker.update(0.01)
        assertThat(alsoNothing).isEmpty()
    }

With a return emptyList() in update, this one passes. The object is still trivial:

class ShipChecker(ship: SolidObject) : ISpaceObject {
    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        return emptyList()
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        return emptyList()
    }
}

Let’s drive out the rest. New test:

    @Test
    fun `creates ShipMaker if no ship seen`() {
        val ship = SolidObject.ship(U.randomPoint())
        val checker = ShipChecker(ship)
        checker.beginInteraction()
        // we see no ship here
        val transaction = checker.finishInteraction()
        assertThat(transaction.removes.toList()[0]).isEqualTo(checker)
        assertThat(transaction.adds.toList()[0]).isInstanceOf(ShipMaker::class.java)
    }

This is at least the second time I’ve had to do that toList()[0] thing. In a moment, let’s push this into some kind of a testing method on Transaction. But first we have a test to make run. IDEA wants to make me a ShipMaker class.

class ShipMaker(ship: SolidObject) : ISpaceObject {
    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        TODO("Not yet implemented")
    }
}

We’re still testing ShipChecker and the test is going to fail a lot.

Empty list doesn't contain element at index 0.

Right. We have no logic. Let’s have some. We want to set a ship flag in begin check it in finish and return suitably.

class ShipChecker(val ship: SolidObject) : ISpaceObject {
    private var missingShip = true
    
    override fun beginInteraction() {
        missingShip = true
    }
    
    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if ( other == ship ) missingShip = false
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        return interactWith(other)
    }

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if ( missingShip ) {
            trans.add(ShipMaker(ship))
            trans.remove(this)
        }
        return trans
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        return emptyList()
    }
}

This is surely close and might pass. Test. Forgot to return emptyList from interactWith. Test passes. I want to write a slightly different test however, because of interactWithOther. I implemented it but it’s not tested.

    @Test
    fun `ShipChecker does nothing if ship seen via withOther`() {
        val ship = SolidObject.ship(U.randomPoint())
        val checker = ShipChecker(ship)
        checker.beginInteraction()
        val nothing = checker.interactWithOther(ship)
        assertThat(nothing).isEmpty()
        val emptyTransaction = checker.finishInteraction()
        assertThat(emptyTransaction.adds).isEmpty()
        assertThat(emptyTransaction.removes).isEmpty()
        val alsoNothing = checker.update(0.01)
        assertThat(alsoNothing).isEmpty()
    }

We are green, and we could commit the checker. Let’s not, because it makes no sense to have it without the maker. The maker will be a bit more difficult, I think. Let’s talk through it.

The Maker needs to wait for some interval, and then, if the area is safe, insert the ship and remove itself. So we need to test the timing, safe insertion, and unsafe.

I’ll put the tests in the current test file, since we are really testing both objects.

Oh my. I nearly forgot that the Maker deals with hyperspace returns and regular you got destroyed returns. OK, we’ll deal with that as we go forward. Maybe we’ll want more objects. Anyway, a test.

    @Test
    fun `maker delays U MAKER_DELAY seconds`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.beginInteraction()
        // nothing in the way
        val nothing = maker.finishInteraction()
        assertThat(nothing.adds).isEmpty()
        assertThat(nothing.removes).isEmpty()
    }

This should drive out some stuff. First, update:

    override fun update(deltaTime: Double): List<ISpaceObject> {
        elapsedTime += deltaTime
        return emptyList()
    }

With, of course, a corresponding var. And I forgot the return again but was forcibly reminded. Test passes, because nothing has to be done, so all those default methods are just fine. Test more:

    @Test
    fun `maker makes after U MAKER_DELAY seconds`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.update(0.01)
        maker.beginInteraction()
        // nothing in the way
        val nothing = maker.finishInteraction()
        assertThat(nothing.firstAdd()).isInstanceOf(ShipChecker::class.java)
        assertThat(nothing.firstRemove()).isEqualTo(maker)
    }

I decided to give myself those first methods:

class Transaction ...
    fun firstAdd(): ISpaceObject {
        return adds.toList()[0]
    }

    fun firstRemove(): ISpaceObject {
        return removes.toList()[0]
    }

Test will fail with an error on the first first, since the sets are empty.

Empty list doesn't contain element at index 0.

We have to do some work, but not much. By the form of the test, we’re assuming that we’ll do the deciding in finishInteraction. Here’s ShipMaker so far:

class ShipMaker(val ship: SolidObject) : ISpaceObject {
    override var elapsedTime: Double = 0.0
    var safeToEmerge = falst

    override fun beginInteraction() {
        safeToEmerge = false
    }

    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        TODO("Not yet implemented")
    }

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            trans.add(ShipChecker(ship))
            trans.remove(this)
        }
        return trans
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        elapsedTime += deltaTime
        return emptyList()
    }
}

I think this should go green. Well, it might have if I had used the conventional spelling for “false”. No, because safeToEmerge is set wrong. Assume safe.

    override fun beginInteraction() {
        safeToEmerge = true
    }

Now it should emerge. It does.

At this point, we could release ShipChecker and ShipMaker and the game would work properly when you are killed or enter hyperspace, except that there’d be no safety checking.

I can’t resist trying it to find out why I’m mistaken.

Yeah wrong. I’m not handling the interactions at all. Fix that. Ship doesn’t come back at all. Why? Because I didn’t add the ship back. Duh. Improve the test. Easier said than done, because now we have more than one item in the adds, so our hack for first doesn’t work.

    @Test
    fun `maker makes after U MAKER_DELAY seconds`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.update(0.01)
        maker.beginInteraction()
        // nothing in the way
        val trans = maker.finishInteraction()
        assertThat(trans.adds.size).isEqualTo(2)
        assertThat(trans.adds).contains(ship)
        assertThat(trans.firstRemove()).isEqualTo(maker)
    }

And fix:


    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            trans.add(ShipChecker(ship))
            trans.add(ship)
            trans.remove(this)
        }
        return trans
    }

Green. Gotta try it, can’t resist. The manual test pays off: Ship emerges still moving with the velocity it had. We need to reset as we did in ShipMonitor. We did that in a ShipMonitor method. We should make the ship do it itself:

    @Test
    fun `maker makes after U MAKER_DELAY seconds`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        ship.velocity = Velocity(50.0,60.0)
        ship.heading = 90.0
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.update(0.01)
        maker.beginInteraction()
        // nothing in the way
        val trans = maker.finishInteraction()
        assertThat(trans.adds.size).isEqualTo(2)
        assertThat(trans.adds).contains(ship)
        assertThat(trans.firstRemove()).isEqualTo(maker)
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)
        assertThat(ship.heading).isEqualTo(0.0)
    }

Test better fail. Does. Fix:

class ShipMaker ...
    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            trans.add(ShipChecker(ship))
            ship.reset()
            trans.add(ship)
            trans.remove(this)
        }
        return trans
    }

class SolidObject ...
    fun reset() {
        velocity = Velocity.ZERO
        heading = 0.0
    }

Green. Now let’s deal with safe emergence. Then we have to decide what to do about hyperspace.

In ShipMonitor we did this:

            if (tooClose(other)) safeToEmerge = false

    private fun tooClose(other:ISpaceObject): Boolean {
        return if (other !is SolidObject) false
        else (ship.position.distanceTo(other.position) < U.SAFE_SHIP_DISTANCE)
    }

Better test this, I want to wind up with good tests here.

    @Test
    fun `maker makes only when safe`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        val asteroid = SolidObject.asteroid(U.CENTER_OF_UNIVERSE)
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.update(0.01)
        maker.beginInteraction()
        maker.interactWithOther(asteroid)
        val nothing = maker.finishInteraction()
        assertThat(nothing.adds).isEmpty()
        assertThat(nothing.removes).isEmpty()
        maker.beginInteraction()
        // nothing
        val trans = maker.finishInteraction()
        assertThat(trans.adds.size).isEqualTo(2)
        assertThat(trans.adds).contains(ship)
        assertThat(trans.firstRemove()).isEqualTo(maker)
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)
        assertThat(ship.heading).isEqualTo(0.0)
    }

This fails with not empty, because we’re not checking yet. So …

class ShipMaker ...
    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if (tooClose(other)) safeToEmerge = false
        return emptyList()
    }

    private fun tooClose(other:ISpaceObject): Boolean {
        return if (other !is SolidObject) false
        else (ship.position.distanceTo(other.position) < U.SAFE_SHIP_DISTANCE)
    }

I expect a pass. I get it. Now there’s just one issue left, hyperspace. The issues include:

  1. We’ll need to know whether we’re in hyperspace or dead.
  2. Hyperspace should retain orientation and velocity. At least my Lua game does.
  3. Hyperspace might cause us to explode. We’ll signify that by returning a destroyer and a splat.
  4. This will be a pain to test.

In researching this issue, I find a defect, and an opportunity, that I hadn’t noticed. This code:

class SolidObject

    fun deathDueToCollision(): Boolean {
        return !controls.hyperspace
    }

class ShipFinalizer : IFinalizer {
    override fun finalize(solidObject: SolidObject): List<ISpaceObject> {
        if ( solidObject.deathDueToCollision())
            solidObject.position = U.CENTER_OF_UNIVERSE
        else
            solidObject.position = U.randomPoint()
        return emptyList()
    }
}

That was supposed to be setting the ship position to center if it was killed, and randomly if it went to hyperspace. But at some past point, Controls got changed to reset the hyperspace flag, because we were otherwise creating multiple destroyers.

This is a problem because we need to know whether the ship has gone to hyperspace or been killed, right here, and the deathDuetoCollision no longer works. Ship could ask Controls different question. Let’s do that. We’ll have another flag in Controls, recentHyperspace.

    fun control(ship: SolidObject, deltaTime: Double): List<ISpaceObject> {
        if (hyperspace) {
            hyperspace = false
            recentHyperspace = true
            return listOf(SolidObject.shipDestroyer(ship))
        }
        turn(ship, deltaTime)
        accelerate(ship, deltaTime)
        return fire(ship)
    }

// And in the main program:
        keyboard.keyUp.listen {
            when (it.name) {
                "d" -> {controls.left = false}
                "f" -> {controls.right = false}
                "j" -> {controls.accelerate = false}
                "k" -> {controls.fire = false}
                "space" -> {
                    controls.hyperspace = false
                    controls.recentHyperspace = false
                }
            }
        }

class SolidObject
    fun deathDueToCollision(): Boolean {
        return !controls.recentHyperspace
    }

This code tells me that we could benefit from making Controls a bit more robust. But it should fix the hyperspace issue.

In ShipFinalizer we’ll set up all the way for hyperspace emergence (leaving velocity and heading as they were, and death, setting them to zero.)

class ShipFinalizer : IFinalizer {
    override fun finalize(solidObject: SolidObject): List<ISpaceObject> {
        if ( solidObject.deathDueToCollision()) {
            solidObject.position = U.CENTER_OF_UNIVERSE
            solidObject.velocity = Velocity.ZERO
            solidObject.heading = 0.0
        } else {
            solidObject.position = U.randomPoint()
        }
        return emptyList()
    }
}

I’d like to commit this bug fix now, but untangling the changes is tricky. I could stash, do again and so on, but that’ far enough outside my usual pattern that it’s not on the table. Let’s just make things work.

My velocity check is failing.

    @Test
    fun `maker makes after U MAKER_DELAY seconds`() {
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        ship.velocity = Velocity(50.0,60.0)
        ship.heading = 90.0
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY)
        maker.update(0.01)
        maker.beginInteraction()
        // nothing in the way
        val trans = maker.finishInteraction()
        assertThat(trans.adds.size).isEqualTo(2)
        assertThat(trans.adds).contains(ship)
        assertThat(trans.firstRemove()).isEqualTo(maker)
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)
        assertThat(ship.heading).isEqualTo(0.0)
    }

Those checks are now mistaken: we aren’t changing the ship status when we remake it. It will be changed when it is finalized. Remove those checks.

We are green and can make a hyperspace test or two. First we’ll just ensure that we make with the ship wherever it was, trusting (perhaps mistakenly) that the finalize gets it right.

    @Test
    fun `makes with ship features unchanged`() {
        val position = Point(123.0,456.0)
        val  velocity = Velocity(200.0,300.0)
        val heading = 37.5
        val ship = SolidObject.ship(position)
        ship.heading = heading
        ship.velocity = velocity
        val maker = ShipMaker(ship)
        maker.update(U.MAKER_DELAY + 0.01)
        maker.beginInteraction()
        // no obstacles
        val trans = maker.finishInteraction()
        assertThat(trans.adds.size).isEqualTo(2)
        assertThat(trans.adds).contains(ship)
        assertThat(ship.position).isEqualTo(position)
        assertThat(ship.velocity).isEqualTo(velocity)
        assertThat(ship.heading).isEqualTo(heading)
    }

Green. We have everything now except for hyperspace emergence failure. Let’s review how that was done and tested in ShipMonitor. I’ll spare you the code.

One test is for ShipMaker to tally the asteroid count, which we need in the odds calculation. We can test that:

    @Test
    fun `maker counts asteroids`() {
        val a = SolidObject.asteroid(U.randomPoint())
        val ship = SolidObject.ship(U.CENTER_OF_UNIVERSE)
        val maker = ShipMaker(ship)
        maker.beginInteraction()
        maker.interactWith(a)
        maker.interactWithOther(a)
        assertThat(maker.asteroidTally).isEqualTo(2)
    }

Test denies existence of asteroidTally. We provide:

    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if (other is SolidObject && other.isAsteroid) {
            asteroidTally += 1
        }
        if (tooClose(other)) safeToEmerge = false
        return emptyList()
    }

Test should run. It does. ShipMaker knows how many asteroids there are. Test its odds calculation much as we did for SHipMonitor:

    @Test
    fun `hyperspace failure checks`() {
        val ship = SolidObject.ship(Point(10.0, 10.0))
        val maker = ShipMaker(ship)
        assertThat(maker.hyperspaceFailure(62, 19)).describedAs("roll 62 19 asteroids").isEqualTo(false)
        assertThat(maker.hyperspaceFailure(62, 18)).describedAs("roll 62 18 asteroids").isEqualTo(true)
        assertThat(maker.hyperspaceFailure(45, 0)).describedAs("roll 45 0 asteroids").isEqualTo(true)
        assertThat(maker.hyperspaceFailure(44, 0)).describedAs("roll 44 0 asteroids").isEqualTo(true)
        assertThat(maker.hyperspaceFailure(43, 0)).describedAs("roll 43 0 asteroids").isEqualTo(false)
    }

Implement via copy/paste:

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidCount: Int): Boolean
            = random0thru62 >= (asteroidCount + 44)

And this I’m not sure how to test, so let’s put it in and see:

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            trans.add(ShipChecker(ship))
            trans.add(ship)
            trans.remove(this)
            if (emergenceFails()) {
                trans.add(SolidObject.shipDestroyer(ship))
                trans.add(SolidObject.splat(ship))
            }
        }
        return trans
    }
    
    private fun emergenceFails(): Boolean {
        if (notInHyperspace()) return false
        return hyperspaceFails()
    }

    private fun notInHyperspace() = ship.position == U.CENTER_OF_UNIVERSE

    private fun hyperspaceFails(): Boolean
            = hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidCount: Int): Boolean
            = random0thru62 >= (asteroidCount + 44)

Other than some weird injection of a fake random number, I don’t see how to test the inside of the emergenceFails if statement. The game is playing correctly and the tests all run.

I’d like to refactor that finishInteractions method. If I use only Kotlin’s refactorings, I can retain confidence. I want to come back and make this testable, somehow. But let’s see if we can make it more clear.

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            buildShipReplacementTransaction(trans)
        }
        return trans
    }

    private fun buildShipReplacementTransaction(trans: Transaction) {
        trans.add(ship)
        trans.add(ShipChecker(ship))
        trans.remove(this)
        if (emergenceFails()) {
            destroyNewlyCreatedShip(trans)
        }
    }

    private fun destroyNewlyCreatedShip(trans: Transaction) {
        trans.add(SolidObject.shipDestroyer(ship))
        trans.add(SolidObject.splat(ship))
    }

IDEA did that, so it is “guaranteed” good.

I think we can remove ShipMonitor, ShipMonitorState, and its tests. We remain green. Commit: ShipMonitor replaced by ShipChecker and ShipMaker.

Let’s reflect.

Reflection

ShipMonitor plus its State was about 100 lines, with four states.

ShipMaker is 71 lines, and ShipChecker is 31, for roughly the same line count.

However, ShipChecker is very simple:

class ShipChecker(val ship: SolidObject) : ISpaceObject {
    private var missingShip = true

    override fun beginInteraction() {
        missingShip = true
    }

    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if ( other == ship ) missingShip = false
        return emptyList()
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        return interactWith(other)
    }

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if ( missingShip ) {
            trans.add(ShipMaker(ship))
            trans.remove(this)
        }
        return trans
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        return emptyList()
    }
}

It just returns a ShipMaker and removes itself if the ship ever goes missing. It does use three methods to do that, plus it has a useless update method. We could perhaps default that in ISpaceObject if we wanted to reduce noise. It’s certainly the minimal do nothing behavior.

As for ShipMaker, it’s shorter than ShipMonitor and much simpler:

Just scan it, we’re going to improve it below.

class ShipMaker(val ship: SolidObject) : ISpaceObject {
    override var elapsedTime: Double = 0.0
    var safeToEmerge = true
    var asteroidTally = 0

    override fun beginInteraction() {
        safeToEmerge = true
        asteroidTally = 0
    }

    override fun interactWith(other: ISpaceObject): List<ISpaceObject> {
        if (other is SolidObject && other.isAsteroid) {
            asteroidTally += 1
        }
        if (tooClose(other)) safeToEmerge = false
        return emptyList()
    }

    override fun interactWithOther(other: ISpaceObject): List<ISpaceObject> {
        return interactWith(other)
    }

    override fun finishInteraction(): Transaction {
        val trans = Transaction()
        if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            buildShipReplacementTransaction(trans)
        }
        return trans
    }

    private fun buildShipReplacementTransaction(trans: Transaction) {
        trans.add(ship)
        trans.add(ShipChecker(ship))
        trans.remove(this)
        if (emergenceFails()) {
            destroyNewlyCreatedShip(trans)
        }
    }

    private fun destroyNewlyCreatedShip(trans: Transaction) {
        trans.add(SolidObject.shipDestroyer(ship))
        trans.add(SolidObject.splat(ship))
    }

    private fun emergenceFails(): Boolean {
        if (notInHyperspace()) return false
        return hyperspaceFails()
    }

    private fun notInHyperspace() = ship.position == U.CENTER_OF_UNIVERSE

    private fun tooClose(other:ISpaceObject): Boolean {
        return if (other !is SolidObject) false
        else (ship.position.distanceTo(other.position) < U.SAFE_SHIP_DISTANCE)
    }

    override fun update(deltaTime: Double): List<ISpaceObject> {
        elapsedTime += deltaTime
        return emptyList()
    }

    private fun hyperspaceFails(): Boolean
            = hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidCount: Int): Boolean
            = random0thru62 >= (asteroidCount + 44)
}

All the action here takes place in the interactions, other than keeping time in the update. The interactions have to keep track of two things, the asteroid tally, and whether it is safe to emerge, that is, no one is too close to where the ship wants to emerge.

There are quite a few methods involved in building up the ship replacement transaction: buildShipReplacementTransaction, emergenceFails, destroyNewlyCreatedShip, notInHyperspace, hyperspaceFails, and hyperspaceFailure. That suggests to me that there is another object that would like to be born, perhaps a hyperspace mechanism. We won’t go there, but let’s see if we can make things a bit more clear with some additional refactoring:

Refactoring

    private fun buildShipReplacementTransaction(trans: Transaction) {
        trans.add(ship)
        trans.add(ShipChecker(ship))
        trans.remove(this)
        trans.accumulate(hyperspaceOperation())
    }

I’m supposing that hyperspaceOperation() returns a possibly empty transaction. Let’s recast this a bit, refactoring to this:

    override fun finishInteraction(): Transaction {
        return if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            buildShipReplacementTransaction()
        } else {
            Transaction()
        }
    }

    private fun buildShipReplacementTransaction(): Transaction {
        return Transaction().also {
            it.add(ship)
            it.add(ShipChecker(ship))
            it.remove(this)
            it.accumulate(possibleHyperspaceFailure())
        }
    }

    private fun possibleHyperspaceFailure(): Transaction {
        return if (hyperspaceFails()) shipDestroyedTransaction()
        else Transaction()
    }

    private fun shipDestroyedTransaction(): Transaction {
        return Transaction().also {
            it.add(SolidObject.shipDestroyer(ship))
            it.add(SolidObject.splat(ship))
        }
    }

    private fun inHyperspace() = ship.position != U.CENTER_OF_UNIVERSE

    private fun hyperspaceFails(): Boolean
            = inHyperspace() && hyperspaceFailure(Random.nextInt(0, 63), asteroidTally)

    // allegedly the original arcade rule
    fun hyperspaceFailure(random0thru62: Int, asteroidCount: Int): Boolean
            = random0thru62 >= (asteroidCount + 44)

Here, we’ve packaged up the possible creation of a Destroyer into building the replacement transaction, which accumulates in a possible ship destruction. I don’t like the use of the word Transaction all the time. Let’s rename.

    override fun finishInteraction(): Transaction {
        return if (elapsedTime > U.MAKER_DELAY && safeToEmerge) {
            replaceTheShip()
        } else {
            Transaction()
        }
    }

    private fun replaceTheShip(): Transaction {
        return Transaction().also {
            it.add(ship)
            it.add(ShipChecker(ship))
            it.remove(this)
            it.accumulate(possibleHyperspaceFailure())
        }
    }

    private fun possibleHyperspaceFailure(): Transaction {
        return if (hyperspaceFails()) destroyTheShip()
        else Transaction()
    }

    private fun destroyTheShip(): Transaction {
        return Transaction().also {
            it.add(SolidObject.shipDestroyer(ship))
            it.add(SolidObject.splat(ship))
        }
    }

Yes, I like that better. Past time to sum up, this is getting long

Summary

We’ve replaced one very complex four-by-two state object with two much simpler ones, in about the same number of lines of code.

We took advantage of our ability to make and destroy these tiny smart objects and created one, ShipChecker, that just notices that the ship is gone, and when it notices, creates a ShipMaker and removes itself so that it won’t get all excited while the ship is being respawned.

The other new object, ShipMaker, waits a while, then spans the ship at its given position and motion, creates a new ShipChecker, and, if it’s handling a hyperspace emergence, it also provides a ship destroyer and a splat to mark the occasion. It removes itself, the maker, so that it won’t make another ship until needed.

On the one hand, I think this is much easier to think about than the state machine version was, and it took me far less time and thinking to do it this way. On the other hand, the overall story is still a bit complicated. I might tell it this way:

Ship Life Cycle Management
The ship can be killed by a collision, or disappear by entering hyperspace. Whenever the ship exists, we have a ShipChecker that just makes sure the ship is still there. If the ship is gone, the Checker creates a ShipMaker and destroys itself.

The ShipMaker waits a while, and until there’s nothing about to collide with wherever the ship is going to reappear. Then it adds the ship and a new ShipChecker, and removes itself.

If this is a hyperspace emergence, and it goes wrong, the ShipMaker also creates a destroyer to kill the ship as soon as it appears, and a splat to make a nice little explosion

That story is not so bad, but the code doesn’t tell the story quite that well. Still, it’s not bad. Possibly, I’ll see whether we can get it even more clear, but for this morning, I think I’ve done just as I set out to do.

One important issue remains, which is that because of the way we roll the dice internally to the build function, we can’t test that part directly. I think we can find a way around that. Maybe next time.

For now, I think it’s better. If you do not, or have other comments, Tweet me up. See you next time!