Dungeon 257
Still trying to drift in a good direction. So far we’ve just barely improved, if at all.
Yesterday, I moved the creation of monsters over to the Dungeon class. It’s still triggered by the GameRunner, but that’s OK as a first step. Today, hope to move to a more visibly better shape, if I can work out what “better” might mean.
The basic idea that I have is to break out the responsibility of building the dungeon, away from either the high-level running, which I think makes sense in GameRunner, and elementary operations in the dungeon, which I plan to base in Dungeon class. I do plan to try to push some elementary operations downward, into Tile and beneath. Some matters are already down there. Anyway that’s for the future.
The shape that is vaguely coming out of the fog for me has most of the centralized operation of the game rooted in GameRunner: objects needing global things done will ask GameRunner to do them (unless they can effectively use the Bus for publish-subscribe). GameRunner may do some operations itself, or defer them to Dungeon, or elsewhere.
To move building out of GameRunner and Dungeon classes, I think I want a separate object, perhaps named DungeonMaker, that builds up the Dungeon. Today, let’s start on that. We should be able to at least get a feeling for whether we like that setup.
Where should we start?
Presently, GameRunner creates the dungeon and other objects needed in the game, in createLevel:
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)
-- paint dungeon correctly
self.dungeon:convertEdgesToWalls()
self:placePlayerInRoom1()
-- customize contents
self:placeWayDown()
self:placeSpikes(15)
self:placeLever()
--self:placeDarkness()
self:placeNPC()
self:setupMonsters(6)
self.keys = self:createThings(Key,5)
self:createThings(Chest,5)
self:createLoots(10)
self:createDecor(30)
self:createButtons()
-- prepare crawl
self.cofloater:startCrawl()
Announcer:sayAndSave(self:crawlMessages(self.dungeonLevel))
self:startTimers()
self:dcFinalize()
end
That’s a lot of stuff. There’s a big learning here. I’m sure that I knew long ago that building the dungeon and running it should be separated. But I kept adding to the single GameRunner class. Now I’m not sure which of these methods just adjusts things inside the dungeon, and which ones store variables that the GameRunner wants rapid access to.
I think it’s reasonable to say that most of these things, the loot and decor and monsters and such, all belong in the dungeon and therefore will be made by whatever finally makes the dungeon. They should probably all reside as member variables in Dungeon class … or so it seems to me now. When we see it happen, we’ll reassess, as always.
But there are things that are not really part of a given dungeon level. The buttons, and the floater, which runs the crawl, can properly belong to the GameRunner, I think.
We actually have multiple methods that create levels. There is the one above, which is the main one used in the game. There’s createLearningLevel, which is used to create, well, the learning level. That level has been developed enough to show how we might do it, but no further. And there’s createHexLevel and createTestLevel.
Something should be done about that, perhaps some kind of dungeon template that feeds into a single creation method that interprets the template and builds whatever dungeon is called for. It’s tempting to divert and do that, but while it would consolidate some code, that code would still be inside GameRunner. The template builder would be more complex than these simple linear methods. I think trying to consolidate those methods in place would not be good. Instead let’s move one to a better place and then see what to do with the others.
Let’s imagine a design and see whether we can implement something that meets the intention of that design.
What (I Think) I Want
Let’s have a new class, DungeonMaker. It creates a Dungeon instance, constituting a dungeon level, and returns it to the caller (GameRunner) who can tuck it away and run the game using it.
For this to make sense, I think the Dungeon needs to contain all the variables for running the game. Yesterday, we made the Dungeon contain the monsters collection. Let’s scan through createLevel and see if there are other GameRunner member variables being created, and if there are, move them somehow into Dungeon.
We see right away that GameRunner knows the rooms:
self.rooms = self.dungeon:createRandomRooms(count)
Dungeon could save the rooms and GameRunner could get them from Dungeon if needed. I suspect that over the course of these changes, GameRunner will not need them at all. But we need to deal with them now.
I’ll create an accessor to replace self.rooms references:
function GameRunner:getRooms()
return self.rooms
end
Now I’ll change all the self.rooms to use the accessor. I’ll spare you the code dump. Done, tested, game works, commit: GameRunner uses accessor for reference to rooms.
We could stop right now. We’ve moved a tiny step toward a better design, and if we need to work on a feature, we can. “Many More Much Smaller Steps”, as Hill tells us.
Now how is our member variable defined? There are four places:
self.rooms = self:makeRoomsFromXYWHA(t, announcements)
self.rooms = self.dungeon:createAllRoom1()
self.rooms = self.dungeon:createRandomRooms(count)
self.rooms = self.dungeon:createRandomRooms(count)
For now I don’t care about the context. I just want to move holding the rooms into Dungeon.
A glance at Dungeon tells me that it, too, has the rooms, when it creates them. In the first of the uses above, the dungeon does not create the rooms … and we don’t give them to the Dungeon. That could be bad. For now, I can remove the self.rooms = from the last three above, and give the Dungeon the rooms in the first case.
self.dungeon.rooms = self:makeRoomsFromXYWHA(t, announcements)
function GameRunner:getRooms()
return self.dungeon.rooms
end
With this in place, I think that the rooms member variable is moved to Dungeon. Test. Commit: rooms member variable moved to Dungeon from GameRunner.
Now there’s just one nasty bit here, and that’s the code above that jams rooms into dungeon. That’s rude. All the other room creations are done by Dungeon and it saves the rooms into itself, which is legit. So we want Dungeon to do this job for us.
Hold my chai.
First I change the call to this:
self.dungeon:makeRoomsFromXYWHA(t, announcements, self)
That will explode with Dungeon not understanding.
GameRunner:64: attempt to call a nil value (method 'makeRoomsFromXYWHA')
stack traceback:
GameRunner:64: in method 'createLearningRooms'
GameRunner:100: in method 'createLearningLevel'
Player:168: in field '?'
Button:79: in method 'performCommand'
Button:64: in method 'touchEnded'
Main:116: in function 'touched'
So we need for Dungeon to understand that method. Here’s one way:
function Dungeon:makeRoomsFromXYWHA(t, announcements, runner)
self.rooms = runner:makeRoomsFromXYWHA(t,announcements)
end
Now we need to make sure that GameRunner’s version returns the collection:
function GameRunner:makeRoomsFromXYWHA(roomTable)
local makeRoom = function(result,roomDef)
return result + Room:fromXYWHA(roomDef, self)
end
return ar.reduce(roomTable, Array{}, makeRoom)
end
It does. So this should work. And it does. Commit: All room collections are stored in and by Dungeon class.
“What just happened?”, you may be asking yourself.
Well, we need for Dungeon to be making all the room collections, so that GameRunner doesn’t have to jam the learning rooms into it. So we ask Dungeon to do the job. It doesn’t know how, so we pass it the runner, and it calls back to the runner, which does the job. This change simply must work: it’s hardly possible to get it wrong, even if you’re me.
Now, again, we’re at a green bar, we could wander off and build a feature or whatever. We could ship. But let’s move the method over instead, because the morning is young.
Reading the code carefully, for the first time, we notice that the announcements parameter is no longer used. That’s because we embedded the announcement, if any, in the rooms description:
function GameRunner:createLearningRooms()
local w = 12
local h = 8
local a2 = { "In this room, there is a health power-up.",
"Please walk next to it (not onto it)",
"and press the ?? button.",
"You'll get an informative message about whatever you're next to." }
local t = {
{2,2, w,h},
{15,2, w,h, a2},
...
So announcements is nil here. We can remove it from both ends, so let’s do that. And I’m getting tired of that name t as well. I must have been in some kind of hurry. One thing at a time, don’t hurry again.
self.dungeon:makeRoomsFromXYWHA(t, self)
function Dungeon:makeRoomsFromXYWHA(t, runner)
self.rooms = runner:makeRoomsFromXYWHA(t)
end
Test. Good. Commit: remove unneeded announcements parameter from makeRoomsFromXYWHA. Again, we could ship.
It seems to me that we can move all this stuff out wholesale. It’ll be easy to move it to Dungeon. I really want to get to a DungeonMaker today but that seems too big a step. Let’s move this stuff to Dungeon.
Here’s the method:
function GameRunner:makeRoomsFromXYWHA(roomTable)
local makeRoom = function(result,roomDef)
return result + Room:fromXYWHA(roomDef, self)
end
return ar.reduce(roomTable, Array{}, makeRoom)
end
It has no references to self. Doesn’t really belong here. Won’t belong to Dungeon much either but we can move it.
function Dungeon:makeRoomsFromXYWHA(roomTable, runner)
local makeRoom = function(result,roomDef)
return result + Room:fromXYWHA(roomDef, runner)
end
self.rooms = ar.reduce(roomTable, Array{}, makeRoom)
end
I guess it belongs to Dungeon more than it did to GameRunner, since we at least get to save the result.
We’re good and can commit: moved makeRoomsFromXYWHA to Dungeon.
There’s something bothering me. We have three methods in Dungeon that are saving room collections in the member variable rooms, createAllRoom1, createRandomRooms, and makeRoomsFromXYWHA. Diversion: Let’s rename that last one to create and move it to live near the others. Test, commit: rename makeRoomsFromXYWHA to create …
Now, as I was saying, three methods save into our rooms member. I’m worried that that may become confusing in the future: we just jumped through some hoops to converge references to rooms in GameRunner.
I don’t see quite what to do about that yet … and I’m after bigger game here: I want to get a DungeonMaker, so that we’ll have a place to put the methods that build a dungeon separate from the ones that support it at game time.
I’m moving toward a design like this:
I’m “sure” that I want the separate Maker object. I’m not sure whether I want GameRunner to call the Maker or Dungeon to do it. Ah. If the Maker creates the Dungeon, we have to call it from GameRunner. If it fills in the Dungeon, we could call it from Dungeon.
So I think we need to call it from GameRunner, at least when we’re done. I think we need to kind of sidle into that state, however. Let’s see …
We set up to create a new dungeon here:
function GameRunner:dcPrepare()
TileLock = false
DungeonContents = DungeonContentsCollection()
self:createNewDungeon()
local dungeon = self.dungeon
if HexMode then
dungeon:createHexTiles(self.tileCountX, self.tileCountY)
else
dungeon:createTiles(self.tileCountX, self.tileCountY)
end
dungeon:clearLevel()
Announcer:clearCache()
end
Let’s see if we can make this work with a new DungeonMaker object. We can go one of two ways here. One is to trust our existing tests and consider this to be a refactoring. The other is to create this new object using TDD, and then use it. It’s tempting to do the former, but the maker is going to be complicated. We’d better TDD it.
DungeonMaker
First test and implementation:
-- DungeonMaker
-- RJ 20220305
function testDungeonMaker()
CodeaUnit.detailed = false
_:describe("Test DungeonMaker", function()
_:before(function()
end)
_:after(function()
end)
_:test("Create maker", function()
local dm = DungeonMaker()
end)
end)
end
DungeonMaker = class()
function DungeonMaker:init()
end
That was easy. What do we want this thing to do? I think we want it to create the dungeon instance.
_:test("Create Dungeon", function()
local dm = DungeonMaker()
local dungeon = dm:createNewDungeon()
_:expect(dungeon:is_a(Dungeon)).is(true)
end)
Should fail not knowing how …
2: Create Dungeon -- DungeonMaker:21: attempt to call a nil value (method 'createNewDungeon')
function DungeonMaker:createNewDungeon()
self.dungeon = Dungeon()
return self.dungeon
end
Test passes. Commit: DungeonMaker class created. We could ship.
Let’s use the new object and method:
function GameRunner:dcPrepare()
TileLock = false
DungeonContents = DungeonContentsCollection()
self:createNewDungeon()
local dungeon = self.dungeon
if HexMode then
dungeon:createHexTiles(self.tileCountX, self.tileCountY)
else
dungeon:createTiles(self.tileCountX, self.tileCountY)
end
dungeon:clearLevel()
Announcer:clearCache()
end
OK it’s easy enough to do the create, but how will we get access to the Maker? In the fullness of time, we may be able to create it, use it, and be done with it, but for now, creation is spread over space and time. I think what we’ll do it let the Dungeon know its maker, since we call the dungeon to do things.
I’m not certain about this. Another possibility is to save the maker in GameRunner, planning to be rid of it later. Be that as it may, I think Dungeon needs to know maker at least for now.
In reviewing that, I see that Dungeon also needs to know runner. I implement, while I’m there:
function Dungeon:init(runner, maker)
self.runner = runner
self.maker = maker
-- declare unset member variables. needs improvement.
self.tileCountX = nil
self.tileCountY = nil
end
Now in the TDD:
_:test("Create Dungeon", function()
local runner = 123
local dm = DungeonMaker()
local dungeon = dm:createNewDungeon(runner)
_:expect(dungeon:is_a(Dungeon)).is(true)
_:expect(dungeon.runner).is(123)
_:expect(dungeon.maker).is(dm)
end)
function DungeonMaker:createNewDungeon(runner)
self.dungeon = Dungeon(runner,self)
return self.dungeon
end
And then here …
function GameRunner:createNewDungeon()
self.dungeon = Dungeon(self)
end
… we can do this …
function GameRunner:createNewDungeon()
self.maker = DungeonMaker()
self.dungeon = self.maker:createNewDungeon(self)
end
Tests run, game works, commit: Dungeons are created by DungeonMaker. (Just creation.)
Ship it. Well, we could. But writing the method above makes me think that the maker needs the runner throughout, at least for now, so let’s create it that way. Change the test first:
_:test("Create Dungeon", function()
local runner = 123
local dm = DungeonMaker(runner)
local dungeon = dm:createNewDungeon()
_:expect(dungeon:is_a(Dungeon)).is(true)
_:expect(dungeon.runner).is(123)
_:expect(dungeon.maker).is(dm)
end)
Test fails of course:
2: Create Dungeon -- Actual: nil, Expected: 123
Fix the class:
function DungeonMaker:init(runner)
self.runner = runner
end
function DungeonMaker:createNewDungeon()
self.dungeon = Dungeon(self.runner,self)
return self.dungeon
end
Test should run, game not so much. We need to fix the create as well:
function GameRunner:createNewDungeon()
self.maker = DungeonMaker(self)
self.dungeon = self.maker:createNewDungeon()
end
We’re good. Commit: DungeonMaker holds GameRunner instance and Dungeon instance. Ship it.
Summary
Our time is up for today. We’ve made some decent progress. We’ve moved more creation over into Dungeon, in the rough direction we have in mind, and we now have a DungeonMaker class to which we plan to move creation code. We have deferred one tiny bit of function over to it, the actual creation of the Dungeon.
Looking back, I can see that I could have done this DungeonMaker creation sooner, but at the time, I just didn’t see that as what I wanted to do. Somehow I needed to push some code around to get the feel of the terrain. So it goes. If I’d seen it sooner, probably I could have moved the XYWHA thingie into DungeonMaker with no more difficulty than I had moving it to Dungeon. Would have saved a step, maybe.
That’s OK. We do what we can, we improve what we see. We don’t have to be omniscient, we just have to pay attention as we go. To me, it’s far more important to go in tiny steps that I understand, than to do the exact right thing, especially before I understand what that is. If there ever really is an “exact right thing”. I often doubt it. There are better things. We do those. Things get better. That’s sufficient.
All in all, we’ve pushed code into a better shape, with more of the creation code centralized (in not quite the right place) and a better place prepared for it to move again.
Step by step. This is the way.
See you next time!