GitHub Repo

Hill tweeted something. Perhaps it had been mentioned at the Zoom Ensemble but if it was I missed it. It seems like a good idea. Spoiler: It was!

Hill tweeted:

Haven’t read 127 yet, so don’t know what Ron’ll do next. I notice that those variables in Flyer that are only used by the AsteroidFinalizer could be moved to the AsteroidFinalizer and removed from Flyer. After that, we’ll feel better about using proper Flyer constructor.

I hadn’t noticed that yet. I suppose at some point I might have. It’s certainly a decent observation: Here’s our AsteroidFinalizer now:

class AsteroidFinalizer: IFinalizer {
    private fun Flyer.asSplit(): Flyer {
        splitCount -= 1
        killRadius /= 2.0
        velocity = velocity.rotate(Math.random() * 360.0)
        return this
    }

    private fun Flyer.asTwin() = Flyer.asteroid(
        pos = position,
        vel = velocity.rotate(Math.random() * 360.0),
        killRad = killRadius,
        splitCt = splitCount
    )

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = asteroid.getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (asteroid.splitCount >= 1) { // type check by any other name
            val meSplit = asteroid.asSplit()
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun Flyer.getScore(): Score {
        val score = when (killRadius) {
            500.0 -> 20
            250.0 -> 50
            125.0 -> 100
            else -> 0
        }
        return Score(score)
    }

Looks to me as if the splitCount could be moved into the finalizer and an adjusted finalizer passed to the children. The current one seems to be stateless but that’s OK. We could keep it immutable if we wished, but I don’t see a big advantage to that so won’t worry about it much.

The killRadius needs to be in the asteroid, at least as things stand now. The score needs killRadius to work, but could readily be changed to use splitCount, since we are aware that we are dealing with an asteroid in its finalizer.

Let’s move in the direction Hill suggests. We’ll start by putting the splitCount into the finalizer. We can default it but should make it a construction parameter:

class AsteroidFinalizer(val splitCount:Int = 2): IFinalizer {

And we can use it … hmm. I don’t see a tiny step at a time way to go about this. Let’s see. How about this:

    private fun Flyer.asSplit(): Flyer {
        splitCount -= 1
        killRadius /= 2.0
        velocity = velocity.rotate(Math.random() * 360.0)
        return this
    }

We extended Flyer for these utilities. Let’s instead write one here. And, while we’re at it, why not create two new asteroids, letting this one die an honorable death?

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = asteroid.getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (splitCount >= 1) {
            val meSplit = asSplit(asteroid)
            objectsToAdd.add(meSplit.asTwin())
            objectsToAdd.add(meSplit)
        }
        return objectsToAdd
    }

    private fun asSplit(asteroid: Flyer): Flyer {
        val newKr = asteroid.killRadius / 2.0
        val newVel = asteroid.velocity.rotate(Math.random() * 360.0)
        val flyer =  Flyer.asteroid(asteroid.position, newVel, newKr)
        flyer.finalizer = AsteroidFinalizer(splitCount - 1)
        return flyer
    }

I think this should work. Let’s test and see. No, not quite because our asTwin is wrong. Let’s just go the whole way. Oh. Now that asSplit makes a new asteroid, we can just use it twice:

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = asteroid.getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (splitCount >= 1) {
            objectsToAdd.add(asSplit(asteroid))
            objectsToAdd.add(asSplit(asteroid))
        }
        return objectsToAdd
    }

Let’s see what happens now. Hmn.

expected: 125.0
 but was: 250.0

Best check that test:

    @Test
    fun `asteroid splits on finalize`() {
        val full = Flyer.asteroid(
            pos = Point.ZERO,
            vel = Velocity.ZERO
        )
        val radius = full.killRadius
        val halfSize= full.finalize()
        assertThat(halfSize.size).isEqualTo(3) // two asteroids and a score
        val half = halfSize.last()
        assertThat(half.killRadius).isEqualTo(radius/2.0)
        val quarterSize = half.finalize()
        assertThat(quarterSize.size).isEqualTo(3)
        val quarter = quarterSize.last()
        assertThat(half.killRadius).isEqualTo(radius/4.0)
        val eighthSize = quarter.finalize()
        assertThat(eighthSize.size).isEqualTo(1)
    }

I add some descriptions:

    @Test
    fun `asteroid splits on finalize`() {
        val full = Flyer.asteroid(
            pos = Point.ZERO,
            vel = Velocity.ZERO
        )
        val radius = full.killRadius
        val halfSize= full.finalize()
        assertThat(halfSize.size).isEqualTo(3) // two asteroids and a score
        val half = halfSize.last()
        assertThat(half.killRadius).describedAs("half").isEqualTo(radius/2.0)
        val quarterSize = half.finalize()
        assertThat(quarterSize.size).isEqualTo(3)
        val quarter = quarterSize.last()
        assertThat(half.killRadius).describedAs("quarter").isEqualTo(radius/4.0)
        val eighthSize = quarter.finalize()
        assertThat(eighthSize.size).isEqualTo(1)
    }

Naturally, the issue is quarter. So what has happened? We got the half size ones OK. Why didn’t they get cut in half again?

The test is wrong. It’s checking half to see what its size is. In the old version, we reduced the radius on an existing asteroid. Now we create two new ones. Fix the test. Copy-paste bug in the test.

Test is green. I happen to run the game. The new asteroids have smaller kill radii, but are drawn same size as before. I bet we’re scaling based on splitCount. Yep:

    override fun draw(asteroid: Flyer, drawer: Drawer) {
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 16.0
        drawer.fill = null
//        drawer.circle(Point.ZERO, asteroid.killRadius)
        val sizer = 30.0
        drawer.scale(sizer, sizer)
        val sc = 2.0.pow(asteroid.splitCount)
        drawer.scale(sc,sc)
        drawer.rotate(asteroid.heading)
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 8.0/30.0/sc
        drawer.scale(1.0, -1.0)
        drawer.lineStrip(rock)
    }

That would have turned up in the compiler as soon as we removed splitCount from Flyer. I go ahead to do that now. It’ll cascade through some code but should be easy to fix up.

So I remove the splitCount parameter from the Flyer constructor, add a splitCount function to Flyer to be used in AsteroidView:

        val sc = 2.0.pow(asteroid.splitCount())

    fun splitCount() = finalizer.splitCount

interface IFinalizer {
    fun finalize(flyer: Flyer): List<IFlyer>
    val splitCount:Int
        get() = 0
}

class AsteroidFinalizer(override val splitCount:Int = 2): IFinalizer {

That makes everything work as intended. It seems a little convoluted to me but makes sense if you trace through it. That’ll be the cost of deferring a number like that over to the finalizer: if anyone needs it, you have to provide for it in the interface and the particular finalizer that wants it.

Commit: AsteroidFinalizer creates the splits and maintains the splitCount internally. Moderately extensive change, all simple.

What else might we move? Oh, let’s work on Score.

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (splitCount >= 1) {
            objectsToAdd.add(asSplit(asteroid))
            objectsToAdd.add(asSplit(asteroid))
        }
        return objectsToAdd
    }

    private fun getScore(): Score {
        val score = when (splitCount) {
            2 -> 20
            1 -> 50
            0 -> 100
            else -> 0
        }
        return Score(score)
    }

Works a treat. Commit: AsteroidFinalizer computes score based on its own splitCount.

Let’s take another look:

class AsteroidFinalizer(override val splitCount:Int = 2): IFinalizer {
    private fun asSplit(asteroid: Flyer): Flyer {
        val newKr = asteroid.killRadius / 2.0
        val newVel = asteroid.velocity.rotate(Math.random() * 360.0)
        val flyer =  Flyer.asteroid(asteroid.position, newVel, newKr)
        flyer.finalizer = AsteroidFinalizer(splitCount - 1)
        return flyer
    }

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = getScore()
        if (score.score > 0 ) objectsToAdd.add(score)
        if (splitCount >= 1) {
            objectsToAdd.add(asSplit(asteroid))
            objectsToAdd.add(asSplit(asteroid))
        }
        return objectsToAdd
    }

    private fun getScore(): Score {
        val score = when (splitCount) {
            2 -> 20
            1 -> 50
            0 -> 100
            else -> 0
        }
        return Score(score)
    }
}

Well, we are an Asteroid finalizer, so score cannot be zero. We can remove that check.

    override fun finalize(asteroid: Flyer): List<IFlyer> {
        val objectsToAdd: MutableList<IFlyer> = mutableListOf()
        val score = getScore()
        objectsToAdd.add(score)
        if (splitCount >= 1) {
            objectsToAdd.add(asSplit(asteroid))
            objectsToAdd.add(asSplit(asteroid))
        }
        return objectsToAdd
    }

I don’t love the form where you build up a list in bits like this, but it’ll do until a better pattern comes to mind.

We need the else in getScore because Kotlin will demand it. It never happens, however. That’s odd but I don’t see how to avoid it.

Summary … kind of …

So we have moved the scoring of asteroids and their splitting to their finalizer. That seems to me to make perfect sense. The expense has been that we refer to the finalizer in the draw method, to get the scale, which is a bit odd. What if we were at least to ask the finalizer for the scale instead of the weird number?

We have this:

    override fun draw(asteroid: Flyer, drawer: Drawer) {
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 16.0
        drawer.fill = null
//        drawer.circle(Point.ZERO, asteroid.killRadius)
        val sizer = 30.0
        drawer.scale(sizer, sizer)
        val sc = 2.0.pow(asteroid.splitCount())
        drawer.scale(sc,sc)
        drawer.rotate(asteroid.heading)
        drawer.stroke = ColorRGBa.WHITE
        drawer.strokeWeight = 8.0/30.0/sc
        drawer.scale(1.0, -1.0)
        drawer.lineStrip(rock)
    }

Let’s just ask the asteroid for its scale, not the splitCount, and have it ask the finalizer.

class Flyer ...
    fun scale() = finalizer.scale()

interface IFinalizer {
    fun finalize(flyer: Flyer): List<IFlyer>
    fun scale(): Double = 1.0
}

class AsteroidFinalizer ...
    override fun scale(): Double {
        return 2.0.pow(splitCount)
    }

Works as advertised. Commit: AsteroidFinalizer now provides asteroid.scale() to draw.

Summary … some more …

So Hill’s idea was a decent one. We’ve narrowed the constructor of the Flyer by one member, splitCount, that was usually unused. We have moved that idea over to the AsteroidFinalizer, with the cost (at present) that all the finalizers need to be able to return scale, because AsteroidView needs it. If we did enough type-fiddling, we might be able to downcast somewhere and avoid that issue but for now everyone else just returns 1.0 in response to scale() and they won’t be asked for it anyway.

I’ll ask my betters for a better way to deal with that. Perhaps they’ll know.

Overall, good idea and a simplification to the Flyer object with very little increase in complication elsewhere. In fact, since the getScore and asSplit methods were moved inside the finalizer, and the asTwin removed entirely, the entire system complexity has reduced a bit. We have removed all the Flyer extension methods that we formerly needed.

Nice work, Hill!