Let’s put some announcements in the rooms of the learning level. That should be easy, and it should give us a sense of how designers might want to work.

We have the lovely Announcer object, which contains a one-shot message and which can be placed in multiple tiles at the same time, one instance in many tiles. My tentative plan is to add a method to Room, announce, that will ring the room with an Announcer.

Already I’m thinking design. Would it be better to have a method that places any object around the rim, and to provide the announcer? Or would it be better to pass in the message and let Room decide how to do it? I think the latter. If we need the former, we’ll surely have the components to do it.

And I’m reminded that hallway carving may be able to overwrite existing room tiles. If that’s the case, we’d have to send the announce messages after the hallways are done, which feels awkward. If I were designing rooms–and, oh yeah, I am–I’d want static-feeling information like the announcement to be declared when the room is declared.

I’m not really sure what hallway carving does about existing room tiles. I know I’ve thought about it before. Let’s not trust my memory. Let’s look.

Dungeon does the actual work of carving halls:

function Dungeon:horizontalCorridor(fromX, toX, y)
    fromX,toX = math.min(fromX,toX), math.max(fromX,toX)
    for x = fromX,toX do
        self:setHallwayTile(x,y)
    end
end

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

Sweet! I’ve done something right for once. We don’t overwrite room tiles. OK, let’s get to it.

Room:announce

An issue to think about is the extent to which we want a room to have a Complete Creation Method, and whether that should include the announce message built in. I think no, for at least these reasons:

  • Not all rooms, probably few rooms, will have the message
  • We might actually have multiple messages for a room; such as before and after a quest is completed;
  • Wow, that’s a neat idea, I’m glad I started this list;

OK, anyway, separate message to Room. Let’s do one. Where’s learning level created, and why do we hate it?

function GameRunner:createLearningLevel()
    self.dungeonLevel = 1
    TileLock = false
    self:createNewDungeon()
    local dungeon = self.dungeon
    self.tiles = dungeon:createTiles(self.tileCountX, self.tileCountY)
    dungeon:clearLevel()
    self:createLearningRooms()
    self:connectLearningRooms()
    dungeon:convertEdgesToWalls()
    self.monsters = Monsters()
    self:placePlayerInRoom1()
    self:placeWayDown()
    --self:placeSpikes(5)
    --self:placeLever()
    self:setupMonsters(6)
    self.keys = self:createThings(Key,5)
    self:createThings(Chest,5)
    self:createLoots(10)
    self:createDecor(30)
    self:createButtons()
    self.cofloater:runCrawl(self:initialCrawl(self.dungeonLevel))
    self:startTimers()
    self.playerCanMove = true
    TileLock = true
end

We should really not be doing all these creation things, I just left them in to make the learning level interesting while I work on it. We’re interested in this:

function GameRunner:createLearningRooms()
    local w = 12
    local h =  8
    local t = {
        {2,2, w,h},
        {15,2, w,h},
        {28,2, w,h},
        {41,2, w,h},
        {2,11, w,h},
        {15,11, w,h},
        {28,11, w,h},
        {41,11, w,h},
        {2,20, w,h},
        {15,20, w,h},
        {28,20, w,h},
        {41,20, w,h},
    }
    for i,r in ipairs(t) do
        r[1] = r[1]+24
        r[2] = r[2]+24
    end
    self.rooms = self:makeRoomsFromXYWH(t)
end

Now I asked why we hate the way we do it, and this code right here is the candidate for why. It’s convenient, just a list of the room coordinates and dimensions, but no useful identification of the room or ability to refer to it later.

I have an idea, though. Or some fraction of one. I was first thinking, welp, we could just append another element to the room coordinates table, an announcement, and if we have one, just use it. Then I thought, I thought nope, let’s make a separate table. And then I thought, I did, yep, that’s it.

Here goes, hold my chai.

function GameRunner:createLearningRooms()
    local w = 12
    local h =  8
    local announcements = {}
    local a2 = { "Welcome to Room 2!", "I fancy I am one of the best rooms",
        "that you've seen so far." }
    annoucements[2] = a2
    local t = {
        {2,2, w,h},
        {15,2, w,h},
        {28,2, w,h},
        {41,2, w,h},
        {2,11, w,h},
        {15,11, w,h},
        {28,11, w,h},
        {41,11, w,h},
        {2,20, w,h},
        {15,20, w,h},
        {28,20, w,h},
        {41,20, w,h},
    }
    for i,r in ipairs(t) do
        r[1] = r[1]+24
        r[2] = r[2]+24
    end
    self.rooms = self:makeRoomsFromXYWH(t, announcements)
end

I’m creating a table, announcements, indexed by room number, but sparse, and passing it to the XYWH method:

function GameRunner:makeRoomsFromXYWH(tab)
    return map(tab, function(e)
        return Room(e[1],e[2],e[3],e[4], self, true)
    end)
end

That’s gonna need a rewrite, but it’s an easy one:

function GameRunner:makeRoomsFromXYWH(roomTable, annTable)
    local result = {}
    for i,e in ipairs(roomTable) do
        table.insert(Room(e[1],e[2],e[3],e[4], self, true))
        if annTable[i] then
            result[#result]:announce(annTable[i])
        end
    end
end

We just check the annTable for an entry and if it has one, we ask the most recent room to announce it.

I expect that when I touch the Learn button we should crash looking for Room:announce.

Well, we do crash but not for the reason I expected:

GameRunner:72: attempt to index a nil value (global 'annoucements')
stack traceback:
	GameRunner:72: in method 'createLearningRooms'
	GameRunner:46: in method 'createLearningLevel'
	Player:157: in field '?'
	Button:59: in method 'performCommand'
	Button:54: in method 'touched'
	GameRunner:535: in method 'touched'
	Main:73: in function 'touched'

Typo in the name:

function GameRunner:createLearningRooms()
    local w = 12
    local h =  8
    local announcements = {}
    local a2 = { "Welcome to Room 2!", "I fancy I am one of the best rooms",
        "that you've seen so far." }
    announcements[2] = a2

Wow, another mistake and an obvious one:

GameRunner:97: wrong number of arguments to 'insert'
stack traceback:
	[C]: in function 'table.insert'
	GameRunner:97: in method 'makeRoomsFromXYWH'
	GameRunner:91: in method 'createLearningRooms'
	GameRunner:46: in method 'createLearningLevel'
	Player:157: in field '?'
	Button:59: in method 'performCommand'
	Button:54: in method 'touched'
	GameRunner:535: in method 'touched'
	Main:73: in function 'touched'

A smarter compiler would have caught these. Maybe.

function GameRunner:makeRoomsFromXYWH(roomTable, annTable)
    local result = {}
    for i,e in ipairs(roomTable) do
        table.insert(result, Room(e[1],e[2],e[3],e[4], self, true))
        if annTable[i] then
            result[#result]:announce(annTable[i])
        end
    end
end

This is kind of like TDD except with no tests. We need to think about that. It’s not ideal. Here we go:

GameRunner:99: attempt to call a nil value (method 'announce')
stack traceback:
	GameRunner:99: in method 'makeRoomsFromXYWH'
	GameRunner:91: in method 'createLearningRooms'
	GameRunner:46: in method 'createLearningLevel'
	Player:157: in field '?'
	Button:59: in method 'performCommand'
	Button:54: in method 'touched'
	GameRunner:535: in method 'touched'
	Main:73: in function 'touched'

Now we need our announce method in Room. Let’s see if we can test that, just to get our mojo workin again.

In the Tests tab we have some room-related tests with a decent, albeit slow setup. Let’s put a test there. Here’s my first cut at a test:

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local ann = {"Message"}
            r:announce(ann)
            local dungeon = Runner:getDungeon()
            checkRing(dungeon, 10,10, 16, 21, ann)
        end)

The as-yet-undefined checkRing will check the outer limits of the room to see if they contain an announcer containing the message ann.

There’s a possible off-by-one here, but I think Rooms add their width and height. Let’s look. They do not!

Test should reflect that. And I was off by two!! A new record!

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local ann = {"Message"}
            r:announce(ann)
            local dungeon = Runner:getDungeon()
            checkRing(dungeon, 10,10, 14, 19, ann)
        end)

Let’s write checkRing before it gets too hard.

Here’s the first half of checkRing:

function checkRing(dungeon, x1,y1, x2,y2, ann)
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTile(x,y1)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
        t = dungeon:privateGetTile(x,y2)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
    end
end

It’s pretty invasive, but I’ll allow it for now, since it’s a test. We fetch the tiles along the top and bottom, get their first (and only) contents item, fetch its messageTable, and it’d better be ann.

Let’s run this and expect to fail looking for announce on Room.

7: Room:announce -- Tests:120: attempt to call a nil value (method 'announce')

Couldn’t be more right than that, could I?

Write the method. I need a dungeon, but the Room has a runner so it can get one. But it’s better if we pass it in, it reduces coupling back to Runner.

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local ann = {"Message"}
            local dungeon = Runner:getDungeon()
            r:announce(dungeon, ann)
            checkRing(dungeon, 10,10, 14, 19, ann)
        end)

I rather expect this to work:

function Room:announce(dungeon, messageTable)
    local tile
    local ann = Announcer(messageTable)
    for x = x1,x2 do
        tile = dungeon:getTile(vec2(x,y1))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(x,y2))
        tile:addDeferredContents(ann)
    end
    for y = y1,y2 do
        tile = dungeon:getTile(vec2(x1,y))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(x2,y))
        tile:addDeferredContents(ann)
    end
end

Let’s see why I’m wrong.

7: Room:announce -- Room:38: bad 'for' limit (number expected, got nil)

Might want to use the member variables, fool.

function Room:announce(dungeon, messageTable)
    local tile
    local ann = Announcer(messageTable)
    for x = self.x1, self.x2 do
        tile = dungeon:getTile(vec2(x,y1))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(x,y2))
        tile:addDeferredContents(ann)
    end
    for y = self.y1, self.y2 do
        tile = dungeon:getTile(vec2(x1,y))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(x2,y))
        tile:addDeferredContents(ann)
    end
end

7: Room:announce -- Tests:503: attempt to call a nil value (method 'privateGetTile')

Hm. The name of the function ends in XY:

function checkRing(dungeon, x1,y1, x2,y2, ann)
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTile(x,y1)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
        t = dungeon:privateGetTileXY(x,y2)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
    end
end

In both places. Remind me to comment on how stupid I seem to be being.

function checkRing(dungeon, x1,y1, x2,y2, ann)
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTileXY(x,y1)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
        t = dungeon:privateGetTileXY(x,y2)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(ann)
    end
end

7: Room:announce -- Tests:505: attempt to index a nil value (local 'ann')

Did I fail to pass in the announcement? No, but I’m overwriting it. Let’s change the calling sequence of our checker:

function checkRing(dungeon, x1,y1, x2,y2, msgTable)
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTileXY(x,y1)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(msgTable)
        t = dungeon:privateGetTileXY(x,y2)
        local ann = t.contents:get(1)
        local msgs = ann.messageTable
        _:expect(msgs).is(msgTable)
    end
end

Let’s sort out those repeated local things. They work, but they’re confusing at best:

function checkRing(dungeon, x1,y1, x2,y2, msgTable)
    local ann
    local msgs
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTileXY(x,y1)
        ann = t.contents:get(1)
        msgs = ann.messageTable
        _:expect(msgs).is(msgTable)
        t = dungeon:privateGetTileXY(x,y2)
        ann = t.contents:get(1)
        msgs = ann.messageTable
        _:expect(msgs).is(msgTable)
    end
end

But I think the message is telling us that our local ann is nil. Let’s add an expect on that.

Yes:

7: Room:announce x,y1 -- Actual: nil, Expected: nil

Bad message but it tells us we didn’t get anything.

I think I’d like to do something a bit simpler here: I don’t trust my checkRing much.

I simplify the test, to zero in on what’s happening. I suspect I’m not doing the right thing to fetch the contents. The new message is that the contents is nil.

Let’s see if we are doing this right. Reviewing Tile and DeferredTable, I think we need to update the table.

After longer than I care to admit, I realize that the announce method still doesn’t include enough selfs, and is storing into the wrong tiles.

function Room:announce(dungeon, messageTable)
    local tile
    local ann = Announcer(messageTable)
    for x = self.x1, self.x2 do
        tile = dungeon:getTile(vec2(x,self.y1))
        tile:addDeferredContents(ann)
        print(tile)
        tile = dungeon:getTile(vec2(x,self.y2))
        tile:addDeferredContents(ann)
        print(tile)
    end
    for y = self.y1, self.y2 do
        tile = dungeon:getTile(vec2(self.x1,y))
        tile:addDeferredContents(ann)
        print(tile)
        tile = dungeon:getTile(vec2(self.x2,y))
        tile:addDeferredContents(ann)
        print(tile)
    end
end

Now I want to put the test back the way it was before I started thrashing:

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local msgs = {"Message"}
            local dungeon = Runner:getDungeon()
            r:announce(dungeon, msgs)
            local tile = dungeon:privateGetTileXY(10,10)
            local c = tile:getContents()
            c:update()
            local c1 = c:get(1)
            local m = c1.messageTable
            _:expect(m).is(msgs)
            --checkRing(dungeon, 10,10, 14, 19, ann)
        end)

This fails, c1 is nil. In a fit of pique, I decide to rez the learning level anyway, and I see this:

level

Not only does it have the room surrounded by the ? icons that represent an Announcer, the messages come out correctly.

Permit me to say “Bloody Hell!” The code is good, the test is bad. We must divert to discuss this.

Interim Retrospective

Until I thought I was done, I was going blithely along thinking that my test was helping me. And in fact, it was. It found one after another of my “stupid” errors. I’m reminded that I wanted to talk about that:

When I have confidence in my tests, especially a new one that I’m working to make run, I really do reduce the number of mental balls I try to keep in the air. I don’t fuss so much about typos: the test will find them. I don’t fuss about a method call being wrong: the test will find them.

Mind you, I don’t just relax and type randomly, but I do relax and focus more on what and how I’m writing the object, and less on the kinds of details that the tests will find for me. And that’s a good thing.

Then, suddenly, we broke through all those simple errors, and fell into the “real” guts of the test, where it rips information, all untimely, out of the objects to check whether things are as they should be.

And that code was wrong. And maybe rightly so. When you go rummaging around inside your objects you are dealing with things you’re not supposed to be touching, and you’re making assumptions about how things are going to be. And sometimes, if you’re me, you’re going to be wrong.

So you step back and try things. I tried printing things, and everything I did made me think that the tiles I was looking at didn’t have any contents at all. The most likely cause of that, it seemed to me, was that my announce method didn’t work and didn’t put the Announcer in there. That turns out not to be the case: the call in the creation of the learning level works just fine.

So my rooting around in the entrails of my objects is what hasn’t worked.

I do want this test to work, so let’s see whether we can make all this right. Let’s posit a method on tile, and do this:

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local msgs = {"Message"}
            local dungeon = Runner:getDungeon()
            r:announce(dungeon, msgs)
            local tile = dungeon:privateGetTileXY(10,10)
            local c1 = tile:privateGetFirstContents()
            local m = c1.messageTable
            _:expect(m).is(msgs)
            --checkRing(dungeon, 10,10, 14, 19, ann)
        end)

Now we just need privateGetFirstContents:

Ah. In the course of this exercise, I finally realize that the DeferredTable isn’t an array. So:

function Tile:privateGetFirstContents()
    local cont = self:getContents()
    for k,v in cont:pairs() do
        return v
    end
end

Now I think the test will work. And it does. Now let’s see if we want to check the entire ring … sure, why not, in for a penny,

        _:test("Room:announce", function()
            local r = Room(10,10,5,10, Runner)
            local msgs = {"Message"}
            local dungeon = Runner:getDungeon()
            r:announce(dungeon, msgs)
            checkRing(dungeon, 10,10, 14, 19, msgs)
        end)

function checkRing(dungeon, x1,y1, x2,y2, msgTable)
    local ann
    local msgs
    local t
    for x = x1,x2 do
        t = dungeon:privateGetTileXY(x,y1)
        ann = t:privateGetFirstContents()
        msgs = ann.messageTable
        _:expect(msgs,"x,y1").is(msgTable)
        t = dungeon:privateGetTileXY(x,y2)
        ann = t:privateGetFirstContents()
        msgs = ann.messageTable
        _:expect(msgs,"x,y2").is(msgTable)
    end
    for y = y1,y2 do
        t = dungeon:privateGetTileXY(x1,y)
        ann = t:privateGetFirstContents()
        msgs = ann.messageTable
        _:expect(msgs,"x1,y").is(msgTable)
        t = dungeon:privateGetTileXY(x2,y)
        ann = t:privateGetFirstContents()
        msgs = ann.messageTable
        _:expect(msgs,"x2,y").is(msgTable)
    end
end

And the code that we need to do the job, smaller than the tests:

function Room:announce(dungeon, messageTable)
    local tile
    local ann = Announcer(messageTable)
    for x = self.x1, self.x2 do
        tile = dungeon:getTile(vec2(x,self.y1))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(x,self.y2))
        tile:addDeferredContents(ann)
    end
    for y = self.y1, self.y2 do
        tile = dungeon:getTile(vec2(self.x1,y))
        tile:addDeferredContents(ann)
        tile = dungeon:getTile(vec2(self.x2,y))
        tile:addDeferredContents(ann)
    end
end

And, finally, let’s see it work.

room2 works

So, there we are. Dare I say “finally!”? Let’s commit: Learning Level now has an interim announcer in Room 2.

Now, is this right? Well, not quite:

function Tile:privateGetFirstContents()
    local cont = self:getContents()
    for k,v in cont:pairs() do
        return v
    end
end

That should be deferred to the table:

function Tile:privateGetFirstContents()
    return self.contents:getFirst()
end

That can legit be a public method on DeferredTable, it’s a sensible kind of accessor.

function DeferredTable:getFirst()
    self:update()
    local k,v = next(self.contents)
    return v
end

The next is Lua’s inner loop function that returns the next pair of elements when called repeatedly. We already use it in DeferredTable, here:

function DeferredTable:isEmpty()
    self:update()
    return next(self.contents) == nil
end

We generally use DeferredTable in my conventional form, which is to let the object serve as the key, but the table allows for key and value to be different:

function DeferredTable:put(k,v)
    self.buffer[k] = v or k
end

So it seemed best to return the v value, not the k.

Tests run, game runs. Commit: push first element logic down from Tile to Tile.contents.

Let’s sum up.

Summary

This has been odd. The actual code worked quite readily. Had I not troubled myself to write a test for it, I’d have been done much sooner. This, of course, flies in the face of our usual claim about TDD, that it delivers value faster.

In this case, it didn’t. What should we learn from this experience?

Well, as I already mentioned, the test was helping me at the beginning, as I worked through all the typos and thinkos that make up programming. And then, suddenly, the test stopped helping. And, frankly, I took longer than I should have to figure out the issue, which, at bottom, was that I had mad a mistaken assumption about how to rip the guts out of the tile. I wasn’t getting anything, and I concluded, mistakenly, that there was nothing there. Fact was, I was asking the wrong question, where’s #1, when that question doesn’t have an answer in a non-array table.

What would have been better? Well, writing a simpler test would have done it. I could have checked the contents for not being empty.

And not ripping the guts out of my innermost object would have done it as well. That’s never a good thing to do.

So I’m glad I had the test, because it has sharpened my attention to my habits, good and bad as they may be.

Next time, I’ll try to start with a much simpler test than the checkRing thing, and when things start to look weird, I’ll try to write an even simpler test rather than debug the complex one.

I’ll try. I expect to fail, frequently. That’s part of programming. Most of us can’t avoid all mistakes. We can, however, work in a way so as to detect them.

And that may be the biggest lesson of this weird situation: that test, broken as it was, kept me focused on the problem longer than I would otherwise have done. As it happened, the code was good and the test bad. That’s quite unusual, and keeping at it until the differences are resolved is a very good thing to do.

Let’s pop up a level and look at how we did this at the topmost level:

function GameRunner:createLearningRooms()
    local w = 12
    local h =  8
    local announcements = {}
    local a2 = { "Welcome to Room 2!", "I fancy I am one of the best rooms",
        "that you've seen so far." }
    announcements[2] = a2
    local t = {
        {2,2, w,h},
        {15,2, w,h},
        {28,2, w,h},
        {41,2, w,h},
        {2,11, w,h},
        {15,11, w,h},
        {28,11, w,h},
        {41,11, w,h},
        {2,20, w,h},
        {15,20, w,h},
        {28,20, w,h},
        {41,20, w,h},
    }
    for i,r in ipairs(t) do
        r[1] = r[1]+24
        r[2] = r[2]+24
    end
    self.rooms = self:makeRoomsFromXYWH(t, announcements)
end

function GameRunner:makeRoomsFromXYWH(roomTable, annTable)
    local dungeon = self:getDungeon()
    local result = {}
    for i,e in ipairs(roomTable) do
        table.insert(result, Room(e[1],e[2],e[3],e[4], self, true))
        if annTable[i] then
            result[#result]:announce(dungeon, annTable[i])
        end
    end
    return result
end

I chose a design for room creation that assumes a full list of rooms, and a separate table of the announcement messages for the rooms with announcers. I’m imagining some future table-driven “language” that might look a little like this:

Room at 2,2 size 12,8
Room at 15,2, size 12,8
announce
    Welcome to Room 2!
    I fancy I am one of the best rooms
    you have seen so far.
Room at 28,2 size 12,8
announce
    You have reached Room 3,
    where you will find your first monster.
    I advise you not to approach it too closely.
    If you leave it alone, it will leave you alone.
monster Ghost at 4,4 strategy "calm"
Room at 41,2 size 12,8
...

Something like that, anyway.

Hm. We could almost do that. Must think about parsing, which we’d rather keep simple. But that’s for later. For now, we have a sample room with announcer.

See you next time!

D2.zip