Dungeon 12: Friday the 13th
My plan, the day notwithstanding, is to get door markers drawn and, with luck, improve the design. But I’m not sure that’s what I’d advise.
Years ago, I decided that Friday the 13th would be my lucky day. I’m not sure that it worked, I’ve been rather lucky every day. Before we dig in, we should have a little talk.
I’ve been building dungeon infrastructure for 11 days now, going on 12. We have nothing running that looks much like a game, just some moving boxes and lines drawn between them. If I were advising a team charged with building a rogue-like dungeon game, I’d be advising them to have something that looks like a game by now.
I believe strongly that in a product situation, it is very important that the product be visibly improving, becoming more and more like the desired product, essentially every day, certainly every week. Someone once said “All methodologies are based on fear”. I think it was Kent Beck, ah yes, in Extreme Programming Explained. What I fear in product development is that management will not see progress and that they will first put us under great pressure, which will slow us down even more, and then finally just as we can see the light at the end of the tunnel, they’ll terminate the project or fire us or something else bad.
I fear that, of course, because it has happened to me. I think that if the product were visible, and improving, management would be less likely to do that, or at least less likely to do it when they shouldn’t. So I always recommend visible product, getting more like The Product every day.
And here, I’m not doing that. Of course, we’re only 11.1 days into the effort, so it might not be time to panic.
But my real reason for working this way on this series is that I’m enjoying it. I enjoy crafting code, figuring out good ways to represent the ideas of the product and the ideas in the code. It’s my series, and I get to do what I want. And there are lessons here about design, and design evolution, that apply everywhere.
But enough. What’s the plan for today?
The Plan for Today Is …
Well, to figure out what to do today. The thing I’m working on is to display on the large map where all the doors will be. From a product viewpoint, we can’t draw a room if we don’t know where the doors are, we can’t decide whether the player or the monster can get into the room if we don’t know where the doors are.
And in the Zoom Ensemble I’m working with, we’re having trouble with the doors. I want to avoid that trouble, and if working with doors here equips me to help better, that’ll be great as well. So that’s what we’re at.
Now the program knows everything we need, at some point. We have the “overlap center” functions, which return one screen coordinate at the center of, well, the overlap of the two rooms, and that’s where we want to put the door center. And the program knows which wall the door goes on, because at some point we compare E.G. Without Loss Of Generality his north and our south to see if they’re close enough together to merit a door.
What I’d like to have is for each room to have a collection of doors, each one knowing both coordinates of an overlap center. The room would draw those doors on the large map, and when we get to drawing expanded rooms, the information would be there.
Right now, Neighbor has this unused but tested method:
function Neighbor:getDoor(aRoom)
local sp
local x,y
local wall,center
local otherRoom = self:otherRoom(aRoom)
if aRoom:closeEnoughOnWestTo(otherRoom) then
sp = SegmentPair:forY(aRoom,otherRoom)
wall = aRoom:east()
center = sp:overlapCenter()
return wall,center
elseif aRoom:closeEnoughOnEastTo(otherRoom) then
sp = SegmentPair:forY(aRoom,otherRoom)
wall = aRoom:west()
center = sp:overlapCenter()
return wall,center
elseif aRoom:closeEnoughOnNorthTo(otherRoom) then
sp = SegmentPair:forX(aRoom,otherRoom)
center = sp:overlapCenter()
wall = aRoom:south()
return center,wall
elseif aRoom:closeEnoughOnSouthTo(otherRoom) then
sp = SegmentPair:forX(aRoom,otherRoom)
center = sp:overlapCenter()
wall = aRoom:north()
return center,wall
else
return nil
end
end
I’m not sure quite why I thought we needed that, and I don’t exactly love the look of it, but it does give us the information we need. I think we could use this to create a Door object that contained the two coordinates that this method returns. The name of getDoor, of course, should probably be getDoorLocation. Be that as it may, where can we use this?
OK, enough thinking. That’s the question. At what point in the program do we have a grasp on the two rooms, so that we’re in a position to create the two doors (one for each room) needed?
There are other questions, not least that this function probably duplicates some work that’s already done in identifying Neighbors, but we’ll worry about that when it bothers us. Right now it’s not happening ever: we never call this method.
This is in Neighbor. Neighbor creation goes like this:
function Neighbor:primary(room1, room2)
return Neighbor(room1,room2,true)
end
function Neighbor:secondary(room1, room2)
return Neighbor(room1,room2,false)
end
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
return n
else
return nil
end
end
function Neighbor:init(room1, room2, isPrimary)
self.isPrimary = isPrimary
self.room1 = room1
self.room2 = room2
if room1.number > room2.number then
self.room2,self.room1 = room1,room2
end
self.mt = getmetatable(self)
self.mt.__tostring = self.toString
end
The metatable stuff is something I’m experimenting with. That gives all Neighbors a new function __tostring, which happens to be called whenever you print an object. So Neighbors print according to the toString function. Maybe we’ll look at that later.
A quick search tells me that no one calls Neighbor() directly, other than one test and the code here. So inside Neighbor should be a perfectly good place to call back to both rooms and tell them “you have a door at these coordinates” or maybe just “you have a door, inquire within if you care”.
Neighbors get created here:
function Room:colorNeighborsIn(rooms, aColor)
self:setColor(aColor)
for i,r in ipairs(rooms) do
if r:isNeighbor(self) then
if r:getColor() ~= aColor then
self:insertNewNeighbor(Neighbors, Neighbor:primary(self, r))
r:colorNeighborsIn(rooms, aColor)
else
self:insertNewNeighbor(Neighbors, Neighbor:secondary(self, r))
end
end
end
end
Now the insertNewNeighbor function is adding a new Neighbor to the Neighbors table, which is contains all the two-room connections. This doesn’t seem much like it really belongs in Room, but that’s for another day. We could create the doors right here. But I think calling back is better.
Dilemma
Should I TDD this? I should. But how? I’d have to create some rooms and align them as neighbors and then check them all to see if they get all the right doors. Meh, that’s boring.
What about our adjacency tests? We have enough to give me confidence in adjacency logic. Or maybe we should extend the getDoor tests. Ah, good idea. Here they are:
_:test("East-West Neighbor has Door information", function()
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(201,124, 301, 144)
_:expect(r1:ewCloseEnough(r2)).is(true)
local sp = SegmentPair:forY(r1,r2)
_:expect(sp:overlapLength()).is(20)
_:expect(r1:closeEnoughOnWestTo(r2), "not west?").is(true)
local n = Neighbor:fromRooms(r1,r2)
_:expect(n).isnt(nil)
local r1x,r1y = n:getDoor(r1)
_:expect(r1x).is(200)
_:expect(r1y).is(134)
local r2x,r2y = n:getDoor(r2)
_:expect(r2x).is(201)
_:expect(r2y).is(134)
end)
_:test("North-South Neighbor has Door information", function()
local r1 = Room:fromCorners(100,100,200,100)
local r2 = Room:fromCorners(125, 50, 145, 98) -- south and smaller
local n = Neighbor:fromRooms(r1,r2)
local r1x,r1y = n:getDoor(r1)
_:expect(r1x).is(135)
_:expect(r1y).is(100)
local r2x,r2y = n:getDoor(r2)
_:expect(r2x).is(135)
_:expect(r2y).is(98)
end)
Perfect, sort of.
What we’re about to do is extend `Neighbor:fromRooms()’ so that it calls back to the rooms, inviting them to add a door. And we’re going to have the rooms in fact add doors. So we can check here to see if our rooms have added a door.
Somewhat messy but it should be a decent test. I’m just going to invade the room to get the info, at least for now. And I think I’ll duplicate one of these tests, to leave out the messy bits of these tests. It’ll make it more clear what we’re testing. Here’s the shell:
_:test("North-South Neighbors get Doors", function()
local r1 = Room:fromCorners(100,100,200,100)
local r2 = Room:fromCorners(125, 50, 145, 98) -- south and smaller
local n = Neighbor:fromRooms(r1,r2)
_:expect(n).isnt(nil)
end)
This works: we did get a Neighbor back. Now let’s check the door tables in and out:
local r1 = Room:fromCorners(100,100,200,100)
local dt1 = r1.doors
_:expect(#dt1).is(0)
This will drive out an empty collection in Room:
20: North-South Neighbors get Doors -- Tests:238: attempt to get length of a nil value (local 'dt1')
We can fetch anything from a table. If it’s not there, we get a nil. This is part of why it’s better not to rip the guts out of other objects. For this test, I’ll allow it.
We respond to this error:
function Room:init(x,y,w,h)
self.number = 666
self.color = color(0,255,0,25)
self.x = x or math.random(WIDTH//4,3*WIDTH//4)
self.y = y or math.random(HEIGHT//4,3*HEIGHT//4)
self.w = w or math.random(WIDTH//40,WIDTH//20)*2
self.h = h or math.random(HEIGHT//40,WIDTH//20)*2
self.doors = {}
end
Test runs. But now:
_:test("North-South Neighbors get Doors", function()
local r1 = Room:fromCorners(100,100,200,100)
local dt1 = r1.doors
_:expect(#dt1).is(0)
local r2 = Room:fromCorners(125, 50, 145, 98) -- south and smaller
local n = Neighbor:fromRooms(r1,r2)
_:expect(n).isnt(nil)
dt1 = r1.doors
_:expect(#dt1).is(1)
end)
20: North-South Neighbors get Doors -- Actual: 0, Expected: 1
Our cunning plan is coming together. Now …
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
room1:addDoor(self)
room2:addDoor(self)
return n
else
return nil
end
end
When the neighbor is determined to be legit, we inform the rooms to add a door.
Now the test will fail differently:
20: North-South Neighbors get Doors -- Neighbor:19: attempt to call a nil value (method 'addDoor')
We respond:
function Room:addDoor(aNeighbor)
local x,y = aNeighbor:getDoor(self)
table.insert(self.doors, {x=x,y=y})
end
I’m putting a little hash table in there for now. This should make the test pass … but it doesn’t …
20: North-South Neighbors get Doors -- Room:210: attempt to index a nil value (local 'self')
Huh. That’s this:
function Room:west()
return self.x - self:halfWidth()
end
That means that someone has called Room:west without providing a parameter. There’s that cool trick from yesterday …
There’s only one direct call to :west in the whole system, and it looks OK to me. Where’s that fancy stuff? That looks OK too.
Gumption trap. Clearly the addDoor is the culprit.
After some delay and some asserts, I find this:
function Neighbor:getDoor(aRoom)
local sp
local x,y
local wall,center
local otherRoom = self:otherRoom(aRoom)
assert(otherRoom, "other room is nil")
The otherRoom comes back nil. That’s odd because:
function Neighbor:otherRoom(aRoom)
if self.room1 == aRoom then
return self.room2
else
return self.room1
end
end
So, somehow, room2 must be nil.
Ah:
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
room1:addDoor(self)
room2:addDoor(self)
return n
else
return nil
end
end
We’re in a constructor. We’re passing self, i.e. the class, to addDoor. Should be:
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
room1:addDoor(n)
room2:addDoor(n)
return n
else
return nil
end
end
And our test runs. Prints and asserts removed. Commit: Room:addDoor sketched in.
That was grueling. I deserve a chai. Back soon.
See? No time at all. Now then … we just have that one test, but since we are white box testing, I think we can be pretty sure that all the rooms now know all their doors. Let’s see if we can draw them:
function Room:draw()
pushMatrix()
fill(255)
stroke(255)
strokeWidth(0)
textMode(CENTER)
translate(self.x,self.y)
local nr = string.format("%d", self.number)
text(nr,0,0)
fill(self.color)
stroke(0,255,0)
strokeWidth(2)
rect(0,0, self.w, self.h)
self:drawDoors()
popMatrix()
end
function Room:drawDoors()
pushMatrix()
pushStyle()
fill(255)
for i,door in ipairs(self.doors) do
translate(door.x,door.y)
rect(0,0,3,3)
end
popStyle()
popMatrix()
end
It surprises me that that does nothing. There must be something I’m not remembering about how this thing is wired up. We must not be calling back from our Neighbor creation. There are all those odd creation methods, with primary and secondary and all that. I bet they’re not all going through the addRoom code.
It all begins here:
function Room:colorNeighborsIn(rooms, aColor)
self:setColor(aColor)
for i,r in ipairs(rooms) do
if r:isNeighbor(self) then
if r:getColor() ~= aColor then
self:insertNewNeighbor(Neighbors, Neighbor:primary(self, r))
r:colorNeighborsIn(rooms, aColor)
else
self:insertNewNeighbor(Neighbors, Neighbor:secondary(self, r))
end
end
end
end
function Room:insertNewNeighbor(neighbors, neighbor)
if not self:neighborPresent(neighbors,neighbor) then
neighbors[neighbor:key()] = neighbor
end
end
But in Neighbor tab:
function Neighbor:primary(room1, room2)
return Neighbor(room1,room2,true)
end
function Neighbor:secondary(room1, room2)
return Neighbor(room1,room2,false)
end
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
room1:addDoor(n)
room2:addDoor(n)
return n
else
return nil
end
end
Right. Turns out fromRooms is that method just created for testing. So we need to call back from a bit further in. I’ll try this:
function Neighbor:init(room1, room2, isPrimary)
self.isPrimary = isPrimary
self.room1 = room1
self.room2 = room2
if room1.number > room2.number then
self.room2,self.room1 = room1,room2
end
self.room1:addDoor(self)
self.room2:addDoor(self)
self.mt = getmetatable(self)
self.mt.__tostring = self.toString
end
This does something, but nothing good:
You see those white dots? Those are the doors. They don’t seem to be in really great positions, do they?
Oh. This is better:
I changed this:
function Room:draw()
pushMatrix()
fill(255)
stroke(255)
strokeWidth(0)
textMode(CENTER)
translate(self.x,self.y)
local nr = string.format("%d", self.number)
text(nr,0,0)
fill(self.color)
stroke(0,255,0)
strokeWidth(2)
rect(0,0, self.w, self.h)
popMatrix()
self:drawDoors()
end
The drawDoors used to be above the popMatrix, so the translate in drawDoors added to the one that moved us to the center.
However, we still have some stars out in space. Looking carefully at the picture, it appears to me that the primary doors are all good, but there are no door marks showing for the secondary (yellow) lines. It seems to be a good guess that the stars are secondaries, somehow misplaced.
function Room:colorNeighborsIn(rooms, aColor)
self:setColor(aColor)
for i,r in ipairs(rooms) do
if r:isNeighbor(self) then
if r:getColor() ~= aColor then
self:insertNewNeighbor(Neighbors, Neighbor:primary(self, r))
r:colorNeighborsIn(rooms, aColor)
else
self:insertNewNeighbor(Neighbors, Neighbor:secondary(self, r))
end
end
end
end
I don’t see anything weird here, except for the recursive call to colorNeighborsIn, which is weird enough. But we wind up passing the same values to both creators. Those are pretty much the same and pass info to the same init:
function Neighbor:primary(room1, room2)
return Neighbor(room1,room2,true)
end
function Neighbor:secondary(room1, room2)
return Neighbor(room1,room2,false)
end
function Neighbor:init(room1, room2, isPrimary)
self.isPrimary = isPrimary
self.room1 = room1
self.room2 = room2
if room1.number > room2.number then
self.room2,self.room1 = room1,room2
end
self.room1:addDoor(self)
self.room2:addDoor(self)
self.mt = getmetatable(self)
self.mt.__tostring = self.toString
end
This is too weird things this morning. This is a sign that I don’t fully understand this code. (I initially typed “fukly” there. Thank you, Dr Freud.) That’s a sign that the code needs to be more clear, because if it’s simple enough even I can understand it.
I think I need to be able to dump the doors from a room and see what I can find out.
function Room:dumpDoors()
print("Room ", self.number,"",self.x,"",self.y)
for i,d in ipairs(self.doors) do
print(i, "", d.x,"",d.y)
end
end
The quotes are to deal with a glitch in Codea print, where he doesn’t ensure a space between values printed. Here’s the pic I’m working from:
I dump Room1, which should have two doors:
Room 1 782.0 600.0 80 62
1 822.0 604.0
2 822.0 604.0
3 782.0 569.0
4 782.0 569.0
Right away we see one problem, which is that we’re creating doors twice. This will be because we check every room against every other, and then suppress duplicates when we go to store them in Neighbors.
As for the values, the room’s corners (kind of wish I had printed them) must be 742,569 and 822,632, or something about like that. So 822 is a valid East wall, and 604 is close to the middle, as we can see in the picture, and 782 is the room center, and 569 is down below it, straight down. So that’s the center of the south wall, and that’s in fact where the door should be. Why isn’t it there?
function Room:drawDoors()
pushMatrix()
pushStyle()
fill(255)
stroke(255)
for i,door in ipairs(self.doors) do
translate(door.x,door.y)
rect(0,0,5,5)
end
popStyle()
popMatrix()
end
If we’re going to translate, we should push and pop the matrix every time. Let’s just not translate.
function Room:drawDoors()
pushMatrix()
pushStyle()
fill(255)
stroke(255)
for i,door in ipairs(self.doors) do
rect(door.x,door.y,10,10)
end
popStyle()
popMatrix()
end
I expanded them to 10x10 to make them more visible. And now we get this:
That looks just right. I take back what I said about not understanding the code, the bug was in the display, not in the creation. That makes me think that my model of what’s going on isn’t wrong. It’s surely incomplete, I can’t remember every detail, but it’s not inconsistent with reality.
I still think things could be more clear.
But we can commit: Rooms know their doors.
This is a good point to break and sum up. So let’s break and sum up. Or sum up and then break.
Summary
Today we closed the loop on a fairly long cycle of work that started long ago with the Neighbor concept, then more recently focused in a bit with the getDoors notion. Then we reorganized the code quite a bit, still not entirely to my satisfaction, and today we linked it all together and aside from a few stumbles, it hangs together.
The stumbles included just getting too fancy with drawing. I like using translate and rotate and all those. They’re very useful when you’re doing sophisticated graphics, because they let you draw anything as though you were drawing it in the center of the screen, and you can ignore a lot of extraneous details. But they are a bit obscure, and if you don’t use them correctly, your drawing wanders off into space …
So maybe that was too fancy, maybe too careless. Anyway, it didn’t take long to find, but it did weaken my confidence, and I so note.
The other issue was in the class creation method:
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
return n
else
return nil
end
end
There were two stumbles here. First done, last discovered, was that this was not a good place to put the calls to addRoom because this is a test-only method. Let’s mark it right now. We should rename, but I don’t want to do a major edit right now.
-- THIS METHOD IS FOR TEST ONLY
function Neighbor:fromRooms(room1, room2)
local n = Neighbor(room1,room2, nil)
if room1:isNeighbor(room2) then
return n
else
return nil
end
end
Commit: commented a test method.
The second error, found first, was that when I called addRoom, I called it with self, not n, and self was the Neighbor class, not an instance, and hilarity ensued. Also confusion.
So I guess I’d have to say that my work today feels a bit slapdash. I wasn’t as focused as I sometimes am. (Probably I was closer to normal than I sometimes am.) There’s not much I can recommend about that. We all go through cycles where we’re really on our game and where we’re a bit off. I guess the best we can do is pay attention, notice when we’re not playing as well as we might, and try to be more careful.
It took me longer than I’d like to find these problems, and it would have been far better not to put them in. Curiously, though, the tests hardly helped, even when failing, because the defect appeared way down in the guts somewhere. And all the code was good … it was just being passed a bad argument that finally got referenced. It’s good that the test found it. And I don’t see how a better test could have found it sooner, at least not a test that I’d be likely to consider writing.
But all in all, some stumbling but a success: we have a rudimentary door notion in the system now.
See you next time!