Dungeon 25

It helps if your tests actually check something. Sounds like a good idea, let’s try it.

We closed last time with a little surprise, in that the corridors went by a route that surprised me. But the tests ran, and I was sure they said the opposite of what was happening. There was a reason for that. Here’s the last test and its supporting function.

        _:test("connect rooms h then v", function()
            local tile
            local msg
            local r,x,y
            local runner = GameRunner()
            local r1 = Room(10,15,5,5, runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(12)
            _:expect(r1y).is(17)
            local r2 = Room(30,40, 6,6, runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(33)
            _:expect(r2y).is(43)
            r2:hvCorridor(r1)
            checkRange(runner, r2x,r2y, r1x, r2y, Tile.isRoom)
            checkRange(runner, r1x,r2y, r1x, r1y, Tile.isRoom)
        end)

function checkRange(runner, x1, y1, x2, y2, checkFunction)
    for x = x1,x2 do
        for y = y1,y2 do
            local t = runner:getTile(x,y)
            local r = checkFunction(t)
            if not r then return r, x, y end
        end
    end
    return true,0,0
end

We see here a “test” that doesn’t check anything. the checkRange function returns a result that can be checked by the caller. It does no expect of its own. It’s intended to be used, as we did earlier, this way:

            runner:verticalCorridor( 15,45, 20) -- y1, y2, x
            r,x,y = checkRange(runner, 20,15, 20,45, Tile.isRoom)
            msg = string.format("%d %d", x, y)
            _:expect(r,msg).is(true)

This isn’t the first time I’ve used the function incorrectly, forgetting to check the results. Let’s change it to check inside.

function checkRange(runner, x1, y1, x2, y2, checkFunction)
    for x = x1,x2 do
        for y = y1,y2 do
            local t = runner:getTile(x,y)
            local r = checkFunction(t)
            if not r then
                local msg = string.format("%d,%d fails", x,y)
                _:expect(r,"checkRange").is(true)
                return r,x,y
            end
        end
    end
    _:expect(true).is(true)
    return true,0,0
end

OK, that’s better. It checks and will fail or get an OK inside, and still returns the results for those who are checking them outside. But the test does not fail for the case in hand, and I think it should. There is another issue: when x1>x2 or y1>y2. We need to sort the input values. Plus another change to get the right message out:

function checkRange(runner, x1, y1, x2, y2, checkFunction)
    x1,x2 = math.min(x1,x2), math.max(x1,x2)
    y1,y2 = math.min(y1,y2), math.max(y1,y2)
    for x = x1,x2 do
        for y = y1,y2 do
            local t = runner:getTile(x,y)
            local r = checkFunction(t)
            if not r then
                local msg = string.format("checkRange %d,%d fails", x,y)
                _:expect(r,msg).is(true)
                return r,x,y
            end
        end
    end
    _:expect(true).is(true)
    return true,0,0
end

Now I get what I expect:

4: connect rooms h then v checkRange 12,43 fails -- Actual: false, Expected: true

Now, at last, I can fix the problem, which comes from this code:

function Room:hvCorridor(aRoom)
    local r1x,r1y = self:center()
    local r2x,r2y = aRoom:center()
    self.runner:horizontalCorridor(r1x,r2x,r2y)
    self.runner:verticalCorridor(r2y,r1y,r1x)
end

Maybe the issue is that this goes from self to aRoom and we want it to go the other way. The argument setup may be reversed from the test’s expectations. The fix would be

function Room:hvCorridor(aRoom)
    local r1x,r1y = aRoom:center()
    local r2x,r2y = self:center()
    self.runner:horizontalCorridor(r1x,r2x,r2y)
    self.runner:verticalCorridor(r2y,r1y,r1x)
end

But the test still fails, saying, as before:

4: connect rooms h then v checkRange 12,43 fails -- Actual: false, Expected: true

OK, let’s set the function back:

function Room:hvCorridor(aRoom)
    local r1x,r1y = self:center()
    local r2x,r2y = aRoom:center()
    self.runner:horizontalCorridor(r1x,r2x,r2y)
    self.runner:verticalCorridor(r2y,r1y,r1x)
end

And let’s sort this out from first principals with a closer look.

The test has room r1 with a center at 12,17. It has r2 with a center at 33,43. We … oh. Duh. Looking at the test again, I had commented out the call to hvCorridor, to make sure the test failed. Putting it back, and re-reversing the function’s self and aRoom gives me an expected result and an unexpected one:

4: connect rooms h then v  -- OK

However, I’m suddenly getting two other failures that I’m sure weren’t there before.

2: backward horizontal corridor checkRange 3,50 fails -- Actual: false, Expected: true
2: backward horizontal corridor 3 50 -- Actual: false, Expected: true

This is the same failure twice, once from inside the function and once from outside. Has that been failing for a while without my noticing? The tests display behind the game board, so I might not have noticed. What’s up?

        _:test("backward horizontal corridor", function()
            local tile
            local msg
            local r,x,y
            local runner = GameRunner()
            runner:horizontalCorridor( 30,10, 50)
            r,x,y = checkRange(runner, 10,50, 3,50, Tile.isRoom)
            msg = string.format("%d %d", x, y)
            _:expect(r,msg).is(true)
        end)

Well, that three is obviously wrong. Maybe I typed a backspace at some point when the cursor wasn’t where I expected it. That should say 30,50. And now we’re green.

Despite a moment’s confusion, this glitch gives me a bit more confidence in my tests and my checkRange function. Sort of a poor-man’s mutation testing. We’re good now.

Commit: hvCorridor goes the right way.

Back On Track

OK, we’re back on track. Our mission now is to dig the other kind of path. We can go first horizontal then vertical. We need to go first vertical then horizontal.

First, though, We should ask a question. Our test for hv is this:

            r2:hvCorridor(r1)
            checkRange(runner, r2x,r2y, r1x, r2y, Tile.isRoom)
            checkRange(runner, r1x,r2y, r1x, r1y, Tile.isRoom)

I’m expecting horizontal from the sender of the message, r2, and vertical to the parameter, r1. To accomplish that we have this code:

function Room:hvCorridor(aRoom)
    local r1x,r1y = aRoom:center()
    local r2x,r2y = self:center()
    self.runner:horizontalCorridor(r1x,r2x,r2y)
    self.runner:verticalCorridor(r2y,r1y,r1x)
end

That code seems kind of upside down and backward. It just doesn’t make sense to me to be calling self r2 and aRoom r1. I think we’d do well to reverse the sense of that, or better yet, to use better names throughout. Let’s try that:

First, I swap two lines:

function Room:hvCorridor(aRoom)
    local r2x,r2y = self:center()
    local r1x,r1y = aRoom:center()
    self.runner:horizontalCorridor(r1x,r2x,r2y)
    self.runner:verticalCorridor(r2y,r1y,r1x)
end

Then lets rename the r2s as start:

function Room:hvCorridor(aRoom)
    local startX,startY = self:center()
    local r1x,r1y = aRoom:center()
    self.runner:horizontalCorridor(r1x,startX,startY)
    self.runner:verticalCorridor(startY,r1y,r1x)
end

And let’s test.

All good. I was sure, but it’s virtually cost-free to test. Now rename the others:

function Room:hvCorridor(aRoom)
    local startX,startY = self:center()
    local endX,endY = aRoom:center()
    self.runner:horizontalCorridor(endX,startX,startY)
    self.runner:verticalCorridor(startY,endY,endX)
end

Tests still good. That reversal of end-start in the call to horizontalCorridorlooks odd, and we can always reverse those parameters because it already works both ways:

function Room:hvCorridor(aRoom)
    local startX,startY = self:center()
    local endX,endY = aRoom:center()
    self.runner:horizontalCorridor(startX,endX,startY)
    self.runner:verticalCorridor(startY,endY,endX)
end

Tests still good. And much better. That will make me much less confused the next time I need to look at this code, which I hope will be never.

Now the vertical-horizontal. We’ll do a test. They’ve been helpful. I start with a simple cut/paste and a rename, and I do plan to use the same two boxes.

        _:test("connect rooms v then h", function()
            local tile
            local msg
            local r,x,y
            local runner = GameRunner()
            local r1 = Room(10,15,5,5, runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(12)
            _:expect(r1y).is(17)
            local r2 = Room(30,40, 6,6, runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(33)
            _:expect(r2y).is(43)
            r2:vhCorridor(r1)
            checkRange(runner, r2x,r2y, r1x, r2y, Tile.isRoom)
            checkRange(runner, r1x,r2y, r1x, r1y, Tile.isRoom)
        end)

I’ve put in the call to vh instead of hv. Those are too subtly different but I expect to use them twice. For now, we’ll let that ride. Let’s rename the variables here, too. It can’t hurt, might help.

        _:test("connect rooms v then h", function()
            local tile
            local msg
            local r,x,y
            local runner = GameRunner()
            local endRoom = Room(10,15,5,5, runner)
            local endX,endY = endRoom:center()
            _:expect(endX).is(12)
            _:expect(endY).is(17)
            local startRoom = Room(30,40, 6,6, runner)
            local startX,startY = startRoom:center()
            _:expect(startX).is(33)
            _:expect(startY).is(43)
            startRoom:vhCorridor(endRoom)
            checkRange(runner, startX,startY, startX, endY, Tile.isRoom) -- vert
            checkRange(runner, startX,endY, endX, endY, Tile.isRoom) -- horz
        end)

I think that’s all good. Now to run it and find that we need the function.

5: connect rooms v then h -- Tests:84: attempt to call a nil value (method 'vhCorridor')

I try this:

function Room:vhCorridor(aRoom)
    local startX,startY = self:center()
    local endX,endY = aRoom:center()
    self.runner:verticalCorridor(startY,endY,startX)
    self.runner:horizontalCorridor(startX,endX,endY) 
end

I still find it difficult thinking in these terms. Let’s see what the test says:

5: connect rooms v then h  -- OK

The test thinks we’re good. I’m not quite sanguine about that, so I’ll adjust the game startup:

function GameRunner:createTestRooms()
    self.rooms = {}
    table.insert(self.rooms, Room(5,5,10,10, self))
    table.insert(self.rooms, Room(20,20,4,5, self))
    table.insert(self.rooms, Room(25,10,8,15, self))
    table.insert(self.rooms, Room(50,50,10,10, self))
    --self.rooms[2]:hvCorridor(self.rooms[1])
    --self.rooms[3]:hvCorridor(self.rooms[2])
    self.rooms[4]:vhCorridor(self.rooms[1])
end

down over

Now I’m convinced. Change the startup to emulate the final plan:

function GameRunner:createTestRooms()
    self.rooms = {}
    table.insert(self.rooms, Room(5,5,10,10, self))
    table.insert(self.rooms, Room(20,20,4,5, self))
    table.insert(self.rooms, Room(25,10,8,15, self))
    table.insert(self.rooms, Room(50,50,10,10, self))
    self.rooms[2]:hvCorridor(self.rooms[1])
    self.rooms[3]:hvCorridor(self.rooms[2])
    self.rooms[4]:vhCorridor(self.rooms[3])
end

We get this interesting map. Commit: vhCorridor.

all connected

Now we need one more connection function, one that given two rooms, connects them randomly with vh or hv. I’ll just write this one:

function Room:connect(aRoom)
    if math.random(1,2) == 2 then
        self:hvCorridor(aRoom)
    else
        self:vhCorridor(aRoom)
    end
end

And apply it in setup:

function GameRunner:createTestRooms()
    self.rooms = {}
    table.insert(self.rooms, Room(5,5,10,10, self))
    table.insert(self.rooms, Room(20,20,4,5, self))
    table.insert(self.rooms, Room(25,10,8,15, self))
    table.insert(self.rooms, Room(50,50,10,10, self))
    self.rooms[2]:connect(self.rooms[1])
    self.rooms[3]:connect(self.rooms[2])
    self.rooms[4]:connect(self.rooms[3])
end

I expect this to give me different layouts on different runs, even with the same rooms:

lay1 lay2 lay3

So that’s good. Commit: random room connections.

Let’s wrap this one up.

Wrapping Up

So the corridor cutting has gone nicely, and generates quite interesting dungeons, even with only a few rooms. I’m interested to see what we get when we have more rooms. We’ll work on that next time, I imagine.

I feel good about this code, and about the idea of dropping the old scheme and going to tiling. I have GeePaw Hill to thank for pushing that idea in our Zoom Ensemble. And there have been at least two things to learn from.

First, the tests that I really didn’t want to write, and that I really thought weren’t necessary, were quite helpful. They were a bit tricky to write, and for a while I thought they were checking things when they weren’t, but even that helped me focus on getting the values right.

Second, improving the names really helped. The generic r1 r2 x1 x2 were easy to invent but not easy to distinguish. Going to names like startX and endY was helpful.

I still don’t like the parameter lists on horizontal and vertical corridor, since one takes x x y and the other one y y x. But they’re only used once in anger, so maybe I’ll let them ride.

Overall, things are going quite well. Next stop, random non-overlapping rooms. See you then!

D2.zip