Dungeon 233--Mistakes of the Past
Frequent readers know that I try to keep my code well-organized and well-designed. No one is perfect. Well, I’m not. Today we face mistakes.
My Theory of Incremental and Iterative Development comes down to a single idea, which today I’ll express this way:
A well-crafted program is ready for whatever new requirements may come along.
There’s nothing like a completely unexpected requirement to test that idea. Now it’s clear on the face of it that if we have just written an excellent incremental compiler for the Pascal language, a new requirement to control the diesel engine of a large tractor is going to mess up our design. Surely no one would expect that.1
But the notion we’re facing is pretty wild. We are asked to change our Dungeon program so that it can
- At least have a version that uses a hex map;
- Ideally allow hex maps and square-tile maps to exist in the same game;
Now I tell you truly that I had no idea that I might think of doing this. The hex idea just came to me from a random conversation about rooms and walls, during our Friday Coding meetings that happen every Tuesday. I just wanted to make a point about rotations allowing a wall or door to be drawn on any room’s edge, without regard to which edge it was.
Then I coded it up and somehow thought “oh wouldn’t it be fun to try to convert Dung to hex maps”. Even worse, I published the thought. That’s the problem with publishing one’s every thought.2
Well, we’re here now, so we might as well learn from doing the hexes, since I’m obviously not going to learn not to blurt out my every thought. So, in addition to publishing whatever “solution” I may come up with, I think there’s great value in trying to learn what gets in the way of this change, and whether my Theory of Incremental and Iterative Programming should handle it.
I think that one thing we’ll discover is that there are parts of the Dung program that are not as well crafted as they could be, that those parts are therefore already not as clear and simple as they could be, and that if they were better crafted, the hex implementation would be easier.
And, in most cases, I expect that we’ll first improve the crafting, and then plug in the hex and square notions to use them. In other cases, we’ll probably just hack through the brush, though I hope that is not common. And sometimes, we’ll probably write similar-looking code and try to remove the duplication.
We’ll see. Let’s look a bit deeper.
Learned So Far
I think the first thing we learned was that a lot of objects depend on Tile, and that we therefore had to turn off a lot of capability before we could get down to drawing a hex map instead of a square map. We removed or turned off
- Three tests with at least a half-dozen asserts;
- We stopped using
defineTile, in favor of just creating a HexMap; - Skipped some code in creating rooms;
- Changed monster creation to have no monsters at all;
- Created no decor, treasure, things, chests, or loots;
- Dropped the tiny map at top right;
- Did not draw any tile contents;
- Did not draw the player inventory;
- Did not show the player on the tiny map;
- Faked a tile where the Player could be placed, but in no sense inside the actual game;
- Turned off illumination.
I just created that list by looking at the diff for yesterday’s commit. In retrospect, it’s not that bad. But if we drill in another level, what will we find?
Coordinates
The Hex code includes an object Coord, that encapsulates the “cube coordinates” of a given hex. Coord includes the ability to add a Coord to another Coord (this is a mistake, by the way) and can step in a give direction and return the coord at that position. It can produce the Coords of a ring of any size around any given Coord. A Coord knows how to translate itself to a very raw “screen position”.
The Hex object itself just knows how to create a ring around itself, using the Coord to get the values. (This seems like the wrong place for that logic. Shouldn’t that be part of HexMap?) Hex can fill itself, but it cannot draw. That is handled in HexMap.
HexMap is mostly just a collection of Hexes, each of which has a Coord. HexMap stores things using a “key”, which happens to be a string “x,y,z” with the cube coordinates of the Hex expressed as a string. That’s done because Lua tables don’t like to have tables as keys, since table comparison is on identity not value.
However, HexMap knows how to draw the hexes:
function HexMap:draw()
pushMatrix()
pushStyle()
translate(WIDTH/2, HEIGHT/2)
local sc = 30
scale(sc)
strokeWidth(1.0/sc)
stroke(255)
for k,hex in pairs(self.tab) do
pushMatrix()
local coord = hex:screenPos()
local cx = coord.x
local cy = coord.y
translate(cx,cy)
for rot = 0,5 do
local bl = hex:boundaryLine(rot)
line(table.unpack(bl))
end
popMatrix()
end
popStyle()
popMatrix()
end
That’s obviously a gross violation of the Law of Demeter. It should just forward to Hex and let the Hex do the drawing. My excuse is that it was near the end of the morning and I just wanted to draw the darn thing, so I copied the code out of the Hex app’s draw function and pasted it in here.
I mentioned that the coordinate arithmetic in Coord is a mistake. In order to get the Hex that is , let’s say, at (1,-1,0) from a given Hex, I add the Coord of the Hex to a Coord(1,-1,0).
function Coord:__add(aCoord)
if aCoord == nil then error("nil aCoord") end
if aCoord:is_a(Coord) then
return Coord(self.x+aCoord.x, self.y+aCoord.y, self.z+aCoord.z)
else
error("Attempt to add a Coord and something else")
end
end
I’m proud that I could make that work at all, but it’s not as correct as it could be. The second Coord, aCoord isn’t really a legitimate Coord. Coords are points in space. But aCoord is a Direction, not a point. So there is a missing data type here, and we’re mixing up the meaning of Coord, sometimes meaning a point in cube space and sometimes a direction:
function Coord:direction(n)
-- n = 1-6
return Coord:directions()[n]
end
function Coord:directions()
return{ Coord(1,-1,0), Coord(1,0,-1), Coord(0,1,-1),Coord(-1,1,0), Coord(-1,0,1), Coord(0,-1,1) }
end
We should really have a Direction object and a Coord object.
But let’s look at the existing Dung code: it’s not even that close to what we need now.
The Tile Object
What follows is a bit incoherent and confusing. That’s kind of the point.
Our current Tile object is heavily loaded. I fell in love with the idea that the game could mostly operate around Tiles, and the result is that there’s a lot in there.
Tiles have “types”, including Edge, Wall, Room, and Hall. Each Tile knows what kind of Tile it is.
Tiles have x and y coordinates, which are abstract rectangular coordinates ranging from 1 to some limit. They store those coordinates as two member variables, and generally use them that way, but other times putting x and y into a vec2 type and manipulating the vector a bit and then pulling the x and y back out.
Either way, this is an example of “Primitive Obsession”, the use of primitive types like number or vec2 instead of a class that is more suitable to the application. I know better. But often, we start with primitives and then we get used to them, and they’re not so bad … and suddenly we have a big handful of undifferentiated numbers.
Tile includes quite a few methods of various flavors. A Tile can draw itself; it can answer whether there is at least one Tile adjacent to it of type Room.
A Tile can illuminate an area around it. This is used to decide how much the Player can see from the Tile she’s standing on.
A Tile has contents, at least the loot or treasure that is there. I don’t remember whether it still knows the player or monsters on it. A look at the code makes me think that it does.
Tiles are mostly managed, but not entirely managed, by the Dungeon object. The Tile, however, knows the current GameRunner instance, but does not know the Dungeon object in which it resides. This is probably nearly OK, but it does mean that some questions involve asking the runner for the current dungeon and asking it a question. A bit naff.
The above may seem chaotic and confused. That’s because the objects I’ve created do not divide up the responsibilities along clear lines. They have been good enough for the current game, but it’s likely that they have not been really good. They’ve probably been slowing us down a bit.
What Should We Do?
Tile
I think a small but important difference between the Hex app’s approach and the Dung app’s is that the Hex has a Coord object that bears a lot of the responsibility for geometry. In the Dung system, the Tile has x and y coordinates, and is therefore rather tightly tied to rectangular ideas.
Tile should have its x y coordinates generalized so as to accept either a HexCoord or SquareCoord without caring which it has. That will of course mean that some questions will be submitted to the Coords, but that is better than doing vector math inside Tile.
Dungeon
Similar concerns exist for Dungeon, which includes all the methods like ‘availableTilesCloserToPlayer`, which allow entities to move in sensible ways. Dungeon knows that the Tiles are square.
Dungeon should be converted to expect a generic Coord to be available, and should forward most of its mapping methods downward.
Dungeon knows the shape of the map, in the sense that it has the map stored in rows of tiles, and assumes that given the coordinates x,y, the corresponding Tile is found like this:
self.tiles[x][y]
Curiously, there’s code like this:
function Dungeon:convertEdgesToWalls()
for i,row in ipairs(self.tiles) do
for j,tile in ipairs(row) do
tile:convertEdgeToWall()
end
end
end
Clearly we have succumbed to the dreaded row-column x-y mistake here. I’m pretty sure that row should be col in the above. Better yet, we should iterate without regard to coordinates wherever we can.
Dungeon should be converted to use a HexMap object or SquareMap object to store Tiles, and should be coded to have no knowledge of the storage scheme.
Coord and Direction
We need HexCoord, HexDirection, SquareCoord, and SquareDirection. I think it’ll be ideal to use axial coordinates in the Hex case, which will give a sort of row-column character to the map. It should be harmless to do that, and might be helpful.
HexMap, SquareMap
Objects for these maps should be created, with the ability to iterate their contents (presumably Tiles), and, given one content item, find related ones using their contained Coords and Directions.
And More ….
I’m sure there will be more changes needed. Let’s plan just a bit.
Starting Plan
I’m inclined to this approach:
Hex Enhancements
First, go back to the D3 Hex experiment and enhance that code so that Hexes draw themselves, there’s a clear difference between Coord and Direction, and there’s a clear and proper division of responsibilities among the classes.
This will help clarify what the larger design should look like, by letting us view, in the small, what a good design should be. As a part of this, I think we should consider inserting a “Tile” object, or perhaps HexTile, in between the Map and the Hex, to give separate places to stand for low level operations like drawing, and higher level operations like managing contents.
Prepare D2
Once that is done, ready the ground in D2 for Hexes, by making similar changes to Tile, probably deriving a new “Square” object analogous to Hex.
We might do this a few times. For example, we know that Dungeon provides a raft of methods for finding geometrically-related Tiles. We might devise equivalents in Hex and then do similar things in Tile and Square.
I am minded to scrap the current DX app, which amounts to a branch (and a spike), and do most of the work in new HEAD versions of D2, because I am generally opposed to branch-focused process.
Grow from D2 to Hex
We want to improve the D2 code as needed, until it will support the Hex code transparently. Probably, in order for this to make sense, we’ll move the Hex code into D2 right after the first step above and then adapt our way down to plugging it in.
Subject to Change
This plan is effective now, October 1, 2021, 1419 Zulu. It is subject to revision in the light of experience, better ideas, or whim, starting October 1, 2021 1420 Zulu.
Let’s Get Started
Let’s return to the D3 program and do some of that changing we spoke about.
First, move this code from Main to HexMap:
function HexMap:draw()
for k,hex in pairs(self.tab) do
pushMatrix()
local coord = hex:screenPos()
local cx = coord.x
local cy = coord.y
translate(cx,cy)
for rot = 0,5 do
local bl = hex:boundaryLine(rot)
line(table.unpack(bl))
end
popMatrix()
end
end
Works. Commit: move draw to HexMap.
Now move the drawing to the Hex:
function HexMap:draw()
for k,hex in pairs(self.tab) do
hex:draw()
end
end
function Hex:draw()
pushMatrix()
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
popMatrix()
end
Works. Commit: Hex:draw() used by HexMap.
Nice. What about fill? The Hex can do that, and does it on command from above. Seems more like the user would provide the hex a fill color and it would just fill with it unconditionally. Or perhaps if it has a color, since the fill is a bit expensive?
Let’s give a hex a fill method that just sets the color, and change the current fill to drawFill.
function Hex:init(x,y,z)
self.coord = Coord(x,y,z)
self.fillColor = nil
end
function Hex:draw()
pushMatrix()
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
popMatrix()
self:drawFill(self.fillColor)
end
function Hex:drawFill(aColor)
if aColor == nil then return end
local m = modelMatrix()
local sc = m[1]
pushMatrix()
pushStyle()
local up = 10.0
local w = up*1.7320508076 -- sqrt(3)
local h = up*1.0
rectMode(CENTER)
scale(1.0/up)
stroke(aColor)
strokeWidth(1)
fill(aColor)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
popStyle()
popMatrix()
end
And in Main, I moved the fill call up to setup:
function setup()
if CodeaUnit then
codeaTestsVisible(true)
runCodeaUnitTests()
end
Hexes = Hex:createMap(6)
local center = Hexes:atXYZ(0,0,0)
center:fill(color(194, 104, 99))
end
Still works. Commit: Hex:fill now sets color. Fill always done in draw: if Hex.fillColor is non-nil.
Now there’s a raft of scaling going on in Main. We have worked out, in the drawFill method, how to get at the current scale and adjust to it. Let’s do that inside draw as well, and remove it from Main, except for the setting of the master scale.
First, I decided that I wanted to put the dot back at the center, just so that we can see that things haven’t shifted.
function draw()
if CodeaUnit then showCodeaUnitTests() end
pushMatrix()
pushStyle()
ellipseMode(CENTER)
stroke(color(255))
fill(255)
local sc = 50.0
strokeWidth(1.0/sc)
translate(WIDTH/2, HEIGHT/2)
pushMatrix()
scale(sc)
Hexes:draw()
popMatrix()
scale(1)
ellipse(0,0, 20,20)
popStyle()
popMatrix()
end
That gives us this picture:
You’ll note that I removed the rectMode(CENTER) from Main. It doesn’t draw any rectangles so it shouldn’t care. I did need two scale settings. In principle, draw starts with a unit transform, but I prefer to protect it with pushes and pops anyway.
Let’s move bits of this to better places. First, let’s extract the map drawing function from the main draw:
function draw()
if CodeaUnit then showCodeaUnitTests() end
pushMatrix()
pushStyle()
ellipseMode(CENTER)
stroke(color(255))
fill(255)
translate(WIDTH/2, HEIGHT/2)
drawMap()
ellipse(0,0, 20,20)
popStyle()
popMatrix()
end
function drawMap()
local sc = 50.0
strokeWidth(1.0/sc)
pushMatrix()
scale(sc)
Hexes:draw()
popMatrix()
end
Works. Commit: Main draws map in separate function.
Looks like I forgot to commit above a bit. Sorry. Let’s reorder drawMap to make more sense.
function drawMap()
pushMatrix()
local sc = 50.0
strokeWidth(1.0/sc)
scale(sc)
Hexes:draw()
popMatrix()
end
Works. Commit: reorder drawMap operations.
Move the scaling over to Hex, and while we’re at it, avoid the local variable.
function drawMap()
pushMatrix()
scale(50)
Hexes:draw()
popMatrix()
end
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
popStyle()
popMatrix()
self:drawFill(self.fillColor)
end
function Hex:getScale()
return modelMatrix()[1]
end
I wrote a new helper method to get scale when I need it. Works. Commit: stroke scaled inside Hex:draw.
Now we should be able to tidy up Hex:draw and drawFill a bit. Certainly can use the same strokeWidth, so remove that from drawFill, which goes from this … er, I note that drawFill no longer uses scale at all. So:
function Hex:drawFill(aColor)
if aColor == nil then return end
pushMatrix()
pushStyle()
local up = 10.0
local w = up*1.7320508076 -- sqrt(3)
local h = up*1.0
rectMode(CENTER)
scale(1.0/up)
stroke(aColor)
strokeWidth(1)
fill(aColor)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
popStyle()
popMatrix()
end
Commit: unused scale in Hex:drawFill removed.
Let’s reorder things to move the style setting upward:
function Hex:drawFill(aColor)
if aColor == nil then return end
pushMatrix()
pushStyle()
stroke(aColor)
fill(aColor)
rectMode(CENTER)
local up = 10.0
local w = up*1.7320508076 -- sqrt(3)
local h = up*1.0
scale(1.0/up)
strokeWidth(1)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
rotate(60)
rect(0,0,w,h)
popStyle()
popMatrix()
end
Works. Commit: move style bits up in Hex:drawFill.
What about that upscaling trick? We had to do that because Codea can’t properly draw little tiny rectangles scaled up. I think I should submit a bug report on that.
Let’s extract an explaining method here.
function Hex:drawFill(aColor)
if aColor then
pushMatrix()
pushStyle()
stroke(aColor)
fill(aColor)
rectMode(CENTER)
self:drawUpscaledFilledRectangles()
popStyle()
popMatrix()
end
end
function Hex:drawUpscaledFilledRectangles()
-- upscale compensates for Codea's not handling tiny rectangles well.
local up = 10.0
local w = up*1.7320508076 -- sqrt(3)
local h = up*1.0
scale(1.0/up)
strokeWidth(1)
rect(0,0,w,h) -- flat ==
rotate(60)
rect(0,0,w,h) -- angled //
rotate(60)
rect(0,0,w,h) -- angled \\
end
I was taught by Kent Beck that a comment is the code’s way of asking to be made more clear. Can we improve this? Maybe if // was a legal method name. But we’d have two or three calls at the very bottom of our drawing routine. That’s an efficiency issue. I have no measurement to tell me that it’s really a problem. Let’s try. First commit: explaining method Hex:drawUpscaledFilledRectangles.
function Hex:drawUpscaledFilledRectangles()
-- upscale compensates for Codea's not handling tiny rectangles well.
local up = 10.0
local w = up*1.7320508076 -- sqrt(3)
local h = up*1.0
scale(1.0/up)
strokeWidth(1)
self:drawFlatRect(w,h)
self:drawLowToHighRect(w,h)
self:drawHighToLowRect(w,h)
end
function Hex:drawFlatRect(w,h)
rect(0,0, w,h)
end
function Hex:drawLowToHighRect(w,h)
pushMatrix()
rotate(60)
rect(0,0, w,h)
popMatrix()
end
function Hex:drawHighToLowRect(w,h)
pushMatrix()
rotate(120)
rect(0,0, w,h)
popMatrix()
end
OK. I added in the push and pop because otherwise those three methods are temporally coupled and must be called in the correct order. Now they can be called in any order.
Is that better? I honestly don’t know. Got a better idea? Tweet me up.
Commit: refactor Hex:drawUpscaledFilledRectangles for hopefully improved clarity.
OK, I think we’ve squeezed all the juice out of the draw function.
Coord vs Direction
CW: Math
Now we have my somewhat esoteric objection about coordinates and directions. I’m not sure it’s important, but I think we should deal with it. The distinction, mathematically, is the distinction between a point and a vector.
A point is a position in space, represented by an ordered collection of numbers. The numbers are called coordinates (which makes me think the current naming is weak), and are often but not always named x, y, and z.
A vector has a magnitude (length) and a direction, but no fixed position in space. In any reasonable space, a vector can also be represented by an ordered collection of numbers, and they will generally also be named x, y, and z, if the coordinates of points have those names.
So the rules are
There are a bunch of points in space, with coordinates (x,y,z) or whatever.
Given any two points P and Q, there is a unique “vector from A to B”, which is written as
v = Q - P
It turns out that in a reasonable space, v will equal
(Q.x-P.x, Q.y-P.y, Q.z-P.z)
that is, the coordinate-by-coordinate difference of the two points.
There is a further rule, which is that for all P, Q, R
(P-Q) + (Q-R) = (P-R)
This rule, once you do some irritating math, leads you to the seemingly obvious fact that if you go from P to Q and thence to R, you can also go directly from P to R.
End CW: Math
Now in practice, we can always behave as if vectors and points are the same thing. Except when we cannot.
If we are at Point(5,4) and we add Vector(3,3), we wind up at Point(8,7). But if we were to add Point(3,3), that would be meaningless. So when we work with a class like Coord, we have to be careful that we have a vector-style, or what I called “direction” Coord when we add two Coords.
I think we’ll do better if we define two classes and become more mindful of the difference. I think the distinction will be almost invisible, until I make a mistake, which, let’s face it, could happen.
I propose to rename Coord to HexPoint and to create HexDirection. We’ll think of these as subclasses of an abstract Point and Direction. One might prefer to call Direction Vector, but since Codea has a type called vector, that seems risky. We’ll try these names.
Here goes. I’ve renamed Coord to HexPoint. I left the method atPoint rather than rename it atHexPoint, thinking that it’ll be a generic method indifferent as to which kind of map we have. We may find that to be a mistake.
Now my Directions want to be a new class, HexDirection.
function HexPoint:directions()
return{ HexPoint(1,-1,0), HexPoint(1,0,-1), HexPoint(0,1,-1),HexPoint(-1,1,0), HexPoint(-1,0,1), HexPoint(0,-1,1) }
end
HexDirection = class()
function HexDirection:init(x,y,z)
self.x = x
self.y = y
self.z = z
end
And this:
function HexPoint:__add(aHexDirection)
if aHexDirection == nil then error("nil aHexPoint") end
if aHexDirection:is_a(HexDirection) then
return HexPoint(self.x+aHexDirection.x, self.y+aHexDirection.y, self.z+aHexDirection.z)
else
error("Attempt to add a HexPoint plus a non-HexDirection")
end
end
I think I’ll spare you the dump of all this code. It was all simple renaming. Commit D3: Create HexDirection, rename Coord to HexPoint.
OK, I think that’s what we wanted to get done in the D3 spike, and it should guide our work toward doing the same thing in the Dung program. At this writing, I still plan, tentatively, to scrap the DX experiment from yesterday, and to build this all directly in the production program D2, working only at HEAD. We’ll see how that goes.
Let’s sum up.
Summary
We thought a lot today and only coded a little. The thinking helped make the code better. I believe that thinking is rarely wasted, and while one might have argued for moving more quickly to code, I just wasn’t ready.
We now have a fairly nice structure in the D3 Hex experiment, but there’s one more thing I’d like to do, which is to insert some kind of object on top of the geometric Hex object, representing the Tile with contents and such.
The idea will be that a Tile has a … a something … a Cell … which is either a Square or a Hex (or G*d save us a Triangle) and the Cell does geometry and drawing and the Tile does game things.
I’m not sure whether we’d do better to insert the Tile thing in D3, to better understand how it should work, or to begin refactoring in the direction of Square in D2. Either way, I’d like to get down to just having D2 as soon as possible.
I’m feeling more optimistic about the Hex project. Maybe I’m just getting used to the fear, but I don’t think so. I think it’s beginning to take shape.
See you next time to find out what I do next. At this moment, I don’t know either, and I look forward to finding out.