Dungeon 237--Well, Hell!

Another idea? A possibly better idea? We don’t pay you to think, we pay you to do.

I wake, fairly reliably, an hour or so before I really want to get up. Today was no exception. After a couple of minutes out of bed, I settled in to think myself back to sleep. And I thought, I thought …

Hexes can be indexed by two dimensions, usually called q and r. Hexes can be laid out in a nearly-rectangular form. Tiles are indexed by two dimensions, and are stored in a rectangular form. There’s a lot of similarity there. Maybe we can do something simpler than the current Map, Point, and Direction, each differentiated by Hex or Cartesian.

Well, I probably didn’t think it all nice like that, but I certainly did think of the rectangular hex layout and the two dimensional indexing, and hey this is all similar. I even thought briefly about extending WASD keys to QWEASD to allow for hex navigation. Do people ever do that? If not, I’ll patent it.

I did have one other thought, but that was later, as I was making my morning iced chai latte. The “real” coordinates for hexes have three dimensions, q,r,s or x,y,z as you may prefer. It appears that no one uses g,h,i. Square tiles could have 3 dimensional coordinates too, just set one of them to a constant, probably zero.

Let’s run with this a bit, but remind me to come back and discuss the Evil Manager’s remark up there in the blurb.

Let’s imagine that we did build our Tiles with three coordinates, x,y,z. How they’re stored is irrelevant to our discussion, I think. We have a table and can put and get Tiles at ((x,y,z) through some magical means. Probably all our Tiles on a given “level” would be the same sort of thing, hex or square. I say probably because …

Suppose each Tile knew the Directions to its neighbors. Hexagonal ones know six directions, square ones know four. Could we possibly make squares and hexes co-exist? I’m not sure, because of the need to force squares to having a zero coordinate. Belay that idea: we’ll stick with a single type on a given level.

So a level has either four directions or six, and either way, we know that three-dimensional vector addition will convert a tile and a direction to a new tile.

Where might we end up?

We could probably have a single Coordinate type, covering a 3-d point, stored as vector or equivalent, changeable by adding a Direction to it. The Direction is also a 3d vector, and the only difference is that there are two sets of directions. Hex ones look as they do now. Cartesian ones would be any four orthogonal vectors, matching well with the storage scheme, if it matters. Fact is, in the Cartesian mode, one of the dimensions never varies, and the table doesn’t care.

So Point and Direction have no differences other than the specific data values in the two types of direction. The Maps have no difference, because Point always has three dimensions, and we just pick any scheme that is reasonable. Probably the current one with the keys converted to a string and hashed into the table.

The only issue that I see just now is that the Tiles need to space themselves differently (the screenPos method) and of course draw themselves differently. In my spike, the D3 program, I only draw hexes, and that’s done by making the HexMap a map of Hexes, which can draw themselves. In a scheme like the one I’m describing here, with Tiles remaining agnostic about whether they’re hexagonal or not, where will the screen position and drawing differences reside?

We don’t know the answer to that, but we don’t know it in yesterday’s scheme either, since we weren’t planning to use the Hex object, just the Map, Point, and Direction ones.

My thinking so far is that this idea is a nearly good one: there is more commonality between Hex and Cartesian than I’ve previously recognized, and it should be exploited. And it would seem that we can use just one kind of Map rather than our present two.

Let’s pause now and listen to Evil Manager:

Evil Manager

Another idea? A possibly better idea? We don’t pay you to think, we pay you to do.

Now, I’ve heard from a lot of programmers whose managers really do say things like that to them, or tell them they are paid to code not paid to test or refactor, and other ridiculous things.

No manager has ever said anything like that to me. I suspect it’s because despite being a kind of chubby nerd, I am somehow scary, and if you talk to me very long at all, you’d find that that’s the kind of thing up with which I will not put.¹

But does Evil Manager have a point? Are we wasting time here? Do we have a “good enough” solution running? Maybe. But there are issues.

We currently have a design with six small classes, three for Cartesian and three for hex. Whatever one of these classes does, the other almost certainly has to do. If they stay much as they are, no problem. If they have to change, then the changes will have to be considered for each of the pair, and may proliferate to double, quadruple, or even hextuple.

Once we proliferate classes in a system, it’s difficult to collapse them again. Even when we find duplication, we’re faced with issues that complicate things. We are tempted to resort to subclassing, or to plug-in strategies, or other such complexities. Rarely, in my experience, do we ever wind up coalescing two or more classes down to one. So what we do here, we will live with for a long time.

The time to make this idea simpler is now. Evil Manager can take a flying leap at a rolling hexagon. We were hired to use our brains. If programming were nothing but typing, they’d have hired court reporters.

Therefore … What?

If I were really dead certain that everything would be really simple, I might take this idea and start installing it. But I’m not quite that certain. Instead, I’m going to stay in D3 for one more day, and see if I can get the code down to fewer classes or otherwise simpler.

I think we’ll go after Map first. If this scheme will work, then there only needs to be one kind of Map. Let’s do that. We’ll rename HexMap, the more robust one, to Map. And we’ll change it to create a “rectangular” map, not the disc one. One nice thing about that is that we can remove all the ring code that we spent so much time writing. No problem, it’s saved right here in the WorkingCopy repo. And I know where there are articles I can look up.

To keep my tests running, maybe I should copy HexMap to Map and work with the separate one? Then I can convert my tests. Let’s do that, see what it might look like.

Ha! Immediately I run into an issue. There’s the problem with thinking. Our map needs to know what kind of Coordinate to create, because hexagonal maps have different indexes from Cartesian. Cartesian will have y==0, I figure, and the hex maps, the y coordinate varies internally, even if we are using q,r format.

Reviewing Red Blob, I conclude that if we assume we’re using axial coordinates x and z, and Cartesian x and z for symmetry, we “merely” need to transform our raw coordinates if and only if we are doing a hex map.

So, I need a test and I need it now. We do have a test for Cartesian Map. Let’s convert it:

        _:test("Cartesian Map", function()
            local map = CartesianMap(10,15, Square)
            for x = 1,10 do
                for y = 1,15 do
                    local sq = map:atXY(x,y)
                    local point = sq:point()
                    _:expect(point:x()).is(x)
                    _:expect(point:y()).is(y)
                end
            end
        end)

That’ll become this:

        _:test("Cartesian Map", function()
            local map = Map(10,15, Map.cartesian, Square)
            for z = 1,10 do
                for x = 1,15 do
                    local sq = map:atXYZ(x,0,z)
                    local point = sq:point()
                    _:expect(point:x()).is(x)
                    _:expect(point:y()).is(0)
                    _:expect(point:z()).is(z)
                end
            end
        end)

Design decisions here: First, we’re going to use x and z as our varying coordinates. When we’re done, only a test will know that. We might even be able to change this test not to know about it. Second, you pass in one of two items as your third parameter, either Map.cartesian or Map.hex. We’ll see in a moment how those are used.

Run the test, see it fail.

7: Cartesian Map -- Map:7: attempt to call a nil value (method 'createRectangle')

That’s because I did this much speculation when I pasted Map from HexMap:

Map = class()

function Map:init(xWidth, yHeight, klass)
    self.tab = self:createRectangle()
end

function Map:atPointPut(aPoint, anObject)
    local key = aPoint:key()
    self:atKeyPut(key, anObject)
end

function Map:atXYZ(x,y,z)
    return self:atPoint(HexPoint(x,y,z))
end

function Map:atPoint(hexPoint)
    return self:atKey(hexPoint:key())
end

function Map:atKey(aKey)
    return self.tab[aKey]
end

function Map:atKeyPut(aKey, aHex)
    self.tab[aKey] = aHex
end

function Map:draw()
    for k,obj in pairs(self.tab) do
        obj:draw()
    end
end

There’s way too much stuff still in there. Let’s plug in the notion of the Map.hex thing and do createRectangle. We’ll remove useless stuff soon.

My next nearly useful cut is this:

function Map:init(xWidth, zHeight, mapType, klass)
    assert(mapType == Map.cartesian or mapType == Map.hex, "MapType must be cartesian or hex")
    self.mapType = mapType
    self:createRectangle(xWidth, zHeight, klass)
end

function Map:createRectangle(xWidth, zHeight, klass)
    self.tab = {}
    for z = 1,zHeight do
        for x = 1,xWidth do
            local pt = self.mapType(x,z)
            print(pt)
            local item = class(pt)
            self:atPointPut(pt,item)
        end
    end
end

function Map.hex(x,z)
    return HexPoint(x,-x-z,z)
end

function Map.cartesian(x,z)
    return CartesianPoint(x,z)
end

That drove out a key function for CartesianPoint:

function CartesianPoint:init(x,z)
    self.xCoord = x
    self.zCoord = z
    self.keyString = x..","..z
end

function CartesianPoint:key()
    return self.keyString
end

I also provided these:

function CartesianPoint:x()
    return self.xCoord
end

function CartesianPoint:y()
    return 0
end

function CartesianPoint:z()
    return self.zCoord
end

The test error is now

7: Cartesian Map -- Map:50: table index is nil

function Map:atKeyPut(aKey, aHex)
    self.tab[aKey] = aHex
end

That assignment is complaining. So aKey is nil.

Arrgh. I did something, probably something right, and now the test has a different error. But I was distracted by the cat, and now I don’t know what I changed. Anyway, the test now says:

7: Cartesian Map -- Map:33: attempt to index a nil value (local 'aPoint')

function Map:atPointPut(aPoint, anObject)
    local key = aPoint:key()
    self:atKeyPut(key, anObject)
end

So we’ve called that with a nil …

Ah. I was going to instrument this method and left off the return.

function Map.cartesian(x,z)
    local pt = CartesianPoint(x,z)
    return pt
end

Now we’re back to this:

7: Cartesian Map -- Map:51: table index is nil

function Map:atKeyPut(aKey, aHex)
    self.tab[aKey] = aHex
end

That’s telling me that my point’s key is nil. Let me verify that:

function Map:atPointPut(aPoint, anObject)
    local key = aPoint:key()
    assert(key, "key is nil")
    self:atKeyPut(key, anObject)
end

That does assert. Weird:

function CartesianPoint:init(x,z)
    self.xCoord = x
    self.zCoord = z
    self.keyString = x..","..z
end

function CartesianPoint:key()
    return self.keyString
end

I am surprised and confused. Let me write a trivial test here, to get my head on straight.

        _:test("CartesianPoint key", function()
            local pt = CartesianPoint(1,1)
            _:expect(pt:key()).is("1,1")
        end)

That fails! Amazing.

Enhance the test:

        _:test("CartesianPoint key", function()
            local pt = CartesianPoint(1,1)
            _:expect(pt.keyString, "string").is("1,1")
            _:expect(pt:key(), "key").is("1,1")
        end)

The first expect passes. The second fails. Have I stupidly left off a return statement? I bet I have. But no. Here’s the code, with prints now sprinkled in:

function CartesianPoint:init(x,z)
    self.xCoord = x
    self.zCoord = z
    self.keyString = x..","..z
    print("CP", x, z, self.keyString)
end

function CartesianPoint:key()
    print("CP:key", self.keyString)
    return self.keyString
end

Here’s the full output on test 7.

CP	1	1	1,1
7: CartesianPoint key string -- OK
7: CartesianPoint key key -- Actual: nil, Expected: 1,1

OK, how can this be failing:

        _:test("CartesianPoint key", function()
            local pt = CartesianPoint(1,1)
            _:expect(pt.keyString, "string").is("1,1")
            _:expect(pt:key(), "key").is("1,1")
        end)

Wait! I don’t see the print “CP:key”. Is there a second key method in here?

Sure enough, there was a second one in there. Somehow I started it and left it. So it overrode the other. Weird. Test again.

I’m getting three failures (and was for a while). Let’s see what they are.

1: HexPoint Creation  -- Actual: function: 0x28224def0, Expected: Invalid HexPoint
3: CartesianPoint Arithmetic -- CartesianPoint:46: attempt to perform arithmetic on a nil value (field 'yCoord')
8: Cartesian Map -- HexPoint:10: Invalid HexCoordinates 1,0,1

Let’s fix 3 first.

function CartesianPoint:__add(aCartesianDirection)
    if aCartesianDirection == nil then error("nil aCartesianDirection") end
    if aCartesianDirection:is_a(CartesianDirection) then
        return CartesianPoint(self.xCoord+aCartesianDirection.xDir, self.zCoord+aCartesianDirection.yDir)
    else
        error("Attempt to add a CartesianPoint plus a non-CartesianDirection")
    end
end

Referencing the z coordinate should fix that. We want to coalesce the Point class but we’re not there yet.

Right, down to two. Why does CartesianMap encounter code in HexPoint?

Ah:

function Map:atXYZ(x,y,z)
    return self:atPoint(HexPoint(x,y,z))
end

We haven’t converted that yet.

function Map:atXYZ(x,y,z)
    return self:atPoint(self.mapType(x,z))
end

We only need to provide x and z in either the Cartesian or he x case, because we compute the y coordinate in hexes now. Run test expecting pass.

8: Cartesian Map -- Tests:93: attempt to index a nil value (local 'sq')

Dammit. We didn’t get anything back from map.

I apologize, I’m going to need to print this bugger. Let’s make it smaller.

No. The indexes in the test are reversed.

        _:test("Cartesian Map", function()
            local map = Map(10,15, Map.cartesian, Square)
            for z = 1,15 do
                for x = 1,10 do
                    local sq = map:atXYZ(x,0,z)
                    print(sq)
                    local point = sq:point()
                    _:expect(point:x()).is(x)
                    _:expect(point:y()).is(0)
                    _:expect(point:z()).is(z)
                end
            end
        end)

Darn row-column x-y error. One test fails now:

1: HexPoint Creation  -- Actual: function: 0x2820c66d0, Expected: Invalid HexPoint

However, those expects in the cartesian map test are printing 450 lines. I gotta do something about that. But first let’s see what happened in test one.

        _:test("HexPoint Creation", function()
            local coord = HexPoint(0,0,0)
            _:expect(coord:invalid()).is(false)
            local f = function() HexPoint(1,1,1) end
            _:expect(f).throws("Invalid HexPoint")
        end)

function HexPoint:init(x,y,z)
    self._x = x
    self._y = y
    self._z = z
    if self:invalid() then error("Invalid HexCoordinates "..x..","..y..","..z) end
    self.keyString = x..","..y..","..z
end

function HexPoint:x()
    return self._x
end

function HexPoint:y()
    return self._y
end

function HexPoint:z()
    return self._z
end

function HexPoint:invalid()
    return 0 ~= self:x() + self:y() + self:z()
end

How did that ever break?

This isn’t going well. It’s not going badly, but it feels choppy, and as if I’m making silly mistakes. I think if I were wise, I’d revert and start over. I am not wise. And I feel like I’m close to ready for the next bit. It’s 1148 local. Let’s see when I next surface.

The test was wrong. It’s 1150 local.

        _:test("HexPoint Creation", function()
            local coord = HexPoint(0,0,0)
            _:expect(coord:invalid()).is(false)
            local f = function() HexPoint(1,1,1) end
            _:expect(f).throws("Invalid HexCoordinates 1,1,1")
        end)

OK, we’re green. We can commit: New Map object can create Cartesian map.

Let me take a short break here. It seems wise. Then I think I’ll write a test for Map and hex layout.

1318 local

I’m back from a run to the pharm, and to Wendy’s, and now I plan to pop Map into my HexMap stuff.

A wild problem appears! My Hex test makes a ring, and I’ve decided not to use the ring shape but instead the nearly rectangular shape.

Let’s just change the Main and see what happens.

...
    Hexes = Map(5,6, Map.hex, Hex)
    local center = Hexes:atXYZ(1,0,1)
    center:fill(color(194, 104, 99))

The display becomes this:

I observe that my map coordinate is illegal. x+y+z is supposed to be zero. Let’s fix that and then see what else is happening.

    local center = Hexes:atXYZ(1,-2,1)

No change to the picture. Surprising but not terribly, because of this:

function Map:atXYZ(x,y,z)
    return self:atPoint(self.mapType(x,z))
end

So we could put the square root of whatever in for y and it would be set to -2. But why isn’t it on the map?

Let’s see what Hex:draw does.

function Hex:draw()
    pushMatrix()
    pushStyle()
    rectMode(CENTER)
    strokeWidth(1.0/self:getScale())
    local coord = self:screenPos()
    local cx = coord.x
    local cy = coord.y
    translate(cx,cy)
    for rot = 0,5 do
        local bl = self:boundaryLine(rot)
        line(table.unpack(bl))
    end
    text(self:point():key(),0,0)
    popStyle()
    popMatrix()
    self:drawFill(self.fillColor)
end

I added in that text call. I get this:

What’s the filled one? Let’s remove the call to fill and find out.

Well! The fill is going into the wrong place. Let’s pick another hex for it. I choose 3,-6,3.

And, hahaha, the fill still goes to the center. I think my drawFill is flawed.

function Hex:drawFill(aColor)
    if aColor then
        pushMatrix()
        pushStyle()
        stroke(aColor)
        fill(aColor)
        rectMode(CENTER)
        self:drawFilledRectangles()
        popStyle()
        popMatrix()
    end
end

This function is called outside the range of the screenPos logic! Let’s fix it:

function Hex:draw()
    pushMatrix()
    pushStyle()
    rectMode(CENTER)
    strokeWidth(1.0/self:getScale())
    local coord = self:screenPos()
    local cx = coord.x
    local cy = coord.y
    translate(cx,cy)
    for rot = 0,5 do
        local bl = self:boundaryLine(rot)
        line(table.unpack(bl))
    end
    self:drawFill(self.fillColor)
    text(self:point():key(),0,0)
    popStyle()
    popMatrix()
end

Now we get this:

That looks somewhat like I intended, although not quite what I was thinking would happen. I had been sort of envisioning a more rectangular layout, not this parallelogram. But that requires a more complex creation approach, which I had forgotten. And I really don’t like z moving downward on the screen. Didn’t I fix that? I suspect that either I didn’t, or I forgot.

-- TODO this needs explanation and/or a diagram
function HexPoint:screenPos()
    local rad = HexPoint:screenRadius()
    local q = self:x()*rad
    local r = self:z()*rad
    local x = (1.7320508076*q + 0.8660254038*r) -- sqrt(3), sqrt(3)/2
    local y = -1.5*r -- we invert z axis for z upward
    return vec2(x,y)
end

Well, there’s the negation. But it’s not appropriate, is it?

Removing that minus sign, we get the expected picture:

OK, that’s good for visual inspections. Let’s see about replacing the use of HexMap with Map in all the tests. We’ll want to rewrite the ring one, I guess. Probably make it just like the Cartesian one.

        _:test("Hex Map", function()
            local map = Map(10,15, Map.hex, Hex)
            for z = 1,15 do
                for x = 1,10 do
                    local hex = map:atXYZ(x,0,z)
                    print(hex)
                    local point = hex:point()
                    _:expect(point:x()).is(x)
                    _:expect(point:y()).is(-x-z)
                    _:expect(point:z()).is(z)
                end
            end
        end)

That one runs. The ring test passes too, but, but we don’t need it, because we’re dropping the ring shape.

4: Screen Positions  -- Actual: -48.0, Expected: 48.0

Right. We fixed that. Test says:

        _:test("Screen Positions", function()
            _:expect(HexPoint(0,0,0):screenPos()).is(vec2(0,0))
            local oneUp = HexPoint(1,0,-1):screenPos()
            _:expect(oneUp.x).is(HexPoint:screenRadius()*0.866, 0.01)
            _:expect(oneUp.y).is(HexPoint:screenRadius()*1.5, 0.01)
        end)

Ah. Used to work because of my - sign, now it’s not there, so the -1 in z is passed through. Let’s pick a cell with positive z, for clarity, and because we’ll likely only use positive z going forward.

That, of course, breaks the other coordinate:

4: Screen Positions  -- Actual: 83.1384387648, Expected: 27.712

The correct test is this:

        _:test("Screen Positions", function()
            _:expect(HexPoint(0,0,0):screenPos()).is(vec2(0,0))
            local oneUp = HexPoint(1,-2,1):screenPos()
            _:expect(oneUp.x).is(HexPoint:screenRadius()*1.5*math.sqrt(3), 0.01)
            _:expect(oneUp.y).is(HexPoint:screenRadius()*1.5, 0.01)
        end)

Ask me why privately, but it is. Tests are green. Commit: Map can create parallelogram hex map.

It’s 1408 local. Let’s sum up and look forward.

Summary

I was a bit ragged today. I’m not sure if I was too distracted, or what. I didn’t make any big mistakes, but it was like I was dropping more balls than usual.

It is possible that instead of working to refactor the D3 spike, I should have just written a new set of point and direction and map objects. But this way seems more likely to cover the bases, even if it does get a bit ragged.

Coming up, I think we can go to a single kind of Point, which will be maintained in the correct form, (x,0,z) or (x,-x-z,z) as appropriate, by the two flags Map.hex and Map.cartesian. Flags to you, functions inside. Semi-nifty, I think.

We might think about making them methods, but they do not depend on self. But they are a bit weird, semi-weird, I think.

To get a single kind of Point, I think we have to go to a single kind of Direction first, because the Point:add method checks data type. The Directions are basically just two tables anyway. Direction is a structure, with no behavior of note. It can extend itself, and we don’t use that.

I think that should all go quite smoothly. Tomorrow. Unless the group derails me at tonight’s meeting of the Friday Coding Ensemble, which meets on Tuesday evenings.

See you tomorrow, I hope with no brilliant new ideas. But you never know.