Python Asteroids on GitHub

I need all the help I can get playing this game and I’m thinking I’d do better if I could shoot down the Saucer’s missiles. Also, some thinking, and hyperspace recharge cycle.

I’m supposing that missile v missile will be trivial. To make it harder, let’s write a test for it.

    def test_missile_v_missile(self):
        pos = Vector2(100, 100)
        zero_vel = Vector2(0, 0)
        m1 = Missile.from_ship(pos, zero_vel)
        m2 = Missile.from_saucer(pos, zero_vel)
        fleets = Fleets()
        fi = FI(fleets)
        fleets.add_flyer(m1)
        fleets.add_flyer(m2)
        interactor = Interactor(fleets)
        interactor.perform_interactions()
        assert not fi.missiles
        assert not fi.saucer_missiles

I think this should do it. My actual plan is that any two missiles can mutually destroy each other, even ship-ship or saucer-saucer. It’s just about impossible to make that happen, but if it does, it does.

I think we just need this to make the test run:

class Missile(Flyer):
    def interact_with_missile(self, missile, fleets):
        if missile.are_we_colliding(self.position, self.radius):
            self.die(fleets)

And it does. I figure it will be impossible for me to test this in the game, but it’s surely good to go. Commit: missiles destroy each other within range.

Miracle of miracles, I actually shot down two saucer missiles that were clearly going to hit me. I like this new feature! One more sticky note off the “Jira” keyboard tray.

Some Thinking

I have a sticky here saying “move -> update (tick?)”, which I believe refers to an old idea about what we should name the various methods that our Flyers receive from the Game. Currently the sequence is this:

    def asteroids_tick(self, delta_time):
        self.control_game()
        self.fleets.move(delta_time)
        self.perform_interactions()
            #begin_interactions
            #interact_with
            #end_interactions
        self.fleets.tick(delta_time)
        self.draw_everything()

I’ve shown in comments the messages that are sent in perform_interactions, so that we see the whole protocol

The note suggests renaming move to update, or, if I recall, to doing move as part of tick, which made sense when tick was first. It is now last.

It seems to me that everything could be done with just begin, interact, end, and draw.

The interaction design requires at least three calls, to give an object time to set its sensor flags, then to sense things, then to take any desired action.

It’s clear that if we removed move, so that the first thing in the cycle was begin, objects would both move and set flags in begin. That’s not coherent. So we should retain move (or update if we want to rename it).

What about end and tick? It seems likely to me that those could be combined, unless there is something in some object’s tick that clearly doesn’t belong as finalizing interactions. If all they do is check status and tick timers, it’s probably OK to combine those.

But wait. Missiles might be a good example. Missiles time out unconditionally:

class Missile(Flyer):
    def tick(self, delta_time, fleets):
        self.tick_timer(delta_time, fleets)

    def tick_timer(self, delta_time, fleets):
        self._timer.tick(delta_time, self.timeout, fleets)

    def timeout(self, fleets):
        fleets.remove_flyer(self)

Right. We could put that in end_interactions, but it makes more sense to me to have end_interactions relate to interactions, not to general time passing, handled bye tick.

I’ve convinced myself that tick should be preserved. And now there’s a new design criterion:

A robot may not injure a human being or, through inaction, allow a human being to come to harm.

No, wait, that’s not it.

The tick method is used for self-timing operations. The end_interactions method is used for interaction-related operations, including timing.

I’d bet a nickel that there are objects with interaction-related timing in tick rather than end. Let’s look. If we’re going to have a design principle we should at least know how well we’re adhering to it.

There are 16 implementors of tick.

Asteroid passes. Coin adds lots of objects and destroys itself. Explosion removes itself and adds Fragments. Fragments time out. GameOver passes. Missile times out. Saucer passes. SaucerMaker breaks the rule:

class SaucerMaker(Flyer):
    def tick(self, delta_time, fleets):
        if self._saucer_gone:
            self._timer.tick(delta_time, self.create_saucer, fleets)

The rule says that should be in end_interactions. Will it break a test if we make that change? Let’s find out.

LOL, quietly but literally. end_interactions method does not receive delta_time. We can’t tick our timers there. We could of course change its signature and pass delta_time in.

This discovery leads me to look for implementors of end_interactions.

Outside of tests, there are none. There’s one in a test, making sure that it gets sent.

Reflection

This discovery tells me to reset my brain and think a bit. I could just walk away, thinking that things are probably all just fine. A wise man might do that. We do have the possibility of removing the end_interactions call entirely, since no one really uses it, and the only would-be users seem to need a parameter we do not provide, delta_time.

However, as I scan through the rest of the implementors of tick, all of them are checking a condition they found during interaction, and using delta_time to tick a timer that they use to delay an action for a respectful interval.

Despite there being no delta_time in end_interactions signature, the way the code is written tells me that ideally, we would pass delta_time and use end_interactions for the things that happen based on what we discover during interactions.

The only outlier is Missile, which uses tick to time itself out.

The design isn’t quite right, is it? It’s good enough, in that it works.

I think it would be “better” if there were no tick operation, and if end_interactions were to receive delta_time.

I’m thinking now about the design of this thing as a framework for other games that we might implement. As it stands as just Asteroids, this is probably OK unless our purpose were to polish it to near perfection. (Which might be our purpose, the way things are going.) But as a framework, we’d want it to be as simple as possible, while retaining all the things you need.

In that context, we might argue that there will just be three calls to which you might with a Flyer to respond, begin, interact, and end. These might all provide delta_time in case you want it.

The general instructions in our imaginary manual might be something like this:

On begin_interactions
Your object should update itself, moving or otherwise adjusting itself, based on delta_time.

Your object should set any internal values that it may wish to use during interaction. (See Interactions.)

On interact_with_
Your object should implement interact_with_thing for any Flyer subclass thing that it needs to interact with.

Here, your object might determine that it is colliding with the Thing, or it might undertake any other Thing-related activity such as counting Things or sending a message to them.

On end_interactions
Your object should perform any operations that can only be done after interactions are complete, such as ticking timers based on what the object has seen. (See Timers.)

If your object uses timers of any kind, this is where they are usually ticked.

I think that design would be as small as we could reasonably make it. It might be too small: is it as clear as having what we have now, with our additional move and tick operations? We could argue that the additional calls make it a bit more clear to the Flyer implementor what she can do, and where to put things.

There’s something that almost seems right to me about the current setup. You get two calls that are just about you. They are named move and tick, perhaps could have better names. Then you get whatever interact_with_ calls you’ve implemented, prefixed by one begin_interactions and followed by one end_interactions. The move comes before the interactions, and the tick comes after.

Another thought. During one cycle, delta_time does not change. We could have the rule be that we pass it in only once. Meh, no. Then people would cache it as a member variable. Wastes space, code, and possibly leads to strange bugs. Belay that idea.

Conclusion, for now

For now, upon reflection, we’ll let it ride. I do think there’s a better design and better terms to be found, but at the moment I don’t see a change that seems to have enough value. I do think I’ll add a note about passing delta_time in the interactions, however, at least to the begin and end ones.

Did I just waste however long that took?

By my lights, it was time well spent. I gained a better understanding of what the objects all do, and a slightly better sense of what the Flyer’s cycle might want to be like. I’d prefer to have had a great idea, but that doesn’t always happen.

YMMV, but I value thinking, so long as it doesn’t drag on forever.

Let’s Code Something

I’d like to program a little something more. Do we have all vestiges of Asteroids removed from Game? Let’s do a quick scan, because if there’s anything left it’ll be a high priority.

There’s only this:

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

            self.asteroids_tick(self.delta_time)

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

    def asteroids_tick(self, delta_time):
        self.control_game()
        self.fleets.move(delta_time)
        self.perform_interactions()
        self.fleets.tick(delta_time)
        self.draw_everything()

The method is named asteroids_tick. And the object we deal with is called fleets. Both those names could be generalized. Not interesting enough, but maybe we should do it someday.

Tickets include:

  • Debug keys
  • hyperspace parameter lists are weird
  • hyperspace refractory period
  • Simplify Saucer
  • Better tests around startup
  • Star field

What does that one about hyperspace parameter lists mean?

class Ship(Flyer):
    def enter_hyperspace_if_possible(self, fleets):
        if not self._can_enter_hyperspace:
            return
        self._can_enter_hyperspace = False
        roll = random.randrange(0, 63)
        if self.hyperspace_failure(roll):
            self.explode(fleets)
        else:
            self.hyperspace_transfer()

    def hyperspace_failure(self, roll):
        return roll > 44 + self._asteroid_tally

    def explode(self, fleets):
        player.play("bang_large", self._location)
        fleets.remove_flyer(self)
        fleets.add_flyer(Explosion.from_ship(self.position))

    def hyperspace_transfer(self):
        x = random.randrange(u.SCREEN_SIZE)
        y = random.randrange(u.SCREEN_SIZE)
        a = random.randrange(360)
        self.move_to(Vector2(x, y))
        dx = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        dy = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        self.accelerate_to(Vector2(dx, dy))
        self._angle = a
        self._can_enter_hyperspace = False

That doesn’t look weird to me. The failure code is obscure, but it’s based on the actual game. It could use some explanation but that’s OK for now.

Transfer could be more clear with an inline, I think.

    def hyperspace_transfer(self):
        x = random.randrange(u.SCREEN_SIZE)
        y = random.randrange(u.SCREEN_SIZE)
        self.move_to(Vector2(x, y))
        self._angle = random.randrange(360)
        dx = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        dy = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        self.accelerate_to(Vector2(dx, dy))
        self._can_enter_hyperspace = False

Commit: minor refactoring, inline and move line.

Nota Bene
We were just browsing the code, looking for something else. We noticed some code that could be more clear. We made it more clear and committed it. The world is just a tiny bit better.

I don’t see a weird parameter list. I destroy the ticket.

What about a refractory delay as long as we’re here?

What the story has in mind is that for some period of time after you go to hyperspace, you cannot go again. Hitting the key does nothing. The hyperspace hypercapacitors need to recharge or something.

Let’s do that. Do I have to do a test? Yes, probably would be wise.

I have existing hyperspace test (and another series of checks for the failure function):

    def test_hyperspace(self):
        impossible = Vector2(-5, -5)
        impossible_angle = 370
        ship = Ship(impossible)
        ship._angle = impossible_angle
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        position = ship.position
        angle = ship._angle
        assert position != impossible and angle != impossible_angle
        assert ship._location.velocity != Vector2(0, 0)
        ship.enter_hyperspace_if_possible([])  # cannot fail
        assert ship.position == position and ship._angle == angle

What do we want to implement? The story might go like this:

After a successful attempt to go to hyperspace, the ship cannot enter hyperspace for a time interval u.SHIP_HYPERSPACE_REFRACTORY_PERIOD. Attempts within that interval have no effect. The period should be 5 seconds for now.

Let’s see if we can write a test that should succeed but fails. I get this much:

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._can_enter_hyperspace

Now, in the real controls code, we have this:

        if keys[pygame.K_SPACE]:
            self.enter_hyperspace_if_possible(fleets)
        else:
            self._can_enter_hyperspace = True

We really have two criteria for re-entering hyperspace. First, the player has to lift off the hyperspace key. Second, the recharge time must have expired. (And I think I’ll use the name RECHARGE rather than REFRACTORY for clarity.)

Let’s first rename that member.

        if keys[pygame.K_SPACE]:
            self.enter_hyperspace_if_possible(fleets)
        else:
            self._hyperspace_key_ready = True

If this thing is any good, it will have refactored my test:

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready

Love me some PyCharm.

Let’s assume there’s another member for recharge complete and test it:

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged

Test fails for lack of that member. Add it:

    def hyperspace_transfer(self):
        x = random.randrange(u.SCREEN_SIZE)
        y = random.randrange(u.SCREEN_SIZE)
        self.move_to(Vector2(x, y))
        self._angle = random.randrange(360)
        dx = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        dy = random.randrange(u.SHIP_HYPERSPACE_MAX_VELOCITY)
        self.accelerate_to(Vector2(dx, dy))
        self._hyperspace_key_ready = False
        self._hyperspace_recharged = False

I have also initialized them both in __init__. Let’s add checks to the test.

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged

Test is still green. So far so good. Now we want it to count down. I think we’ll use a Timer, because that’s our coding standard. First complete the test:

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged
        ship.tick(0.1, [])
        assert not ship._hyperspace_recharged
        ship.tick(u.SHIP_HYPERSPACE_RECHARGE_TIME, [])
        assert ship._hyperspace_recharged

The test fails on the last assert. Perfect. We’re almost there, but not quite.

Now to make it pass again.

class Ship(Flyer):
    def __init__(self, position):
        self.radius = 25
        self._location = MovableLocation(position, Vector2(0, 0))
        self._hyperspace_key_ready = True
        self._hyperspace_recharged = True
        self._hyperspace_timer = Timer(u.SHIP_HYPERSPACE_RECHARGE_TIME)
        ...

And …

class Ship(Flyer):
    def tick(self, delta_time, fleets):
        if not self._hyperspace_recharged:
            self._hyperspace_timer.tick(delta_time, self.recharge)

    def recharge(self):
        self._hyperspace_recharged = True

Test is green again. But we need to do a bit more. We need to show that the ship did not enter hyperspace after the first tick and did after the second.

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged
        ship.move_to(impossible)
        ship.tick(0.1, [])
        assert not ship._hyperspace_recharged
        assert ship.position == impossible
        ship.tick(u.SHIP_HYPERSPACE_RECHARGE_TIME, [])
        assert ship._hyperspace_recharged
        assert ship.position != impossible

After we enter hyperspace the first time, we move the ship to impossible, so that we can check whether it has moved. (There is a small chance that it would stay in the same place on the second successful call. Wouldn’t that be a delicious intermittent test?)

After the time has elapsed, the test should pass. Right now, I expect it to fail on the assert == impossible.

It doesn’t, because I forgot to set the key ready.

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged
        ship.move_to(impossible)
        ship._hyperspace_key_ready = True
        ship.tick(0.1, [])
        assert not ship._hyperspace_recharged
        assert ship.position == impossible
        ship.tick(u.SHIP_HYPERSPACE_RECHARGE_TIME, [])
        assert ship._hyperspace_recharged
        assert ship.position != impossible

I also forgot to try to enter hyperspace, which kind of makes the test useless. Try again:

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged
        ship.move_to(impossible)
        ship._hyperspace_key_ready = True
        ship.tick(0.1, [])
        ship.enter_hyperspace_if_possible([])
        assert not ship._hyperspace_recharged
        assert ship.position == impossible
        ship.tick(u.SHIP_HYPERSPACE_RECHARGE_TIME, [])
        ship.enter_hyperspace_if_possible([])
        assert ship._hyperspace_recharged
        assert ship.position != impossible

        ship.tick(0.1, [])
        ship.enter_hyperspace_if_possible([])
        assert not ship._hyperspace_recharged
>       assert ship.position == impossible
E       assert <Vector2(999, 947)> == <Vector2(-5, -5)>

OK, that’s what I expect. Now one more change and I expect a pass.

Darn, I keep getting the test wrong. I have to check the flag before entering.

    def test_hyperspace_recharge(self):
        impossible = Vector2(-5, -5)
        ship = Ship(impossible)
        assert ship._hyperspace_key_ready
        assert ship._hyperspace_recharged
        ship._asteroid_tally = 99
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible
        assert not ship._hyperspace_key_ready
        assert not ship._hyperspace_recharged
        ship.move_to(impossible)
        ship._hyperspace_key_ready = True
        ship.tick(0.1, [])
        ship.enter_hyperspace_if_possible([])
        assert not ship._hyperspace_recharged
        assert ship.position == impossible
        ship.tick(u.SHIP_HYPERSPACE_RECHARGE_TIME, [])
        assert ship._hyperspace_recharged # <=== moved
        ship.enter_hyperspace_if_possible([])
        assert ship.position != impossible

The test passes with this code in Ship:

    def enter_hyperspace_if_possible(self, fleets):
        if not self._hyperspace_key_ready or not self._hyperspace_recharged:
            return
        self._hyperspace_key_ready = False
        roll = random.randrange(0, 63)
        if self.hyperspace_failure(roll):
            self.explode(fleets)
        else:
            self.hyperspace_transfer()

We are green. Let’s commit, then refactor. Commit: hyperspace has recharge period of 5 seconds.

Now let’s reverse that if, to begin with.

    def enter_hyperspace_if_possible(self, fleets):
        if self._hyperspace_key_ready and self._hyperspace_recharged:
            self._hyperspace_key_ready = False
            roll = random.randrange(0, 63)
            if self.hyperspace_failure(roll):
                self.explode(fleets)
            else:
                self.hyperspace_transfer()

I was thinking whether we should reset recharged right there, but it doesn’t seem fair to penalize the next ship for the bad luck of the previous one if its hyperspace generator explodes.

This code is a bit more distributed around than I’d like. These two flags work together here, but they are reset here, and in transfer and set in control and in tick.

It’s tempting to have a tiny object to deal with this. It might work like this:

  1. HyperspaceGenerator has two internal flags, ready and recharged. It inits to ready and recharged.
  2. We create one in __init__.
  3. In enter_hyperspace_if_possible we ask it can_try_hyperspace.
  4. It answers True if both ready and recharge. I think it should reset ready right then.
  5. When the key goes up, we tell it set_ready.
  6. If we go to hyperspace, we tell it trigger_hyperspace, and it resets recharged.
  7. When we tick, we send tick to the generator. It ticks as needed and resets recharged as needed.

This seems like a lot, but if you think about it, we’re already doing all those things, except that we’ve separated it between two apparently independent flags.

I think we should do that. And I think we’ll save it for next time. This article is long enough, and the generator will require a fresh mind.

Summary

We allowed missiles to destroy each other. It was quite simple, even with a test.

We considered our overall cycle design and the methods called. We still think it could be better but do not see just what changes would make it better.

Then we implemented the hyperspace recharge period, with a fairly intricate test (but it’s an intricate topic) and then refactored the code a tiny bit. And we have an idea for a tiny object to help Ship be a bit simpler by using a separate HyperspaceGenerator.

We’ll probably do that. I remove the recharge ticket and add a generator ticket.

I wonder … if we build a HyperspaceGenerator object, will it be easier to test hyperspace than it was this morning. My confusion in the test might indicate that the objects themselves are not cooperating as well as they might. Since they’re just independent booleans, that’s not surprising. We’ll see if we can improve this whole scheme today or tomorrow.

A good morning. See you next time!