Python Asteroids on GitHub

I think the coin test is better than it was, but it isn’t what it could be. Let’s do better, just for the practice. I surprise myself with a challenge.

Coin tests now look like this:

    def all_classes_except(self, excluded_classes):
        excluded_names = [k.__name__ for k in excluded_classes]
        all_classes = self.all_known_classes()
        remaining = [k for k in all_classes if k.__name__ not in excluded_names]
        return remaining

    def all_known_classes(self):
        return {
            Asteroid, BeginChecker, EndChecker,
            Fragment, GameOver, Missile, Saucer, SaucerMaker,
            Score, ScoreKeeper, Ship, ShipMaker, Thumper, WaveMaker}

    def test_no_unchecked_classes(self):
        # must check names because pytest recompiles.
        known_names = [k.__name__ for k in self.all_known_classes()]
        subs = Flyer.__subclasses__()
        new = [k for k in subs if k.__name__ not in known_names]
        assert not new

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(Asteroid())
        coin.slug(fleets)
        desired = [GameOver, SaucerMaker, ScoreKeeper, Thumper, WaveMaker]
        undesired = self.all_classes_except(desired)
        for klass in desired:
            assert fi.select_class(klass)
        for klass in undesired:
            assert not fi.select_class(klass)

    def test_quarter(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(Asteroid())
        coin.quarter(fleets)
        desired = [SaucerMaker, ScoreKeeper, ShipMaker, Thumper, WaveMaker]
        undesired = self.all_classes_except(desired)
        for klass in desired:
            assert fi.select_class(klass)
        for klass in undesired:
            assert not fi.select_class(klass)

These are now pretty good. They know all the Flyer subclasses that existed at the time the tests were written, and for each type of coin, they have a list of all the classes that that coin should instantiate into the fleets. They check that an instance of each of those is present after executing the coin, and they check that no other classes from the known list, are present. There is also one test, test_no_unchecked_classes that detects new Flyer subclasses that are not reflected in the static list all_known_classes. That will fail when we add new Flyer subclasses and that will trigger us to update these tests.

That seems nearly good, but lets list things that we might not like:

One of the coins could put an instance of a non-Flyer into the mix. That would be weird, but I’ve made weird mistakes before. Since we only check known subclasses, this error would slip through. It would be better if the final bit of the coin tests could check for any objects not of the same class as expected.

The class checking is done by name, not class identity. When we check for identity, the tests can fail looking for BeginChecker and EndChecker. I believe that’s because pytest recompiles the tests and therefore the class declared in all_classes is not the same as the one compiled into the tests.

I think we can fix the second situation by making a separate file for the support object, those two Checkers and the FleetsInspector. If we do not put tests in there, I hope pytest will not recompile the file. I’m not sure about that, but it’s the thing to do anyway.

Let’s change these tests to check the class, not the name. We’re on a fresh commit, so we can roll back if this doesn’t work out.

That’s easily done and of course I get to edit a lot of imports to hook things back up. Commit: move FI and Checker classes to tools to try to avoid recompiling.

Now let’s see if we can convert back to checking classes rather than class names in the coin tests.

    def all_classes_except(self, excluded_classes):
        all_classes = self.all_known_classes()
        return all_classes - excluded_classes

I had to change the excluded classes to be sets, as here:

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(Asteroid())
        coin.slug(fleets)
        desired = {GameOver, SaucerMaker, ScoreKeeper, Thumper, WaveMaker}
        undesired = self.all_classes_except(desired)
        for klass in desired:
            assert fi.select_class(klass)
        for klass in undesired:
            assert not fi.select_class(klass)

But this isn’t really what we want, is it? We really want to be sure that there are no instances of any class other than those desired. We don’t really want undesired at all.

What we need is to get a list of all classes represented in fleets, and then remove our expected, and expect that set to be empty.

Let’s have FI help us, since that’s its job, helping us.

class FleetsInspector:
    def all_classes(self):
        return set(map(lambda flyer: flyer.__class__, self.fleets.flyers))

I think that should do it.

I recast one test:

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(Asteroid())
        coin.slug(fleets)
        desired = {GameOver, SaucerMaker, ScoreKeeper, Thumper, WaveMaker}
        undesired = fi.all_classes() - desired
        for klass in desired:
            assert fi.select_class(klass)
        assert not undesired

That should do the trick. Let’s briefly break it:

    def test_slug(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(Asteroid())
        coin.slug(fleets)
        desired = {SaucerMaker, ScoreKeeper, Thumper, WaveMaker}
        undesired = fi.all_classes() - desired
        assert not undesired
        for klass in desired:
            assert fi.select_class(klass)

That fails just as I’d like:

        undesired = fi.all_classes() - desired
>       assert not undesired
E       AssertionError: assert not {<class 'game_over.GameOver'>}

I like checking undesired first, though in actual use either order is OK.

I change the other two tests to match. Green. Commit: coin tests now check for instances of any class other than those in the desired list.

Now there’s no one using the all_except, so I remove that. And we have this test, still using names:

    def test_no_unchecked_classes(self):
        # must check names because pytest recompiles.
        known_names = [k.__name__ for k in self.all_known_classes()]
        subs = Flyer.__subclasses__()
        new = [k for k in subs if k.__name__ not in known_names]
        assert not new

If my tools change avoided recompiling, I should be able to do this with classes, not names.

    @staticmethod
    def all_known_flyer_subclasses():
        return {
            Asteroid, BeginChecker, EndChecker,
            Fragment, GameOver, Missile, Saucer, SaucerMaker,
            Score, ScoreKeeper, Ship, ShipMaker, Thumper, WaveMaker}

    def test_no_unchecked_classes(self):
        # if this fails, we need to update `all_known_flyer_subclasses`
        # and re-verify the coin tests to be sure they don't need updating
        subs = set(Flyer.__subclasses__())
        assert not subs - self.all_known_flyer_subclasses()

That runs green. It’s really a belt+suspenders test but I feel better having it. Commit: improve test_no_unchecked_classes.

Reflection and Summary

I think the coin tests are now actually better: they plug a very unlikely hole, the possibility that we would put an instance of some class into the mix, that is not one of the desired classes. Previously, we were only checking for known Flyer subclasses not in the desired list. Small hole, but a hole.

Moving the tools out of the test file resolved the issue with BeginChecker and EndChecker showing up where they shouldn’t. Maybe. It is possible that the preceding changes fixed that issue. Either way, the tools should be off in a separate tests/tools.py, so that was worth doing as well.

Now I would freely grant that these tests are probably not as important as tests for interactions or the like, but they are central to the game working correctly, even though they may never break and may never find a problem. They do express, separately from the coin implementations, what the coins should do, so they do serve to verify what we could read in the coin.py module.

What remains unclear, I think, is why this creates the Asteroids game:

{SaucerMaker, ScoreKeeper, ShipMaker, Thumper, WaveMaker}

While this creates an attract screen displaying GAME OVER.

{GameOver, SaucerMaker, ScoreKeeper, Thumper, WaveMaker}

Nor is it obvious that we could leave the ScoreKeeper out of the slug mix and everything would still be just fine, except that no score would appear in attract mode.

The cooperation of the starting collection creates a dynamic situation that acts like the game of Asteroids in the one case, and acts like an attract screen in the other, but there is nothing anywhere in the code that tells us why that happens, nothing that gives us guidance on how to create some different game.

A wild challengeosaur has appeared!

That makes me think of a challenge for the code and for myself.

Suppose we have released this very fun game, and we get an idea to make it even better: two player mode. In two player mode, there are two scores, one on the left and one on the right. Each score gets the same number of starting ships, and play alternates, first with a player scoring on the left scores, then the second player on the right, back and forth.

Obviously some rigmarole is needed, highlighting one score or the other, depending on whose turn it is. If player two gets a free ship and player one does not, then after player one uses up all his ships, player two plays alone. And so on.

How easy, or how difficult, will it be to add two player mode to the existing game? It’s pretty clear that I wasn’t planning for it. Will this break our scheme? Or will our scheme make it easy? Or … probably most likely … will it be not terribly difficult but not trivial?

Shall we find out? Shall we try it? Stay tuned …