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"
``````

``````local TileHall = "hall"
``````

``````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.

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
``````

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.

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?

``````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.

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:

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.

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.