Let’s get items properly distributed. After that, some leveling, perhaps. I don’t know. I’ll have to wait and see what I do.

The final mission yesterday was to place all the “things” in rooms other than room number one. The current code uses randomRoomTile to select an open tile in which to place the item. That code looks like this:

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

It turns out that this code has an odd point of failure, which is that hallways are drawn from room center to room center, with the result that there are always some tiles in a room that don’t have the room’s room number. So this code doesn’t realize that such tiles are meant to be off-limits, and they get accepted.

I tried placing objects before drawing hallways, but that led to some odd graphical effects. I’m guessing those are due to lighting, since the princess always has some location after the initial setup. Be that as it may, it didn’t work well.

I have some ideas for a better way:

  1. Give hallways their own room number, and check for that as well as one.
  2. Cause hallways not to overwrite existing room tiles, leaving the original room numbers.
  3. … well, I don’t have a third idea offhand. Sorry.

OK, maybe I have a more grand idea. We could do all these things:

  1. Create two new tile types, hall and door;
  2. Cause halls to have their own number;
  3. Cause halls not to overwrite room floors;
  4. Cause halls to mark the tiles where they pierce the walls as “door”.

This may seem a bit speculative, and that’s a fair cop, but I do need to solve doors at some point, and knowing where they might be will be part of that. I think it’s also a bit elaborate and may require a lot of change.

Here’s an example of what I’m worried about:

function Tile:drawMapCell(center)
    if self.kind ~= TileRoom or not self.seen then return end
    pushMatrix()
    pushStyle()
    rectMode(CENTER)
    translate(center.x, center.y)
    fill(255)
    rect(0,0,TileSize,TileSize)
    popStyle()
    popMatrix()
end

That is the code that draws the map cells. It draws only room tiles. If we create edge and door tiles, this code will have to change. There are probably quite a few such things lying about. This is what happens when you try to distinguish objects by their members instead of using classes. We’ve stumbled onto another design issue, not a huge and terrible one, but at least a bit of trouble lurks here.

A “Decision”

Here’s a plan. We’ll work toward a solution where hallways and doors can be identified as separate items, and where hallways “end” at the doors, rather than tracking across the room’s floor tiles. To that end, we’ll review how hallways are done, then work out a plan.

GameRunner connects room n to room n-1:

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

Rooms connect to other rooms, randomly starting horizontally or vertically:

function Room:connect(aRoom)
    if math.random(1,2) == 2 then
        self:hvCorridor(aRoom)
    else
        self:vhCorridor(aRoom)
    end
end

Rooms draw corridors by asking GameRunner to draw them:

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

GameRunner draws them

function GameRunner:horizontalCorridor(fromX, toX, y)
    fromX,toX = math.min(fromX,toX), math.max(fromX,toX)
    for x = fromX,toX do
        self:setTile(Tile:room(x,y, self))
    end
end

function GameRunner:verticalCorridor(fromY, toY, x)
    fromY,toY = math.min(fromY,toY), math.max(fromY,toY)
    for y = fromY, toY do
        self:setTile(Tile:room(x,y, self))
    end
end

It does seem that there’s some duplication there that we could remove, doesn’t it? Doing that might be more tricky than we’d like, and for now, we’re on a different mission, namely working out how to avoid marking existing room tiles.

What if, instead of calling setTile there at the bottom, we had a method, oh, ensureRoomTile(x,y) that will set non-room tiles to room and leave room tiles alone. That should do the job for the current issue of placing things, and give us a place to stand for other changes.

I decide to call it setHallwayTile, which is a more meaningful name:

function GameRunner: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 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

function GameRunner:setHallwayTile(x,y)
    self:setTile(Tile:room(x,y,self))
end

This is a pure refactoring, with no behavioral change. Test. All good. Now let’s make it work. Should we commit first? Why not: refactor hallway creation down to new setHallwayTile.

Now then … if the tile is already a room, no need to reset it.

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

This should ensure that we no longer see things showing up in the princess’s starting room.

The placement works, but I am still seeing occasional graphical oddities, like seas of blueish tiles, and once in a while a view through a wall that shouldn’t be there. As soon as I move, the oddity disappears. I even see patches of map that shouldn’t be there:

glitch

map

The map issue is controlled by the seen flag in tiles. We need to be sure that that’s properly set when we do our new level:

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

function Tile:convertToEdge()
    self.kind = TileEdge
    self:initDetails()
end

function Tile:initDetails()
    self.contents = DeferredTable()
    self.seen = false
    self:clearVisible()
    self.tile = nil
end

That all seems to be in order. I wonder whether what’s happening has to do with the princess’s location. Exploring, I find Tiles drawing code:

function Tile:drawLargeSprites(center)
    -- we have to draw something: we don't clear background. Maybe we should.
    for i,sp in ipairs(self:getSprites(self:pos(), tiny)) do
        pushMatrix()
        pushStyle()
        translate(center.x,center.y)
        if not self.currentlyVisible then tint(0) end
        sp:draw()
        popStyle()
        popMatrix()
    end
end

function Tile:getSprites(pos, tiny)
    if not self.tile then
        if self:isRoom() then
            self.tile = self:getRandomFloorSprite()
        else
            self.tile = TileSprites[self.kind]
        end
    end
    local result = {self.tile}
    return result
end

This is a bit odd, in that we only have one sprite per tile, and yet we are acting as if there will be a collection. I think we used to draw contents at this point, and we no longer do that.

Let’s make this code simpler while we’re here.

function Tile:getSprite(pos, tiny)
    if not self.tile then
        if self:isRoom() then
            self.tile = self:getRandomFloorSprite()
        else
            self.tile = TileSprites[self.kind]
        end
    end
    return self.tile
end

This is a straightforward lazy init now, returning one tile. Accordingly:

function Tile:drawLargeSprites(center)
    -- we have to draw something: we don't clear background. Maybe we should.
    local sp = self:getSprite(self:pos(), tiny)
    pushMatrix()
    pushStyle()
    translate(center.x,center.y)
    if not self.currentlyVisible then tint(0) end
    sp:draw()
    popStyle()
    popMatrix()
end

I’ll check and see if anyone else calls getSprites. No. Now this method is plural and needn’t be, so let’s rename.

function Tile:drawSprites(tiny)
    local center = self:graphicCenter()
    if tiny then
        self:drawMapCell(center)
    else
        self:drawLargeSprite(center)
    end
end

OK, that’s all well and good but isn’t going to solve the problem. Let’s test and commit though: refactor tile sprites to only expect one sprite.

(I noticed that I hadn’t committed the new floor-preserving hallways, so committed that separately.)

Again, I look at the map drawing:

function Tile:draw(tiny)
    pushMatrix()
    pushStyle()
    spriteMode(CENTER)
    self:drawSprites(tiny)
    if not tiny and self.currentlyVisible then self:drawContents(tiny) end
    popStyle()
    popMatrix()
end

function Tile:drawSprites(tiny)
    local center = self:graphicCenter()
    if tiny then
        self:drawMapCell(center)
    else
        self:drawLargeSprite(center)
    end
end

function Tile:drawMapCell(center)
    if self.kind ~= TileRoom or not self.seen then return end
    pushMatrix()
    pushStyle()
    rectMode(CENTER)
    translate(center.x, center.y)
    fill(255)
    rect(0,0,TileSize,TileSize)
    popStyle()
    popMatrix()
end

When tiny is true, we call drawMapCell and clearly we don’t draw anything but rooms that are seen. Where is seen set? This is the only place where it’s set true:

function Tile:setVisible()
    self.currentlyVisible = true
    self.seen = true
end

This is called here:

function Tile:illuminateLine(dx,dy)
    local max = 8
    local pts = Bresenham:drawLine(0,0,dx,dy)
    for i,offset in ipairs(pts) do
        local pos = self:pos() + offset
        local d = self:pos():dist(pos)
        if d > max then break end
        local tile = self.runner:getTile(pos)
        tile:setVisible(d)
        if tile.kind == TileWall then break end
    end
end

Called here:

function Tile:illuminate()
    local count = 25
    for x = 0,count do
        self:illuminateLine(x,count)
        self:illuminateLine(x,-count)
        self:illuminateLine(-x,count)
        self:illuminateLine(-x,-count)
    end
    for y = 0,count-1 do
        self:illuminateLine(-count,y)
        self:illuminateLine(count,y)
        self:illuminateLine(-count,-y)
        self:illuminateLine(count,-y)
    end
end

Called here:

function Player:init(tile, runner)
    self.alive = true
    self.tile = tile
    self.tile:illuminate()
    self.tile:addDeferredContents(self)
    self.runner = runner
    self.keys = 0
    self.healthPoints = 12
    self.speedPoints = 8
    self.strengthPoints = 10
    self.attributeSheet = AttributeSheet(self,750)
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
    self.accumulatedDamage = 0
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

function Player:moveBy(aStep)
    self.runner:clearTiles()
    self.tile = self.tile:legalNeighbor(self,aStep)
    self.tile:illuminate()
end

We’re currently creating a new princess, which we shouldn’t do, so let’s explore that code:

function GameRunner:createLevel(count)
    TileLock=false
    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

function GameRunner:placePlayerInRoom1()
    local r1 = self.rooms[1]
    local rcx,rcy = r1:center()
    local tile = self:getTile(vec2(rcx,rcy))
    self.player = Player(tile,self)
end

I don’t like the answer that’s rising to the top of my head. No, not “Situation hazy, ask again later”. I’m thinking that this may be a timing problem, with the game drawing and deciding what’s visible, while we’re busily revamping the dungeon. This is consistent with what we see, in that once we move the player, the tiles that are mistakenly visible become invisible. However, the parts of the map that are mistakenly illuminated do not become invisible.

One thing we must do is keep the same princess on a new level, else she loses her current status. So we must at least deal with that. If there isn’t a current player, it’s the first level, and we’ll create her. If there is one, we need to move her to the desired tile. And she should then illuminate.

Currently we set her tile here:

function Player:moveBy(aStep)
    self.runner:clearTiles()
    self.tile = self.tile:legalNeighbor(self,aStep)
    self.tile:illuminate()
end

Let’s refactor:

function Player:moveBy(aStep)
    self:moveTo(self.tile:legalNeighbor(self,aStep))
end

function Player:moveTo(aTile)
    self.runner:clearTiles()
    self.tile = aTile
    self.tile:illuminate()
end

Test … and something odd happens. Now, when I step into the WayDown, the screen is all black until I move. I think this may be OK. I’ll continue the changes to placePlayerInRoom:

function GameRunner:placePlayerInRoom1()
    local r1 = self.rooms[1]
    local rcx,rcy = r1:center()
    local tile = self:getTile(vec2(rcx,rcy))
    if self.player then
        self.player:moveTo(tile)
    else
        self.player = Player(tile,self)
    end
end

This does not work at all. Once I enter a WayDown, the screen draws incorrectly and the player doesn’t move. Do I need to update contents? Yes. The entity is part of the room’s contents and in the legal move nest we do this:

function Tile:moveEntrant(anEntity, newTile)
    self:removeContents(anEntity)
    newTile:addDeferredContents(anEntity)
end

When we create the princess, we add her to the contents, but now we’re not. Let’s do that …

function GameRunner:placePlayerInRoom1()
    local r1 = self.rooms[1]
    local rcx,rcy = r1:center()
    local tile = self:getTile(vec2(rcx,rcy))
    if self.player then
        tile:addDeferredContents(self.player)
        self.player:moveTo(tile)
    else
        self.player = Player(tile,self)
    end
end

Now I can move but I can’t see much until I move. We need to illuminate again before she can see, but we are illuminating in the moveTo. What’s up with that?

I’m getting that “you’d better revert” feeling pretty solidly now.

A bit more testing convinces me, as even with the contents addition, I’m finding myself out in space and unable to move most of the time.

OK, before I do any more damage, let’s revert.

Done. Now a break. It’s 1040 and I’ve been at this since 0840, except for a brief zoom call of maybe 10 or 15 minutes. I’ll make a chai.

Now Then

What’s going on here? I want to argue that, in essence, we’re hacking. We know that when we create our first level, things work as intended. Now it’s possible that the current scheme of the game is too intricate, but it works just fine on the first level.

What I’ve been trying to do is to create another level, but not a whole new game arena. Instead, I’ve been trying to go through the game and set everything back to its initial state. That’s just about guaranteed trouble. We generally create objects in the right state and mostly change only the bits that change as the game goes on. Setting the object “back” isn’t something that was contemplated.

So let’s go with the flow. Let’s modify GameRunner to create the level, in its entirety, after GameRunner:init, instead of partially in init and partially elsewhere. We’ll still need to deal with retaining the princess’s properties but everything else can be brand new without a problem. Why do I say that? Because everything works without a problem at the beginning.

So let’s see GameRunner:init:

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.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:setTile(tile)
        end
    end
    self.cofloater = Floater(self, 50,25,4)
    self.musicPlayer = MonsterPlayer(self)
end

I argue that the first three lines there are suitable to init and then the tile creation isn’t, and that the floater setup and music player setup are. So first:

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:createTiles()
    self.cofloater = Floater(self, 50,25,4)
    self.musicPlayer = MonsterPlayer(self)
end

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:setTile(tile)
        end
    end
end

This should work as before, and does. Commit: refactor createTiles out of GameRunner:init.

Now let’s remove the call from init and put it into createLevel. This probably breaks some tests, who are probably expecting to have some tiles. We’ll see.

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(self, 50,25,4)
    self.musicPlayer = MonsterPlayer(self)
end

function GameRunner:createLevel(count)
    self:createTiles()
    TileLock=false
    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

I’m hopeful about this but do expect some tests to fail.

No tests fail but I do need to clear the lock before trying to create:

function GameRunner:createLevel(count)
    TileLock=false
    self:createTiles()
    self:clearLevel()
    ...

I wonder if we still need that clearLevel. We’ll find out. Test.

tiles

When I enter the WayDown, I see the picture above. Still getting the strange graphical effect. It goes away as soon as I move. What does the WayDown do?

function WayDown:actionWith(aPlayer)
    self.runner:createLevel(12)
end

Nothing special. Now I am confused. What else is happening here that could explain this?

Ah, I have a thought. We trigger the rebuilding of the dungeon in WayDown:

function WayDown:actionWith(aPlayer)
    self.runner:createLevel(12)
end

That’s called during a move operation! There’s all that intricate multiple dispatch to sort our the classes:

function Player:startActionWithWayDown(aWayDown)
    aWayDown:actionWith(self)
end

    t[WayDown][Player] = {moveTo=TileArbiter.acceptMove, action=Player.startActionWithWayDown}

function TileArbiter:moveTo()
    local entry = self:tableEntry(self.resident,self.mover)
    local action = entry.action -- <---
    if action then action(self.mover,self.resident) end
    local result = entry.moveTo(self)
    return result
end

So after the action, we call the moveTo, which is acceptMove, which ultimately treks back through the legal move logic and moves the player. That will certainly have a shot at undoing some of the work we’re doing in createLevel.

We just can’t do that. Instead, we need to tell GameRunner that we want a new level, and let it do that when things are quiet. The way it is now, we create a level and then do some weird move thing with the player.

I wonder what would happen, though, if we set the WayDown moveTo to refuse the move. No, even that amounts to a move, just a move “back” to the tile you’re on.

We need to defer the rerolling of the dungeon.

I have no idea how to do that, but I’m happy nonetheless, because I understand what’s happening.

And the fix starts out clear:

function WayDown:actionWith(aPlayer)
    self.runner:createNewLevel()
end

Now we just have one tiny detail: implementing createNewLevel. What do we know? We know that this call occurs during the player’s move, and before her turnComplete occurs. That looks like this:

function GameRunner:playerTurnComplete()
    self.playerCanMove = false
    self:moveMonsters()
    self.playerCanMove = true
end

At the point of entry to this method, the player’s move is over and the monsters aren’t moving yet. I think we can safely reroll the dungeon at this point … still creating a new princess. We’ll deal with that later.

So …

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(self, 50,25,4)
    self.musicPlayer = MonsterPlayer(self)
    self.requestNewLevel = false
end

function GameRunner:createNewLevel()
    self.requestNewLevel = true
end

function GameRunner:playerTurnComplete()
    if self.requestNewLevel then
        self:createLevel(12)
        self.requestNewLevel = false
    else
        self.playerCanMove = false
        self:moveMonsters()
        self.playerCanMove = true
    end
end

We can’t safely drop into the monster move, I suspect, so we’ll just roll the new level and let the player move. Let’s see what happens.

Everything seems to be fine now, except that we’re rerolling the princess. Her init is pretty complicated:

function Player:init(tile, runner)
    self.alive = true
    self.tile = tile
    self.tile:illuminate()
    self.tile:addDeferredContents(self)
    self.runner = runner
    self.keys = 0
    self.healthPoints = 12
    self.speedPoints = 8
    self.strengthPoints = 10
    self.attributeSheet = AttributeSheet(self,750)
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
    self.accumulatedDamage = 0
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

One possibility is to clone the princess. But maybe we can do better than that. It seems that she has a lot of plain initializing, and just a few things that connect her into the dungeon.

But what if we were to have an optional third parameter to the creation, another princess? Then we could copy over whatever we want to save.

First I’d better commit: new levels working except that princess is rerolled.

Now let’s refactor init a bit.

function Player:init(tile, runner)
    self.alive = true
    self.tile = tile
    self.tile:illuminate()
    self.tile:addDeferredContents(self)
    self.runner = runner
    self:initAttributes()
    self.attributeSheet = AttributeSheet(self,750)
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

function Player:initAttributes()
    self.keys = 0
    self.healthPoints = 12
    self.speedPoints = 8
    self.strengthPoints = 10
    self.accumulatedDamage = 0
end

The code needs more improvement:

function Player:init(tile, runner)
    self.alive = true
    self.runner = runner
    self:initTile(tile)
    self:initAttributes()
    self.attributeSheet = AttributeSheet(self,750)
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

function Player:initTile(tile)
    self.tile = tile
    self.tile:illuminate()
    self.tile:addDeferredContents(self)
end

I think we can safely move the attribute sheet and alive flag inside the initAttributes function …

function Player:init(tile, runner)
    self.runner = runner
    self:initTile(tile)
    self:initAttributes()
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

function Player:initAttributes()
    self.alive = true
    self.keys = 0
    self.healthPoints = 12
    self.speedPoints = 8
    self.strengthPoints = 10
    self.accumulatedDamage = 0
    self.attributeSheet = AttributeSheet(self,750)
end

And of course:

function Player:init(tile, runner)
    self.runner = runner
    self:initTile(tile)
    self:initAttributes()
    self:initSounds()
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

function Player:initSounds()
    self.deathSound = asset.downloaded.A_Hero_s_Quest.Hurt_3
    self.hurtSound = asset.downloaded.A_Hero_s_Quest.Hurt_1
    self.pitch = 1.5
end

And, because we’re fanatics:

function Player:init(tile, runner)
    self.runner = runner
    self:initTile(tile)
    self:initAttributes()
    self:initSounds()
    self:initVerbs()
end

function Player:initVerbs()
    self.attackVerbs = {"whacks", "bashes", "stabs", "slaps", "punches", "slams", "strikes"}
end

We’re not fanatical enough to factor out the setting of self.runner.

I think this is all refactoring and that we’re good to go. Testing … all is well. Commit: refactoring player:init.

Now our mission can be seen as copying the old princess’s attributes into the new princess, here:

function GameRunner:placePlayerInRoom1()
    local r1 = self.rooms[1]
    local rcx,rcy = r1:center()
    local tile = self:getTile(vec2(rcx,rcy))
    self.player = Player(tile,self)
end

Let’s have a class method for this purpose:

function GameRunner:placePlayerInRoom1()
    local r1 = self.rooms[1]
    local rcx,rcy = r1:center()
    local tile = self:getTile(vec2(rcx,rcy))
    self.player = Player:cloneFrom(self.player, tile,self)
end

We’ll let the cloneFrom deal with there being no one to clone from.

function Player:cloneFrom(oldPlayer, tile, runner)
    local player = Player(tile,runner)
    if oldPlayer then player:copyAttributes(oldPlayer) end
    return player
end

And this is a bit tricky but if I put it beside the initAttributes I have a chance …

-- N.B. initAttributes and copyAttributes need to match
--      on all cloned attributes

function Player:initAttributes()
    self.alive = true
    self.attributeSheet = AttributeSheet(self,750)
    self.accumulatedDamage = 0
    -- cloned attributes
    self.keys = 0
    self.healthPoints = 12
    self.speedPoints = 8
    self.strengthPoints = 10
end

function Player:copyAttributes(oldPlayer)
    self.keys = oldPlayer.keys
    self.healthPoints = oldPlayer.healthPoints
    self.speedPoints = oldPlayer.speedPoints
    self.strengthPoints = oldPlayer.strengthPoints
end

I don’t love that. Let’s see what we can do to improve it but first we have to make it work. And it works as advertised, now we retain our attributes as we go down to a new level.

But this code is pretty fragile: these two methods have to match. There is, however, some serious duplication here that we can work on.

Duplication? You ask? Yes. Look at all those lines going

    self.x = oldPlayer.x

And all those other matching ones going

    self.x = someConstant

You’re going to have to hold my beer on this one. But first commit: cloned princess retains attributes between levels.

Now then:

function Player:clonedAttributes()
    return { keys=0, healthPoints = 12, speedPoints = 8, strengthPoints = 10 }
end

function Player:initAttributes(attrs)
    self.alive = true
    self.attributeSheet = AttributeSheet(self,750)
    self.accumulatedDamage = 0
    -- cloned attributes
    self.keys = attrs.keys
    self.healthPoints = attrs.healthPoints
    self.speedPoints = attrs.speedPoints
    self.strengthPoints = attrs.strengthPoints
end

Now we have to init that way from here:

function Player:init(tile, runner)
    self.runner = runner
    self:initTile(tile)
    self:initAttributes(self:clonedAttributes())
    self:initSounds()
    self:initVerbs()
end

And we should be working as before. And we are. Now we can change this:

function Player:cloneFrom(oldPlayer, tile, runner)
    local player = Player(tile,runner)
    if oldPlayer then player:copyAttributes(oldPlayer) end
    return player
end

To this:

function Player:cloneFrom(oldPlayer, tile, runner)
    local player = Player(tile,runner)
    if oldPlayer then player:initAttributes(oldPlayer) end
    return player
end

And delete the copyAttributes function. It’s all good. And now, this is why you’re holding my beer:

function Player:initAttributes(attrs)
    self.alive = true
    self.attributeSheet = AttributeSheet(self,750)
    self.accumulatedDamage = 0
    -- cloned attributes
    for k,v in pairs(self:clonedAttributes) do
        self[k] = attrs[k]
    end
end

Now, whenever we create a new player attribute that needs to be retained, we’ll just add it and its default to the table. And I just invented a better name:

function Player:retainedAttributes()
    return { keys=0, healthPoints = 12, speedPoints = 8, strengthPoints = 10 }
end

function Player:initAttributes(attrs)
    self.alive = true
    self.attributeSheet = AttributeSheet(self,750)
    self.accumulatedDamage = 0
    for k,v in pairs(self:retainedAttributes()) do
        self[k] = attrs[k]
    end
end

And do you think we should extract that for loop? I do:

function Player:initAttributes(attrs)
    self.alive = true
    self.attributeSheet = AttributeSheet(self,750)
    self.accumulatedDamage = 0
    self:initRetainedAttributes(attrs)
end

function Player:initRetainedAttributes(attrs)
    for k,v in pairs(self:retainedAttributes()) do
        self[k] = attrs[k]
    end
end

Now you can give me my beer back.

What Just Happened?

We just made an interesting refactoring.

We had two methods that duplicated a sequence of setters. One of the methods had literal values defined in, the other copied them from another instance. We created a default “instance” consisting of a table of key-value pairs. We passed that table into the setter method, which we changed to copy the values from its parameter.

Since an object is just a table, if we passed in our old to-be-cloned object, the same method worked for that. We removed our copy method and used the plain init method.

Then we converted the sequence of calls to a loop over the default table, using it as a definition of all the copiable variables. Henceforth, when we add an attribute to the player, we need only add a default entry to that table.

This seems to me to be a good thing.

Then we pulled that loop out as a separate method, as one does.

I am confident enough to declare the following commit: New levels correctly created when player steps on WayDown tile.

Let’s Sum Up

We set out to make new levels work, graphically and correctly copying the princess into the new level. As part of that effort, we changed the hallway-carving code to avoid changing tiles that are set to type room, which allows all room tiles to have their room number.

This allows us, in turn, to avoid putting treasures or monsters too close to the player. (They can still show up in view, sometimes, since rooms and halls can overlap. This will generally manifest as a double-width hallway, or a hallway that is right along the edge of a room.)

We may look into a more sophisticated allocation of objects across the level, and we certainly want to restrict certain monsters, and probably certain other objects, to specific levels of the dungeon. That is for another day.

For today, we’ve got a playable multi-level dungeon going on, and that’s good.

However, it wasn’t as easy as it might have been. No, that’s not fair to say. Once I went about it the right way, today’s final way, it was pretty straightforward. I don’t see much bending over backward going on here. It’s true that we had to defer the dungeon rerolling until things calmed down, but that makes sense even if it didn’t come right to my mind. The solution of rerolling right in the middle of a move was doomed never to work.

But I do feel that there’s some design improvement waiting to be done. The GameRunner is part GameMaker and part GameRunner, and also part GameCommunicator. That’s a bit much.

Player and Monster are both over 200 lines of code, which makes me suspect that they may not be as cohesive as they could be. And refactoring Player’s init method makes me feel certain that other inits are going to be pretty messy as well.

So there’s cleanup to do. I think we’ll try to bear down on that just a bit, as we put in new features. That will help ensure that they go in correctly, and that we leave the campground better than we found it.

Good job today! Thanks for your help, I couldn’t have done it without you. See you next time!

D2.zip