Kotlin 121
I believe I learned enough yesterday to make a good job of the ShipMonitor today. One question is whether to start over, or move forward. I make the right decision, of course.
I did some design yesterday afternoon or evening, and a fair amount of thinking about my ShipMonitor idea. I think that I see what needs to be done. I’m not sure whether to start over — generally our standard advice when things don’t go right — or to simply push this into the right shape.
First, let’s sum up the idea and what it takes, in my now somewhat educated belief, to make it go.
The central idea is simple:
Put an object, the ShipMonitor, into the universe. Using the collision logic, the monitor determines whether the ship is still present. If not, The monitor uses the collision logic to destroy itself. When it is called to split, it returns itself and a ship. Voila! The ship is back.
With a bit more elaboration:
Put an object, the ShipMonitor, into the universe. The monitor, on every update-collide-split cycle, looks to see whether the ship is in the mix. If the ship is present, the monitor does nothing. If there is ever a cycle with no ship, the monitor goes “active”. On the next collision call it returns itself as destroyed. That means that the monitor will be deleted, but get a call to split during this cycle. When that happens, the monitor returns itself, and the ship, which the split logic duly puts back into the mix.
Yesterday, I discovered a fatal flaw (with yesterday’s work), namely that the way collision works, if the monitor says yes to a collision call, both it and whatever it was paired with will be split. Since the only things out there are asteroids, some (or all) asteroids would get split.
Today I have a plan for that: rather than unconditionally return both elements of a collision and split (or destroy) them, we’ll have the collision logic return whatever it wants. Regular flyers will still return the pair, but the monitor will just return itself, so that no other object is harmed.
In addition, I plan to add a state to the monitor. As we’ll talk about below, that new state isn’t necessary but it is a way to avoid some processing.
As for my design thinking, I did draw some diagrams, some of which I’ll share with you. First is a sketch of the monitor’s state diagram and its transitions:
Then we have a sketch of the code that will be needed. It’s not really intended to be read, though I can in fact make it out. The value was in the writing, not the reading. It’s not impossible that I’ll refer to it, of course. That’s why I kept it. Well, that and the fact that keeping things is easier than not. Those pictures have moved from my living room iPad to the one on my desk, to the Mac, all by themselves. I hope they never decide to rise up against us humans …
Which Path?
We’re left with the question of the path to take. Should I roll back and do it all over, or should I keep what I’ve got and edit it into submission?
If I roll back, what it’ll demonstrate is that sometimes (almost always? (always?)) starting over is better. If i push forward, it will demonstrate that even when code isn’t right, we can make it right.
To decide, let’s examine what we have. My biggest concern is that the collision logic needs to be changed, and since it is now implemented a bit differently the changes might be more extensive.
What we have:
The Flyer, née FlyingObject has been changed so that there’s an interface, IFlyer and two implementors, Flyer and ShipMonitor.1 The interface is moderately extensive:
interface IFlyer {
abstract val killRadius: Double
abstract val position: Vector2
abstract val ignoreCollisions: Boolean
fun collidesWith(other: IFlyer): Boolean
fun collidesWithOther(other: IFlyer): Boolean
fun draw(drawer: Drawer)
fun move(deltaTime: Double)
fun split(): List<IFlyer>
fun update(deltaTime: Double): List<IFlyer>
}
I think we’ll be able to get rid of some of those vals, but we’ll see. We’ll need the rest, because our ShipMonitor has to be skulking among the asteroids as if it were a real thing flying about the universe as things do.
We’ve changed the collision logic a bit. In the collision searching logic, given each unique pair of objects (a,b) we call a.collidesWith(b), which, for Flyer objects, just does b.collidesWithOther(a), which ensures that whether the monitor is first or second in the pair, the monitor’s collision logic is entered. Let me underline that:
This double dispatch ensures that when the ShipMonitor is checked for colliding, it always uses its own logic, not that of ordinary Flyers. If we didn’t do the double dispatch, sometimes the Flyer would get the job, sometimes the ShipMonitor. That would contaminate the Flyer logic.
All that is in place and the dispatches work as intended. Whenever ShipMonitor is checked for colliding, it uses its own logic, no matter who was asked first.
The ShipMonitor implements a simple state machine, using states from an enum:
enum class ShipMonitorState {
HaveSeenShip, LookingForShip, Active
}
These are probably sufficient to do what we want, but adding another state will be better, I think. Its purpose will be to have the monitor report only one collision, if it reports any, rather than reporting one on every comparison. We might rename those states for clarity. Right now they mean:
- HaveSeenShip
- ShipMonitor knows that there is a ship in the mix and is happy.
- LookingForShip
- This state is set on
update. If the ship appears, we go to HaveSeenShip. If, however, we enterupdatein LookingForShip, we know that the ship was absent on the last cycle and we go to Active. - Active
- In this state, we want to report the ShipMonitor as having collided (with something). When we do that, we are guaranteed a call to
splitas part of the collision cycle. Upon that call, we know that the monitor has been removed in this cycle, and the ship is missing. So we return the monitor and the ship as the result of the call tosplit.
Neat, huh? I think so. I could be wrong.
The glitch in the matrix is that present collision logic automatically returns both elements of a collision for deletion:
class Game
fun colliders() = flyers.pairsSatisfying { f1, f2 -> f1.collidesWith(f2) }
class Flyers
fun pairsSatisfying(pairCondition: (IFlyer, IFlyer) -> Boolean): MutableSet<IFlyer> {
val pairs = mutableSetOf<IFlyer>()
pairsToCheck().forEach { p ->
if (pairCondition(p.first, p.second)) {
pairs.add(p.first)
pairs.add(p.second)
}
}
return pairs
}
That there is yer problem right there. The list of colliders unconditionally gets both elements of the call to pairCondition, which is first.collidesWith(second).
We have to modify the collision logic so that the function called (currently collidesWith) returns the damaged objects, rather than just returning “yeah we collided”. In the Flyer case, we’ll still return both. In the ShipMonitor case, we’ll just return the monitor, not whatever we’re pretending collided with it.
And … I think we have a plan … because with that changed, I think what we have will pass the entire scenario test. So there is no need to roll back, but we do need to make a somewhat substantial change to the collision logic. But if we started over … that’s exactly what we’d have to do anyway.
Here are our implementations of collidesWith:
class Flyer
override fun collidesWith(other: IFlyer): Boolean {
return other.collidesWithOther(this)
}
override fun collidesWithOther(other: IFlyer):Boolean {
if ( this === other) return false
if ( this.ignoreCollisions && other.ignoreCollisions) return false
val dist = position.distanceTo(other.position)
val allowed = killRadius + other.killRadius
return dist < allowed
}
class ShipMonitor
override fun collidesWith(other: IFlyer): Boolean {
if (state == ShipMonitorState.Active) return true
if (other === ship) state = ShipMonitorState.HaveSeenShip
return false
}
override fun collidesWithOther(other: IFlyer): Boolean {
return collidesWith(other)
}
We want those methods to return, not a Boolean, but a list of IFlyer. And we want to change pairsSatisfying to match.
I see no clever way to do this. I’ll just tick through it.
override fun collidesWith(other: IFlyer): List<IFlyer> {
return other.collidesWithOther(this)
}
override fun collidesWithOther(other: IFlyer): List<IFlyer> {
if ( this === other) return false
if ( this.ignoreCollisions && other.ignoreCollisions) return false
val dist = position.distanceTo(other.position)
val allowed = killRadius + other.killRadius
return if (dist < allowed) listOf(this,other) else emptyList()
}
I had to change the signature in the IFlyer interface, of course. And now I have to change ShipMonitor:
override fun collidesWith(other: IFlyer): List<IFlyer> {
if (state == ShipMonitorState.Active) return listOf(this)
if (other === ship) state = ShipMonitorState.HaveSeenShip
return emptyList()
}
override fun collidesWithOther(other: IFlyer): List<IFlyer> {
return collidesWith(other)
}
Now there are a few issues, one of which is to change the pairsSatisfying code. That looks like this:
fun pairsSatisfying(pairCondition: (IFlyer, IFlyer) -> Boolean): MutableSet<IFlyer> {
val pairs = mutableSetOf<IFlyer>()
pairsToCheck().forEach { p ->
if (pairCondition(p.first, p.second)) {
pairs.add(p.first)
pairs.add(p.second)
}
}
return pairs
}
Now the pairCondition, which should be named something else, returns a collection to be added to the result. Let’s make it work, then make it right.
fun pairsSatisfying(pairCondition: (IFlyer, IFlyer) -> List<IFlyer>): MutableSet<IFlyer> {
val pairs = mutableSetOf<IFlyer>()
pairsToCheck().forEach { p -> pairs.addAll(pairCondition(p.first, p.second))
}
return pairs
}
I think we can run the tests and see what happens. I actually expect them to pass, but surely I’ve missed something. Right, forgot those early exits:
override fun collidesWithOther(other: IFlyer): List<IFlyer> {
if ( this === other) return emptyList()
if ( this.ignoreCollisions && other.ignoreCollisions) return emptyList()
val dist = position.distanceTo(other.position)
val allowed = killRadius + other.killRadius
return if (dist < allowed) listOf(this,other) else emptyList()
}
Test. There are two tests explicitly testing the old collidesWith to demonstrate indexing through things. I’ll modify those. It’s straightforward, same change as made above.
Test. One test fails:
[on top]
expected: true
but was: [com.ronjeffries.ship.Flyer@18230356, com.ronjeffries.ship.Flyer@d8305c2]
Right, gonna be same problem.
@Test
fun `collision calculation`() {
val ship = Flyer(
Vector2.ZERO,
Vector2.ZERO,
100.0
)
val asteroid = Flyer(
position = Vector2.ZERO,
velocity = Vector2.ZERO,
killRadius = 1000.0
)
assertThat(ship.collidesWith(asteroid)).describedAs("on top").isEqualTo(true)
val tooFar = Vector2(ship.killRadius + asteroid.killRadius + 1, 0.0)
var rotated = tooFar.rotate(37.0)
ship.position = rotated
assertThat(ship.collidesWith(asteroid)).describedAs("too far").isEqualTo(false)
val closeEnough = Vector2(ship.killRadius + asteroid.killRadius - 1, 0.0)
rotated = closeEnough.rotate(37.0)
ship.position = rotated
assertThat(ship.collidesWith(asteroid)).describedAs("too close").isEqualTo(true)
}
Those calls to collidesWith are all returning lists now. treat them accordingly.
assertThat(ship.collidesWith(asteroid).size).describedAs("on top").isEqualTo(2)
And so on. We are green. Commit: Changed collidesWith logic to return colliding objects, not just true/false with accumulation later.
Let’s turn on the parts of the ShipMonitor test that I turned off, and se if they work, and if not, make them work. Then we have some renaming to do.
This test runs:
@Test
fun `ship monitor correctly adds a new ship`() {
val sixtieth = 1.0/60.0
val ship = Flyer.ship(Vector2(1000.0, 1000.0))
val asteroid = Flyer.asteroid(Vector2.ZERO, Vector2.ZERO)
val monitor = ShipMonitor(ship)
val game = Game()
game.add(ship)
game.add(asteroid)
game.add(monitor)
assertThat(game.flyers.size).isEqualTo(3)
assertThat(game.flyers.flyers).contains(ship)
assertThat(monitor.state).isEqualTo(ShipMonitorState.HaveSeenShip)
// nothing colliding
game.update(sixtieth)
game.processCollisions()
assertThat(game.flyers.size).isEqualTo(3)
assertThat(game.flyers.flyers).contains(ship)
assertThat(monitor.state).isEqualTo(ShipMonitorState.HaveSeenShip)
// ship colliding, make two asteroids and lose ship
ship.position = Vector2.ZERO
game.update(sixtieth)
game.processCollisions()
assertThat(game.flyers.size).isEqualTo(3)
assertThat(game.flyers.flyers).doesNotContain(ship)
assertThat(monitor.state).isEqualTo(ShipMonitorState.HaveSeenShip)
// now we discover the missing ship
game.update(sixtieth)
game.processCollisions()
assertThat(game.flyers.size).isEqualTo(3)
assertThat(game.flyers.flyers).doesNotContain(ship)
assertThat(monitor.state).isEqualTo(ShipMonitorState.LookingForShip)
// now we go active, return monitor damaged, get split,
// thus adding in ship and monitor.
game.update(sixtieth)
assertThat(monitor.state).describedAs("just switched").isEqualTo(ShipMonitorState.Active)
game.processCollisions()
assertThat(game.flyers.flyers).contains(ship)
assertThat(game.flyers.flyers).contains(monitor)
assertThat(monitor.state).isEqualTo(ShipMonitorState.HaveSeenShip)
}
Long but shows the whole life cycle. I also update the visible version of the game:
fun createContents() {
val ship = newShip()
add(ship)
add(ShipMonitor(ship))
for (i in 0..7) {
val pos = Vector2(random(0.0, 10000.0), random(0.0,10000.0))
val vel = Vector2(1000.0, 0.0).rotate(random(0.0,360.0))
val asteroid = Flyer.asteroid(pos,vel )
add(asteroid)
}
}
And the game runs on screen, instantly recreating the ship after it is destroyed.
Commit: ShipMonitor recreates ship instantly if it is destroyed.
Now we have some naming issues to deal with, around the changes to the collision logic:
fun colliders() = flyers.pairsSatisfying { f1, f2 -> f1.collidesWith(f2) }
The method pairsSatisfying isn’t returning pairs (and never really was), and collidesWith connotes boolean and now really returns … what … the objects destroyed in a collision between a and b?
Let’s rename pairsSatisfying to collectFromPairs. Easily done. Thanks, IDEA. Now what about that other name, and the ones implied by it?
How about a.collisionDamageWith(b)? Let’s try that and follow it through.
The rename changed the interface and the two classes implementing it. Impressive. We’re left with this in the interface:
interface IFlyer {
abstract val killRadius: Double
abstract val position: Vector2
abstract val ignoreCollisions: Boolean
fun collisionDamageWith(other: IFlyer): List<IFlyer>
fun collidesWithOther(other: IFlyer): List<IFlyer>
fun draw(drawer: Drawer)
fun move(deltaTime: Double)
fun split(): List<IFlyer>
fun update(deltaTime: Double): List<IFlyer>
}
The corresponding name, I guess, is collisionDamageWithOther. Still green. Commit: renaming collision and pairs methods.
I’d like to get rid of those val entries in the interface, but I don’t see how to do it. This method refers to them:
override fun collisionDamageWithOther(other: IFlyer): List<IFlyer> {
if ( this === other) return emptyList()
if ( this.ignoreCollisions && other.ignoreCollisions) return emptyList()
val dist = position.distanceTo(other.position)
val allowed = killRadius + other.killRadius
return if (dist < allowed) listOf(this,other) else emptyList()
}
Since that method accepts IFlyer, all IFlyers have to have those vals. Since the call to this method may be received in Flyer or in ShipMonitor, it has to have an IFlyer parameter.
I see no way out, other than to check type somewhere and thus convince Kotlin that they’re only accessed in Flyer. Frankly, I don’t think it’s worth it, since it’s working just fine.
Now there’s one more thing I wanted to look at, here in ShipMonitor:
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
if (state == ShipMonitorState.Active) return listOf(this)
if (other === ship) state = ShipMonitorState.HaveSeenShip
return emptyList()
}
After the monitor goes active, every collision comparison will return listOf(this). Since the results are accumulated in a Set, this is harmless: we’ll just turn up once. But we will in fact go through that logic for every object in the universe, which means we’ll exercise the Set logic umpty2 times. We could make that happen just once with an additional state. Call it, oh, DoneReporting.
enum class ShipMonitorState {
HaveSeenShip, LookingForShip, Active, DoneReporting
}
Now, when we trigger our report when Active, set the state to DoneReporting. This:
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
if (state == ShipMonitorState.Active) return listOf(this)
if (other === ship) state = ShipMonitorState.HaveSeenShip
return emptyList()
}
Becomes this:
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
if (state == ShipMonitorState.Active) {
state = ShipMonitorState.DoneReporting
return listOf(this)
}
if (other === ship) state = ShipMonitorState.HaveSeenShip
return emptyList()
}
This will work as it stands, think. Green. Yes. Can we make it more clear with an explicit reference to that state?
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
if (state == ShipMonitorState.Active) {
state = ShipMonitorState.DoneReporting
return listOf(this)
}
if (state == ShipMonitorState.DoneReporting) return emptyList()
if (other === ship) state = ShipMonitorState.HaveSeenShip
return emptyList()
}
I think that’s better, but why not do the full state check here?
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
var result:List<IFlyer> = emptyList()
state = when (state) {
ShipMonitorState.Active -> {
result = listOf(this)
ShipMonitorState.DoneReporting
}
ShipMonitorState.LookingForShip -> {
when {
other === ship -> ShipMonitorState.HaveSeenShip
else -> ShipMonitorState.LookingForShip
}
}
ShipMonitorState.HaveSeenShip -> ShipMonitorState.HaveSeenShip
ShipMonitorState.DoneReporting -> ShipMonitorState.DoneReporting
}
return result
}
That’s rather longer and not notably clearer. We could do this:
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
var result:List<IFlyer> = emptyList()
state = when (state) {
ShipMonitorState.Active -> {
result = listOf(this)
ShipMonitorState.DoneReporting
}
ShipMonitorState.LookingForShip -> {
when {
other === ship -> ShipMonitorState.HaveSeenShip
else -> ShipMonitorState.LookingForShip
}
}
else -> state
}
return result
}
Shorter, no less clear. I think all the indentation is nasty though. Let’s try this:
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
var result:List<IFlyer> = emptyList()
state = if (state == ShipMonitorState.Active) {
result = listOf(this)
ShipMonitorState.DoneReporting
}
else if (state == ShipMonitorState.LookingForShip && other === ship) {
ShipMonitorState.HaveSeenShip
}
else state
return result
}
That’s not bad. I’ll try again.
override fun collisionDamageWith(other: IFlyer): List<IFlyer> {
var result:List<IFlyer> = emptyList()
state = when {
state == ShipMonitorState.Active -> {
result = listOf(this)
ShipMonitorState.DoneReporting
}
state == ShipMonitorState.LookingForShip && other === ship -> {
ShipMonitorState.HaveSeenShip
}
else -> state
}
return result
}
I think we’ll go with that one … I like the shape a bit better. YMMV.
Let’s commit: Refactored ShipMonitor collisionDamageWith.
Let’s sum up: I think we’re done here.
Summary
Yesterday I got almost to the end of this and then discovered that the collision logic needed to work a little differently, so that a collision need not destroy two objects. I was a few hours in and just didn’t have it in me to work through that issue.
As we’ve seen today, the changes were in fact pretty simple: change the collidesWith to return whatever was to be destroyed, instead of saying yes or no. Modify the callers to expect that result. There was just one place in the production code, and three tests.
I modified the scenario test a bit: I had thought it would take another pass through update etc to trigger the new ship, but I was mistaken. It then ran, as does the “game” itself.
We’re left with the ShipMonitor creating a new ship immediately upon discovering that it’s gone. (Actually it just uses the same ship. No reason not to that I can see.)
With things working correctly, we cleaned up some names and made the state transitions more explicit in the ShipMonitor. I could imagine going further into a state machine design, but that seems to me to be more than we really need.
I conclude that my decision to go forward rather than roll back was probably righteous: we didn’t have to change much, if any, of what had gone before, only to modify the collision detection the same as we’d have had to do had we rolled back.
Maybe I’m just post-justifying what I did, and certainly had we rolled back it would also have gone well, but there would have been a lot of repetition in the work, doing what was done yesterday. Anyway, we’re here now, and here is a good place.
What we do need is for there to be a delay between the ship getting destroyed and it coming back. And, if I recall the way the game is supposed to work, The ship is supposed to return in a relatively safe place, not too near to any asteroids.
My idea for handling the timing would be to avoid the transition to active until the ship has been missing for some amount of time. WWe could record the time when the ship was last seen or something. Since we get deltaTime on update, that should be easy enough.
All for another day. For how, I want to assess how I feel about this change.
What I like, what I really really like, is that there’s just a little object in space that notices the absence of the ship and does something about it. I think hat same object can probably deal with how many ships you get, and with displaying GAME OVER and INSERT 25 CENTS TO PLAY. We’ll see.
The alternative design, I think, would have been to have logic in the Game so that after each cycle, it would look through the objects for a ship (how would it know?), and if it didn’t find it, go into one of a number of different game states. It’s hard to know without doing it, but I think that would have been at least this complex,, and would seem “larger”, a bigger deal than this.
We just have a little swimmer in space who checks things out and fixes things up, pretty automatically.
I think I am well pleased with how this turned out, and while it took longer than I might have hoped, it really went in pretty smoothly.
Coming up, we need missiles, which should be straightforward. We need the ShipMonitor to handle more state changes, including GAME OVER and such. We need the Saucer and its special missiles.
Oh, and it would be nice if we could control the game from the keyboard or something.
Follow along: we’ll get there. See you next time!