GitHub Decentralized Repo
GitHub Centralized Repo

I have a cunning plan for doing the ship insertion in the centralized version. I predict that this is going to be nice.

Yesterday afternoon, we patched in some simple code that used a OneShot and it’s associated DeferredAction to rez the ship a few seconds after its unfortunate demise. That worked exactly as intended. Left out, intentionally, was checking that it was safe to emerge, which is a feature of the game that ensures that the ship doesn’t pop in and immediately get run over by an asteroid.

Yesterday, I did not see quite how I was going to accomplish that feature, though of course I was sure there would be a way to do it. I was thinking that in the worst case we’d set up some kind of dual logic in the game where the DeferredAction would call back and set a flag on the game that would put it in a mode of checking safety and rezzing the ship. Or, I thought, maybe I could use one DeferredAction to spawn another if it wasn’t safe to emerge.

This morning, I have a new idea that seems just right, a new object, DeferredConditionalAction.

Let’s review DeferredAction. I honestly don’t remember how it works, just what it does. I know it’s quite simple:

class DeferredAction(
    val delay: Double,
    initialTransaction: Transaction,
    private val action: (Transaction) -> Unit
) : ISpaceObject, InteractingSpaceObject {
    var elapsedTime = 0.0

    init {
        elapsedTime = 0.0
        initialTransaction.add(this)
    }

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
        if (elapsedTime >= delay ) {
            action(trans)
            trans.remove(this)
        }
    }

Right. On its update, it just checks to see if time has elapsed and if it has, it executes its action and removes itself from the mix, using the provided transaction. Of course, in this “centralized” version of the game, the deferred action doesn’t really go into the mix, it goes into a special list in knownObjects. As such, it doesn’t really interact with any other objects.

I’m proposing a new class, DeferredConditionalAction, that has an additional parameter, a function returning a Boolean and checking that in addition to checking the time. I suspect that we could even just add an optional parameter to DeferredAction, but for now, my idea is a new very similar class.

Tentative Plan

Let’s begin by providing the function that we’ll want to check. Game needs to be able to answer whether it is safe for the Ship to emerge. We’ll test-drive that.

Then we’ll test-drive the DeferredConditionalAction object.

Then, we’ll install it, much as we spiked yesterday afternoon.

Let’s get to it:

Can ship emerge?

    @Test
    fun `saucer makes it unsafe for ship`() {
        val mix = SpaceObjectCollection()
        val game = Game(mix)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val saucer= Saucer()
        mix.add(saucer)
        assertThat(game.canShipEmerge()).isEqualTo(false)
    }

We want it to be unsafe if the saucer exists, or if any asteroids are too close. This is more than enough test for right out of the box. We need the method.

    fun canShipEmerge(): Boolean {
        return false
    }

The test should fail on the first assert. It does. Let’s make it work. We’ll check all the target objects, but in the end we need to check missiles as well. We’ll need a bit more testing to drive it all out.

    fun canShipEmerge(): Boolean {
        for ( target in knownObjects.targets ) {
            if ( target is Saucer ) return false
        }
        return true
    }

Test should run. It does. Another test, for asteroids.

    @Test
    fun `asteroid too close makes it unsafe for ship`() {
        val mix = SpaceObjectCollection()
        val game = Game(mix)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val asteroid= Asteroid(Point(100.0, 100.0))
        mix.add(asteroid)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val dangerousAsteroid = Asteroid(U.CENTER_OF_UNIVERSE + Point(50.0, 50.0))
        assertThat(game.canShipEmerge()).isEqualTo(false)
    }

That’ll fail, on the final check. It does. Implement:

    fun canShipEmerge(): Boolean {
        for ( target in knownObjects.targets ) {
            if ( target is Saucer ) return false
            if ( target is Asteroid ) {
                val distance = target.position.distanceTo(U.CENTER_OF_UNIVERSE)
                if ( distance < U.SAFE_SHIP_DISTANCE ) return false
            }
        }
        return true
    }

Test should run. It fails. Careful study shows me that I didn’t add the dangerous asteroid to the mix. Duh.

    @Test
    fun `asteroid too close makes it unsafe for ship`() {
        val mix = SpaceObjectCollection()
        val game = Game(mix)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val asteroid= Asteroid(Point(100.0, 100.0))
        mix.add(asteroid)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val dangerousAsteroid = Asteroid(U.CENTER_OF_UNIVERSE + Point(50.0, 50.0))
        mix.add(dangerousAsteroid)
        assertThat(game.canShipEmerge()).isEqualTo(false)
    }

Test should run. It does. Good so far.

Now currently, missiles only live for 3 seconds, and that’s the emergence delay, so in principle, currently, we don’t need to check. But we might change the timing, so let’s ensure that there are no missiles.

    @Test
    fun `missile makes it unsafe for ship`() {
        val mix = SpaceObjectCollection()
        val game = Game(mix)
        assertThat(game.canShipEmerge()).isEqualTo(true)
        val missile= Missile(Point(100.0, 100.0))
        mix.add(missile)
        assertThat(game.canShipEmerge()).isEqualTo(false)
    }

This should fail. It does. Implement:

    fun canShipEmerge(): Boolean {
        for ( target in knownObjects.targets ) {
            if ( target is Saucer ) return false
            if ( target is Asteroid ) {
                val distance = target.position.distanceTo(U.CENTER_OF_UNIVERSE)
                if ( distance < U.SAFE_SHIP_DISTANCE ) return false
            }
        }
        for ( attacker in knownObjects.attackers ) {
            if ( attacker is Missile ) return false
            if ( attacker is Saucer ) return false // already checked but hey
        }
        return true
    }

I decided to do the redundant check for Saucer. Can’t hurt, might help. Test should be green. It is.

Commit: Game can answer canShipEmerge.

DeferredConditionalAction

Do we have a DeferredAction test? We have! How nice. It looks like this:

class DeferredActionTest {
    var done = false
    @Test
    fun `triggers after n seconds`() {
        val trans = Transaction()
        DeferredAction(2.0, trans) { _ -> done = true}
        val tmw = trans.firstAdd()
        val newTrans = Transaction()
        tmw.update(1.1, newTrans)
        assertThat(done).isEqualTo(false)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).isEmpty()
        tmw.update(1.1, newTrans)
        assertThat(done).isEqualTo(true)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).contains(tmw)
    }
}

I think we can pretty much duplicate this test with an instance of our new object. I’ll try to make the new test more clear if I can.

I think this test is good:

    @Test
    fun `conditional action triggers after n seconds and true condition`() {
        val trans = Transaction()
        var ready = false
        val cond = { ready }
        val dca = DeferredConditionalAction(2.0, cond, trans) { _ -> done = true }
        val newTrans = Transaction()
        dca.update(1.1, newTrans)
        assertThat(done).isEqualTo(false)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).isEmpty()
        dca.update(1.1, newTrans)
        assertThat(done).isEqualTo(true)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).contains(dca)
    }

I just need the DeferredConditionalAction class. I’ll build it inside the DeferredCondition file, because it’s virtually the same. (So much the same that I’m tempted to just create another constructor. Maybe later.)

class DeferredConditionalAction(
    val delay: Double,
    val cond: () -> Boolean,
    initialTransaction: Transaction,
    private val action: (Transaction) -> Unit
) : ISpaceObject, InteractingSpaceObject {
    var elapsedTime = 0.0

    init {
        elapsedTime = 0.0
        initialTransaction.add(this)
    }

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
        if (elapsedTime >= delay && cond() ) {
            action(trans)
            trans.remove(this)
        }
    }

    override val subscriptions: Subscriptions = Subscriptions()
    override fun callOther(other: InteractingSpaceObject, trans: Transaction) {}
}

I actually expect this to work. Let’s find out why it won’t. Well, there are reasons. Let’s review the test for correctness. I got ahead of myself.

    @Test
    fun `conditional action triggers after n seconds and true condition`() {
        val trans = Transaction()
        var ready = false
        val cond = { ready }
        val dca = DeferredConditionalAction(2.0, cond, trans) { _ -> done = true }
        val newTrans = Transaction()
        dca.update(1.1, newTrans)
        assertThat(done).isEqualTo(false)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).isEmpty()
        dca.update(1.1, newTrans)
        assertThat(done).isEqualTo(true)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).contains(dca)
    }

OK, let’s do this right.

    @Test
    fun `conditional action triggers after n seconds and true condition`() {
        val trans = Transaction()
        var ready = false
        val cond = { ready }
        val dca = DeferredConditionalAction(2.0, cond, trans) { _ -> done = true }
        val newTrans = Transaction()
        dca.update(1.1, newTrans)
        assertThat(done).describedAs("not time yet").isEqualTo(false)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).isEmpty()
        dca.update(1.1, newTrans)
        assertThat(done).describedAs("not ready yet").isEqualTo(false)
        ready = true
        dca.update(0.1, newTrans)
        assertThat(done).describedAs("time up and ready").isEqualTo(true)
        assertThat(newTrans.adds).isEmpty()
        assertThat(newTrans.removes).contains(dca)
    }

I wasn’t setting the ready flag. Now the test should run green, and it does. Commit: DeferredConditionalAction class implemented.

Now we’re ready to plug the new class in, but first I need a break. I’ll be back soon.

Ship creation

Break accomplished, I think we’re ready. I’m going to put this in without a test. I think some existing tests will serve and I fully intend to test it in game. A test first might be more wise, but I didn’t get any wiser while on break.

We’ll create a new OneShot:

Oh! WRONG! OneShot always creates a DeferredAction! Look:

class OneShot(private val delay: Double, private val action: (Transaction)->Unit) {
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        deferred = deferred ?: DeferredAction(delay, trans) {
            deferred = null
            action(it)
        }
    }

    fun cancel(trans: Transaction) {
        deferred?.let { trans.remove(it); deferred = null }
    }
}

We’re not ready.

What can we do?

The easiest thing to do, I think, might be to create a new OneShot that creates a DCA.

Another possibility would be to change OneShot to always create a DCA, providing a true condition as a default.

Yet another possibility might be to extend DeferredAction to have a second constructor and do the same with OneShot.

Whichever way we go, we’re a bit derailed.

I think I should a new OneShot and worry about improving the situation later. But there are quite a few tests for OneShot and I should redo those for the new object if I do it.

I’m going to spike something. I’m going to try to make DeferredAction have two constructors and a new conditional that defaults to true but is always there.

Here’s what we do:

What I decide to do is to make DeferredConditionalAction have two constructors:

class DeferredConditionalAction(
    val delay: Double,
    val cond: () -> Boolean,
    initialTransaction: Transaction,
    private val action: (Transaction) -> Unit
) : ISpaceObject, InteractingSpaceObject {
    constructor(delay: Double, initialTransaction: Transaction, action: (Transaction) -> Unit):
            this(delay, { true }, initialTransaction, action)

With this in place, I think we can replace all references to DeferredAction with DeferredConditionalAction. Let’s try that. I still consider myself to be spiking, but this might work, in which case I’ll keep it.

I replace them all and run my tests. Green. Try the game just to be sure. Game works.

I’m going with this. Remove DeferredAction safely. Still green. Rename DeferredConditionalAction to DeferredAction. Still green.

Commit: Merge DeferredConditionalAction and DeferredAction into one class, DeferredAction with optional conditional.

Now then. Currently in OneShot, we find this:

class OneShot(private val delay: Double, private val action: (Transaction)->Unit) {
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        deferred = deferred ?: DeferredAction(delay, trans) {
            deferred = null
            action(it)
        }
    }

    fun cancel(trans: Transaction) {
        deferred?.let { trans.remove(it); deferred = null }
    }
}

Let’s modify OneShot to have a conditional parameter and give it a new constructor that defaults it, just like we did with DeferredAction. Existing tests should suffice for now.

class OneShot(private val delay: Double, val cond: ()->Boolean, private val action: (Transaction)->Unit) {
    constructor (delay: Double, action: (Transaction)->Unit):
            this(delay, { true }, action)
    var deferred: DeferredAction? = null
    fun execute(trans: Transaction) {
        deferred = deferred ?: DeferredAction(delay, cond, trans) {
            deferred = null
            action(it)
        }
    }

    fun cancel(trans: Transaction) {
        deferred?.let { trans.remove(it); deferred = null }
    }
}

That looks good. Test. Tests are green. I should create a test for the new kind of OneShot. A truly wonderful person might have done that first.

    @Test
    fun `condition is honored`() {
        val trans = Transaction()
        var ready = false
        var count = 0
        val once = OneShot(2.0, { ready }) { count += 1}
        assertThat(count).isEqualTo(0)
        once.execute(trans)
        val defer = trans.firstAdd()
        defer.update(0.1, trans)
        assertThat(count).describedAs("not yet").isEqualTo(0)
        defer.update(2.2, trans)
        assertThat(count).describedAs("not ready").isEqualTo(0)
        ready = true
        defer.update(0.1, trans)
        assertThat(count).describedAs("ready").isEqualTo(1)
    }

The test runs. count doesn’t increase until we’re ready. This is righteous. Commit: OneShot accepts optional conditional parameter.

OK. I think now we’re actually ready to do the delay thing. I am tired, however, and so I’ll just spike it in and we’ll see what happens. I’ll probably roll it back. Probably.

class Game(val knownObjects:SpaceObjectCollection = SpaceObjectCollection()) {
    private var lastTime = 0.0
    private var numberOfAsteroidsToCreate = 0
    private val saucer = Saucer()
    private lateinit var ship: Ship

    private val waveOneShot = OneShot(4.0) { makeWave(it) }
    private val saucerOneShot = OneShot( 7.0) {it.add(saucer)}
    private val shipOneShot = OneShot(U.SHIP_MAKER_DELAY, { canShipEmerge() }) { it.add(ship) }
    // all OneShot instances go here:
    private val allOneShots = listOf(waveOneShot, saucerOneShot, shipOneShot)

    private fun createInitialObjects(
        trans: Transaction,
        shipCount: Int,
        controls: Controls
    ) {
        cancelAllOneShots()
        trans.clear()
        val scoreKeeper = ScoreKeeper(shipCount)
        knownObjects.scoreKeeper = scoreKeeper
        val shipPosition = U.CENTER_OF_UNIVERSE
        ship = Ship(shipPosition, controls)
//        val shipChecker = ShipChecker(ship, scoreKeeper)
//        trans.add(shipChecker)
    }

I remembered to create and save the ship, because it needs to stay connected to the controls.

Test the game. Minor detail, forgot to check for the ship!

    fun cycle(elapsedSeconds: Double, drawer: Drawer? = null) {
        val deltaTime = elapsedSeconds - lastTime
        lastTime = elapsedSeconds
        tick(deltaTime)
        beginInteractions()
        processInteractions()
        finishInteractions()
        U.AsteroidTally = knownObjects.asteroidCount()
        createNewWaveIfNeeded()
        createSaucerIfNeeded()
        createShipIfNeeded() // <=== this was added
        drawer?.let { draw(drawer) }
    }

    private fun createShipIfNeeded() {
        if ( knownObjects.attackers
        	.find { it is Ship } != null ) return
        val trans = Transaction()
        shipOneShot.execute(trans)
        knownObjects.applyChanges(trans)
    }

That should work. Try again. It sort of works, but not quite. The game starts right up running. We need not to add the ship if there aren’t any to take.

I tried that before triggering the OneShot but that consumed all the ships right away. The check goes here:

    private val shipOneShot = OneShot(U.SHIP_MAKER_DELAY, { canShipEmerge() }) {
       if ( scoreKeeper.takeShip() ) it.add(ship) 
    }

I had to cache the scoreKeeper as a member variable to make that work,. I used lateinit for now.

That works as advertised with one tiny glitch. The game screen stays blank for a bit before it displays GAME OVER. That’s because, the old way, the ShipChecker runs right away and consumes the non-ship from our initial setup, so the ScoreKeeper knows it is out of ships. In the new scheme, we don’t check until the ship delay has elapsed. All we need to do to fix that is to init shipCount to -1 in the initial setup.

    fun createInitialContents(controls: Controls) {
        initializeGame(controls, 0)
    }

That becomes:

    fun createInitialContents(controls: Controls) {
        initializeGame(controls, -1)
    }

Should come up GAME OVER instantly now. Yes. The game works properly. Let’s run the tests and see what breaks.

One of the tests objects to there being no scoreKeeper: lateinit is never a great idea. And another finds three objects where it expects two. If I recall from yesterday, that’s going to be correct.

But I’m tired and the article is long enough. I have some other things to do today as well.

I’m going to pause on a red test, which will remind me what to work on when I come back. Let’s sum up.

Summary

This went well.

canShipEmerge
First, I test-drove the canShipEmerge function. That had quite a few cases to check.
DeferredConditionalAction

Then I built DeferredConditionalAction with a test. The test was originally just copied from another and wasn’t quite right, but when I got the test right, the new class worked.

Oops, not quite

At that point I thought we could just create a OneShot and build the feature, but I was mistaken, because OneShot always created a plain old DeferredAction, and we need a condition. So after just a bit of code study, I extended DeferredConditionalAction to have a second constructor that defaults the test condition. That meant that the DeferredConditionalAction could be used everywhere that DeferredAction was used.

Then we could remove DeferredAction and rename the DeferredConditionalAction to DeferredAction.

Then we extended OneShot to accept an optional conditional as well, and to pass it on to DeferredAction.

All of that was well tested and was committed.

A spike

Then, just now, I spiked in essentially the same code that I did yesterday. I discovered that the new scheme wasn’t checking to see if ships were available, which meant that the game didn’t start up in GAME OVER, and also meant that you got an infinite number of ships while playing. Can’t make any money that way.

All that was corrected and is now working in the game, with two broken tests. In the sequel, I’ll decide whether to keep this code and clean up the tests, or to do it over. I think I’ll keep the code, but I’d like to do that with a bit of time to forget the details, the better to assess how well it’s done.

Bottom line

We have a running version of the game committed, without our new scheme, and our new scheme is actually working and nearly ready to ship.

When we close this out, we’ll be able to remove ShipChecker and ShipMaker classes and probably their tests, though we might want to recast some of them to test the new scheme.

Overall, over the course of three to five sessions, this has gone quite nicely. No real problems on the road to centralizing the decisions that the game makes.

Soon, we’ll be in a position to compare the two approaches.

Next time

Next time, we’ll almost certainly ship the new capability with the checker and maker removed.

Please join me then!