Doors, or I’ll know the reason why! Just in: Ron bites bear!

I thought about doors a bit and I think I have a plan that will work. Roughly, it goes like this:

We’ll make a new “kind” of tile, TileHall. It will behave like a room tile but will be distinguishable from room tiles. I think we’ll have a new property isFloor that we’ll use in most of the places we now check isRoom, We’ll be judicious, of course, which is our hallmark.

Then when we carve halls, we’ll lay down hallway tiles. And, as I supposed yesterday, we’ll look at the current tile, and the previous one laid down, to decide whether to make a given hallway tile as “door”. (I wonder whether we should mark the room tile as “door” also. Probably not, but it will be easy to do if needed.)

I still think we’ll want to draw the hallways from a room toward the corner. And I want to be sure we’re doing that, because I suspect that the “From/To” logic I put in there may not be being called correctly. I’m sure the carver will carve in the specified direction, but I’m not sure if we’re always moving outward from the rooms.

So that’s the plan, such as it is. Let’s dive in.

New Kind of Tile

Tile = class()

local TileWall = "wall"
local TileRoom = "room"
local TileEdge = "edge"

Add:

local TileHall = "hall"

And add:

function Tile:isFloor()
    return self.kind == TileHall or self.kind == TileRoom
end

function Tile:isHall()
    return self.kind == TileHall
end

Now examine senders of isRoom and change the suitable ones to isFloor:

Some changed in other fairly clear ways:

function Tile:isOpenHallway(up,down,left,right)
    if not self:isHall()  or not self:isEmpty() then return false end
    return up:isHall() and down:isHall() and not left:isHall() and not right:isHall()
    or left:isHall() and right:isHall() and not up:isHall() and not down:isHall()
end

There, I changed all the isRoom to isHall.

Other than that one, I changed all the isRoom to isFloor, except for this:

function Dungeon:setHallwayTile(x,y)
    local t = self:privateGetTileXY(x,y)
    if not t:isRoom() then
        self:defineTile(Tile:room(x,y,t.runner))
    end
end

This method is going to be involved in our new carving logic, and I’m not sure whether the call should be isRoom or not, in the long run. For now, it should be isRoom. And we should make a hall tile. I need this:

        self:defineTile(Tile:hall(x,y,t.runner))

function Tile:hall(x,y, runner)
    assert(not TileLock, "Attempt to create room tile when locked")
    return Tile(x,y,TileHall, runner)
end

At this point, I expect the game to work, but a number of hallway carving tests to fail. Let’s find out.

I find one little oddity: the Spikes can occur in a room. They’re supposed to only go into hallways. Let’s see how they get placed.

function GameRunner:placeSpikes(count)
    for i = 1,count or 20 do
        local tile = self:getDungeon():randomHallwayTile()
        Spikes(tile)
    end
end

Let’s check that method:

function Dungeon:randomHallwayTile()
    while true do
        local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
        local tile = self:getTile(pos)
        local up = self:getTile(pos+vec2(0,1))
        local down = self:getTile(pos-vec2(0,1))
        local left = self:getTile(pos-vec2(1,0))
        local right = self:getTile(pos+vec2(1,0))
        if tile:isOpenHallway(up,down,left,right) then return tile end
    end
end

Hm, I was sure that method was changed:

function Tile:isOpenHallway(up,down,left,right)
    if not self:isHall()  or not self:isEmpty() then return false end
    return up:isHall() and down:isHall() and not left:isHall() and not right:isHall()
    or left:isHall() and right:isHall() and not up:isHall() and not down:isHall()
end

Hm how could that possibly fail?

While touring the dungeon looking for spikes in the wrong place, I notice that the hallways are not showing up on the small map. Need to take a look at that. Oh my:

function Tile:drawMapCell(center)
    if self.kind == TileRoom and self.seen then
        pushMatrix()
        pushStyle()
        rectMode(CENTER)
        translate(center.x, center.y)
        fill(255)
        rect(0,0,TileSize,TileSize)
        popStyle()
        popMatrix()
    end
end

What we have here is an object taking advantage of the details of an accessor, and we’re worse off for it.

function Tile:drawMapCell(center)
    if self:isFloor() and self.seen then
        pushMatrix()
        pushStyle()
        rectMode(CENTER)
        translate(center.x, center.y)
        fill(255)
        rect(0,0,TileSize,TileSize)
        popStyle()
        popMatrix()
    end
end

Let’s look for other access to self.kind just for grins. I think that was the only troublesome one. But about those spikes? I want to see it happen again.

A number of tours later, I’ve not see Spikes in a room. Let’s not worry about that. It’s time to make the tests run again, and then work on the door thing. Lots of tests, but probably all the same, unless we want to check more accurately.

1: Horizontal corridor checkRange 5,7 fails -- Actual: false, Expected: true
1: Horizontal corridor 5 7 -- Actual: false, Expected: true
2: backward horizontal corridor checkRange 5,7 fails -- Actual: false, Expected: true

And so on.

        _:test("Horizontal corridor", function()
            local tile
            local msg
            local dungeon = Runner:getDungeon()
            dungeon:horizontalCorridor( 5,10, 7)
            tile = Runner:getTile(vec2(4,7))
            _:expect(tile:isEdge()).is(true)
            tile = Runner:getTile(vec2(11,7))
            _:expect(tile:isEdge()).is(true)
            r,x,y = checkRange(dungeon, 5,7, 10,7, Tile.isRoom)
            msg = string.format("%d %d", x, y)
            _:expect(r,msg).is(true)
        end)

We can change those isRoom references to isFloor and we should find all the tests running again.

We find a couple more:

2: is open hallway tile up down up down OK -- Actual: false, Expected: true
3: is open hallway tile left right  -- Actual: false, Expected: true

        _:test("is open hallway tile up down", function()
            local tile, up, down, left, right
            tile = Tile:wall(100,100, Runner)
            up = Tile:wall(100,101, Runner)
            down = Tile:wall(100,99, Runner)
            left = Tile:wall(99,100, Runner)
            right = Tile:wall(101,100, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right)).is(false)
            up = Tile:room(100,101, Runner)
            down = Tile:room(100,99, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right), "central isn't room").is(false)
            tile = Tile:room(100,100, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right), "up down OK").is(true)
            left = Tile:room(99,100, Runner)
            right = Tile:room(101,100, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right), "up down left right not OK").is(false)
        end)
        
        _:test("is open hallway tile left right", function()
            local tile, up, down, left, right
            tile = Tile:room(100,100, Runner)
            up = Tile:wall(100,101, Runner)
            down = Tile:wall(100,99, Runner)
            left = Tile:room(99,100, Runner)
            right = Tile:room(101,100, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right)).is(true)
            up = Tile:room(100,101, Runner)
            down = Tile:room(100,99, Runner)
            _:expect(tile:isOpenHallway(up,down,left,right), "up down left right not OK").is(false)
        end)

I reckon every one of those Tile:room wants to be Tile:hall, don’t you? That does the trick. All the tests pass.

Commit: Hallways are now paved with TileHall tiles, not TileRoom tiles.

Now for the Hard Part

Arguably, and I’m about to argue it, what we just did was a refactoring. We changed the implementation of the system, but we didn’t change its functionality. Yes we added a whole new constant TileHall and used it in places where we used to use TileFloor, and so on.

Usually when we think of refactoring, we think in terms of moving code around, maybe creating a new method containing mostly existing code, and so on. This case felt almost like a new implementation, the implementation of this new thing, a hallway tile.

But same behavior, new implementation: refactoring.

And with a purpose. Often you’ll see me refactoring just to make code better, because I can, and because I want to show how easy it is. But one good purpose for refactoring is to prepare the system for something new. To make a place for the new thing to land, new objects for the new thing to use or refer to.

And that’s what we just did … at least that was my plan.

Now I want to write some tests for the new “door” feature. My objective will be to tell certain hall tiles that they can be doors. We’ll do that by adding a new field and accessors, which I am not going to TDD, because I am sure that the tests I’m about to write will test these just fine:

function Tile:initDetails()
    self.contents = DeferredTable()
    self.seen = false
    self:clearVisible()
    self.sprite = nil
    self.door = false
end

function Tile:canBeDoor()
    return self.door
end

function Tile:setCanBeDoor()
    self.door = true
end

I’m not in love with these names, but they’ll do for now.

Test and commit: Tile has door property.

Now we get to write some tests, or adapt some. The hallway tests might be fertile ground for enhancement: let’s look.

Here’s one:

        _:test("connect rooms h then v", function()
            local tile
            local msg
            local r,x,y
            local dungeon = Runner:getDungeon()
            local r1 = Room(2,2,3,3, Runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(3)
            _:expect(r1y).is(3)
            local r2 = Room(10,11,3,3, Runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(11)
            _:expect(r2y).is(12)
            r2:hvCorridor(dungeon,r1)
            checkRange(dungeon, r2x,r2y, r1x, r2y, Tile.isFloor)
            checkRange(dungeon, r1x,r2y, r1x, r1y, Tile.isFloor)
        end)

We can calculate easily which tiles should be able to be doors. Well, sort of easily. There should be door capability at (5,3) and (11,10), if I’ve counted squares correctly1.

card

So therefore …

        _:test("connect rooms h then v", function()
            local tile
            local msg
            local r,x,y
            local dungeon = Runner:getDungeon()
            local r1 = Room(2,2,3,3, Runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(3)
            _:expect(r1y).is(3)
            local r2 = Room(10,11,3,3, Runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(11)
            _:expect(r2y).is(12)
            r2:hvCorridor(dungeon,r1)
            checkRange(dungeon, r2x,r2y, r1x, r2y, Tile.isFloor)
            checkRange(dungeon, r1x,r2y, r1x, r1y, Tile.isFloor)
            tile = dungeon:privateGetTileXY(5,3)
            _:expect(tile:canBeDoor(),"door @ 5,3").is(true)
            tile = dungeon:privateGetTileXY(11,12)
            _:expect(tile:canBeDoor(),"door @ 11,12").is(true)
        end)

This is supposed to fail both those last two expectations.

4: connect rooms h then v door @ 5,3 -- Actual: false, Expected: true
4: connect rooms h then v door @ 11,12 -- Actual: false, Expected: true

Now, as an extra check, I’m going to hammer a door in just for a moment:

            tile = dungeon:privateGetTileXY(5,3)
            tile:setCanBeDoor()
            _:expect(tile:canBeDoor(),"door @ 5,3").is(true)
            tile = dungeon:privateGetTileXY(11,12)
            _:expect(tile:canBeDoor(),"door @ 11,12").is(true)

Now the first should pass and the second fail. And yes.

Now we only need to set the flag while drawing. We’ll drill into the hvCorridor and vhCorridor methods:

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

And I’m not at all sure that these directions are correct. And I’m not sure whether we need to care. But let’s allow ourselves to require that the two hallways be drawn away from a room center to the corner location.

No. We’re going from start to end on each. Should be:

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

We’ll have the same issue with vh:

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

Should be:

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

This is hard to think about. Anyway, I think that’s good. Now in the dungeon methods:

function Dungeon:horizontalCorridor(fromX, toX, y)
    local increment = 1
    if fromX > toX then
        increment = -1
    end
    for x = fromX,toX,increment do
        self:setHallwayTile(x,y)
    end
end

We changed these yesterday to always proceed in the provided order, from-to. Now I think what needs to be done it to consider the current tile, at current x,y, and the previous tile already inspected. What does `setHallwayTile do again?

function Dungeon:setHallwayTile(x,y)
    local t = self:privateGetTileXY(x,y)
    if not t:isRoom() then
        self:defineTile(Tile:hall(x,y,t.runner))
    end
end

I don’t feel smart enough to modify that method, let’s instead change the top level loop.

I try this. Here I fetch the tile to check into curr, and if it’s not a room tile I set it to a hall tile. And then, if the previous tile is a room tile (not floor tile), I set the curr to be able to be a door. I really expected this to work but the print there will let you know that it didn’t.

function Dungeon:horizontalCorridor(fromX, toX, y)
    local prev,curr
    local increment = 1
    if fromX > toX then
        increment = -1
    end
    prev = self:privateGetTileXY(fromX,y)
    for x = fromX,toX,increment do
        curr = self:privateGetTileXY(x,y)
        if not curr:isRoom() then
            curr = Tile:hall(x,y,curr.runner)
            self:defineTile(curr)
            if prev:isRoom() then
                print("setting door "..x.." "..y)
                curr:setCanBeDoor()
            end
        end
        prev = curr
    end
end

Here’s what I see in the console:

setting door 9 12
4: connect rooms h then v door @ 5,3 -- Actual: false, Expected: true
4: connect rooms h then v door @ 11,12 -- Actual: false, Expected: true
setting door 5 3

The 9, 12 looks as if we’re going horizontal from the 11,12 room not from the 3,3 room. Let’s review the test, maybe my mind is on backward.

        _:test("connect rooms h then v", function()
            local tile
            local msg
            local r,x,y
            local dungeon = Runner:getDungeon()
            local r1 = Room(2,2,3,3, Runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(3)
            _:expect(r1y).is(3)
            local r2 = Room(10,11,3,3, Runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(11)
            _:expect(r2y).is(12)
            r2:hvCorridor(dungeon,r1)
            checkRange(dungeon, r2x,r2y, r1x, r2y, Tile.isFloor)
            checkRange(dungeon, r1x,r2y, r1x, r1y, Tile.isFloor)
            tile = dungeon:privateGetTileXY(5,3)
            _:expect(tile:canBeDoor(),"door @ 5,3").is(true)
            tile = dungeon:privateGetTileXY(11,12)
            _:expect(tile:canBeDoor(),"door @ 11,12").is(true)
        end)

Sure enough, I’m connecting from r2. So my supposed doors should be at 9,12, and 3,5. Change the test.

setting door 9 12
4: connect rooms h then v door @ 3,5 -- Actual: false, Expected: true
setting door 5 3

I got the one but not the other. Why not? Because I haven’t fixed vertical corridor to match horizontal:

function Dungeon:verticalCorridor(fromY, toY, x)
    local prev,curr
    local increment = 1
    if fromY>toY then
        increment = -1
    end
    prev = self:privateGetTileXY(x,fromY)
    for y = fromY, toY, increment do
        curr = self:privateGetTileXY(x,y)
        if not curr:isRoom() then
            curr = Tile:hall(x,y,curr.runner)
            self:defineTile(curr)
            if prev:isRoom() then
                print("setting door "..x.." "..y)
                curr:setCanBeDoor()
            end
        end
        prev = curr
    end
end

I think I may detect some duplication there … but the test runs. We have correctly flagged both exit hallway tiles as having doors.

Remove the print. Change the draw to color the canBeDoor tiles.

function Tile:drawLargeSprite(center)
    -- we have to draw something: we don't clear background. Maybe we should.
    local sp = self:getSprite(self:pos(), false)
    pushMatrix()
    pushStyle()
    textMode(CENTER)
    translate(center.x,center.y)
    if not self.currentlyVisible then tint(0) end
    if self:canBeDoor() then tint(255,0,0) end
    sp:draw()
    popStyle()
    popMatrix()
end

door1

door2

That’s working as anticipated. I do expect that hallways crossing other rooms will not show doors, on the entry side. But on the exit side, they should. Let’s look at the developer map.

dev map

I’ve circled some of those cases on the above picture. They are all credibly paths from one room to another, cutting into and out of a third. Generally they show a door at the one end and not the other.

I have a plan for fixing that, but first, that duplication:

function Dungeon:horizontalCorridor(fromX, toX, y)
    local prev,curr
    local increment = 1
    if fromX > toX then
        increment = -1
    end
    prev = self:privateGetTileXY(fromX,y)
    for x = fromX,toX,increment do
        curr = self:privateGetTileXY(x,y)
        if not curr:isRoom() then
            curr = Tile:hall(x,y,curr.runner)
            self:defineTile(curr)
            if prev:isRoom() then
                curr:setCanBeDoor()
            end
        end
        prev = curr
    end
end

function Dungeon:verticalCorridor(fromY, toY, x)
    local prev,curr
    local increment = 1
    if fromY>toY then
        increment = -1
    end
    prev = self:privateGetTileXY(x,fromY)
    for y = fromY, toY, increment do
        curr = self:privateGetTileXY(x,y)
        if not curr:isRoom() then
            curr = Tile:hall(x,y,curr.runner)
            self:defineTile(curr)
            if prev:isRoom() then
                curr:setCanBeDoor()
            end
        end
        prev = curr
    end
end

All that stuff inside the for loops is identical. How do I know? I pasted it.

The inbound case–a hallway entering a place that is already tiled as isRoom–is that we find curr to be isRoom, and prev is isHall. We’ll have to check that before we clobber curr. Maybe we shouldn’t even reuse curr, that’s kind of tacky.

First, extract method:

function Dungeon:horizontalCorridor(fromX, toX, y)
    local prev,curr
    local increment = 1
    if fromX > toX then
        increment = -1
    end
    prev = self:privateGetTileXY(fromX,y)
    for x = fromX,toX,increment do
        curr = self:privateGetTileXY(x,y)
        self:setHallAndDoors(prev,curr)
        prev = curr
    end
end

function Dungeon:setHallAndDoors(prev,curr)
    if not curr:isRoom() then
        curr = Tile:hall(x,y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    end
end

function Dungeon:verticalCorridor(fromY, toY, x)
    local prev,curr
    local increment = 1
    if fromY>toY then
        increment = -1
    end
    prev = self:privateGetTileXY(x,fromY)
    for y = fromY, toY, increment do
        curr = self:privateGetTileXY(x,y)
        self:setHallAndDoors(prev,curr)
        prev = curr
    end
end

I expect this to work as before. It doesn’t. Let’s back it out and try again.

I need a save point. Commit: room exit doors properly marked (and displayed in red).

I really have no idea why that didn’t work. I’ll try to do the same thing again, not with redo but refactoring by hand, as before.

Hm I do get the same error:

1: Horizontal corridor -- Dungeon:399: attempt to index a nil value (field '?')

Let’s see what that is.

function Dungeon:defineTile(aTile)
    assert(not TileLock, "attempt to set tile while locked")
    local pos = aTile:pos()
    self.tiles[pos.x][pos.y] = aTile -- <-- 399
end

Is it the pos value that’s wrong, or is self.tiles missing?

It has to be either pos.x or pos.y being out of range. Does index a nil value mean that x or y are nil? Or pos?

Ah. Bad refactoring:

function Dungeon:setHallAndDoors(prev,curr)
    if not curr:isRoom() then
        curr = Tile:hall(x,y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    end
end

x and y aren’t defined. A refactoring tool would have caught that. But that’s ok, we can get them.

function Dungeon:setHallAndDoors(prev,curr)
    local pos = curr:pos()
    if not curr:isRoom() then
        curr = Tile:hall(pos.x,pos.y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    end
end

That works as before.

What about that revert?

Why did I revert first, and only debug the second time? Well, the refactoring was tricky enough that I thought I might have made an oversight, so I tried it again. Often when I do that, I don’t make the same mistake twice. This time, I did.

Anyway now we can have both horizontal and vertical use the same code. And now I can write a test for the entrance case. I think this requires a new test.

No, I think we can add a third room to the existing test and then check its tiles.

        _:test("connect rooms h then v", function()
            local tile
            local msg
            local r,x,y
            local dungeon = Runner:getDungeon()
            local r1 = Room(2,2,3,3, Runner)
            local r1x, r1y = r1:center()
            _:expect(r1x).is(3)
            _:expect(r1y).is(3)
            local r2 = Room(10,11,3,3, Runner)
            local r2x,r2y = r2:center()
            _:expect(r2x).is(11)
            _:expect(r2y).is(12)
            local r3 = Room(2,11, 3,3, Runner)
            local r3x,r3y = r3:center()
            _:expect(r3x).is(3)
            _:expect(r3y).is(12)
            r2:hvCorridor(dungeon,r1)
            checkRange(dungeon, r2x,r2y, r1x, r2y, Tile.isFloor)
            checkRange(dungeon, r1x,r2y, r1x, r1y, Tile.isFloor)
            tile = dungeon:privateGetTileXY(3,5)
            _:expect(tile:canBeDoor(),"door @ 3,5").is(true)
            tile = dungeon:privateGetTileXY(9,12)
            _:expect(tile:canBeDoor(),"door @ 9,12").is(true)
            tile = dungeon:privateGetTileXY(3,10)
            _:expect(tile:canBeDoor(),"door @ 3,10").is(true)
            tile = dungeon:privateGetTileXY(5,12)
            _:expect(tile:canBeDoor(),"door @ 5,12").is(true)
        end)

I’ve added a 3x3 room directly above room1 and to the left of room2. It should get doors as shown. I expect that the 3,10 door will pass and the 5,12 won’t. Let’s see how wrong I am.

4: connect rooms h then v door @ 3,10 -- Actual: false, Expected: true
4: connect rooms h then v door @ 5,12 -- Actual: false, Expected: true

Meh. Why didn’t the 3,10 one work? Ah. I’m wrong. In this case, neither should work. Both those doors are inbound, one from the right, one from below. Whew! I was scared there for a minute.

Now for the new feature in our newly extracted method:

function Dungeon:setHallAndDoors(prev,curr)
    local pos = curr:pos()
    if not curr:isRoom() then
        curr = Tile:hall(pos.x,pos.y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    end
end

Let’s see. The entering a room case has curr as isRoom and prev as isHall, so it can be an else:

function Dungeon:setHallAndDoors(prev,curr)
    local pos = curr:pos()
    if not curr:isRoom() then
        curr = Tile:hall(pos.x,pos.y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    else -- curr is room, may need to set prev
        if prev:isHall() then
            prev:setCanBeDoor()
        end
    end
end

We don’t have to define the tile, because it’s already a room tile and we don’t overwrite those.

I rather expect this to pass the tests and to look right on screen. But no:

4: connect rooms h then v door @ 3,10 -- Actual: false, Expected: true
4: connect rooms h then v door @ 5,12 -- Actual: false, Expected: true

I think I’d like to see if this is ever happening, so I put in a print, and it isn’t happening. I recall a learning from yesterday, which is that tables are set by reference, which means that resetting the curr tile inside my new function doesn’t change curr outside.

So:

function Dungeon:horizontalCorridor(fromX, toX, y)
    local prev,curr
    local increment = 1
    if fromX > toX then
        increment = -1
    end
    prev = self:privateGetTileXY(fromX,y)
    for x = fromX,toX,increment do
        curr = self:privateGetTileXY(x,y)
        curr = self:setHallAndDoors(prev,curr)
        prev = curr
    end
end

function Dungeon:setHallAndDoors(prev,curr)
    local pos = curr:pos()
    if not curr:isRoom() then
        curr = Tile:hall(pos.x,pos.y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    else -- curr is room, may need to set prev
        if prev:isHall() then
            prev:setCanBeDoor()
        end
    end
    return curr
end

I expect this to work. Tests pass. Here’s the map:

map right

Check out that room at the top. The multi-hall cells are all properly marked as canBeDoor. When we install actual doors, we will of course want to check for this kind of case, butt the tiles are marked just as intended.

That’s what I’m talkin’ about!

Turn off the red? No, I think I’ll leave it turned on just for sake of interest.

Commit: hallway tiles abutting rooms are all marked as canBeDoor.

Let’s … Sum Up

Well now, I told you we’d bite the bear today, didn’t I? And we did.

It went rather well. We made a place to stand by adding a new kind of tile, and modifying the system so that the tw type types, room and hall, behaved just as if they were both room tiles. That did involve a tiny bit of thinking, as there was some code figuring out whether tiles were hallway tiles or not, and that had to be changed differently from most of the other references.

All that was done to change no behavior but to make it possible to do what we had to do, arrange hallway carving to detect the borders between rooms and halls.

We did that in two passes, first handling the transition from room to hall, and when that worked, doing the transition from hall to room.

That all went rather nicely, except that a manual refactoring, extracting a method from horizontal and vertical carvers, was wrong. Let me rephrase that: I did it wrong. Once it was spotted, it was an easy fix.

We could have fixed it differently, by passing in x and y rather than pulling it out of the tile. I’m not sure why I preferred pulling it out. Let’s look:

function Dungeon:setHallAndDoors(prev,curr)
    local pos = curr:pos()
    if not curr:isRoom() then
        curr = Tile:hall(pos.x,pos.y,curr.runner)
        self:defineTile(curr)
        if prev:isRoom() then
            curr:setCanBeDoor()
        end
    else -- curr is room, may need to set prev
        if prev:isHall() then
            prev:setCanBeDoor()
        end
    end
    return curr
end

Let’s clarify that a bit to make a point:

function Dungeon:setHallAndDoors(prevTile,currTile)
    local pos = currTile:pos()
    if not currTile:isRoom() then
        currTile = Tile:hall(pos.x,pos.y,currTile.runner)
        self:defineTile(currTile)
        if prevTile:isRoom() then
            currTile:setCanBeDoor()
        end
    else -- curr is room, may need to set prev
        if prevTile:isHall() then
            prevTile:setCanBeDoor()
        end
    end
    return currTile
end

I renamed prev and curr to prevTile and currTile. Now when we look at this method we see that almost everything it does is manipulate tiles, not the current class, Dungeon. We have feature envy.

Intuitively I felt that tiles know their x and y, and that passing x and y around all naked was wrong. And, perhaps, I was forming the feeling that this method really wants to be a method on Tile, calling back to dungeon or perhaps returning the desired tile to dungeon.

That’s for another day, but let’s do commit this renaming: rename parms in setHallAndDoors to highlight feature envy.

So. Was yesterday wasted and today pulling our nuts out of the fire, whatever that means? I think not. I think that the work done yesterday, which didn’t result in a solution, refined my thinking about exit and entrance.

It also included a key notion, that of always drawing hallways starting in a room, which made thinking about the state changes easier. I do suspect that the state changes will work in either direction but they certainly do work now.

A good day. Sorry, bear. See you next time!

D2.zip

  1. I’m a math major, Jim, not an arithmetic major. ↩