Python Asteroids on GitHub

I think I can do better than I’ve done with the Fleets object. Let’s think about it.

Where I’d like to get, you’ll recall, is to a place where the Fleets object only has one collection in it, containing all the various kinds of Flyers. One thing that’s in my way is that there are users of the other collections from outside Fleets, and there are specialized Fleets whose capability needs to be moved elsewhere, almost certainly into a Flyer subclass.

If I recall correctly from this morning, I have tests that are accessing the special collections directly, and, in particular, some tests that are relying on missiles in the input missiles collection. Furthermore, these tests are now using list comprehensions to find out what’s inside Fleets:

    def test_missile_asteroid_scores_with_missiles_in_others(self):
        fleets = Fleets()
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        fleets.add_asteroid(asteroid)
        missile = Missile.from_ship(pos, Vector2(0, 0))
        fleets.add_flyer(missile)
        fleets.add_scorekeeper(ScoreKeeper())
        interactor = Interactor(fleets)
        interactor.perform_interactions()
        interactor.perform_interactions()
        missiles = [m for m in fleets.all_objects if isinstance(m, Missile)]
        assert not missiles
        assert interactor.testing_only_score == 20
        asteroids = [a for a in fleets.all_objects if isinstance(a, Asteroid)]
        assert len(asteroids) == 2

Let’s make a new object, a FleetsTestInspector, to be used inside tests to access specialized subsets of the Fleets content, without regard to where things go. We might even give it the ability to put things in if need be.

We won’t need to write specialized tests for this object I believe, because it will be used in tests that won’t run if the object doesn’t work. At least that’s how it seems to me.

I’ve named the class FleetsInspector and given it an alias of FI. Here’s its first use:

    def test_missile_asteroid_scores_with_missiles_in_others(self):
        fleets = Fleets()
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        fleets.add_asteroid(asteroid)
        missile = Missile.from_ship(pos, Vector2(0, 0))
        fleets.add_flyer(missile)
        fleets.add_scorekeeper(ScoreKeeper())
        interactor = Interactor(fleets)
        interactor.perform_interactions()
        interactor.perform_interactions()
        missiles = FI(fleets).missiles  # <===
        assert not missiles
        assert interactor.testing_only_score == 20
        asteroids = FI(fleets).asteroids  # <===
        assert len(asteroids) == 2

And the definition:

class FleetsInspector:
    def __init__(self, fleets):
        self.fleets = fleets

    @property
    def all(self):
        return self.fleets.all_objects

    @property
    def asteroids(self):
        return [a for a in self.all if isinstance(a, Asteroid)]

    @property
    def missiles(self):
        return [m for m in self.all if isinstance(m, Missile)]


FI = FleetsInspector

Tests are green. Commit: FleetsInspector used in a test.

OK, that’s at least neater, and it means that we should be able to remove all the references to Fleets’ accessors from tests by extending the FleetsInspector. We might even give it other capabilities as well.

Now in Fleets, I want to implement and use access methods for all the objects, not just the few that I have. Let’s consolidate them and make sure we have them all.

class Fleets:
    # adds and removes

    def add_asteroid(self, asteroid):
        self._asteroids.append(asteroid)

    def remove_asteroid(self, asteroid):
        self._asteroids.remove(asteroid)

    def add_flyer(self, flyer):
        self.others.append(flyer)

    def remove_flyer(self, flyer):
        self.others.remove(flyer)

    def add_missile(self, missile):
        self.add_flyer(missile)

    def remove_missile(self, missile):
        self.missiles.remove(missile)
        self.others.remove(missile)

    def add_score(self, score):
        self.others.append(score)

    def remove_score(self, score):
        self.others.remove(score)

    def add_scorekeeper(self, scorekeeper):
        self.others.append(scorekeeper)
    # no remove

    # no add_saucer
    def remove_saucer(self, saucer):
        self.saucers.remove(saucer)

    # no add ship
    def remove_ship(self, ship):
        self.ships.remove(ship)

I think we’ll need those missing adds for saucer and ship but I’m leaving them out for now. I think all the others are needed right now. I’ll check by looking for access to each of the individual Fleet instance properties.

I find one, the tests are green. Commit: adding and using add_ and remove_ on all Fleets Fleet instances, in aid of removing the accessors.

Here’s a test that needs improvement:

    def test_saucer_spawn(self):
        saucers = []
        fleets = Fleets([], [], saucers, [], [])
        saucer_fleet = fleets.saucers
        saucer_fleet.tick(0.1, fleets)
        assert not saucers
        saucer_fleet.tick(u.SAUCER_EMERGENCE_TIME, fleets)
        assert saucers

We should recast this thus:

    def test_saucer_spawn(self):
        fleets = Fleets()
        fleets.tick(0.1)
        assert not FI(fleets).saucers
        fleets.tick(u.SAUCER_EMERGENCE_TIME)
        assert FI(fleets).saucers

That’s much nicer. And green. Commit. Improve test to use FI.

I’ve got stuff going on now, so I’ll peck away gently here, being careful not to break things.

This test took me some rewriting:

    def test_unsafe_because_saucer_missile(self):
        fleets = Fleets()
        missile = Missile(u.CENTER, Vector2(0, 0), [0, 0, 0], [0, 0, 0])
        assert not FI(fleets).ships
        fleets.tick(u.SHIP_EMERGENCE_TIME - 1)
        fleets.add_saucer_missile(missile)
        fleets.tick(1)
        assert not FI(fleets).ships
        FI(fleets).clear_saucer_missiles()
        fleets.tick(0.001)
        assert FI(fleets).ships

Missile lifetime and ship emergence time are equal, so I had to insert the missile after some time had elapsed. I’m not sure whether this case can arise … perhaps if the saucer fires a missile after the ship goes under. Anyway it works.

Commit: recast test.

Distractions are too high for safe work. Let’s sum up.

Summary

I think the FleetInspector will at least make tests easier to write, and as long as it mostly uses access to Fleets.all_objects it should be relatively impervious to change. There are some invasive tests and I’ll try to improve them as I run across them.

I’m sure some of them are going to bite me, and recent experience suggests that I need to be more careful than I have been, and I’ve been pretty careful.

More tomorrow, enough for today. See you next time!