Dungeon 175
Still making a place for our new designed dungeon capability to stand. What I’m about to do may surprise you, and raise questions.
It’s 0853, the cat is fed and on the deck. It’s clear and sunny and Radio Margaritaville is on the HomePod. I have a hand-crafted iced chai by my side, and a banana to stave off starvation. Life is good.
Time to talk about something that may strike the reader as odd. Or wrong.
Yesterday, we moved a half-dozen or so methods from GameRunner to Dungeon, with at least two goals in mind. We want to reduce GameRunner’s responsibilities, and in the specific case, we want to move dungeon generation out of the GameRunner class. It went well, with no real confusion, and just one revert. I still don’t know what was wrong the first time. I just reverted and did it again.
Now Dungeon’s interface has grown by a half-dozen methods or so, and, so far, GameRunner hasn’t quite shrunk by that same amount. There are still things that it mediates, though it does delegate the work to Dungeon.
But Dungeon now has one more responsibility almost complete: building the dungeon. What we “really” want is for dungeon building to be embodied in a class of its own (or maybe even a few builder classes), not to be part of Dungeon, which has its own interests.
So, probably not today, maybe not until Tuesday, but soon, I’m planning to move all the stuff that we just moved into Dungeon, into a new class, called something like DungeonMaker.
What’s up with that? Why the hell would you go to the effort of moving a bunch of methods from one class to another, only to move them again a few days later? Isn’t that wasteful, redundant, too much, and excessive?
Well, yes–and no. Yes, because certainly it is two steps instead of one, and all things being equal, one step would be better.
An analogy comes to me. If we are going from the first floor to the second floor of some building, we often find that there are two flights of stairs to get there. isn’t that wasteful, redundant, too much, and excessive? Generally, it is not. A single flight would often take up too much space, or be so steep as to be unsafe, or requiring very tall steps instead of standard height. Two flights is better.
And two steps is better here, in my opinion. And I have two reasons for thinking that. One of them is quite specific: I tried going in a single step only a day before, and it didn’t go well. I started to get tangled up in ideas and to lose the thread. It was too hard. The other reason is general: If we have two classes A and B that are not properly sharing responsibilities, and we add in a new, not yet created class C, all of our decisions are complicated by the new inter-class relations. We used to have just one: A::B. Adding in C, we now have A::B, B::C, A::C, three times as many relationships to think about.
So, yes, I fully intend to move a number of methods from Dungeon to DungeonMaker, having just moved them from GameRunner to Dungeon a day or so before.
I expect it to be easier. My strong preference is to make small steps. Yesterday was a super example of that, where we did nine commits and a revert in about two hours. I’m not saying that to blow my own horn, just to point out that we happened to go in tiny steps, moving a method or sometimes two, inch by inch, step by step, and it went well.
I like it when things go well. Let’s see what we might do today.
Let’s Find Something Easy
It’s Saturday, after all, no sense working too hard. That said, I can imagine a move that might be valuable, but that could be troublesome.
Here’s GameRunner createLevel:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createNewDungeon()
self:createTiles()
self:clearLevel()
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
self:placePlayerInRoom1()
self:placeWayDown()
self:placeSpikes(5)
self:placeLever()
self:setupMonsters(6)
self.keys = self:createThings(Key,5)
self:createThings(Chest,5)
self:createLoots(10)
self:createDecor(30)
self:createButtons()
self.cofloater:runCrawl(self:initialCrawl(self.dungeonLevel))
self:startTimers()
self.playerCanMove = true
TileLock = true
end
It takes a lot to build a level. Now some of those methods now forward to Dungeon:
function GameRunner:createTiles()
self.tiles = self:getDungeon():createTiles(self.tileCountX, self.tileCountY)
end
function GameRunner:clearLevel()
self:getDungeon():clearLevel()
end
function GameRunner:connectRooms()
self:getDungeon():connectRooms(self.rooms)
end
function GameRunner:convertEdgesToWalls()
self:getDungeon():convertEdgesToWalls()
end
Now, our mission here is to move all the dungeon creation out of GameRunner. Over time, almost all those self: calls back into GameRunner will want to be calling other objects, be they DungeonMaker, Dungeon, or Monsters. And there will probably be new objects, serving DungeonMaker, to make buttons or whatever.
So I am tempted to move createLevel out of GameRunner this morning. I can see roughly how to do it, by calling back to GameRunner when it still holds that method, moving the methods one at a time. I can see two ways to go.
First, we could move all this over to Dungeon, much as we moved the other methods yesterday. I think that’ll be pretty straightforward, though there will be wiring to do.
However, this could be a chance to start the DungeonMaker class, by starting it off with just one method, createLevel. Despite thinking just minutes ago that I’d defer making DungeonMaker for a few days, this does seem like a decent chance to do it. I’m tempted.
In a sense it doesn’t matter where we move it, to Dungeon or DungeonMaker. The work will be about the same. But there are two paths we could follow.
Suppose that in GameRunner:createLevel we have an instance of Dungeon and/or DungeonMaker in hand. We could go through createLevel one method at a time. Let me demonstrate by doing it:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createNewDungeon()
self:createTiles()
self:clearLevel()
self:createRandomRooms(count)
...
Instead of having a forwarding method createTiles in GameRunner, we can do this:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createNewDungeon()
local dungeon = self.dungeon
self.tiles = dungeon:createTiles(self.tileCountX, self.tileCountY)
dungeon:clearLevel()
self.rooms = dungeon:createRandomRooms(count)
dungeon:connectRooms(self.rooms)
dungeon:convertEdgesToWalls()
self:placePlayerInRoom1()
...
This amounts to no more than inlining those five methods, which we can now remove. Removing them breaks a few tests, which were calling Runner:createTiles, but there were only two lines needing changes, both to this:
Runner.dungeon:createTiles(Runner.tileCountX, Runner.tileCountY)
All is working. Commit: GameRunner:createLevel directly delegates to Dungeon.
We need to make these same changes to our createLearningLevel method. It’s not in use just now, but we’d best keep it in sync. (Discussion to follow.)
function GameRunner:createLearningLevel()
self.dungeonLevel = 1
TileLock = false
self:createNewDungeon()
local dungeon = self.dungeon
self.tiles = dungeon:createTiles(self.tileCountX, self.tileCountY)
dungeon:clearLevel()
self:createLearningRooms()
self:connectLearningRooms()
dungeon:convertEdgesToWalls()
self.monsters = Monsters()
self:placePlayerInRoom1()
...
Commit: update createLearningLevel to properly delegate to Dungeon.
Earlier, way up there, I mentioned two ways to go. Clearly I’ve committed to the first one, which is to change the methods inside createLevel to forward over to Dungeon as appropriate. The further we go with this, the more the method displays “feature envy”, the hint that if you’re sending a million messages to another object to get one thing done, that object needs a single method that does those million things. So the pressure to move createLevel grows until we finally do it.
The second way is to recognize that that’s what’s going to happen, and move the createLevel method first. We could even move it to a new class, knowing that it’s supposed to wind up there anyway.
The second way is harder, though it seems more direct. Why? Because all those self:thisAndThat calls have to be changed to call back over to GameRunner. Oh, it’s not a lot harder. We could probably get it nearly working by just bulk replacing self:foo with self.gameRunner:foo.
Either way is viable in this case. There are probably cases to strongly prefer one or the other. I didn’t decide between them. Instead, I wanted to demonstrate the inside-out approach, and having done it, I’m inclined to stick with it until we get around half the methods calling dungeon and half not. I suspect that that will be a great time to move the create method to a new class, and then bring over the helper methods from Dungeon.
He’s not exactly designing here, is he?
It would be possible to plan all this out. We could draw some kind of UML diagram and decide what methods remain on GameRunner, which ones go to Dungeon, which ones to DungeonMaker, and which ones go to other classes which may or may not exist. In other situations, I might recommend at least a rough plan for this kind of thing. Maybe a diagram on the team whiteboard showing the status and some arrows saying where things wanted to go. I know that other experts would recommend doing a full design of a thing like this, with a formal diagram and so on.
I do not favor that sort of thing for most cases. Certainly if the product is large enough, and the team large enough, there might be a need for such a thing. I used to like to do diagrams like that. What I discovered was that when my teams and I did them, they were never right. The code didn’t want to be the way the diagram said. In my view, when a plan and reality conflict, the plan is wrong.
But here and now, I prefer to proceed with lots of thinking, but few committed design plans, allowing the code to tell me what it would like to do. I do this for two good reasons. First, I enjoy the pleasant sense of just nudging the code into better shape.
Second, I think that many teams do not get a lot of time to figure out grand designs, and working incrementally, listening to the code, may be better for them. So I like to work that way, to give folks a sense that it’s possible, even easy, to get to good designs bit by bit.
What Now?
As I was doing this, I got an idea. What if we had a button on the screen that, if you push it, tosses you into the learning Level. That would be “like” a feature, and it would mean that I wouldn’t have to patch the source code to create a learning level when I work on it.
I think I’ll repurpose the “Fight” button, which doesn’t do anything but allow a monster move cycle.
No, that will confuse my user. I’ll add it to the right of Fight.
function GameRunner:createCombatButtons(buttons)
table.insert(buttons, Button:textButton("??", 100,350))
table.insert(buttons, Button:textButton("Fight",200,350))
table.insert(buttons, Button:textButton("Learn", 300, 350))
end
That looks good. Let’s see if we can make it do something.
Buttons send their name, in lower case, to the player. I think we’d like to change that. But let’s see if we can forward the message somehow. The button should crash looking for learn on player:
Button:59: attempt to call a nil value (field '?')
stack traceback:
Button:59: in method 'performCommand'
Button:54: in method 'touched'
GameRunner:498: in method 'touched'
Main:65: in function 'touched'
Well, not a helpful diagnostic but that’s what’s going on.
We’ll start with the naive implementation:
function Player:learn()
self.runner:createLearningLevel()
end
I suspect this won’t quite work, but it might. Wow, it does:
So that’s done, or at least done enough. Commit: New learn button starts learning level.
Let’s sum up. That was so easy I want to stop while I’m ahead. And it is 1032, so we’re coming up on two hours, and I still have to write the summary.
Summary
We’re taking a sort of “Strangler” approach to moving the dungeon-making code out of GameRunner, by moving individual creation methods out, and then delegating to them inline in the base-level creation methods.
“Inline” is important. Yesterday, I was changing the individual methods to delegate, which allowed me to leave the base level creators alone. But that builds up a growing set of methods that do little more than defer to the new class, keeping the GameRunner class still polluted with concepts that we’re trying to move elsewhere.
So, today, I think that finding references to moved methods and changing them is better than keeping them as separate forwarding methods. This way, GameRunner slowly gets simpler. That’s a good thing.
I chose to leave the base methods in GameRunner. Sooner or later, the feature envy will get so strong that we’ll pull them over to the Dungeon or a new class. We might still leave forwarders there for those base creation methods, since the GameRunner probably should have the final say on when we can create a new level.
We’ll be guided, however, by looking at who knows what and when they know it. We could conceivably wind up with GameRunner suddenly finding itself running a new dungeon without any concern at all. Much of the dungeon’s state information is in the Tiles. It might be quite nice if GameRunner wasn’t saving any game state at all.
We’ll keep that in mind as we go forward. I want to touch for a moment on what GeePaw Hill refers to as “The City on the Hill”. I think he’ll agree with what I say here:
We often have some grand goal that we’re shooting for, a beautiful city on the hill, such that if we fight bravely, and our hearts are pure, one day we’ll get there and live in glory.
Yeah, no. We do want to have some big visions of where we’re heading, of really nice, clean, elegant design in our system. And we should always move toward that fine City whenever we can. It’s just that we should never expect to “get there”.
The fact is, we’re always building the City. We build parts of it, and we build them as well as we can. And, since we are human, and we have finite skills and finite time, there will always be places that aren’t quite right. There will be dark alleyways where we should never go alone. There will be ramshackle buildings that need renovation or to be torn down and replaced.
But every day, we can make our City brighter and more nearly glorious than the day before. And that is the work of the software developer, whether the City is our product, our Making App, our team dynamic, or ourselves.
Step by step, inch by inch, slowly we turn our world into a better one.