Hello, loves!

I was watching Antiques Roadshow, as one does, when it came to me that a tiny object might be just the thing for DungeonView. Let’s find out.

DungeonView needs a lot of work, but the code of concern this morning is this:

class DungeonView(arcade.View):
    def __init__(self, dungeon, testing=False):
        ...
        self.dungeon_floor_sprites = arcade.SpriteList()
        self.cell_sprites: dict[Cell, Sprite] = dict()
        self.content_views: dict[Content, ContentView] = dict()
        self.content_views_by_cell: dict[Cell, list[ContentView]] = defaultdict(list)
        self.illuminated_cells: set[Cell] = set()
        self.content_sprite_list = None
        ...

DungeonView has way too many collections, but this morning we’ll look at these two:

        self.dungeon_floor_sprites = arcade.SpriteList()
        self.cell_sprites: dict[Cell, Sprite] = dict()

Those are used, variously:

    def setup(self):
        self.setup_scroller()
        self.setup_cameras()
        self.dungeon_floor_sprites = arcade.SpriteList()
        self.create_rooms(self.dungeon_floor_sprites)
        self.create_content_lists()

    def create_rooms(self, shape_list):
        for room in self.dungeon.rooms:
            view = RoomView(room)
            for cell, sprite in view.generate_sprites(self.dungeon.layout):
                self.cell_sprites[cell] = sprite
                self.dungeon_floor_sprites.append(sprite)

    def on_draw(self):
        self.clear()
        self.scroll_dungeon_camera()
        with self.dungeon_camera.activate():
            self.dungeon_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.scroller_camera.activate():
            self.scroller.draw()

    def illuminate_cell(self, cell):
        sprite = self.cell_sprites[cell]
        sprite.visible = True
        self.illuminated_cells.add(cell)
        for content_view in self.content_views_by_cell[cell]:
            content_view.just_illuminate()

What’s going on is simpler than this may appear. For drawing efficiency, we keep all the floor cell tile sprites in a SpriteList, which can be drawn all in one go. But when we illuminate a room, we need to set visible on the individual sprites for illuminated cells, so we need access to the sprite for each particular cell.

So, currently, we have the full list dungeon_floor_sprites and a dictionary, cell_sprites, for those two purposes.

The idea this morning is to cover both uses with a single object. Let’s TDD it, just for fun.

class TestKeyedSpriteList:
    def test_creation(self):
        assert False

Fails. Perfect. I think we’ll write very few tests here, but let’s start out in the classic form, just creating the object.

class TestKeyedSpriteList:
    def test_creation(self):
        keyed = KeyedSpriteList(SpriteList())

Fails because there is no such thing. Make such thing:

class KeyedSpriteList:
    def __init__(self, sprite_list):
        self.sprite_list = sprite_list

I didn’t declare the sprite_list type, because for testing purposes, I want to use an ordinary list. That’s also part of why we provide the sprite list to our new object. Another reason is that we might conceivably want to create the actual SpriteList externally, although I think that will be unlikely.

New test:

    def test_add_and_access(self):
        contents = list()
        keyed = KeyedSpriteList(contents)
        keyed.add('cell_a', 'sprite_A')
        keyed.add('cell_b', 'sprite_B')
        assert len(contents) == 2
        assert 'sprite_A' in contents
        assert 'sprite_B' in contents
        assert keyed['cell_a'] == 'sprite_A'
        assert keyed['cell_b'] == 'sprite_B'

We expect to add a cell and a sprite, but the object doesn’t care what we add. We expect all the sprites to show up in our provided content list and we expect to be able to access them by name using a subscripting notation.

Test fails, of course. We code:

class KeyedSpriteList:
    def __init__(self, sprite_list):
        self.sprite_dict = dict()
        self.sprite_list = sprite_list

    def add(self, cell, sprite):
        self.sprite_dict[cell] = sprite
        self.sprite_list.append(sprite)

Test still fails but I expect it to fail on the second last assert.

>       assert keyed['cell_a'] == 'sprite_A'
               ^^^^^^^^^^^^^^^
E       TypeError: 'KeyedSpriteList' object is not subscriptable

That’s just right. Make it work:

class KeyedSpritList:
    def __getitem__(self, cell):
        return self.sprite_dict[cell]

And we are green.

And we are at a nice point to commit this code. Commit: TDDing KeyedSpriteList.

Now let’s move the KeyedSpriteList to the ‘src’ tree: I built it inside the test file, as I often do with h/t to Keith Braithwaite. Done. Commit: move KSL to prod.

Brief Reflection

We could just bang this thing in: there are only a few places needing to be changed, including, I think, one test that may break along the way.

But let’s pretend that we have lots of places needing to be changed, and do it incrementally, one place at a time, never breaking anything (well, trying never to break anything), rinse repeat.

First, we create the new object in DungeonView’s init.

class DungeonView:
    def __init__...
        ...
        self.dungeon_floor_sprites = arcade.SpriteList()
        self.keyed_floor_sprites = KeyedSpriteList(self.dungeon_floor_sprites)
        self.cell_sprites: dict[Cell, Sprite] = dict()
        ...

No harm done. Commit: converting to use KSL over weeks and weeks.

Now we have to get it loaded.

I tried this:

    def create_rooms(self, shape_list):
        for room in self.dungeon.rooms:
            view = RoomView(room)
            for cell, sprite in view.generate_sprites(self.dungeon.layout):
                self.cell_sprites[cell] = sprite
                self.dungeon_floor_sprites.append(sprite)
                self.keyed_floor_sprites.add(cell, sprite)

But tests fail because we are adding to the same SpriteList twice. We need to give our KSL its own list.

        self.keyed_floor_sprites = KeyedSpriteList(arcade.SpriteList())

Tests go green. Code above is OK. Commit: converting to use KSL over weeks and weeks.

Weeks later, find a place that can use the new object: draw.

class DungeonView:
    def on_draw(self):
        self.clear()
        self.scroll_dungeon_camera()
        with self.dungeon_camera.activate():
            self.dungeon_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.scroller_camera.activate():
            self.scroller.draw()

We could fetch the list and draw it but why not let the KSL do the job. Add the method and use it.

class KeyedSpriteList:
    def draw(self):
        self.sprite_list.draw()

class DungeonView:
    def on_draw(self):
        self.clear()
        self.scroll_dungeon_camera()
        with self.dungeon_camera.activate():
            self.keyed_floor_sprites.draw()
            self.draw_contents()
            self.draw_passages()
            self.draw_flood()
        with self.scroller_camera.activate():
            self.scroller.draw()

We do test that one on the screen, and are not surprised when it works. Commit. Is the floor list used anywhere else? It appears not. Remove all the references.

    def create_rooms(self):
        for room in self.dungeon.rooms:
            view = RoomView(room)
            for cell, sprite in view.generate_sprites(self.dungeon.layout):
                self.cell_sprites[cell] = sprite
                self.keyed_floor_sprites.add(cell, sprite)

We discovered along the way that create_rooms wasn’t even using the provided list, so we removed the parameter. Commit.

Users of cell_sprites are of interest now. Here’s one:

    def illuminate_cell(self, cell):
        sprite = self.cell_sprites[cell]
        sprite.visible = True
        self.illuminated_cells.add(cell)
        for content_view in self.content_views_by_cell[cell]:
            content_view.just_illuminate()

We can use KSL here.

    def illuminate_cell(self, cell):
        sprite = self.keyed_floor_sprites[cell]
        sprite.visible = True
        self.illuminated_cells.add(cell)
        for content_view in self.content_views_by_cell[cell]:
            content_view.just_illuminate()

Commit. That’s the only local reference that was left. A test will fail when we remove the instance variable. It’s referring to cell_sprites and we make it refer to keyed_floor_sprites and we’re good.

Six commits, basically one possible change at a time, no breakage. And we have reduced two instance variables down to one, and removed a few lines of code. Modified a few more.

Reflection

Was it worth all that to remove what amounts to a single instance variable? I don’t know. It has taken an hour to do the work and write the article. We have taken two objects which could in principle get out of sync and encapsulated them so that they will always stay in sync. We have taken two lines of code that always had to be done together and put them together in one place that everyone else just uses without knowing what’s behind the curtain.

A better question to ask might be “Do you kind of wish you had thought of this object at the moment you created that second structure, whichever one it was?” And that one is easy: I do wish I had, and I’m a bit more dedicated to noticing places where a tiny object can help me.

Part of my purpose in programming, whether for show, like here, or for real, as I used to do in the olden days, is to keep my design as clear and proper as I can. I do not have the ability to figure that out in advance, and if anyone has, I have not met them. I do have the ability to improve things as I become aware of them, and every decent programmer has that ability as well.

Summary

A tiny object makes the code a bit better. It was easy to write, easy to install, and is cute as a kitten. Well, almost.

See you next time!