Well, dungeon CREATION. We’ll start with a little refactoring. I remember why I recommend testing bug fixes.

Before we proceed further with our little “language”, helpers for making rooms, let’s take a look at some of the ways they are being created now.

Presently, all our visible experiments are done by editing main, which currently looks like this:

def main():
    space = CellSpace(64, 56)
    # random.seed(234)
    dungeon = Dungeon()
    number_of_rooms = random.randint(2,2)
    maker = RoomMaker()
    origin = space.at(32, 28)
    diamond = maker.diamond(number_of_cells=25, origin=origin)
    diamond.return_cells()
    round = maker.round(radius=8, origin=origin)
    diamond.reclaim_cells_from(round)
    dungeon.add_room(diamond)
    dungeon.add_room(round)
    for _ in range(number_of_rooms):
        size = random.randint(60, 100)
        origin = space.random_available_cell()
        room = maker.cave(number_of_cells=size, origin=origin)
        dungeon.add_room(room)
        origin = space.random_available_cell()
        room = maker.diamond(number_of_cells=random.choice([25, 61]), origin=origin)
        dungeon.add_room(room)
        origin = space.random_available_cell()
        rad = random.randint(4, 7)
        room = maker.round(radius=rad, origin=origin)
        dungeon.add_room(room)
        origin = space.random_available_cell()
        size = random.randint(100, 200)
        room = maker.experimental(number_of_cells=size, origin=origin)
        if len(room.cells) < 100:
            print(len(room.cells))
        dungeon.add_room(room)
    # for room_1, room_2 in zip(dungeon.rooms, dungeon.rooms[1:]):
    #     dungeon.find_path_between_rooms(space, room_1, room_2)
    while not dungeon.is_fully_connected:
        origin = space.random_available_cell()
        if origin is None:
            break
        size = random.randint(100, 200)
        room = maker.experimental(number_of_cells=size, origin=origin)
        if len(room.cells) >= 100:
            dungeon.add_room(room)
        else:
            room.return_cells()
            print(f'too small {len(room.cells)} cells')
    print(f'{dungeon.is_fully_connected=}\n{len(dungeon.rooms)=}')
    view = DungeonView(dungeon)
    view.main_loop()

I think it’s fair to say that that’s not very well factored. If it were, then testing some new layout to see if it looks right would be more like writing a function and calling it than the mess above. Let’s begin by extracting some functions and see what we get.

I should say that my personal programming style will probably lead me to move those functions into some class or classes, but we’ll wait to see what things look like before worrying about that.

We’ll start by moving that fully-connected code out. We’ll call it, oh, connect rooms. I think it should extract OK.

...
    # for room_1, room_2 in zip(dungeon.rooms, dungeon.rooms[1:]):
    #     dungeon.find_path_between_rooms(space, room_1, room_2)
    connect_rooms(dungeon, maker, space)
    view = DungeonView(dungeon)
    view.main_loop()


def connect_rooms(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    while not dungeon.is_fully_connected:
        origin = space.random_available_cell()
...

I noticed when testing main that we looped in connected, continually finding small rooms. I think I’d like to have it give up and return, because if it loops I don’t get to see what was going on, and I’d like to know what kind of layout fails.

Wait, let’s look at that code more carefully:

def connect_rooms(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    while not dungeon.is_fully_connected:
        origin = space.random_available_cell()
        if origin is None:
            break
        size = random.randint(100, 200)
        room = maker.experimental(number_of_cells=size, origin=origin)
        if len(room.cells) >= 100:
            dungeon.add_room(room)
        else:
            room.return_cells()
            print(f'too small {len(room.cells)} cells')
    print(f'{dungeon.is_fully_connected=}\n{len(dungeon.rooms)=}')

Suppose we are almost connected. We go to add one more room. It is small, but it connects us up. We declare it too small and throw it away. But it was our only chance. Could that happen? I don’t know … Suppose we have a room alone up in a corner and other rooms came close but none connected. Suppose they other rooms surround our lone room, very closely. We can only connect by filling in that small area. But we reject that solution as too small. Let’s change the function to keep the room if it finds we are connected on a small room.

def connect_rooms(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    while not dungeon.is_fully_connected:
        origin = space.random_available_cell()
        if origin is None:
            break
        size = random.randint(100, 200)
        room = maker.experimental(number_of_cells=size, origin=origin)
        if len(room.cells) >= 100 or dungeon.is_fully_connected:
            dungeon.add_room(room)
        else:
            room.return_cells()
            print(f'too small {len(room.cells)} cells')
    print(f'{dungeon.is_fully_connected=}\n{len(dungeon.rooms)=}')

I think we have plugged a small hole. And things work so commit: refactoring main, fixed bug that could loop trying to connect.

Nota Bene
Often when we are refactoring code to improve it, we will encounter opportunities like the one above, either an actual defect that has not been noticed or perhaps that has been noticed but never fixed, or just something we can make better. When we see such things, let’s improve them.

You’ll have seen the sage advice, even from me, that says that when we find a bug we should write a test to show that it exists, then fix it, and show the test runs. I’m not doing that here. Perhaps I should, but creating the case seems too difficult.

Except that having said that, I see how to write a fairly simple test. For our sins, let’s try it.

I happen to have a file “test_dungeon_making”, which seems like a not unreasonable home for this test.

    def test_connect_with_small_room(self):
        space = CellSpace(8,8)
        maker = RoomMaker()
        dungeon = Dungeon()
        d1 = space.at(1,1)
        r1 = maker.diamond(number_of_cells=5, origin=d1)
        dungeon.add_room(r1)
        d2 = space.at(6,6)
        r2 = maker.diamond(number_of_cells=5, origin=d2)
        dungeon.add_room(r2)
        assert not dungeon.is_fully_connected
        connect_rooms(dungeon, maker, space)
        assert dungeon.is_fully_connected

If I’m not mistaken, which assumes facts not in evidence, this test creates a small space and puts two small diamond rooms in it, at diagonally opposite corners. The dungeon is not fully connected at that point. Then we call connect_rooms, after which it should be, and there is not enough space to allow for a 100-cell room, so the second part of the if should trigger.

It does not, and we loop. Why? Because when we check to see if the room is connected, we have not added our short little room. Our “fix” doesn’t fix.

N.B.
I begin to remember why I, with so many others, advise always writing a test …
def connect_rooms(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    while not dungeon.is_fully_connected:
        origin = space.random_available_cell()
        if origin is None:
            break
        size = random.randint(100, 200)
        room = maker.experimental(number_of_cells=size, origin=origin)
        dungeon.add_room(room)
        if dungeon.is_fully_connected:
            break
        if len(room.cells) <= 100:
            dungeon.remove_room(room)
            print(f'too small {len(room.cells)} cells')
    print(f'{dungeon.is_fully_connected=}\n{len(dungeon.rooms)=}')

class Room:
    def remove_room(self, room):
        room.return_cells()
        self.rooms.remove(room)

We are green. Commit: add test showing bug not fixed, fix it correctly.

Reflection

Well. We don’t often see me get my comeuppance quite so quickly, do we? I still recommend fixing things we encounter as we refactor, and I will try harder to test things even if I don’t initially see how. As it turned out just a little thought about the concern, rooms separated only by a small space, gave me the key to the test, make two easy rooms not far apart in a small space.

I had expected that the test would run, I’d disable the new clause and show it failing, put the clause back, voila. But no. My fix wasn’t. The new one is in fact a fix.

I’ve made a note about the remove: it’s not very bullet proof. I think it’s good enough: if we use it wrongly the program will crash, and we have plenty of time to have that happen. Anything we do when someone tries to return a room that isn’t in the dungeon will be wrong anyway, and, as far as I’m concerned, one exception is as bad as another.

Let’s get back to improving main.

There is a sequence near the top of main that is building that round room with the diamond in the middle. Let’s extract that to a function:

    space = CellSpace(64, 56)
    # random.seed(234)
    dungeon = Dungeon()
    number_of_rooms = random.randint(2,2)
    maker = RoomMaker()
    make_diamond_in_round_room(dungeon, maker, space)
    for _ in range(number_of_rooms):
        ...

def make_diamond_in_round_room(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    origin = space.at(32, 28)
    diamond = maker.diamond(number_of_cells=13, origin=origin)
    diamond.return_cells()
    round = maker.round(radius=5, origin=origin)
    diamond.reclaim_cells_from(round)
    dungeon.add_room(diamond)
    dungeon.add_room(round)

I made the room smaller, because it takes up too much space. We’ll leave the call in for now but of course our purpose in pulling out these functions is to make them available when we want them and not otherwise.

For the rest of main, for now, I’m going to rig it to draw just two of each of the regular room types, and then connect everything up.

def main():
    space = CellSpace(64, 56)
    # random.seed(234)
    dungeon = Dungeon()
    maker = RoomMaker()
    make_diamond_in_round_room(dungeon, maker, space)
    number_of_rooms = random.randint(2,2)
    for _ in range(number_of_rooms):
        make_a_diamond_room(dungeon, maker, space)
        make_a_round_room(dungeon, maker, space)
    # for room_1, room_2 in zip(dungeon.rooms, dungeon.rooms[1:]):
    #     dungeon.find_path_between_rooms(space, room_1, room_2)
    connect_rooms(dungeon, maker, space)
    view = DungeonView(dungeon)
    view.main_loop()

The extracted methods are just what you’d expect:

def make_a_round_room(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    origin = space.random_available_cell()
    rad = random.randint(4, 7)
    room = maker.round(radius=rad, origin=origin)
    dungeon.add_room(room)


def make_a_diamond_room(dungeon: Dungeon, maker: RoomMaker, space: CellSpace):
    origin = space.random_available_cell()
    room = maker.diamond(number_of_cells=random.choice([25, 61]), origin=origin)
    dungeon.add_room(room)

We get pictures that look as I’d expect:

map showing a few known shaped rooms connected with cave-like rooms

Green, commit: main is better factored.

Let’s quit on a win again today.

Summary

We have two gains from today’s work, and both should impact us soon:

First, making new experiments in main will be easier now, and easier to understand.

Second, we found and fixed a problem.

Third, we found a simple way to test a fairly complex situation.

Fourth, we found that our fix wasn’t a fix and found a fix that was.

Fifth, and this is the one we really had in mind: we have isolated some code for making rooms, and that will help inform us about the kinds of new room-making capabilities will be useful.

Two
Whenever I say I have two ideas or understandings or anything like that, I always seem to come up with more than two. Today we really reaped a bounty: five. Woot!

I really started the refactoring to help me identify what kinds of room creation we’d been doing, so as to learn from it. But we got many benefits, perhaps as many as two.

See you next time!