The spikes go up and down in the view. They need to react to a lever, and should also Deal Damage to Dot. We hit a snag, but do not panic.

The idea with the lever goes like this: The Lever has four positions, starts in position 0. For each of the four positions, a lever connected to Spikes should put the spikes in these conditions: 0: cycle up and down, deal damage as directed; 1: stop in the up position, deal damage; 2: stop in the down position, no damage; 3: cycle up and down, deal damage.

It is possible that we will want the four conditions to be randomized, so that the player can’t just figure out spike levers once and for all. Save that idea for later.

This should be fairly easy to code in a procedural way, and we’ll see how that looks before deciding on anything more clever. It would be good if we could test this behavior. As things stand, Spikes have no tests. Here’s the code:

class Animated(Content):
    @classmethod
    def spikes(cls, name):
        resource1 = '/Users/ron/Desktop/DungeonTiles/png/objects/trap/1.png'
        resource2 = '/Users/ron/Desktop/DungeonTiles/png/objects/trap/2.png'
        resources = [resource1, resource2]
        return cls(name, resources)

    def __init__(self, name, resources):
        self.resources = resources
        self.name = name
        self.time = 0
        self.scale = 0.75
        self.state = 0

    def interact_with_player(self, dungeon):
        pass

    def run(self, dungeon):
        self.state = 0
        self.time = 0
        def callback(event, delta_time):
            print(delta_time)
            self.time += delta_time
            if self.time >= 1:
                self.time = 0
                self.state = (self.state+1)%len(self.resources)
                dungeon.publish('state_number', self.name, self, self.state)

        dungeon.subscribe('on_update', 'view', callback)

I am immediately angry at myself for calling this AnimatedContent. It was almost certainly too soon to assume that this was going to be reusable code. In my defense, I had DungeonControl to copy-paste from. Not a very strong defense, really.

I am Very Tempted to create subclasses here. The only thing holding me back is that people I respect assure me that inheriting from a concrete class is a bad thing to do and that other approaches, such as pluggable methods, Method Object, and Strategy are better. That’s as may be, but subclassing is really pretty nice, done in moderation.

What we’ll do, or try to do, is to build in the control and damage logic so that we can configure other Animated content to work similarly but differently as required. If that turns out to be feasless, we’ll try something else.

Let’s have a test and see what breaks. I think we’ll have a new test file for spikes.

class TestSpikes:
    def test_hookup(self):
        assert False

The initial test as specified in the prophecies fails as expected. Our test file has been found and we can write some real tests.

I’m having difficulty devising a simple test. The spikes (Animated) instance only subscribes to the on_update event. That’s fine, I was thinking we could ask PubSub whether it has a subscription for our object. But PubSub only holds on to callbacks, not to the sender of the subscription, which PubSub does not know: it just calls functions from its list.

I can ask PubSub for the subscriptions for an event and count them. I can’t see a way to see whether an individual subscription has a callback aimed at my spikes. This test at least runs:

class TestSpikes:
    def test_subscribe(self):
        pub_sub = PubSub()
        spikes = Animated.spikes('spikes')
        spikes.run(pub_sub)
        subs = pub_sub.subscriptions[('on_update', 'view')]
        assert len(subs) == 1

OK, we are confident that the spikes subscribe to ‘on_update’, because we can see it its code that it does and we have a subscription in our private PubSub.

We want also to subscribe to messages from a control, of the same name as our spikes. The control publishes itself and state number with the event ‘state_number’.

Let’s require our spikes to subscribe to ‘state_number’, under its own name, and then make it so.

    def test_subscribes_to_control(self):
        pub_sub = PubSub()
        spikes = Animated.spikes('spikes')
        spikes.run(pub_sub)
        subs = pub_sub.subscriptions[('state_number', 'spikes')]
        assert len(subs) == 1

This fails with a KeyError, because PubSub doesn’t like to be asked for things it doesn’t have, and it uses try/except to ask forgiveness rather than permission. Perhaps it should do otherwise but that’s not our current mission.

Make the spikes subscribe to controls with the same name:

    def run(self, dungeon):
        self.state = 0
        self.time = 0
        def callback(event, delta_time):
            print(delta_time)
            self.time += delta_time
            if self.time >= 1:
                self.time = 0
                self.state = (self.state+1)%len(self.resources)
                dungeon.publish('state_number', self.name, self, self.state)

        dungeon.subscribe('on_update', 'view', callback)
        def control_changed(event, modifier, control, state_number):
            print(f'control_state {state_number}')
            raise NotImplementedError

        dungeon.subscribe('state_number', self.name, control_changed)

The test passes. I want to see the callback called, so extend the test:

    def test_subscribes_to_control(self):
        pub_sub = PubSub()
        spikes = Animated.spikes('spikes')
        spikes.run(pub_sub)
        subs = pub_sub.subscriptions[('state_number', 'spikes')]
        assert len(subs) == 1
        pub_sub.publish('state_number', 'spikes', self, 666)

This fails as expected, printing 666 and then raising NotImplementedError.

However. It took me about seventy-three tries to get the calling sequences straight. They are checked only at run time and really only know if you have the right number of parameters. We’ll want to do something about that, though I’m not sure what we can do.

Possibly we should use only named parameters in PubSub, but I’m not even sure that would help.

Let’s remove the check from the last line of the test, and we can then commit: Animated (spikes) is learning how to connect to controls. in process.

Now we can address the PubSub issue. Part of the problem is that it passes the event string on every callback:

class PubSub:
    def publish(self, event, caller_id, *args, **kwargs):
        specific = (event, caller_id)
        self.execute_callbacks(specific, event, args, kwargs)
        general = (event, '')
        self.execute_callbacks(general, event, args, kwargs)

    def execute_callbacks(self, key, event, args, kwargs):
        try:
            subscriptions = list(self.subscriptions[key])
            for subscription in subscriptions:
                subscription.callback(event, *args, **kwargs)
                if subscription.one_shot:
                    self.subscriptions[key].remove(subscription)
        except KeyError:
            pass

This will be a breaking change, but let’s stop doing that. We can certainly change execute_callbacks but we’ll have to search out all the subscribes and change their callbacks not to expect the event.

Let’s do that first, just to make sure no one looks at the event. No … every time I change one, a test breaks. We’ll do it in the other order and let the tests help.

    def execute_callbacks(self, key, args, kwargs):
        try:
            subscriptions = list(self.subscriptions[key])
            for subscription in subscriptions:
                subscription.callback(*args, **kwargs)
                if subscription.one_shot:
                    self.subscriptions[key].remove(subscription)
        except KeyError:
            pass

Four tests break, fix those. If anything other than removing event from the parameters is needed, I’ll mention it. Otherwise I’ll spare you the tedium.

I need to check more cases, however, so I’ll search for all subscribes and check their callbacks.

I found them all, fixed them, and tested extensively in the game and all is well. Commit: Animated (spikes) listens to ‘state_number’. Unfortunately, it hears its own state changing.

At least hearing its own state doesn’t cause it to send a ‘state_number’, at least not yet, but that would have come soon, resulting, I suspect, in a horrendous crash of unprecedented proportions.

I think that we’ll need to change the rules to separate the ‘state_number’ publication that means “I’m changing my own state update my view”, from the publication that means “I’m a control and if I control you, here is what you need to know.”

Whether the issue is deeper than that, I’m not sure, but I think that change will suffice. If that’s the case, the lever control would publish two events where here it publishes just one:

class DungeonControl:
    def interact_with_player(self, dungeon):
        self.state += 1
        if self.state >= len(self.resources):
            self.state = 0
        dungeon.publish('state_number', self.name, self, self.state)

Possibly the event should publish something other than n integer, such as a string or enum or some such more symbolic notion.

This will require reflection: we have hit a design issue and I don’t think we should just bull ahead without time to reflect and discuss with our team or rubber duck as the case may be.

Summary

We got a couple of tests in place for the Animated content item. We observe that it was quite probably premature to generalize it from Spikes to Animated. We have a connection working between the control and the spikes, and have observed that the event agreed upon between control and controlled item needs not to be the same event as is used to update the view, since our controlled object needs to distinguish the cases.

And we simplified the PubSub protocol to leave out the event name. This was a breaking change, the way we did it, and forced a “big” refactoring. Was there a non-breaking way? Unquestionably. We could have provided a second subscribe method whose callbacks did not get the event, and distinguished the case in the Subscription object, which already contains a one_shot flag and could have included a no_event flag. Then new or adjusted subscribers would use the new method while un-adjusted ones could use the old.

We could even have done a rename of subscribe to deprecated_subscribe, since we would surely be set up to support that kind of program-wide renaming here. If we couldn’t deal with that, we’d just use the new name.

So an incremental approach would be possible, showing once again that there really are no refactorings that have to be done all at once.

So … not bad for a quick session. We’ve done worse, and this is a decent step in roughly the right direction.

See you next time!