Recording some small changes.

I wasn’t going to mention these in print but I think one might be barely worth it. I started by finding and renaming the two remaining uses of “owner” where we now have “room”.

Here’s the one that I think may be worth mentioning: We are directly setting the room member in cells, both via take_xy in CellSpace and elsewhere in tests.

class CellSpace:
    # kind of a convenience method for tests? any real use?
    def take_xy(self, x, y, room):
        cell = self.at(x, y)
        if cell is None:
            raise IndexError
        cell.take(room)

class Cell:
    def take(self, room):
        self.room = room

Let’s try a new concept, assign to room. I probe around and find a soft spot. There are no senders of CellSpace.take. Remove it, green, commit tidying. Right above that I find this:

    # kind of a convenience method for tests? any real use?
    def take_xy(self, x, y, room):
        cell = self.at(x, y)
        if cell is None:
            raise IndexError
        cell.take(room)

All the senders of that are tests. Here is an example:

    def test_border_characterization(self):
        space = CellSpace(5, 5)
        space.take_xy(3,3, 1)
        space.take_xy(2,3, 1)
        space.take_xy(4,3, 2)
        c_33 = space.at(3,3)
        borders = c_33.borders()
        assert len(borders) == 4
        assert borders["n"] == "wall"
        assert borders["e"] == "door"
        assert borders["w"] == "open"
        assert borders["s"] == "wall"

I can multi-line edit these not to do that:

    def test_border_characterization(self):
        space = CellSpace(5, 5)
        space.at(3,3).take(1)
        space.at(2,3).take(1)
        space.at(4,3).take(2)
        c_33 = space.at(3,3)
        borders = c_33.borders()
        assert len(borders) == 4
        assert borders["n"] == "wall"
        assert borders["e"] == "door"
        assert borders["w"] == "open"
        assert borders["s"] == "wall"

That makes me optimistic. Let’s first change Cell.take to assign_to_room. Tests all path. Commit: rename method.

class Cell:
    def assign_to_room(self, room):
        self.room = room

Now I’m going to go after those other uses of take_xy and get rid of it. Another multi-line edit does the trick. Remove method. Green. Commit remove take_xy.

There is another problematical method in CellSpace.

    # kind of a convenience method for tests? any real use?
    def has_cell(self, x, y):
        cell = self.at(x, y)
        return cell is not None and cell.is_available

There are just a few uses in tests, easily multi-edited:

    def test_available(self):
        space = CellSpace(5, 5)
        assert space.at(3,3).is_available
        space.at(3,3).room = 666
        assert not space.at(3,3).is_available
        assert space.at(3,3).is_in_a_room

And a similar change in another test, which I’ll spare you. Green. Commit: remove has_cell.

Here’s all that’s left of CellSpace:

class CellSpace:
    def __init__(self, max_x, max_y):
        self.cells = set()
        self.cell_array = []
        self.width = max_x
        self.height = max_y
        for x in range(max_x):
            col = []
            self.cell_array.append(col)
            for y in range(max_y):
                cell = Cell(x, y, self)
                self.cells.add(cell)
                col.append(cell)

    def at(self, x, y) -> Cell|None:
        if 0 <= x < self.width and 0 <= y < self.height:
            return self.cell_array[x][y]
        else:
            return None

    def random_available_cell(self):
        return random.choice(list(self.cells))

Is that random available cell even used? I think so, but it is clearly wrong if it is, since all cells are always in there. Yes. This is a bug!!

def main():
    bank = CellSpace(64, 56)
    # random.seed(234)
    dungeon = Dungeon()
    number_of_rooms = random.randint(12, 12)
    for _ in range(number_of_rooms):
        size = random.randint(90, 90)
        origin = bank.random_available_cell()
        room = Room(bank, size, origin)
        dungeon.add_room(room)
    for room_1, room_2 in zip(dungeon.rooms, dungeon.rooms[1:]):
        dungeon.find_path_between_rooms(bank, room_1, room_2)
    view = DungeonView(dungeon)
    view.main_loop()

This explains the occasional size 1 room in the middle of another room. We have basically snatched that cell out from under the other room. And that raises another issue which is that we were able to do that. Once a room is assigned, we really shouldn’t be able to assign it again. One thing at a time:

class CellSpace:
    def __init__(self, max_x, max_y):
        self.cells: list[Cell] = list()
        self.cell_array = list()
        self.width = max_x
        self.height = max_y
        for x in range(max_x):
            col = []
            self.cell_array.append(col)
            for y in range(max_y):
                cell = Cell(x, y, self)
                self.cells.append(cell)
                col.append(cell)

    def random_available_cell(self):
        return random.choice([cell for cell in self.cells if cell.is_available])

Might as well just let it be a list to begin with since we have to search it anyway. It’s kind of a pain, and we could keep track of cells as they are assigned, but I think that’s out of scope for right now. Green. Commit: fix bug in random_available_cell.

Let’s see if I can properly type-hint the cell_array.

self.cell_array: list[list[Cell]] = list()

That seems to work and I think it’s just that simple. Commit: type hint.

I think we’ll pause for the day. We have simplified CellSpace to the bare minimum. Rooms are now “assigned” rather than “taken”. I think we could go further here, perhaps linking Room and CellSpace more intimately, but I think we may want to explore some different room generation ideas, so we’ll stay away from refining that relationship too closely.

Small improvements, several commits and a reduction in code. Test setup is slightly more tricky, but I think more clear and we got rid of three test-only methods.

A decent little session. See you next time!