In For a Penny
Hello, loves!
I’ve decided to commit this and refactor. Can always reset if I have to. General rule: try always to move forward.
We’ll begin with some tests for what we have. What do we have? I don’t know, that was hours ago. Here’s just the DungeonView init, evidence that all is not well:
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.room_sprite_list = None
self.content_views: dict[Content, ContentView] = dict()
self.content_views_by_room: dict[Room, list[ContentView]] = dict()
self.content_views_by_cell: dict[Cell, list[ContentView]] = defaultdict(list)
self.room_views: dict[Room, RoomView] = dict()
self.cell_sprites: dict[Cell, Sprite] = dict()
self.illuminated_cells: set[Cell] = set()
self.content_sprite_list = None
self.dungeon_camera = None
self.dungeon_camera_bounds = None
self.scroller = None
self.scroller_camera = None
We can be pretty sure that 17 lines of instance variables is about 13 too many. So we’ll keep that in mind. Are there any of those that are no longer referenced? content_views_by_room seems not to be. Removed, commit.
What about the room_views? They’re defined but not used. Remove, commit.
These two structures seem to have some redundancy:
def __init...
self.room_sprite_list = arcade.SpriteList()
self.content_views: dict[Content, ContentView] = dict()
def setup(self):
self.setup_scroller()
self.setup_cameras()
self.room_sprite_list = arcade.SpriteList()
self.create_rooms(self.room_sprite_list)
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.room_sprite_list.append(sprite)
def on_draw(self):
self.clear()
self.scroll_dungeon_camera()
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()
Can’t we just put them in cell_sprites and draw the values? No the one is a SpriteList, which we really want to use. Let’s rename the member to dungeon_floor_sprites. Commit.
I think that’s all the redundancy I can eliminate from here. But I think there are some unused bits elsewhere.
The only caller of RoomView.illuminate is a test:
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(room)
room_view.cell_sprites = {cell: FakeSprite() for cell in cells}
room_view.illuminate(dungeon_view, Cell(12, 12), 3)
lit_cell = Cell(13, 13)
lit = room_view.cell_sprites[lit_cell]
assert lit_cell in dungeon_view.illuminated_cells
assert lit.visible == True
unlit_cell = Cell(9, 9)
assert not unlit_cell in dungeon_view.illuminated_cells
unlit = room_view.cell_sprites[unlit_cell]
assert unlit.visible == False
Ugly. I think we’ll remove RoomView.illuminate and let this test be our lead-in to actual testing. Sure enough, the test fails.
The good news is, I have a test that is working and checking the new code:
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)
layout.ensure_connected()
layout.make_passages()
layout.make_borders()
dungeon.just_set_player_position(Cell(12,12))
dungeon_view.create_rooms(arcade.SpriteList())
dungeon_view.illuminate_around_dot()
lit_cell = Cell(13, 13)
lit = dungeon_view.cell_sprites[lit_cell]
assert lit_cell in dungeon_view.illuminated_cells
assert lit.visible == True
unlit_cell = Cell(9, 9)
assert not unlit_cell in dungeon_view.illuminated_cells
unlit = dungeon_view.cell_sprites[unlit_cell]
assert unlit.visible == False
The bad news is that it is capital U Ugly. But it is a place to stand, and from which to do better.
I’m calling this progress. The DungeonView is simpler and it has an actual test that is exercising most of the new hammered-in code. So we can clean up this test, refactor the code, and move toward better.
And that is the name of the game. See you next time!