Python 56 - Design-Driven Mistakes?

Python Asteroids on GitHub

I am concerned about an aspect of the design which is inviting mistakes. How might we address it? I surprise myself with a much simpler change than I had expected to make today.

Admittedly, this program is so close to done that, if the point were to have an Asteroids game, we’d just finish it, burn it into ROM and start collecting quarters. But the point here, as always, is more about observing what the code tells us and improving things that need improving, because that’s what a good programmer does all day, every day: we change code¹.

One good reason to improve our code is that sometimes the existing design seems to encourage mistakes when we change it. We have an example here in outer space: inserting a quarter.

The program has a game instance, and the game instance has many things, including collections for the space objects, plus a persistent ship and saucer instance, plus some timers, and doo bee doo bee doo bee.

The Game class initialization code is about 55 lines long and is divided, for “clarity”, into eight methods, counting __init__. That’s a lot. This is a clear sign that Game probably does the work of two or more classes, not just one.

What’s worse, however, is that when you “insert a quarter”, the initialization for restarting the game is a dozen lines of code. That’s not the real problem: the real problem is, which dozen lines? Today’s answer is:

    def insert_quarter(self, number_of_ships):
        self.asteroids = []
        self.missiles = []
        self.ships = []
        self.asteroids_in_this_wave = 2
        self.game_over = False
        self.init_saucer_timer()
        self.score = 0
        self.ships_remaining = number_of_ships
        self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
        self.init_wave_timer()
        self.delta_time = 0

And, looking at this, I see an example of our problem. Why does this code init the asteroids, missiles, and ships collection, but not the saucers collection? Surely that’s a bug. What would happen if you were to insert a quarter when the saucer was on the screen?

I tried it, and I’ll tell you: everything disappears but the saucer, which continues to fly across the screen firing missiles. If you’re unlucky enough, your new ship will rez and the leftover saucer will instantly shoot you down. No fair!

So that’s a bug. I’ll fix it right now.

    def insert_quarter(self, number_of_ships):
        self.asteroids = []
        self.missiles = []
        self.saucers = []
        self.ships = []
        self.asteroids_in_this_wave = 2
        self.game_over = False
        self.init_saucer_timer()
        self.score = 0
        self.ships_remaining = number_of_ships
        self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
        self.init_wave_timer()
        self.delta_time = 0

Commit: clear saucers in insert_quarter.

That’s the problem. When we make changes to the code, some objects, notably Game, need to to have insert_quarter re-initialize them to the right starting condition. That’s very easy to forget, and I often do.

Mutable Objects

One root cause of this, I think, is the use and re-use of mutable objects.

It seems to me that there are some parts of the game that must, by the nature of things, be mutable. Somewhere there must be a collection of missiles, which come and go as they are fired, hit things, or time out. They have varying coordinates as they fly along, and so on.

Momentary Digression

I find myself thinking far more deeply about this than I had intended to. I’m wondering how far one could go toward having no mutable objects at all. Could we somehow keep all the changing values on the stack? Could we create a new position object every time we move, use it, and somehow keep it in the air?

Interesting challenge, but not my point here. My point here is:

When we create an instance of Game, we hold on to it forever. But the player perceives multiple games. She puts in another quarter, and new ships and asteroids appear, ready to do battle. Internally, however, our program uses the same instance of Game, and in the insert_quarter method, re-initializes what needs to be re-initialized. More or less, subject to whatever I forgot.

It could be different.

If the code created a new instance of Game when the player types “q” to insert a quarter, we could do all the initialization at creation time and our problem of what to do when creating a game, and our problem of what to do when she inserts a quarter would become one problem, not two. And that would be good.

If we did that, Game would still be a mutable object, changing as game play goes on, but it would not be re-used. So it would have to keep track of game state, but it wouldn’t be responsible for setting itself back to the original proper state. There’d be fewer opportunities to make mistakes, and that would be a good thing.

I’m thinking that what should be done is to extract a new class, AsteroidsGame from Game, and have Game create a new one every time you insert a quarter. Game could even, in principle, create a SpaceWarGame or SpaceInvadersGame instead.

I expect this extraction to be tricky. My rough plan is actually more like extracting BaseGame from Game, pulling out just the bits that are generic. Either way, there seems to be a lot to do.

Another Possibility

Creating a fresh game each time a quarter is inserted seems like a good idea. Could we do it right now? Let’s look at how Game is instantiated now. Here’s the main program:

# Asteroids Main program

from game import Game

asteroids_game: Game

if __name__ == "__main__":
    asteroids_game = Game()
    asteroids_game.main_loop()

I’m curious what would happen if we didn’t hold on to the game.

if __name__ == "__main__":
    Game().main_loop()

The game comes up and works fine. I am fascinated: who is holding on to the instance of Game? I guess it’s the main program’s stack.

Now if we actually wanted inserting a quarter to start a new instance, we could, I suppose, just create one and call its main loop. If we did that inside the first one’s main loop, we’d be recurring, and building a stack of Game instances, and that would be bad.

But if we could replace the asteroids_game instance with a new one, that might be a quick way to resolve our issue: we’d have to change a few things, so that the first time it comes up in Game Over mode but after that comes up running, but that should be easy enough.

Maybe we’ll have two ways to exit from main_loop, one indicating that we want the program to terminate, and one indicating that we want a new instance.

        self.game_init()
        while self.running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    self.running = False

            self.asteroids_tick(self.delta_time)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

We could still do the pygame.quit, which I think just shuts down pygame, not the program. It’s returning to main that quits the program.

I’ll try some hackery:

if __name__ == "__main__":
    for i in range(3):
        asteroids_game = Game()
        asteroids_game.main_loop()

So now, when I type Command+W to close the window, it immediately reopens, because of the loop, until the loop runs out, and the program exits.

So suppose we arranged that the pygame.QUIT (which seems to be triggered by Command+W) would return False, meaning “don’t keep running”, and when we just want a fresh game, we return True.

We’d have to rig something to ensure that the game starts, the first time, in GAME OVER state, if we really care about that feature. This might actually work.

I’ve done this much:

# Asteroids Main program

from game import Game

asteroids_game: Game

if __name__ == "__main__":
    keep_going = True
    while keep_going:
        asteroids_game = Game()
        keep_going = asteroids_game.main_loop()

class Game
    def main_loop(self):
        self.game_init()
        self.keep_going = False
        while self.running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    self.running = False
                    self.keep_going = False

            self.asteroids_tick(self.delta_time)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()
        return self.keep_going

Everything works the same now, since insert_quarter isn’t changing things.

I’ll commit this much: setting up for recreating game on insert quarter.

Now let’s call insert_quarter at the end of game’s long init cycle. That should start the game immediately, with everything else the same.

I really expected this to start me up in a live game:

    def __init__(self, testing=False):
        self.init_general_game_values()
        self.init_asteroids_game_values()
        self.init_space_objects()
        # self.init_timers()
        self.init_pygame_and_display(testing)
        if not testing:
            self.insert_quarter(u.SHIPS_PER_QUARTER)

What does the q key actually do?

        if keys[pygame.K_q]:
            self.insert_quarter(u.SHIPS_PER_QUARTER)

So why does the game think there are no ships?

Aside

All this confusion is due, in large part, to my decision to reuse the same Game instance for subsequent games. That’s why I want to change it: it’s helping me to be wrong, and I don’t need any help with that. There is some subtle difference between what goes on the first time and subsequent times that I’m missing.

I find that if I change this, the game starts as intended:

    def game_init(self):
        self.running = True
        self.saucer.init_for_new_game()
        self.insert_quarter(u.SHIPS_PER_QUARTER)

That last line used to say (0), not (u.SHIPS_PER_QUARTER). Now the game starts in running mode. I think I’ll allow that for now.

Now let’s change what happens when you hit the q key.

        if keys[pygame.K_q]:
            self.keep_going = True
            self.running = False

That works as intended: the game starts with a running game, inserting a quarter flashes the screen, creating a whole new window, and gives you a fresh game.

I think that’s a decent save point. Commit: new Game instance created for each quarter, new window and all.

Summary

So that was odd.

I fully intended to start working toward carefully extracting an AsteroidsGame that could be recreated by insert_quarter. There are other good reasons to do that, such as allowing the code that is not asteroids-dependent to be used for other games, but resolving the dual initialization issue seemed to be most important, because it has turned out to be error-prone.

But somehow I got the idea of “just” recreating the whole game in a loop in main, and with just a bit of ball-peening I was able to make that work.

There’s improvement needed, now that it’s working, including the fact that insert_quarter is now being called twice, once in my explicit __init__ code and once in game_init. There will be other duplicated initializations done once under the __init__ and again in insert_quarter.

But the fundamental concern, the need to re-use Game instances for more than one game is just gone. Once we normalize the init code a bit, there will be far fewer opportunities to do it wrong. It’ll generally just work, or not work. No more of this quaint little “if you insert a quarter with the saucer on the screen” stuff. Or, at least, less of it.

Fascinating. This is not what I expected. Much easier, and in that regard at least, better.

Huh. I surprised myself. That’s always good. See you next time!