Dungeon 42: The Answer?
Probably not the answer but maybe some small ones.
There’s an issue with the handling of chests. Maybe even more than one issue. Entities in the game interact with others by moving onto the same tile. Presently, if I’m not mistaken, all such moves are allowed. This isn’t ideal. Chests are a good example. A preferable flow should be something like this:
- Move onto a closed chest without a key: move refused;
- Move onto a closed chest with a key: move accepted, chest opens;
- Move onto an open chest: move accepted, receive treasure.
In the latter case, we could refuse the move just as well. We could make it be that you can’t step onto a chest, ever. However, right now a chest can be placed in a hallway, and if it’s a narrow hallway, the hallway is blocked. That’s often harmless but sometimes it would block the player from completing the game.
So I think we have two things to do:
First, require that chests and other immovable objects cannot block hallways or doorways. Second, adjust the interaction logic so that a move onto an occupied tile can be accepted or refused based on what’s in there.
Along the way, now that I think of it: Third, arrange not to put down two things onto one tile. Once a tile has contents, let’s not put anything else into it. (We might later decide to permit certain combinations, or even to require them. For now, one thing only.)
Let’s do that first. It should be easy, so it’l be a good warmup.
One Thing per Tile
Should we TDD this? You know I don’t want to. I claim we don’t need to. Here’s thing creation:
function GameRunner:createChests(n)
self.chests = {}
for i = 1,n or 1 do
local tile = self:randomRoomTile()
table.insert(self.chests, Chest(tile,self))
end
end
function GameRunner:createKeys(n)
self.keys = {}
for i = 1,n or 1 do
local tile = self:randomRoomTile()
table.insert(self.keys, Key(tile,self))
end
end
function GameRunner:createMonsters(n)
self.monsters = {}
for i = 1,n or 1 do
local tile = self:randomRoomTile()
table.insert(self.monsters, Monster(tile,self))
end
end
I am now almost certain that I see some duplication here. I think we could perhaps do this:
function GameRunner:createMonsters(n)
self.monsters = self:createThings(Monster,n)
end
function GameRunner:createThings(aClass, n)
local things = {}
for i = 1,n or 1 do
local tile = self:randomRoomTile()
table.insert(things, aClass(tile,self))
end
return things
end
That works. We just pass in the class, and pass the local table back out. Now we can replace this:
function GameRunner:createLevel(count)
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
local r1 = self.rooms[1]
local rcx,rcy = r1:center()
local tile = self:getTile(vec2(rcx,rcy))
self.player = Player(tile,self)
self:createMonsters(5)
self:createKeys(10)
self:createChests(10)
end
With this:
function GameRunner:createLevel(count)
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
local r1 = self.rooms[1]
local rcx,rcy = r1:center()
local tile = self:getTile(vec2(rcx,rcy))
self.player = Player(tile,self)
self.monsters = self:createThings(Monster,5)
self.keys = self:createThings(Key,10)
self:createThings(Chest,10)
end
And delete those three specialized create methods, leaving only createThings
. Commit: refactor and combine creation of things in rooms.
Now about that method randomRoomTile
. It’s just this:
function GameRunner:randomRoomTile()
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile.kind == "room" then return tile end
end
end
This can select a tile in a hallway, because hallway tiles are of type room. We could change that, but it wouldn’t solve my concern, which is that a thing could block the only exit from or entrance into a room.
A harsh requirement would be to require that all 8 cells surrounding the one we pick here should be of type floor and have no contents. (We do want to add the “no contents” rule real soon now anyway.)
We do have a collection rooms
in GameRunner, though we don’t use it. We could use it here. However, I’ve kind of made a design decision to do everything at the level of Tiles. For now, let’s require that randomRoomTile
only finds room tiles completely surrounded by room tiles. How hard could it be?
I’m still resisting TDDing this. I hate trying to set up the geometry for this kind of test. So far, I’ve gotten away with it. We should talk about whether that’s just luck, or what.
Let’s extend randomRoomTile
:
function GameRunner:randomRoomTile()
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile.kind == "room" and tile:isInOpen() then return tile end
end
end
We’ll ask the tile whether it is in the open.
function GameRunner:randomRoomTile()
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile.kind == "room" and tile:isInOpen() then return tile end
end
end
Before I get far, I realize I should ask the tile the whole question.
function GameRunner:randomRoomTile()
while true do
local pos = vec2(math.random(1, self.tileCountX), math.random(1,self.tileCountY))
local tile = self:getTile(pos)
if tile:isOpenRoom() then return tile end
end
end
function Tile:isOpenRoom()
local r
for x = -1,1 do
for y = -1,1 do
r = self:getNeighbor(vec2(x,y))
if r.kind ~= TileRoom then return false end
end
end
return true
end
I really expect this to work. If not, we’ll have a little discussion about tests.
In my first test, I found a room with five chests in it. I suppose that a larger room has a higher chance of being picked as a place for things to be put down. That’s an interesting effect that I certainly didn’t consider.
Nor, I think, would TDD have caused me to consider it. And it does seem to be working as intended, or at least as written.
And we are placing huge numbers of chests and keys, just to make them easy to find.
Still, the tendency to place things in large rooms is concerning. First, though, I’d better commit: things placed only in the open.
Thinking informally, the number of tiles in a room increases with the square of its wall lengths. so a room 2x as large has 4x the number of tiles and a correspondingly large chance of receiving a thing. That bears thinking about. Possibly we should take advantage of the room structure and place things with equal probability or even in small rooms first.
I don’t have a purpose for the room structure at present. I’ve kept it because there wasn’t much sense throwing it away, although in principle we could. It just seems wrong to completely throw out a concept that we use early on, given that the coast of keeping it is low. And since there are presently only 12 rooms per level, it is low indeed.
We’ll save the concern for later, in any case. Now I’d like to provide for things encountered on squares to refuse entry to something trying to come in. (The entrant will only be a player or a monster. Nothing else moves.) Let’s look at the legal move logic. Player does this:
function Player:moveBy(aStep)
self.runner:clearTiles()
self.tile = self.tile:legalNeighbor(self,aStep)
self.tile:illuminate()
end
Monsters, depending on the kind of move they’re making, do one of these things:
function Monster:moveTowardAvatar()
local dxdy = self.runner:playerDirection(self.tile)
if math.random() < 0.5 then
self.tile = self.tile:legalNeighbor(self,vec2(0,dxdy.y))
else
self.tile = self.tile:legalNeighbor(self,vec2(dxdy.x,0))
end
end
function Monster:makeRandomMove()
local moves = {vec2(-1,0), vec2(0,1), vec2(0,-1), vec2(1,0)}
local move = moves[math.random(1,4)]
self.tile = self.tile:legalNeighbor(self,move)
end
It all comes down to legalNeighbor
:
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
if newTile:isRoom() then
self:removeContents(anEntity)
newTile:interactionFrom(anEntity)
newTile:addContents(anEntity)
return newTile
else
return self
end
end
This function can return the proposed new tile (current tile plus a step), or the current tile, itself. What we should perhaps do is to have the interaction return a boolean indicating whether the move is permitted or not. This would allow for a number of interesting possibilities, including not stepping on chests.
I am still torn on TDD. I think here, I’d better try it, not so much because I think I can’t code this without it, but because I want to reinforce the habit, which is a good one. It’s like exercise, I suppose. Some of us have to force ourselves to start, even though we know it’s a good thing.
Quit nagging me about exercise. Let’s see about player:chest interactions.
_:test("player steps on chest", function()
end)
There, are you happy now? Oh, you want more. This is how it starts. Let’s see what we want the interaction to be. I think it’s likely to change how the legal...
function is coded.
Chests don’t know whether they are open or closed. Right now, they interact like this:
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
if newTile:isRoom() then
self:removeContents(anEntity)
newTile:interactionFrom(anEntity)
newTile:addContents(anEntity)
return newTile
else
return self
end
end
function Tile:interactionFrom(anEntity)
for k,entity in pairs(self.contents) do
entity:interact(anEntity)
end
end
Let me rename the variables here for clarity before we move on:
function Tile:interactionFrom(enteringEntity)
for k,residentEntity in pairs(self.contents) do
residentEntity:interact(enteringEntity)
end
end
That should help us remember who starts the interaction. Commit: rename vars in Tile:interactionFrom.
In the case in hand, the chest receives interact
with the player. It doesn’t know what’s interacting yet, so it sends:
function Chest:interact(anEntity)
anEntity:startActionWithChest(self)
end
Player receives that message:
function Player:startActionWithChest(aChest)
aChest:open()
end
And opens the chest. We’re OK with that, but we want the chest to refuse the inbound step.
The rule has to be that the resident gets to decide whether the entrant gets in. So we want that entrance loop to record the replies from interact
, which need to be revised to return something indicating whether the move is allowed.
It’s irritating, however, to have to change all of them, but I see no better choice.
So we want a tile with a chest, another tile with a player, and we want to request entry from the player.
I still don’t like this method’s structure:
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
if newTile:isRoom() then
self:removeContents(anEntity)
newTile:interactionFrom(anEntity)
newTile:addContents(anEntity)
return newTile
else
return self
end
end
It requires me to do the step thing. I’d like to just create a couple of tiles and check them. Let’s pull out the insides of this method. They should be separated anyway.
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
return self:validateMoveTo(newTile)
end
function Tile:validateMoveTo(newTile)
if newTile:isRoom() then
self:removeContents(anEntity)
newTile:interactionFrom(anEntity)
newTile:addContents(anEntity)
return newTile
else
return self
end
end
Now the validate
method doesn’t care where the inbound entrant is from. But let’s go further.
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
return self:validateMoveTo(anEntity,newTile)
end
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
end
function Tile:moveEntrant(anEntity, newTile)
self:removeContents(anEntity)
newTile:interactionFrom(anEntity)
newTile:addContents(anEntity)
end
Now I’ve broken out the nitty-gritty of moving the entity from the logic of whether to do it. But I want to do the interaction first. So we’ll move that up:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
newTile:interactionFrom(anEntity)
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
end
function Tile:moveEntrant(anEntity, newTile)
self:removeContents(anEntity)
newTile:addContents(anEntity)
end
And now we’re where we want to be to TDD this thing.
Here’s what I’ve got so far:
_:test("player steps on chest", function()
local runner = GameRunner()
local chestTile = Tile:room(10,10,runner)
local chest = Chest(chestTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
end)
I’ve created two adjacent cells, one with a Chest and one with a Player. I am pretty sure I won’t need the game runner, but I provided it just in case. We’ll see.
Now the player wants to try and fail to enter the chest tile from its current (player) tile, so we do:
_:test("player steps on chest", function()
local runner = GameRunner()
local chestTile = Tile:room(10,10,runner)
local chest = Chest(chestTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
local chosenTile = playerTile:validateMoveTo(player, chestTile)
_:expect(chosenTile).is(playerTile)
end)
This is hard to think about. double-dispatch often is. Let’s run this, expecting a fail.
12: player steps on chest -- Actual: Tile[10][10]: room, Expected: Tile[11][10]: room
As expected, the test accepted the move when it should not have. Let’s add open checks for fun:
_:test("player steps on chest", function()
local runner = GameRunner()
local chestTile = Tile:room(10,10,runner)
local chest = Chest(chestTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
_:expect(chest:isOpen()).is(false)
local chosenTile = playerTile:validateMoveTo(player, chestTile)
_:expect(chosenTile).is(playerTile)
_:expect(chest:isOpen()).is(true)
end)
12: player steps on chest -- Tests:176: attempt to call a nil value (method 'isOpen')
function Chest:isOpen()
return self.pic == self.openPic
end
The open checks work, as expected, and the move was accepted, which we don’t wish for.
Sunday Becomes Monday
Up to this point, the preceding was written on Sunday. It’s now Monday, 0900, and I’m back. I may need retraining, as it has been nearly 24 hours since I last worked on this.
The good news is, we have a failing test.
_:test("player steps on chest", function()
local runner = GameRunner()
local chestTile = Tile:room(10,10,runner)
local chest = Chest(chestTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
_:expect(chest:isOpen()).is(false)
local chosenTile = playerTile:validateMoveTo(player, chestTile)
_:expect(chosenTile).is(playerTile)
_:expect(chest:isOpen()).is(true)
end)
Our player is trying to move onto the tile where the chest is. To review, that move begins when the player tries to step onto the tile containing the chest:
function Tile:legalNeighbor(anEntity, aStepVector)
local newTile = self:getNeighbor(aStepVector)
return self:validateMoveTo(anEntity,newTile)
end
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
newTile:interactionFrom(anEntity)
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
end
function Tile:moveEntrant(anEntity, newTile)
self:removeContents(anEntity)
newTile:addContents(anEntity)
end
The legalNeighbor
function fetches the tile to which the player is trying to move, and checks to see if the move is valid. Presently, if the tile is a room tile, the move is considered valid. But before that’s done, the new tile gets an opportunity to have all its residents interact with the would-be entrant:
function Tile:interactionFrom(enteringEntity)
for k,residentEntity in pairs(self.contents) do
residentEntity:interact(enteringEntity)
end
end
Each resident has its own way of handling interact
, but generally speaking, if they intend to interact, they will send a startInteractionWith[Type]
message to the entrant. Here’s what the chest does:
function Chest:interact(anEntity)
anEntity:startActionWithChest(self)
end
And the player does this:
function Player:startActionWithChest(aChest)
aChest:open()
end
This isn’t robust enough, we need to deal with the necessity to have a key, and the behavior of an open chest differs from that of a closed one. But for now, it’s what we do.
But the behavior of player vs monster is quite different.
function Player:startActionWithMonster(aMonster)
aMonster:die()
end
function Monster:startActionWithPlayer(aPlayer)
aPlayer:die()
end
If a player steps into a monster tile, the monster is surprised, and the player kills it. But if the monster steps into the player tile, the player is surprised, and the monster kills her.
Now we’re trying to make the chest refuse entry for the player, and probably for any entity, because we don’t want monsters sitting on the chests. Or do we? No, let’s make it be that monsters can step on the chest tile but the player cannot. We’re doing the player for now.
Now looking at the core code, we can begin to see what we want.
If the move is not to be allowed, this code must return self
rather than newTile
:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
newTile:interactionFrom(anEntity)
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
end
Therefore, newTile:interactionFrom
is going to have to return a boolean, or perhaps we could have it return the correct tile. Though I have been warned about booleans in OO code, I’m going with that for now:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
local canEnter = newTile:interactionFrom(anEntity)
if canEnter then
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
else
return self
end
end
This is nasty but I’m trying to get my test to run. Now we expect all the interactionFrom
implementations to return a boolean, but they’ll default to nil, so this will mean that no one can enter an occupied tile. Assuming that I’m not confused. I expect the test to run.
And it does. However, a side effect of this implementation is that no one can move, since all the interacts return nil, which is interpreted as false.
I’m torn here. I don’t want to have to modify all those methods (there are perhaps as many as three). But I don’t want to reverse the sense of that boolean, and I don’t want to pass the new and old tiles further down.
First, rename interactionFrom
, now that it has a new capability, to allowEntranceFrom
. That also gives us the opportunity to return the appropriate boolean, as well as …
No. Let’s push the decision down. That’s usually the right thing. This is a method on Tile, presently sent to the new tile, which is going to check its contents. So we’ll call the method … newTile:attemptedEntranceBy
:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
for k,residentEntity in pairs(self.contents) do
residentEntity:interact(enteringEntity)
end
return self
end
I wish that name connoted more about select room, but for now I want to get wired up.
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
return newTile:attemptedEntranceBy(anEntity, self)
else
return self
end
end
Well, that’s nicer, isn’t it? And for now, all entrances will be allowed. So our test should fail again.
12: player steps on chest -- Actual: Tile[10][10]: room, Expected: Tile[11][10]: room
Now we need to extend the attempted entrance method:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
for k,residentEntity in pairs(self.contents) do
residentEntity:interact(enteringEntity)
end
return self
end
Now here, I think we’ll try for a boolean, and the rule will be to return true if you wish to allow entry, false otherwise.
The overall rule is that if anyone wants to let you into the room, you can go in:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
for k,residentEntity in pairs(self.contents) do
acepted = accepted or residentEntity:interact(enteringEntity)
end
if accepted then
return self
else
return oldRoom
end
end
The result here surprises me in two ways. I rather expect that all the interact
methods return nil, and so I expected our test to fail, and instead I get another failure. This test is failing:
_:test("tile contents", function()
local runner = GameRunner()
local room = Room(10,10,10,10,runner)
local tile = runner:getTile(vec2(15,15))
local player = Player(tile,runner)
_:expect(tile.contents).has(player)
player:moveBy(vec2(1,0))
_:expect(tile.contents).hasnt(player)
local t2 = runner:getTile(vec2(16,15))
_:expect(t2.contents).has(player)
end)
Ah. The moveBy is failing, because so far, no one can move. So it is proper for the other test to succeed: no one can move. We need to make all the interact
methods return their boolean. And the name is bad, should be acceptEntrance
. Therefore:
function Monster:interact(anEntity)
anEntity:startActionWithMonster(self)
end
In the fullness of time, we may enhance this interaction but for now, monsters accept the move:
function Monster:acceptEntrance(anEntity)
anEntity:startActionWithMonster(self)
return true
end
function Player:acceptEntrance(anEntity)
anEntity:startActionWithPlayer(self)
return true
end
function Key:acceptEntrance(anEntity)
anEntity:startActionWithKey(self)
return true
end
And finally:
function Chest:acceptEntrance(anEntity)
anEntity:startActionWithChest(self)
return false
end
Now I expect the current test to run.
12: player steps on chest -- Tile:173: attempt to call a nil value (method 'interact')
Well, it would help if we used our new method name:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
for k,residentEntity in pairs(self.contents) do
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if accepted then
return self
else
return oldRoom
end
end
Nice thing about tests, even simple ones can remind me of things I’d forget.
The current test passes, but the other one fails …
10: tile contents -- Actual: table: 0x293d95380, Expected: Player (15,15)
The test is this:
_:test("tile contents", function()
local runner = GameRunner()
local room = Room(10,10,10,10,runner)
local tile = runner:getTile(vec2(15,15))
local player = Player(tile,runner)
_:expect(tile.contents).has(player)
player:moveBy(vec2(1,0))
_:expect(tile.contents).hasnt(player)
local t2 = runner:getTile(vec2(16,15))
_:expect(t2.contents).has(player)
end)
Ooo, I wonder if we forgot to do contents swapping correctly. Let’s annotate that test:
_:test("tile contents", function()
local runner = GameRunner()
local room = Room(10,10,10,10,runner)
local tile = runner:getTile(vec2(15,15))
local player = Player(tile,runner)
_:expect(tile.contents, "started wrong").has(player)
player:moveBy(vec2(1,0))
_:expect(tile.contents, "didn't leave").hasnt(player)
local t2 = runner:getTile(vec2(16,15))
_:expect(t2.contents, "didn't arrive").has(player)
end)
10: tile contents didn't leave -- Actual: table: 0x287e02a00, Expected: Player (15,15)
10: tile contents didn't arrive -- Actual: table: 0x287e00100, Expected: Player (15,15)
Looks a lot like the move just wasn’t accepted, since the player’s tile hasn’t changed.
Oh … this code:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
for k,residentEntity in pairs(self.contents) do
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if accepted then
return self
else
return oldRoom
end
end
If there are no members in contents, we default to not allowing entry. But if nothing else is on the square, entry is legal.
Hm. I fear we have to count them.
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
local residents = 0
for k,residentEntity in pairs(self.contents) do
residents = residents + 1
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if residents == 0 or accepted then
return self
else
return oldRoom
end
end
That’s irritating but I expect it to work.
But there’s still a flaw, the same two methods. I really think I forgot to move the contents. Indeed:
function Player:moveBy(aStep)
self.runner:clearTiles()
self.tile = self.tile:legalNeighbor(self,aStep)
self.tile:illuminate()
end
That clearly doesn’t update tile contents. What about monsters? They don’t either. In three separate places they just set their tile.
This used to work. Where was that updating actually done?
function Tile:moveEntrant(anEntity, newTile)
self:removeContents(anEntity)
newTile:addContents(anEntity)
end
Where was that called? Ah, here:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
newTile:interactionFrom(anEntity)
self:moveEntrant(anEntity, newTile)
return newTile
else
return self
end
end
Now we have:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
return newTile:attemptedEntranceBy(anEntity, self)
else
return self
end
end
Left that out. Now the attemptedEntranceBy
can return either room. We don’t mind the cost of unconditionally removing and adding, so …
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
local tile = newTile:attemptedEntranceBy(anEntity, self)
self:moveEntrant(anEntity,tile)
return tile
else
return self
end
end
Now I expect to be back in order.
Tests pass. Furthermore, the chest opens when the princess tries to step on it, but it doesn’t let her in. Perfect.
I’ll see if a monster can step in. Conveniently, it cannot. This means I can hide from a monster by going on the other side of a chest from it.
But I had decided monsters could step on chests. That must not be done yet. But first commit: player cannot step on chest.
Now what about monsters stepping on chests?
function Chest:acceptEntrance(anEntity)
anEntity:startActionWithChest(self)
return false
end
We do not know whether anEntity
is a player or monster. We do know that we may get a callback from our startAction
message, but not always:
function Player:startActionWithChest(aChest)
aChest:open()
end
function Monster:startActionWithChest(aChest)
end
So if we get an open call, we know we should refuse entry and otherwise not. That’s kind of weird, though. We could set a member variable here, and clear it in open
, but that’s obscure and gets more strange when there get to be more interactions.
I’d really like to know here and now whether this entity is the player. It would be “easy” to implement areYouPlayer
on all the entities, but that seems weird. But I only have two options that I can think of: wait for a callback, or ask a question. the “tell don’t ask” dictum suggests waiting.
Let’s try it. It’s sure to work and it might be OK. If it turns out to be nasty, we’ll fix it.
function Chest:acceptEntrance(anEntity)
self.entrantIsAllowed = true
anEntity:startActionWithChest(self)
return self.entrantIsAllowed
end
function Chest:isOpen()
self.entrantIsAllowed = false
return self.pic == self.openPic
end
Now I expect monsters to step over chests just fine. Curiously, they do not.
Here are two ghosts lined up, unable to cross the chest. They also seem unable to step into each others square, where they used to be able to do that and then one of them would rest.
Can they step onto the princess’s square? They cannot, although at one point the princess got killed somehow.
We need a test.
_:test("monster steps on player", function()
local runner = GameRunner()
local monsterTile = Tile:room(10,10,runner)
local monster = Monster(monsterTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
local chosenTile = monsterTile:validateMoveTo(player,playerTile)
_:expect(chosenTile).is(playerTile)
end)
13: monster steps on player -- Player:64: attempt to call a nil value (method 'startActionWithPlayer')
function Player:acceptEntrance(anEntity)\
anEntity:startActionWithPlayer(self)
return true
end
The player is sending startActionWithPlayer to itself. Have I called the validate wrongly? Yes. The validate method wants the mover and the proposed tile:
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
local tile = newTile:attemptedEntranceBy(anEntity, self)
self:moveEntrant(anEntity,tile)
return tile
else
return self
end
end
Let’s rename the parameter to make that more clear:
function Tile:validateMoveTo(movingEntity, newTile)
if newTile:isRoom() then
local tile = newTile:attemptedEntranceBy(movingEntity, self)
self:moveEntrant(movingEntity,tile)
return tile
else
return self
end
end
Now our test:
_:test("monster steps on player", function()
local runner = GameRunner()
local monsterTile = Tile:room(10,10,runner)
local monster = Monster(monsterTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
local chosenTile = monsterTile:validateMoveTo(monster,playerTile)
_:expect(chosenTile).is(playerTile)
end)
I do expect a failure, of course, and I get it:
13: monster steps on player -- Actual: Tile[10][10]: room, Expected: Tile[11][10]: room
But I also get a crash:
GameRunner:165: attempt to index a nil value (field 'player')
stack traceback:
GameRunner:165: in method 'playerDistance'
Monster:30: in field 'callback'
...in pairs(tweens) do
c = c + 1
end
...
I think creating that GameRunner in the test has borked something.
I’ve broken something, and I’m not sure what. Time to revert, and perhaps do something to clear my head. At least, revert.
Curiously, I immediately get the same error and crash. Ignoring the test, the errors go away. So the test is flawed. Maybe I should have kept the rest. But no, When confused, revert, do again, fail again, fail better.
Revert
I’m very tempted to look at my recent diffs and see what I did wrong. Let’s not. We did have a nice test, however, so let’s recover that from here in the article.
The test configuration probably needs deeper configuration. The crash is coming while running a tween that’s trying to move a monster. Once created, monsters set up their tween delays and try to move. To do that they check distance from the player. GameRunner doesn’t know the player in our test, and wants to. I think it’ll suffice to jam the player in.
_:test("monster steps on player", function()
local runner = GameRunner()
local monsterTile = Tile:room(10,10,runner)
local monster = Monster(monsterTile, runner)
local playerTile = Tile:room(11,10,runner)
local player = Player(playerTile,runner)
runner.player = player
local chosenTile = monsterTile:validateMoveTo(monster,playerTile)
_:expect(chosenTile).is(playerTile)
end)
Good. Test fails, no subsequent crash:
13: monster steps on player -- Actual: Tile[10][10]: room, Expected: Tile[11][10]: room
Now why can’t he step on us?
function Tile:validateMoveTo(anEntity, newTile)
if newTile:isRoom() then
local tile = newTile:attemptedEntranceBy(anEntity, self)
self:moveEntrant(anEntity,tile)
return tile
else
return self
end
end
We ask the playerTile about entrance for a monster
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
local residents = 0
for k,residentEntity in pairs(self.contents) do
residents = residents + 1
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if residents == 0 or accepted then
return self
else
return oldRoom
end
end
We find the player in contents and ask player to accept entrance from the monster:
function Player:acceptEntrance(anEntity)
anEntity:startActionWithPlayer(self)
return true
end
Player is trying to always accept entrance. I think we should be returning true
and accepting the move.
A bad feeling …
I’m getting a bad feeling about this. There are two reasons. One, it is now 11 AM, so I’ve been at this for two hours, and that’s about as long as I want to work in a patch. Two, I’m finding the back-and-forth logic here hard to think about, which makes me think that this double-dispatch callback model of interaction may be too fancy or too intricate.
But for now, let’s find out what’s up.
I really want to run this one test in isolation, so that I can add in some tracing without too much output.
I’ll comment the others out. Test conveniently continues to fail.
A print in Player:
function Player:acceptEntrance(anEntity)
print("Player:accept ", anEntity)
anEntity:startActionWithPlayer(self)
return true
end
Player:accept table: 0x28ff03cc0
Got there. But doesn’t Monster know how to print itself? It didn’t but now it does:
function Monster:__tostring()
return string.format("Monster (%d,%d)", self.tile:pos().x,self.tile:pos().y)
end
Player:accept Monster (10,10)
Now we know we returned true from here, we see the line of code. So something is wrong here:
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
local residents = 0
for k,residentEntity in pairs(self.contents) do
residents = residents + 1
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if residents == 0 or accepted then
return self
else
return oldRoom
end
end
Why is accepted not set to true here? I am confused.
hahahahaha … look at the spelling of accepted:
acepted = accepted or residentEntity:acceptEntrance(enteringEntity)
Lua happily created a new global variable named acepted, without mentioning it to anyone. Not its best feature.
function Tile:attemptedEntranceBy(enteringEntity, oldRoom)
local accepted = false
local residents = 0
for k,residentEntity in pairs(self.contents) do
residents = residents + 1
accepted = accepted or residentEntity:acceptEntrance(enteringEntity)
end
if residents == 0 or accepted then
return self
else
return oldRoom
end
end
And the test passes. Delicious. Uncomment the rest, do again.
They all run. Now to find a ghost and see if it can step over a chest and onto the princess.
It can’t step onto a chest. It can step onto the princess, but she doesn’t die.
We know this happened:
function Player:acceptEntrance(anEntity)
anEntity:startActionWithPlayer(self)
return true
end
That should trigger this:
function Monster:startActionWithPlayer(aPlayer)
aPlayer:die()
end
And that should do this:
function Player:die()
self.alive = false
end
And that should tint her black. Who’s not getting called, and why?
I put a print in Player:die
, and got this:
die Player (11,56)
Yet she’s not displaying in black.
Oh:
function Player:draw(tiny)
local dx = -2
local dy = -3
pushMatrix()
pushStyle()
spriteMode(CORNER)
local center = self:graphicCorner()
if not self.alive then tint(0) end
if tiny then
tint(255,0,0)
sx,sy = 180,272
else
tint(255)
sx,sy = 80,136
end
sprite(asset.builtin.Planet_Cute.Character_Princess_Girl,center.x+dx,center.y+dy, sx,sy)
popStyle()
popMatrix()
end
I’ve patched the code to display her in white always. Fix that:
function Player:draw(tiny)
local dx = -2
local dy = -3
pushMatrix()
pushStyle()
spriteMode(CORNER)
local center = self:graphicCorner()
if not self.alive then tint(0) end
if tiny then
tint(255,0,0)
sx,sy = 180,272
else
sx,sy = 80,136
end
sprite(asset.builtin.Planet_Cute.Character_Princess_Girl,center.x+dx,center.y+dy, sx,sy)
popStyle()
popMatrix()
end
Now she turns black, the color of regal death. So sad. Remove my prints. Commit? Not yet, monsters still can’t cross chests. Unless I decide to accept that. It’s kind of nice to be able to hide. Let’s make sure it’s working properly and intentionally and leave it that way. We need a test to verify and document the behavior.
_:test("monster can't enter chest tile", function()
local runner = GameRunner()
local playerTile = Tile:room(15,15,runner)
local player = Player(playerTile,runner)
runner.player = player -- needed because of monster decisions
local chestTile = Tile:room(10,10,runner)
local chest = Chest(chestTile, runner)
local monsterTile = Tile:room(11,10,runner)
local monster = Monster(monsterTile,runner)
local chosenTile = monsterTile:validateMoveTo(monster, chestTile)
_:expect(chosenTile).is(monsterTile)
end)
This passes, as anticipated. I want to verify why, though. This is another sign that this code is too tricky to understand.
function Chest:acceptEntrance(anEntity)
anEntity:startActionWithChest(self)
return false
end
Chests don’t accept entrance, period. Before something else confused me, I was struggling with how to conditionally allow monsters onto them. For now, they can’t come in.
Let’s commit: tiles and their contained entities can conditionally allow or deny entry to another entity.
That commit message alone should tell us something about the complexity here, but some of it is inherent to what we’re trying to accomplish. Let’s discuss that as we …
Sum Up
Well. That seemed harder than it should have. I suspect not reverting would have helped, but I know what things not going well feels like, and that was it. Nonetheless, we do have, at this point, reasonable interactions going on between monsters, players, chests, and keys. There’s still more to be done, however, and while it may not get any harder, it won’t likely get any easier.
One way or another, we will need to specify the interactions between all these, plus any other monster types that may come along, and it surely won’t get any less complicated. There’s something difficult to think about going on here, and it’s just about impossible to memorize how it goes.
- When an entity tries to move to a new tile, it sends
legalNeighbor
to its own tile, specifying itself and a step (a small vector). legalNeighbor
fetches the new tile and sendsvalidateMoveTo
to itself, passing the entity and the new tile.validateMoveTo
checks to see whether the new tile is a room (which is bad), and then sends the new tileattemptedEntranceBy
, passing the entity and the old tile.attemptedEntranceBy
asks all its contained entities whether theyacceptEntrance
by the entering entity.- Every entity must implement
acceptEntrance
. This is their chance to start an interaction, so each one sendsstartActionWith[Type]
to the entering entity. - Each moving entity must be prepared to receive a
startActionWith[Type] message from any existing entity, mobile or not. It can ignore the message, or take action. Player tells chests to open. Monsters and players tell the other to die. Monsters tell other monsters to "rest", which idles them for a while. In principle, what happens after the
startAction` call should condition whether the proposed move that set this off should be accepted or not. - After the return from
startAction...
, the entity must return true or false, indicating whether or not it accepts the move. - Back in
attemptedEntranceBy
, f any of the contained entities does accept entrance, or there are none, the method returns the new room, otherwise the original room. - Back in
validateMoveTo
, we receive whichever room the entity is to wind up in, and we remove it from the old room’s contents and place it in the new room’s contents. Then we return whichever room it was. - The entity sets its tile to the returned value.
This doesn’t describe the details of anything like an actual battle, which might require a number of exchanges to determine an outcome. We could imagine some kind of monster that asks questions of the player, accepting inputs of some kind, or one that makes a complex determination based on where the player has been. It could be anything.
It seems to me that the above is obviously too much. One core reason is that I set out to do it with double dispatch. I had that trick in mind, and it has caused a proliferation of methods. But there is more going on than just that.
A small improvement might be to send validateMoveTo
to the new tile, which has a more legitimate interest in knowing whether it is a room tile or not (and we could even, one day, have different tile types, which might simplify some aspects. (And might make some things worse. At any rate, I don’t see that as desirable right now.)
Part of the complexity is that we switch our attention from room to room, and part is that we switch our attention from an entity in the room to the entity trying to get in. We have no place to stand that lets us consider all the elements at once.
It might be that this should be resolved into a communication between entities, but even that leaves us with the dispatch bouncing back and forth.
I’m coming to feel certain that this is not a good situation, and that we need something better. At this moment I don’t know of anything better, but I have an idea.
A Decision Table
I could imaging drawing a table of entities vs entities, and marking whether a move from one to the other was to be allowed or not. Maybe we’d then turn that table into an object that somehow made the decision for us.
But as I wrote the paragraph above, it popped into my mind that the decision in hand is between entities, not between tiles. Tiles of room type don’t care what’s in them. It’s what’s in them that cares.
We finally bring it down to that, with acceptEntrance
but it takes us too long to get down to that and then too long to resolve it.
Let’s look at this problem afresh tomorrow. For now, we have the chest-opening behavior changed to keep the player out, and that’s a decent start.
See you next time!