Python 63 - Keep trying ...

Let’s move some more Game responsibilities closer to where they belong. (It goes better this time!)

    def asteroids_tick(self, delta_time):
        self.fleets.tick(delta_time)
        self.check_ship_spawn(self.ship, self.ships, delta_time)
        self.check_saucer_firing(delta_time, self. saucers, self.saucer_missiles, self.ships)
        self.check_next_wave(delta_time)
        self.control_game(self.ship, delta_time)
        self.process_collisions()
        self.draw_everything()
        if self.game_over: self.draw_game_over()

    def check_saucer_firing(self, delta_time, saucers, saucer_missiles, ships):
        for saucer in saucers:
            saucer.fire_if_possible(delta_time, saucer_missiles, ships)

It seems rather clear that the Saucer could be checking for itself, if it had access to saucer_missiles. And we’re at least part of the way there, passing Fleets to tick:

class Fleets:
    def tick(self, delta_time):
        all_true = True
        for fleet in self.fleets:
            if not fleet.tick(delta_time, self):
                all_true = False
        return all_true

class SaucerFleet:
    def tick(self, delta_time, fleets):
        super().tick(delta_time, fleets)
        if not self.flyers:
            self.timer.tick(delta_time)
        return True

class Fleet:
    def tick(self, delta_time, fleets):
        result = True
        for flyer in self:
            result = flyer.tick(delta_time, self, fleets) and result
        return result

class Saucer:
    def tick(self, delta_time, fleet, _fleets):
        self.move(delta_time, fleet)
        return True

The saucer already has the timer in itself:

class Saucer:
    # noinspection PyAttributeOutsideInit
    def set_firing_timer(self):
        self.missile_timer = Timer(u.SAUCER_MISSILE_DELAY, self.fire_if_missile_available)

    def fire_if_missile_available(self, saucer_missiles, ships):
        if self.a_missile_is_available(saucer_missiles):
            self.fire_a_missile(saucer_missiles, ships)
            return True
        else:
            return False

    @staticmethod
    def a_missile_is_available(saucer_missiles):
        return len(saucer_missiles) < u.SAUCER_MISSILE_LIMIT

    def fire_a_missile(self, saucer_missiles, ships):
        saucer_missiles.append(self.create_missile(ships))

Seems like we need to provide both the saucer_missiles and ships collections (not even the fleets?). I wouldn’t expect len to work on the saucer_missiles fleet. I’d best do a test.

Right. Turns out that Fleets.saucer_missiles is this:

    @property
    def saucer_missiles(self):
        return self.fleets[3].flyers

That’s not really on. We shouldn’t be exposing those inner collections.

Let me change that one to just return the fleet and see what breaks. My new test fails:

    def test_len(self):
        saucer_missiles = []
        fleets = Fleets([], [], [], saucer_missiles, [])
        s_m_fleet = fleets.saucer_missiles
        assert len(s_m_fleet) == 0

Fleet does not understand len but it could. Well, actually, it couldn’t. You can’t just implement len on an arbitrary class. There must be a trick to it. Yes, you have to implement __len__ OK:

    def __len__(self):
        return len(self.flyers)

Right, test passes. I’ll change the other accessors. I’m just trying to get everyone accessing a Fleet instead of the flyers collection.

One test needed to drill deeper to work but it works.

    def test_access(self):
        asteroids = ["asteroid"]
        missiles = ["missile"]
        saucers = ["saucer"]
        saucer_missiles = ["saucer_missile"]
        ships = ["ship"]
        fleets = Fleets(asteroids, missiles, saucers, saucer_missiles, ships)
        assert fleets.asteroids.flyers == asteroids
        assert fleets.missiles.flyers == missiles
        assert fleets.saucers.flyers == saucers
        assert fleets.saucer_missiles.flyers == saucer_missiles
        assert fleets.ships.flyers == ships

Let’s try the game, surely that can’t work. Right: Fleet has no clear. Build it:

class Fleet:
    def clear(self):
        self.flyers.clear()

Fleet does not know extend. I’ll see if this works:

class Fleet:
    def extend(self, *args):
        self.flyers.extend(*args)

And:

 targeting_angle = self.angle_to(ships[0])
                                    ~~~~~^^^
TypeError: 'ShipFleet' object is not subscriptable

Hm. When you go to make a collection you run into stuff. This one calls for this:

class Fleet:
    def __getitem__(self, item):
        return self.flyers[item]

It seems that we are green and the game works. Commit: Fleets returns Fleet instance, not flyer collection via properties. Fleet instances understand subscripting, extend, and len.

Reflection

This is substantially less interesting than I had hoped for, but the tests and running the game have shown me what’s needed. I think I’ll add to the tests. If I had known I needed those things, I’d have done it already.

    def test_len_etc(self):
        saucer_missiles = []
        fleets = Fleets([], [], [], saucer_missiles, [])
        s_m_fleet = fleets.saucer_missiles
        assert len(s_m_fleet) == 0
        s_m_fleet.extend([1, 20, 300])
        assert len(s_m_fleet) == 3
        assert s_m_fleet[1] == 20

This was worth doing, because I learned that extend takes one list, not variable args, so:

    def extend(self, list):
        self.flyers.extend(list)

Green. Commit: added tests for Fleet.extend and subscripting.

Reflection Continued

Still not very interesting but sometimes you just have to prepare for what you really want to do. Let’s review again how saucer firing works now, since our buffers got flushed with all that collection rigmarole.

    def check_saucer_firing(self, delta_time, saucers, saucer_missiles, ships):
        for saucer in saucers:
            saucer.fire_if_possible(delta_time, saucer_missiles, ships)

Let’s see if we can change this one to pass and do the job in SaucerFleet.

class SaucerFleet:
    def tick(self, delta_time, fleets):
        super().tick(delta_time, fleets)
        saucer_missiles = fleets.saucer_missiles
        ships = fleets.ships
        for saucer in self:
            saucer.fire_if_possible(delta_time, saucer_missiles, ships)
        if not self.flyers:
            self.timer.tick(delta_time)
        return True

That works in game, with three tests failing. I’ll remove the check and its call from Game. Now let’s make those tests good.

The first one is just that FakeFlyer doesn’t understand fire_if_possible. Fix that:

class FakeFlyer:
    def fire_if_possible(self, _delta_time, _saucer_missiles, _ships):
        pass

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

Now the code is looking for saucer_missiles in None. Will also be looking for ships, no doubt. I need to pass in a Fleets, I think.

    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

That runs.

The final one is another test for saucer spawning. I must have created one not realizing I already had one. Remove the second one.

We are green. Double-check the game. Game is solid. Commit: revise tests verifying move of saucer firing logic.

Are we there yet?

No, we are not. We should move the logic from here:

class SaucerFleet:
    def tick(self, delta_time, fleets):
        super().tick(delta_time, fleets)
        saucer_missiles = fleets.saucer_missiles
        ships = fleets.ships
        for saucer in self:
            saucer.fire_if_possible(delta_time, saucer_missiles, ships)
        if not self.flyers:
            self.timer.tick(delta_time)
        return True

It should go down into Saucer.tick, so we know that that method wants saucer_missiles and ships (or else fleets). I think all the tick methods expect fleets, so our code should just work, if we move this logic to Saucer.tick:

class Saucer:
    def tick(self, delta_time, fleet, fleets):
        saucer_missiles = fleets.saucer_missiles
        ships = fleets.ships
        self.fire_if_possible(delta_time, saucer_missiles, ships)
        self.move(delta_time, fleet)
        return True

class SaucerFleet:
    def tick(self, delta_time, fleets):
        super().tick(delta_time, fleets)
        if not self.flyers:
            self.timer.tick(delta_time)
        return True

Tests remain green, game continues to work (and the tests should assure me of that but I check anyway). Commit: move saucer firing logic to Saucer.tick.

Summary

I think that went a bit more nicely. It was more than a little boring changing Fleet to be more like a collection, but it was easy to do and worth having done it at least once. After that I just had to turn off the check in Game and move the logic to SaucerFleet, and after the tests were aligned with that, move the logic again to Saucer.

Going in two steps allowed me to be sure that it was going to work before committing to the three-rail shot that it now is, Game to Fleets to Fleet to Saucer.

I feel somewhat redeemed. Let’s print this before I break something.

See you next time!