Today I’m going to start plugging the new Map classes into the actual Dung program. I expect a little chaos. For large values of ‘little’.

Right. There are at least two ways we might do this. One would be to rig up the D3 project to be able to be used as a dependency, which would keep the already excessive number of tabs down in D2, the actual Dung program. The other is to copy and paste the code directly into Dung. The former method would mean that whenever the Map classes need changing, we’d have to exit D2, enter D3, make the changes, then go back to D2 and (probably) reconnect. So I think I’ll go with moving the tabs.

There are three tabs in D3 (besides its Main), Tests, MapPoint, and Map. I could combine them into one but keeping them the same as D3 will allow me to move back and forth, if I must, more readily. I guess I just bite the bullet and do it.

Right off the bat something goes wrong. I paste the tests into a new tab, and run the program. Of course I expect all those tests to fail, and they do. But it appears that my other tests didn’t run at all, nor did the green/red text appear. Don’t you just hate when something like that happens?

After a bit of messing about, I convince myself that all the tests are running. I check that by intentionally breaking several different ones. They are just running very silently now. And rapidly, I might add.

And the game still runs, so that’s good. Now I think I can move over the other tabs.

The second error surprises me. Tests fail. It turns out that there is a class named Map already, in the PathFinder. Oops.

I’ll change that class name to PathMap. I think it’s internal to PathFinder but I’ll search just to be sure.

And that turns up another issue. I have that clever function (not object or method) named map, that applies a function to a table, returning f(element) for all elements. I think I’ll let that ride.

The substitution of PathMap for Map in PathFinder works. I believe that the new classes and tests are in the Dung system, and doing no harm. Best commit: New Map classes and tests included and not installed.

Review So Far

What have we learned? Well, one thing is that we need to be more careful with our naming, although I suspect a wiser programmer than I might have managed to make what is now PathMap a local class to PathFinder, but that turns out to be more tricky than one might wish. I’d have to play with local classes a bit more to be sure how best to set them up.

Beyond that, I guess the discovery calls for “more care” in devising ideas like this, but fact is, that’s not likely to happen. We are as careful as we are. If I were the sort of person who makes checklist and checks through them, then I could add something to my infinite checklist and tick through it at some finite speed, but that’s not happening.

Other than that, the installation has gone pretty smoothly so far, I am not tricked by this seemingly reasonable result so far, because the next thing will be a first experiment with the new map.

First Try

I expect to revert out what we do now. If I were a branching kind of person, I might do what follows in a branch, but I am not such a person.

I plan to just find where we create the new tiles in Dungeon, and create a cartesian map instead, and see what explodes. I expect a lot, but it may be that we can find and fix the issues quickly. There is always hope.1

Level creation starts in GameRunner:

function GameRunner:createLevel(count)
    -- determine level
    self.dungeonLevel = self.dungeonLevel + 1
    if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
    
    self:dcPrepare()
    
    -- customize rooms and connections
    self.rooms = self.dungeon:createRandomRooms(count)
    self.dungeon:connectRooms(self.rooms)
...


function GameRunner:dcPrepare()
    TileLock = false
    DungeonContents = DungeonContentsCollection()
    self:createNewDungeon()
    local dungeon = self.dungeon
    self.tiles = dungeon:createTiles(self.tileCountX, self.tileCountY)
    dungeon:clearLevel()
    Announcer:clearCache()
end

I reckon we need to look at this:

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

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

Already I see some issues. Our new Map will create instances of a provided class and put them into the Map. But here we have a bit more going on.

I think we can ignore the define, since the Map will pass the position to the Tile it creates. But we would really like to create our Tile differently.

I take a quick look at Tile.

function Tile:init(x,y,kind, runner)
    if runner ~= Runner then
        print(runner, " should be ", Runner)
        error("Invariant Failed: Tile must receive Runner")
    end
    self.position = vec2(x,y)
    self.kind = kind
    self.runner = runner
    self:initDetails()
end

function Tile:edge(x,y, runner)
    return Tile(x,y,TileEdge, runner)
end

So we have learned a thing or two. One is that our Tiles have x and y in them, and we’d like them to have a MapPoint instead.

This is going to be a moderately large concern, entailing changes to a fair number of methods inside Tile. And, frankly, I think we should see the situation as an existing problem in the design. Tile is treating two independent primitive variables, x and y, containing numbers, as a logical “thing”, i.e. “the coordinates of the tile”.

In a more perfect design, we’d have some kind of coord variable there, and we’d be better off than we are. I mention this because as we encounter trouble with this new requirement, we want to get a sense of what it would have taken for us to be better prepared. Here, the answer is that we actually knew better than we did. That’s OK … no one is perfect … but this was not a failure of grand design, it was a failure of day-to-day execution.

Yabbut …

Yabbut, the real question is what are we going to do about this? Let’s see if we can come up with more than one idea, so that the idea we come up with is at least relatively good.

  1. We could change Tile:init to expect some kind of coordinate object, and to default to type edge. I’m not sure quite how we’d get the runner passed in.
  2. We could change Map to expect a function instead of a class, and to call that function. We could define that function so that it had the runner as an accessible value and could plug it in. The function could even break out x and y if we were willing to break encapsulation that badly.
  3. We could change Map to expect a callback object, and it could call an agreed method on that object, like create, passing the coordinate, and that callback object would return whatever was needed.

It seems to me that ideas 2 and 3 won’t work well unless we deal with the coordinate vs x,y issue in Tile. And doing so will surely help us later. That aside, I think 3 is better than 2.

Let’s see about converting Tile to have a member variable coord or something like that. Essentially we’re going to refactor Tile to be better suited to use Map.

Another look at Tile makes me feel better about this idea:

function Tile:init(x,y,kind, runner)
    if runner ~= Runner then
        print(runner, " should be ", Runner)
        error("Invariant Failed: Tile must receive Runner")
    end
    self.position = vec2(x,y)
    self.kind = kind
    self.runner = runner
    self:initDetails()
end

We already store the x and y in self.position! That’s helpful. We could perhaps change the creation methods all to expect a position. Let’s see what we mostly do with the position variable.

We only have two uses. The one above and this:

function Tile:pos()
    return self.position
end

OK, so far this doesn’t look too bad. The x and y go away quickly and the position is fairly nicely covered. How do we use pos()?

Dungeon uses Tile:pos() fairly frequently, mostly to find neighbors. That’s probably reasonable: Dungeon is the current object that knows the shape of things. In the future, we’re expecting to support that usage and/or to push some of it down into Map, now that we have (what we hope (q.v.) is) better support.

Here are the uses of pos() in Tile: I’ll comment on each as we go through them, with an eye to what we might need to do.

function Tile:directionTo(aTile)
    local t = aTile:pos()
    return vec2(sign(t.x-self:pos().x), sign(t.y-self:pos().y))
end

Our new tiles will each have a Point instance. We can implement directionTo on Point and delegate.

function Tile:distance(aTile)
    return self:pos():dist(aTile:pos())
end

Similarly, we can delegate to Point. Currently this is using vector distance. We can replicate that if need be.

function Tile:drawLargeSprite(center)
    -- we have to draw something: we don't clear background. Maybe we should.
    local sp = self:getSprite(self:pos(), false)
...

The getSprite function appears not to use either of its parameters. Should be easy to fix.

function Tile:getNeighbor(aVector)
    local newPos = self:pos() + aVector
    return self.runner:getTile(newPos)
end

We’ll have to make aVector be a Direction, most likely. Then we can defer to the Map

function Tile:getSurroundingInfo()
    local byte = 0
    local ck = { vec2(1,1),vec2(0,1),vec2(-1,1), vec2(1,0),vec2(-1,0), vec2(1,-1),vec2(0,-1),vec2(-1,-1) }
    for i,p in ipairs(ck) do
        byte = byte<<1
        local pos = self:pos() + p
        local tile = self.runner:getTile(pos)
        if tile:isFloor() then
            byte = byte|1
        end
    end
    return byte
end

This little bit of arcanity seems to be returning a byte with a 1 bit wherever the neighboring tile is of type floor. I hope to hell there’s a test for that, but I bet there isn’t. Anyway we can surely replicate it in map, perhaps by just returning the designated neighbors and letting the caller sort out what it’s trying to do.

function Tile:graphicCorner()
    return (self:pos() - vec2(1,1))*TileSize
end

We’ll need to change this but it’ll be much like what’s here.

function Tile:hasRoomNeighbor()
    local ck = { vec2(1,0),vec2(1,1),vec2(0,1),vec2(-1,1), vec2(-1,0),vec2(-1,-1),vec2(0,-1),vec2(1,-1)}
    for i,p in ipairs(ck) do
        local pos = self:pos() + p
        local tile = self.runner:getTile(pos)
        if tile:isFloor() then return true end
    end
    return false
end

Answer whether any of the neighboring tiles is of type floor (room). Do like the byte one.

function Tile:illuminateLine(dx,dy)
    local max = IlluminationLimit
    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

This may just work. If not it’ll be close. As with some of the others, it does bring up an issue, which is that to fetch a Tile we call back to Runner, which will call down to Dungeon, which (in the new scheme) will call Map. Seems a bit round the horn but doesn’t pose any large issues.

function Tile:manhattanDistance(aTile)
    return manhattan(self:pos(), aTile:pos())
end

Easily changed. Probably should defer to Point.

Let’s reflect. Vampires are excused but should read along.

Reflection noitcelfeR

The biggest issue that I see is that there are a lot of things to change in Tile, plus about four or five in Dungeon, plus a few odds and ends in the tostring methods and such.

I’m concerned about some of the methods, like that neighbor one that returns a byte of bits. That may not be used frequently, and we might miss fixing it up, leaving a defect in the program. That would be bad.

One possibility would be to create a new method, maybe position() and use it in converted methods, placing an assert in pos() to trap any users. Even so, we might not find the byte one.

I’ve made myself curious. I put an assert in there and the program immediately crashes. So we can stop worrying about whether we miss that one out.

The hasRoomNeighbor one is harder to elicit. I can find no calls to it in the code. That’s interesting.

I did however manage to crash the game, with this message:

Map:23: MapType must be cartesian or hex
stack traceback:
	[C]: in function 'assert'
	Map:23: in field 'init'
	... false
    end

    setmetatable(c, mt)
    return c
end:24: in function <... false
    end

    setmetatable(c, mt)
    return c
end:20>
	(...tail calls...)
	Player:249: in local 'method'
	EventBus:112: in method 'publish'
	Inventory:314: in function <Inventory:309>
	(...tail calls...)
	Main:106: in function 'touched'

I was trying to spawn a pathFinder. And somehow we got into the Map code.

Ah, found it. GameRunner creates the Map for the PathFinder, and was calling Map. Changed to PathMap. Commit: Fixed GameRunner to use PathMap class for PathFinder map.

Now it really seems that there is no occurrence of hasRoomNeighbor being called. Imma delete that and commit: removed unused hasRoomNeighbor.

What Now?

I think we have a bit of figuring out to do before we try to plug the Map into Dungeon. Let’s talk about why.

It seems clear that we could just plug it in and then rely on our tests (and probably some game play, since we know that our tests aren’t the best) to work through whatever breaks. However, that does not seem to me to be a two-hour job. It seems to me that it could take a few days of two-hours to accomplish. That would be bad.

Why would that be bad? Because for however many days that took, I couldn’t release the program, I couldn’t fix other defects if there were any, I couldn’t add a quick feature if one was needed. The program would be broken.

Of course, I could branch. But my friends don’t branch, and if they don’t branch, then I’d be no friend of theirs if I did. Seriously, we don’t lock up our build with broken code, and we don’t branch because long-lived branches are hard to integrate.

What I require of myself during this difficult time of putting hexagons into the Dung, is that I can (and do) release the game every day while the project goes on … and that I release it unbroken. Let’s talk about why.

Fear, that’s why!

Kent Beck once said that all methods are based on fear, whatever the method adherent fears is at the core of the method. What I fear is that my supporters, who want things like better treasures in the Dungeon and who are wondering why the CatGirl says she’s going to give you something and then doesn’t, those supporters will become tired of waiting for features and cancel the project. I fear that because it has happened to me, more times than I’d care to count.

So the Big Lesson I’ve taken from my years in “Agile” is the notion that the product must always be ready to ship, that we can always add new capabilities to it, and that even when we have sweeping changes to make to it, we can do that incrementally, bit by bit, step by step2, without long delays either in features or in progress about large-scale changes.

Now there’s just one of me, so I really can’t be working at the same time on making CatGirl give out some treasure and on hexagonal maps. But I can behave as though there were more than one of me and make all my changes accordingly. That means that putting hexagons in needs to be done bit by bit, step by step, q.v.

We’re good today: The new code is in the product and is doing no harm. And the article is long enough and the time has gone on long enough to let us stop for the morning. And we need one or more ideas for moving forward.

Here’s an idea that just popped into my head. What if we were to create our existing tiles array, and in addition a corresponding Map. And then we might begin to defer some of the old operations to the Map, and we could take our time figuring out a good way to get those more complex Tiles into the Map, and so on.

I’m not going to decide that today, but tomorrow, when we do the next steps, we need to get at least some of the Map into play and we need not to break the game,

I look forward to seeing whether I can accomplish those things.

I hope (q.v.) you’ll join me then.

  1. Hope is not a strategy. – Michael Henos ↩

  2. … slowly I turned … ↩