A short session Friday, another Saturday, and we have Cell disconnected from it container. A triumph of small steps!

Friday AM

This morning’s session will be short, as I have to leave at about 0800 for an appointment. Might work when I return, might not. What can we do in a half hour or less?

I think there is just one use of current, the shim from Cell to DungeonLayout that has allowed us to remove the space / layout member from the instances, as we move capability from Cell to Layout or elsewhere, inch by inch, step by step, Niagara Falls! The current use of current is here:

class Cell:
    @property
    def neighbors(self):
        from dungeonlayout import current
        return current.neighbors(self)

We’re green and clean. That method claims to have seven users, one of them in Cell itself, one in DungeonLayout(!), one in Flooder, the others in tests.

Fix the one in DungeonLayout.

    def path_map(self, source, can_use, randomness=0.0):
        reached: dict[Cell, Cell|None] = {source: None}
        for c, _ in (Flooder(layout=self, origin=source)
                .select(can_use)
                .randomness(randomness)
                .flood()):
            for neighbor in self.neighbors(c):
                if neighbor not in reached and can_use(neighbor):
                    reached[neighbor] = c
        return reached

The for used to refer to c.neighbors. Fixed, green, commit: remove use of cell.neighbors.

OK, Flooder:

class Flooder:
    def _get_neighbors(self, next_cell):
        if random.random() < self._randomness:
            return next_cell.random_neighbors
        return next_cell.neighbors

Ah there are two references to cell methods. Fix the second:

class Flooder:
    def _get_neighbors(self, next_cell):
        if random.random() < self._randomness:
            return next_cell.random_neighbors
        return self._layout.neighbors(next_cell)

Do we have random_neighbors in the Layout? I’ll just forward the message and see what breaks. I’m pretty sure the method is not there, but soon it will be.

class Flooder:
    def _get_neighbors(self, next_cell):
        if random.random() < self._randomness:
            return self._layout.random_neighbors(next_cell)
        return self._layout.neighbors(next_cell)

Half a dozen tests fail. We provide:

class DungeonLayout:
    def random_neighbors(self, cell):
        neighbors = self.neighbors(cell)
        return random.sample(neighbors, len(neighbors))

Green. Commit. Should have committed after the first change, sorry.

I think we can remove Cell.random_neighbors now. Tried, green. Commit: remove method.

At this point I see that available_neighbors has only a few uses, so I’ll divert for a look at that.

No, change isn’t just the quick patch, some of RoomMaker’s collector classes still don’t have the layout available.

We’re out of time for now. I’ll return anon.

Anon = Saturday

Right. Yesterday got away from me. What’s next?

I think the thing will be to finish up the classes in RoomMaker. The rule will be, we pass the layout to all of RoomMaker’s subordinate Collector classes unconditionally. The consistency will be well worth any savings that could come from not passing it. I’m on a clean commit, my first steps will be to pass in the Layout and provide the standard __init__. Here’s one such, before:

class ExperimentalCellCollector:
    def __init__(self):
        self.cells:list[Cell] = []
        self.growth_candidates:list[Cell] = []

We’ll just use the Change Signature refactoring in PyCharm, to get:

class ExperimentalCellCollector:
    def __init__(self, layout):
        self.layout = layout
        self.cells:list[Cell] = []
        self.growth_candidates:list[Cell] = []

PyCharm finds and fixes our only call:

class RoomMaker:
    def experimental(self, *, number_of_cells: int, origin: Cell, **kwargs):
        builder = ExperimentalCellCollector(self.layout)
        cells: list[Cell] = builder.build(number_of_cells=number_of_cells, origin=origin)
        return Room(cells)

We’re green. Commit: add layout parm to ExperimentalCellCollector.

The method we want to fix is this one:

class ExperimentalCellCollector:
    def find_adjacent_cell(self, previous):
        if random.random() < 1.0:
            available = previous.available_neighbors
            if available:
                return choice(available)
        for cell in sample(self.growth_candidates,
                           len(self.growth_candidates)):
            available = cell.available_neighbors
            if available:
                return choice(available)
            else:
                self.growth_candidates.remove(cell)
        return None

it’s those calls to available_neighbors that we’re after. They are to be changed thus:

    def find_adjacent_cell(self, previous):
        if random.random() < 1.0:
            available = self.layout.available_neighbors(previous)
            if available:
                return choice(available)
        for cell in sample(self.growth_candidates,
                           len(self.growth_candidates)):
            available = self.layout.available_neighbors(cell)
            if available:
                return choice(available)
            else:
                self.growth_candidates.remove(cell)
        return None

Green and main runs. Commit: ExperimentalCellCollector no longer references Cell.available_neighbors.

That seems to be the only Collector that didn’t already accept and use layout. We’ll find out soon in any case, when we remove the method from Cell. Let’s see who else may be using it.

PyCharm thinks that RoomMaker might be using it, but we know better. Remove the method to be sure.

All good. Commit: remove Cell.available_neighbors.

Our next target is Cell.neighbors:

class Cell:
    @property
    def neighbors(self):
        from dungeonlayout import current
        return current.neighbors(self)

This is the only remaining reference to our current shim that allows Cell to see the layout. Our mission, of course, is to remove its need to do that. We look for senders of neighbors. There are three tests and no production code. Fix:

    def test_neighbors(self):
        layout = DungeonLayout(5, 5)
        cell = layout.at(2,3)
        neighbors = layout.neighbors(cell)
        assert len(neighbors) == 4
        for n in [layout.at(1, 3), layout.at(3, 3), layout.at(2, 2), layout.at(2, 4)]:
            assert n in neighbors

Green. Commit: remove use of Cell.neighbors.

    def test_edge_neighbors(self):
        layout = DungeonLayout(2, 3)
        cell = layout.at(0,0)
        neighbors = layout.neighbors(cell)
        assert len(neighbors) == 2

Green again. Commit again.

The last one is in a rather complex test:

    def test_build(self):
        random.seed(234)
        layout = DungeonLayout(64, 56)
        size = 100
        origin = layout.at(32, 28)
        room = RoomMaker(layout).cave(number_of_cells=size, origin=origin)
        assert len(room.cells) == 100
        assert origin.room == room
        assert origin in room
        self.verify_contiguous(room)

    def verify_contiguous(self, room):
        start = room.cells[0]
        cell_set = set(room.cells)
        self.remove_cell_and_neighbors(start, cell_set)
        assert len(cell_set) == 0

    def remove_cell_and_neighbors(self, cell, cell_set):
        cell_set.remove(cell)
        self.remove_neighbors(cell, cell_set)

    def remove_neighbors(self, cell, cell_set):
        neighbors = cell.neighbors
        for neighbor in neighbors:
            if neighbor in cell_set:
                self.remove_cell_and_neighbors(neighbor, cell_set)

The right thing to do is to pass the layout right on down. We can do it by changing signature starting with the verify_conguous method.

    def test_adding_rooms(self):
        layout = DungeonLayout(100, 100)
        assert layout.number_of_rooms == 0
        size = 10
        origin = layout.at(32, 28)
        room = RoomMaker(layout).cave(number_of_cells=size, origin=origin)
        layout.add_room(room)
        assert layout.number_of_rooms == 1

    def verify_contiguous(self, layout, room):
        start = room.cells[0]
        cell_set = set(room.cells)
        self.remove_cell_and_neighbors(start, cell_set)
        assert len(cell_set) == 0

Green. Commit: passing layout down …

Next step didn’t work alone, had to do two:

    def remove_cell_and_neighbors(self, layout, cell, cell_set):
        cell_set.remove(cell)
        self.remove_neighbors(layout, cell, cell_set)

    def remove_neighbors(self, layout, cell, cell_set):
        neighbors = cell.neighbors
        for neighbor in neighbors:
            if neighbor in cell_set:
                self.remove_cell_and_neighbors(layout, neighbor, cell_set)

Those two methods call each other. Had to add layout to each. Easily done. Commit.

I believe we have removed all the references to Cell.neighbors. Remove it to find out for sure.

No, by golly, a test failed! What is it? Oh, silly me, I forgot to use layout in the method that needed it. Got ahead of myself. Put Cell.neighbors back. Fix up the test:

    def remove_neighbors(self, layout, cell, cell_set):
        neighbors = layout.neighbors(cell)
        for neighbor in neighbors:
            if neighbor in cell_set:
                self.remove_cell_and_neighbors(layout, neighbor, cell_set)

Green. Commit: use the layout, tiny fool!

Now remove the method again. Green. Commit: remove Cell.neighbors method.

Now look for references to our shim, in DungeonLayout:

current: DungeonLayout = DungeonLayout(1,1)

class DungeonLayout:
    deltas = [(-1, 0), (1, 0), (0, -1), (0, 1)]

    def __init__(self, max_x=10, max_y=10):
        global current
        current = self
        self.cells = dict()
        for x in range(max_x):
            for y in range(max_y):
                self.cells[(x,y)] = Cell(x, y)
        self.rooms = []

Those are the only references, right there. Remove them all.

class DungeonLayout:
    deltas = [(-1, 0), (1, 0), (0, -1), (0, 1)]

    def __init__(self, max_x=10, max_y=10):
        self.cells = dict()
        for x in range(max_x):
            for y in range(max_y):
                self.cells[(x,y)] = Cell(x, y)
        self.rooms = []

Green. main works. Commit: Cell no longer references the Layout, Callooh! Callay!

Cheerful Reflection

This most recent effort started with Wild Idea, where it occurred to me to try to turn Cell into nothing but an xy tuple. The major part of that involved disconnecting cells from each other, so that they would not have access to the CellSpace, or to the DungeonLayout that replaced it. The process has extended from April 27th to May 2nd, and was done in 50 commits, each one leaving the tests all green and the program working. Those 50 commits, done over 5 days, could have been done over 50 days or 200, if need be, with any number of other changes going on in between, features or whatever.

Using our newly-named Niagara Falls method, step by step, inch by inch, and supported in large part by reliable machine refactoring steps, we now have Cell trimmed down almost as far as we’d like, and the troubling connection of the Cell to its container is entirely removed.

This is a very nice outcome, a substantial improvement, done in tiny steps. Quite fine.

There is more to do—there always is. In this area, the Cell still knows its Room, although I think it does nothing but hold on to it for checking purposes. Whatever use it makes of the Room, I think we want it gone. Cell’s methods are down to these, plus a dunder or two: distance, is_available, is_in_a_room, manhattan_distance, room, vector_diff, x, xy, y.

So that’s all quite good. But as mentioned in Horrendous Mess™, the DungeonLayout is now a wretched hive of scum and villainy. But the scum and villains are now in, or very near to, their proper place.

Tomorrow, or perhaps Monday, we’ll look around and decide what to do next. One somewhat big decision has to do with whether we’re going to turn this dungeon builder into an actual game. I don’t really have a better idea, but perhaps something will come to mind.

For now, a very pleasing sequence of 50 commits extended over multiple days, to a conclusion much like the one we were aiming for.

See you next time!