Dungeon 236--One More Thing ...
I promise, I’m nearly ready to plug in the new hex coordinates. There’s just one more thing that I want to do …
I got to thinking, as one does, and did a bit of review of Amit’s articles on the Red Blob site, and before I move the Point and Direction code into D2, there’s a bit of tidying that really ought to be done. I do hope to start plugging it in today. We’ll see.
My plan stands much as it was yesterday, going for a record in how long a plan lasts. I’m gong to move over just the point and direction code, and build the Dungeon class on a substrate of Tiles containing either cartesian or hexagonal Points. I actually expect this change to be easy … and I expect to encounter some surprises. Most of the surprises will probably reflect design flaws in the code that’s already there. We’ll see.
I do plan to retain the difference between Point and Direction, which should really be called Point and Vector. I’m reluctant to use the word vector, because Codea Lua has a vector type. We’ll use Direction for now.
Let’s start here …
HexDirection = class()
function HexDirection:init(x,y,z)
self.x = x
self.y = y
self.z = z
end
function HexDirection:direction(n)
-- n = 1-6
return HexDirection:directions()[n]
end
function HexDirection:directions()
return{ HexDirection(1,-1,0), HexDirection(1,0,-1), HexDirection(0,1,-1),HexDirection(-1,1,0), HexDirection(-1,0,1), HexDirection(0,-1,1) }
end
My plan is to use Lua’s vec3 type as the underlying structure for both Directions and Points. So:
function HexDirection:init(x,y,z)
self.vector = vec3(x,y,z)
end
That requires me to make corresponding changes in HexPoint:
Rather a Long Delay Later …
This isn’t working. That trivial change has broken some tests and I’m not sure why. Having made the change to vector in all the places I need it, I don’t see much advantage. Revert, and let’s import this stuff.
I hate it when a cool idea comes to naught. But truth is, there are only a couple of places where the vector could be used, and fetching the x y z won’t be much more costly. I thought it would make the code somewhat brighter, but it’s not all that much better.
Importing
I really only want to import the Points and Directions, not the HexMap and SquareMap, which my new thinking says we don’t need as such. We will have something like them, but …
I was going to say “but we’ll create them in situ”, and that got me thinking about how we’ll create them.
My rough plan is this: when the Dungeon goes to create a level, it begins by creating a table, self.tiles that contains the tiles that make up the base of the Dungeon. We want two new objects to replace that table, a HexMap and a CartesianMap. When we ask for those to be created, we’ll pass in a class or function to call to create a Tile, to be arranged as the Map may decide.
My current Maps don’t really do that. HexMap can’t fill itself at all, it relies on someone else to create the rings that make it up. The CartesianMap does create an 2d array, but it is an array of Squares.
Neither of these is quite the thing. Let’s make it work, and make it right, inside the D3 spike, and only then move it over. There’s no point in moving over too early and creating work that is more complicated in the larger system.
Flip-Flopping?
There is nothing much that ticks me off more than the current media and political notion of “flip-flopping”, pulled out every time someone changes their position. Smart people change position all the time, because smart people are learning.
So no. I am adjusting my plan based on new understanding. That’s a good thing, in programming and in life.
OK, let’s get our Maps ready.
Cartesian Map
The basic idea is that the maps will be given enough parameters to know how big a map to create, and a class to instantiate. By convention, the class init will accept a Point, a HexPoint or CartesianPoint as appropriate.
So we’ll do some tests to make sure that’s what happens. Inside this spike, I’ll create Hex objects and Square objects, just because the Hexes can draw themselves and we have a picture as part of the output of the spike. In principle, I could put any objects in there. That’s kind of the point.
We have this now:
CartesianMap = class()
function CartesianMap:init(xWidth, yHeight)
self.xWidth = xWidth
self.yHeight = yHeight
self.columns = {}
for x = 1,self.xWidth do
self.columns[x] = {}
for y = 1, self.yHeight do
self.columns[x][y] = Square(x,y)
end
end
end
function CartesianMap:atXY(x,y)
return self.columns[x][y]
end
Square = class()
function Square:init(x,y)
self.coord = CartesianPoint(x,y)
end
function Square:coords()
return self.coord
end
We want to pass in a class and instantiate it, there where we do the square. And we want to pass it a CartesianPoint, not x and y, Here’s the new code:
CartesianMap = class()
function CartesianMap:init(xWidth, yHeight, klass)
self.xWidth = xWidth
self.yHeight = yHeight
self.columns = {}
for x = 1,self.xWidth do
self.columns[x] = {}
for y = 1, self.yHeight do
self.columns[x][y] = klass(CartesianPoint(x,y))
end
end
end
function CartesianMap:atXY(x,y)
return self.columns[x][y]
end
Square = class()
function Square:init(aPoint)
self.coord = aPoint
end
function Square:coords()
return self.coord
end
Surprisingly, we have a test for CartesianMap:
_:test("Cartesian Map", function()
local map = CartesianMap(10,15)
for x = 1,10 do
for y = 1,15 do
local sq = map:atXY(x,y)
local c = sq:coords()
_:expect(c.xCoord).is(x)
_:expect(c.yCoord).is(y)
end
end
end)
However. Let me do some renaming in there to better make my point, no pun intended:
_:test("Cartesian Map", function()
local map = CartesianMap(10,15)
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)
This expresses the situation better. The square has a point and we want to ask the point what its x and y are. Let’s not rip the guts out, let’s do it right.
Square = class()
function Square:init(aCartesianPoint)
self.cartesianPoint = aCartesianPoint
end
function Square:point()
return self.cartesianPoint
end
I’m not entirely happy having to call that member variable cartesianPoint, but Lua won’t let me name a member variable and a function with the same name.
Do the tests run? Not yet, we didn’t pass a class to the creation.
_: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)
8: Cartesian Map -- Tests:102: attempt to call a nil value (method 'x')
Right. I didn’t implement x and y on CartesianPoint. Let’s be about doing that.
function CartesianPoint:x()
return self.xCoord
end
function CartesianPoint:y()
return self.yCoord
end
Tests run. Commit: CartesianMap creates instances of provided class.
At last, we’re making a bit of headway. I started an hour ago, and just committed seven new lines and about ten changed lines of code. Sometimes it be like that.
Now we need to do similar things with the HexMap. Let’s start with the tests this time, like the smart people do.
_:test("Hive", function()
local map = HexMap()
local h1,h2,h3
h1 = Hex(1,-1,0)
map:add(h1)
h2 = Hex(-1,0,1)
map:add(h2)
h3 = Hex(1,1,-2)
map:add(h3)
local g1, g2, g3
g1 = map:atXYZ(1,-1,0)
_:expect(g1).is(h1)
g2 = map:atPoint(HexPoint(-1,0,1))
_:expect(g2).is(h2)
g3 = map:atQR(1,-2)
_:expect(g3).is(h3)
end)
_:test("Ring 1", function()
local ringPoints = { HexPoint(1,-1,0), HexPoint(1,0,-1), HexPoint(0,1,-1),HexPoint(-1,1,0), HexPoint(-1,0,1), HexPoint(0,-1,1) }
local map = Hex:createMap(1)
for i,d in ipairs(ringPoints) do
local h = map:atPoint(d)
local err = "at "..tostring(d).." found "..tostring(h)
_:expect(h,err).isnt(nil)
end
end)
We have issues here. First of all, the HexMap doesn’t expect to create anything but the basic tables. It expects to be given objects to place in the map. That’s not entirely silly, and made sense at the time, but our new design calls for the map to create the substrate and populate it. Let’s remove the first test entirely, and beef up the ring test.
_:test("Ring 1", function()
local ringPoints = { HexPoint(1,-1,0), HexPoint(1,0,-1), HexPoint(0,1,-1),HexPoint(-1,1,0), HexPoint(-1,0,1), HexPoint(0,-1,1) }
local map = HexMap(1,Hex)
for i,d in ipairs(ringPoints) do
local h = map:atPoint(d)
local err = "at "..tostring(d).." found "..tostring(h)
_:expect(h:is_a(Hex),err).is(true)
_:expect(d:x()).is(h:point():x())
_:expect(d:y()).is(h:point():y())
_:expect(d:z()).is(h:point():z())
end
end)
I think this is about right. We’ll need some accessors in HexPoint, and we need to change how HexMap works.
Presently HexMap is nothing but a table with umpty ways to access its contents:
HexMap = class()
function HexMap:init()
self.tab = {}
end
function HexMap:add(aHex)
self:atKeyPut(aHex:key(), aHex)
end
function HexMap:atXYZ(x,y,z)
return self:atPoint(HexPoint(x,y,z))
end
function HexMap:atPoint(hexPoint)
return self:atKey(hexPoint.key)
end
function HexMap:atQR(q,r)
return self:atPoint(HexPoint(q, -q-r, r))
end
function HexMap:atKey(aKey)
return self.tab[aKey]
end
function HexMap:atKeyPut(aKey, aHex)
self.tab[aKey] = aHex
end
function HexMap:draw()
for k,hex in pairs(self.tab) do
hex:draw()
end
end
Do you remember me saying that those accessors were rather speculative? They sure as heck were. We need about one of them. Before this is over, I’ll remove the ones that are unused.
I start with this. Rather more code than I would like to write, but there we are.
function HexMap:init(radius, klass)
self.tab = {}
local centerPoint = HexPoint(0,0,0)
local centerObject = klass(centerPoint)
self:atPointPut(centerPoint, centerObject)
self.createDisc(radius, klass)
end
function HexMap:createDisc(radius, klass)
for i = 1,r do
self:createRing(r, klass)
end
end
function HexMap:createRing(radius, klass)
local points = HexPoint:createRing(radius)
for i,point in pairs(points) do
local object = klass(point)
self:atPointPut(point, object)
end
end
function HexMap:atPointPut(aPoint, anObject)
local key = aPoint:key()
self:atKeyPut(key, anObject)
end
The error is:
6: Ring 1 -- HexPoint:19: nil aHexPoint
That comes from here:
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
Right at the top. We’ve tried to add a nil to a HexPoint. The only adding we do is in creating the ring of coordinates.
function HexMap:createRing(radius, klass)
local points = HexPoint:createRing(radius)
for i,point in pairs(points) do
local object = klass(point)
self:atPointPut(point, object)
end
end
function HexPoint:createRing(radius, start)
local cell = start or HexPoint(0,0,0)
if radius == 0 then
return cell
end
cell = HexPoint:stepsFrom(cell,radius,HexDirection:direction(5))
local coords = {}
for _,direction in ipairs(HexDirection:directions()) do
for _ = 1,radius do
cell = self:oneStepFrom(cell,direction)
table.insert(coords, cell)
end
end
return coords
end
function HexPoint:stepsFrom(aHexPoint, numberOfSteps, dir)
for _ = 1,numberOfSteps do
aHexPoint = self:oneStepFrom(aHexPoint,dir)
end
return aHexPoint
end
function HexPoint:oneStepFrom(aHexPoint,direction)
return aHexPoint + direction
end
I am somewhat confused, since I don’t think I changed anything in there. WorkingCopy agrees.
I remove some redundant code for creating rings from Hex and change it to expect a Point on creation:
function Hex:init(aPoint)
self.hexPoint = aPoint
self.fillColor = nil
end
I’m just following my nose, guided by the test failures. This isn’t quite as brilliant as if I just knew what to do, but it’s a simple step-by-step process most of the time. What’s failing now?
6: Ring 1 -- HexMap:29: attempt to call a string value (method 'key')
Right forgot to put a key() method on HexPoint.
function HexPoint:init(x,y,z)
self.x = x
self.y = y
self.z = z
if self:invalid() then error("Invalid HexPointinates "..x..","..y..","..z) end
self.keyString = x..","..y..","..z
end
function HexPoint:key()
return self.keyString
end
That breaks more tests. Probably people fetching key directly.
_:test("HexPoint hashing works", function()
local tab = {}
local c1 = HexPoint(1,0,-1)
_:expect(c1:key()).is("1,0,-1")
local c2 = HexPoint(1,-1,0)
local c3 = HexPoint(1,0,-1)
tab[c1:key()] = "first"
tab[c2:key()] = "second"
tab[c3:key()] = "replaced"
_:expect(tab[c2:key()]).is("second")
_:expect(tab[c1:key()]).is("replaced")
_:expect(tab[c3:key()]).is("replaced")
end)
Right. Now we’re down to one fail:
6: Ring 1 -- HexMap:15: bad 'for' limit (number expected, got nil)
_:test("Ring 1", function()
local ringPoints = { HexPoint(1,-1,0), HexPoint(1,0,-1), HexPoint(0,1,-1),HexPoint(-1,1,0), HexPoint(-1,0,1), HexPoint(0,-1,1) }
local map = HexMap(1,Hex)
for i,d in ipairs(ringPoints) do
local h = map:atPoint(d)
local err = "at "..tostring(d).." found "..tostring(h)
_:expect(h:is_a(Hex),err).is(true)
_:expect(d:x()).is(h:point():x())
_:expect(d:y()).is(h:point():y())
_:expect(d:z()).is(h:point():z())
end
end)
Must be a flaw in HexMap:init? Not quite:
function HexMap:createDisc(radius, klass)
for i = 1,r do
self:createRing(r, klass)
end
end
So that’s entirely wrong. I edited it for “clarity”. Should have edited it for correctness:
function HexMap:createDisc(radius, klass)
for r = 1,radius do
self:createRing(r, klass)
end
end
I am amused. The message now is:
6: Ring 1 -- HexMap:15: bad 'for' limit (number expected, got table)
That says that radius above is a table. Ah:
self.createDisc(radius, klass)
The old “dot shoulda been colon” bug.
6: Ring 1 -- Tests:92: attempt to index a nil value (local 'h')
_:test("Ring 1", function()
local ringPoints = { HexPoint(1,-1,0), HexPoint(1,0,-1), HexPoint(0,1,-1),HexPoint(-1,1,0), HexPoint(-1,0,1), HexPoint(0,-1,1) }
local map = HexMap(1,Hex)
for i,d in ipairs(ringPoints) do
local h = map:atPoint(d)
local err = "at "..tostring(d).." found "..tostring(h)
_:expect(h:is_a(Hex),err).is(true)
_:expect(d:x()).is(h:point():x())
_:expect(d:y()).is(h:point():y())
_:expect(d:z()).is(h:point():z())
end
end)
The atPoint returned a nil. Let’s see how it works:
function HexMap:atPoint(hexPoint)
return self:atKey(hexPoint.key)
end
Right. Either call the method or access the member variable. Not half of one six a dozen of the other.
6: Ring 1 -- Tests:93: attempt to call a number value (method 'x')
So that’s in the expect above with d:x() in it. Either a Point doesn’t know x() or a Hex doesn’t. My money’s on Hex:
function Hex:x()
return self:point():x()
end
function Hex:y()
return self:point():y()
end
function Hex:z()
return self:point():z()
end
function Hex:point()
return self.hexPoint
end
That change involves changing a bunch of internal accesses as well. Remind me to talk about that below.
Tests pass now, but the drawing still fails.
Hex:115: attempt to index a nil value (field 'coord')
stack traceback:
Hex:115: in method 'screenPos'
Hex:48: in method 'draw'
HexMap:60: in method 'draw'
Main:31: in function 'drawMap'
Main:22: in function 'draw'
Right. No one tested that.
function Hex:screenPos()
return self.coord:screenPos()
end
We don’t have a coord any more, we have a hexPoint. Should have renamed this.
We are green and have a suitable picture.
I’ve been in here for a couple of hours. Took longer than I expected. Let’s sum up, then decide what’s next.
Summary
Someone:
Don’t forget to talk about accessors and member variables.
Thanks! Since a Lua object is just a table, it can have only one key with any given name. That means that if we have a member variable called x, we can’t have a method named x and vice versa. So when I decided to have methods x(), y() and z(), I had to rename my member variables of the same names.
And that meant that every method that referenced member variables, pretty much all of them, needed to be changed. In a more powerful IDE that would have been automated. In Codea, it’s a manual process, and one that I frequently get 90 percent right.
Even worse, it leaves me with a concern over what to name the members and accessors. In the present case I went to member is _x and accessor is x(), which I would like better had I been doing it right along. The problems with this naming thing don’t seem to hit me when I’m building a class. I suspect that they’re inconsistent, but they are small and mostly fairly clear, so I don’t get much trouble. But when I go to change a member name or method name … it gets messy.
Following a standard, such as naming all members with leading underbars, might be better for me. But here in this program, going back to change old ones would be error-prone, and not going back means that I get used to member names without underbars and follow that pattern instead of the possibly preferred underbar scheme.
I don’t see a way around this concern just now. I’ll try to use underbar members more consistently, but in this program it’s going to be inconsistent.
Oh, I forgot to commit. Let’s do. Commit: Maps now create arbitrary contents based on a provided class.
We still have a bunch of unused methods in HexMap. I’ll search for usage and remove all the unused ones now.
I was able to remove HexMap:add and atQR. So that’s slimmed down a bit. Enough for now anyway. Commit: remove add and atQR methods from HexMap.
The evil of the day …
I think this is sufficient for today. I had really thought I was ready to plug these objects into the real Dung program, but I was mistaken. This is not the first time. It’s also why even a short range estimate is often wrong.
I could have stuck to my guns and decided to put these objects into Dung this morning, with the plan to refactor in place. Given the problems I ran into, that would have been a bad idea. It could have only taken me longer to get things sorted out, especially since Dung doesn’t have any code to even create a HexMap. Changing plan was the right thing to do, and I am fortunate not to have somebody hanging over me going “You said you were going to put that stuff into D2 today, what’s wrong with you?”
There’s nothing wrong with me. Well, there probably is, but one thing that isn’t wrong is that I made a better choice about what to put into D2 and when.
I still feel kind of badly that I didn’t get the objects converted to use vectors. But it didn’t really improve much code, so probably I shouldn’t have much regret. I might look again sometime.
But tomorrow … I really think we’ll package these objects up and put them into D2 Dung. Am I right? We’ll find out together.
See you then!