P-284: Improve Framework?

Python Asteroids+Invaders on GitHub

I’m pleased that Invaders has been implemented without framework changes. And I see what I think is a desirable change. I learn otherwise.

Yesterday we were reviewing PlayerMaker and did some refactoring around these methods:

    def give_player_another_turn(self, fleets):
        fleets.remove(self)
        delay_until_new_player = 2.0
        delay_a_bit_longer = 2.1
        self.provide_new_player(delay_until_new_player, fleets)
        self.provide_new_maker(delay_a_bit_longer, fleets)

    def provide_new_player(self, delay_until_new_player, fleets):
        player_capsule = TimeCapsule(delay_until_new_player, InvaderPlayer(), self.reserve)
        fleets.append(player_capsule)

    def provide_new_maker(self, delay_a_bit_longer, fleets):
        maker_capsule = TimeCapsule(delay_a_bit_longer, PlayerMaker())
        fleets.append(maker_capsule)

The trick here, and frankly I love it, is that the TimeCapsule object sits in the mix until its timer runs down, at which point it adds its second parameter to the mix, and if it has a third one, removes that. In the case in hand, this causes a new player object to appear after two seconds, while one reserve player is removed from the screen.

So the TimeCapsule is a simple but clever way—if I do say so myself—to cause something to happen after a discrete delay. Fine, righteous, elegant solution (in my unbiased opinion)¹.

However, it occurred to me at some time in the recent past that we have an object that is responsible for adding and removing things from the mix: the mix itself, the Fleets object.

class Fleets:
    def append(self, flyer):
        self.flyers.append(flyer)

    def remove(self, flyer):
        # more pythonic?
        try:
            self.flyers.remove(flyer)
        except ValueError:
            pass

So, I said to myself, I said, “Why shouldn’t the Fleets object have a facility to add_this_later? Wouldn’t that be better?”

And, you know what? I think that would be better, so my plan is to provide that facility in Fleets, and use it.

There are (at least) two ways to go with this. I could actually write tests for the new method or methods, as part of the Fleets tests. Or, I could simply program by intention (which I was reliably informed yesterday is also called “programming by wishful thinking”), calling the methods and trusting existing tests to ensure that it all works.

As much as I like the “by intention” idea, I think it’s more prudent to test-drive these methods directly.

It turns out that Fleets doesn’t have many direct tests at all. That’s somewhat OK: it is very simple, and if it doesn’t work, nothing works. Anyway, we’ll add some new ones.

What should we implement? If we’re to be guided by TimeCapsule, we might want a method that adds one thing and deletes another, because that’s how TimeCapsule works. Alternatively, we could just have two methods append_later and remove-Later and waste an additional capsule. Let’s do the basic methods and then see how we like using them.

    def test_add_later(self):
        fleets = Fleets()
        fi = FI(fleets)
        maker = PlayerMaker()
        fleets.append_later(maker, 2.0)

PyCharm already knows this won’t work. I think I’ll add the check and then deal with the method.

    def test_add_later(self):
        fleets = Fleets()
        fi = FI(fleets)
        maker = PlayerMaker()
        fleets.append_later(maker, 2.0)
        assert fi.time_capsules

The FleetsInspector already knows the time_capsules method. Nice. Now:

    def append_later(self, flyer, seconds):
        self.append(TimeCapsule(seconds, added_flyer=flyer))

The test runs. Before we commit, however, there’s an issue. I had to import TimeCapsule into the Fleets object, so that I could refer to it. But TimeCapsule is not part of the core. It is an InvadersFlyer subclass. For this change to be righteous, we need to move TimeCapsule into the core.

Nota Bene

I did not think of this before deciding to set out on this little journey. I was thinking we had a small matter of adding a couple of methods to Fleets. Now we see that we need to add a whole class to the core. Somewhat bigger deal. It won’t be difficult—at least I don’t think it’ll be difficult. If I had a team and was not pairing right now, I would ask for someone to pair with me, and/or ask the team. I have no team. I think we’ll try it.

Here’s the TimeCapsule:

class TimeCapsule(InvadersFlyer):
    def __init__(self, time, added_flyer, removed_flyer=None):
        self.to_add = added_flyer
        self.to_remove = removed_flyer
        self.time = time

    @property
    def mask(self):
        return None

    @property
    def rect(self):
        return None

    def interact_with(self, other, fleets):
        other.interact_with_timecapsule(self, fleets)

    def tick(self, delta_time, fleets):
        self.time -= delta_time
        if self.time <= 0:
            fleets.remove(self)
            fleets.append(self.to_add)
            if self.to_remove:
                fleets.remove(self.to_remove)

I see two things. One: we can try making it inherit from Flyer. If that goes well, we’re good. Two: we’ll be able to remove the useless mask and rect properties that InvaderFlyers require. Three²: we’ll need to do something about interact_with_timecapsule.

I believe that no one actually uses interact_with_timecapsule. Quick check. I am mistaken! More bad news. What are those three implementors doing?

Good news! They are just saying pass. Recall that we decided not to require InvadersFlyer subclasses to implement all the interact_with_x methods because it was cluttering and because Hill hates implementation inheritance and would therefore be ticked off. So I’ve removed all the empty implementations of that method, and a bunch of others as well. Let’s commit those three non-users.

Arrgh. I have broken something. And I committed it before I realized. And pushed it. Something went wrong in ShotController: I must have removed too much. Right. I quickly find it in history and put back the two methods that had contents, removed in a fit of excess. Green. Commit again.

OK. Better take a break. Made a mistake and then made myself nervous through having made the mistake and committing and pushing it to prod.

Reflection

OK, slow down just a bit. We’re going to try making TimeCapsule inherit from Flyer, and given that that works, we’ll see about moving it over to the core.

And reviewing that idea, we find another issue. As things stand, Fleets does not know Flyer. Now that is a bit off, although in a duck-typing world it works. But many of Fleets’ methods accept a parameter flyer and in a more strict language, you’d need at least an interface to refer to.

And right now, Flyer and its subclasses are not in the core, they are at the top of the file hierarchy. But that’s more a matter of poor packaging on my part, not a matter of thoughtful decision-making. I have not had to worry much about packaging here in my living room³, so I think we’ll be OK here.

Let’s just re-root TimeCapsule and see how things go.

Not as well as one might hope. Because we’ve re-rooted TimeCapsule, it is now subject to receiving all the interact_with_X methods in both Invaders and, if we ever used it there, Asteroids or any other game.

For now, we cannot re-root TimeCapsule. However, we are green. So we can, if we wish, go ahead with adding the append_delayed and other methods to Fleets. But should we? I suspect now. Let’s reflect.

Reflection

Currently, the Fleets object does know some things about the objects it manages. It knows that they respond to the calls in its cycle method:

    def cycle(self, delta_time, screen):
        self.update(delta_time)
        self.perform_interactions()
        self.tick(delta_time)
        self.draw(screen)

Furthermore, the core includes Interactor, Fleets’ helper object. Between Fleets and Interactor, the core knows that all the objects must respond to begin_interactions, interact_with, and end_interactions.

However, we know that objects in different games will implement interact_with differently, specifically referring to objects unique to that game. Right now, interact_with_timecapsule is such a method.

So. It seems to me that it is really not legitimate for Fleets to add objects of its own into the mix, because, if it does, some later game might send interact_with_surprise to one of Fleets’ objects, and that would be bad. And because our double-dispatch interact_with is at the center of how the design works, I don’t see an easy way around this.

I conclude that we may have a good idea here, but we do not have a viable idea for implementing it.

• It might be a useful notion for Fleets to have an “add or remove later” facility: we have some identified need for it.

• Fleets probably should not implement that facility by adding things to the mix. If it is to do it, it should keep some kind of a timing list internally.

Therefore: if we do want this facility, we’ll need to undertake an extension to Fleets, probably adding an explicit timing list that will be checked on each cycle. That sounds to me like about one session’s worth of work. If we do that work, we can eliminate three or four uses of TimeCapsule, and remove the class.

Discovery

In counting how many users of TimeCapsule there are, I encountered this one:

coin.py
def invaders(fleets):
    fleets.clear()
    left_bumper = u.BUMPER_LEFT
    fleets.append(Bumper(left_bumper, -1))
    fleets.append(Bumper(u.BUMPER_RIGHT, +1))
    fleets.append(TopBumper())
    fleets.append(InvaderFleet())
    fleets.append(PlayerMaker())
    fleets.append(ShotController())
    fleets.append(InvaderScoreKeeper())
    fleets.append(RoadFurniture.bottom_line())
    fleets.append(TimeCapsule(10, InvadersSaucer()))
    for i in range(3):
        fleets.append(ReservePlayer(i))
    half_width = 88 / 2
    spacing = 198
    step = 180
    for i in range(4):
        place = Vector2(half_width + spacing + i * step, u.SHIELD_Y)
        fleets.append(RoadFurniture.shield(place))

Coin knows a lot about the game classes:

from asteroids.game_over import GameOver
from asteroids.saucermaker import SaucerMaker
from asteroids.scorekeeper import ScoreKeeper
from asteroids.shipmaker import ShipMaker
from asteroids.thumper import Thumper
from asteroids.wavemaker import WaveMaker
from invaders.bumper import Bumper
from invaders.invader_score import InvaderScoreKeeper
from invaders.invaderfleet import InvaderFleet
from invaders.invaders_saucer import InvadersSaucer
from invaders.playermaker import PlayerMaker
from invaders.reserveplayer import ReservePlayer
from invaders.roadfurniture import RoadFurniture
from invaders.shotcontroller import ShotController
from invaders.timecapsule import TimeCapsule
from invaders.top_bumper import TopBumper

That’s natural, though: it has to know how to set up a game. We could do something more indirect, perhaps, but so far we have not accommodated that kind of flexibility. Furthermore, coin.py is in the core, and given what it knows, probably shouldn’t be.

One Conclusion

If we were really going to extract a game framework from what we have here, there is packaging work that would need to be done.

Two Conclusion

For now, this idea has to wait. It would be easy enough to make it work, but doing it in the simple way I was contemplating does not seem to be possible.

Red Conclusion

It would be possible to add concrete methods to InvaderFlyer that perform the delayed append and remove. Aside from the inheritance of concrete methods, that would be as easy as what we were trying over in Fleets.

Blue Conclusion⁴

For now, we’ll roll back the new test and code in Fleets. Decent idea, didn’t pan out.

Summary

We have some good conclusions here. Along the way, I cleaned out a number of pass methods that were lying around. (And, unfortunately, cleaned up two that should have been left in. Quickly discovered and fixed.)

Could I have figured out these conclusions without all this unnecessary coding? Possibly. I certainly could have decided not to do it. No code needed for that. But along the way, driven by seeing more clearly what would really have to happen, I’ve learned a bit about what would be needed to do this job, and to make the little system we have more nearly general enough to be packaged up as a general purpose gaming framework.

I don’t mind spending a little time learning. And letting the code participate in my learning seems to help me—and the code.

Oh. By the way. If you read these things, I’d appreciate a toot or something, but not tweet, I don’t go there any more, just to let me know there’s someone out there. I’d write the articles anyway, but it would be nice if they’re being read. And, if you’d like to see me improve what I do, let me know about that too.