No promises, just a look at Tile and its friends.
I did some refactoring yesterday after writing D-316, and probably we’ll see the results of that as we go. There were six commits, four just named “tidying”, reflecting a couple of added comments about constraints, but mostly reordering methods. I found an unused method and removed it, and added a new method
contentsDo in Tile, and used it in
doQueries, which became this:
function Tile:doQueries() self:contentsDo(function(obj) obj:query() end) end
That’s supported by the new
function Tile:contentsDo(f) for _,obj in ipairs(self:getContents()) do f(obj) end end
I wanted to use the FP functions to do that, but we do not have an
do FP function, and
map won’t work because you can’t collect nils. Someday, I’ll enhance FP, but yesterday was not that day, and that day may never come.
My tentative plan for today is to check to see whether any objects actually hold onto their tile, and to check users of
self:currentTile() to see what they’re up to. That might lead to a useful idea on narrowing the breadth of the Tile class, which still has 59 methods after yesterday’s deletion of one.
I’ll start with a search for
self.tile, hoping to find few or none. To my delight, there are no occurrences of “self.tile:” or “self.tile “ or “self.tile.”. That makes me fairly certain that no dungeon objects are holding on to their tile. Now we’ll look at
self:currentTile(). I’m sure there are a bunch of those.
And there are. I’l show them here, so that you can browse with me. I’m looking for any patterns of usage, just opening my mind to ideas.
function Entity:remove() self:currentTile():removeContents(self) end
This one I like. It seems necessary and is not invasive. I wonder, though, why it is so low in the hierarchy rather than up in DungeonObject. Don’t we remove other objects? So I will divert for a moment and check that out. I think we may have two ways to accomplish the same thing. Loot must get removed, let’s look there.
Sure enough, I find this:
function Loot:actionWith(aPlayer) self:currentTile():removeContents(self) self.item:addToInventory() end
Same line of code. Duplication, who knew? Let’s move the remove function up to DungeonObject and use it here.
function Loot:actionWith(aPlayer) self:remove() self.item:addToInventory() end function DungeonObject:remove() self:currentTile():removeContents(self) end
Test. Green. Also tested some loot and decor just to be sure. Commit: move remove function to DungeonObject, use in loot.
Now I’ll look for other
removeContents to see if there are other candidates. I find that Key does the same. Fix that. Test. Commit: Change Key to use DO:remove().
You may wonder whether those two changes were worth doing. I think they were. There used to be two textually different ways to remove yourself from the dungeon, although they were equivalent. Now there is just one way and everyone who wants to be removed uses it. In addition, every occurrence of
currentTile is a reference to a very broad object, and we’d on a quest to limit those. So let’s get back to it:
function Entity:manhattanDistanceFromTile(tile) return self:currentTile():manhattanDistance(tile) end
I let that one ride. I think when we cover the Tile by calling just one method on it like that, it’s probably OK. And we might be able to return a more narrow instance that only knows the few methods we use that way.
This next one is a bit more irritating:
function Monster:manhattanDistanceFromPlayer() return self.runner:playerManhattanDistance(self:currentTile()) end
Let’s see how that’s done.
function GameRunner:playerManhattanDistance(aTile) return self:getDungeon():manhattanToPlayer(aTile) end function Dungeon:manhattanToPlayer(tile) return self:manhattanBetween(tile, self:playerTile()) end function Dungeon:manhattanBetween(tile1, tile2) return tile1:manhattanDistance(tile2) end
I don’t mind those much, as they are all one-liners, drilling down. We should note, however, that if DungeonObjects knew the Dungeon, the call to the runner could be eliminated. We’re not on that quest just now, and I don’t feel like accepting a side quest. It’s Sunday and will be time for breakfast soon.
function Monster:__tostring() return string.format("Monster (%d,%d)", self:currentTile():pos().x,self:currentTile():pos().y) end
There are a few of these
__tostring methods, and I do want them to display their position. They’re not in the main stream, so even though I do have an idea for them, namely moving the duplication part of these up to DungeonObject, I think they’re low priority.
This is interesting:
function Monster:basicMaintainRangeToPlayer(dungeon) local tiles = dungeon:availableTilesSameDistanceFromPlayer(self:currentTile()) self:basicRandomMove(tiles) end
I was wondering where this method gets its dungeon and the answer is that it ultimately asks the runner, here:
function Entity:getDungeon() return self.runner:getDungeon() end
We can remove a method from GameRunner, I think. Back to this:
function Monster:manhattanDistanceFromPlayer() return self.runner:playerManhattanDistance(self:currentTile()) end function GameRunner:playerManhattanDistance(aTile) return self:getDungeon():manhattanToPlayer(aTile) end
We can inline that:
function Monster:manhattanDistanceFromPlayer() return self:getDungeon():manhattanToPlayer(self:currentTile()) end
Test that. We’re good. Commit: Monster gets distance to player from dungeon not thru Runner.
There’s another caller of that method tho:
_:test("distance from avatar", function() local x,y = 100,100 local player = Player:privateFromXY(104,104, runner) runner.player = player local tile = runner:getTile(vec2(x,y)) local d = runner:playerManhattanDistance(tile) -- manhattan _:expect(d).is(8) end)
So let’s just defer that by fetching the dungeon:
_:test("distance from avatar", function() local x,y = 100,100 local player = Player:privateFromXY(104,104, runner) runner.player = player local tile = runner:getTile(vec2(x,y)) local d = runner:getDungeon():manhattanToPlayer(tile) -- manhattan _:expect(d).is(8) end)
Now we can remove that method. Commit: remove unused playerManhattanDistance method.
That side quest was valuable as it let us remove a method. So far this morning we have moved a couple of things, removed some lines, and little else. The program is overall just a bit simpler.
There are a bunch of uses in the monster move strategy but they are all just passing the current tile into distance calculations and such. I think they are OK.
Here’s something interesting:
function Monster:drawMonster() if not self:currentTile():isVisible() then return end pushMatrix() pushStyle() spriteMode(CENTER) noTint() local center = self:currentTile():graphicCenter() translate(center.x,center.y) self.behavior.flip(self) self.animator:draw() popStyle() popMatrix() end
We have two calls to
currentTile in the same method. Would this be better?
function Monster:drawMonster() local currentTile = self:currentTile() if not currentTile:isVisible() then return end pushMatrix() pushStyle() spriteMode(CENTER) noTint() local center = currentTile:graphicCenter() translate(center.x,center.y) self.behavior.flip(self) self.animator:draw() popStyle() popMatrix() end
I’m saying yes, but on another day I might say no. Testl Commit: use local currentTile in drawMonster.
I note the call to
function Monster:xScaleToFaceTowardPlayer() local dir = self.runner:playerDirection(self:currentTile()) return ((dir.x > 0) and self.facing) or -self.facing end
This is called, ultimately, from inside
drawMonster, but it’s a bit of a slog from the
self.behavior.flip(self) down to this method. I think we’ll leave that for next time, with a sticky note, and go see about brekkers.
Let’s sum up.
Today we removed complexity, or, channeling Colin Chapman, we added simplicity. Not much, just a bit.
What we have seen so far is that there are a few special methods, like
remove, that reference the current Tile, and those were improved by using that method in all the classes that needed to remove themselves from the Dungeon.
Other uses of the current tile were fairly righteous, and we unwound one reference to the runner because we had a more direct way to get the distance to the player.
There are a raft of methods about selecting useful tiles relative to oneself, and those may be candidates for breaking out of Tile somehow, but it was too much for a Sunday.
Overall, we’ve simplified the system, and done seven commits in less than an hour. A good morning.
And a good morning to you, as well.