Dungeon 21
Another run at moving into the new hallway rooms and back out again. Fortunately, as I mention at the end, my purpose isn’t to show you how smart I am.
My last session was so horrid that I just scrapped all the work. I spent half my unproductive time debugging, and then the next half trying to write a test. The latter was closer to working, but my brain fried, so I stopped. I could have saved the test, but I felt it would probably be better to start over with a clear head.
Our mission is to allow the avatar to move from one room into another, by attempting a move beyond the boundary of the current room, and having a neighboring room discover that the new point is inside it. So we “transform” the avatar position from being somewhere in the first room, to being somewhere in the other room.
Before we dig into this, I think I’d like to make some improvements to the existing code to make things a bit more reasonable.
One thing I’d like to do is to change the representation in RoomPosition. Presently, it looks like this:
function RoomPosition:init(aRoom, x, y)
self.room = aRoom
self.x = x
self.y = y
end
But what is x? It turns out that it is supposed to be the room-relative position of the avatar. We create RoomPositions this way.
function Player:init(aRoomNumber, x, y)
self.position = RoomPosition(Rooms[aRoomNumber], x, y)
end
And …
function RoomPosition:clamp(halfW,halfH, wall)
local lox = -halfW+wall
local hix = halfW-wall
local loy = -halfH+wall
local hiy = halfH-wall
local x = clamp(lox,self.x,hix)
local y = clamp(loy,self.y,hiy)
return RoomPosition(self.room,x,y)
end
And, in Main:
Avatar = Player(1,0,0)
So the x and y are room local. But to decide whether we’ve moved beyond a room’s x and into the neighbor’s x, we have to convert back and forth between screen coordinates and local. I want to go to screen coordinates throughout.
Let’s start at player creation. Let’s pass RoomPosition a room, not a room number, and default the coordinates.
Avatar = Player(Rooms[1])
function Player:init(aRoom, x, y)
self.position = RoomPosition(aRoom, x, y)
end
function RoomPosition:init(aRoom, x, y)
local center = aRoom:center()
self.room = aRoom
self.x = x or center.x
self.y = y or center.y
end
OK at this point, we’ll set the room position x and y for our player to the room center. (Room:center() returns a vec2.)
function RoomPosition:screenLocation()
return self.x,self.y
end
Now there’s this:
function RoomPosition:clamp(halfW,halfH, wall)
local lox = -halfW+wall
local hix = halfW-wall
local loy = -halfH+wall
local hiy = halfH-wall
local x = clamp(lox,self.x,hix)
local y = clamp(loy,self.y,hiy)
return RoomPosition(self.room,x,y)
end
I’m not sure what’s going on there. Who calls this?
function Room:stayInRoom(aRoomPosition)
local w = self.w/2
local h = self.h/2
return aRoomPosition:clamp(w,h,5)
end
That’s called here:
function Room:legalMoveTo(aRoomPosition)
for k,door in pairs(self.doors) do
if door:contains(aRoomPosition) then
return aRoomPosition
end
end
return self:stayInRoom(aRoomPosition)
end
Now I decided yesterday not to use the wall margin (5) at all. So we want this method to be just clamp().
function Room:stayInRoom(aRoomPosition)
return aRoomPosition:clamp()
end
function RoomPosition:clamp()
lox,loy,hix,hiy = self.room:corners()
local x = clamp(lox,self.x,hix)
local y = clamp(loy,self.y,hiy)
return RoomPosition(self.room,x,y)
end
OK, Let’s run this and see what it does.
She’s out in space. Why isn’t she at the center of Room 1? Ah. We can’t create her until the rooms stabilize now, since we need the room’s real coordinates.
I’m concluding that making the RoomPosition work with full screen coordinates isn’t a good enough idea to justify the cost of the change.
Revert.
Let’s do change the clamping to remove the 5 and to call back for the necessary info:
function Room:stayInRoom(aRoomPosition)
return aRoomPosition:clamp()
end
Commit: improve clamp.
Saturn’s Day
I started this article Friday night, and it’s now 0810 Saturday. I’m still on the quest to get the princess to move between two rooms, and still on the subtask of tidying up the code a bit before taking another run at the real issue.
This little project has gone a long time with no interesting deliverables, and I’m sure my management would be wondering what I’ve been doing for a whole week, or however long it has been.
But tidying up is important. My work the past couple of days has kind of confused me, and that’s a signal that the code is confusing. It could also be a sign of intellectual decline, but I’m discounting that for personal reasons. So I want the code to be as simple, and as clear, as I can reasonably accomplish.
Since we’re moving the Player, I’m still giving that class, and the RoomPosition that it uses, a bit of focus. Last night, I tried to switch RoomPosition to use screen coordinates instead of room-local coordinates, but that didn’t go well. One issue with that idea was that the Avatar is created during setup, and then the rooms move around. Since the Avatar’s RoomPosition was expressed in screen coordinates, it didn’t move, and the result was that she was off in space somewhere. We’ll stick with room-relative for now, and convert back and forth as we may need to.
RoomPosition will have to deal with local and global coordinates anyway, so maybe it will balance out.
Anyway here’s a little thing in Player:
function Player:keyDown(aKey)
local step = 2
if aKey == "w" then self.position = self.position:move(0,step)
elseif aKey == "a" then self.position = self.position:move(-step,0)
elseif aKey == "s" then self.position = self.position:move(0,-step)
elseif aKey == "d" then self.position = self.position:move(step,0)
end
end
This is, of course, what moves the player around. It contains duplication. Let’s refactor:
function Player:keyDown(aKey)
local step = 2
if aKey == "w" then self:move(0,step)
elseif aKey == "a" then self:move(-step,0)
elseif aKey == "s" then self:move(0,-step)
elseif aKey == "d" then self:move(step,0)
end
end
function Player:move(dx,dy)
self.position = self.position:move(dx,dy)
end
Not much difference, but a bit nicer. To keep in the habit of frequent commits, commit: refactor Player:keyDown.
Speculation
Having had so much trouble with a seemingly simple idea, I’ve speculated quite a lot about what could be going wrong with moving between rooms. The scheme is very simple, it seems to me.
We’ve build hallway rooms that are adjacent to regular rooms. The hallway room’s end butts right up against the regular room’s side. If the regular room ends at 200, then the hallway starts at 201. So if we were to try to step the princess beyond 200 to 201 or 202, the hallway room would see that she’s inside it and report back a position in the new room.
It occurs to me that the princess moves by 2 pixels right now. If two rooms are already directly adjacent, which happens a lot, the hallway will have zero length and so when she steps forward she can’t possibly hit it. At one time I had a plan for that. The idea was that the hallways would be created with a small extension on each end, into the two rooms they connect. Think of a doormat. So there would always be a bit of doormat to step on, causing acceptance into the hallway room. We may have to go back to that.
However, there are plenty of hallways with non-zero length as well, so I think I can proceed without worrying about the zero length case yet.
So what could be going wrong? Again, this is speculation, and only the code knows. But we’ve reverted all the code, so this is more consideration of what needs testing.
- If somehow the hallways were created not to be adjacent, then when the princess steps beyond the room’s boundary, the hallway would not yet accept her, and the room would move her back inside. So gaps between room and hallway would be bad.
- There could be something wrong with the code that checks to see if the hallway room wants to receive the avatar.
Clearly I need good tests against both these possibilities. So let’s explore what we have now:
What we have now …
We have this class called “Door”. I used to have in mind that it would be the connection that you’d travel to get from one room to the next. The current scheme is that we just use the doors to produce new full-fledged rooms between the original rooms connected by the door. The process goes like this:
As we draw, we’re in the zoomed out mode, and we do this:
function drawDungeonCreation()
background(40, 40, 50)
if CodeaUnit then showCodeaUnitTests() end
pushMatrix()
pushStyle()
for i,r in ipairs(Rooms) do
r:draw()
end
Avatar:draw()
adjustPositions()
popStyle()
popMatrix()
end
function adjustPositions()
if AdjustCount > 0 then
if ElapsedTime - AdjustTime > AdjustDelay then
AdjustTime = ElapsedTime
AdjustCount = Room:adjustAll(Rooms)
if AdjustCount == 0 then
Room:setAllToInteger(Rooms)
Room:createFloors(Rooms)
Rooms[1]:colorNeighborsIn(Rooms, color(255,0,0,100))
createRoomsFromDoors()
end
end
end
end
After the adjustment is over, we make sure the rooms are on integer boundaries, create their floors, find connections, and then call the relatively new createRoomsFromDoors, which is what creates our hallway rooms. That goes like this:
function createRoomsFromDoors()
for k,door in pairs(AllDoors) do
local room = door:createRoom()
room.number = #Rooms+1
table.insert(Rooms,room)
end
end
I should mention that I’m not very fond of the room numbering and defaults. But what about tests? We do have two tests for createRoom:
_:test("Rooms from doors horizontal", function()
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local x1,y1 = d:getDoorCoordinates(r1)
_:expect(x1).is(200)
_:expect(y1).is(150)
local x2,y2 = d:getDoorCoordinates(r2)
_:expect(x2).is(210)
_:expect(y2).is(y1)
local newRoom = d:createRoom()
local nx1,ny1,nx2,ny2 = newRoom:corners()
_:expect(nx1).is(201)
_:expect(nx2).is(209)
_:expect(ny1).is(146)
_:expect(ny2).is(154)
end)
_:test("Rooms from doors vertical", function()
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(125,210, 175, 310)
local d = Door:fromRooms(r1,r2)
local x1,y1 = d:getDoorCoordinates(r1)
_:expect(x1).is(150)
_:expect(y1).is(200)
local x2,y2 = d:getDoorCoordinates(r2)
_:expect(x2).is(x1)
_:expect(y2).is(210)
local newRoom = d:createRoom()
local nx1,ny1,nx2,ny2 = newRoom:corners()
_:expect(nx1).is(146)
_:expect(nx2).is(154)
_:expect(ny1).is(201)
_:expect(ny2).is(209)
end)
Neither of these deals with the short zero-length door concern, but I plan to defer that. Let’s do an ignored test to nag a bit:
_:ignore("Test adjacent room-door-zero-lenth hallway", function()
_:expect(false).is(true)
end)
Those two existing tests give me enough confidence that the longer hallways, at least, will be adjacent to the rooms in question, so I think we can proceed to test movement between rooms.
Finally. But one does have to plan sometimes. I think I’ll copy one of those tests and then move its story forward. Starting from this:
_:test("Avatar uses horizontal hallway", function()
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local x1,y1 = d:getDoorCoordinates(r1)
_:expect(x1).is(200)
_:expect(y1).is(150)
local x2,y2 = d:getDoorCoordinates(r2)
_:expect(x2).is(210)
_:expect(y2).is(y1)
local newRoom = d:createRoom()
local nx1,ny1,nx2,ny2 = newRoom:corners()
_:expect(nx1).is(201)
_:expect(nx2).is(209)
_:expect(ny1).is(146)
_:expect(ny2).is(154)
end)
Now how are doors normally created? Do we use fromRooms in or regular code? We do not. So this isn’t following the real code. The door creation must be done in the coloring method? Right, it’s done 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:insertNewDoor(AllDoors, Door:primary(self, r))
r:colorNeighborsIn(rooms, aColor)
else
self:insertNewDoor(AllDoors, Door:secondary(self, r))
end
end
end
end
We create primary or secondary doors. What does that do?
function Door:primary(room1, room2)
return Door(room1,room2,true)
end
function Door:secondary(room1, room2)
return Door(room1,room2,false)
end
-- THIS METHOD IS FOR TEST ONLY
function Door:fromRooms(room1, room2)
local n = Door(room1,room2, nil)
if room1:isNeighbor(room2) then
return n
else
return nil
end
end
-- instance methods
function Door: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
OK, it looks to me as if everyone really goes through the same init so we should be OK.
The self.mt stuff is a Lua trick to give the Door object a tostring method that gives it a pretty print instead of just saying “table: 0x1234654” or something.
I’m comfortable extending that test. I’ll pull out the redundant asserts and comment. And check to see that the room now has the neighbor:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(d)
end)
Error is:
25: Avatar uses horizontal hallway -- CodeaUnit:94: bad argument #1 to 'for iterator' (table expected, got nil)
Makes me think that r1 doesn’t have any neighbors. Do we have that concept implemented? Shouldn’t the table at least be there, empty? It should, but that was done in a thread we reverted. Here’s Room:init:
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)//8)*8
self.h = h or ((math.random(HEIGHT//40,HEIGHT//20)*2)//8)*8
self.doors = {}
end
Add
self.neighbors = {}
But what’s going on with that room from door creation? Aren’t we giving them back to the rooms? That’s done like this:
function createRoomsFromDoors()
for k,door in pairs(AllDoors) do
local room = door:createRoom()
room.number = #Rooms+1
table.insert(Rooms,room)
end
end
The rooms are added to the main room collection, but they are not given back to the rooms as neighbors. We have to invent that concept. We’ll have createRoom do it:
function Door:createRoom()
local roomHalfWidth = 4
local x1,y1 = self:getDoorCoordinates(self.room1)
local x2,y2 = self:getDoorCoordinates(self.room2)
if y1 == y2 then -- horizontal
if x2 < x1 then
x1,x2 = x2,x1
y1,y2 = y2,y1
end
y1 = y1 - roomHalfWidth
y2 = y2 + roomHalfWidth
return Room:fromCorners(x1+1,y1,x2-1,y2)
else
if y2 < y1 then
x1,x2 = x2,x1
y1,y2 = y2,y1
end
x1 = x1 - roomHalfWidth
x2 = x2 + roomHalfWidth
return Room:fromCorners(x1,y1+1,x2,y2-1)
end
end
This could use a little refactoring, couldn’t it? But for now:
function Door:createRoom()
local roomHalfWidth = 4
local result
local x1,y1 = self:getDoorCoordinates(self.room1)
local x2,y2 = self:getDoorCoordinates(self.room2)
if y1 == y2 then -- horizontal
if x2 < x1 then
x1,x2 = x2,x1
y1,y2 = y2,y1
end
y1 = y1 - roomHalfWidth
y2 = y2 + roomHalfWidth
result = Room:fromCorners(x1+1,y1,x2-1,y2)
else
if y2 < y1 then
x1,x2 = x2,x1
y1,y2 = y2,y1
end
x1 = x1 - roomHalfWidth
x2 = x2 + roomHalfWidth
result = Room:fromCorners(x1,y1+1,x2,y2-1)
end
self.room1:addNeighbor(result)
self.room2:addNeighbor(result)
end
Our test now demands:
Door:140: attempt to call a nil value (method 'addNeighbor')
stack traceback:
Door:140: in method 'createRoom'
Main:79: in function 'createRoomsFromDoors'
Main:67: in function 'adjustPositions'
Main:43: in function 'drawDungeonCreation'
Main:31: in function 'draw'
No surprise but I like to let the test lead when it can.
function Room:addNeighbor(aRoom)
table.insert(self.neighbors, aRoom)
end
We can use a regular indexed table here. Now I rather expect my test to run.
Oh. I forgot to return the result. Duh.
...
self.room1:addNeighbor(result)
self.room2:addNeighbor(result)
return result
end
That should help. It’s so nice having tests, they fill in so many of the blanks in my head. Now the error is:
25: Avatar uses horizontal hallway -- Actual: table: 0x281de14c0, Expected: 666->666 n
That’s from this test line:
_:expect(r1.neighbors).has(d)
I meant to expect the new room, not the frakkin door:
_:expect(r1.neighbors).has(newRoom)
Test runs, room now has the necessary neighboring room to continue the test. And because of the preceding test, I’m confident enough that the adjacent room has the right coordinates to allow our avatar to make the transfer.
Now we are ultimately going to change this code:
function RoomPosition:move(deltaX,deltaY)
local proposedX = self.x + deltaX
local proposedY = self.y + deltaY
return self.room:legalMoveTo(RoomPosition(self.room,proposedX,proposedY))
end
function Room:legalMoveTo(aRoomPosition)
for k,door in pairs(self.doors) do
if door:contains(aRoomPosition) then
return aRoomPosition
end
end
return self:stayInRoom(aRoomPosition)
end
Now what this code does now is to allow the avatar to approach the doorway, where she’d otherwise be stopped by the wall. But I changed that logic so that she can always go to the wall. Maybe that was a mistake. Anyway, she can go to the wall and try to go beyond, so we want to change this code, probably removing or commenting out the door bit and then adding in the room transition bit.
Here’s a proposed test:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,199,199)
rp:move(0,0)
_:expect(rp.room).is(r1)
end)
We know that RoomPosition:move calls legalMoveTo, so that should do what it does and the test should pass. And it does. Let’s comment out the doors trick right now:
function Room:legalMoveTo(aRoomPosition)
--[[
for k,door in pairs(self.doors) do
if door:contains(aRoomPosition) then
return aRoomPosition
end
end
]]--
return self:stayInRoom(aRoomPosition)
end
And move twice more, expecting the second move to wind up in newRoom. First this:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,199,199)
rp:move(0,0)
_:expect(rp.room).is(r1)
rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp.x, "stepped").is(200)
end)
Whee! We have a failure:
25: Avatar uses horizontal hallway stepped -- Actual: 199, Expected: 200
Now why are we not allowed to move to 200? Perhaps our definition of what’s inside is off:
function Room:legalMoveTo(aRoomPosition)
return self:stayInRoom(aRoomPosition)
end
function Room:stayInRoom(aRoomPosition)
return aRoomPosition:clamp()
end
function RoomPosition:clamp()
local lox,loy,hix,hiy = self.room:localCorners()
local x = clamp(lox,self.x,hix)
local y = clamp(loy,self.y,hiy)
return RoomPosition(self.room,x,y)
end
function clamp(lo,val,hi)
return math.max(math.min(val,hi), lo)
end
Huh. That looks right to me but let’s see what localCorners returns.
lox,loy,hix,hiy = r1:localCorners()
_:expect(hix, "hix").is(200)
Ah at least the test reveals an interesting mistake:
25: Avatar uses horizontal hallway hix -- Actual: 50.0, Expected: 200
The RoomPosition is room-local. I was thinking global. So I’ve started way the heck outside the room.
Let’s revise the test:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,49,49)
rp:move(0,0)
_:expect(rp.room).is(r1)
_:expect(rp.x, "unstepped").is(49)
rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp.x, "stepped").is(50)
end)
This fails, returning 49 for the stepped value:
25: Avatar uses horizontal hallway stepped -- Actual: 49, Expected: 50
Seems like it has to be in the clamping. Let’s check the clamping values again. I removed that check and perhaps should have fixed it. But this passes:
lox,loy,hix,hiy = r1:localCorners()
_:expect(hix, "hix").is(50)
So why is the legalMove thing failing? Let’s call it directly.
No, wait. We can’t hold on to our rp, we have to refresh it every time. RoomPosition is a value object and doesn’t self-modify. Recast the test:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,49,49)
lox,loy,hix,hiy = r1:localCorners()
_:expect(hix, "hix").is(50)
rp = rp:move(0,0)
_:expect(rp.room).is(r1)
_:expect(rp.x, "unstepped").is(49)
rp = rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp.x, "stepped").is(50)
end)
And the test passes so far. Now one more move should give me a position inside newRoom (except that that’s not implemented yet). We’re about to get the failing test we have been working for.
rp = rp:move(1,0)
_:expect(rp.room).is(newRoom)
_:expect(rp.x, "new").is(-4)
I’m not sure about the -4 but it should be - something like that. I really want these guys to work on global coordinates but now’s not the time. But we could be asking the rp questions instead of ripping its guts out. Let’s do that now to focus our attention.
rp = rp:move(0,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "unstepped").is(49)
rp = rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "stepped").is(50)
rp = rp:move(1,0)
_:expect(rp.room).is(newRoom)
_:expect(rp:localX(), "new").is(-4)
I’ll just do the localX for now, but this style will let us better change the underlying implementation. Law of Demeter: keep your hands out of other people’s pockets. (per Chet Hendrickson).
function RoomPosition:localX()
return self.x
end
This may seem odd. Why not just grab x? Because everyone who just grabs x knows a secret about RoomPosition, namely that it happens to use room-local coordinates (right now). If that ever changes, grabbing x will no longer work. But if we change it now, we’l change localX and that will continue to work. And that’s why you don’t put your hands in other people’s pockets.
Our new checks fail:
25: Avatar uses horizontal hallway -- Actual: table: 0x281c67f00, Expected: table: 0x281c67d80
25: Avatar uses horizontal hallway new -- Actual: 50.0, Expected: -4
Let’s see about pretty-printing Room.
function Room:init(x,y,w,h)
self.mt = getmetatable(self)
self.mt.__tostring = self.toString
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)//8)*8
self.h = h or ((math.random(HEIGHT//40,HEIGHT//20)*2)//8)*8
self.doors = {}
self.neighbors = {}
end
function Room:toString()
return string.format("Room %d at (%d,%d) %d by %d", self.number, self.x,self.y, self.w,self.h)
end
Now that error message is more enlightening:
25: Avatar uses horizontal hallway -- Actual: Room 666 at (150,150) 100 by 100, Expected: Room 666 at (205,150) 8 by 8
I’m really hating those default 666 numbers. Made sense to me at the time. I think I want the corners of the room, though, instead of the w/h.
Ah, that was worth doing:
25: Avatar uses horizontal hallway -- Actual: Room 666 at (150,150) corners (100,100) (200,200), Expected: Room 666 at (205,150) corners (201,146) (209,154)
Imma like that. So we have our failing test and we can make it work.
What’s the Plan, and Is It Cunning?
The plan is to go through all the neighbors and see whether any of them will accept the proposed room coordinate we have. If they do, they’ll return two items, a true, meaning accepted, and a new RoomPosition, in the room in question.
It goes like this:
function Room:legalMoveTo(aRoomPosition)
for i,room in ipairs(self.neighbors) do
accepted, newRoomPosition = room:acceptRoomPosition(aRoomPosition)
if accepted then return newRoomPosition() end
end
return self:stayInRoom(aRoomPosition)
end
If we run out test we should fail looking for acceptRoomPosition.
25: Avatar uses horizontal hallway -- Room:372: attempt to call a nil value (method 'acceptRoomPosition')
A SMOP to implement that and we should be good to go. We stub:
function Room:acceptRoomPosition(aRoomPosition)
return false,aRoomPosition
end
This should get our original failure back and confirms that the loop works about right. And we do get those two failures back on the room and the -4. (Which I’m still not entirely sure about.)
Oh, wow I just thought of something. We’ve gone to some trouble to put the original rooms on integer positions. There’s no reason to think that the hallways are also on integer positions. This could lead to some issues with fractional values. Another thing to worry about … later.
Now we’ll have the room figure out the other case. I don’t remember how I did that before, and that’s probably a good thing.
Here’s my first cut so far:
function Room:acceptRoomPosition(aRoomPosition)
local rect = self:rectangle()
local x,y = aRoomPosition:screenLocation()
if rect:contains(x,y) then
local newRP = RoomPosition:fromScreenLocation(self, x,y)
return true, newRP
else
return false,aRoomPosition
end
end
If the Room’s rectangle includes the screen location of the proposed position, then we want to create a new position, starting with our room, and the screen location in question. We don’t have the fromScreenLocation constructor yet. Also we are irritated by converting back and forth. Also we question whether some of this work would better be done in RoomPosition, but–with a lot of luck–we can probably make this work. We need:
function RoomPosition:fromScreenLocation(x,y)
centerVec = self.room:center()
return RoomPosition(self.room, x-centerVec.x, y-centerVec.y)
end
I have fair confidence in this, and plenty of confidence that the test will fail if it isn’t right. And it does, as before:
25: Avatar uses horizontal hallway -- Actual: Room 666 at (150,150) corners (100,100) (200,200), Expected: Room 666 at (205,150) corners (201,146) (209,154)
25: Avatar uses horizontal hallway new -- Actual: 50.0, Expected: -4
Let’s verify that the test is doing what I expect it’s doing:
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,49,49)
lox,loy,hix,hiy = r1:localCorners()
_:expect(hix, "hix").is(50)
rp = rp:move(0,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "unstepped").is(49)
rp = rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "stepped").is(50)
rp = rp:move(1,0)
_:expect(rp.room).is(newRoom)
_:expect(rp:localX(), "new").is(-4)
end)
Yes, that looks to me to be what I expect. Lets write a new test to directly check our new accept method:
_:test("adjacent room is accepted for move", function()
local rp
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
r1:addNeighbor(n1)
local rp = RoomPosition(r1, 200,150)
newRp = r1:legalMoveTo(rp)
_:expect(newRp, "should be same room").is(rp)
end)
I expected this to pass. However, room positions don’t understand the .is test, and they don’t print well. Let’s extend them for both. I’ll have to learn how to do the equality test.
I think this does the job:
function RoomPosition:init(aRoom, x, y)
self.mt = getmetatable(self)
self.mt.__tostring = self.toString
self.mt.__eq = self.isEqual
self.room = aRoom
self.x = x
self.y = y
end
function RoomPosition:toString()
return string.format("RP %d (%d,%d)", self.room.number, self.x,self.y)
end
function RoomPosition:isEqual(aRoomPosition)
return self.room == aRoomPosition.room and self.x == aRoomPosition.x and self.y == aRoomPosition.y
end
Now my new test …
_:test("adjacent room is accepted for move", function()
local rp
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
r1:addNeighbor(n1)
local rp = RoomPosition(r1, 200,150)
newRp = r1:legalMoveTo(rp)
_:expect(newRp, "should be same room").is(rp)
end)
… fails before I expect it to:
26: adjacent room is accepted for move should be same room -- Actual: RP 666 (50,50), Expected: RP 666 (200,150)
What even is that new RP? And let’s do something about all the rooms getting the same number. For now, just this:
self.number = 666000 + math.random(1000)
Test again:
~~~26: adjacent room is accepted for move should be same room – Actual: RP 666533 (50,50), Expected: RP 666533 (200,150)
Darn, I'm using screen coordinates again, am I not? Test should be:
~~~lua
_:test("adjacent room is accepted for move", function()
local rp
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
r1:addNeighbor(n1)
local rp = RoomPosition(r1, 50,50) -- on the edge
newRp = r1:legalMoveTo(rp)
_:expect(newRp, "should be same room").is(rp)
end)
26: adjacent room is accepted for move should be same room -- OK
OK. We are definitely seeing a problem, at least in test-writing, where we have to do these conversions between screen coordinates and local coordinates in our head. On the Zoom Ensemble last night, GeePaw Hill was arguing for some kind of simple DSL to set up these tests. I’m not sure we need that, but we do need something.
But we’re back on track for now, let’s press on. How about this:
rp = RoomPosition(r1,51,50) -- just over into n1
testRp = RoomPosition(n1, -10, 5)
newRp = r1:legalMoveTo(rp)
_:expect(newRp).is(testRp)
Now rp is just outside r1, at 51 x but still 50 y, i.e. in the middle of the vertical edge and one pixel outside. We expect the result, newRp, to be on the very left edge of n1, in the middle. n1 is 10 high, so I’m hoping for 5 in y, and it is (damn me) 19 wide, so I am shooting for -10. If we’re very wrong, that’s one thing and if we’re nearly right that’ll be another. Let’s find out.
~~~lua26: adjacent room is accepted for move – Actual: RP 666092 (50,50), Expected: RP 666842 (-10,5)
Tests still aren't helping me as much as they might. But it seems we are still getting back r1, with the value clamped.
We need to hone in. Here's the `legalMoveTo`:
~~~lua
function Room:legalMoveTo(aRoomPosition)
for i,room in ipairs(self.neighbors) do
accepted, newRoomPosition = room:acceptRoomPosition(aRoomPosition)
if accepted then return newRoomPosition() end
end
return self:stayInRoom(aRoomPosition)
end
Let’s call the accept method. Wasn’t that our original plan for this test anyway?
_:test("adjacent room is accepted for move", function()
local rp
local accept
local newRp
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
r1:addNeighbor(n1)
rp = RoomPosition(r1, 50,50) -- on the edge
newRp = r1:legalMoveTo(rp)
_:expect(newRp, "should be same room").is(rp)
rp = RoomPosition(r1,51,50) -- just over into n1
testRp = RoomPosition(n1, -10, 5)
accept, newRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(newRp).is(testRp)
end)
We’re on the right track. Let’s strip this test down to essentials to keep our confusion as low as possible:
_:test("adjacent room is accepted for move", function()
local rp
local accept
local expectedRp
local actualRP
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
rp = RoomPosition(r1,51,50) -- just over into n1
expectedRp = RoomPosition(n1, -10, 5)
accept, actualRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(actualRp).is(expectedRp)
end)
Some renaming and stripping.
We create our two rooms, adjacent. I believe that n1 is adjacent to r1 at local y 50, and local x 51. So we create a RoomPosition at that location and ask that it be accepted.
26: adjacent room is accepted for move accepted -- Actual: false, Expected: true
26: adjacent room is accepted for move -- Actual: RP 666214 (51,50), Expected: RP 666558 (-10,5)
This is consistent with the false return for acceptRoomPosition.But we expect the true condition. Here’s the code. We’re in n1.
function Room:acceptRoomPosition(aRoomPosition)
local rect = self:rectangle()
local x,y = aRoomPosition:screenLocation()
if rect:contains(x,y) then
local newRP = RoomPosition:fromScreenLocation(self, x,y)
return true, newRP
else
return false,aRoomPosition
end
end
This wouldn’t work if the rectangle were wrong, if the screen location were wrong, or if rectangle contains didn’t work. Let’s check:
rec = n1:rectangle()
_:expect(rec).is(Rectangle(0,0,0,0))
I’m not sure what the rectangle should be, so I let it ride. And my first pretty print walks back objecting that an input has no integer representation: %d won’t print a float value.
Fixing that, we see:
26: adjacent room is accepted for move -- Actual: Rect (210.500000,150.000000) 19x10, Expected: Rect (0.000000,0.000000) 0x0
Well, there’s surely part of the problem, the room is not on an integer boundary (and basically cannot be) and it’s 19 wide. I’m tempted to change the test to align it. I want the rectangle’s corners to think about what’s going on.
I even printed them, they look OK:
Corners 201.0 145.0 220.0 155.0
I am so temped to print what’s going on inside the function, but I’m feeling committed to working through tests.
So I guess I have to check rectangle. Again.
This is an interesting failure:
26: adjacent room is accepted for move -- Actual: Rect (210.500000,150.000000) 19x10, Expected: Rect (210.500000,150.000000) 19x10
That tells me that there’s rounding going on somewhere in those numbers, despite how equal they look when printed. But wait, I didn’t do Rectangle.__eq so it’s comparing at the table level. With that added, that part of the test passes. We have:
26: adjacent room is accepted for move -- Actual: RP 666705 (51,50), Expected: RP 666095 (-10,5)
Coming out of this test:
_:test("adjacent room is accepted for move", function()
local rp
local accept
local expectedRp
local actualRP
local rect
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
rec = n1:rectangle()
_:expect(rec).is(Rectangle(210.5,150,19,10))
rp = RoomPosition(r1,51,50) -- just over into n1
expectedRp = RoomPosition(n1, -10, 5)
accept, actualRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(actualRp).is(expectedRp)
end)
So I want to check whether the rectangle thinks it includes the point.
I add this:
accept = rec:contains(51,50)
_:expect(accept, "wasn't inside").is(true)
And I get this:
26: adjacent room is accepted for move wasn't inside -- Actual: false, Expected: true
Aha, he moaned. My rectangle doesn’t contain the point. Let’s review contains for not being stupid but we have already tested it. I’m kind of suspecting rounding.
function Rectangle:contains(x,y)
local result = x <= self.x + self.w/2 and y <= self.y + self.h/2
and self.x - self.w/2 <= x and self.y - self.h/2 <= y
--print("rect contains ", self.x,"",self.y,"", self.w,"",self.h,"\n",x,"",y,"=",result)
return result
end
You can see that I’ve been here before, with this print statement. Let’s turn it on.
rect contains 210.5 150.0 19 10
51 50 = false
Well, that’s interesting. It surely shouldn’t contain 51,50. Bad test. But I got a similar printout from the later parts of the test:
rect contains 210.5 150.0 19 10
201.0 200.0 = false
So that’ll be from the real call. But why are we asking about 201,200 even there?
Long Aside
When it’s this hard to write tests, we usually stop writing tests. But it shouldn’t be this hard to write tests, and when it is, what we are often being told is that our objects aren’t helping us enough. The “correct” responses include:
- Stop, revert, improve objects with more refined TDD, then continue what we were doing;
- Push on through the muck until our new thing works, then improve the objects;
- Push on through the muck until our new thing works, then back away slowly.
Unfortunately, we too often try to do #2, find that the improvement is difficult and hey it works anyway, and drop down into #3. That leaves us with code that isn’t helping us enough.
Also unfortunately, at this moment, I don’t quite see how these objects could be more helpful, so I’m going to try #2, and try to avoid falling into #3. But I will almost inevitably fall into #3.
We do have a chance of survival. My wife has brought me a lovely turkey sandwich, so I propose to take a break. Maybe I’ll be smarter when I come back.
Sunday 0809
After that long a break, I’m sure I’ll be more brave, more clever, and generally more wise. Or possibly not.
This message was confusing me:
rect contains 210.5 150.0 19 10
201.0 200.0 = false
The proximate reason is this print:
function Rectangle:contains(x,y)
local result = x <= self.x + self.w/2 and y <= self.y + self.h/2
and self.x - self.w/2 <= x and self.y - self.h/2 <= y
print("rect contains ", self.x,"",self.y,"", self.w,"",self.h,"\n",x,"",y,"=",result)
return result
end
And that’s because I’m printing the member variables in my prints, not the corners, which are more readily understood. So I’m going to improve that first. However, I remain concerned that the objects aren’t helping me as much as they should.
At the same time, I’m suspecting a rounding issue, and leaning more and more toward making the hallway rooms long enough to extend into the regular rooms, ensuring that it’s possible to step into them.
First the print, then decide a next step.
The improved message is this:
Rect (201.000000,145.000000) to (220.000000,155.000000
contains 201.0 200.0 = false
As I asked earlier, how is it that we are asking about y = 200 when our hallway is centered around 150. Let’s see the test.
_:test("adjacent room is accepted for move", function()
local rp
local accept
local expectedRp
local actualRP
local rect
local r1 = Room:fromCorners(100,100,200,200)
local n1 = Room:fromCorners(201, 145, 220, 155)
rec = n1:rectangle()
_:expect(rec).is(Rectangle(210.5,150,19,10))
accept = rec:contains(51,50)
_:expect(accept, "wasn't inside").is(true)
rp = RoomPosition(r1,51,50) -- just over into n1
expectedRp = RoomPosition(n1, -10, 5)
accept, actualRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(actualRp).is(expectedRp)
end)
Here’s an example of what I mean about the objects not helping. Our starting room, r1, has x and y going from 100 to 200, and we’ve arranged the neighbor room, n1, so that it goes from 145 to 155. Since RoomPositions are local, when the princess tries to go down that hallway, her local coordinates will be about (51,50), i.e. middle of r1, trying to step just outside. But rectangles use screen coordinates, so we have to feed the rectangle her screen coordinates, which are, if i’m not mistaken, (201,150).
First of all, the need to do all this mental arithmetic just about guarantees that I’ll make a mistake–as I seem to have done here. Furthermore, the need to do conversions back and forth in the code just about guarantees that I’ll miss out some of those.
I’m not clear yet on how to make the objects more helpful, though I still think it would help a lot if everyone used what I’m calling screen coordinates. For now, let’s see if changing those test numbers helps.
This passes:
rec = n1:rectangle()
_:expect(rec).is(Rectangle(210.5,150,19,10))
accept = rec:contains(201,150)
_:expect(accept, "wasn't inside").is(true)
So far so good. Now this:
rp = RoomPosition(r1,201,150) -- just over into n1
expectedRp = RoomPosition(n1, -10, 5)
accept, actualRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(actualRp).is(expectedRp)
Here, I expect it to be accepted, and I’m not sure if the RP values will be quite as I expect. We’ll see:
Oh, that was dumb. Again the objects aren’t helping. RoomPosition wants to be built with relative coordinates. However, we do have this:
function RoomPosition:fromScreenLocation(x,y)
centerVec = self.room:center()
return RoomPosition(self.room, x-centerVec.x, y-centerVec.y)
end
Darn, no, I can’t use that either. I’m going to have to leave in the adjusted values here. Back to this way:
rp = RoomPosition(r1,51,50) -- just over into n1
expectedRp = RoomPosition(n1, -10, 5)
accept, actualRp = n1:acceptRoomPosition(rp)
_:expect(accept, "accepted").is(true)
_:expect(actualRp).is(expectedRp)
And the prints and errors are:
Rect (201.000000,145.000000) to (220.000000,155.000000
contains 201.0 200.0 = false
26: adjacent room is accepted for move accepted -- Actual: false, Expected: true
26: adjacent room is accepted for move -- Actual: RP 666826 (51,50), Expected: RP 666484 (-10,5)
OK, we’re calling our rectangle compare with 201,200, despite using an rp of 51,50. So the conversion must somehow be wrong. Check acceptRoomPosition.
function Room:acceptRoomPosition(aRoomPosition)
local rect = self:rectangle()
local x,y = aRoomPosition:screenLocation()
if rect:contains(x,y) then
local newRP = RoomPosition:fromScreenLocation(self, x,y)
return true, newRP
else
return false,aRoomPosition
end
end
It seems like somehow, screenLocation has returned 201,200 when given 51,50. Let’s check that code:
function RoomPosition:screenLocation()
return self.room.x + self.x, self.room.y + self.y
end
How can that not work? It would help if room x and y were named centerX and centerY, but that’s what they mean.
I am clearly in a near-brainless debugging cycle here, my mode 2 above devolving to mode 3. But I don’t have any feeling at all that starting over again would help. Perhaps if I were to do the rather painful conversion to all screen coordinates, that would help. But just a bit more debugging … the slippery slope.
I’m tossing some prints in there:
acceptRoomPosition
Rect (201.000000,145.000000) to (220.000000,155.000000
RP 666357 (51,50)
converted to 201.0 200.0
Well that seems clear. Let me verify that in the test:
rp = RoomPosition(r1,51,50) -- just over into n1
local x,y = rp:screenLocation()
_:expect(x, "conversion").is(201)
_:expect(y, "conversion").is(150)
As expected (finally)
26: adjacent room is accepted for move conversion -- Actual: 200.0, Expected: 150
How could this be? It could be if the room’s center isn’t where it should be.
I add this, and it passes:
local c = r1:center()
_:expect(c, "center").is(vec2(150,150))
Shaking my head, I note that position (51,50) in a room centered at (150,150) does seem a lot like (201,200).
The test is WRONG! [EXPLETIVES DELETED]
The hallway is at r1’s vertical center, so its y = 0.
The number conversions are killing me. They’re slowing me down, they’re making me want to debug, they’re pointing my attention in the wrong direction and they are ticking me off.
Back to the test, fixing:
rp = RoomPosition(r1,51,0) -- just over into n1
Still some errors. Here’s one of them:
26: adjacent room is accepted for move -- RoomPosition:9: attempt to index a nil value (field 'room')
That’s this:
function RoomPosition:fromScreenLocation(x,y)
centerVec = self.room:center()
return RoomPosition(self.room, x-centerVec.x, y-centerVec.y)
end
The message means there’s an RP that doesn’t have a room set. Who’s calling this?
function Room:acceptRoomPosition(aRoomPosition)
local rect = self:rectangle()
local x,y = aRoomPosition:screenLocation()
if rect:contains(x,y) then
local newRP = RoomPosition:fromScreenLocation(self, x,y)
return true, newRP
else
return false,aRoomPosition
end
end
This is good news: we were about to return the answer we’ve been looking for. However, Room and RoomPosition don’t agree on the signature of fromScreenLocation. It’s supposed to be a class method, and this is the sole usage, so we’ll fix it to be as expected:
-- class methods
function RoomPosition:fromScreenLocation(aRoom, x,y)
centerVec = aRoom:center()
return RoomPosition(aRoom, x-centerVec.x, y-centerVec.y)
end
The names aren’t helping much. Let’s rename:
function RoomPosition:fromScreenLocation(aRoom,screenX, screenY)
centerVec = aRoom:center()
return RoomPosition(aRoom, screenX-centerVec.x, screenY-centerVec.y)
end
It’s still a crock, of course, but maybe a slightly clearer crock. Everyone should be using the same kind of coordinates, at least ideally, but we will have the issue however we proceed, because things in a room probably do want to be room-relative. Maybe. All I’m sure of right now is that it’s a mess. I do expect the tests to improve, however.
Here’s a very telling message:
26: adjacent room is accepted for move -- RoomPosition:25: bad argument #3 to 'format' (number has no integer representation)
Clearly we have a fractional coordinate. The pretty-prints are helping in the test displays. After changing some percent-d to percent-f, I get:
26: adjacent room is accepted for move -- Actual: RP 666755 (-9.500000,0.000000), Expected: RP 666755 (-10.000000,5.000000)
That doesn’t surprise me, because our room is 19 wide and our center is off. But we did make it inside the room. So I’ll mod the test:
Might have helped had I looked at both halves of the message. Now I get:
26: adjacent room is accepted for move -- Actual: RP 666534 (-9.500000,0.000000), Expected: RP 666534 (-9.500000,5.000000)
Obviously, this is the same brain bug. Since our hallway is centered such that she enters in the middle, the coordinate should be zero. The code is right, the test is wrong. With that change, our detailed test passes. We have two other fails, from above. Before I look, I’m hoping they are the same fumbled coordinates. Here are the messages:
25: Avatar uses horizontal hallway -- Actual: Room 666656 at (150,150) corners (100,100) (200,200), Expected: Room 666859 at (205,150) corners (201,146) (209,154)
25: Avatar uses horizontal hallway new -- Actual: 50.0, Expected: -4
Guess we’d better inspect that test. It’s basically the one that drove me to do the more detailed one that I’ve been struggling with for nigh on to fifteen years.
_:test("Avatar uses horizontal hallway", function()
-- same as "Rooms from doors horizontal" with some asserts removed
local r1 = Room:fromCorners(100,100,200,200)
local r2 = Room:fromCorners(210,125, 310, 175)
local d = Door:fromRooms(r1,r2)
local newRoom = d:createRoom()
_:expect(r1.neighbors).has(newRoom)
local rp = RoomPosition(r1,49,49)
lox,loy,hix,hiy = r1:localCorners()
_:expect(hix, "hix").is(50)
rp = rp:move(0,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "unstepped").is(49)
rp = rp:move(1,0)
_:expect(rp.room).is(r1)
_:expect(rp:localX(), "stepped").is(50)
rp = rp:move(1,0)
_:expect(rp.room).is(newRoom)
_:expect(rp:localX(), "new").is(-4)
end)
The “stepped” check passed, so the errors are these last two. I am suspecting a rounding problem, so I’m going to begin by moving by 2, rather than 1, to see if it fixes the problem. But no. Essentially the same message So let’s see if I’ve made a coordinate error. And in fact I have. Yay, me.
The horizontal hallway is arranged at the room’s Y center, which means that the room position we start with should be (49,0) not (49,49). These numbers are killing me. But let’s plug in the right value … and now we just have one error:
25: Avatar uses horizontal hallway -- Room:373: attempt to call a table value (global 'newRoomPosition')
What even is that? I have no recollection of that at all. Here’s the code:
function Room:legalMoveTo(aRoomPosition)
for i,room in ipairs(self.neighbors) do
accepted, newRoomPosition = room:acceptRoomPosition(aRoomPosition)
if accepted then return newRoomPosition() end
end
return self:stayInRoom(aRoomPosition)
end
Right. Finally got to the right place, and then called the return value. Fix that:
function Room:legalMoveTo(aRoomPosition)
for i,room in ipairs(self.neighbors) do
accepted, newRoomPosition = room:acceptRoomPosition(aRoomPosition)
if accepted then return newRoomPosition end
end
return self:stayInRoom(aRoomPosition)
end
I almost feel optimistic about the test passing now. And, after I change my move back to (1,0), it does pass.
And that makes me think that the princess might be able to move between rooms. And she almost can:
She can move into the hallways! Yay! She can’t get out! Boo!
Why not? I’m betting that while I give the adjacent rooms their neighbors, I don’t give the hallway rooms the corresponding neighbors. We’ll leave that for later. I’m tired, and happy for now. Commit: can move from rooms to halls and not back.
Monday Closing
After rather a difficult period, by end of day Sunday, I’ve managed to get our intrepid princess to move from a room into a hallway. Unfortunately, once she gets there, she’s trapped, but I am quite sure that I understand why: the hallways don’t have any rooms registered as neighbors.
But the process has been far too difficult. Now, some things really are difficult to program, I imagine, but generally speaking, if it feels difficult it’s more likely because our code’s structure and design is getting in the way of doing good things.
Alan Kay famously said “Point of view is worth 80 IQ points”, and among the ways that phrase rings true is that when our code is helping us, we can see further and see better what to do to improve it. And when it isn’t helping us, our minds become dull and we progress more and more slowly, with more and more defects. This is a slippery slope. The further we go down it, the worse it gets.
A lot of the trouble in this code is “natural”. I have essentially no experience with a program of this kind, I’ve not studied the existing technology, and I’m just winging it, pushing in directions that seem right. As such, I’m bound to make mistakes. Some of those will be immediate, and immediately discovered. Others will be chunks of code, object definitions, that seem OK but turn out to be–let’s say, gently–less than ideal.
I’m good with that. My purpose here isn’t to show you how smart I am. It is to show how our behavior as real programmers affects what happens, and to explore and demonstrate what we can try in order to make the work flow better, and our lives less painful.
So in this sequence with a somewhat happy ending, we’ve seen multiple attempts made, multiple ideas tried, and multiple reversions of the code. Of all the things I’ve learned in this sequence, I think the reversions are the best part.
I’m serious. If I were even better at recognizing when I’m down a rathole and backing out, I believe overall progress would have been better. Less time would have been spent banging my head against the code.
Smaller lessons are present as well.
I still don’t turn soon enough to TDD.
Much more of this program is readily created using TDD than the more graphical Asteroids and Space Invaders. Yet, being an old, and old-school programmer, I am too ready to just code, confident that I can get it right. That’s not as true as it might be.
The Room object is too big.
Room is exactly 400 lines long this morning. The tests are longer, but they don’t negatively impact the design by being large. In fact, they mostly help. The next largest tab is Main, which is almost always a mess, and it’s only 83 lines long.
We can be almost certain that Room is not cohesive at that size. That is, it is almost certainly carrying out multiple purposes, not just one. In my experience, having many objects that have one purpose is far better than having just a few objects with many purposes. Room needs improvement.
There are inconsistencies in how things are done.
The Rectangle object was introduced late on, and actually with no specific intended purpose. It really only has one use, so far, and that is to answer whether a point is inside it. However, I wouldn’t swear that that’s the only way we determine whether a point is inside a rectangle, and I am quite certain that there is code in there that checks two rectangles to see whether they intersect. (It’s used in the code that jostles the rooms until they don’t overlap.) That question should be forwarded to Rectangle, which should be extended to provide that capability.
There are similar inconsistencies, similar things being done in multiple places. Those should be centralized, with supporting objects as needed.
The jostling really should be done outside the draw logic.
I just wanted to watch the rooms jostle, and there are readers out there who enjoy watching it as well. Even if we want the jostling to be visible–and there’s no real point to it–we shouldn’t have the code that does that, finds room paths, and creates hallway rooms from doors, all inside the drawing function.
I’m sure there’s more.
Suffice to say, the code isn’t in great shape, and I can feel that it’s holding me back. We’ve built up technical debt, and we need to decide what to do about it.
We’ll talk about that next time.
See you then!