Dungeon 174

Let’s take another swing at moving dungeon creation out of GameRunner. We’ll focus on creating our learning level’s layout.

Since my first cut at this idea came to no good end, I believe I need to review the existing code and objects a bit more carefully, to be sure that I can hook things up properly.

No. I’m going to let the code participate in this planning and review. I’m going to start doing, and will try to be a bit more judicious before I make a structural decision.

Right now, the learning level is created in GameRunner, as are the regular random levels:

function GameRunner:createLearningLevel()
    self.dungeonLevel = 1
    TileLock = false
    self:createTiles()
    self:clearLevel()
    self:createLearningRooms()
    self:connectLearningRooms()
    self: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

I see two main options. First is to move the dungeon creation into the Dungeon class, which holds the tiles and does some work for GameRunner and others. Dungeon has these methods already:

__tostring
availableTilesAtDistanceFromPlayer
availableTilesAtDistanceFromTile
availableTilesCloserToPlayer
availableTilesCloserToTile
availableTilesFurtherFromPlayer
availableTilesFurtherFromTile
availableTilesSameDistanceFromPlayer
cellIsAccessible
defineTile
farawayOpenRoomTile
furthestFrom
getTile
horizontalCorridor
init
is_a
manhattanBetween
manhattanToPlayer
nearestContents
neighborOffsets
neighbors
playerTile
privateGetTileXY
randomHallwayTile
randomPositionAvoiding
randomRoomTile
randomRoomTileAvoiding
setHallwayTile
setTestPlayerAtPosition
verticalCorridor

Looking at that list, I notice the horizontalCorridor and verticalCorridor method, which leads me to look at the connectRooms function in GameRunner:

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 that leads me to Room:connect:

function Room:connect(dungeon, aRoom)
    if math.random(1,2) == 2 then
        self:hvCorridor(dungeon, aRoom)
    else
        self:vhCorridor(dungeon, aRoom)
    end
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

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

So here, Dungeon is helping to create the dungeon. Why didn’t we pass the room collection on down and let it do the whole job? These are the questions that have no answers.

But most of Dungeon is about run-time activities, and interrogating tile status during creation. It has the defineTile function, which does set a specific tile:

function Dungeon:defineTile(aTile)
    assert(not TileLock, "attempt to set tile while locked")
    local pos = aTile:pos()
    self.tiles[pos.x][pos.y] = aTile
end

I’m still trying to decide between doing more creation in Dungeon, or doing it in a separate DungeonMaker class.

GameRunner knows the tiles, and it often uses a Dungeon instance to deal with them:

function GameRunner:createTiles()
    self.tiles = {}
    for x = 1,self.tileCountX+1 do
        self.tiles[x] = {}
        for y = 1,self.tileCountY+1 do
            local tile = Tile:edge(x,y, self)
            self:defineTile(tile)
        end
    end
end

Here, we create them directly, but … then at the last second:

function GameRunner:defineTile(aTile)
    self:getDungeon():defineTile(aTile)
end

We let Dungeon put them away–into our table, because Dungeon has a pointer to the same tiles table as GameRunner.

This is not good separation of concerns at all.

How did it get that way? It just growed. How could a smart person like me let that happen? It seemed OK at the time. Even Homer sometimes nods. (Not that Homer.)

OK, here’s what I’m going to try. I’m going to have a new class, DungeonMaker, with the intention that, ultimately, it has all the dungeon creation code in it. It will collaborate with Dungeon class to do the creation. Somehow, between them, they’ll set things up so that GameRunner just has an instance of Dungeon and does no creation or tile-mongering at all.

This may not work.

I think I’ll start by pushing creation from GameRunner over to Dungeon. As Dungeon picks up new things that seem wrong for it, we’ll take that opportunity to push again, to DungeonMaker.

At this moment, I do not see how to TDD this. I’ll try to get into the mode of doing so as soon as I can. At the moment this is exploratory, but I am optimistic that it’s going to work.

I was thinking we could start by pushing the code from connectRooms over to Dungeon. That raises the question of whether Dungeon is given the rooms, or created them. In the long run, we’ll want it to create them, and to hold on to them in case GameRunner needs them. But let’s pass them in, because this change should be easy, just to get started.

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

That becomes:

function GameRunner:connectRooms()
    self:getDungeon():connectRooms(self.rooms)
end

function Dungeon:connectRooms(rooms)
    for i,r in ipairs(rooms) do
        if i > 1 then
            r:connect(self, rooms[i-1])
        end
    end
end

This should be harmless. Tests run, game runs. Commit: room connection moved to Dungeon from GameRunner.

That went nicely. Let’s move createTiles. For now, we’ll let it return the tiles so that GameRunner can hold on to its own copy.

function GameRunner:createTiles()
    self.tiles = {}
    for x = 1,self.tileCountX+1 do
        self.tiles[x] = {}
        for y = 1,self.tileCountY+1 do
            local tile = Tile:edge(x,y, self)
            self:defineTile(tile)
        end
    end
end

I decided to pass in the table size:

function Dungeon:createTiles(tileCountX, tileCountY)
    local tiles = {}
    for x = 1,tileCountX+1 do
        tiles[x] = {}
        for y = 1,tileCountY+1 do
            local tile = Tile:edge(x,y, self)
            self:defineTile(tile)
        end
    end
    self.tiles = tiles
    return tiles
end

This conflicts with the current init:

function Dungeon:init(tiles, runner, player)
    self.tiles = tiles
    self.runner = runner
    self.tileCountX = #self.tiles - 1
    self.tileCountY = #self.tiles[1] - 1
    self.player = player
end

Let’s change the init to allow for a dungeon without tiles, and let’s set the counts in our new method:

function Dungeon:init(tiles, runner, player)
    self.tiles = tiles
    self.runner = runner
    self.player = player
    if self.tiles then
        self.tileCountX = #self.tiles - 1
        self.tileCountY = #self.tiles[1] - 1
    end
end

function Dungeon:createTiles(tileCountX, tileCountY)
    self.tileCountX = tileCountX
    self.tileCountY = tileCountY
    local tiles = {}
    for x = 1,tileCountX+1 do
        tiles[x] = {}
        for y = 1,tileCountY+1 do
            local tile = Tile:edge(x,y, self.runner)
            self:defineTile(tile)
        end
    end
    self.tiles = tiles
    return tiles
end

And finally:

function GameRunner:createTiles()
    self:getDungeon():createTiles(self.tileCountX, self.tileCountY)
end

Test. First time thru, I forgot to say self.runner when I created the tile.

Next defect, defineTile wants to look at the tile array. We need to set it as a member variable at the top.

I was trying to get fancy and use a local table “for efficiency”. Bleh. Do this:

function Dungeon:createTiles(tileCountX, tileCountY)
    self.tileCountX = tileCountX
    self.tileCountY = tileCountY
    self.tiles = {}
    for x = 1,tileCountX+1 do
        self.tiles[x] = {}
        for y = 1,tileCountY+1 do
            local tile = Tile:edge(x,y, self.runner)
            self:defineTile(tile)
        end
    end
    return self.tiles
end

Error:

attempt to index a nil value
stack traceback:
	[C]: in for iterator 'for iterator'
	GameRunner:32: in method 'clearLevel'
	GameRunner:145: in method 'createLevel'
	Dungeon:18: in field '_before'
	codeaUnit:44: in method 'test'
	Dungeon:30: in local 'allTests'
	codeaUnit:16: in method 'describe'
	Dungeon:10: in function 'testDungeon'
	[string "testDungeon()"]:1: in main chunk
	codeaUnit:139: in field 'execute'
	Tests:515: in function 'runCodeaUnitTests'
	Main:6: in function 'setup'

One more look at this and then I’m going to revert. We want to move clearLevel anyway, but what’s wrong now?

Oh. After carefully saying I’d return the tiles and save them in GameRunner, I forgot.

function GameRunner:createTiles()
    self.tiles = self:getDungeon():createTiles(self.tileCountX, self.tileCountY)
end

All good now. Commit: moved createTiles function to Dungeon class.

Let’s do clearLevel:

function GameRunner:clearLevel()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:convertToEdge()
        end
    end
end

function GameRunner:clearLevel()
    self:getDungeon():clearLevel()
end

function Dungeon:clearLevel()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:convertToEdge()
        end
    end
end

This ought to work. And does. We’re getting semi-good at this.

Commit: clearLevel function moved to Dungeon class.

Now clearTiles, which should be renamed as well.

function GameRunner:clearTiles()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:clearVisible()
        end
    end
end

function Player:moveBy(aStep)
    self.runner:resetVisibility()
    local tile = self.tile:legalNeighbor(self,aStep)
    tile:moveObject(self)
end

function GameRunner:resetVisibility()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:clearVisible()
        end
    end
end

Test the rename. Works. Commit? No, I’ll do the move first.

function GameRunner:resetVisibility()
    self:getDungeon():resetVisibility()
end

function Dungeon:resetVisibility()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:clearVisible()
        end
    end
end

Test. All good. Commit: moved resetVisibility function to Dungeon class.

OK what else does GameRunner do that Dungeon could help with?

function GameRunner:convertEdgesToWalls()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:convertEdgeToWall()
        end
    end
end

Same drill.

function GameRunner:convertEdgesToWalls()
    self:getDungeon():convertEdgesToWalls()
end

function Dungeon:convertEdgesToWalls()
    for i,row in ipairs(self.tiles) do
        for j,tile in ipairs(row) do
            tile:convertEdgeToWall()
        end
    end
end

Test. Commit: convertEdgesToWalls moved to Dungeon class.

What else? I’m looking at references in GameRunner to self.tiles.

One more easy one:

function GameRunner:dungeonSize()
    return self.tileSize*(self.tileCountX+1), self.tileSize*(self.tileCountY+1)
end

I’m wondering how Dungeon is going to know the tile size. I think we should create it knowing that. And, shouldn’t we compute the value at creation time and cache it?

One thing at a time.

function GameRunner:init()
    self.tileSize = 64
    self.tileCountX = 85 -- if these change, zoomed-out scale 
    self.tileCountY = 64 -- may also need to be changed.
    self.cofloater = Floater(50,25,4)
    self.musicPlayer = MonsterPlayer(self)
    self:initializeSprites()
    self.dungeonLevel = 0
    self.requestNewLevel = false
    self.playerRoom = 1
    Bus:subscribe(self, self.createNewLevel, "createNewLevel")
end

function GameRunner:getDungeon()
    return Dungeon(self.tiles, self, self.player)
end

I think we’re moving toward the GameRunner just creating the Dungeon instance and deferring all the dungeon stuff to it, including drawing. We will probably create a new one for each level. So let’s first stop creating dungeon with tiles, but instead with tileSize. I’m not entirely comfortable with that. I think tileSize is a system-wide constant, but we don’t have that concept.

I’m going to try something somewhat large.

No. It’s too soon. Let’s at least move this:

function GameRunner:createRandomRooms(count)
    local r
    self.rooms = {}
    while count > 0 do
        count = count -1
        local timeout = 100
        local placed = false
        while not placed do
            timeout = timeout - 1
            r = Room:random(self.tileCountX,self.tileCountY, 5,14, self)
            if self:hasRoomFor(r) then
                placed = true
                table.insert(self.rooms,r)
                r:paint(#self.rooms)
            elseif timeout <= 0 then
                placed = true
            end
        end
    end
end

That will require:

function GameRunner:hasRoomFor(aRoom)
    return aRoom:isAllowed(self)
end

Which does:

function Room:isAllowed()
    for x = self.x1,self.x2 do
        for y = self.y1,self.y2 do
            local t = self.runner:getTile(vec2(x,y))
            if not t:isEdge() then return false end
        end
    end
    return true
end

That’s interesting, in that we pass the runner to isAllowed but don’t use it in there. If we were to use it, we could pass in the dungeon, which understands getTile just fine. So:

function Room:isAllowed(dungeon)
    for x = self.x1,self.x2 do
        for y = self.y1,self.y2 do
            local t = dungeon:getTile(vec2(x,y))
            if not t:isEdge() then return false end
        end
    end
    return true
end

function GameRunner:hasRoomFor(aRoom)
    return aRoom:isAllowed(self:getDungeon())
end

This should still work. But it doesn’t. Revert.

Let’s try again to pass our dungeon to isAllowed and make it use it. I really have no idea what I did wrong. If it happens again, I’ll debug to learn something I don’t know. And I expect it to happen again.

function GameRunner:hasRoomFor(aRoom)
    return aRoom:isAllowed(self:getDungeon())
end

function Room:isAllowed(dungeon)
    for x = self.x1,self.x2 do
        for y = self.y1,self.y2 do
            local t = dungeon:getTile(vec2(x,y))
            if not t:isEdge() then return false end
        end
    end
    return true
end

I’m sure that’s exactly what I did before. But I guess it wasn’t, because now it’s working. Bizarre.

Now let’s move the random room maker:

function GameRunner:createRandomRooms(count)
    local r
    self.rooms = {}
    while count > 0 do
        count = count -1
        local timeout = 100
        local placed = false
        while not placed do
            timeout = timeout - 1
            r = Room:random(self.tileCountX,self.tileCountY, 5,14, self)
            if self:hasRoomFor(r) then
                placed = true
                table.insert(self.rooms,r)
                r:paint(#self.rooms)
            elseif timeout <= 0 then
                placed = true
            end
        end
    end
end

Move that. First do this:

function Dungeon:createRandomRooms(count)
    local r
    self.rooms = {}
    while count > 0 do
        count = count -1
        local timeout = 100
        local placed = false
        while not placed do
            timeout = timeout - 1
            r = Room:random(self.tileCountX,self.tileCountY, 5,14, self)
            if self:hasRoomFor(r) then
                placed = true
                table.insert(self.rooms,r)
                r:paint(#self.rooms)
            elseif timeout <= 0 then
                placed = true
            end
        end
    end
    return self.rooms
end

Then move or inliine hasRoomFor

function GameRunner:hasRoomFor(aRoom)
    return aRoom:isAllowed(self:getDungeon())
end

We’ll inline it:

function Dungeon:createRandomRooms(count)
    local r
    self.rooms = {}
    while count > 0 do
        count = count -1
        local timeout = 100
        local placed = false
        while not placed do
            timeout = timeout - 1
            r = Room:random(self.tileCountX,self.tileCountY, 5,14, self)
            if r:isAllowed(self) then
                placed = true
                table.insert(self.rooms,r)
                r:paint(#self.rooms)
            elseif timeout <= 0 then
                placed = true
            end
        end
    end
    return self.rooms
end

We’re returning the rooms because GameRunner wants them, but we’re retaining them against future need. (YAGNI?)

function GameRunner:createRandomRooms(count)
    self.rooms = self:getDungeon():createRandomRooms()
end

I think this should work, but I’m feeling nervous.

Well, I do have to pass in count:

function GameRunner:createRandomRooms(count)
    self.rooms = self:getDungeon():createRandomRooms(count)
end

Ah, an error:

Tile:76: Invariant Failed: Tile must receive Runner
stack traceback:
	[C]: in function 'error'
	Tile:76: in field 'init'
	... false
    end
end:20>
	(...tail calls...)
	Room:84: in method 'paint'
	Dungeon:198: in method 'createRandomRooms'
	GameRunner:185: in method 'createRandomRooms'
	GameRunner:130: in method 'createLevel'
	Dungeon:18: in field '_before'
	codeaUnit:44: in method 'test'
	Dungeon:30: in local 'allTests'
	codeaUnit:16: in method 'describe'
	Dungeon:10: in function 'testDungeon'
	[string "testDungeon()"]:1: in main chunk
	codeaUnit:139: in field 'execute'
	Tests:515: in function 'runCodeaUnitTests'
	Main:6: in function 'setup'

I passed in self, not self.runner.

            r = Room:random(self.tileCountX,self.tileCountY, 5,14, self.runner)

And that works. Commit: createRandomRooms moved to Dungeon class.

OK, the only references to self.tiles in GameRunner are the drawing calls, and the dungeon size one. Let’s first create a dungeon, with no tiles, but with a size, and hold on to it in GameRunner. (I do think we’ll replace it from time to time, when we create a new level. We’ll see how that goes when we do it.)

We’re In Big Trouble, Guys! – Ed Anderi

Ah, there’s an issue. Dungeon is supposed to have a reference to the player, so we can’t really initialize one until we have a player, unless we change that rule. It does have a reference to the runner. Its only use of runner is to pass it on to things it creates, like tiles and rooms.

Let’s begin by making it ask the runner for the player when it is needed.

function Dungeon:init(tiles, runner)
    self.tiles = tiles
    self.runner = runner
    if self.tiles then
        self.tileCountX = #self.tiles - 1
        self.tileCountY = #self.tiles[1] - 1
    end
end

No more player accepted. a getter for player:

function Dungeon:getPlayer()
    return self.runner:getPlayer()
end

Does runner have that? No. Provide it.

function GameRunner:getPlayer()
    return self.player
end

Use it:

function Dungeon:playerTile()
    return self:getPlayer():getTile()
end

Uh oh, we have a weird test hack:

function Dungeon:setTestPlayerAtPosition(aVector)
    self.player = DungeonTestPlayer(self, aVector)
end

Oh that’s nasty. This is what happens when things are hard to test and we plug in fake objects: when we change the object we have to change the tests.

There’s really only one test that uses this, and it is saving and restoring the Runner. So let’s go ahead and invade:

function Dungeon:setTestPlayerAtPosition(aVector)
    self.runner.player = DungeonTestPlayer(self, aVector)
end

This may explode but I rather suspect that it won’t. Yes, tests run and game runs. Commit: Dungeon fetches player from GameRunner rather than holding it.

We are left with the drawing code in GameRunner, which references its copy of tiles, and these two methods:

function GameRunner:dungeonSize()
    return self.tileSize*(self.tileCountX+1), self.tileSize*(self.tileCountY+1)
end

function GameRunner:getDungeon()
    return Dungeon(self.tiles, self, self.player)
end

And we need to think about the bigger picture here for a moment. We’ve moved a lot of interesting stuff into Dungeon. Here’s its current protocol:

__tostring
availableTilesAtDistanceFromPlayer
availableTilesAtDistanceFromTile
availableTilesCloserToPlayer
availableTilesCloserToTile
availableTilesFurtherFromPlayer
availableTilesFurtherFromTile
availableTilesSameDistanceFromPlayer
cellIsAccessible
clearLevel
connectRooms
convertEdgesToWalls
createRandomRooms
createTiles
defineTile
farawayOpenRoomTile
furthestFrom
getPlayer
getTile
horizontalCorridor
init
is_a
manhattanBetween
manhattanToPlayer
nearestContents
neighborOffsets
neighbors
playerTile
privateGetTileXY
randomHallwayTile
randomPositionAvoiding
randomRoomTile
randomRoomTileAvoiding
resetVisibility
setHallwayTile
setTestPlayerAtPosition
verticalCorridor

Now, I believe that we’ll want to divide up Dungeon’s protocol into dungeon-running operations and dungeon-making operations, and move the making ones to a new class. The first phase was to remove the functionality from GameRunner. GameRunner still has the “big” creation methods, createLevel and createLearningLevel.

All that is to come. What is somewhat bugging me just now is that GameRunner’s getDungeon gets a new Dungeon instance on every call.

That reminds me, Dungeon doesn’t want to be passed player. Change getDungeon:

function GameRunner:getDungeon()
    return Dungeon(self.tiles, self)
end

Works. Commit: GameRunner no longer passes player to Dungeon.

I think what I’d like to do is create a new Dungeon instance when we create a level, and cache it thereafter. Like this:

function GameRunner:createLevel(count)
    self.dungeonLevel = self.dungeonLevel + 1
    if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
    TileLock=false
    self:createNewDungeon()
    self:createTiles()
...

function GameRunner:createLearningLevel()
    self.dungeonLevel = 1
    TileLock = false
    self:createNewDungeon()
    self:createTiles()
...

function GameRunner:createNewDungeon()
    self.dungeon = Dungeon(self)
end

function GameRunner:getDungeon()
    return self.dungeon
end

function Dungeon:init(runner)
    self.runner = runner
    -- declare unset member variables. needs improvement.
    self.tiles = nil
    self.tileCountX = nil
    self.tileCountY = nil
end

Does this run? Some tests explode, finally, in testDungeon2.

GameRunner:28: attempt to index a nil value
stack traceback:
	GameRunner:28: in method 'createTiles'
	Tests:21: in field '_before'
	codeaUnit:44: in method 'test'
	Tests:31: in local 'allTests'
	codeaUnit:16: in method 'describe'
	Tests:10: in function 'testDungeon2'
	[string "testDungeon2()"]:1: in main chunk
	codeaUnit:139: in field 'execute'
	Tests:515: in function 'runCodeaUnitTests'
	Main:6: in function 'setup'

This test is telling me that I need to create the Dungeon sooner. Let’s create it in GameRunner:init. I’m going to leave the create calls in the level creators for now. That fixes the problem. I need to test for a second level working though. And it does.

I think we’re at a good place. Commit: dungeon created in GameRunner and cached for use, rather than continually recreating.

Let’s do a quick retro and then grab some lunch. After that, a short topdown run to fill my tank for a longer trip in a few days. It’s lovely out today.

Retro

We have moved, in all, half a dozen methods, and about 50 lines of code over to Dungeon, and forwarded messages from GameRunner to use them. We’ll find that a number of those forwards move as well, as we continue to move capability over to Dungeon (and, I imagine, to a soon-to-be-created DungeonMaker).

It went perfectly smoothly, by which I mean I only reverted once–and still don’t know what I typed wrongly–and all the other mistakes were easy to fix. With a refactoring tool, I think most of those quick issues would have disappeared, as the refactoring tool would have unraveled the issues.

Such is the pleasure of working with a simple system. I rather prefer it for what I’m doing. It feels more like programming and less like asking someone else to program for me. In a real product, I’d want the better tools.

I feel that we’ve made good progress toward having a place to stand for our dungeon making efforts, and acknowledge that it has been a few days since we saw any direct progress on the learning level.

I am reminded of this site, called to my attention by Bruce Onder. It has a vast array of interesting articles about game creation. I mean years’ worth of good and interesting work. Makes me feel small, and like a beginner.

D2.zip