Hello, loves!

We’ll remain on the quest to simplify DungeonView a bit longer. I did some things last night, and we’ll do more this morning.

As things stood when last we met, the DungeonView maintained more than one SpriteList, one for floor tiles and — I’ve already forgotten — one or more for content items. With a simple change, now all the sprites are in one list.

class DungeonView(arcade.View):
    def __init__(self, dungeon):
        if arcade.window_commands._window:
            super().__init__()
        self.dungeon = dungeon
        self.pub_sub = dungeon.pub_sub
        self.subscribe(dungeon, self.pub_sub)
        self.setup_assets()
        self.sprite_list = arcade.SpriteList()
        self.keyed_floor_sprites = KeyedSpriteList(self.sprite_list)
        self.content_views: dict[Content, ContentView] = dict()
        self.content_views_by_cell: dict[Cell, list[ContentView]] = defaultdict(list)
        self.cameras = None
        self.keys = KeyPress(self, self.dungeon, self.pub_sub)

We create just that one SpriteList, and then use it everywhere, in the floor sprites KeyedSpriteList, but also:

    def setup(self):
        self.cameras = Cameras(self, self.dungeon.max_x, self.dungeon.max_y, zoom=4)
        self.create_room_sprites()
        self.create_content_lists(self.sprite_list)

    def create_content_lists(self, sprite_list):
        self.content_views = dict()
        for cell, content in self.dungeon.layout.contents.items():
            for item in content:
                self.make_view_and_sprite(cell, item, sprite_list)

    def make_new_content(self, cell, item):
        self.make_view_and_sprite(cell, item, self.sprite_list)

    def make_view_and_sprite(self, cell, item, sprite_list):
        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)
        sprite_list.append(view.sprite)
        if self.keyed_floor_sprites[cell].visible:
            view.sprite.visible = True

The current code gets all the sprites in the one list, although we still have those supporting structures for finding them. But in drawing, we just draw the KeyedSpriteList:

    def on_draw(self):
        self.clear()
        self.cameras.scroll_dungeon_cam(self.dungeon.player_cell)
        with self.cameras.dungeon_cam.activate():
            self.keyed_floor_sprites.draw()
            self.draw_adventurer()
            self.draw_flood()
        with self.cameras.scroller_cam.activate():
            self.cameras.scroller.draw()

So that’s a bit of a hack. In make_view_and_sprite, we just append a sprite to the KeyedSpriteList’s internal list, because we still have a handle to it. The content sprites do not appear in the keys, just in the sprites.

The code is at the “works” stage of “Make it work, make it right”. To make it right we should do two things: ensure that every sprite in the KeyedSpriteList has a key that points to it, and use that key for all the other accesses to the sprites, so that we can remove the other constructs, content_views and content_views_by_cell.

If that makes sense. We’ll have to check to see what use we make of those, especially content_views_by_cell, which may have a legitimate purpose. They both might, but I don’t think so.

I think the first thing to do is to rename keyed_floor_sprites to something connoting what it is, all the sprites. I think keyed_sprites is the right name for that.

    self.keyed_sprites = KeyedSpriteList(self.sprite_list)

Now what I’d like to do is to put the content items into the KeyedSpriteList with a proper key. But first, I think we need to figure out what those other two dictionaries are there for. First content_views:

    def __init...
        ...
        self.content_views: dict[Content, ContentView] = dict()
        ...

    def make_view_and_sprite(self, cell, item, sprite_list):
        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)
        sprite_list.append(view.sprite)
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

The dictionary is used during subscription callbacks:

    def subscribe_to_remove_content(self, pub_sub):
        def callback(*, pub_sub, content):
            self.content_view_do(content, lambda view: view.remove())
        pub_sub.subscribe('remove_content', '', callback)

    def subscribe_to_state_number(self, pub_sub):
        def callback(*, pub_sub, content, state):
            self.content_view_do(content, lambda view: view.set_state(state))
        pub_sub.subscribe('state_number', '', callback)

    def content_view_do(self, content_item, action):
        try:
            view = self.content_views[content_item]
            action(view)
        except KeyError:
            return

We have to dig down to ContentView remove and set_state:


class ContentView:
    textures = TextureProvider()
    def __init__(self, cell, content_item, resources, scale=0.5):
        self.cell = cell
        self.item = content_item
        self.sprite = self.textures.load_sprite(resources)
        self.sprite.set_texture(0)
        self.sprite.visible = False
        self.sprite.scale = scale_texture(self.sprite.textures[-1], scale)

    def just_illuminate(self):
        self.sprite.visible = True

    def remove(self):
        self.cell.remove_content(self.item)
        self.sprite.remove_from_sprite_lists()

    def set_state(self, state_number):
        self.sprite.set_texture(state_number)

Were it not for that cell.remove_content bit, we could just fetch the sprite and remove it or set its visible or texture.

That cell.remove_content doesn’t belong in the view! It’s a dungeon operation, actually a layout operation:

class Cell:
    def remove_content(self, content):
        self.layout.remove_content_from_cell(content, self)

class DungeonLayout:
    def remove_content_from_cell(self, content, cell):
        self.contents[cell].remove(content)

The publisher of the ‘remove_content’ message should be doing this. And this looks odd:

    def receive_content_from_cell(self, content, containing_cell):
        contents = containing_cell.contents()
        if content in contents:
            self.pub_sub.publish('remove_content',
                                 '', content=content)
        if content in contents:
            # event may remove the content, done this way to support a test
            contents.remove(content)
        self.player_inventory.append(content)

That’s more weird than we need, but let’s change the view not to delete the contents.

class ContentView:
    def remove(self):
        self.sprite.remove_from_sprite_lists()

I want to be sure that the content is actually still removed. We have some tests for this. Here’s a key one:

    def test_contents_to_player(self):
        layout = DungeonLayout(10, 10)
        room_cell = Cell(5, 5)
        room = Room([room_cell])
        layout.add_room(room)
        dungeon = Dungeon(layout)
        factory = ContentFactory()
        item_1 = factory.receivable(name="treasure", resource='none', scale=0.5)
        shape = [(0.33, 0.66), (0.66, 0.66), (0.5, 0.33)]
        item_2 = factory.receivable(name="more treasure", resource='none', scale=0.5)
        room_cell.add_content(item_1)
        room_cell.add_content(item_2)
        dungeon.set_player_position_with_interaction(room_cell)
        assert item_1 in dungeon.inventory()
        assert item_2 in dungeon.inventory()
        assert room_cell.contents() == []

The final assert assures me that this process removes the cell contents as intended. So the removal of that remove_content from ContentView breaks nothing. Commit: ContentView no longer removes cell content.

More to the point, it seems to me that if we were to store the content item’s sprite in the KeyedSpriteList, using the item as the key, we’d be on the right track.

For that to actually work, we’ll need to get those subscriptions to accept a sprite. (Alternatively, as an interim step, we could probably create a ContentView on the fly.)

First, let’s get the sprites into the KSL. We need to change this:

class DungeonView:
    def make_view_and_sprite(self, cell, item, sprite_list):
        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)
        sprite_list.append(view.sprite)
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

Where we append to the sprite list, we need instead to add to the KSL. I think the item is a suitable key: we’ll see it again when we act on it. I’ll want to sort this out later but for now, we’ll access the KSL directly in the method.

    def make_view_and_sprite(self, cell, item, sprite_list):
        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.keyed_sprites.add(item, view) # <===
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

This passes all the tests. I want to run it. It fails. Back that out, see if it still fails. It does not. Good news, we have one line that fails. Oh, duh, I added the view, not the sprite. Someone should have told me.

    def make_view_and_sprite(self, cell, item, sprite_list):
        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.keyed_sprites.add(item, view.sprite) # <===
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

Full disclosure, PyCharm put cell where item belongs in the above, and I accepted it. I’m sure glad I don’t have the “AI” (ptui!) turned on, its mistakes would be harder to spot.

With that in place everything still works. No surprise. Commit: moving toward removal of content dictionaries.

Now the content_views dict is from item to view, and the KSL is from item to sprite. Let’s see if we can make a small step by making our use of content_views provide just the sprite. Where do we use it? Only here:

    def subscribe_to_remove_content(self, pub_sub):
        def callback(*, pub_sub, content):
            self.content_view_do(content, lambda view: view.remove())
        pub_sub.subscribe('remove_content', '', callback)

    def subscribe_to_state_number(self, pub_sub):
        def callback(*, pub_sub, content, state):
            self.content_view_do(content, lambda view: view.set_state(state))
        pub_sub.subscribe('state_number', '', callback)

    def content_view_do(self, content_item, action):
        try:
            view = self.content_views[content_item]
            action(view)
        except KeyError:
            return

I had hoped to create a ContentView on the fly as a quick step toward looking up the sprite, but ContentView’s constructor creates a sprite. We need the real one.

Maybe we can change the subscriptions to do the right thing, one at a time.

Oh, what if we added another do, like this:

    def content_sprite_do(self, content_item, action):
        try:
            sprite = self.keyed_sprites[content_item]
            action(sprite)
        except KeyError:
            return

Now, change the remove subscription:

    def subscribe_to_remove_content(self, pub_sub):
        def callback(*, pub_sub, content):
            self.content_sprite_do(content, lambda sprite: sprite.remove_from_sprite_lists())
        pub_sub.subscribe('remove_content', '', callback)

If that works, and I think it will, we get one free joyous cry. It does. Callooh! Callay! (That counts as one.)

Can we change the other one accordingly? It looks like this:

    def subscribe_to_state_number(self, pub_sub):
        def callback(*, pub_sub, content, state):
            self.content_view_do(content, lambda view: view.set_state(state))
        pub_sub.subscribe('state_number', '', callback)

class ContentView:
    def set_state(self, state_number):
        self.sprite.set_texture(state_number)

We sure can!

    def subscribe_to_state_number(self, pub_sub):
        def callback(*, pub_sub, content, state):
            self.content_sprite_do(content, lambda sprite: sprite.set_texture(state))
        pub_sub.subscribe('state_number', '', callback)

Test that. Works: The spikes cycle and the switch toggles.

Commit: No more use of content_views in prod.

Now remove the content_view_do method and the variable and its users. Commit. We’re left with the content_views_by_cell thing. What is that used for?

class DungeonView:
    def __init...
        ...
        self.content_views_by_cell: dict[Cell, list[ContentView]] = defaultdict(list)
        ...

    def make_view_and_sprite(self, cell, item, sprite_list):
        resources = item.resources
        scale = item.scale
        view = ContentView(cell, item, resources, scale)
        view.sprite.position = cell.center_position(cell_size)
        self.content_views_by_cell[cell].append(view)
        self.keyed_sprites.add(item, view.sprite)
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

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

I think we actually do need that collection, but I think we can make it contain just the sprites in question. We need to change it to tuck away the sprite there in make, and set the sprite visible in illuminate.

Make it so.

class DungeonView(arcade.View):
    def __init__(self, dungeon):
        if arcade.window_commands._window:
            super().__init__()
        self.dungeon = dungeon
        self.pub_sub = dungeon.pub_sub
        self.subscribe(dungeon, self.pub_sub)
        self.setup_assets()
        self.sprite_list = arcade.SpriteList()
        self.keyed_sprites = KeyedSpriteList(self.sprite_list)
        self.content_sprites_by_cell: dict[Cell, list[Sprite]] = defaultdict(list)
        self.cameras = None
        self.keys = KeyPress(self, self.dungeon, self.pub_sub)

    def make_view_and_sprite(self, cell, item, sprite_list):
        resources = item.resources
        scale = item.scale
        view = ContentView(cell, item, resources, scale)
        view.sprite.position = cell.center_position(cell_size)
        self.content_sprites_by_cell[cell].append(view.sprite)
        self.keyed_sprites.add(item, view.sprite)
        if self.keyed_sprites[cell].visible:
            view.sprite.visible = True

    def illuminate_cell(self, cell):
        sprite = self.keyed_sprites[cell]
        sprite.visible = True
        for content_sprite in self.content_sprites_by_cell[cell]:
            content_sprite.visible = True

That works. Commit: convert contents_view_by_cell to contents_sprites_by_cell

Now we can chase that use of ContentView in the make method. We surely don’t need to create that object and toss it away? Or do we?

class ContentView:
    textures = TextureProvider()
    def __init__(self, cell, content_item, resources, scale=0.5):
        self.cell = cell
        self.item = content_item
        self.sprite = self.textures.load_sprite(resources)
        self.sprite.set_texture(0)
        self.sprite.visible = False
        self.sprite.scale = scale_texture(self.sprite.textures[-1], scale)

It does some useful work on the texture. Let’s retain it, simplify, and rename it.

class ContentSpriteMaker:
    textures = TextureProvider()
    def __init__(self, resources, scale=0.5):
        self.sprite = self.textures.load_sprite(resources)
        self.sprite.set_texture(0)
        self.sprite.visible = False
        self.sprite.scale = scale_texture(self.sprite.textures[-1], scale)

And in use:

    def make_view_and_sprite(self, cell, item, sprite_list):
        sprite = ContentSpriteMaker(item.resources, item.scale).sprite
        sprite.position = cell.center_position(cell_size)
        self.content_sprites_by_cell[cell].append(sprite)
        self.keyed_sprites.add(item, sprite)
        if self.keyed_sprites[cell].visible:
            sprite.visible = True

And we’re good. Commit: Replace ContentView with ephemeral ContentSpriteMaker.

Wall of text. Let’s sum up.

Summary

In a series of a half-dozen commits (that could have been eight or ten, I think) we have removed one of the troubling extra collections in DungeonView, the content_views, replaced the content_views_by_cell with content_sprites_by_cell. We have simplified DungeonView so that the class formerly known as ContentView is an ephemeral helper named ContentSpriteMaker, and thus simplified the class formerly known as ContetnView by removing all its methods.

We moved a questionable call from the DungeonView down to the dungeon and layout, telling them to remove content when in fact they already knew to do that.

DungeonView is down to 136 lines. I’m not sure if that’s a reduction from last night or not, but it’s down ten from yesterday’s last article.

We could arguably replace ContentSpriteMaker with a method. I think I prefer it this way, since it offloads a concept and some code.

Once again we see that small changes can make large improvements. DungeonView has fewer responsibilities than it had this morning, and yet all the functionality is still there. Interesting, isn’t it?

See you next time!