Hello, loves!

I think we can do without the info object entirely. Let’s find out. Bear takes a nibble but I think we’re OK.

It occurred to me that we can probably put an item’s extra variables right in the procedure that creates a CombinedContent item, and reference them using nonlocal inside the callbacks. It turns out that our existing callback test actually demonstrates the idea:

    def test_subscriptions(self):
        count = 0
        def increment(*, content, state):
            nonlocal count
            print('counting')
            count += 1
        sub = Subscription(event='do_it',
                              caller_id='',
                              callback=increment)
        content = CombinedContent(
            name='item',
            resources = [],
            scale=1,
            subs=[sub]
        )
        content.run(self)
        self.pub_sub.publish('do_it',
                             caller_id='xxx',
                             content=self,
                             state=1)
        assert count == 1

We declare the variable in the setup, reference it via nonlocal in the callback function, and check it back in the test. And it works, which isn’t really much of a surprise. But it does mean that we don’t need the info feature. We’ll keep it or lose it depending on how things look when we’re done.

I think we can try making the appearing object or the animated one next. Which one looks easier? Ah, an easy decision, it’s Appearing:

class AppearingContent(Content):
    def interact_with_player(self, dungeon) -> bool:
        if self.visible:
            dungeon.receive_content(self)
            dungeon.announce(f'You have received {self.name}!')
        return True

    def run(self, dungeon):
        dungeon.publish('state_number', '', content=self, state=False)
        self.visible = False
        def callback(content, state):
            dungeon.announce(f'{self.name} has appeared!')
            dungeon.publish('state_number', '', content=self, state=True)
            self.visible = True
        dungeon.subscribe_once('state_number', self.name, callback)

Let’s see if we have a test for that one. We have not. Bad Ron, no biscuit. We’ll write one now.

Ah, there is an issue. We want to subscribe to state_number and then announce and publish some things. The callback for state_number does not send us anything that understands those two messages.

The interact callback does get an Interactor, which can do the job. We can perhaps cache that … but no, we are not guaranteed to do interact before the state is signaled. It depends on where Dot walks.

This problem is not unique to the idea of using nonlocal. If we use the info idea, we still will have no access to the dungeon.

I think I’ll try a hack and then see if I can improve things. The CombinedContent will receive run before anything else happens. That receives the Dungeon, and let’s just cache it in the item’s members so that people can get at it. We don’t like contents knowing their owners but I don’t see a way out of this unless we were to pass the Dungeon on every event … and PubSub does not know the Dungeon either.

I’ll change CombinedContent thus:

class CombinedContent:
    def __init__(self, *, name, resources, scale,
                 interaction=lambda self, interactor: True,
                 info=None,
                 subs=[]):
        self.name = name
        self.resources = resources
        self.scale = scale
        self.state = 0
        self.info = info
        self.subs = subs
        self.interact_with_player = types.MethodType(interaction, self)
        self.dungeon = None

    def run(self, dungeon: Dungeon):
        self.dungeon = dungeon
        self.dungeon.subscribe_all(self.subs)

A delay occurs …

The good news is that this test runs:

    def test_appearing(self):
        name = 'a thing'
        visible = False
        dungeon = None
        def interaction(self, interactor):
            nonlocal visible, dungeon
            dungeon = interactor
            if visible:
                interactor.receive_content(self)
                interactor.announce(f'You have received {self.name}!')
            return True
        def has_appeared(content, state):
            nonlocal visible, dungeon
            dungeon.announce(f'{name} has appeared!')
            dungeon.publish('state_number', 'x', content=self, state=state)
            visible = True
        sub = Subscription(event='state_number',
                           caller_id=name,
                           callback=has_appeared,
                           one_shot=True)
        item = CombinedContent(name=name,
                               resources=['a', 'b'],
                               scale=0.5,
                               interaction=interaction,
                               subs=[sub]
                               )
        item.run(self)
        item.interact_with_player(self)
        assert self.message is None
        self.pub_sub.publish('state_number', name, content=self, state=1)
        assert self.message == 'a thing has appeared!'

The bad news includes:

I got a recursion error in that I was publishing ‘state_number’ in the has_appeared call, to get the image to appear, because DungeonView subscribes to that message to know when to update things. I am not sure how it knows quite what to do in this case, so we’ll look at that below.

I had to change the caller_id of the publish to avoid the recursion. This is because the protocol for appearing is that it monitors ‘state_number’ events referring to its own name, because, well, that’s the convention we were using: the Button for the key uses the same name as the key. We need a better protocol to be sure we don’t recur. Ideally, PubSub would detect that but I’m not sure that’s even possible.

I had to add the one_shot flag to the Subscription object, which isn’t a big deal, but right in the middle of everything else it was a bit disruptive.

I had to use the nonlocal trick to cache the dungeon in the actual setup of the CombinedContent, because the callbacks do not include a self parameter and in fact as constituted they don’t even know it. So that is a double crock because we shouldn’t have to cache it at all, and we had to jam it back into the setup code where the callbacks were defined.

I think that if I were to remove the interact_with_player call, the test will crash looking for the dungeon variable, which will not have been set. Yes.

    def has_appeared(content, state):
        nonlocal visible, dungeon
>       dungeon.announce(f'{name} has appeared!')
        ^^^^^^^^^^^^^^^^
E       AttributeError: 'NoneType' object has no attribute 'announce'

Time to break and reflect upon our sins.

Reflection

I think the bear got a little bite of us here, but we escaped fatal damage.

In the existing AppearingContent, we define the callback inside run, which it overrides from the Content protocol, where it does have access to the dungeon, and thus to publish and announce. In the new scheme, we’re trying to make everything happen at setup time and at that time we do not know the dungeon. In addition, callbacks in general do not know the dungeon except for the fact that they have been, up until now, defined inside the run method.

The current scheme, a serious crock, still doesn’t work without a guaranteed call to pass the dungeon in and save it somewhere where callbacks can find it. A top-level “singleton” style of instance might do the job. It need not be the dungeon, just something that supports publish-subscribe and announce. (So far.)

I have one idea … the content parameter, in this case, is probably the CombinedContent and it does know the dungeon because of the run crock. So this test might pass:

    def test_appearing(self):
        name = 'a thing'
        visible = False
        dungeon = None
        def interaction(self, interactor):
            nonlocal visible, dungeon
            dungeon = interactor
            if visible:
                interactor.receive_content(self)
                interactor.announce(f'You have received {self.name}!')
            return True
        def has_appeared(content, state):
            nonlocal visible, dungeon
            content.dungeon.announce(f'{name} has appeared!')
            content.dungeon.publish('state_number', 'x', content=self, state=state)
            visible = True
        sub = Subscription(event='state_number',
                           caller_id=name,
                           callback=has_appeared,
                           one_shot=True)
        item = CombinedContent(name=name,
                               resources=['a', 'b'],
                               scale=0.5,
                               interaction=interaction,
                               subs=[sub]
                               )
        item.run(self)
        # item.interact_with_player(self)
        assert self.message is None
        self.pub_sub.publish('state_number', name, content=item, state=1)
        assert self.message == 'a thing has appeared!'

In this case (only, but it might generalize) our callbacks receive a content parameter, which is some content item, in this case the very one we’re testing. So in the has_appeared callback, not the best name, content knows dungeon and the messages work.

The callback name should be time_to_appear or appear, not has appeared because it hasn’t, yet.

One not unreasonable thing might be to pass the PubSub instance to every callback, making it the standard first parameter. That’s a bit of a large change, but it might not be terrible. And it doesn’t give anyone access to anything they shouldn’t be able to see, since we do want them to be able to publish and subscribe at will.

I’m also wondering about interact_with_player. It has that special status with a separate callback being created, and installed as if it were an actual method. Should we turn it into a standard callback? Or should all subscriptions be installed similarly to interact? Would we be happier if they all had access to self?

One way or the other, making event callbacks and interact work using the same mechanism seems desirable if we can do it.

Much to think about …

Therefore …

First, I want to sit with this a bit. Since it’s all inside the test file, I can commit it at my leisure, i.e. right now.

When I return, tomorrow, I imagine, I think we’ll look at passing the PubSub instance into all event callbacks. If that works, and I feel that it will, we can unwind some of the weirdness.

I think we can accomplish the goal of having a single very simple Content class. Some of the construction code looks a bit gnarly, and that is a bit concerning, but it’s probably no worse, and possibly a bit simpler than the old way. We’ll look into that.

I am dented but undaunted. The test runs, and the implementation “works”, now the thing is to make it “right”, or more nearly so. I think we’re OK.

See you next time!