Kotlin 251 - Some cleaning?
GitHub Decentralized Repo
GitHub Centralized Repo
GitHub Flat Repo
And it came to pass on the morning of the third day that the cat did again inspect the offering, and did see that it was good, and did eat thereof. Secure in this knowledge, the programmer did begin his work.
I think I’ll do a bit of cleaning this morning, then maybe other things. I’ve been given an idea.
Yesterday, Dirk Groot suggested an idea that I had been toying with, namely building one or more data classes / structs to contain things like the game configuration or constants. Dirk went on to suggest that a Universe struct might contain asteroids and a ship, that the ship might contain missiles, and that perhaps firing a missile removed it from the ship and added it to the universe.
I am inclined toward the basic idea, and not toward the second part. Not that it isn’t a good idea: it may well be. My reason is that I am choosing to hew more closely to the original Asteroids design, which just had a big flat structure laid out for the objects, more or less one after the other. That said, there sure is a lot of appeal to the notion of the ship firing missiles until it runs out and missiles coming back to the ship’s supply after they time out or hit something.
It’s a question of what I’m trying to do with this design. Am I doing the best I can do with 21st century Kotlin, except without objects, or am I following more of a 1979 style of programming, except not in 6502 assembler? I’m already bumping up against the issue of modern vs old school:
For example, I have an overloaded function that allows me to define things using integers instead of the required doubles:
fun move(spaceObject: SpaceObject, width: Int, height: Int, deltaTime: Double) {
move(spaceObject, width.toDouble(), height.toDouble(), deltaTime)
}
fun move(spaceObject: SpaceObject, width: Double, height: Double, deltaTime: Double) {
with (spaceObject) {
x = limit(x+dx*deltaTime, width)
y = limit(y+dy*deltaTime, height)
}
}
That’s just used for convenience in testing but it’s pretty modern. Of course, if we did need a similar thing in assembler it might look like this:
moveInt jsr toDouble
move sta tempWidth
lda dx
mul deltaTime
...
We’d just put the overloaded version right above the real one, do the conversion, and drop in. Those were the days.
Further, as modern ideas go, I’ve been thinking about accepting Vector2 as a native data type and using it instead of all the separate x and y code that I have now.
In the Zoom call last night, someone mentioned that it looked like the procedural code could be refactored to use objects pretty readily, and I agree with that. As Dirk Groot pointed out:
I think that much of the good practices of OO design apply to non-OO design as well. It’s just that with OO you have some more tools available (inheritance and dynamic polymorphism), which non-OO languages don’t have.
If we were to go with Dirk’s idea, we might make something like this:
private fun newMissile(): SpaceObject
= SpaceObject(SpaceObjectType.MISSILE, 0.0, 0.0, 0.0, 0.0, 0.0, false)
private fun newShip(): SpaceObject
= SpaceObject(SpaceObjectType.SHIP, 0.0, 0.0, 0.0, 0.0, 0.0, false)
data class AsteroidsGame(
val shipMissiles: Array<SpaceObject> = arrayOf(newMissile(), newMissile(), newMissile(), newMissile())
val saucerMissiles: Array<SpaceObject> = arrayOf(newMissile(), newMissile())
val ship: SpaceObject = newShip()
)
Presumably we’d add in a Saucer and some asteroids as we extend the game. Dirk’s idea, it seems to me, leads a bit further, toward having a collection “Universe” that includes the things that move, with missiles sometimes being in the Universe and sometimes being in the ship (or saucer), ready to be fired.
Darn1. The more I think about the idea, the more I want to try it. But it’s definitely a big deviation from what I set out to do. Please tell me that I don’t have to do yet another version of Asteroids!
OK, a plan: we’ll stick with the current path for a while, sticking with as little data structure as I can tolerate, to see how nasty it gets. When it finally gets intolerable, then we’ll see about adding structure along the lines of what Dirk suggests.
I don’t promise to stick to this plan even for an hour. As we go, we’ll stay alert to changes that will make our job easier, and we’ll have the basic rule be that we can do anything Kotlin can do other than add methods to classes.
And, we might even break that one, but I don’t expect to.
Let’s do some work. I think I’ll move all the globals to the Game file, just so I’ll know where they all are.
// Globals
var controls_left: Boolean = false
var controls_right: Boolean = false
var controls_accelerate: Boolean = false
var controls_fire: Boolean = false
var controls_hyperspace: Boolean = false
const val Width = 1024
const val Height = 1024
lateinit var spaceObjects: Array<SpaceObject>
lateinit var Ship: SpaceObject
That wasn’t as bad as I thought, there are just those few, unless I missed some. If I did, they’ll turn up as I edit.
I think we should look at the setup and have it actually set the globals directly. That’s not good design, though. As Dirk pointed out, it would be “better” to pass a struct that contains everything. I think we’ll let it stand, because this is Asteroids in somewhat the old style. I think I’ll recast the parameters to the setup and rename it as well.
fun createInitialObjects(missileCount: Int, asteroidCount: Int): Array<SpaceObject> {
val objects = mutableListOf<SpaceObject>()
for ( i in 1..missileCount) {
objects.add(newMissile())
}
Ship = newShip()
objects.add(Ship)
return objects.toTypedArray()
}
We’ll rename this to createGame
and remove the return.
fun createGame(missileCount: Int, asteroidCount: Int) {
val objects = mutableListOf<SpaceObject>()
for ( i in 1..missileCount) {
objects.add(newMissile())
}
Ship = newShip()
objects.add(Ship)
spaceObjects = objects.toTypedArray()
}
It’s a matter of moments to align the calls with the new signature, and we are green. Commit: createGame creates directly into the game’s globals.
Let’s do something about missile timing. I think that our design style requires all the space objects to have all the same fields, so if we add a timing field to any of them, all of them will have it, even if it is to be ignored.
You’d imagine that we’d set the timer to some positive value, tick it down, and when it hits zero, set the object inactive. But only the missiles should be ticked. Or maybe later, the missiles and the saucer. Let’s assume that we’ll just tick the ones that need ticking. So we’ll add a field.
data class SpaceObject(
val type: SpaceObjectType,
var x: Double,
var y: Double,
var dx: Double,
var dy: Double,
var angle: Double = 0.0,
var active: Boolean = true,
)
I don’t think we should have all those vars in the constructor. Some of them make more sense as associated fields not required to construct. We’ll work toward that, but for now, let’s just add the timer:
data class SpaceObject(
val type: SpaceObjectType,
var x: Double,
var y: Double,
var dx: Double,
var dy: Double,
var angle: Double = 0.0,
var active: Boolean = true,
) {
var timer: Int = 0
}
We’ll set up the missiles with a 3 second timer, because that’s what we use in the other versions. And for convenience, let’s make the timer a Double.
fun fireMissile() {
controls_fire = false
val missile: SpaceObject = availableShipMissile() ?: return
val offset = Vector2(50.0, 0.0).rotate(Ship.angle)
missile.x = offset.x + Ship.x
missile.y = offset.y + Ship.y
val velocity = Vector2(166.6, 0.0).rotate(Ship.angle)
missile.dx = velocity.x + Ship.dx
missile.dy = velocity.y + Ship.dy
missile.timer = 3.0
missile.active = true
}
And let’s tick those timers:
fun gameCycle(
spaceObjects: Array<SpaceObject>,
width: Int,
height: Int,
drawer: Drawer,
deltaTime: Double
) {
for (spaceObject in spaceObjects) {
if (spaceObject.type == SpaceObjectType.SHIP) {
applyControls(spaceObject, deltaTime)
}
if (spaceObject.type == SpaceObjectType.MISSILE){
tickTimer(spaceObject, deltaTime)
}
move(spaceObject, width, height, deltaTime)
}
for (spaceObject in spaceObjects) {
if (spaceObject.active) draw(spaceObject, drawer)
}
}
private fun tickTimer(spaceObject: SpaceObject, deltaTime: Double) {
if (spaceObject.timer > 0) {
spaceObject.timer -= deltaTime
if (spaceObject.timer <= 0.0) spaceObject.active = false
}
}
I rather expect my missiles to time out and to be able to fire again. That is exactly what happens. Commit: missiles time out after three seconds.
We should have a test for that, however. Let’s add one. We have this test:
@Test
fun `can fire four missiles`() {
createGame(6, 26)
assertThat(activeMissiles()).isEqualTo(0)
fireMissile()
assertThat(activeMissiles()).isEqualTo(1)
fireMissile()
assertThat(activeMissiles()).isEqualTo(2)
fireMissile()
assertThat(activeMissiles()).isEqualTo(3)
fireMissile()
assertThat(activeMissiles()).isEqualTo(4)
fireMissile()
assertThat(activeMissiles()).isEqualTo(4)
}
Let’s just add a phase to that.
@Test
fun `can fire four missiles`() {
createGame(6, 26)
assertThat(activeMissiles()).isEqualTo(0)
fireMissile()
assertThat(activeMissiles()).isEqualTo(1)
fireMissile()
assertThat(activeMissiles()).isEqualTo(2)
fireMissile()
assertThat(activeMissiles()).isEqualTo(3)
fireMissile()
assertThat(activeMissiles()).isEqualTo(4)
fireMissile()
assertThat(activeMissiles()).isEqualTo(4)
val missile = spaceObjects.find { it.type == SpaceObjectType.MISSILE }
tickTimer(missile!!, 3.1)
assertThat(activeMissiles()).isEqualTo(3)
}
I expect that to run green. It does not. Why? Because I have picked an inactive one to decrement. Change the find.
val missile = spaceObjects.find {
it.type == SpaceObjectType.MISSILE && it.active == true}
Green. Commit: added test for missile timer.
Let’s rename that activeMissiles
function:
fun activeMissileCount(): Int {
return spaceObjects.count { it.type == SpaceObjectType.MISSILE && it.active == true}
}
Slightly better, since that’s what it is. Commit: tidying rename.
Make it private. Commit: tidying private.
Shall we sum up? Let’s.
Summary
We’re thinking about adding more structure to our data and producing fewer globals in the process. Now, the fact is, now that I think of it, the spaceObjects and ship almost don’t need to be global at all, do they? The tests want to look at them, but no other functions should need them. Maybe we’ll see about hiding them and then providing some test functions to get at them.
Dirk’s idea about a struct to hold the stuff is a good one, and the idea about moving things from inside the ship to outside and back is awfully enticing, if only to try it and see how it feels. But I’m kind of locked in on having a fixed structure like that of the original game. That may well be a mistake.
Overall, though, it seems to me to be going well. It’s a bit weird having a timer in all the space objects, but we kind of bought that when we committed to a fixed structure. We’re trying not to have spacial data types, just one struct kind of thing that represents any object in the game.
I freely grant that that is probably a bad constraint to have accepted. It doesn’t help us, and probably hurts us. On the other hand, sometimes you play left-handed just to get good at playing left-handed.
But the game cycle is really quite simple:
fun gameCycle(
spaceObjects: Array<SpaceObject>,
width: Int,
height: Int,
drawer: Drawer,
deltaTime: Double
) {
for (spaceObject in spaceObjects) {
if (spaceObject.type == SpaceObjectType.SHIP) {
applyControls(spaceObject, deltaTime)
}
if (spaceObject.type == SpaceObjectType.MISSILE){
tickTimer(spaceObject, deltaTime)
}
move(spaceObject, width, height, deltaTime)
}
for (spaceObject in spaceObjects) {
if (spaceObject.active) draw(spaceObject, drawer)
}
}
We can refactor that a bit. Extract two methods:
fun gameCycle(
spaceObjects: Array<SpaceObject>,
width: Int,
height: Int,
drawer: Drawer,
deltaTime: Double
) {
update(spaceObjects, deltaTime, width, height)
draw(spaceObjects, drawer)
}
private fun update(
spaceObjects: Array<SpaceObject>,
deltaTime: Double,
width: Int,
height: Int
) {
for (spaceObject in spaceObjects) {
if (spaceObject.type == SpaceObjectType.SHIP) {
applyControls(spaceObject, deltaTime)
}
if (spaceObject.type == SpaceObjectType.MISSILE) {
tickTimer(spaceObject, deltaTime)
}
move(spaceObject, width, height, deltaTime)
}
}
private fun draw(spaceObjects: Array<SpaceObject>, drawer: Drawer) {
for (spaceObject in spaceObjects) {
if (spaceObject.active) draw(spaceObject, drawer)
}
}
We can refactor further:
private fun update(
spaceObjects: Array<SpaceObject>,
deltaTime: Double,
width: Int,
height: Int
) {
for (spaceObject in spaceObjects) {
controlTheShip(spaceObject, deltaTime)
updateTimers(spaceObject, deltaTime)
move(spaceObject, width, height, deltaTime)
}
}
private fun controlTheShip(spaceObject: SpaceObject, deltaTime: Double) {
if (spaceObject.type == SpaceObjectType.SHIP) {
applyControls(spaceObject, deltaTime)
}
}
etc.
I don’t think we’ll do that just now, so I’ll roll it back. I think we’d like to get rid of most of the parameters first, and we’ll save that for another article. The point, though, is that the code is actually nicely modular and really pretty tight.
I think this version will be clear, and smaller than the OO version. That shouldn’t surprise us too much: Asteroids is really a pretty simple problem, so that if we lose some organization from not using methods, we don’t lose much. In a larger program, working this way would probably get really sticky. I’m not going to try that.
See you next time!
-
Not what I really said. ↩