Drug-free, our intrepid author looks upon his works. Does he despair? (Spoiler, No, Mikey likes it!)

Still sleeping in recliner to keep from drowning overnight, but without aid of Day- or Ny-Quil or the like. I am sober this morning if not entirely clever. Someone asked me yesterday if I would reread the drug-addled article I wrote yesterday, and I said probably not. I have no choice, of course, but to reread the code.

My good friend GeePaw Hill was slacking yesterday about why some object should be named Supplies rather than Supplier, and he has a point. If he ever writes down his more detailed thoughts, perhaps I’ll reflect them here. Be that as it may, it’s better, in my view, to name an object after what it is rather than what it does. On that dimension, GameRunner might be an adequate name, but Dungeon is probably better.

As such, TileAccessor and TileFinder are perhaps not the best names ever. Perhaps the name Tiles would be better. Here are the Tiles: they can do some things.

Since the path we’re on seems to be going to leave us with the new object, presently named TileAccessor, which actually holds the Map, an indexed collection of things at x, y, [and z] coordinates, and subsuming TileFinder into that object, I suspect we should rename the new one to Tiles. That should be an easy way to start the morning.

Tiles = class()

function Tiles:getMap()
    return self.map
end

function Tiles:setMap(aMap)
    self.map = aMap
end

function Tiles:tilePairs()
    return self.map:pairs()
end

The Tiles can’t even return a tile yet. That’s interesting, but it’s because we’re doing the getting with the TileFinder. We’ll get there, I reckon.

Now I’ll change the references to TileAccessor and its lower-case equivalents.

function Dungeon:init(runner, tileCountX, tileCountY)
    self.runner = runner
    self.tileCountX = tileCountX or assert(false, "must provide tile count")
    self.tileCountY = tileCountY or assert(false, "must provide tile count")
    self.map = nil -- set in createTiles
    self.rooms = Array{}
    self.tiles = Tiles()
    Bus:subscribe(self,self.nearestContentsQueries, "requestinfo")
end

function Dungeon:getMap()
    return self.tiles:getMap()
end

function Dungeon:setMap(aMap)
    self.tiles:setMap(aMap)
end

function Dungeon:tilePairs()
    return self.tiles:tilePairs()
end

This should all work, so I won’t even test.

Hahaha of course I’ll test. We’re good. Commit: Rename TileAccessor to Tiles.

Now what about TileFinder? Let’s review it before deciding its fate. That seems like the smart thing to do.

TileFinder = class()

function TileFinder:init(dungeon)
    self.dungeon = dungeon
end

function TileFinder:getTile(x,y)
    return self.dungeon:privateGetTileXY(x,y)
end

function TileFinder:asTileFinder()
    return self
end

I believe that where we want to wind up is with all uses of TileFinder replaced with Tiles. I know there is some intricacy bound up in the timing of creation of the dungeon, the map, and the tiles (and Tiles), so I’m a bit afraid of doing it all in one go.

One possible smaller step would be to init the TileFinder with a Tile instead of a Dungeon, and make that work. Then it should be relatively easy to replace TileFinder references with Tiles.

Yes. I’m not sharp enough for big bites today. (Or any day, probably, but especially not today.) Let’s do that.

I recast TileFinder thus:

function TileFinder:init(dungeon)
    self.tiles = dungeon:tiles()
end

function TileFinder:getTile(x,y)
    return self.tiles:getTileXY(x,y)
end

This should be enough to drive out some behavior. Test.

TileFinder:133: attempt to call a table value (method 'tiles')

OK, good, that’s what I wanted. But, dammit, I can’t have a method named tiles and a member named tiles. Have to use get I guess.

function TileFinder:init(dungeon)
    self.tiles = dungeon:getTiles()
end

function Dungeon:getTiles()
    return self.tiles
end

Test.

TileFinder:137: attempt to call a nil value (method 'getTileXY')

As expected. We crib the method from here:

function Dungeon:privateGetTileXY(x,y)
    return self:getMap():atXYZ(x,0,y) or Tile:edge(x,y, self:asTileFinder())
end

So …

function Tiles:getTileXY(x,y)
    return self.map:atXYZ(x,0,y) or Tile:edge(x,y,self)
end

Test, expecting things to work but open to reality. Fortunately, because:

Tile:92: attempt to call a nil value (method 'asTileFinder')

Sure. That’s coming from here:

function Tile:init(mapPoint,kind, dungeonOrTileFinder)
    assert(mapPoint:is_a(MapPoint), "Tile requires MapPoint")
    self.mapPoint = mapPoint
    self.kind = kind
    self.tileFinder = dungeonOrTileFinder:asTileFinder()
    self:initDetails()
end

Well, I reckon since Dungeon and TileFinder both know asTileFinder, we must have a Tiles showing up here. Let’s assume that …

function Tiles:asTileFinder()
    return self
end

If this doesn’t work, I’ll have to think. I’m hoping that’s not necessary. It isn’t. Tests and game run. I think we can commit: TileFinder now uses Tiles rather than dungeon connection.

Now we compare the protocol of TileFinder and Tiles, to see about just using Tiles instead of TileFinder:

Tiles = class()

function Tiles:asTileFinder()
    return self
end

function Tiles:getMap()
    return self.map
end

function Tiles:getTileXY(x,y)
    return self.map:atXYZ(x,0,y) or Tile:edge(x,y,self)
end

function Tiles:setMap(aMap)
    self.map = aMap
end

function Tiles:tilePairs()
    return self.map:pairs()
end

TileFinder = class()

function TileFinder:init(dungeon)
    self.tiles = dungeon:getTiles()
end

function TileFinder:getTile(x,y)
    return self.tiles:getTileXY(x,y)
end

function TileFinder:asTileFinder()
    return self
end

Well, there’s not much there. We got to this early, which is good. There, however, we see our ongoing confusion over whether getTile is taking an x,y or something else. At this level, it appears we only need the x,y version, since it’s the only one we have implemented. So I can rename the getTileXY in Tiles to getTile. I’ll test that in situ first.

All is well. Now then. Let’s find all the creators of TileFinder and make them create Tiles instead. There are a lot of them. This might be tricky. We’ll see. It might be straightforward if we don’t rename any methods yet.

function GameRunner:tileFinder()
    return self:getDungeon():asTileFinder()
end

Becomes:

function GameRunner:tileFinder()
    return self:getDungeon():getTiles()
end

function GameRunner:tileFinder()
    return self:getDungeon():getTiles()
end

This is sloppy. Should be converged. We’ll get there. For now, forward to the other one:

function GameRunner:tileFinder()
    return self:asTileFinder()
end

It seems to me that if we have duplicate methods and can’t fix up the senders right now, we should at least forward all the dups to the same single functional method.

There are zillions of calls to GameRunner’s asTileFinder and tileFinder methods, mostly in tests. They are fine, we’ve fixed them at the root. Here’s one that matters:

function Dungeon:asTileFinder()
    return TileFinder(self)
end

Here we do the thing:

function Dungeon:asTileFinder()
    return self:getTiles()
end

All the rest are references. There are a lot of methods thinking “finder” when I now want them to be thinking “tiles”, but I think we’ve got them all wired to Tiles now. Test to find out why I’m not as right as I might have been.

Well, there’s an assert, but I’m not sure why Codea’s Find didn’t show it. Probably I just didn’t notice.

function Room:init(x,y,w,h, tileFinder, paint, announcement)
    assert(tileFinder:is_a(Tiles), "Room requires a Tiles instance")
    self.tileFinder = tileFinder
    if paint == nil then paint = true end
    self.x1 = x
    self.y1 = y
    self.x2 = x + w - 1
    self.y2 = y + h - 1
    if paint then
        self:paint()
    end
    if announcement then
        self:announce(announcement)
    end
end

All better there. But a lot of tests are failing as well, I think.

Yes, lots of tests fail. In trying to create the Learning dungeon, I get this convenient error:

GameRunner:372: attempt to call a nil value (method 'asTileFinder')
stack traceback:
	GameRunner:372: in method 'tileFinder'

Hmm. That’s this:

function GameRunner:tileFinder()
    return self:asTileFinder()
end

All the test failures are also triggered by this same failure. Well, darn. I was sure there was an asTileFinder in GameRunner. There isn’t. Somehow I confused myself. Anyway:

function GameRunner:tileFinder()
    return self:getDungeon():getTiles()
end

I think that should fix everything. It does. Let’s commit and then remove TileFinder class and commit again. First: Former users of TileFinder all get Tiles.

Now remove the class and test. We’re good. Commit: TileFinder class removed, tab renamed to Tiles.

Time to reflect, and maybe stop.

ReStrospective

I’m still not at my best today, fighting whatever dread disease has attacked me. So I might call it a day, but let’s see what has happened over the past few sessions.

At first we (well, let me take the blame, I) created the TileFinder Façade to provide access to the tiles of the Dungeon, allowing us to pass a TileFinder instead of a whole dungeon around. This was a matter of clarity, because it is clear at a glance what an object given a TileFinder might do, and it is far from clear what someone might do if they had their hands on an entire dungeon.

Then yesterday, despite the Nyquil hangover, I thought that instead of having the TileFinder own an entire Dungeon, we could instead have a similar object that just held the map, defer to it in Dungeon, and hand it around instead of TileFinder to others needing Tile access. That caused me to create the TileAccessor object yesterday.

Today, I renamed the TileAccessor to Tiles, representing what it is rather than what it does. Then we (thanks for rejoining me) converted TileFinder to use Tiles, and then removed the now unused TileFinder.

This may seem wasteful. Why wasn’t I (oh, sure, step away when the trouble starts) smart enough to go to Tiles all in one go. It’s simple enough, I certainly could have.

I guess I’d agree that going from Dungeon to TileFinder to TileAccessor to Tiles to removing TileFinder is 20 or 30 more lines of typing than going from Dungeon to Tiles all in one go, and I don’t know whether I’d have had such an easy time of it, only shorter, or not.

What I do know is that each of these tiny steps made sense at the time, and the resulting code was easy to morph into the shape I wanted. If I got paid, I’d get paid for making things work, not for big leaps of understanding. And, based on many of my big leaps of understanding, I shouldn’t be paid for those, because so many of them wind up in tears, anger, and long debugging sessions.

We were in a not-so-good place. We moved to a better place. We moved again, again, again, each time to a somewhat better place.

In my view, that is the way.

See you next time!


D2.zip