Thoughts on throwing things away. And let’s move these monsters.

I was thinking this morning about some topics that connect in my mind, if nowhere else.

  • Parts of the dungeon program are pretty nice; some are quite messy;
  • I’m sure that if I started over, I could get this far much faster;
  • Over a half-century of experience tells me that when I got this far, parts would be pretty nice and some would be quite messy;
  • If the excellent people working on Codea worked like I do, they could release good things sooner;
  • Their code is probably pretty nice in some places, messy in others; and finally:
  • Why working as I do is better than throwing big things away and starting over.

GeePaw Hill calls it “The Money Proposition”: We work the way we do because it lets us put a valuable working program in the hands of users faster than any other way we know.

The way we work–the way I work–Hill can speak for himself–is roughly this:

I express what I want in terms of very small, independent ideas. “Make the player’s health increase slowly until her limit.” “Change monsters to move near the player but not to attack.” I try to size the ideas I work on so that they can be done in a day, two at most.

When building an idea, I try to work in even smaller steps. Note one small step toward the idea. I try to write a test for that step whenever I can. Then I implement the step, and verify that the step works.

Frequently, when all the current tests are working, I try to improve the code. I draw back a bit, look at the overall shape of things, especially where I’ve just been working, and I try to improve the shape of the code without breaking it. My tests help me do that.

After all those tiny steps are done, in a matter of a day or two, I have a new idea built into the program, and given that my testing is good enough, I know that this version is better than yesterday’s. Everything works, including one new idea.

I can ship it.

Every one or two days, I can ship it. Not every one or two or six months. Every one or two days.

That’s the way we work. It’s basically the way everyone in my community works, and we do it because it gives us working code with more capability, day in and day out.

However.

As constant readers have seen here, sometimes it just doesn’t go quite that way. I have released somewhat serous defects into the Dung program, what, three times? In over 100 days of work, and certainly well over 50, maybe 75 working releases? That’s not bad for an old guy, and I know people who can do this better than I can.

My unusual mission in these articles is to let you see what happens when I mess up. I don’t plan to mess up: I’m human and it just happens. That’s the price of using humans in your process. The benefit is that we are often quite glorious. And although cats can program, they refuse to, so we’re the best you can get, so far.

So I mess up. I skip testing something because I can’t see how, or because it’ll be a pain. Sometimes I get away with it. Sometimes I don’t, and you get to see me struggling, confused and debugging, for a day or a few days. There’s a lesson in here: don’t do that.

Sometimes I don’t get the code clean enough. Things go well for a while, but over time, when I pass through that code, it slows me down. And here’s the curious thing: we almost always find ourselves passing through the bad code. Why? Because it typically has its fingers in places it shouldn’t. It typically bundles many different responsibilities together. It contains all the ideas that we didn’t quite get right, but that were important to get right.

So what do we do? For a while, we live with it. On a given day, even if the mess is slowing us down by 50%, putting in the feature just takes longer, but it’s still not too long. Our inability to see through the mess lets us make a few more mistakes, but the tests find most of them and the users find the rest.

Finally, we notice that this thing is giving us trouble, and we know we want to fix it. We need to fix it, because it is discernibly slowing us down. We’re losing money here, gang, we need to fix this.

We could rewrite the whole program. I’ve been tasked to do that a few times in my life and never once–never once–did it go well. Just don’t.

Or we could rewrite and replace the big ball of worms in the middle of our program. We can do it in parallel. We’ll work on it until it’s ready, and then we’ll plug it in. This trick never works. Well, probably it has worked in the past, but opening a beer by smashing it on your forehead has worked in the past, and most of us don’t do that.

Or, we can slowly, incrementally, move responsibilities out of the ball of worms, making it a bit better every time we need to work on it. Slowly, over time, it gets better and we begin to go faster. And since we’ll be moving and improving the bits that we actually use, we’ll go proportionally faster than you’d expect. It’s a 90-10 thing. If we can improve a thing that is a small percentage of the ball, but that we pass through, use, and edit a large percentage of the time, we win back speed and quality quickly.

That’s what we try to do, and when we do it well, it works delightfully. When it works not so well, it’s still better than a two-year rewrite that gets cancelled, or a 42-day “big refactoring” that never quite catches up, or that turns out to be missing some important plugs after all.

What I’m doing here is exploring what happens when I program, trying to work in the style that I am certain, based on long real experience is best. And working as a human, who gets tired, who sometimes lacks a good idea, who often makes mistakes, and whose cat simply will not help.

Your mission, should you decide to accept it, is to observe and try to understand, and to decide what you’re going to do about your own ball of worms in your own situation.

Observe, think, decide.

Now let’s get to work.

Moving the Monsters

We have a (nearly) working implementation of our new monster motion idea, which is that they’ll approach the player and cluster around, but they won’t attack unless the player corners them or attacks them. We have one “unsolved” issue, which is that we’d like them to move in, and then keep their distance, rather than oscillating between moving toward and away. I’m not sure why we want that, but we do.

And we have a new scheme for doing that, and it seems to me to promise a better approach to monster movement in general.

The core notion is “Manhattan distance”, MD, basically the sum of the x distance and y distance between two points. We saw, via some drawings that I made, that to approach the player, the monster wants to move to a square whose MD is one less than his current MD. To move away, he wants to move to a square whose MD is one greater. To stay at the same MD, well, move to a square that has the same MD.

The current code isn’t bad, but it goes through a few oddball calculations about the direction to the player, a vector of 1s and 0s. I think the MD code will be cleaner, more obvious, and probably can be pushed further down into the more elementary objects in the system.

As the tech lead on this project, I’ve decided that the learning and probable code improvement is worth this effort. And as the product owner, I agree. For what it’s worth, the cat has raised no objections.

However.

Yesterday I was speculating about a fairly difficult refactoring / replacement of the game’s overall structure, thinking that the Dungeon class might give us more help, and that some more GameRunner work might be pushed into Dungeon. I think that’s correct, but I believe I was reaching too far. I was thinking about actually removing GameRunner’s connection to the Tiles and placing them solely under Dungeon class’s control. I was getting close to saying “let’s do it!”

Today, I’m saying, sure, maybe we’ll get there, but, today, let’s just take some smalls steps on the way to getting monsters to move using Manhattan distance.

Let’s see where we left things.

Dungeon Helps

Our most recent test was just a beginning:

        _:test("Dungeon helps monster moves", function()
            local dungeon = Runner:getDungeon()
            dungeon:setPlayerPosition(vec2(30,30))
            local monsterTile = dungeon:getTile(vec2(34,34))
            _:expect(dungeon:manhattanToPlayer(monsterTile)).is(8)
        end)

It tells us, however, what we’re about. We’re going to get the Dungeon class to help us pick moves for the monster.

Here’s what I was thinking. Suppose we could say soething like this to the dungeon: Give me all the Tiles, adjacent to me, with Manhattan Distance to the player of D. D would be one more or one less than our MD, or possibly equal. (Equal MD requires a diagonal move, which is easy enough but new to us.)

The Dungeon would return a collection of Tiles, almost invariably two of them. (We could implement it to return only room tiles, in which case it could be fewer.) Then we, the monster, select one randomly, or, if we’re a smarter monster than the current crop, we could select based on some other criterion, like moving near a colleague, or moving to block a doorway. For now, we’d select randomly.

That design would almost certainly collapse our current moving strategy down to some really nice clean code.

So let’s do it. We need something else to ask the Dungeon to do. How about this:

        _:test("Dungeon helps monster moves", function()
            local dungeon = Runner:getDungeon()
            dungeon:setPlayerPosition(vec2(30,30))
            local monsterTile = dungeon:getTile(vec2(34,34))
            _:expect(dungeon:manhattanToPlayer(monsterTile)).is(8)
            local tiles = dungeon:availableTilesCloserToPlayer(monsterTile)
            local x1,x2 = dungeon:getTile(vec2(33,34)), dungeon:getTile(vec2(34,33))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
        end)

I started out thinking, ask for the distance you want, in this case 7, since we’re at 8. Then I thought, why not just ask for closer. Dungeon knows everything it needs in order to do that.

Test should fail looking for that avail... method.

2: Dungeon helps monster moves -- Tests:442: attempt to call a nil value (method 'availableTilesCloserToPlayer')

Yay, life is good.

I typed this in, optimistically:

function Dungeon:availableTilesCloserToPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile) - 1
    local pos = aTile:pos()
    local result = {}
    for delta in pairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    return result
end

Rather than explain it, I’ll run the tests.

2: Dungeon helps monster moves -- TestDungeon:99: attempt to call a nil value (method 'neighborOffsets')

Oh yea, I forgot. Tests are great

function Dungeon:neighborOffsets()
    return { 
    vec2(-1,-1), vec2(0,-1), vec2(1,-1),
    vec2(-1,0),              vec2(1,0),
    vec2(-1,1),  vec2(0,1),  vec2(1,1)
    }
end

That ought to be the offsets of one’s neighbors. Run test.

2: Dungeon helps monster moves -- TestDungeon:100: bad argument #-1 to 'add' (vec2)

That’s here, of course:

        local t = self:getTile(pos + delta)

I should have said ipairs in the loop. Saying pairs made delta be an integer. And anyway I need to refer to the other element. Sorry, had another language loaded into my brain.

function Dungeon:availableTilesCloserToPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile) - 1
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    return result
end

Test runs!! Nice. Let’s test the other two cases we care about.

        _:test("Dungeon helps monster moves", function()
            local tiles, x1,x2
            local dungeon = Runner:getDungeon()
            dungeon:setPlayerPosition(vec2(30,30))
            local monsterTile = dungeon:getTile(vec2(34,34))
            _:expect(dungeon:manhattanToPlayer(monsterTile)).is(8)
            tiles = dungeon:availableTilesCloserToPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(33,34)), dungeon:getTile(vec2(34,33))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
            tiles = dungeon:availableTilesFurtherFromPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(35,34)), dungeon:getTile(vec2(34,35))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
        end)

We need the new function:

function Dungeon:availableTilesFurtherFromPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile) + 1
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    return result
end

I think I see some duplication here. Does the test run? It does. Woot!

Let’s refactor:

function Dungeon:availableTilesCloserToPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile) - 1
    return self:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
end

function Dungeon:availableTilesFurtherFromPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile) + 1
    return self:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
end

function Dungeon:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    return result
end

This was just an ExtractMethod refactoring. Nothing to it. Tests still run.

Let’s do the third request:

            tiles = dungeon:availableTilesSameDistanceFromPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(33,35)), dungeon:getTile(vec2(35,33))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)

And the new function:

function Dungeon:availableTilesSameDistanceFromPlayer(aTile)
    local desiredDistance = self:manhattanToPlayer(aTile)
    return self:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
end

Tests run. Code is nice. I feel a commit is in order: Dungeon can find candidate tiles at closer to, further from, or same distance from player.

Now Let’s Plug It In

Well, we have this nice new capability, let’s plug it into the monster movement functions. We have this test for selecting our ovement methods:

        _:test("Select moveTowardAvatar range 4-10", function()
            local method
            method = Monster:selectMove(10)
            _:expect(method, "range 10").is("moveTowardAvatar")
            method = Monster:selectMove(4)
            _:expect(method, "range 4").is("moveTowardAvatar")
            method = Monster:selectMove(3)
            _:expect(method, "range 3").is("maintainRange")
            method = Monster:selectMove(2)
            _:expect(method, "range 2").is("moveAwayFromAvatar")
            method = Monster:selectMove(1)
            _:expect(method, "range 1").is("moveTowardAvatar")
            method = Monster:selectMove(0)
            _:expect(method, "range 0").is("moveAwayFromAvatar")
        end)

So those are the methods that do the moving. Here they are:

function Monster:moveAwayFromAvatar()
    local dxdy = -1* self.runner:playerDirection(self.tile)
    self:moveInDirection(dxdy)
end

function Monster:moveTowardAvatar()
    local dxdy = self.runner:playerDirection(self.tile)
    self:moveInDirection(dxdy)
end

function Monster:moveInDirection(dxdy)
    local rand = math.random()
    local move = self:chooseDxDy(dxdy,rand)
    self.tile = self.tile:legalNeighbor(self,move)
end

function Monster:chooseDxDy(dxdy,rand)
    if dxdy.x == 0 or dxdy.y == 0 then
        return dxdy
    elseif rand < 0.5 then
        return vec2(0,dxdy.y)
    else
        return vec2(dxdy.x,0)
    end
end

function Monster:maintainRange()
    self:moveAwayFromAvatar()
end

We can do better, especially if you’ll permit me to roll in the random selection:

How about this:

function Monster:moveTowardAvatar()
    local dungeon = self.runner:getDungeon()
    local tiles = dungeon:availableTilesCloserToPlayer(self.tile)
    self.tile = self.tile:legalNeighbor(self,tiles[math.random(1,#tiles)])
end

I have high hopes for that. My hopes are somewhat dashed by this message when I was about to encounter a monster:

TestDungeon:140: attempt to index a nil value (local 'pos')
stack traceback:
	TestDungeon:140: in function <TestDungeon:139>
	(...tail calls...)
	TestDungeon:151: in method 'manhattanToPlayer'
	TestDungeon:96: in method 'availableTilesCloserToPlayer'
	Monster:247: in field '?'
	Monster:145: in method 'chooseMove'
	Monsters:61: in method 'move'
	GameRunner:329: in method 'playerTurnComplete'
	Player:228: in method 'turnComplete'
	Player:167: in method 'keyPress'
	GameRunner:283: in method 'keyPress'
	Main:33: in function 'keyboard'

The error was fired here:

function Dungeon:getTile(pos)
    return self:privateGetTileXY(pos.x, pos.y)
end

Called from here:

function Dungeon:manhattanToPlayer(tile)
    return tile:manhattanDistance(self:playerTile())
end

Ah, right. The Dungeon doesn’t yet know where the player is. We poked that in as part of our development when we discovered, no surprise, that we’d need to know it. We’ll need to put it in. We have this:

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

We change to this:

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

And when we create it:

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

And then we have to fetch the tile:

function Dungeon:playerTile()
    return self.player:getTile()
end

I elected to make finding the player tile dynamic. Right now, when we get a Dungeon instance, everything is static, but if we were to cache it, the player could move about from time to time. So I saved the player and ask her for her tile. Seemed fair.

I have even higher hopes now. However this breaks my tests, which had that hackery to put in the player position.

        _:test("Dungeon helps monster moves", function()
            local tiles, x1,x2
            local dungeon = Runner:getDungeon()
            dungeon:setPlayerPosition(vec2(30,30))
            local monsterTile = dungeon:getTile(vec2(34,34))
            _:expect(dungeon:manhattanToPlayer(monsterTile)).is(8)
            tiles = dungeon:availableTilesCloserToPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(33,34)), dungeon:getTile(vec2(34,33))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
            tiles = dungeon:availableTilesFurtherFromPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(35,34)), dungeon:getTile(vec2(34,35))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
            tiles = dungeon:availableTilesSameDistanceFromPlayer(monsterTile)
            x1,x2 = dungeon:getTile(vec2(33,35)), dungeon:getTile(vec2(35,33))
            _:expect(tiles).has(x1)
            _:expect(tiles).has(x2)
        end)

Nothing for this one but to create a player and pass her in. Irritating. I’m going to ignore the test fail for a moment and do a live test as well.

As I feared, I get a run-time error as well.

Tile:191: bad argument #-1 to 'add' (vec2)
stack traceback:
	[C]: in metamethod 'add'
	Tile:191: in method 'getNeighbor'
	Tile:323: in method 'legalNeighbor'
	Monster:248: in field '?'
	Monster:145: in method 'chooseMove'
	Monsters:61: in method 'move'
	GameRunner:329: in method 'playerTurnComplete'
	Player:228: in method 'turnComplete'
	Player:167: in method 'keyPress'
	GameRunner:283: in method 'keyPress'
	Main:33: in function 'keyboard'

Silly me. legalNeighbor wants, not a tile, but a step vector.

function Tile:legalNeighbor(anEntity, aStepVector)
    local newTile = self:getNeighbor(aStepVector)
    return self:validateMoveTo(anEntity,newTile)
end

We can make our new method use validateMoveTo instead:

function Monster:moveAwayFromAvatar()
    local dungeon = self.runner:getDungeon()
    local tiles = dungeon:availableTilesFurtherFromPlayer(self.tile)
    self.tile = self.tile:validateMoveTo(self,tiles[math.random(1,#tiles)])
end

function Monster:moveTowardAvatar()
    local dungeon = self.runner:getDungeon()
    local tiles = dungeon:availableTilesCloserToPlayer(self.tile)
    self.tile = self.tile:validateMoveTo(self,tiles[math.random(1,#tiles)])
end

This works in play. The monsters come close, then try to move away. Sometimes they can’t: that’s usually because we’re not checking in our new Dungeon code for the tiles being of type Room. If we do that, there is a chance there will be none, so we’ll have to deal with that possibility in the code above. For now, let’s make that test run, and we can commit. (The same-distance move is still not implemented, let’s not forget that.)

This is more tricky than I’d like. As always, setting up enough of a dungeon to allow a player to be correctly created is difficult. I’m going to provide a fake one, and inject it with a similar method to the one we had:

        _:test("Dungeon helps monster moves", function()
            local tiles, x1,x2
            local dungeon = Runner:getDungeon()
            dungeon:setTestPlayerAtPosition(vec2(30,30))
            local monsterTile = dungeon:getTile(vec2(34,34))
...

We’ll leave it to the Dungeon to deal with this special case. It creates an instance of a special class:

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

And the class:

DungeonTestPlayer = class()

function DungeonTestPlayer:init(dungeon, aVector)
    self.dungeon = dungeon
    self.pos = aVector
end

function DungeonTestPlayer:getTile()
    return dungeon:getTile(self.pos)
end

I expect that the test may run and if not it’ll be clear what I broke. And indeed, it runs. Tragedy averted.

Let’s commit: monsters move using new dungeon tile finding. Same distance still is “away from”.

Now let’s do the same distance one:

function Monster:maintainRange()
    local dungeon = self.runner:getDungeon()
    local tiles = dungeon:availableTilesAtDistanceFromPlayer(self.tile)
    self.tile = self.tile:validateMoveTo(self,tiles[math.random(1,#tiles)])
end

There’s a bit of duplication in these methods but not much and I don’t really see how to improve the situation.

A quick run then commit again. Ah, here’s why we do run the game:

Monster:228: bad argument #1 to 'random' (interval is empty)
stack traceback:
	[C]: in function 'math.random'
	Monster:228: in field '?'
	Monster:145: in method 'chooseMove'
	Monsters:61: in method 'move'
	GameRunner:329: in method 'playerTurnComplete'
	Player:228: in method 'turnComplete'
	Player:167: in method 'keyPress'
	GameRunner:283: in method 'keyPress'
	Main:33: in function 'keyboard'

I’m not sure what’s up but it seems likely that we didn’t get any tiles back. We need to deal with that case.

I think we should put the change here:

function Dungeon:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    return result
end

If result is empty, let’s return the input tile, which is always legit.

function Dungeon:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance then
            table.insert(result, t)
        end
    end
    if #result == 0 then
        result = {aTile}
    end
    return result
end

Run again.

final

As you can see in the movie above, the monsters hang around, but do pretty well at keeping away if you get too close to them. Sometimes I expect them to move away but they do not. There are some cases where I have the Death Fly pinned to the bottom wall where I was pressing the useless Flee button, which gives the monsters a turn, and I expected the DeathFly to move to the side to get further away, but it didn’t.

Ah, that’s because it selected the wall, randomly, to move back, which it couldn’t. A nice feature would be not to include non-Room tiles in the output. An easy change, shall we do it?

function Dungeon:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance and t:isRoom() then
            table.insert(result, t)
        end
    end
    if #result == 0 then
        result = {aTile}
    end
    return result
end

I think this will work fine in play but I am concerned that during test all the tiles are probably of type Edge. The tests may break.

As I feared, this breaks all the tests. But in play, the monsters move away so effectively that you can only get into a fight with them if you corner them. Otherwise they just sidle away. That’s kind of good.

Now for the tests, we could do one of two things. We could hammer all the tiles in the test to room. Or we could change the code we just wrote to accept a type Edge tile. Those rarely occur in the game, and really never should. If we do return one in play, the move would just get rejected anyway.

Let’s do that.

function Dungeon:availableTilesAtDistanceFromPlayer(aTile, desiredDistance)
    local pos = aTile:pos()
    local result = {}
    for i, delta in ipairs(self:neighborOffsets()) do
        local t = self:getTile(pos + delta)
        local d = self:manhattanToPlayer(t)
        if d == desiredDistance and (t:isRoom() or t:isEdge()) then
            table.insert(result, t)
        end
    end
    if #result == 0 then
        result = {aTile}
    end
    return result
end

I expect the tests to run. They do. Commit: monsters escape better than ever.

Ship It and Sum Up

Oh, wait, there are some excess methods we can remove now, the ones that used to be used in the old mover scheme. It’s unfortunate that Codea can’t readily find those for us. If you ask it for references to a string, it’ll find it, but it can’t just scan the whole program for you.

Monster:moveInDirection can be removed as unused. chooseDirection is only used in tests, so they and the method can go.

While I’m thinking about it, let’s check that move selection again:

function Monster:selectMove(range)
    if range > 10 then
        return "makeRandomMove"
    elseif range >= 4 then
        return "moveTowardAvatar"
    elseif range == 3 then
        return "maintainRange"
    elseif range == 2 then
        return "moveAwayFromAvatar"
    elseif range == 1 then
        return "moveTowardAvatar"
    else
        return "moveAwayFromAvatar"
    end
end

That’s OK if the range is an integer. However:

function Monster:chooseMove()
    -- return true if in range of player
    if self:isDead() then return false end
    local range = self:distanceFromPlayer()
    local method = self:selectMove(range)
    self[method](self)
    return range <= 10
end

But we could use our new manhattanFromPlayer. Let’s see:

function Entity:distanceFromPlayer()
    return self.runner:playerDistance(self.tile)
end

Well, this is a bit intricate but let’s first change the meaning:

function GameRunner:playerDistance(aTile)
    return self.player:distance(aTile)
end

We’ll just change the meaning.

function GameRunner:playerDistance(aTile)
    return self:getDungeon():manhattanToPlayer(aTile)
end

This isn’t ideal, since we are changing a concept, but my plan is to rename the methods shortly. Let’s test. One tests in testTiledGame:

7: distance from avatar  -- Actual: 8.0, Expected: 5.6

That’s probably no surprise. Let’s look.

        _: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:playerDistance(tile)
            _:expect(d).is(5.6,0.1)
        end)

Yes. We’ve changed the meaning. New answer is 4+4 or 8.

I added a comment, but I still plan some renaming.

        _: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:playerDistance(tile) -- manhattan
            _:expect(d).is(8)
        end)

Tests are back to green.

Rename playerDistance to playerManhattanDistance.

Rename distanceFromPlayer to manhattanDistanceFromPlayer.

Tests run. Game:

damn flies

These damn flies won’t leave me alone, but they never come close enough to swat. Bastards.

Commit again: removed dead methods, renamed some to refer to manhattan. Let’s really ship and sum up:

Ship and Sum

This is how it’s supposed to be. The tests help us work through even a moderately complicated replacement of an algorithm, and we wind up with a cleaner and more cler understanding of monster motion. We have better deferral of computations downward in the hierarchy (though not yet perfect).

We were never notably confused, guided almost always by tests, although sometimes by rather clear and obvious failures in the real code. That’s unfortunate: it would be better had we had tests that found those cases.

One case, we’d already been aware of, when our move finders couldn’t find any cells to move to. We hadn’t catered to it, and we still don’t have a test for it. On the other hand, it seems to me that it obviously works, so I’m not too worked up about it. We did add some very nice tests and made them all work immediately.

Reflecting back on the early remarks in this article, we have a near-perfect example–at least as nearly perfect as this human is ever likely to be–of how and why we work the way we do.

We went from idea to working code in a couple of days. We then observed that the code wasn’t marvelous, and thought of a new approach, with a new definition of distance, Manhattan distance. We worked up functions to compute that, and plugged them in in another couple of mornings.

Idea to done: two mornings. Better implementation and a simpler system: two more mornings.

This is the way.


D2.zip