Using Caller_ID
With the new event modifier, I think we can eliminate the button_pressed event.
But first … I do like the notion of calling the modifier caller_id, so I think I’ll do that renaming. PyCharm helpfully renames throughout all the implementations of the protocol.
But next … I don’t like this magic one_shot flag appearing in the public argument list:
class PubSub:
def subscribe(self, event, caller_id, callback, one_shot=False):
key = (event, caller_id)
subscription = Subscription(callback, one_shot)
try:
self.subscriptions[key].append(subscription)
except KeyError:
self.subscriptions[key] = [subscription]
def subscribe_once(self, event, caller_id, callback):
self.subscribe(event, '', callback, one_shot=True)
Let’s extract a private method:
def subscribe(self, event, caller_id, callback, one_shot=False):
subscription = Subscription(callback, one_shot)
self._store_subscription(subscription, event, caller_id)
def _store_subscription(self, subscription, event, caller_id):
key = (event, caller_id)
try:
self.subscriptions[key].append(subscription)
except KeyError:
self.subscriptions[key] = [subscription]
Then use it in subscribe_once:
def subscribe_once(self, event, caller_id, callback):
subscription = Subscription(callback, True)
self._store_subscription(subscription, event, caller_id)
Then set the flag literally in subscribe:
def subscribe(self, event, caller_id, callback, one_shot=False):
subscription = Subscription(callback, False)
self._store_subscription(subscription, event, caller_id)
Then change the protocol in the abstract base class:
class PubSubProtocol(ABC):
@abstractmethod
def subscribe(self, event, caller_id, callback):
pass
Double check the Dungeon’s version. It’s correctly converted. Commit: remove irritating one_shot parameter.
Reflection
We saw some opportunities for improvement, with the more friendly and meaningful name caller_id, and a simple refactoring that hides the one_shot flag fully inside the PubSub implementation, instead of making it visible in the protocol. Much nicer, very easy and quick to do. Further comment down in Summary.
Now the plan we rode in on was to replace the ‘button_pressed’ event with a personalized ‘state_number’ event. The signup is in AppearingContent, and the message sending is in Button.
class Button:
def interact_with_player(self, dungeon):
dungeon.publish(self.name + ' button_pressed', '')
self.state += 1
if self.state >= len(self.resources):
self.state = 0
dungeon.publish('state_number', '', self, self.state)
class AppearingContent:
def run(self, dungeon):
dungeon.publish('state_number', '', self, False)
self.visible = False
def callback(event):
dungeon.announce(f'{self.name} has appeared!')
dungeon.publish('state_number', '', self, True)
self.visible = True
dungeon.subscribe_once(self.name + ' button_pressed', '', callback)
We’ll want to remove the ‘button_pressed’ publish, and change the state_number one to include self.name:
class Button:
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)
And change the subscription in AppearingContent:
class AppearingContent:
def run(self, dungeon):
dungeon.publish('state_number', self.name, self, False)
self.visible = False
def callback(event):
dungeon.announce(f'{self.name} has appeared!')
dungeon.publish('state_number', self.name, self, True)
self.visible = True
dungeon.subscribe_once('state_number', self.name, callback)
I decided arbitrarily that the convention should be to always include one’s name if one is a content item publishing messages. We might even work up a way to formalize that, but not now.
The tests are green, but I want to know if the game works.
It would be a good idea if I set up the callbacks and messages correctly.
def run(self, dungeon):
dungeon.publish('state_number', False)
self.visible = False
def callback(event):
dungeon.announce(f'{self.name} has appeared!')
dungeon.publish('state_number', True)
self.visible = True
dungeon.subscribe_once('state_number', self.name, callback)
That should work somewhat better. It does not. I get this error:
self.execute_callbacks(general, event, args, kwargs)
File "/Users/ron/PycharmProjects/dungeon/src/pub_sub.py", line 51, in execute_callbacks
subscription.callback(event, *args, **kwargs)
TypeError: DungeonView.subscribe_to_state_number.<locals>.callback() missing 2 required positional arguments: 'item' and 'state_number'
I do not understand that. I think we’ll revert, make sure all is well, then try again.
All is well after the reset. I wonder if I need a test, or what?
I’ll try again, slowly. First AppearingContent:
def run(self, dungeon):
dungeon.publish('state_number', '', self, False)
self.visible = False
def callback(event):
dungeon.announce(f'{self.name} has appeared!')
dungeon.publish('state_number', '', self, True)
self.visible = True
dungeon.subscribe_once('state_number', self.name, callback)
This should result in the item not appearing, as we will not get the message. We’ve specified our name as caller_id and Button is not using it yet. Nothing happens when Dot steps on the button, as expected.
Change Button to include its name in the state publication:
def interact_with_player(self, dungeon):
dungeon.publish(self.name + ' button_pressed', '')
self.state += 1
if self.state >= len(self.resources):
self.state = 0
dungeon.publish('state_number', self.name, self, self.state)
I think this will fail, and I think I know why. First test in game. Yes, fails as before.
I think the defect is simple: the callback function was not revised to include the parms that are published with ‘state_number’.
def run(self, dungeon):
dungeon.publish('state_number', '', self, False)
self.visible = False
def callback(event, button, state):
dungeon.announce(f'{self.name} has appeared!')
dungeon.publish('state_number', '', self, True)
self.visible = True
dungeon.subscribe_once('state_number', self.name, callback)
Changed the callback to have the expected parameters. Should work now. Does. Perfect. Commit: AppearingContent no longer subscribes to ‘button_pressed’.
Now stop sending it.
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)
Test again, expecting all good. All is good. Commit: Button no longer sends ‘button_pressed’.
Reflection
I’m glad I reset. It helps reset the mind and in doing the changes again, the issue came to me. Had I not reset, I’d have resorted to printing or similar debugging thinking. That generally wastes time, although I am not always wise enough to just reset when I probably should.
There are things to be concerned about:
- There was no test to detect that problem, and there probably could be;
- The AppearingContent should probably check the button to be sure we are seeing the
downstate. Currently it will go visible on the first state change of the Button whether up or down. - Making Content items always include their name with events seems like a good idea. Not yet done and should be done so that one cannot forget.
We’ll address those in due time, I imagine. For now, I think it’s break time.
Summary
As often happens, I come here with a particular thing in mind, like switching from ‘button_pressed’ to ‘state_number’. And then I start right off doing something else. That’s not just because I am Chaotic Neutral, although that does probably contribute. The main reason is that I try to stay alert to opportunities to improve the code that I’m working on.
I generally recommend only improving code that is being visited anyway, although here chez Ron we might randomly improve anything that comes to mind, because we have no deadlines or waiting customers. But in Real Life™, there isn’t always enough spare time to go looking for areas to improve, but we really do better if we do a little improvement where we are going to work anyway. So I try to scan the code, see what it might like, and give it a little treat when I can.
Then we usually get down to the case in hand, this time the conversion to a different publication. Recall that we actually built the necessary feature, including a caller ID string as part of the subscription, yesterday. That was a tiny bit speculative but worked out just fine. I felt that doing that separately would be better than driving the change from the application level.
Right, wrong? Not important. Better than it was, that’s what’s important.
See you next time!