Are we close enough to having the right Border classes to try to draw them? Let’s take a look at how we might do that.

It has been about 19 hours since I last looked at the border code, so naturally my recollection of the details is poor.

Comment:
Perhaps slightly more fair to myself, I’d say that I do not try to remember the details of my code, so there’s no telling whether I could remember it if I wanted to. I do remember generally how it works, but my working style here is to put what I’m thinking into the code and leave it there, and then to discover my thinking when I next review the code. Naturally, one loses a lot of detail, but it’s there in the code. When the code is particularly good, this works well. When it’s not, I suspect that remembering it isn’t much use anyway.
Observation:
I do recall that yesterday we created three classes, BorderMap, BorderList, and Border. Remembering that reminds me that the BorderMap is a map of all the cells in all the rooms of the layout; the BorderList is a list of the Border objects for a given cell, and the Border object contains a tuple for the side of the cell (1,0) for east, and so on, and a string indicating what that border should be, a wall or whatever.

We spoke yesterday about whether the little objects were worth making, since we could clearly build the equivalent thing as a dictionary of lists of tuples. I think that dictionary of list of tuples doesn’t communicate as much as BorderMap of BorderLists of Borders.

That’s one of the reasons making little objects can pay off. But I digress — squared.

This morning I want to explore using these new objects to draw the borders in our map, which is currently done with some short but ad hoc code. Let’s look at what we have now, starting with main, so as to see where to put our new objects.

def main():
    # random.seed(35)
    layout = DungeonLayout(64, 56)
    # random.seed(234)
    maker = RoomMaker(layout)
    make_diamond_in_round_room(layout, maker)
    make_a_diamond_room(layout, maker)
    make_a_round_room(layout, maker)
    number_of_rooms = random.randint(8, 8)
    for _ in range(number_of_rooms):
        make_a_cave_room(layout, maker)
        make_experimental_room(layout, maker)
    layout.ensure_connected()
    dungeon = Dungeon(layout)
    dungeon.populate()
    target = Cell(32, 28)
    options = layout.furthest_cells(target)
    chosen_cell = random.choice(options)
    dungeon.place_player_at(chosen_cell)
    view = DungeonView(dungeon)
    view.main_loop()

class DungeonView:
        def main_loop(self):
            ...
            self.screen.fill(background)
            # self.draw_grid(color)
            self.draw_rooms()
            self.draw_contents()
            self.clock.tick(60)
            pygame.display.flip()
            ...

    def draw_rooms(self):
        for room in self.dungeon.rooms:
            RoomView(self.dungeon, room).draw(self.screen)

class RoomView:
    def draw(self, screen):
        cells = set(self.room.cells)
        for cell in cells:
            self.draw_colored_cell(cell, screen)
            self.draw_border(cell, screen)

So there it is. We’ll want to define the borders in the layout, since Border is built on a layout, as we’ll see in a moment. For now, we’ll open-code this, but it’s worth noting that main is showing us that we need to do a better job of tying things together: main is very ad-hoc. I think we’ll probably work toward a Layout.finalize() method or something similar.

For now main will say this:

    ...
    layout.ensure_connected()
    layout.make_borders()
    dungeon = Dungeon(layout)
    ...

Layout will say this:

class Layout:
    def make_borders(self):
        self.border_map = BorderMap(self)

I’ll run main just to be sure we have it all wired up. Looks good. Commit: working toward drawing borders with new Borders objects.

Now we have to do some work. Here’s the current code for draw_border in RoomView:

class RoomView:
    def draw_border(self, cell, screen):
        borders = self.borders(cell)
        colors = {"open":"black", "door":"green", "wall":"red"}
        directions = {'n':(0,-1), 's':(0,1), 'e':(1,0), 'w':(-1,0)}
        lines = self.make_lines(cell)
        for direction, neighbor_offset in directions.items():
            color = colors[borders[direction]]
            self.line(*lines[neighbor_offset], color, screen)

    def borders(self, cell):
        result = dict()
        for k,v in {'n':(0,-1), 's':(0,1), 'e':(1,0), 'w':(-1,0)}.items():
            result[k] = self._border_at(cell, v)
        return result

    def _border_at(self, cell, offset):
        neighbor = self.layout.at_offset(cell.xy, offset)
        if neighbor is None:
            return "wall"
        neighbor_room = self.layout.get_room(neighbor)
        if neighbor_room is None:
            return "wall"
        elif neighbor_room == self.layout.get_room(cell):
            return "open"
        else:
            return "door"

    def make_lines(self, cell):
        cx0 = cell.x * cell_size
        cy0 = cell.y * cell_size
        cx1 = cx0 + cell_size
        cy1 = cy0 + cell_size
        bottom = ((cx0, cy1), (cx1, cy1))
        top = ((cx0, cy0), (cx1, cy0))
        left = ((cx0, cy0), (cx0, cy1))
        right = ((cx1, cy0), (cx1, cy1))
        lines = {(0, 1): bottom, (0, -1): top, (-1, 0): left, (1, 0): right}
        return lines

I notice one thing here, which is that we’re going to be returning ‘partition’ where now we return ‘door’, so I’ll add ‘partition’ to the color map:

        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}

I changed the door color to white so that if one occurs, it’ll show up as white, not green like the partitions. Running now, I get all white where the partitions go. Could change the _border_at to return partition, but no real reason to do so.

I think that rather than try to refactor this mess, I’ll fetch the new info at the top and use it. But I do like the lines method. It returns a dictionary from our side tuples to the actual coordinates of the line on that side. We can use that. Let me just try this:

class RoomView:
    def borders(self, cell):
        return self.layout.get_borders(cell)

Feature envy here but we’ll let it ride. Layout needs to know that method.

class DungeonLayout:
    def get_borders(self, cell):
        return self.border_map[cell]

A test is failing. What is it? It’s a borders check that now needs to ask the layout to build the border map. Hm, still fails. Follow nose. The test is an old one, checking the old form. We’ll mark it to skip for now. Back to work:

I scrawl this:

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            color = colors[border.border]
            self.line(*lines[side], color, screen)

I’m not at all confident with this and it isn’t clear but it’s close. Run main. Woot! It looks right:

"map with correct-seeming borders"

I almost don’t believe it. Remove the other unused methods from RoomView. Check by drawing with a different color. Feel amazed but gratified. Commit: RoomView uses Border objects.

Reflection

I was about 75% surprised when that worked. I was sure it was close, but really expected that some little mistake must be in there. Let’s look at what we have and see what we have done and how it might be improved.

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            color = colors[border.border]
            self.line(*lines[side], color, screen)

    def make_lines(self, cell):
        cx0 = cell.x * cell_size
        cy0 = cell.y * cell_size
        cx1 = cx0 + cell_size
        cy1 = cy0 + cell_size
        bottom = ((cx0, cy1), (cx1, cy1))
        top = ((cx0, cy0), (cx1, cy0))
        left = ((cx0, cy0), (cx0, cy1))
        right = ((cx1, cy0), (cx1, cy1))
        lines = {(0, 1): bottom, (0, -1): top, (-1, 0): left, (1, 0): right}
        return lines

    def line(self, start_pos, end_pos, color, screen):
        pygame.draw.line(screen, color, start_pos, end_pos)

Line should be renamed to draw_line, surely. Commit: tidying.

I don’t think I like that the Border object has a member border:

@dataclass
class Border:
    side: tuple
    border: str

The border.border just looks too odd. We could rename the class to BorderType, although that invites type as a variable name, which Python doesn’t like. How about boundary for the member name? Let’s try it.

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            color = colors[border.boundary]
            self.draw_line(*lines[side], color, screen)

Better, I think, than border.border. Let’s see about that * trick. We know what’s coming back from lines. Extract variable and edit:

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            color = colors[border.boundary]
            start, finish = lines[side]
            self.draw_line(start, finish, color, screen)

Let’s inline the draw_line.

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            color = colors[border.boundary]
            start, finish = lines[side]
            pygame.draw.line(screen, color, start, finish)
What are you doing?
What I am doing is making this code more compact, with the expectation that after I filter out all the old methods, making it as close to inline as I can get it, a better refactoring may appear … or that I’ll like it. Either could happen, but I have a feeling that there’s an improvement coming.

Reorder lines:

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            side = border.side
            start, finish = lines[side]
            color = colors[border.boundary]
            pygame.draw.line(screen, color, start, finish)

Inline.

    def draw_border(self, cell, screen):
        colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            start, finish = lines[border.side]
            color = colors[border.boundary]
            pygame.draw.line(screen, color, start, finish)

Commit.

I think it might be nice if the Border would select the color from the colors dictionary for us. Let’s see if we can make that reasonable. First let’s extract that colors list to a class variable.

class RoomView:
    boundary_colors = {"open":"black", "partition":"green", "door":"white", "wall":"red"}

    def draw_border(self, cell, screen):
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            start, finish = lines[border.side]
            color = self.boundary_colors[border.boundary]
            pygame.draw.line(screen, color, start, finish)

Now let’s ask the Border to do it for us:

    def draw_border(self, cell, screen):
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            start, finish = lines[border.side]
            color = border.select_boundary(self.boundary_colors)
            pygame.draw.line(screen, color, start, finish)

And in Border:

@dataclass
class Border:
    side: tuple
    boundary: str

    def select_boundary(self, a_dictionary):
        return a_dictionary[self.boundary]

Still good. Commit.

Would it be too much to ask Border to return the for its side? Let’s look at the view again:

class RoomView:
    def draw_border(self, cell, screen):
        borders: BorderList = self.layout.get_borders(cell)
        lines = self.make_lines(cell)
        for border in borders:
            start, finish = lines[border.side]
            color = border.select_boundary(self.boundary_colors)
            pygame.draw.line(screen, color, start, finish)

    def make_lines(self, cell):
        cx0 = cell.x * cell_size
        cy0 = cell.y * cell_size
        cx1 = cx0 + cell_size
        cy1 = cy0 + cell_size
        bottom = ((cx0, cy1), (cx1, cy1))
        top = ((cx0, cy0), (cx1, cy0))
        left = ((cx0, cy0), (cx0, cy1))
        right = ((cx1, cy0), (cx1, cy1))
        lines = {(0, 1): bottom, (0, -1): top, (-1, 0): left, (1, 0): right}
        return lines

Rename make_lines. But wait, make_lines has nothing to do with a room. It’s crying out to be a method on Cell. And where did it get cell_size? Eeek:

from params import cell_size

`params`
# dungeon parameters
screen_width = 1024
screen_height = 1024 - 128
cell_size = 16

I think we’ll find that that needs improvement before we get much further. Maybe.

Reflection

This is really good so far: the new borders went in easily and work as intended. The code just wants a bit more improvement.

At this moment, I find myself thinking, uncertain what to do. Good sign that we may need a break. But let’s see if we can firm up our thoughts a bit.

I’d like to have a function from side (0,1) for east and all that, to the coordinates of the desired line.

It appears that we consider the ‘top’ line of the cell to be at -1 in y, and the bottom line at one in y. The left line is at -1 in x, the right at 1 in x.

Let x and y be the coordinates of the cell. I think we want this:

side name from to
(0, -1) top x, y x+s, y
(0, 1) bottom x, y+s x+s, y+s
(-1, 0) left x, y x, y+s
(1, 0) right x+s, y x+s, y+s

I do want a break, but I think our notation here for side, (0, 1) meaning east and so on, isn’t really helping us. It is useful in selecting which neighbor to look at, but not for computing the coordinates of the desired line.

Maybe we should just look it up. I keep trying to think of a clever calculation on the side to provide what we need for the coordinates, but that might just be a crock.

What if the cell were more helpful. It could perhaps answer questions like top_left(size). Or we could do it in one of our views. Hm, possibly …

We’ll break now—it is bacon day—and come back in our next session with a better idea — or not.

See you then!