Python 141

Python Asteroids on GitHub

Observations from Rickard. And let’s remove some Flyer subclasses. I still think I’ve gone too far.

Rickard and I had a few exchanges on Mastodon, which I’ll not quote here, and he has kindly written an article about his concerns.

As I get it, and I freely grant that I may not quite have it, Rickard is not comfortable with just “micro tests” and wants to see more integrated tests. I am not sure how far apart we’d be if we were working together: perhaps not as far as our exchanges suggest.

In my Asteroids game today, I use quite a few tests that are, I think, integrated exactly where Rickard would put them, namely setting up some objects in a Fleets, and then calling perform_interactions and checking to see if the outcome is what it should be.

This kind of test is really pretty good, because it will generally answer whether the objects in the mix do in fact interact properly during the perform_interactions cycle. Since that cycle interacts all pairs of objects in the mix, it’s good when there are two objects under test, and even better when there are more than two.

I do have some tests that just call the interact_* methods directly. These tests could use the perform_interactions scheme but, at the time when I wrote them, the other way seemed better. Here’s an example:

    def test_thumper_thumps(self):
        # thumper ticks between 30 and 8 60ths
        thumper = Thumper(beat1, beat2)
        thumper.interact_with_ship(None, None)
        thumper.interact_with_asteroid(None, None)
        assert not thump
        time = 31/60
        thumper.tick(time, None)
        assert thump == 1
        thumper.tick(time, None)
        assert thump == 2
        thumper.tick(time, None)
        assert thump == 1

This test just wants to know that if the Thumper sees a ship and at least one asteroid, it will start ticking in about a half second, and tick thereafter at that speed. So it needs the interactions to set the Thumper’s internal state, and then just ticks to see whether it triggered its beats.

We could do that through Fleets, but in my view it would be harder to set up and no easier to understand.

Would it provide any more confidence? Well, we can see how it might. It would give us confidence that the Thumper can survive the entire cycle; it would ensure that something in begin or end didn’t break; and similar valid concerns.

Even that doesn’t exercise everything: the full cycle is update, begin, process, end, tick, draw. It probably should not be that complex, but it grew that way and we have not as yet decided to simplify it.

The point of tests is to build confidence. Rickard and I can productively talk about what tests give us how much confidence, and why, but there is no right answer, other than if you feel the need for another test, please create it. If all the confirming tests I write work and never find anything, I’ll probably learn not to write so many, or learn to write tests that are more difficult.

I write fewer tests when I am confident that the code is good. This is not a good habit, it’s just what I observe myself doing. The reason why it’s mot a good habit? Because I am sometimes wrong to be confident and there’s no way for me to know when that is. So writing more tests seems wasteful, because some large percentage of the time, it is wasteful. And then, some small percentage of the time, it pays off.

I suppose we should only wear our seat belt if we are going to get in an accident. The trick is knowing when that is.

I also write fewer tests when they are difficult or tedious to write. And as things stand, the Fleets-level perform_interactions tests are kind of a pain to write. Here’s an example:

    def test_missile_v_asteroid(self):
        pos = Vector2(100, 100)
        zero_vel = Vector2(0, 0)
        missile = Missile.from_ship(pos, zero_vel)
        asteroid = Asteroid(2, pos)
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(missile)
        fleets.append(asteroid)
        fleets.perform_interactions()
        assert not fi.missiles
        assert len(fi.asteroids) == 2

The test has Arrange of 9 lines, Act of 1 line, Assert of 2 lines. In short, it’s a pain to set up.

I have not come up with an easier way to do the setup. In particular, since the interesting interactions often relate to collisions, sometimes you want objects close together, and sometimes you don’t, so specifying their position and such is part of the deal. And since they often want to be created with a velocity, you get to do that as well.

This isn’t very difficult. Why is he whining about this, you’re probably asking yourself. Well, I’m just reporting that the kind of person I am, when faced with only about this much difficulty, sometimes skips writing the test. It’s never as hard as I felt it would be, and the lack of a test is too often far worse than I thought it would be.

I’m just a human being, reporting what I do, even when it’s clear that I could do better. And, happily, I do manage to do better. This version of Asteroids is, I think better tested than any I’ve done before. And the testing is better this far into the project than it was at the beginning.

So I kind of improve. And, often, I fall down. Such is life.

Summary: Testing

I don’t feel a disagreement with Rickard. I agree that we get our confidence a bit differently, and if we were pairing, which could happen, we’d write every test that one or the other of us wanted. I think that’s always best: if anyone on the team wants a test, we create it. It is fair, I think, to ask them to create it, but it’s not fair to refuse to help.

Let’s do some work. First we’ll think about why, and what kind.

Gratuitous Complexity

There’s no doubt in my mind that this decentralized model is hard to think about, in an odd sort of way. Let’s take the Coin as an example. My thoughts probably went something like this:

When the program starts, it should start in attract mode, with asteroids and saucer but no ships. When the user types “q”, for “quarter”, the game should enter game mode, which will spawn a ship, then some asteroids, then later run the saucer, and game play should commence.

If we just populate the Fleets with a SaucerMaker and a WaveMaker and a GameOver, it would run in attract mode. If we populate with SaucerMaker, WaveMaker and ShipMaker, it would run the game. Oh, yeah, we need the ScoreKeeper and Thumper as well.

What if we had a new Flyer, Coin, that we would just toss into the mix when the user types “q”. Then the Game only has to know to create a Coin: it has no idea what the Coin even is. Then, I guess on the tick event, the Coin will clear the mix, dump the right starting configuration into the mix, and remove itself.

Neat!

And to start in attract mode, there could just be another kind of Coin. Call it a “slug”.

Now it turns out that Coin is just about that simple:

class Coin(Flyer):

    @classmethod
    def quarter(cls):
        return cls(True, True)

    @classmethod
    def slug(cls):
        return cls(False, True)

    @classmethod
    def no_asteroids(cls):
        return cls(True, False)

    def __init__(self, is_quarter=True, want_asteroids=True):
        self.is_quarter = is_quarter
        self.want_asteroids = want_asteroid

    def tick(self, delta_time, fleets):
        fleets.clear()
        fleets.append(SaucerMaker())
        fleets.append(ScoreKeeper())
        fleets.append(Thumper())
        if self.want_asteroids:
            fleets.append(WaveMaker())
        fleets.append(ShipMaker() if self.is_quarter else GameOver())

    def interact_with(self, other, fleets):
        other.interact_with_coin(self, fleets)
        
    def interact_with_asteroid(self, asteroid, fleets):
        pass

    def interact_with_explosion(self, explosion, fleets):
        pass

    def interact_with_fragment(self, fragment, fleets):
        pass

    def interact_with_missile(self, missile, fleets):
        pass

    def interact_with_saucer(self, saucer, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        pass

    def draw(self, screen):
        pass

Of course, since it’s a Flyer, it has to implement all those interact_with_* methods that it doesn’t need. And it has to call interact_with_coin, because those are the rules, and that means that everyone else has to implement interact_with_coin, or at least think about it.

Now I still think it’s really nifty to just toss in a coin and the game starts up. And it was easy to write. It was only slightly tedious to implement because of the required methods, but PyCharm will generate them. And because interact_with_coin isn’t abstract but defaulted, the other classes didn’t need to change.

But if you want to understand how the game starts up, you have to retrace that thinking, starting here:

    def __init__(self, testing=False):
        self.delta_time = 0
        self.init_pygame_and_display(testing)
        self.fleets = Fleets()
        self.fleets.append(Coin.slug())

Then you find Coin and you think about tick, and then you see what objects go in there. And if you’re new to the system, you have to think about what those do. I do not have to think about those, because I’m not new here … but even I might have forgotten Thumper or ScoreKeeper today, except that I had the code in front of me.

We have almost no references to Coin, all in Game. It has no tests. (See above. I was confident that I would get it right since it had to be tried in game play anyway.)

Each of the references is of the form

        self.fleets.append(Coin.xyz())

Where xyz is one of its class methods, quarter, slug, or no_asteroids. The last one is a debug key to let me test encounters with the saucer: it creates a game with no WaveMaker and therefore no asteroids.

Add simplicity

If we had a call that wasn’t much more difficult than the one above, but that didn’t put Coin into the mix, we could simplify coin, simplify the rest of the system because Coin isn’t in the mix, and things might generally be just a bit simpler.

Why couldn’t we say this instead:

        Coin.slug(fleets)

We could. Let’s do. Let’s just change all the statements and make it work. Or would this be just a super time to write a test? Yes, it would.

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        Coin.slug(fleets)
        assert fi.saucermakers
        assert fi.scorekeepers
        assert fi.thumpers
        assert not fi.wavemakers
        assert not fi.shipmakers

FleetInspector doesn’t know any of those properties. Extend it. Tedious but easy.

Now in Coin, we can check for the fleets member and use it only if we get it. Or shall we just let it be broken?

Let’s first make it work in situ, then break it.

class Coin(Flyer):
    @classmethod
    def slug(cls, fleets):
        coin = cls(False, True)
        coin.tick(0, fleets)
        return coin

I sort of expected my test to run, but it doesn’t. Makes me glad that I wrote it.

The test is wrong, we do get a wavemaker.

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        Coin.slug(fleets)
        assert fi.saucermakers
        assert fi.scorekeepers
        assert fi.thumpers
        assert fi.wavemakers
        assert not fi.shipmakers

That passes. But I didn’t check fleets, so other tests broke.

    @classmethod
    def slug(cls, fleets=None):
        coin = cls(False, True)
        if fleets:
            coin.tick(0, fleets)
        return coin

We’re green. Commit: Coin.slug adds directly to fleets if one provided.

Let’s do the others the same way, tests first.

    def test_quarter(self):
        fleets = Fleets()
        fi = FI(fleets)
        Coin.quarter(fleets)
        assert fi.saucermakers
        assert fi.scorekeepers
        assert fi.thumpers
        assert fi.wavemakers
        assert fi.shipmakers

    @classmethod
    def quarter(cls, fleets=None):
        coin = cls(True, True)
        if fleets:
            coin.tick(0, fleets)
        return coin

Green. Commit: Coin.quarter adds directly to fleets if one provided.

    def test_quarter(self):
        fleets = Fleets()
        fi = FI(fleets)
        Coin.quarter(fleets)
        assert fi.saucermakers
        assert fi.scorekeepers
        assert fi.thumpers
        assert fi.wavemakers
        assert fi.shipmakers


    @classmethod
    def no_asteroids(cls, fleets=None):
        coin = cls(True, False)
        if fleets:
            coin.tick(0, fleets)
        return coin

Commit: Coin.no_asteroids adds directly to fleets if one provided.

Now find all the users of Coin and fix them:

class Game:
    def __init__(self, testing=False):
        self.delta_time = 0
        self.init_pygame_and_display(testing)
        self.fleets = Fleets()
        Coin.slug(self.fleets)

    def accept_coins(self):
        keys = pygame.key.get_pressed()
        if keys[pygame.K_q]:
            Coin.quarter(self.fleets)
        elif keys[pygame.K_a]:
            Coin.no_asteroids(self.fleets)

Test game, these are all game operations. Works. Commit: Game now uses option to pass fleets to Coins.

Now there is no reason why Coin should be a Flyer. Strip it and change the tick method to give it a better name and not bother with the useless delta_time.

class Coin:

    @classmethod
    def quarter(cls, fleets=None):
        coin = cls(True, True)
        if fleets:
            coin.populate(fleets)
        return coin

    @classmethod
    def slug(cls, fleets=None):
        coin = cls(False, True)
        if fleets:
            coin.populate(fleets)
        return coin

    @classmethod
    def no_asteroids(cls, fleets=None):
        coin = cls(True, False)
        if fleets:
            coin.populate(fleets)
        return coin

    def __init__(self, is_quarter=True, want_asteroids=True):
        self.is_quarter = is_quarter
        self.want_asteroids = want_asteroids

    def populate(self, fleets):
        fleets.clear()
        fleets.append(SaucerMaker())
        fleets.append(ScoreKeeper())
        fleets.append(Thumper())
        if self.want_asteroids:
            fleets.append(WaveMaker())
        fleets.append(ShipMaker() if self.is_quarter else GameOver())

Commit: Coin is no longer a flyer.

But wait, there’s more. We can remove the interact_with_coin wherever it may be.

class Flyer:
    def interact_with_coin(self, missile, fleets):
        pass

Gone. Green. Commit: remove unnecessary interact_with_coin from Flyer.

I think we’re done here, at least for now. Between the testing discussion and the Coin changes you’re probably tired of reading.

There is more we could do. We could make the fleets parameter required, and we could perhaps refactor to remove the conditionals in the generation. But we’re tired, and when we’re tired, we stop.

Summary: Code Changes

I am becoming more aware that while these little objects seem to me to be easy to write, they are less easy to understand later. And each one that we add adds at least a bit of complexity everywhere, because we have to at least think about whether and how other objects interact with them.

While there might be a general way to reduce the difficulty, such as separating out Actors vs Audience, that is yet to be invented and might not really help. What certainly helps a bit is to remove the objects that just pop in and pop out, like Coin, which we just did. Explosion is another of those: it creates Fragments and then pops out.

Score might be a candidate, but it interacts with ScoreKeeper, and since every game’s scoring is different, we might want to keep that relationship the same. Other games would likely use a completely different ScoreKeeper and different kinds of Score objects. So that one may want to continue to exist.

I’m sure we can get rid of Explosion just as we did with Coin. We got some tests out of doing Coin, and the same might be true of Explosion.

But that is for another time. For now, we’ve done enough.

See you next time!