Experiment

– Samuel Beckett¹

Yesterday at session end, I tried to hack in a minimal-path idea. Failed. Today let’s try to fail better.²

The code in main that we are presently running for drawing paths is this:

    suites = dungeon.define_suites()
    path_cells = []
    for s1, s2 in zip(suites, suites[1:]):
        path = s1.find_path_cells(s2)
        path_cells.extend(path)
    path_unique = list(set(path_cells))
    path_room = Room(path_unique, path_unique[0], 'path')
    dungeon.add_room(path_room)

Here’s my thinking from yesterday, which I now see was fatally flawed:

We have, in Dungeon, the notion of “fully-connected”, which creates a suite starting in the first room and determines whether that suite contains all the rooms. If it does, the dungeon is fully connected. We’re not using that in main, but we could. I wonder if it would be difficult. Let me take a look.

Having thought those thoughts, I tried to gin up a scheme that looped until the dungeon was fully connected. I’m not sure what I did, exactly, but I failed three or four times, with various results including loops and crashes. But the real defect was in the idea. If the dungeon were fully connected, there would be only one suite. The zip would find nothing in the second list and the loop would not run. We’d try to create a room with no cells and the Room creation would fail for want of a 0th element in the path.

Even worse, if there are two or more suites, the code above will connect 1 to 2, 2 to 3, and so on, which will in fact be a minimal connection.

A few quick experiments with main assure me that the issues above are as described. We need to fail better. What will help with that includes:

• We could use some tests, at least one to deal with the single suite crash, and perhaps others that will come to mind;
• The ad-hoc code in main should be turned into methods on the various objects that we have so carefully created for that very purpose.

THe main issue with tests for these things is that they are kind of a pain to figure out. I often draw a little diagram on a quad-ruled card.

Let’s TDD out a new method on Dungeon to connect all the suites with minimal paths. That will let us move the code from main into Dungeon, to serve as the basis for doing it better.

We already have a test that is close to what we need:

    def test_path_between_suites(self):
        space = CellSpace(10, 10)
        dungeon = Dungeon()
        maker = RoomMaker()
        a_cell = space.at(1, 1)
        a = maker.cave(number_of_cells=1, origin=a_cell, name='a')
        b = maker.cave(number_of_cells=1, origin=space.at(1,0), name='b')
        c = maker.cave(number_of_cells=1, origin=space.at(0,1), name='c')
        d = maker.cave(number_of_cells=1, origin=space.at(5,4), name='d')
        e_cell = space.at(4, 3)
        e = maker.cave(number_of_cells=1, origin=e_cell, name='e')
        f = maker.cave(number_of_cells=1, origin=space.at(5,3), name='f')
        dungeon.add_room(a)
        dungeon.add_room(b)
        dungeon.add_room(c)
        dungeon.add_room(d)
        dungeon.add_room(e)
        dungeon.add_room(f)
        suites = dungeon.define_suites()
        path = suites[0].find_path_room(suites[1])
        assert len(path.cells) == a_cell.manhattan_distance(e_cell) - 1

That’s based on this sketch that I made a few days back:

That method returns a Room, and I think we don’t really want that any more. We’ll test the method calls and remove the room one.

...
        suites = dungeon.define_suites()
        cells = suites[0].find_path_cells(suites[1])
        assert len(cells) == a_cell.manhattan_distance(e_cell) - 1

Commit: moving path building to Dungeon and other suitable classes.

Now, what do we want? We want our paths to be able to cross. The current scheme is that we save up all the path cells and create a single, possibly discontinuous Room containing all of them. That allows paths to cross and just share whatever cells are common to them.

I have a concern, however, relating to the cave-like structures that we sometimes have used as paths. Do we want those to be part of the same scheme?

The main reason for combining all the paths into one room was to deal with the intersections. If we do not allow the paths to use the cells of other paths, they can’t cross and we can find ourselves with a dungeon that cannot be fully connected. If we make each path a separate room, and allow them to cross, then the crossing path/room will steal the crossing cells from the crossed path/room, and it will become discontinuous and no longer a path.

A cell knows what room it is in. That capability seems quite valuable for moving around. We could perhaps extend that notion to allow a cell to be in multiple rooms, but that seems likely to be a more significant decision, one we shouldn’t just make without serious consideration.

Technical Debt?

The original notion of “technical debt”, from Ward Cunningham, referred to the design represented in the code falling behind the design ideas in our heads. I have a bit of a sensation just now that whatever we do, there is a bit of slippage between the current code and the design that is almost but not quite in my head. What will we do? We’ll wait until we see better what should be done, and we’ll do it then. We will not add a design complexity now, when we don’t see a better idea clearly.

We may be seeing technical debt coming into being. We’ll find out, if it is, and maybe even remember this thought. For now, we’ll stick with the plan.

The plan being: create a method on Dungeon that connects all the suites minimally, allowing path crossings, and producing a single room whose name is ‘path’.

We’ll use a new test:

    def test_create_path_room(self):
        space = CellSpace(10, 10)
        dungeon = Dungeon()
        c_00 = space.at(0, 0)
        c_40 = space.at(4, 0)
        c_44 = space.at(4, 4)
        c_04 = space.at(0, 4)
        room_00 = Room([c_00], c_00, 'room_00')
        room_40 = Room([c_40], c_40, 'room_40')
        room_44 = Room([c_44], c_44, 'room_44')
        room_04 = Room([c_04], c_04, 'room_04')
        dungeon.add_room(room_00)
        dungeon.add_room(room_40)
        dungeon.add_room(room_44)
        dungeon.add_room(room_04)
        cells = dungeon.straight_path_cells()
        assert len(cells) == 9

We have four rooms in four corners so they should be connected by three discontinuous segments of three cells each. This is enough to fail and it does. The code is pretty straightforward:

    def straight_path_cells(self):
        cells = []
        suites = self.define_suites()
        print(f'Found {len(suites)} suites')
        for s1, s2 in zip(suites, suites[1:]):
            cells.extend(s1.find_path_cells(s2))
        return list(set(cells))

We are here to create a room, so we’ll modify the test:

...
        dungeon.add_room(room_04)
        path_room = dungeon.straight_path_room()
        assert len(path_room.cells) == 9

And add the method:

    def straight_path_room(self):
        cells = self.straight_path_cells()
        return Room(cells, cells[0], 'path')

We are green. Commit: initial straight_path_room

Now we can change main to use our new method and try a few dungeons. They look good:

We can still fail, I think, if we have only one Suite. A quick patch to main confirms that. Write a test:

    def test_single_suite_does_not_crash(self):
        space = CellSpace(10, 10)
        dungeon = Dungeon()
        c_44 = space.at(4, 4)
        room = Room([c_44], c_44, 'room_44')
        dungeon.add_room(room)
        path_room = dungeon.straight_path_room()
        assert path_room is None

We could perhaps gin up a room with no cells but that seems iffy. So we’ll return None if there is no path needed. The test fails on an index error, as expected.

    def straight_path_room(self):
        cells = self.straight_path_cells()
        if cells:
            return Room(cells, cells[0], 'path')
        else:
            return None

Empty collections are falsy in Python. Love it or hate it, there it is.

One more thing. Let’s make the two new methods private, and have one that adds a path room if it can.

    def add_path_room(self):
        room = self._straight_path_room()
        if room:
            self.add_room(room)

    def _straight_path_room(self):
        cells = self._straight_path_cells()
        if cells:
            return Room(cells, cells[0], 'path')
        else:
            return None

    def _straight_path_cells(self):
        cells = []
        suites = self.define_suites()
        for s1, s2 in zip(suites, suites[1:]):
            cells.extend(s1.find_path_cells(s2))
        return list(set(cells))

Green. Plugged the add method into main and that works. Commit: Dungeon can add a path room of straight paths if needed to connect suites.

We’ve been at this for about 90 minutes, so a good stopping point. Let’s sum up.

Summary

We had a known defect in path-making if there was only one Suite: the game crashed. Not good. Our only real code that created paths was ad-hoc code in main, calling some moderately low-level methods on Dungeon and Suite and even Room.

Now we have two new tests, one that confirms that we are creating a suitable path, and another that checks that the one-suite situation is properly handled, producing no path room. The work is now done in Dungeon, which can and does call on other classes as needed. The main code is shorter and simpler.

All of this proceeded rather simply, with at least three phases of small changes. We eked out three commits and probably could have done one or two more had I been more in that frame of mind. My bad. I’ll try to fail better on that: I almost always miss at least one opportunity to commit. It rarely bites me, but once in a while my last save point is further back than I’d like. Live and learn even after all these years. I should say “re-learn”, I guess. Maybe “re-re-learn”.

We folded some methods of Dungeon in to be private, currently used in tests and the Dungeon class only. I noticed a few other candidates for the same treatment that we’ll address the next time we notice them.

I’d like to examine today’s work tomorrow, or on some future day, to see whether everything is in the best possible class. There is one static method in there now, which suggests it would like to be moved somewhere else, and there may be other such candidates. I like to look for that sort of thing when my mind and eyes are fresh, which is not the case now.

As for paths in general, my thoughts include:

• The paths we currently get are very right-angled cutting over and then down instead of wending their way diagonally.
• We do not have a test for crossing, although it can be seen to work when we run main.
• I’d like to do more work on the wandering cave-like paths: it would be particularly nice if a single path notion could somehow blend from straight line paths like today’s to wandering cave-like ones, with some settings or something.

There’s always something. But for now, a decent morning, a decent improvement.

See you next time!