Kotlin 133: Is it safe out there?

GitHub Repo

Before a respawned ship appears, the game is supposed to check to be sure that it won’t emerge colliding. How, in our distributed system, can we do that? At this writing, I do not know.

I also don’t know whether I’m writing into the void even more than usual today. My site was down all day yesterday, and still is today. I can’t reach it at all from 50 miles away. Internationally, people get various messages but it seems to be mostly down for the world as well. I am dismayed: out there somewhere are my two, maybe three readers, wanting their next Ron Jeffries fix.

For now, I’ll distract myself with a problem that I do not, at this writing, know how to solve. I can express what we need in code. If we change this:

    override fun update(deltaTime: Double): List<IFlyer> {
        elapsedTime += deltaTime
        var toBeCreated: List<IFlyer> = emptyList()
        state = when (state) {
            HaveSeenShip -> LookingForShip
            LookingForShip -> {
                elapsedTime = 0.0
                WaitingToCreate
            }
            WaitingToCreate -> {
                if (elapsedTime >= 3.0) {
                    toBeCreated = listOf(shipReset())
                    HaveSeenShip
                } else WaitingToCreate
            }
        }
        return toBeCreated
    }

To this:

    WaitingToCreate -> {
        if (elapsedTime >= 3.0 && safeToEmerge()) {
            toBeCreated = listOf(shipReset())
            HaveSeenShip
        } else WaitingToCreate
    }

And if safeToEmerge() actually worked, we’d be done.

Coding this gives me a partial idea. I can’t directly code safetoEmerge() to check: ShipMonitor doesn’t have access to anything that knows what’s where. In fact there is no object in the system that knows what’s where. Flyers know where they are individually, and the Flyers collection just knows all the flyers. But my idea is this:

If ShipMonitor had q function that could be called to say “it’s safe”, it could store that information and my safeToEmerge() could look like this:

    private fun safeToEmerge() {
        return storedSafetyFlat
    }

So … what if we had a new special Flyer that could sit at some location, and check to see if anything was colliding with it, and report that info back to the ShipMonitor? The ShipMonitor could create it when it wants to know if an area is clear. We could even use the object to check hyperspace emerging, when and if we get around to that.

How would the thing work? It would want to participate in collision interactions. However, instead of returning the info to destroy itself and whatever it collides with, it would just message ShipMonitor (which it would have to have a copy of) that it didn’t see a collision.

That seems simple, but it’s not quite that easy. Let’s think a bit more deeply.

Aside: Notice how I’m playing with solutions, trying to find one that seems simple enough. I think of and discard ideas. I certainly thought of more and discarded more than I was able to record: some ideas pop in and get rejected almost unconsciously.

We are in ShipMonitor update while all this spawning goes on. The cycle is to call update, then processInteractions. We can’t know whether this new object, SafetyChecker, will be called to update before or after us, so we can’t have it just clear its flag on update: if it went ahead of us, it would have set itself to safe and we’d always see it in that form even if after the interactions it would be unsafe.

Suppose the SafetyChecker just sets a flag to unsafe if it gets in a collision, and never sets it to safe. Could we arrange that ShipMonitor sets the SafetyChecker to “safe” in one update, and then, having done that, checks it next time around.

Seems weird. What if SafetyChecker sent a message to ShipMonitor whenever it sees a collision. That would be easy to implement. So imagine it’s repeatedly saying “not now” if it senses danger. In ShipMonitor, could we implement our states to make use of this flag?

We have the state WaitingToCreate. What if it looked like this:

    WaitingToCreate -> {
        if (elapsedTime < 3.0) {
            WaitingToCreate
        }else if (elapsedTime >= 3.0) {
            if ( safeToEmerge) {
                toBeCreated = listOf(shipReset())
                HaveSeenShip
            } else {
                safeToEmerge = true
                WaitingToCreate
            }
        } else WaitingToCreate
    }

That’s a bit nasty but if there was an object sending us this:

    fun unsafe() {
        safeToEmerge = false
    }

I think it would do the job. That state transition is too complicated, and is showing that, if we are to go this way, we need at least one more state. Ideally we might have WaitingForTimer, then WaitingForSafety.

Aside: A simpler idea has appeared. This is the value of thinking: once in a while we think a useful thought.

Another idea comes to mind: The ShipMonitor already participates in collisions with all other objects. What if we change this:

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        }
        return emptyList() // no damage done here
    }

Could we change that so that if anything else comes by that would collide with some known point of emergence), we’d set the flag ourselves. That would allow us to make a different more favorable distance check. We’re going to see all the objects anyway. Let’s try something.

We’re spiking here. We do not promise to keep this and we should probably throw it away … unless we don’t. (I’m not a fanatic about spikes. If I like what I get, I allow myself to keep it. Don’t try this at home.)

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        } else {
            checkSafety(other)
        }
        return emptyList() // no damage done here
    }

    private fun checkSafety(other:IFlyer) {
        if (Point(5000.0, 5000.0).distanceTo(other.position) < 2000.0) {
            safeToEmerge = false
        }
    }

    WaitingToCreate -> {
        if (elapsedTime < 3.0) {
            WaitingToCreate
        }else if (elapsedTime >= 3.0) {
            if (safeToEmerge) {
                toBeCreated = listOf(shipReset())
                HaveSeenShip
            } else {
                safeToEmerge = true
                WaitingToCreate
            }
        } else WaitingToCreate
    }

In game play, this code delays spawning until a circle of radius 2000 is clear. I know because I drew the circle. So this works. I think it’ll break at least one test. Let’s see.

[on time] 
expected: HaveSeenShip
 but was: WaitingToCreate

Right. In this test, safeToEmerge will not be set because the test isn’t calling the interaction code:

    @Test
    fun `delayed creation of ship`() {
        val ship = Flyer.ship(Point(10.0, 10.0))
        val mon = ShipMonitor(ship)
        assertThat(mon.state).isEqualTo(ShipMonitorState.HaveSeenShip)
        var created = mon.update(1.0/60.0)
        assertThat(mon.state).isEqualTo(ShipMonitorState.LookingForShip)
        assertThat(created).isEmpty()
        created = mon.update(1.0/60.0)
        assertThat(mon.state).isEqualTo(ShipMonitorState.WaitingToCreate)
        assertThat(created).isEmpty()
        created = mon.update(1.0)
        assertThat(mon.state).describedAs("too soon").isEqualTo(ShipMonitorState.WaitingToCreate)
        assertThat(created).describedAs("too soon").isEmpty()
        created = mon.update(2.1)
        assertThat(mon.state).describedAs("on time").isEqualTo(ShipMonitorState.HaveSeenShip)
        assertThat(created).describedAs("on time").contains(ship)
        assertThat(ship.position).isEqualTo(Point(5000.0, 5000.0))
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)
    }

If we set the safety flag prior to the update (2.1) we should be good to go.

        assertThat(mon.state).describedAs("too soon").isEqualTo(ShipMonitorState.WaitingToCreate)
        assertThat(created).describedAs("too soon").isEmpty()
        mon.safeToEmerge = true
        created = mon.update(2.1)
        assertThat(mon.state).describedAs("on time").isEqualTo(ShipMonitorState.HaveSeenShip)
        assertThat(created).describedAs("on time").contains(ship)
        assertThat(ship.position).isEqualTo(Point(5000.0, 5000.0))
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)

Tests are green. Let’s clean up the code. A radius of 2000 is quite large, and it took a long time for the area to clear while I was testing. So it needs to be smaller. I think I’ll go with 1000.

class ShipMonitor ...
    private val safeShipDistance = 1000.0

    private fun checkSafety(other:IFlyer) {
        if (Point(5000.0, 5000.0).distanceTo(other.position) < safeShipDistance) {
            safeToEmerge = false
        }
    }

Test. Green. Commit: Ship waits to respawn until radius 1000 around it is clear.

Now this method is just too busy:

    override fun update(deltaTime: Double): List<IFlyer> {
        elapsedTime += deltaTime
        var toBeCreated: List<IFlyer> = emptyList()
        state = when (state) {
            HaveSeenShip -> LookingForShip
            LookingForShip -> {
                elapsedTime = 0.0
                WaitingToCreate
            }
            WaitingToCreate -> {
                if (elapsedTime < 3.0) {
                    WaitingToCreate
                }else if (elapsedTime >= 3.0) {
                    if (safeToEmerge) {
                        toBeCreated = listOf(shipReset())
                        HaveSeenShip
                    } else {
                        safeToEmerge = true
                        WaitingToCreate
                    }
                } else WaitingToCreate
            }
        }
        return toBeCreated
    }

I think that first if guard clause isn’t helping me. This is better:

     WaitingToCreate -> {
         if (elapsedTime >= 3.0) {
             if (safeToEmerge) {
                 toBeCreated = listOf(shipReset())
                 HaveSeenShip
             } else {
                 safeToEmerge = true
                 WaitingToCreate
             }
         } else WaitingToCreate
     }

It’s still inherently tricky, though, because that flag is being set elsewhere, so it’s not obvious what’s going on. What if we said this:

    WaitingToCreate -> {
        if (elapsedTime >= 3.0) {
            if (safeToEmerge) {
                toBeCreated = listOf(shipReset())
                HaveSeenShip
            } else {
                startCheckingForSafeEmergence()
                WaitingToCreate
            }
        } else WaitingToCreate
    }

That seems to me to communicate that something is going on that we’re just starting here.

And up in the interaction code …

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        } else {
            checkSafety(other)
        }
        return emptyList() // no damage done here
    }

This code is hiding a side effect of setting the flag. Let’s make it a function instead:

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        } else {
            if (tooClose(other)) safeToEmerge = false
        }
        return emptyList() // no damage done here
    }

    private fun tooClose(other:IFlyer): Boolean {
        return (Point(5000.0, 5000.0).distanceTo(other.position) < safeShipDistance)
    }

I’m not in love with this, but it passes the tests and works as intended. Commit the tidying. Let’s sum up.

Summary

The good news is that we now have this needed capability, holding off from spawning the ship until its spawning pool is clear. And all the details are in just one object, the ShipMonitor.

The bad news, in my view, is that it is a very tricky object. In particular, when we are in the state WaitingToCreate, there are essentially two substates, not known to be safe and known to be safe. And those are not explicit in the state code.

Let’s abandon summarizing for a moment and see if we can fix that. Let’s have another state, and probably rename the WaitingToCreate one … let’s see … WaitingForTime, and WaitingForSafety?

Let’s improve our tests to reflect this new behavior:

    @Test
    fun `delayed creation of ship`() {
        val ship = Flyer.ship(Point(10.0, 10.0))
        val mon = ShipMonitor(ship)
        assertThat(mon.state).isEqualTo(ShipMonitorState.HaveSeenShip)
        var created = mon.update(1.0/60.0)
        assertThat(mon.state).isEqualTo(ShipMonitorState.LookingForShip)
        assertThat(created).isEmpty()
        created = mon.update(1.0/60.0)
        assertThat(mon.state).isEqualTo(ShipMonitorState.WaitingForTime)
        assertThat(created).isEmpty()
        created = mon.update(1.0)
        assertThat(mon.state).describedAs("too soon").isEqualTo(ShipMonitorState.WaitingForTime)
        assertThat(created).describedAs("too soon").isEmpty()
        created = mon.update(2.1)
        assertThat(mon.state).describedAs("time OK").isEqualTo(ShipMonitorState.WaitingForSafety)
        assertThat(created).describedAs("not yet").isEmpty()
        mon.safeToEmerge = true
        created = mon.update(0.01)
        assertThat(created).describedAs("safe").contains(ship)
        assertThat(ship.position).isEqualTo(Point(5000.0, 5000.0))
        assertThat(ship.velocity).isEqualTo(Velocity.ZERO)
    }

I think that’s right. Kotlin and IDEA want some changes:

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

And in the code:

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

    override fun interactWith(other: IFlyer): List<IFlyer> {
        if (state == LookingForShip) {
            if (other == ship)
                state = HaveSeenShip
        } else if (state == WaitingForSafety){
            if (tooClose(other)) safeToEmerge = false
        }
        return emptyList() // no damage done here
    }

Tests are green. New states work, and I think they help with clarity.

One more thing:

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

I’m still not loving this but it’s better. Test green, commit: added WaitingForTime and WaitingForSafety to ShipMonitorState, and removed WaitingToCreate to improve clarity of handling safe emergence.

Back to Summary:

Summary (continued)

OK, that’s better. The feature went in quite easily, always a good sign. I think we could argue that ShipMonitor handles just one thing: ensure ships respawn safely. It does have a sort of two-phase operation, between update and interaction, but that’s the nature of all our objects.

What issues am I feeling? Well, we still have an issue with constants, like the very magical Point(5000.0,5000.0), which is really something like CenterOfUniverse. We have a lot of magical values in the code and they lead to problems updating, because often all of the occurrences have to be changed at the same time, to the same value, and we miss one or more.

And I don’t really like that long story test for the ShipMonitor update. Maybe we should break it into lots of tiny steps, though I do like the fact that this way shows the actual flow once you read through the complexity.

There is something weird about the way the specials, especially ShipMonitor work … they can set an internal state in update, adjust that state in interactWith, and then check it, the next time around, in update. That works beautifully, but it’s far from obvious. Any object that does that is toggling between at least two states. I’d like the code to express that better. I’ll try to think about that.

But overall,I am pleased with the result and hope that someday you’ll be able to read the article.

See you next time … I hope …