Just for fun, let’s see about moving find_path into DungeonLayout.

In DungeonLayout, find_path just forwards to Cell.

class DungeonLayout:
    def find_path(self, source, target, room_list):
        return source.find_path(target, room_list)

class Cell:
    def find_path(self, target, room_list):
        def can_use(cell):
            return cell.is_available or cell.room in room_list
        reached = self.path_map(can_use, 1.0)
        path = []
        start = target
        while start is not None:
            path.append(start)
            start = reached.get(start)
        return path

So we copy the code to DungeonLayer, edit it lightly and forward path_map instead:

class DungeonLayout:
    def find_path(self, source, target, room_list):
        def can_use(cell):
            return cell.is_available or cell.room in room_list
        reached = self.path_map(source, can_use, 1.0)
        path = []
        start = target
        while start is not None:
            path.append(start)
            start = reached.get(start)
        return path

    def path_map(self, source, can_use, randomness=0.0):
        return source.path_map(can_use, randomness)

Tests all run, commit: implement find_path in DungeonLayout.

Now see who’s still using the one in Cell. No one is, we already hooked Suite to the Layout. Remove the method. Commit: remove Cell:find_path.

Now path_map has two uses, the one in DungeonLayout and this one:

class PathHelper:
    def __init__(self, layout, source, target):
        self.layout = layout
        self.source = source
        self.length, self.theta = self.get_length_and_theta(source, target)
        rooms = [source.room, target.room]
        self.map = source.path_map(lambda c: c.is_available or c.room in rooms)

We’ll leave that one alone until the Layout works. Copy the method over and edit lightly:

Ah, that doesn’t quite work because the Flooder doesn’t understand layout. Belay that change, revert.

Oh, right, Flooder has those 21 uses counting 5 imports.

This time we’ll change the signature but do a lazy init of the new layout member.

class Flooder:
    def __init__(self, origin, layout=None):
        from dungeonlayout import current
        self_layout = layout if layout else current
        self._delivered = None
        self._to_be_delivered = None
        self._origin = origin
        self._select = lambda cell: True
        self._next_value = lambda cell, value: value + 1
        self._initial_value = 0
        self._randomness = 0.0

Lots of breakage but it’ll be because I asked to init layout with layout. Should be a quick fix.

Needed to pass layout into some of the RoomMaker Collectors:

class RoundCellCollector:
    def __init__(self, layout):
        self.layout = layout

    def build(self, *, radius: int, origin: Cell):
        cells: list[Cell] = []
        enough = radius * 1.5 # 1.4 seems to work but tight?
        for c, _ in Flooder(origin, self.layout).available().flood():
            if (d := c.distance(origin)) < radius:
                cells.append(c)
                c.room = self
            if d > enough:
                return cells
        return cells

Should do them all for symmetry but we have broken tests.

Had to pass self as layout here:

class DungeonLayout:
    def find_suite(self, room) -> Suite:
        suite_rooms: set[Room] = set()
        start = room.cells[0]
        for cell, _ in Flooder(start, self).in_any_room().flood():
            suite_rooms.add(cell.room)
        return Suite(self, suite_rooms)

By the way, I plan to reverse those two arguments to Flooder but I thought putting the layout last might make my refactoring easier. I’m not sure it did.

The call to path_map in Cell is still in use and now needs a layout, which it does not have. Replace that with None, that’s why we did the patch in Flooder:

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

Only one test now failing. That’s also fixable with None:

class Cell:
    def furthest_cells(self):
        maximizer = MaximizingMachine()
        for cell, distance in (Flooder(self, None)
                .in_any_room()
                .flood()):
            maximizer.process(cell, distance)
        return [cd.cell for cd in maximizer.items]

We are green. Commit: converting Flooder to expect a layout but cope if one is not provided.

I’m not entirely sure that this was the best course, but we got quickly back to green, so we’ll continue. Our mission was to move path_map from Cell to DungeonLayout. We’ll copy it over and edit again:

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

Used explicit args there in Flooder, as I was briefly confused without them. We are green. Commit: moving path_map to DungeonLayout.

Now is anyone using Cell.path_map? Only PathHelper:

class PathHelper:
    def __init__(self, layout, source, target):
        self.layout = layout
        self.source = source
        self.length, self.theta = self.get_length_and_theta(source, target)
        rooms = [source.room, target.room]
        self.map = source.path_map(lambda c: c.is_available or c.room in rooms)

This can use the layout.

class PathHelper:
    def __init__(self, layout, source, target):
        self.layout = layout
        self.source = source
        self.length, self.theta = self.get_length_and_theta(source, target)
        rooms = [source.room, target.room]
        self.map = self.layout.path_map(source, lambda c: c.is_available or c.room in rooms)

Green. Remove Cell.path_map. Still green. Commit: path_map removed from Cell.

Now let’s go back to that hack in Flooder and see if we can remove it:

class Flooder:
    def __init__(self, origin, layout=None):
        from dungeonlayout import current
        self_layout = layout if layout else current
        self._delivered = None
        self._to_be_delivered = None
        self._origin = origin
        self._select = lambda cell: True
        self._next_value = lambda cell, value: value + 1
        self._initial_value = 0
        self._randomness = 0.0

It still has all those references, 21 of them. Just remove the hack and see who complains. No one. Perfect, got ‘em all.

class Flooder:
    def __init__(self, origin, layout):
        self_layout = layout
        self._delivered = None
        self._to_be_delivered = None
        self._origin = origin
        self._select = lambda cell: True
        self._next_value = lambda cell, value: value + 1
        self._initial_value = 0
        self._randomness = 0.0

Let’s reverse those two arguments with Change Signature. I’ll ask for keyword args here as well:

class Flooder:
    def __init__(self, *, layout, origin):
        self_layout = layout
        self._delivered = None
        self._to_be_delivered = None
        self._origin = origin
        self._select = lambda cell: True
        self._next_value = lambda cell, value: value + 1
        self._initial_value = 0
        self._randomness = 0.0

Obligingly, PyCharm fixes them all up to look like this:

    def find_suite(self, room) -> Suite:
        suite_rooms: set[Room] = set()
        start = room.cells[0]
        for cell, _ in Flooder(layout=self, origin=start).in_any_room().flood():
            suite_rooms.add(cell.room)
        return Suite(self, suite_rooms)

We are green. Commit: remove None hack and re-order arguments to FLooder init.

Summary

Some good results this session:

  1. We moved find_path out of Cell, into DungeonLayout where it makes more sense;
  2. Ditto path_map;
  3. Disconnected most of Flooder’s cell connections, moved to the Layer.

The sequence included a revert, when we moved too quickly to disconnect Flooder. We fixed that by allowing a None layout construction parameter, and then in Flooder, using our shim to grab the current Layout. By the end of the sequence, that hack was removed and Flooder’s users now all provide a suitable Layout.

Revert
It’s interesting. To me a revert very soon after starting something seems like a good thing. I’ve learned, quickly, where not to go. A revert after a long while seems a bit more like a defeat, but with any luck it still implies that I’ve learned something important.

I almost never revert back over a commit. I am “committed” to moving forward, not back, and in fact I’d have to read up on how to get back behind whatever is currently committed: I just don’t do it.

: It’s usually the case that I can make a small move, find the tests running, commit, inching forward in the Niagara Falls1 sense. In that mode, a revert is just a quick “oops, don’t step there”, and try again, better.

I don’t know whether the trick of providing a temporary access to a global or current value of a class has a name or not, but as we move capability from Cell to DungeonLayout and other classes, it has proved very helpful, because it lets us keep things working, and also provides an easy way to find where the trick is in use, because it requires an import and reference to a specific global, easy to find in the code.

Bottom line, we patched in a connection to an “emergency layout”, moved a bit of code from one class to another with very light edits, and then removed the path. Went smoothly.

I can wait to see what I do next. See you soon!


  1. I have taken to calling the step by step inch by inch approach the “Niagara Falls Method”, named after an ancient comedy routine that probably predates your birth. I’m here to entertain myself, after all. ↩