P-236 - A Better Way?

Python Asteroids+Invaders on GitHub

My timing defects have caused some thinking, some of it mine. There might be a better way. (This is long, because there’s a lot of design thinking. Then some rather interesting doing.)

Tomas Aschan and I had an exchange, regarding the “timing” defects that have arisen due to Flyers changing important state during interactions:

Tomas:
My remedy would be to collect info in the interact phase, but not actually act on it until end_interactions. I.e., the world is “static” when objects look at each other, and then all objects evolve independently depending on what they’ve seen.

Ron:
yes, that is arguably better, with the significant drawback that all the objects kind of become two-phase, generally more stateful. Ideally, yes they’d be unchanged until end. I’d hope for a standard mechanism to buffer and trigger changes. Might be amusing to try …

As I see it, the issues include:

• The fundamental reason behind begin_interactions and end_interactions “events” is to permit the receiving Flyer object to set up a state in begin, evaluate events during interact_with, possibly changing that state, and then take some action at end.

• We do allow certain actions during interact_with, specifically adding or removing objects from the Fleets instance. This is typically used to add something new, a Score or Explosion, and to remove oneself, typically upon destruction by collision.

• Ideally, a Flyer would only change state during update, its opportunity to move itself and perform other cyclical actions.

• Arguably, changing state during the interaction is always a problem, as is any change to an object that is out of time phase with other changes. Ideally, an object changes only with one rhythm.¹

We have some Flyers who do things quite neatly:

Scorekeeper interacts with Score. It adds the score to its accumulator. That’s all it does during interactions. That’s the only place where it changes state. That’s good.

We have some who are more concerning:

ShipMaker sets the assumption, in begin_interactions, that there are no ships. If it sees a ship during interactions, it sets the assumption to oh, right, there are ships after all. At end_interactions, if the initial assumption is true, there are no ships, and it begins the operation of making a new one.

So ShipMaker is a bit more intricate but at least it only changes during the begin - interact - end cycle.

And we have some who are even more concerning, because they do both.

The InvaderShot was a perfect example. During update, it moves along, plunging inexorably down, down, falling upon the player like a hellish rain. And, during the begin - interact - end cycle, it observed that it was colliding with a player shot, cutting short its rain of terror². Unfortunately, the InvaderShot changed state in such a way as to confuse the up and coming player shot, which thus avoided its rightful almost inevitable destruction.

The InvaderShot changes state at two different times, in two different rhythms, and that’s not so good.

You may be thinking, well, its not really a different rhythm, it’s still inside a single game cycle, which encompasses update, begin interactions, interact with, end interactions, tick, and draw. Yes, but no. I argue that interact_with occurs many times. It’s an inner loop, while update, begin, end, tick, and draw are outer. So even if you only respond once, you’re on a different time line.

This would be only a nicety were it not for the fact that we’ve seen it cause a failure.

Therefore

I think it would be a good rule to say that an object should never change its externally visible state during the interact part of the begin - interact - end cycle. What do we mean by “externally visible”? Well, it’s vague, isn’t it, but it’s something like “any information that another interacting object might rely on, like your position you tiny fool!”

A better rule would be “never change state during interact”; only change state during the events that occur once per event cycle. (Basically all the others, update, begin, end, tick, draw.)

But the whole point of the cycle is to do things during interact. Initially, all one did was remove oneself from fleets, or add something new to fleets, like a tasty explosion or score. I think it was only later that I realized that everything we needed to do could be done using the full cycle, leading to the current decentralized design we have. And I do like that design.

Dilemma

It seems clear that changing state inside interact is a potential problem, and that if the object also changes state in any of the outer methods, there is likely a cohesion issue, a rhythm issue, that’s at least a concerning tension in the code if not an actual problem.

But changing state inside interact is what allows objects like ScoreKeeper and ShipMaker to work at all. So we have two competing tensions, the need on the one hand and the possible reduction of cohesion on the other, to deal with.

How do we resolve that kind of tension, class? That’s right! Typically we resolve it with a new object, dividing the responsibilities between the existing object and the new one.

Emerging Resolution

So it begins to seem that the begin - interact - end cycle is best used to make an assumption in (or before) begin, validate it or invalidate it during interact, and take action during end. And for many objects, if not all of them, we’d prefer to do that without changing state, or at least without changing externally visible state.

There is a related pattern, Momento, that comes to mind. With Memento, we record our internal state in a Momento object, which can be used later to restore our state. Momento pattern is often used as part of “undo” logic, where the outer object gathers up Momento objects before changes, and keeps them to use when restoring state if it needs to.

There are other related ideas, Future and Promise, which are typically values that promise to return an answer if you ask for it, but that do not necessarily compute the answer until you ask.

These are all examples of what we might call “deferred action”. In object terms, they are objects that will change the state of the original object, or compute some value, or take some action, only if later evaluated.

Another notion might be that of a “callback”. Sometimes an object provides a “callback”, which might be a lambda or bound method, that can be executed later.

Taking Shape

So, we begin to think:

What if our Flyers could send themselves a reminder to do something in end_interactions, rather than doing it during interact_with?

If they could do that, then the rhythm is not broken. They can update at one of the events that occurs only once per cycle, rather than then and also at the many times per cycle rhythm of interact_with.

We begin to see an idea. But suppose we did have a callback or Future or Momento. (A Momento of the future? Remember that time next year when we won the lottery? I have written down the numbers here …) Who could hold on to it for us?

Becoming Clearer

The object itself could do it. There could be a hidden variable that we use, by convention, to tuck away these reminders. This would be much the same as we do now.

Shield

The Shield fix essentially does this. The bug was that it changed its mask during interact_with, and the mask is externally visible by design. The fix was that it caches the updated copy of the mask and only applies it in end_interactions, after all the excitement has subsided.

If we were to “formalize” a way of doing this in the Flyers, it would probably be better than the ad hoc catch as catch can willy nilly way we do it now.

But another possibility is to add this responsibility to the Fleets object. Fleets already protects users of the begin - interact - end cycle, to the extent that it guarantees that any objects removed or added to the mix will not affect the current interaction cycle. We could extend Fleets to have a “mail to the future” kind of capability, probably just a call-back, that would be made available or sent sometime around the end-interactions event.

Further Musing

How might it work? Well, suppose we could pass an object to Fleets, giving it our own object ID and anything else we wanted to provide. Something like fleets.remind_me(self, anyReminder, and then at any future time, by recommendation during end_interactions, we could say fleets.get_reminders(self), and get all our reminders back. Fleets would keep a dictionary by object id, which it would automatically clear before begin and / or after end, so that it was only valid for one cycle.

Ideally, we might want to say that the reminders would be provided in the order received, just in case the caller might want to say “no ships”, “yes ships”, although in that case saying nothing and properly interpreting “yes ships” would do the job. We’ll save that rule idea in the event that we need it. Sequencing your reminders would probably be too clever an idea anyway.

Converging?

Providing the facility in Fleets seems better to me than a mere convention in Flyer, and better than putting a delegate of some kind into every Flyer.

Try it

I’m going to TDD the feature into Fleets. This is the first change to the core objects since starting Invaders, but this is emphatically not a feature just for Invaders. This feature is needed just as much by Asteroids, if not more, because of its ShipMaker, WaveMaker, ScoreKeeper and so on.

We don’t have many direct tests on Fleets, but we do exercise it a lot with all our various tests that check object interactions. But we do have a test_fleets file where we can put these new tests.

Grr

I was hoping to do my test with lambda but Python lambda expressions are expressions, so I can’t store inside one³. So here’s my first cut at a test:

    def test_reminders(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.set_true)
        assert not obj.reminded
        for method in fleets.get_reminders(obj):
            method()
        assert obj.reminded

I created a little class just for testing:

class Remindable:
    def __init__(self):
        self.reminded = False

    def set_true(self):
        self.reminded = True

I build the two methods into Fleets:

    def remind_me(self, sender, reminder):
        try:
            self.reminders[sender].append(reminder)
        except KeyError:
            self.reminders[sender] = [reminder]

    def get_reminders(self, sender):
        try:
            return self.reminders[sender]
        except KeyError:
            return []

I’ve initialized reminders in __init__ and in begin_interactions. I went a bit beyond the test, but I think it’s best to do the thing when I think of the thing. Memory is unreliable.⁴

Let’s do some more tests. We want to have more than one reminder. I think we’d like to have some reminders with parameters too. Let’s see if we can work up to that.

    def test_two_reminders(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.set_true)
        fleets.remind_me(obj, obj.set_value)
        assert not obj.reminded
        assert obj.value == 37
        for method in fleets.get_reminders(obj):
            method()
        assert obj.reminded
        assert obj.value == 42

With the addition to Remindable:

class Remindable:
    def __init__(self):
        self.reminded = False
        self.value = 37

    def set_true(self):
        self.reminded = True

    def set_value(self):
        self.value = 42

But we’d like to pass arguments to our methods via reminders. A test for that:

class Remindable:
    def __init__(self):
        self.reminded = False
        self.value = 37

    def set_true(self):
        self.reminded = True

    def set_value(self, value=42):
        self.value = value

Now set_value takes a parameter that defaults to 42. The old test runs. The new test does not:

    def test_parameter(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.set_value, 666)
        assert obj.value == 37
        for method in fleets.get_reminders(obj):
            method()
        assert obj.value == 666

We need remind_me to accept a variable number of arguments after the first one, which is required.

It turns out that I’m not sure just how to do this. Also I wish I had committed. I set my new test to skip and try some things.

    def test_reminder_form(self):
        obj = Remindable()
        reminder = [obj.set_value, 666]
        func = reminder[0]
        arg = reminder[1]
        func(arg)
        assert obj.value == 666

Here I’m just testing that I can put a function and an argument in a list and call it properly. I’m not sure how I’m going to deal with the argument list, and it’s fancy enough that I want to sneak up on it.

I want to try two arguments. Let’s have a new method on Remindable.

class Remindable:
    def __init__(self):
        self.reminded = False
        self.value = 37
        self.compared = False

    def compare(self, a, b):
        self.compared = a == b

And try it:

    def test_two_parameters(self):
        obj = Remindable()
        assert not obj.compared
        reminder = [obj.compare, [666, 333+333]]
        func = reminder[0]
        args = reminder[1]
        func(*args)
        assert obj.compared

This tells me, I think, how to do this. It also raises a question: should all reminders be functions, in which case should we call them from Fleets, or should they be arbitrary objects such that we should just return them? We’re working on the latter form now.

    def test_parameter(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.compare, 666, 333+333)
        assert obj.value == 37
        for reminder in fleets.get_reminders(obj):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        assert obj.compared

I think this is what I want, at least for now. Once I get the parameters working, I’ll reflect.

    def remind_me(self, sender, reminder, *args):
        reminder = [reminder, args]
        print(reminder)
        try:
            self.reminders[sender].append(reminder)
        except KeyError:
            self.reminders[sender] = [reminder]

    def get_reminders(self, sender):
        try:
            return self.reminders[sender]
        except KeyError:
            return []

OK, now we expect more arguments and pack them up, as a list, in the small reminder list, and put that into the list of all reminders.

And the tests, once I get them all used to a list coming back, all run. And they are not attractive:

    def test_parameters(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.compare, 666, 333+333)
        assert obj.value == 37
        for reminder in fleets.get_reminders(obj):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        assert obj.compared

If we left it like this, then all the would-be users of this thing would have to unpack and decode the list that comes back. Instead, let’s have it be that the Flyers provide a callback and parameters to the fleets, as my examples are doing anyway, and the fleets calls all the reminders when we ask it to.

I’ll change my tests:

    def test_reminders(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.set_true)
        assert not obj.reminded
        fleets.execute_reminders(obj)
        assert obj.reminded

    def test_two_reminders(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.set_true)
        fleets.remind_me(obj, obj.set_value)
        assert not obj.reminded
        assert obj.value == 37
        fleets.execute_reminders(obj)
        assert obj.reminded
        assert obj.value == 42

    def test_parameters(self):
        fleets = Fleets()
        obj = Remindable()
        fleets.remind_me(obj, obj.compare, 666, 333+333)
        assert obj.value == 37
        fleets.execute_reminders(obj)
        assert obj.compared

And fleets:

class Fleets:
    def execute_reminders(self, sender):
        for reminder in self.get_reminders(sender):
            func = reminder[0]
            args = reminder[1]
            func(*args)

We are green. I’m not sure I love the terms remind_me and execute_reminders, but they’ll do for now.

I think we can safely commit this. It works, I feel that it works pretty much as we would wish, and in any case it is not used. Commit: initial tests and implementation of Fleets.reminders.

I think we could do better, perhaps marking the second parameter of remind_me as a Callable.

    def remind_me(self, sender, reminder: Callable, *args):
        reminder = [reminder, args]
        print(reminder)
        try:
            self.reminders[sender].append(reminder)
        except KeyError:
            self.reminders[sender] = [reminder]

PyCharm might make use of that annotation. We’ll see.

Now let’s see how we might use this thing. We have a possibility in PlayerShot, which uses end_interactions and a should_die flag. Let’s see if the reminder does it better.

class PlayerShot(InvadersFlyer):
    def __init__(self, position=u.CENTER):
        offset = Vector2(2, -8*4)
        self.velocity = Vector2(0, -4*4)
        maker = BitmapMaker.instance()
        self.bits = maker.player_shot
        self._mask = pygame.mask.from_surface(self.bits)
        self._rect = self.bits.get_rect()
        self.position = position + offset
        self.should_die = False

    def begin_interactions(self, fleets):
        self.should_die = False

    def hit_invader(self):
        self.should_die = True

    def end_interactions(self, fleets):
        if self.should_die:
            fleets.remove(self)

This one is a bit odd, because hit_invader is sent from InvaderFleet, because Invaders are not flyers and don’t interact normally.

class Invader:
    def interact_with_group_and_playershot(self, shot, group, fleets):
        if self.colliding(shot):
            shot.hit_invader()
            group.kill(self)
            fleets.append(InvaderExplosion(self.position))

I wish I had started with a more direct case but this isn’t too challenging. First, though, we need to pass fleets to the hit_invader method. We probably already should have. Ah. But, if we do … we can just do this:

    def begin_interactions(self, fleets):
        pass

    def hit_invader(self, fleets):
        fleets.remove(self)

    def end_interactions(self, fleets):
        pass

I’m glad we had this little chat. That’s a better way to do that anyway. Commit: improve handling of PlayerShot.hit_invader.

Do I have no reason to use my wonderful new feature?

The ShotController timing issue was due to overloading the meaning of InvaderShot position with a special value that meant not available. We don’t do that any more. The shot has a flag saying whether it is available. Let’s see how we manage that flag. When we’re told to fire, we set the flag:

class InvaderShot(InvadersFlyer):
    def fire_from(self, position, fleets):
        self._available = False
        self.position = position
        fleets.append(self)

That’s called in the outer cycle, from ShotController. But the reset is done here:

class InvaderShot(InvadersFlyer):
    def die(self, fleets):
        self._available = True
        fleets.remove(self)

And that is called from more than one place:

class InvaderShot(InvadersFlyer):
    def interact_with_invaderplayer(self, player, fleets):
        self.die_on_collision(player, fleets)

    def interact_with_playershot(self, shot, fleets):
        self.die_on_collision(shot, fleets)

    def interact_with_shield(self, shield, fleets):
        self.die_on_collision(shield, fleets)

    def die_on_collision(self, flyer, fleets):
        if self.colliding(flyer):
            self.die(fleets)

Those are candidates for a reminder. But there is another caller:

class InvaderShot(InvadersFlyer):
    def move(self, fleets):
        self.moves += 1
        self.update_map()
        self.position = self.position + Vector2(0, 16)
        if self.position.y >= u.SCREEN_SIZE:
            self.die(fleets)

Well, arguably that’s legit since it is in the outer cycle.

Let’s do it. We’ll fix die on collision not to make the change then and there, but instead to do it on end_interaction, using a reminder.

    def die_on_collision(self, flyer, fleets):
        if self.colliding(flyer):
            fleets.remind_me(self, self.die, fleets)

Also:

    def end_interactions(self, fleets):
        fleets.execute_reminders(self)

Tests are failing but I bet they don’t call end_interactions.

    def test_dies_on_shield(self):
        fleets = Fleets()
        fi = FI(fleets)
        shield = Shield(Vector2(100, 100))
        maker = BitmapMaker.instance()
        shot = InvaderShot(Vector2(100, 100), maker.rollers)
        assert shot.colliding(shield)
        fleets.append(shot)
        assert fi.invader_shots
        shot.interact_with_shield(shield, fleets)
        fleets.execute_reminders(shot)
        assert not fi.invader_shots

Adding the execute_reminders call does the job. I wonder … should we just always execute the reminders rather than requiring a call? I bet we should.

Make the other test run. Same fix. Green.

I think we want the rule to be that if an object files reminders, they will be called before? after? calling its end_interactions method.

Before, because the end should be, well, the end. And, since we’re calling them, we’ll clear them so you don’t call them twice.

Back to the Fleets tests.

    def test_fleets_interaction_cycle(self):
        obj = Remindable()
        fleets = Fleets()
        fleets. append(obj)
        fleets.append(Remindable())
        fleets.begin_interactions()
        fleets.perform_interactions()
        fleets.end_interactions()
        assert obj.compared

This was a bit more tricky than I wanted, because there have to be at least two objects in Fleets for the interactions to run, and so I was confused for a while. We now have this:

class Remindable:
    def __init__(self):
        self.reminded = False
        self.value = 37
        self.compared = False

    def set_true(self):
        self.reminded = True

    def set_value(self, value=42):
        self.value = value

    def compare(self, a, b):
        self.compared = a == b

    def begin_interactions(self, fleets):
        pass

    def end_interactions(self, fleets):
        pass

    def interact_with(self, other, fleets):
        fleets.remind_me(self, self.compare, 96, 96)

That object does a reminder now on interact_with, critical to the test. And in Fleets:

    def begin_interactions(self):
        self.reminders = {}
        for flyer in self.all_objects:
            flyer.begin_interactions(self)

    def end_interactions(self):
        for flyer in self.all_objects:
            self.execute_reminders(flyer)
            flyer.end_interactions(self)

    def remind_me(self, sender, reminder: Callable, *args):
        reminder = [reminder, args]
        try:
            self.reminders[sender].append(reminder)
        except KeyError:
            self.reminders[sender] = [reminder]

    def execute_reminders(self, sender):
        for reminder in self.get_reminders(sender):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        self.reminders[sender] = []

    def get_reminders(self, sender):
        try:
            reminder = self.reminders[sender]
            return reminder
        except KeyError:
            return []

Rather more intricate than I might prefer, but it’s early days. Now back in our user object:

class InvaderShot(InvadersFlyer):

    def die_on_collision(self, flyer, fleets):
        if self.colliding(flyer):
            fleets.remind_me(self, self.die, fleets)

    def die(self, fleets):
        self._available = True
        fleets.remove(self)

We no longer have an end_interactions, because we don’t have to ask for our callbacks, we automatically get them. This should now work in the game. I do not think we have sufficient tests for this. Early days, we’ll work on it.

It does work fine, all the various ways of having InvaderShots get killed work. Let’s reflect.

Reflection

We have a new facility in Fleets, remind_me, which takes at least two parameters, the object wanting to be reminded and a callable. Additional optional arguments will be passed to the callable when it is executed.

At the time of end_interactions, Fleets will call back any reminders for each object before sending it end_interactions.

Reminders are reset for each remindee after being called. (That means that our resetting of the reminders dictionary in Fleets.begin_interactions is unnecessary. I think I’ll leave it.)

Looking at this, I don’t quite like it:

    def remind_me(self, sender, reminder: Callable, *args):
        reminder = [reminder, args]
        try:
            self.reminders[sender].append(reminder)
        except KeyError:
            self.reminders[sender] = [reminder]

    def execute_reminders(self, sender):
        for reminder in self.get_reminders(sender):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        self.reminders[sender] = []

I think it would be better if this were more symmetric, this way:

    def remind_me(self, sender, reminder: Callable, *args):
        reminder = [reminder, args]
        try:
            self.reminders[sender]
        except KeyError:
            self.reminders[sender] = []
        self.reminders[sender].append(reminder)

    def execute_reminders(self, sender):
        for reminder in self.get_reminders(sender):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        self.reminders[sender] = []

Why do I prefer that? Because in both cases, I “start” reminders at an empty list. But you know what? It’d be better to remove the key entirely. The object in question is being held on to. Eeek, we could be building up infinite garbage. I’m glad I’m resetting the dictionary. I think we should remove the key.

    def execute_reminders(self, sender):
        for reminder in self.get_reminders(sender):
            func = reminder[0]
            args = reminder[1]
            func(*args)
        try:
            del self.reminders[sender]
        except KeyError:
            pass

I’m starting to want a dictionary with a default. We could build one. Python probably already has one. For now, we have lots of safety here.

Commit again: use del to be sure Fleets doesn’t hold on to reminder users.

This article has become quite long. Let’s sum up.

Summary

Little Things

At the detail level, the little class Remindable is rather nice. It was just created for testing, but was very useful in driving out the shape of the reminders. It takes advantage of Python’s duck typing, in that it can survive inside Fleets even though it does not inherit from Flyer.

Speaking of little objects, we have that odd structure inside the reminders dictionary, a list of lists of callables with arguments. There probably should be a small class whose instances we’d keep in that list. I think we’ll have occasion to think about that in an upcoming session.

Bigger Things

I think we have an interesting solution in place to the desire to avoid changing object state inside the interaction cycle. Yes, we can do it if we’re careful not to change aspects that others look at, but that requires a level of care that we might sometimes not attain.

Instead, we have a new pattern of implementation that we are planning to use from here on out, and perhaps to retrofit when we maintain things. During interactions, an object can say remind_me to fleets, and just prior to end_interactions, fleets will call it back on each of the reminders it holds for that object.

(I think it’s likely that this scheme means that end_interactions may be unnecessary. Almost no one uses it even now, and the callbacks might cover those cases.)

Now there’s no question in my mind that a callback is more mysterious than, say, setting a flag earlier and checking it later, but the flag-setting can get tricky, and the fact that it can be done in various ways means that it will be done in various ways, and programmers will be faced with those various ways, and will have to figure out each case and sooner or later, some programmer, probably this one, is going to misunderstand the perfectly sensible flags and break something.

Instead, let’s have a standard way to request future action, and then use it whenever we want to defer action to the future. I think it’ll be better. We’ll see.

Needing deferred action seem like a rare situation, which does call the idea into question. But it has been used in practice now, once, and works nicely. Would it be useful in objects like ShipMaker or WaveMaker? I suspect that there are other places where it could be useful.⁵

It’s a bit like finalize, isn’t it? Must think about that.

Could it be simpler? I think possibly it might. Now that the actions are called from Fleets, we might be able to do without the dictionary and keep the Callables in a simple list. They’re all bound methods anyway, so they know their receiver. That’ll be worth trying, but at 700 lines of article input and five elapsed hours (with some time out to put air in my tires) I’m done for now.

Overall, I think this is a righteous idea with a decent first implementation. I look forward to hearing from my colleagues and friends who may agree or have a better idea.

Perhaps the most interesting thing about this whole exercise is all the other things that it brought to mind, such as Momento, Future, Promise, deferred action, and the peek we get at new possibilities, such as the chance that Fleets and Interactor might be simplified if we standardized on this scheme.

And I really must look up some work on Promises. These callbacks are kind of like promises to oneself. Hmm. Fascinating!

New ideas are so refreshing, and sometimes even useful!

See you next time!