Hello, loves!

We need to deal with the illumination issue in our breadcrumbs feature. Should be easy. Need to quell a rumor first.

I want to deal right now with the craven rumor that I am a member of the so-called Illuminati. First of all, the Illuminati organization does not exist. Furthermore, if they did exist, I would never have joined them. In addition, even if I had joined them, I would have left them long ago. And anyway, I’m sorry now.

With that out of the way, let’s turn our attention to the matter of Illumin illuminating, sorry, for some reason I often mistakenly capitalize words starting with those letters, illuminating the dungeon. Specifically, when we place new content, if the cell it is placed in is already lighted, we’ll want to set it visible.

Unfortunately, unless I miss my guess, the program does not currently know what cells have seen the light. We just set their sprites to visible when they are illuminated. So we’ll need to add a bit of mechanism. Should be no problem.

Also fortunately, there is a feature that we’d like to have, and since we need to visit that code, we’ll see about doing that too. The feature is: “Normally cells around Dot illuminate out to some radius, probably 3, and stay that way. Let’s change that so that if she is carrying ‘a brilliant torch’ in inventory, the entire room illuminates when she enters”.

We begin but looking to see what code and tests might support illumination.

class ContentView:
    def illuminate(self, dot, range):
        if dot.manhattan_distance(self.cell) < range:
            self.sprite.visible = True

Seeing this makes me suspect that we’re going to encounter some undesirable duplication. We’ll deal with it if so.

class DungeonView:
    def illuminate(self):
        dot = self.dungeon.player_cell
        room = dot.room
        view = self.room_views[room]
        view.illuminate()
        for content_view in self.content_views_by_room[room]:
            content_view.illuminate(dot, range=4)

There must be something in RoomView:

class RoomView:
    def illuminate(self):
        dot = self.dungeon.player_cell
        for cell, sprite in self.cell_sprites.items():
            if dot.manhattan_distance(cell) < 4:
                sprite.visible = True

There’s that duplication. We see here why even the simplest duplication is suspicious. We cannot change the illumination logic, even the illumination distance, without changing code in at least two places. That can only lead to trouble.

We forage a bit further.

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

Hm it seems to me that this is a bit much. We surely don’t need to illuminate on every draw cycle. Illumination will only change when Dot moves. I think I see why I may have taken this expensive shortcut:

class DungeonView:
        if symbol == arcade.key.RIGHT:
            self.dungeon.move_player(Direction.EAST)
        elif symbol == arcade.key.LEFT:
            self.dungeon.move_player(Direction.WEST)
        elif symbol == arcade.key.UP:
            self.dungeon.move_player(Direction.NORTH)
        elif symbol == arcade.key.DOWN:
            self.dungeon.move_player(Direction.SOUTH)

My first reaction here is to extract a method and move the illuminate to that method. But I find myself thinking ahead, what if there is some kind of glowing creature who illuminates areas she enters? And then I think YAGNI, and anyway that illumination will have different logic, so let’s proceed with the extraction.

I note that there seem to be no tests for illumination, and I can see why that would be, since all it really does is fiddle with sprite.visible. We could probably write a test, but just now I’m not even sure what I want.

Let’s do the extract and then look around further.

    def move_player(self, direction: Direction):
        self.dungeon.move_player(direction)
        self.illuminate()

    def on_key_press(self, symbol: int, modifiers: int) -> bool | None:
        if self.key_lock is not None:
            return
        self.key_lock = symbol
        if symbol == arcade.key.RIGHT:
            self.move_player(Direction.EAST)
        elif symbol == arcade.key.LEFT:
            self.move_player(Direction.WEST)
        elif symbol == arcade.key.UP:
            self.move_player(Direction.NORTH)
        elif symbol == arcade.key.DOWN:
            self.move_player(Direction.SOUTH)

I even remembered to illuminate after the move. I did have to do the extraction manually, PyCharm couldn’t seem to figure out what I wanted. No biggie.

A test breaks. It’s using a fake Dungeon to check whether keystrokes are working. I do a bad thing, because I can’t think of a good thing. I made the fake dungeon return None for Dot’s cell, and did this:

    def illuminate(self):
        dot = self.dungeon.player_cell
        if not dot: return # crock to allow a test to run
        room = dot.room
        view = self.room_views[room]
        view.illuminate()
        for content_view in self.content_views_by_room[room]:
            content_view.illuminate(dot, range=4)

We skip out if there is no Dot. Not entirely unreasonable except that there always is.

Illumination still works as before. Well, after I remember to illuminate once at the beginning:

class DungeonView:
    def run(self):
        self.setup()
        self.dungeon.run()
        self.window.show_view(self)
        self.illuminate()
        arcade.run()

I think we’ll write a couple of tests. In one we’ll create a room and check that we illuminate only part of it, in the other we’ll give Dot a brilliant torch and check that we illuminate all of it.

The thing is, as things stand, these tests will be harder to write than the code. All that layout dungeon room dungeon view room view stuff.

Let’s see if we can find a way to do something simpler.

Here is the RoomView illuminate again:

class RoomView:
    def illuminate(self):
        dot = self.dungeon.player_cell
        for cell, sprite in self.cell_sprites.items():
            if dot.manhattan_distance(cell) < 4:
                sprite.visible = True

If we were to populate a RoomView’s cell_sprites dictionary with something reasonable, maybe we could just test directly.

We’ll begin by extracting a method that receives a cell to illuminate around, so that we don’t have to have a dungeon instance. Ah, but we will also need to know whether she has the brilliant torch. Let’s deal separately with that.

class RoomView:
    def illuminate(self):
        dot = self.dungeon.player_cell
        self.illuminate_around(dot)

    def illuminate_around(self, cell):
        for cell, sprite in self.cell_sprites.items():
            if cell.manhattan_distance(cell) < 4:
                sprite.visible = True

Ah, we should add a parameter for the distance. Change Signature.

class RoomView:
    def illuminate(self):
        dot = self.dungeon.player_cell
        self.illuminate_around(dot, 4)

    def illuminate_around(self, cell, distance):
        for cell, sprite in self.cell_sprites.items():
            if cell.manhattan_distance(cell) < distance:
                sprite.visible = True

OK, now I think we can write a test using a fake sprite.

class FakeSprite:
    def __init__(self):
        self.visible = False

class TestContentView:
    def test_room_view_illuminate(self):
        layout = DungeonLayout(20,20)
        dungeon = Dungeon(layout)
        cells = [Cell(x,y) for x in range(9,15) for y in range(9,15)]
        room = Room(cells, layout)
        layout.add_room(room)
        view = RoomView(dungeon, room)
        view.cell_sprites = {cell: FakeSprite() for cell in cells}
        view.illuminate_around(Cell(12, 12), 3)
        lit = view.cell_sprites[Cell(13,13)]
        assert lit.visible == True
        unlit = view.cell_sprites[Cell(9,9)]
        assert unlit.visible == False

This test passes once I fix the implementation:

class RoomView:
    def illuminate_around(self, center, distance):
        for cell, sprite in self.cell_sprites.items():
            if cell.manhattan_distance(center) < distance:
                sprite.visible = True

OK. Let’s test illuminate_room into existence. We’ll need it shortly, for the brilliant torch feature. It’s premature to do it, but only a little.

    def test_full_room_illuminate(self):
        layout = DungeonLayout(20,20)
        dungeon = Dungeon(layout)
        cells = [Cell(x,y) for x in range(9,15) for y in range(9,15)]
        room = Room(cells, layout)
        layout.add_room(room)
        view = RoomView(dungeon, room)
        view.cell_sprites = {cell: FakeSprite() for cell in cells}
        view.illuminate_room()
        lit = view.cell_sprites[Cell(13,13)]
        assert lit.visible == True
        unlit = view.cell_sprites[Cell(9,9)]
        assert unlit.visible == True

That demands this:

class RoomView:
    def illuminate_room(self):
        for sprite in self.cell_sprites.values():
            sprite.visible = True

However. We really want to know which cells are illuminated, and I think we want to know that in the Dungeon. Let’s figure out what we need.

In Dungeon, we have this:

class Dungeon:
    def show_path_to(self, item_name, layout):
        my_resources = '/Users/ron/Desktop/DungeonTiles/png/objects/'
        item = ContentFactory().decor(name="skel", resource=my_resources + 'Skeleton1.png', scale=0.5)
        path = self.find_path_to(item_name)
        for cell in path:
            self.place_content_at(cell, item)
            layout.make_view_and_sprite(cell, item)

class DungeonLayout:
    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_room[cell.room].append(view)
        self.content_sprite_list.append(view.sprite)

We need to illuminate added content if the cell we’re adding it to is illuminated. But we don’t know. Is illumination a layout property or a view property? Pretty clearly a view property.

Let’s enhance one of those tests.

    def test_room_view_illuminate(self):
        layout = DungeonLayout(20,20)
        dungeon = Dungeon(layout)
        cells = [Cell(x,y) for x in range(9,15) for y in range(9,15)]
        room = Room(cells, layout)
        layout.add_room(room)
        room_view = RoomView(dungeon, room)
        room_view.cell_sprites = {cell: FakeSprite() for cell in cells}
        room_view.illuminate_around(Cell(12, 12), 3)
        lit_cell = Cell(13, 13)
        lit = room_view.cell_sprites[lit_cell]
        assert room_view.is_lit(lit_cell)
        assert lit.visible == True
        unlit_cell = Cell(9, 9)
        assert not room_view.is_lit(unlit_cell)
        unlit = room_view.cell_sprites[unlit_cell]
        assert unlit.visible == False

I ask the room_view is_lit because I have access to it. Unfortunately, RoomView is created on a Dungeon, not a DungeonView. That’s just wrong, especially since DungeonView creates them. Change Signature. Quickly done, just had to change a few tests. I hate the tests of these views, something needs to be done.

    def test_room_view_illuminate(self):
        layout = DungeonLayout(20,20)
        dungeon = Dungeon(layout)
        dungeon_view = DungeonView(dungeon, True)
        cells = [Cell(x,y) for x in range(9,15) for y in range(9,15)]
        room = Room(cells, layout)
        layout.add_room(room)
        room_view = RoomView(dungeon_view, room)
        room_view.cell_sprites = {cell: FakeSprite() for cell in cells}
        room_view.illuminate_around(Cell(12, 12), 3)
        lit_cell = Cell(13, 13)
        lit = room_view.cell_sprites[lit_cell]
        assert room_view.is_lit(lit_cell)
        assert lit.visible == True
        unlit_cell = Cell(9, 9)
        assert not room_view.is_lit(unlit_cell)
        unlit = room_view.cell_sprites[unlit_cell]
        assert unlit.visible == False

We have one test failing, the one above, for want of is_lit. Add to RoomView:

class RoomView:
    def is_lit(self, cell):
        return self.dungeon_view.is_lit(cell)

And in DungeonView, Wishful Thinking:

    def is_lit(self, cell):
        return cell in self.illuminated_cells

Of course there is no such thing as illuminated_cells, so the test is telling me to have it.

class DungeonView(arcade.View):
    def __init__(self, dungeon, testing=False):
        ...
        self.illuminated_cells = set()
        ...

Test still fails because we’re not ever marking anything as lit.

class RoomView:
    def illuminate_around(self, center, distance):
        for cell, sprite in self.cell_sprites.items():
            if cell.manhattan_distance(center) < distance:
                sprite.visible = True

    def illuminate_room(self):
        for sprite in self.cell_sprites.values():
            sprite.visible = True

These get changed:

    def illuminate_around(self, center, distance):
        for cell, sprite in self.cell_sprites.items():
            if cell.manhattan_distance(center) < distance:
                self.dungeon_view.light_up(cell)
                sprite.visible = True

    def illuminate_room(self):
        for cell, sprite in self.cell_sprites.items():
            self.dungeon_view.light_up(cell)
            sprite.visible = True

I used the method name light_up because illuminate is in use and it was the only thing I was sure I could use. We’ll see about normalizing names later.

class DungeonView:
    def light_up(self, cell):
        self.illuminated_cells.add(cell)

Tests pass. Let’s commit this, it’s nearly good. Commit: working on illumination.

Finally, we can check the visibility and set it properly.

class Dungeon:
    def show_path_to(self, item_name, dungeon_view):
        my_resources = '/Users/ron/Desktop/DungeonTiles/png/objects/'
        item = ContentFactory().decor(name="skel", resource=my_resources + 'Skeleton1.png', scale=0.5)
        path = self.find_path_to(item_name)
        for cell in path:
            self.place_content_at(cell, item)
            dungeon_view.make_view_and_sprite(cell, item)

class DungeonView:
    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_room[cell.room].append(view)
        self.content_sprite_list.append(view.sprite)
        if self.is_lit(cell):
            view.sprite.visible = True

And that works, in the sense that it displays our skull breadcrumbs wherever the tiles are already lit.

map showing skull breadcrumbs in illuminated cells

I was interrupted in the middle of all this for a long phone call with an old crony, so time is up, and we’re at a decent stopping point. Commit: breadcrumb path is still short-cutting but displays properly.

Summary

On the bright side, all the work came down to tiny one line methods, and that’s a good sign. However, the tests are still too hard to set up, involving building big structures of layout, dungeon, dungeon view, room view, cell, ad infinitum. And Some of these classes have too many instance variables, a pretty solid sign that they are not cohesive:

class Dungeon(PubSubProtocol):
    def __init__(self, layout):
        self.layout = layout
        self.player_cell = None
        self.player_inventory = []
        self.contents= defaultdict(list)
        self.announcements = []
        self.pub_sub = PubSub()
        self.flood_list = SpriteList()

class DungeonView(arcade.View):
    def __init__(self, dungeon, testing=False):
        if not testing:
            super().__init__()
        self.dungeon = dungeon
        self.subscribe(dungeon)
        self.key_lock = None
        self.room_sprite_list = None
        self.content_views = dict()
        self.content_views_by_room = dict()
        self.room_views = dict()
        self.illuminated_cells = set()
        self.current_room = None
        self.content_sprite_list = None
        self.dungeon_camera = None
        self.dungeon_camera_bounds = None
        self.scroller = None
        self.scroller_camera = None
        self.door_texture = arcade.load_texture('/Users/ron/Desktop/DungeonTiles/png/objects/door3/1.png')

DungeonView, in particular, knows way too many things. This arrangement is why we are forced to create a nest of object just to test simple things like whether a cell is lit.

This sort of thing happens. Our design seems reasonable, we make what seem like reasonable changes, we hang one more thing on the class and next thing you know it’s gone all wonky. This is the sort of thing that makes programmers say “we have to rewrite the whole thing”. I have said that quite often. I think that once, our of perhaps five or more attempts, we actually managed to rewrite the whole thing before they cancelled the project. I’d never do that again. Instead, I’d try to stay ahead of this bad a situation, by paying attention and refactoring all along … and when, as seems to have happened here, I didn’t notice as soon as I should have … I’d still refactor slowly toward better. And that’s what we’ll need to do here.

The trick, and it can be a bit painful, is not to attack the refactoring as a big effort, but to proceed with small inadequate improvements, too little too late. And, unless free time suddenly materializes, we stay away from the parts of the code that are not really bothering anyone. We improve the code where the features take us.

I have one tentative but somewhat firm notion for going forward. I suspect we should introduce a CellView object, to be kept in the RoomViwe objects, which should be kept in the DungeonView object. It is tempting to have the little view objects know their parent, but I’ve learned to avoid that scheme where I can, instead passing the parent down where it’s needed, but we’ll see. If we go with that kind of scheme, we might have:

  • a keyboard object hat handles all the keyboard work
  • a collection of RoomViews, each knowing a collection of CellView objects, each knowing a cell (index) and whatever sprite info is needed.
  • Dungeon probably has to know Rooms and Rooms know Cells
  • We might have to break down and allow Cells to know something, such as their contents. I’d rather not, though.
  • Cameras off in a separate object
  • Scroller and Announcements ditto

We may have to draw a diagram or two. Or maybe we’ll just smush things around as seems appropriate. We’re not likely to go seriously wrong.

Bottom line, we have some progress toward our breadcrumb feature, and the method for illuminating the whole room is there to be used, we just didn’t have time to hook it up.

Stuff happens, old cronies call up. It’s all good. See you next time!