GitHub Repo

the OneShot and DeferredAction are looking for places to be used. It’s ‘small boy with hammer’ time!

Let’s look at some users of elapsedTime to see whether DeferredAction can help them.

There are a few possibilities. In Missile we have this code:

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
        if (elapsedTime > lifetime) trans.remove(this)
        position = (position + velocity * deltaTime).cap()
    }

We could replace the first two lines with a deferred action that removes the Missile:

    private val oneShot = OneShot(3.0) { it.remove(this)}

    override fun update(deltaTime: Double, trans: Transaction) {
        oneShot.execute(trans)
        position = (position + velocity * deltaTime).cap()
    }

That works. Let’s rename our oneShot:

    private val timeOut = OneShot(3.0) { it.remove(this)}

    override fun update(deltaTime: Double, trans: Transaction) {
        timeOut.execute(trans)
        position = (position + velocity * deltaTime).cap()
    }

I wonder: couldn’t we just do the whole thing in line, avoiding the variable?

Yes, but it’s weird:

        OneShot(3.0) { it.remove(this )}.execute(trans)

I don’t think I like that, we’ll put it back at timeOut.

Musing …

What we could do is allow something like this:

OneShot(3.0).execute(trans) { it.remove(this) }

That would require a default action in the constructor, easily enough done, and an overloaded execute that would also be straightforward. Or we could use a slightly different syntax like this:

OneShot().executeDelayed(3.0, trans) { it.remove(this) }

Still a pretty cryptic syntax. We’ll belay that idea for now.

Back on Main Thread …

After the OneShot timeout goes in, we have a test failing, because it’s expecting edits in a transaction that no longer occur. We’ll deal with that shortly. We can do a bit more work in the OneShot and remove finalize:

    private val timeOut = OneShot(3.0) { 
        it.remove(this)
        it.add(Splat(this))
    }

That lets us remove the finalize method, and its subscription. Righteous in my view. Let’s fix up that test. Er, tests, because moving the splat creation broke another test. Readily fixed:

    @Test
    fun `splat death`() {
        val mix = SpaceObjectCollection()
        val ship = Ship(
            position = U.randomPoint()
        )
        val missile = Missile(ship)
        mix.add(missile)
        val game = Game(mix)
        assertThat(mix.contains(missile)).isEqualTo(true)
        game.cycle(0.0)
        assertThat(mix.any { it is DeferredAction }).describedAs("deferred action should be present").isEqualTo(true)
        game.cycle(3.1)
        assertThat(mix.contains(missile)).describedAs("missile should be dead").isEqualTo(false)
        assertThat(mix.any { it is Splat }).describedAs("splat should be present").isEqualTo(true)
        game.cycle(5.2)
        assertThat(mix.any { it is Splat }).describedAs("splat should be gone").isEqualTo(false)
    }

That runs green. Now the other … just checks whether the missile dies. The test we just did does that. Rename it, remove the other.

    @Test
    fun `missile and splat death`() {

Commit: Missile uses OneShot DeferredAction to time out.

We can do the Splat the same way:

    var elapsedTime = 0.0
    private val timeOut = OneShot(U.SPLAT_LIFETIME) { it.remove(this) }

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
        timeOut.execute(trans)
    }

This is not so righteous in my opinion. The old code was simpler in that it did the remove directly and we don’t get to eliminate elapsed time because the view needs it. We could argue for consistency but this is pretty clear:

    var elapsedTime = 0.0
    private val lifetime = U.SPLAT_LIFETIME
    private var view = SplatView(lifetime)

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

I do like the new OneShot and DeferredAction objects but in this case, they’re just not justified. This nail survives. We’ll leave this one as it is.

What about ShipMaker? It does some timing, here:

        afterInteractions = { trans->
            if (inHyperspace() || elapsedTime > U.MAKER_DELAY && safeToEmerge) {
                replaceTheShip(trans)
            }
        }

What we really want here, though, is no delay if we’re in hyperspace and otherwise a delay until it i safe to emerge. We don’t want to use the OneShot if we’re in hyperspace, so we’d have to change that logic and we only really have the very simple update code to work with:

    override fun update(deltaTime: Double, trans: Transaction) {
        elapsedTime += deltaTime
    }

But … the rules for hyperspace is that we emerge immediately, and if we hit something too bad, and there’s an added chance of a malfunction, while emerging from being shot down or crashing, we get a delay and safe entry. When we replaceTheShip in ShipMaker, we always go through the HyperspaceOperation, which checks to see if you’re in hyperspace.

Maybe we could sort this out in ShipChecker, whose current code is this:

afterInteractions = { trans ->
    if ( missingShip && scoreKeeper.takeShip()) {
        trans.add(ShipMaker(ship, scoreKeeper))
        trans.remove(this)
    }
}

ShipChecker is removing itself because it doesn’t want to run while it waits for the maker. We could “fix” that with a OneShot, leave the Checker running, and not need to restart it in the Maker. That would simplify some things at least. But we can’t issue a long delay here because hyperspace is supposed to be immediate (according to rules that I just guessed at).

I’m going to set this aside, since this was just a few moments’ afternoon playtime, but what we might do is something like this:

  1. In Checker, create two OneShots:
    • a long one that includes the code to check for safe emergence;
    • a short one that includes the code for Hyperspace malfunction;
  2. Trigger whichever OneShot is called for, based on whether the ship is in hyperspace.
  3. Remove ShipMaker altogether
  4. Rename ShipChecker to ShipMaker?

For now, though, we’ll sum up.

Summary

When I did the commit above, I noticed that I had apparently skipped a large commit this morning, so I committed all whatever it was. A better person would have sorted through the diffs and done multiples. Since few people have told me they’re following the repo, I took the easy way out. I am not a good person, but I’m so good looking that I forgive myself.

Like so many hammers, the OneShot/DeferredAction is looking for a nail. We found one in Missile, but Splat is already so simple that the object doesn’t help enough to be worth the additional complexity. I’d like to argue that it’s really pretty simple and we use it all over and so it’ll be just fine, but replacing a single if statement … just not enough benefit.

I think there’s a very good chance that the OneShot will simplify ship making a bit and a decent chance that we’ll be able to save a whole object or create two simpler ones, perhaps a SafeShipMaker and HyperspaceShipMaker. We’ll see how that goes.

But not today. Today, we’re done. It’s the weekend. See you next time!