More DungeonView
Hello, loves!
If there was a feature I wanted, I’d do that. There isn’t, so we’ll look for some more small but significant improvements. Fiddling while Rome burns? Why not?
In a Mastodon exchange involving flying monkeys ( my pretties!) @GeePawHill subtly suggested that if my DungeonView is too hard to test, I might be “doing it wrong”, and that perhaps it’s doing things that actually belong in the model, Dungeon, et al. He challenged me to think of something that clearly should not be in the model but that requires logic. We’ll keep an eye out for that.
Mostly, however, we’re just here to try to carve DungeonView up into smaller chunks, so as to make it easier to understand and to work with. Along the way we’ll keep an eye out for “logic”, conditional or looping code that might suggest something.
One thing comes immediately to mind as soon as I open the class: it has about seventy-leben lines of setup code. Let’s start by looking at that. It seems likely that a lot of that might be offloaded into a DungeonViewerInitializer or summat.
class DungeonView(arcade.View):
def __init__(self, dungeon, testing=False):
if not testing:
super().__init__()
self.dungeon = dungeon
self.pub_sub = dungeon.pub_sub
self.subscribe(dungeon, self.pub_sub)
self.setup_assets()
self.key_lock = None
self.keyed_floor_sprites = KeyedSpriteList(arcade.SpriteList())
self.content_views: dict[Content, ContentView] = dict()
self.content_views_by_cell: dict[Cell, list[ContentView]] = defaultdict(list)
self.content_sprite_list = None
self.cameras = None
def subscribe(self, dungeon, pub_sub):
self.subscribe_to_announce(dungeon, pub_sub)
self.subscribe_to_remove_content(pub_sub)
self.subscribe_to_state_number(pub_sub)
def run(self):
self.setup()
self.dungeon.run()
self.window.show_view(self)
self.illuminate_around_dot()
arcade.run()
def setup(self):
self.setup_cameras()
self.setup_scroller()
self.create_rooms()
self.create_content_lists()
@classmethod
def setup_assets(cls):
arcade_resources = ':resources:images/items/'
arcade_resources = arcade.resources.resolve(arcade_resources)
my_resources = '/Users/ron/Desktop/DungeonTiles/png/objects/'
arcade.resources.add_resource_handle('ron',arcade_resources)
arcade.resources.add_resource_handle('ron',my_resources)
def create_content_lists(self):
self.content_views = dict()
self.content_sprite_list = arcade.SpriteList()
for cell, content in self.dungeon.layout.contents.items():
for item in content:
self.make_view_and_sprite(cell, item)
def make_view_and_sprite(self, cell, item):
resources = item.resources
scale = item.scale
view = ContentView(cell, item, resources, scale)
view.sprite.position = cell.center_position(cell_size)
self.content_views[item] = view
self.content_views_by_cell[cell].append(view)
self.content_sprite_list.append(view.sprite)
if self.keyed_floor_sprites[cell].visible:
view.sprite.visible = True
def setup_cameras(self):
zoom = 4
self.cameras = Cameras(self, self.max_x, self.max_y, zoom)
def setup_scroller(self):
self.make_initial_announcement()
def create_rooms(self):
for room in self.dungeon.rooms:
view = RoomView(room)
for cell, sprite in view.generate_sprites(self.dungeon.layout):
self.keyed_floor_sprites.add(cell, sprite)
def make_initial_announcement(self):
for message in [
'Welcome to the Friendly Dungeon of Doom',
'Here you will encounter many new friends,',
'some of whom will not attack you.',
'Some of you may die, but we are willing to make that sacrifice.',
'Good luck! You will need it.'
]:
self.pub_sub.publish('announce', 'view', message=message)
Wow, there’s eighty-plus lines we use once and then never again while the game runs. The whole class file is only 226 lines.
One concern comes right to mind: that call to self.dungeon.run(). We’re going to be setting up our initial look at the dungeon here so we want to be sure it’s ready to go. What goes on there now?
class Dungeon:
def run(self):
self.layout.run(self.pub_sub)
Not much there, so …
class DungeonLayout:
def run(self, pub_sub):
for cell, contents in self.contents.items():
for content in contents:
content.run(pub_sub)
Ah. OK, so that’s the way it is. I call. Next card:
class Content:
def run(self, pub_sub):
for sub in self.subs:
sub.callback = types.MethodType(sub.callback, self)
pub_sub.subscribe_all(self.subs)
Right. This is all about content subscriptions, which can only be done once the PubSub exists. We’ll keep that in mind.
Since it’s still in view in my text editor, I note make_initial_announcment, which really seems to me to be a dungeon thing, to be done at the end of dungeon.run. Would that work if we just moved it? Easy enough to find out.
That does work. Sweet. Remove the one in the view. That’s this:
def setup_scroller(self):
self.make_initial_announcement()
So we remove that, too. Commit: moving initial announcements to dungeon.
Reflection
That move makes sense. All the announcements should come from things that happen in the dungeon. So.
If we only had a few minutes to make the DungeonView better, we could return to our oar. And, while the move has made Dungeon larger, it doesn’t reduce its cohesion, since announcing is already one of its functions. And we do reduce coupling between view and model. I think it’s a net win. Down to 211 lines in DV.
Where were we? Oh, right, looking for ways to make DungeonView smaller and more manageable.
I noticed when scrolling down to get DV’s line count, in the keyboard handler:
...
elif symbol == arcade.key.A:
for message in [
'You typed A!',
'Very clever!',
'You have received a kitten',
'from the Cat Distribution System!',
'Guard it, it may be important!']:
self.pub_sub.publish('announce','view', message=message)
elif symbol == arcade.key.B:
for message in [
'You have discovered a gold coin!',
'There is a small cage here.',
'Your lamp has gone out!!',
'You have been eaten by a grue!',
]:
self.pub_sub.publish('announce', 'view', message=message)
elif symbol == arcade.key.H:
self.dungeon.make_initial_announcement()
...
Those are just there to entertain myself and check the announcement stuff. Also there is a breakpoint in the B one, which I’d like to retain but maybe only when you type Control+B. Let’s do that.
elif symbol == arcade.key.B and (modifiers & arcade.key.MOD_CTRL):
message = 'You have been eaten by a breakpoint!'
self.pub_sub.publish('announce', 'view', message=message)
pass
I set the breakpoint on the pass. Commit: tidying key press a bit. 199 lines.
I think we’ll want to move the keyboard processing entirely outside DungeonView. That should be perfectly doable, but this morning I’m looking for easier prey.
Scrolling, I notice this:
class DungeonView:
def subscribe_to_announce(self, dungeon, pub_sub):
def callback(*, pub_sub, message):
dungeon._announce(message)
pub_sub.subscribe('announce', '', callback)
Why is the view subscribing to ‘announce’, only to call the dungeon? Can’t the dungeon subscribe to things? Why shouldn’t it subscribe to ‘announce’? Make it so:
class Dungeon:
def __init__(self, layout):
self.layout = layout
self.player_cell = None
self.player_inventory = []
self.announcements = []
self.pub_sub = PubSub()
def callback(*, pub_sub, message):
self._announce(message)
self.pub_sub.subscribe('announce','', callback)
self.flood_list = SpriteList()
A test fails. That’s because the DV also subscribes to that and all the announcements get filed twice. Remove that.
Done. All good. Commit: move announcement subscription to Dungeon. 192 lines in DV.
I notice these properties:
class DungeonView:
@property
def max_x(self):
return self.dungeon.max_x
@property
def max_y(self):
return self.dungeon.max_y
They’re used but once. Not paying off. Inline them.
class DungeonView:
def setup_cameras(self):
self.cameras = Cameras(self, self.dungeon.max_x, self.dungeon.max_y, zoom=4)
We might consider inlining that as well. Let’s do. First commit as we are, in case we don’t like it.
def setup(self):
self.cameras = Cameras(self, self.dungeon.max_x, self.dungeon.max_y, zoom=4)
self.create_rooms()
self.create_content_lists()
I think that’s as clear as setup_cameras, which used to be all complicated. Commit. 178 lines.
Reflection
This seems like a good place to pause. Let’s see what we accomplished. We slimmed DungeonView down from 226 to 178 lines. (A few of those were in removal of excess white space.) We eliminated a subscription from DungeonView, moving it to Dungeon instead. Since it dealt with announcements, which are presently part of Dungeon, that improved cohesion and reduced coupling.
I see two “big” improvements that may be profitable. One will be to move the keyboard processing off to a separate little object. That should be straightforward, although just now I don’t see a series of machine refactorings that will do the job. I think it’ll have to be done manually but still should be easy. The other improvement will be to move as much of the init / setup / get ready / get set / go logic off to one or more helpers.
I’m not sure without study, but that might get DungeonView very close to doing nothing but on_update and on_draw, plus adjusting its sprites and such based on dungeon’s published updates, which seem to come down right now to ‘remove_content’ and ‘state_number’. I think we’ll push illumination down into Dungeon, and if we do that will require at least one more subscription, to update our sprites’ visibility. I think that part will be “interesting”.
Summary
Over eight commits, every one of which was suitable for production, we’ve removed about fifty lines of code from DungeonView, improving coupling and cohesion as we did it. Each of those changes was just a few lines and could readily be done while tidying up other work in the area, or just as a bit of relaxation.
I think that the odds are very good that we can improve any design incrementally, and bring it back to where it “should” be. So good that I’d just bet “yes” in he absence of serious evidence to the contrary.
Of course … all this is moot, since programming, and possibly human society, and maybe the human species, is about over. But while we were destroying ourselves, we made some good art and good crafts.
On that cheery note, I’ll invite you to stop by next time!