Hello, loves!

If we were to combine our formerly three now two sprite lists into one, would we lose some duplicated code?

I think we might. Let’s find out.

There are two sprite-oriented collections that the Maker and View use:

    def __init__(self, dungeon):
        self.setup_assets()
        self.dungeon = dungeon
        self.pub_sub = dungeon.pub_sub
        self.keyed_sprites = KeyedSpriteList(arcade.SpriteList())
        self.content_sprites_by_cell: dict[Cell, list[Sprite]] = defaultdict(list)
        self.setup()

The KeyedSpriteList maintains a list of all sprites, which we draw all at once, and a list indexed by cell — or, I think, by content item — used to look up individual sprites.

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)

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

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

The content_sprites_by_cell is a dictionary from cell to a list of sprites representing all the content sprites currently in that cell.

We use that list to illuminate contents when we illuminate a cell:

class DungeonView:
    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

My cunning plan, such as it is, is to move that collection inside the keyed_sprites which should, I think bring this method inside the class:

class DungeonView # and class DungeonViewMaker:
    def make_view_and_sprite(self, cell, item):
        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

We have a test file from the KSL, let’s write a test for the new capability.

    def test_add_content_by_cell(self):
        keyed = KeyedSpriteList(SpriteList())
        keyed.add_content('cell_a', 'sprite_A')
        keyed.add_content('cell_a', 'sprite_B')
        contents = keyed.content_at('cell_a')
        assert len(contents) == 2

Add ‘em, get ‘em back. Easy enough:

class KeyedSpriteList:
    def add_content(self, cell, sprite):
        self.content_by_cell[cell].append(sprite)

    def content_at(self, cell):
        return self.content_by_cell[cell]

Green. Commit: enhancing KSL for contents by cell.

Now, I kind of wish I had looked into this earlier, can we rig things up so as to ultimately push that make_view method inside KSL? Not sure but we can certainly change things to use it. First modify both those methods:

No, wait. We would like our new add_content to add the sprite to the main list as well. Enhance the test. Ah. For that to work I need to create real sprites. No, we can just test against an ordinary list.

    def test_add_content_by_cell(self):
        keyed = KeyedSpriteList(list())
        keyed.add_content('cell_a', 'sprite_A')
        keyed.add_content('cell_a', 'sprite_B')
        contents = keyed.content_at('cell_a')
        assert len(contents) == 2
        assert len(keyed.sprite_list) == 2

And …

class KeyedSpriteList:
    def add_content(self, cell, sprite):
        self.content_by_cell[cell].append(sprite)
        self.sprite_list.append(sprite)

Test passes, commit again.

Now then let’s look at the make method:

    def make_view_and_sprite(self, cell, item):
        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

Are we ever actually fetching sprites by the ‘item’ handle? Doesn’t matter, we need them in the main sprite list.

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

Changed to use the add_content version. Curiously, content stops working, I’m not sure why. Let’s go fix up the lines using the old dictionary.

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

And I think we have to remove content differently now:

Bah! This isn’t right. Reset.

I’m not clear on how the content_sprites_by_cell is used.

OK … I’ve got it working. Hold on while I clean it up a bit.

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

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

    def add_content(self, item, cell, sprite):
        self.content_by_cell[cell].append(sprite)
        self.add(item, sprite)

    def content_at(self, cell):
        return self.content_by_cell[cell]

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

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

And in both classes the new make:

class DungeonView:
    def make_view_and_sprite(self, cell, item):
        sprite = ContentSpriteMaker(item.resources, item.scale).sprite
        sprite.position = cell.center_position(cell_size)
        self.keyed_sprites.add_content(item, cell, sprite)
        if self.keyed_sprites[cell].visible:
            sprite.visible = True

And the new usage bit:

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

And we’re green and everything works. Commit: removed contents_sprite_by_cell instance variable.

Now can we move that make method inside the KSL?

class KeyedSpriteList:
    def make_view_and_sprite(self, cell, item):
        sprite = ContentSpriteMaker(item.resources, item.scale).sprite
        sprite.position = cell.center_position(cell_size)
        self.add_content(item, cell, sprite)
        if self[cell].visible:
            sprite.visible = True

And in DungeonView, forward to it:

class DungeonView:
    def make_view_and_sprite(self, cell, item):
        self.keyed_sprites.make_view_and_sprite(cell, item)

And in the DungeonViewMaker:

class DungeonViewMaker:
    def create_content_lists(self):
        for cell, content in self.dungeon.layout.contents.items():
            for item in content:
                self.keyed_sprites.make_view_and_sprite(cell, item)

Test. Good. Commit: moved make_view_and_sprite to KeyedSpriteList.

Summary

So, what have we wrought?

DungeonView is down to 93 lines, a new low. The Maker is down to 43 lines. KeyedSpriteList is 35 lines, up from maybe 27 or 28. And we eliminated an instance variable from each of Maker and View, and eliminated passing that variable back and forth, by enclosing it in KeyedSpriteList.

I think that’s all good. KeyedSpriteList now embodies all the ways we access sprites, by cell, by item, and by a list associated with the cell. From the outside, it’s fairly reasonable, but we should look around and see whether we should move more sprite-making inside it, or what. Possibly, that doesn’t belong there, although the fact that it removed a method from two other classes makes me think it’s better if not perfect.

I am slightly troubled by the sprite_dict being keyed by item or by cell, as a mixed-type of that kind doesn’t seem to me to have any semantic coherence. (That is, it doesn’t really make sense.) But it works fine externally, so maybe there’s something inside that class that would make it better.

At this writing, I think the code is better. I’m sure that it could be better still, and I’m somewhat confident that someday it will be.

May we all live so long, and in peace. See you next time!