Python 018 - Is This All Crap?
Today’s story is to make the missiles damage asteroids. But I have a larger question. P.S. I am the Cat …
The quiet and brilliant Bill Wake read some of these articles and commented that he thought it a bit odd for the main loop to be grabbing something from the objects and calling it. He was referring to this code:
if ship.active: ship.mover.move(dt)
for asteroid in asteroids:
asteroid.mover.move(dt)
for missile in missiles.copy():
missile.update(missiles, dt)
for missile in missiles:
missile.mover.move(dt)
Our convention at the moment is that all the space objects have a mover instance member, and that you ask the mover to move the object. Now, I think that this makes a bit more sense if you think of the mover as a delegate and you imagine that the space objects all implement the Mover interface, which in Kotlin might look like this:
interface Mover { val mover: ObjectMover }
With that declaration, we all know that its implementors can provide a mover and we know how to call it, because it, too, has known behavior. In Python we don’t need to declare such things, but we can still do them.
I don’t know whether Bill would feel better in the presence of an interface like that. Either way the question makes me examine my motivations, and examine what I’ve done here so far.
All the objects currently use the same kind of mover, an instance of Mover. That could change: asteroids just drift. Missiles just drift, but they time out. The ship just drifts, but it can accelerate (and rotate). The saucer just drifts,, but occasionally makes an abrupt change of direction.
So in principle, each object could have, and perhaps should have, a separate kind of mover. But we have no need for that now, and we may never have the need for a separate object: if each object actually winds up with its own type of mover … maybe the mover should just be implemented as methods on the individual object, or as a helper object owned by the individual object.
Now that we’ve had this little chat, I think that what we have here is not crap, but it is a matter of premature design. I developed approximately three lines of duplicate code, thought of inheritance as a way to get rid of it, then chatted about why we shouldn’t do that, and so I went with a delegation / component pattern instead.
I think the right thing to do was probably to wait until the game takes on more shape, and then decide.
Remove Mover
So therefore, we’ll begin the day by getting rid of the Mover class.
Here’s Mover:
class Mover:
def __init__(self, position, velocity):
self.position = position.copy()
self.velocity = velocity.copy()
def accelerate_by(self, accel):
self.velocity = self.velocity + accel
def move(self, deltaTime):
position = self.position + self.velocity * deltaTime
position.x = position.x % u.SCREEN_SIZE
position.y = position.y % u.SCREEN_SIZE
self.position = position
The accelerateBy method is just there for the ship’s purpose. No one else wants it.
We can get rid of this object by moving the position and velocity members, and the move method, to each of our space object classes, and then either causing them to answer self to mover, or by changing the main loop not to defer to mover. Let’s see how easy we can make this change. Maybe PyCharm has some help for us. I know it has a Copy capability. Let’s see what it does. No, it’s just for copying files and high-level things. We’ll do this the hard way, one at a time. Missile might be easiest.
Missile
class Missile:
def __init__(self, position, velocity):
self.radius = 2
self.mover = Mover(position, velocity)
self.time = 0
def draw(self, screen):
pygame.draw.circle(screen, "white", self.mover.position, 4)
def update(self, missiles, delta_time):
self.time += delta_time
if self.time > 3:
missiles.remove(self)
Right. Change it to return itself as mover, move the move method, move the lines from Mover’s init:
class Missile:
def __init__(self, position, velocity):
self.position = position.copy()
self.velocity = velocity.copy()
self.radius = 2
self.mover = self
self.time = 0
def move(self, deltaTime):
position = self.position + self.velocity * deltaTime
position.x = position.x % u.SCREEN_SIZE
position.y = position.y % u.SCREEN_SIZE
self.position = position
I had to import u to get the constants. All is well. Commit: Missile is its own mover.
OK, fine. Let’s do Asteroid.
Asteroid
This one is slightly more tricky, because asteroids compute their position and velocity, but it’s still trivial:
def __init__(self, size=2, position=None):
asteroid_radii = [16, 32, 64]
try:
self.radius = asteroid_radii[size]
except IndexError:
self.radius = asteroid_radii[2]
self.offset = Vector2(self.radius, self.radius)
self.position = position if position else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
self.velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self.mover = self
self.surface = SurfaceMaker.asteroid_surface(self.radius * 2)
I think we could make this more clear by moving things around a bit.
def __init__(self, size=2, position=None):
self.mover = self
self.position = position if position else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
self.velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
asteroid_radii = [16, 32, 64]
try:
self.radius = asteroid_radii[size]
except IndexError:
self.radius = asteroid_radii[2]
self.offset = Vector2(self.radius, self.radius)
self.surface = SurfaceMaker.asteroid_surface(self.radius * 2)
Good enough for now. We should probably break that method up a bit. But we’re good. Commit: Asteroid is its own mover.
Now the ship
Ship
I make the obvious changes to Ship and one test fails the auto-run. Let’s find out what it is.
I laugh: we spoke of this just above:
def power_on(self, dt):
self.accelerating = True
accel = dt * self.acceleration.rotate(-self.angle)
> self.mover.accelerate_by(accel)
E AttributeError: 'Ship' object has no attribute 'accelerate_by'
Right, move that over. The tests start running green, and the Ship flies. Commit: Ship is its own mover.
I think there may be no references to Mover, other than perhaps in a test. I remove one test and two imports. Now I can safe delete mover.
Now let’s look for “senders” of mover …
That was a bit tedious but came down to deleting .mover a lot and then some self.mover = self lines. Now there is no mover stuff anywhere. Commit: references to mover removed.
Let’s reflect.
Reflection
It’s striking to me today, how similar all programming is. Not just because I’ve been implementing Asteroids since October 2022, but because the issues that arise seem to me to be always quite similar. It’s generally easy, I think, to see how to do a specific thing, at least once you get skilled at slicing requirements down to small steps. Then it comes down to deciding where the responsibilities are, and soon you find that you don’t have them allocated quite as they should be, so you move them around.
All the issues seem to me to come up in this one little program.
Well, not quite all. If we were doing multi-threading we’d have some different issues. And there’s the hassle of building a GUI, we’ve avoided that. There’s reading input and parsing it. But so much of those things comes down to the same issues of where to put the responsibilities.
Or so it seems to me.
Coming down to Asteroids, we removed an object this morning, and in order to do it, we have duplicated the move method into each space object class, and we have moved the position and velocity members from the Mover back into the space object. Now I was born to reject duplication, so I’m a bit bothered by this duplication, but I’ve decided to sit with it for a while and see what comes up.
One thought has already come up, and it goes like this: Maybe there should not be separate classes for Asteroid, Ship, Missile, and Saucer.
We were doing that in our “flat” implementation in Kotlin. If I recall — it was literally days ago after all — we just had one kind of space object, and they had different components to provide certain kinds of behavior that differed between the “types”. And that’s certainly how the original assembly-language Asteroids worked. A simple type field in the table and lots of if statements.
And that’s kind of what one does in an Entity - Component - System design, objects are differentiated only by the various components they hold.
Putting the Mover in, a day or so ago, reminded me of ECS, and I think I mentioned it at the time. So we’ve just moved away from that, but we’re retaining our types as classes, at least for now.
I’m reminded of one of the things that GeePaw said during the Kotlin Asteroids exercises, something about “once you know the type of an object, never forget it”. Another darn thing to worry about.
These various design ideas all apply forces to the design. Knowing the types of things makes us want separate classes or at least a type field. Removing duplication drives us toward inheritance (ptui!?) or delegation. Delegation pushes us toward pointer-following and complexity.
In a program of any size at all, we can rarely if ever find a perfect balance. It seems that there’s always something not to like. And we can’t spend our lives pushing on push-rods and pulling on threads. We have to make the changes that they pay us for, like making the missiles actually shoot something.
For me, that’s the value of a little project series like I’ve been doing: it’s a safe playground where there’s no more pressure to build features than we want to feel. We can take a week or a month to try different design ideas.
We hope you are wise …
Of course, if you’re a wise person with a programming job, you might not choose to spend your evenings and weekends programming. You might have friends, hobbies, other valuable things to do. And that’s fine. I happen to spend a lot of time programming, but I don’t have a day job, so I still only spend a few hours a day at it, which leaves plenty of time for other more important life matters. Like naps. Or feeding the cat.
Anyway, this morning, I chose to remove a design decision that I deemed premature, based on a random comment from Bill. My code, my rules. Now I have a bit of duplication that bugs me a bit … but not much.
Missiles
Let’s see if we can get the missiles to break some rocks. But first, I need to make my morning iced chai latte. I got things in the wrong order this morning, because the kitchen was in use. I’ll be right back.
See? That didn’t take long.
Let’s look at how we’re handling collisions now.
def check_collisions():
if ship.active:
for asteroid in asteroids.copy():
ship.collideWithAsteroid(asteroid)
if not ship.active:
asteroids.remove(asteroid)
radius = asteroid.radius
size = [16, 32, 64].index(radius)
if size > 0:
a1 = Asteroid(size - 1, asteroid.position)
asteroids.append(a1)
a2 = Asteroid(size - 1, asteroid.position)
asteroids.append(a2)
ship.active = True
That could use a little refactoring. Let’s at least do this:
def check_collisions():
check_ship_vs_asteroid()
def check_ship_vs_asteroid():
if ship.active:
for asteroid in asteroids.copy():
ship.collideWithAsteroid(asteroid)
if not ship.active:
asteroids.remove(asteroid)
radius = asteroid.radius
size = [16, 32, 64].index(radius)
if size > 0:
a1 = Asteroid(size - 1, asteroid.position)
asteroids.append(a1)
a2 = Asteroid(size - 1, asteroid.position)
asteroids.append(a2)
ship.active = True
Now this invites us to write a method about missiles vs asteroids. Let’s put the asteroids in charge this time.
Along the way I notice that name collideWithAsteroid and change it to collide_with_asteroid per PEP 8.
I get this far:
def check_asteroids_vs_missiles():
for asteroid in asteroids.copy():
for missiles in missiles.copy():
asteroid.collide_with_missile(missile, missiles, asteroids)
def check_collisions():
check_ship_vs_asteroid()
check_asteroids_vs_missiles()
I figure I need to pass the collections over to the asteroid, so that it can do the necessary modifications. We’ll see how we feel about that in due time. I just copy over and modify some code, a clear sign that something needs improvement.
def collide_with_missile(self, missile, missiles, asteroids):
if self.withinRange(missile.position, missile.radius):
missiles.remover(missile)
asteroids.remove(self)
size = [16, 32, 64].index(self.radius)
if size > 0:
a1 = Asteroid(size - 1, self.position)
asteroids.append(a1)
a2 = Asteroid(size - 1, self.position)
asteroids.append(a2)
I do, however, think this should “just work”. Python does not agree. It thinks that I can’t copy missiles. I had a typo, missiles when I meant missile. I think PyCharm might own that one.
def check_asteroids_vs_missiles():
for asteroid in asteroids.copy():
for missile in missiles.copy():
asteroid.collide_with_missile(missile, missiles, asteroids)
Another typo, remover for remove. That one’s on me.
def collide_with_missile(self, missile, missiles, asteroids):
if self.withinRange(missile.position, missile.radius):
missiles.remove(missile)
asteroids.remove(self)
size = [16, 32, 64].index(self.radius)
if size > 0:
a1 = Asteroid(size - 1, self.position)
asteroids.append(a1)
a2 = Asteroid(size - 1, self.position)
asteroids.append(a2)
And we have a game:
Let’s sum up.
Summary
First and foremost, since 0800 this morning we have “totally redesigned motion” and implemented the ability for missile to shoot down asteroids. That’s about two hours, counting all this blathering about software. So a very good morning.
The design … well, not much worse, not much better. Simpler because we’ve gotten rid of Mover, but a bit more duplication through doing that.
We have some pretty massive duplication, all that asteroid splitting code, perhaps as many as eight lines duplicated. Plus the duplication of the four lines of move code, at least three times. Horrors!
And the asteroid collision code is rather deeply nested as well, three whole tab stops. Clear sign that we need some more methods or functions.
What I do like, I think, is the practice of passing the necessary collections to the collision code, so that it can remove and add objects as need be. We copy the collections before iterating them, making the updates safe. Is it costly to copy them? Maybe, but on this computer, who can tell? Not we humans, that’s for sure.
I think we need to settle on a sequence of operations, probably update, including motion, and then collisions. Right now that’s all in open code and it’s rather ragged.
With an update/move phase we can pass the controls down into the Ship and let it deal with them directly.
These things will improve the code, I think. What else?
We might want to have a score. We need a timer on the ship rather than making it reappear instantly where it was. We need the saucer.
But we have a game, at least until you run out of asteroids to shoot. We need new waves.
None of this seems daunting, does it? That might be because we’re getting too good at Asteroids after the fifth or sixth time implementing it.
But I’m having fun and I am the Cat who programs by himself, and all programs are alike to me.
See you next time!