Dungeon 104
Monster levels should be easy. Then I wander off into code improvement.
Now that there are levels, in a sense, let’s see if we can separate the monsters out by level, with tougher ones further down.
It turns out that we already have monsters identified by level, with entries like these:
m = {name="Poison Frog", level=3, health={4,8}, speed = {2,6}, strength={8,11},
attackVerbs={"leaps at", "smears poison on", "poisons", "bites"},
dead=asset.frog_dead, hit=asset.frog_hit,
moving={asset.frog, asset.frog_leap}}
table.insert(MT,m)
m = {name="Ankle Biter", level=4, health={9,18}, speed={3,7}, strength={10,15},
attackVerbs={"grinds ankles of", "cuts ankle of", "saws at feet of"},
dead=asset.spinnerHalf_dead, hit=asset.spinnerHalf_hit,
moving={asset.spinnerHalf, asset.spinnerHalf_spin}}
table.insert(MT,m)
To really make them tougher, we’ll want to invent things like armor class, or at least make them harder to hit somehow. Speed differences can give us some help there. But for now, let’s see about allocating them by level.
I feel like a test might be helpful here. Not very helpful, but a bit helpful. I’m trying to be good, I really am …
_:test("Can select monsters by level", function()
Monster:initMonsterTable()
local t = Monster:getMonstersAtLevel(2)
_:expect(#t).is(3)
end)
There are three, according to my count. This test should fail, there being no getMonstersAtLevel function.
1: Can select monsters by level -- Tests:307: attempt to call a nil value (method 'getMonstersAtLevel')
The tests are getting to be a pain. There are a lot of them, and when just one fails, it’s hard to find that failure in the list in the little sidebar. This makes me less likely to test things. In addition, there are some tests turned off, which is always a bad sign.
For now, we want to make this work.
function Monster:getMonstersAtLevel(level)
if not MT then self:initMonsterTable() end
local t = {}
for i,m in ipairs(MT) do
if m.level == level then
table.insert(t,m)
end
end
return t
end
We’re green.
I decided to lazy-init MT, which allows me to pull the init line from the test:
_:test("Can select monsters by level", function()
local t = Monster:getMonstersAtLevel(2)
_:expect(#t).is(3)
end)
We’re green. Now of course we don’t actually know our level, so we need to put that into GameRunner. I init dungeonLevel to 0, and then start here:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createTiles()
self:clearLevel()
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
self:placePlayerInRoom1()
self:placeWayDown()
self.monsters = self:createThings(Monster,6)
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
self.keys = self:createThings(Key,5)
self:createThings(Chest,5)
self:createLoots(10)
self:createButtons()
self.cofloater:runCrawl(self:initialCrawl())
self.playerCanMove = true
TileLock = true
end
For now, I think DL maxes at 4 as there are no level 5 monsters.
Now that I actually look at the monster creation code here, I see that we just use createThings, which just does this:
function GameRunner:createThings(aClass, n)
local things = {}
for i = 1,n or 1 do
local tile = self:randomRoomTile(1)
table.insert(things, aClass(tile,self))
end
return things
end
So it will just create a Monster, which is done like this:
function Monster:init(tile, runner, mtEntry)
if not MT then self:initMonsterTable() end
self.alive = true
self.tile = tile
self.tile:addDeferredContents(self)
self.runner = runner
self.mtEntry = mtEntry or self:randomMtEntry()
self.dead = self.mtEntry.dead
...
end
function Monster:randomMtEntry()
if not MT then self:initMonsterTable() end
local index = math.random(1,#MT)
return self:getMtEntry(index)
end
Meh. That creates a random monster from the whole table. I noticed, by the way, that there is only one level 4 monster, the Ankle Biter. A whole level populated with Ankle Biters seems odd, but of course we can add monsters at will. It’s just a pain.
Note that we can create a monster from its MT entry directly. So let’s do this:
function GameRunner:setupMonsters(n)
self.monsters = {}
local mt = Monster:getMonstersAtLevel(self.dungeonLevel)
for i = 1,n or 1 do
local mtEntry = mt[math.random(#mt)]
local tile = self:randomRoomTile(1)
table.insert(self.monsters, Monster(tile,self,mtEntry))
end
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
end
Darn, that’s a lot of code to have no test. But to test it would be horrible. We need to think about this. For now, let’s see what happens.
On level 1, I found ghosts and pink slimes, and a death fly. On level 2, I found vampire bats and toothheads. I didn’t see a murder hornet, but I’m sure the feature is working as intended. Now just for fun I’m going down to the ankle biter domain.
Ankle biters are tough. So this feature is working as intended. Commit: monsters chosen by level 1-4.
Now What About This Testing?
Or, properly, lack of testing. I wrote a trivial test for this code:
function Monster:getMonstersAtLevel(level)
if not MT then self:initMonsterTable() end
local t = {}
for i,m in ipairs(MT) do
if m.level == level then
table.insert(t,m)
end
end
return t
end
But no test for this code:
function GameRunner:setupMonsters(n)
self.monsters = {}
local mt = Monster:getMonstersAtLevel(self.dungeonLevel)
for i = 1,n or 1 do
local mtEntry = mt[math.random(#mt)]
local tile = self:randomRoomTile(1)
table.insert(self.monsters, Monster(tile,self,mtEntry))
end
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
end
There are certainly things that could have gone wrong here, and we could in fact imagine a test for it. We could make a GameRunner, let it init its tiles, set dungeon level, call this function (which might better take level as a parameter), and then check to see if there are the right number of monsters in the level and that they are all of the right level.
Just for sake of the comparison, let me actually write that, so we can assess it rationally. I start with this:
_:test("monsters correctly initialized", function()
local gm = GameRunner()
gm:createLevel(12)
local mt = gm.monsters
_:expect(#mt).is(6)
end)
That passes. So far so good. Now …
_:test("monsters correctly initialized", function()
local gm = GameRunner()
gm:createLevel(12)
local mt = gm.monsters
_:expect(#mt).is(6)
for i,m in ipairs(mt) do
_:expect(m.level).is(1)
end
end)
So that’s not really that awful to have written. So why didn’t I write it? Very likely with one more test, maybe this:
_:test("monsters correctly initialized level 2", function()
local gm = GameRunner()
gm:createLevel(12)
gm:createLevel(12)
local mt = gm.monsters
_:expect(#mt).is(6)
for i,m in ipairs(mt) do
_:expect(m.level).is(2)
end
end)
This took seconds to cut and paste to check the second level, and it gives me great confidence that the monster initialization is correct.
So it’s not all that bad. It is, however, a massive test in the sense that it creates and destroys 10880 tiles among other activities. And I have at least some fear that a test like this might mess up things over in the real game, despite being careful to keep everything isolated.
I knew darn well that this monster level thing worked. And still, when the tests ran, I felt more confidence, a kind of calmness or comfort. Yet I didn’t want to write the tests.
So there’s something–more likely some things–about the system design and the dev environment that is pushing against writing tests. Every time I do one, I can immediately see that it’s easier than I thought and gives me more confidence than I had before. And yet I don’t write them.
Let’s think about why. I really see two main reasons:
- Tools: Codea isn’t helpful about the tests. They dump information into the console and you have to scroll through it to get an useful information.
- Design: Tests seem to require a lot of setup. This one was relatively easy, but inside there was a lot going on. Even here we did have to reach into the guts of GameRunner to find out what it had done about monsters.
Tools
It would take some development to fix number one. I think what one would do would be to build some kind of internal results structure, perhaps just a list of failed, then successful tests, and display it. Heck, it might even suffice to change the console display to show only the broken tests, not the innumerably OK ones.
Tool work is important. Yes, individuals and interactions over processes and tools, but we’re not recommending coding in Cuneiform here. Codea, yes, Cuneiform no. But it does take us away from whatever our main purpose is, such as writing article number 104 for the Dungeon program.
Design
When tests require a lot of setup, or a lot of digging to get answers, or when they do lots of work that’s irrelevant to what we’re trying to find out, what we’re learning is that our objects aren’t helping us. Generally speaking, they are too inter-connected, and quite likely we’re dealing with at least one object that is doing too much.
Let’s look at our recent new methods in that light:
function Monster:getMonstersAtLevel(level)
if not MT then self:initMonsterTable() end
local t = {}
for i,m in ipairs(MT) do
if m.level == level then
table.insert(t,m)
end
end
return t
end
function GameRunner:setupMonsters(n)
self.monsters = {}
local mt = Monster:getMonstersAtLevel(self.dungeonLevel)
for i = 1,n or 1 do
local mtEntry = mt[math.random(#mt)]
local tile = self:randomRoomTile(1)
table.insert(self.monsters, Monster(tile,self,mtEntry))
end
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
end
We caused Monster to create a utility table, and then we used that table in GameRunner to create a bunch of Tile objects and random Monsters, put them into a table in GameRunner, and start their timers.
In the course of that, we create randomRoom tiles this way:
function GameRunner:randomRoomTile(roomToAvoid)
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile:getRoomNumber() ~= roomToAvoid and tile:isOpenRoom() then return tile end
end
end
This code fetches tiles from the tile array and interrogates them to see whether they are suitable to its needs.
What do these things have to do with game running? Very little. They do have to do with setting up the game, maybe, but even then why does the game runner know all these intimate details about tiles and monsters and their positions and their levels?
GameRunner has become a stash for all kinds of data and behavior, and while it all seems to work, we do encounter instances where the behavior is rather mysterious and surprising.
These are all indicators that our design has begun to go bad. I’d like to call it “technical debt”, because that’s a natural part of building software, where after a while we see a better way of doing things and we use that better way and the software gets smaller and better.
But I don’t see a better way. I don’t have some new insight that’s going to collapse all this down into a nicer structure.
This code just isn’t very well designed, and we’re seeing signs of that as we do things. So far, the signs aren’t terrible, though there have been some days when I buried myself, but by and large, the design is serviceable. But good? No, I don’t think I’d call it good.
Now I’m not here to beat myself up, nor to beat you up if you’ve got some code like this lying about. I’m here to detect what’s wrong with the code, not what’s wrong with you and me. There’s nothing wrong with you and me, we’re perfectly ordinary human beings, full of joys and sorrows, good habits and bad, hopes and dreams, struggling with problems that are always just a bit larger than we are.
The question before us in this work is to see the work as it truly is, and to make it better.
Yesterday, I think it was, I commented that I wanted to try to bear down a bit on improving the code. Today’s code review helps me redouble that desire.
The question I’m asking myself today is “what can I do today, based on what I’ve just thought about, to make this better?
Making This Better
To make this better, we should look right here:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createTiles()
self:clearLevel()
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
self:placePlayerInRoom1()
self:placeWayDown()
self:setupMonsters(6) -- <---
self.keys = self:createThings(Key,5)
self:createThings(Chest,5)
self:createLoots(10)
self:createButtons()
self.cofloater:runCrawl(self:initialCrawl())
self.playerCanMove = true
TileLock = true
end
We call a method in this class, GameRunner, to create six random monsters (at the level stored in self.dungeonLevel). Why don’t we ask Monster to do that for us?
Now, we could put our call to Monster right here, or we could plug it into the self:setupMonsters function. Since there are two parts to that function, getting the monsters and setting their timers, I think we should work there. So:
function GameRunner:setupMonsters(n)
self.monsters = {}
local mt = Monster:getMonstersAtLevel(self.dungeonLevel)
for i = 1,n or 1 do
local mtEntry = mt[math.random(#mt)]
local tile = self:randomRoomTile(1)
table.insert(self.monsters, Monster(tile,self,mtEntry))
end
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
end
Let’s first see what we’d like to say:
function GameRunner:setupMonsters(n)
self.monsters = Monster:getRandomMonsters(self, 6, self.dungeonLevel)
for i,monster in ipairs(self.monsters) do
monster:startAllTimers()
end
end
That’s what I’d like to say: get me six monsters at this level. Let’s see if we can change our tests to help us:
_:test("Can select monsters by level", function()
local t = Monster:getMonstersAtLevel(2)
_:expect(#t).is(3)
end)
_:test("monsters correctly initialized", function()
local gm = GameRunner()
gm:createLevel(12)
local mt = gm.monsters
_:expect(#mt).is(6)
for i,m in ipairs(mt) do
_:expect(m.level).is(1)
end
end)
_:test("monsters correctly initialized level 2", function()
local gm = GameRunner()
gm:createLevel(12)
gm:createLevel(12)
local mt = gm.monsters
_:expect(#mt).is(6)
for i,m in ipairs(mt) do
_:expect(m.level).is(2)
end
end)
Tell the truth, these tests, though they are a bit remote, should do the job for us as they stand. Let’s run them.
2: monsters correctly initialized -- GameRunner:354: attempt to call a nil value (method 'getRandomMonsters')
No surprise there. And we code this:
function Monster:getRandomMonsters(runner, number, level)
local t = self:getMonstersAtLevel(level)
local result = {}
for i = 1,number do
local mtEntry = t[math.random(#t)]
local tile = runner:randomRoomTile()
local monster = Monster(tile, runner, mtEntry)
table.insert(result, monster)
end
return result
end
Basically the same but now it’s at least half-way in the right class, Monster.
Is this a big deal? Not at all, and that’s the point. We’ve made the design a tiny bit better, with a tiny bit of effort. And, with luck, we’ll be more likely to remember to “tell don’t ask”, telling our objects to do what we want rather than interrogating them and doing stuff on their behalf elsewhere.
Now we do see that we had to call back to get a random room tile, and that’s a bit more tricky, because GameRunner owns the array of tiles and no one else does.
I’d like to do something about that. Looking out longer term, maybe there is a GameBoard object or something that has the dungeon in it. Maybe its name is Dungeon, now that I mention it. And maybe that object could pick up the load from GameRunner and other folks who are manipulating the tiles.
Right now, what I’d like to do would be to forward the randomRoomTile stuff to this new object, letting it do the work. Right now, it will serve as a sort of helper object to allow GameRunner to offload some work.
Let’s try that. And let’s test it. Here’s my first test:
-- testDungeon
-- RJ 20210224
local dungeon
function testDungeon()
CodeaUnit.detailed = true
_:describe("Test Dungeon Utilities", function()
_:before(function()
local gm = GameRunner()
gm:createLevel(12)
dungeon = Dungeon(gm.tiles)
end)
_:after(function()
end)
_:test("Dungeon has tiles", function()
_:expect(#dungeon.tiles).is(86)
end)
end)
end
Dungeon = class()
function Dungeon:init(tiles)
self.tiles = tiles
end
It runs. Note that I created the Dungeon by just ripping the guts out of GameRunner. That won’t stand so I’ll change the before:
_:before(function()
local gm = GameRunner()
gm:createLevel(12)
dungeon = gm:getDungeon()
end)
function GameRunner:getDungeon()
return Dungeon(self.tiles)
end
So that’s fine. Now we want help getting a random room tile. Here’s GameRunner now:
function GameRunner:randomRoomTile(roomToAvoid)
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile:getRoomNumber() ~= roomToAvoid and tile:isOpenRoom() then return tile end
end
end
So:
function GameRunner:randomRoomTile(roomToAvoid)
return self:Dungeon():randomRoomTile(roomToAvoid)
end
function Dungeon:randomRoomTile(roomToAvoid)
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile:getRoomNumber() ~= roomToAvoid and tile:isOpenRoom() then return tile end
end
end
We do not know tileCountX and tileCountY, nor getTile, and we must. Let’s compute the counts:
_:test("Dungeon has tiles", function()
_:expect(#dungeon.tiles).is(86)
_:expect(dungeon.tileCountX).is(85)
_:expect(dungeon.tileCountY).is(64)
end)
function Dungeon:init(tiles)
self.tiles = tiles
self.tileCountX = #self.tiles - 1
self.tileCountY = #self.tiles[1] - 1
end
This needed to be change, I had a self: left in it:
function GameRunner:randomRoomTile(roomToAvoid)
return Dungeon():randomRoomTile(roomToAvoid)
end
Now I get an error that surprises me:
TestDungeon:33: attempt to get length of a nil value (field 'tiles')
stack traceback:
TestDungeon:33: in field 'init'
... false
end
setmetatable(c, mt)
return c
end:24: in global 'Dungeon'
GameRunner:323: in method 'randomRoomTile'
Monster:24: in method 'getRandomMonsters' ...
Oh, well certainly that code is still wrong. I meant:
function GameRunner:randomRoomTile(roomToAvoid)
return self:getDungeon():randomRoomTile(roomToAvoid)
end
But I rather wish that I hadn’t changed that code until my test worked. So I’ll revert that bit. OK, my tests correctly get the tile counts. Now let’s test our new random room code:
_:test("Get a random room", function()
local tile = dungeon:randomRoomTile(1)
_:expect(tile:getRoomNmber()).isnt(1)
_:expect(tile:isOpenRoom()).is(true)
end)
I expect this to fail looking for getTile.
2: Get a random room -- TestDungeon:46: attempt to call a nil value (method 'getTile')
And we give Dungeon that:
function Dungeon:getTile(pos)
return self:privateGetTileXY(pos.x, pos.y)
end
function Dungeon:privateGetTileXY(x,y)
if x<=0 or x>self.tileCountX or y<=0 or y>self.tileCountY then
return Tile:edge(x,y, self)
end
return self.tiles[x][y]
end
Hm …
2: Get a random room -- TestDungeon:28: attempt to call a nil value (method 'getRoomNmber')
Perhaps we should have used the conventional spelling of “number”:
_:test("Get a random room", function()
local tile = dungeon:randomRoomTile(1)
_:expect(tile:getRoomNumber()).isnt(1)
_:expect(tile:isOpenRoom()).is(true)
end)
Test runs. Plug the new capability into GameRunner:
function GameRunner:randomRoomTile(roomToAvoid)
return self:getDungeon():randomRoomTile(roomToAvoid)
end
Tests run, game runs fine. Commit: moved randomRoomTile to new Dungeon class.
This is, of course, just a start. Over time we can move more and more logic over to Dungeon. For now, I’m tempted to improve the fact that we create a new dungeon on every use. That’s not a big deal, though, since it’s just a few lines of code. Let’s defer it: we have no reason to think it’s a problem.
And let’s do this while we’re at it:
function GameRunner:getTile(aPosition)
return self:getDungeon():getTile(aPosition)
end
And let’s now replace GameRunner’s private getter:
function GameRunner:privateGetTileXY(x,y)
return self:getDungeon():privateGetTileXY(x,y)
end
All should be well. I’d rather remove it, but it’s used. We’ll go in steps. Everything works. Let’s see who’s using that private call and see if we can make them stop. Here’s one:
function GameRunner:setHallwayTile(x,y)
local t = self:privateGetTileXY(x,y)
if not t:isRoom() then
self:setTile(Tile:room(x,y,self))
end
end
That can be moved to Dungeon:
function Dungeon:setHallwayTile(x,y)
local t = self:privateGetTileXY(x,y)
if not t:isRoom() then
self:setTile(Tile:room(x,y,self))
end
end
function GameRunner:setHallwayTile(x,y)
self:getDungeon():setHallwayTile(x,y)
end
There are a number of tests calling the private thing as well. However, they are in a test suite that I’m not running at present, the tests for the hallways and a lot of other stuff. We’ll remove the method from GameRunner and deal with that when we turn those tests back on.
We need setTile:
function Dungeon:setTile(aTile)
assert(not TileLock, "attempt to set tile while locked")
local pos = aTile:pos()
self.tiles[pos.x][pos.y] = aTile
end
And to call it from GameRunner.
function GameRunner:setTile(aTile)
self:getDungeon():setTile(aTile)
end
Everything runs. Commit: moving functionality to Dungeon class.
I think this is a good spot to pause. I expect some of my three readers are scratching their heads. Let’s sum up.
Summary - That Delegation
I think you can see how this is going to go. We’ve built this Dungeon object and we’re moving manipulation of the tile array into it. Over time, we can envision that more and more of that kind of thing will move over there.
But at what cost??? Most of the actual work is either requested by GameRunner on its own behalf, and formerly done in GameRunner and now passed on down to Dungeon. Or, operations are requested of GameRunner from outside GameRunner and it passes them on to a Dungeon.
Even “worse”, we create a new Dungeon on every call to it. Isn’t this a step backwards in efficiency? Is clarity even increased by passing perfectly good questions on down to some other object?
Well, my answers are “efficiency is not really (much) worse” and “yes, clarity is increased”. Let’s take them one at a time.
Efficiency
Yes, we create a new Dungeon on essentially every use of it, and it does at least a couple of lines of initialization, and then we call the method we really had in mind. That is unquestionably more code executed and therefore more time used.
However, we have absolutely no reason to think that it is too much. The game runs just fine. Furthermore, we could readily save the creation by implementing getDungeon like this:
function GameRunner:getDungeon()
if not self.dungeon then self.dungeon = Dungeon(self.tiles) end
return self.dungeon
Why am I not doing that now? Because I don’t want to figure out where to clear self.dungeon, even though I know it has to be done in createLevel and probably nowhere else. Lazy inits are neat but they verge on cool, so I’ll avoid it for now, but I’ll know I could do it.
When the time comes, there may even be other things that will be better, an explicit init in createLevel, etc. But I think it’s more likely that the issue will be solve another way, by giving Dungeon the responsibility to create new levels internally.
So efficiency, right now, isn’t a concern to me.
Clarity
YMMV, but I prefer small, well-focused objects, and Dungeon is smaller than GameRunner and always will be smaller than GameRunner is right now, so moving things over to it is a win. In addition, GameRunner has many responsibilities, and Dungeon has just one, managing the tiles of the Dungeon. Over time, I expect Dungeon to grow, and GameRunner to shrink accordingly.
We’ll see what happens as we go forward, and we’ll all make our own judgments on whether we’d do this sort of thing. To my taste, these have been a few small steps of improvement.
Just browsing …
I find myself browsing to see what else I could move over to Dungeon class. I was looking, for example, at this part of the hallway carving:
function GameRunner:verticalCorridor(fromY, toY, x)
fromY,toY = math.min(fromY,toY), math.max(fromY,toY)
for y = fromY, toY do
self:setHallwayTile(x,y)
end
end
That could certainly move. It’s called from here:
function Room:hvCorridor(aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
self.runner:horizontalCorridor(startX,endX,startY)
self.runner:verticalCorridor(startY,endY,endX)
end
function Room:vhCorridor(aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
self.runner:verticalCorridor(startY,endY,startX)
self.runner:horizontalCorridor(startX,endX,endY)
end
Those are called from here:
function Room:connect(aRoom)
if math.random(1,2) == 2 then
self:hvCorridor(aRoom)
else
self:vhCorridor(aRoom)
end
end
And that’s called from GameRunner:
function GameRunner:connectRooms()
for i,r in ipairs(self.rooms) do
if i > 1 then
r:connect(self.rooms[i-1])
end
end
end
We could readily move all that over to Dungeon. Let’s do that, bit by bit. There are at least two ways to go about it. One would be to move the connect call first and build down, the other to move the bottom-most calls first and build up.
Let’s do it top down, because I think it’ll be enlightening.
I think it’s clear that we should create a Dungeon in connectRooms and pass it down to the room, who will pass it on. Let’s see if we can keep things working each step of the way.
First:
function GameRunner:connectRooms()
local dungeon = self:getDungeon()
for i,r in ipairs(self.rooms) do
if i > 1 then
r:connect(dungeon, self.rooms[i-1])
end
end
end
And then:
function Room:connect(dungeon, aRoom)
if math.random(1,2) == 2 then
self:hvCorridor(dungeon, aRoom)
else
self:vhCorridor(dungeon, aRoom)
end
end
And then:
function Room:hvCorridor(dungeon, aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
self.runner:horizontalCorridor(startX,endX,startY)
self.runner:verticalCorridor(startY,endY,endX)
end
function Room:vhCorridor(dungeon, aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
self.runner:verticalCorridor(startY,endY,startX)
self.runner:horizontalCorridor(startX,endX,endY)
end
Everything should still be working, we’ve just passed down an additional parameter, as yet unused. And tests pass and game plays. We could commit now and ship if we wanted to. But we don’t want to.
function Room:vhCorridor(dungeon, aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
dungeon:verticalCorridor(startY,endY,startX)
dungeon:horizontalCorridor(startX,endX,endY)
end
function Room:hvCorridor(dungeon, aRoom)
local startX,startY = self:center()
local endX,endY = aRoom:center()
dungeon:horizontalCorridor(startX,endX,startY)
dungeon:verticalCorridor(startY,endY,endX)
end
And then:
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:verticalCorridor(fromY, toY, x)
fromY,toY = math.min(fromY,toY), math.max(fromY,toY)
for y = fromY, toY do
self:setHallwayTile(x,y)
end
end
I expect this to work, and indeed it does. Now can I remove setHallwayTile from gameRunner? I see no one using it now. Indeed we can, and when we check our commit status we find -1+15 lines in TestDungeon (where the class currently lives), -9+9 in Room, where we just chonked in a parameter and finally used it, and -19+2 lines in GameRunner. So that’s a nice bit of messing about removed from GameRunner.
Commit: Move corridor carving to Dungeon class.
With that as a demonstration of how Dungeon is going to help clarify things, I think I’ll wrap this up. We’ve added a cool new feature, different monsters on different levels, and in so doing we recognized that the system’s design is holding us–well, me–back from testing, and we improved the design rather nicely in response. And we’re not done improving yet, I’ll wager.
If there’s a lesson here, for me it is that with a decent design, which we have, and with some useful tests, we can improve the design fairly readily. I’d like to have better tests and I’d like to turn on that one batch that are presently turned off.
Maybe next time.