Dungeon 291
What do we do when we find two classes that are rather similar? We make them more similar, of course! (That’’s not what happens.)
We are engaged in setting up the Dung program so that it is “easy” to be more specific about what kinds of things can be found in the dungeon. We want to control how many of each item can be found, and probably to control what kinds of decor are found, what dangers there are, and what things go in which rooms. There’s probably no limit to what people will ask for once we start giving them any control at all.
I currently have 19 or 20 cards (yellow sticky notes) listing things that need doing. I’m sure that a few of them are done, but most of them are not. So today I plan to work on one of those notes:
Chest -> Decor
The Chest class and Decor class are quite similar. Take a look. Here’s Chest:
Chest = class(DungeonObject)
function Chest:init(tile, loot)
tile:moveObject(self)
self.closedPic = "mhide01"
self.openPic = asset.open_chest
self.pic = self.closedPic
self.opened = false
self.loot = loot
end
function Chest:draw(tiny, center)
if tiny then return end
sprite(Sprites:sprite(self.pic),center.x,center.y, 96,127)
end
function Chest:isOpen()
return self.opened
end
function Chest:open()
if self:isOpen() then return end
self.opened = true
self.pic = self.openPic
self:currentTile():moveObject(self.loot)
end
function Chest:query()
Bus:informDotted("I am probably a mysterious chest.")
end
function Chest:zLevel()
return 1
end
And here’s Decor:
function Decor:init(tile, loot, kind)
self.kind = kind or Decor:randomKind()
self.sprite = DecorSprites[self.kind]
if not self.sprite then
self.kind = "Skeleton2"
self.sprite = DecorSprites[self.kind]
end
self.loot = loot
tile:moveObject(self)
self.scaleX = ScaleX[math.random(1,2)]
local dt = {self.doNothing, self.doNothing, self.castLethargy, self.castWeakness}
self.danger = dt[math.random(1,#dt)]
self.open = false
end
function Decor:__tostring()
return "Decor"
end
function Decor:actionWith(aPlayer)
if not self.open then
self.open = true
self:currentTile():moveObject(self.loot)
local round = CombatRound(self,aPlayer)
self.danger(self,round)
end
end
function Decor:castLethargy(round)
local msg = "Fumes! A feeling of lethargy comes over you!"
local dam = math.random(1,5)
self:damage(round,msg,dam,"_speed")
end
function Decor:castWeakness(round)
local msg = "A poison needle! Suddenly you feel weakened!"
local dam = math.random(1,5)
self:damage(round,msg,dam,"_strength")
end
function Decor:damage(round, message, damage, attribute)
round:appendText(message)
round:applyDamage(damage, attribute)
round:publish()
end
function Decor:doNothing()
end
function Decor:draw(tiny, center)
if tiny then return end
pushMatrix()
translate(center.x, center.y)
scale(self.scaleX, 1)
sprite(self.sprite,0,0, 50,50)
popMatrix()
end
function Decor:getDecorKeys()
local keys = {}
for k,v in pairs(DecorSprites) do
table.insert(keys, k)
end
return keys
end
function Decor:giveItem()
if self.item then
self.item:addToInventory()
self.item = nil
end
end
function Decor:randomKind()
return DecorKinds[math.random(1,#DecorKinds)]
end
function Decor:queryMessagesPrivate()
local answers = DecorMessages[self.kind] or {"I am junk.", "I am debris", "Oh, just stuff.", "Well, mostly detritus, some trash ..."}
return answers[math.random(1,#answers)]
end
function Decor:query()
Bus:informDotted(self:queryMessagesPrivate())
end
function Decor:zLevel()
return 1
end
Now, at first glance, those may not seem very similar, but at a closer look, it seems that everything in Chest has an equivalent in Decor. One difference is that Chest does its thing when sent open, and Decor is sent actionWith.
The Morning’s Question
Today’s question, as shown in the blurb, is
What do we do when we find two classes that are rather similar? We make them more similar, of course!
Now some of us, on a given day, might answer differently. Things we might choose to do include:
- Make a common superclass and move same stuff up;
- Merge the one into the other;
- Write a new class that can do what each of the others can do.
I have come to believe that when two batches of code are similar, the thing to do is to make them more and more similar, and when they are similar enough, eliminate the duplication.
I still recall, at a very early XP Immersion class, in the early 2000s, when, faced with two methods that were vaguely similar, Kent Beck made them identical by making the simpler of the two more complex than it needed to be. Then he removed the duplication, making the code far more simple and clear. All were amazed. I often wish that we had a video of the process.
I don’t suppose what we’ll see here will be amazing, but I expect it to be interesting. Let’s get started.
- Spoiler:
- What happens is very different from what I planned. It quickly comes to me that there is something easier than making the two classes alike.
Make Chest More Like Decor
I think we’ll do most of our changes to Chest, but I don’t know that, and I’m not committed to it. We’ll start with Chest because it’s simpler. Thinking of where we’re going, it is a simple “kind of decor”, in that it doesn’t do damage or some of the other things that Decor does. Perhaps it should.
I do think that giveItem in Decor is probably no longer used. I’ll do a quick search for that.
It’s only used in that ignored test. I’ll remove the method and deal with the test later. It’ll keep irritating me until I do.
Let’s change Chest to use actionWith rather than open.
function Chest:actionWith(ignoredPlayer)
if self:isOpen() then return end
self.opened = true
self.pic = self.openPic
self:currentTile():moveObject(self.loot)
end
I must now change what the Player does. That’s in startActionWithChest, if I recall.
function Player:startActionWithChest(aChest)
aChest:open()
end
Change that:
function Player:startActionWithChest(aChest)
aChest:actionWith(self)
end
Test. We’re good. Commit: Player sends actionWith to Chest, as to Decor.
So far so good … except …
Change of Plan
You know what? Now that I’m here, I think I have a better idea.
Decor has lots of different kinds of decor, which are defined in a couple of tables:
local DecorKinds = {
"Skeleton1","Skeleton1","Skeleton1","Skeleton1","Skeleton1",
"Skeleton2","Skeleton2","Skeleton2","Skeleton2","Skeleton2",
"BarrelEmpty",
"BarrelClosed",
"BarrelFull",
"Crate",
"PotEmpty",
"PotFull",
}
local DecorSprites = {
Skeleton1=asset.Skeleton1,
Skeleton2=asset.Skeleton2,
BarrelEmpty=asset.barrel_empty,
BarrelClosed=asset.barrel_top,
BarrelFull=asset.barrel_full,
Crate=asset.Crate,
PotEmpty=asset.pot_empty,
PotFull=asset.pot_full,
}
(There’s a third table full of clever messages indexed by the Decor type. “Yorick, is that you??” and so on.)
What if we were to add Chest to this list? Wouldn’t it just work, except for the detail of not opening up?
Let’s do that. I comment out all the other choices, so we’ll get nothing but Chests:
local DecorKinds = {
--[["Skeleton1","Skeleton1","Skeleton1","Skeleton1","Skeleton1",
"Skeleton2","Skeleton2","Skeleton2","Skeleton2","Skeleton2",--]]
"Chest",
--[["BarrelEmpty",
"BarrelClosed",
"BarrelFull",
"Crate",
"PotEmpty",
"PotFull",--]]
}
local DecorSprites = {
Skeleton1=asset.Skeleton1,
Skeleton2=asset.Skeleton2,
Chest=asset.Chest,
BarrelEmpty=asset.barrel_empty,
BarrelClosed=asset.barrel_top,
BarrelFull=asset.barrel_full,
Crate=asset.Crate,
PotEmpty=asset.pot_empty,
PotFull=asset.pot_full,
}
Run. Let’s see what breaks. Well that’s interesting. Take a look.
The Chests are created, but they display with odd pictures. I’ve seen them look like the direction buttons, and small heads, and small rocks. That aside, it seems to work.
I decide that this is promising. Looking into the Chest class, I find that it says this:
function Chest:init(tile, loot)
tile:moveObject(self)
self.closedPic = "mhide01"
self.openPic = asset.open_chest
self.pic = self.closedPic
self.opened = false
self.loot = loot
end
Let me put that string into Decor and see if that works better. I have some doubt.
local DecorSprites = {
Skeleton1=asset.Skeleton1,
Skeleton2=asset.Skeleton2,
Chest="mhide01",
BarrelEmpty=asset.barrel_empty,
BarrelClosed=asset.barrel_top,
BarrelFull=asset.barrel_full,
Crate=asset.Crate,
PotEmpty=asset.pot_empty,
PotFull=asset.pot_full,
}
Try that. I get this warning, and still a weird display.
Asset keys (e.g. "Documents:MySprite") are deprecated. Please use static assets, e.g., sprite(asset.documents.mySprite, ...) instead.
OK, time to get serious. Compare the draw functions:
function Chest:draw(tiny, center)
if tiny then return end
sprite(Sprites:sprite(self.pic),center.x,center.y, 96,127)
end
function Decor:draw(tiny, center)
if tiny then return end
pushMatrix()
translate(center.x, center.y)
scale(self.scaleX, 1)
sprite(self.sprite,0,0, 50,50)
popMatrix()
end
Let’s see what happens if we use the Sprites trick. The Chests now appear, but they are half size. No surprise, we used size 50,50 instead of the 96,127 we use for Chest.
There is also the matter of the scaling: The Decor can swap X to -X to make the Decor appear flipped, making for more variation.
That aside, the treasure does appear and is given to us.
Let’s create a new table of info in Decor, the scale of the picture. We’ll just have one new value in it, for Chest.
As I did it, I decided not to build the table yet, as there is just one case:
function Decor:draw(tiny, center)
if tiny then return end
pushMatrix()
translate(center.x, center.y)
scale(self.scaleX, 1)
local sx,sy = self:setSize(self.kind)
sprite(Sprites:sprite(self.sprite),0,0, sx,sy)
popMatrix()
end
function Decor:setSize(aKind)
if aKind == "Chest" then
return 96,127
else
return 50,50
end
end
This should tell me whether it works. It looks good, though I confess I wonder what the natural ones look like. Anyway:
The new chests work, showing and giving their items. They do not, of course, open up. Let’s make that happen.
function Decor:actionWith(aPlayer)
if not self.open then
self.open = true
self:currentTile():moveObject(self.loot)
local round = CombatRound(self,aPlayer)
self.danger(self,round)
end
end
This can become this:
function Decor:actionWith(aPlayer)
if not self.open then
self.open = true
self.sprite = self:openSprite(self.kind)
self:currentTile():moveObject(self.loot)
local round = CombatRound(self,aPlayer)
self.danger(self,round)
end
end
function Decor:openSprite(aKind)
if aKind == "Chest" then
return asset.open_chest
else
return self.sprite
end
end
Try that. Works a treat!
The chests are appearing randomly flipped in X, as do other Decor:
We may wish to turn that off. If we do, it’ll be done by changing this code:
function Decor:init(tile, loot, kind)
self.kind = kind or Decor:randomKind()
self.sprite = DecorSprites[self.kind]
if not self.sprite then
self.kind = "Skeleton2"
self.sprite = DecorSprites[self.kind]
end
self.loot = loot
tile:moveObject(self)
-- change the following line to avoid flipping Chests
self.scaleX = ScaleX[math.random(1,2)]
local dt = {self.doNothing, self.doNothing, self.castLethargy, self.castWeakness}
self.danger = dt[math.random(1,#dt)]
self.open = false
end
For now I think we’ll let it ride. Maybe the player would discover that one kind of Chest is guaranteed not to be a Mimic. Let’s clean this up by adjusting the starting table to allow all our different types again, and a few more chests, because we like them.
local DecorKinds = {
"Skeleton1","Skeleton1","Skeleton1","Skeleton1","Skeleton1",
"Skeleton2","Skeleton2","Skeleton2","Skeleton2","Skeleton2",
"Chest","Chest","Chest","Chest",
"BarrelEmpty",
"BarrelClosed",
"BarrelFull",
"Crate",
"PotEmpty",
"PotFull",
}
I’ve already commented out the placeChests function:
function DungeonBuilder:customizeContents()
self:placeSpikes(15)
self:placeLever()
--self:placeDarkness()
self:placeNPC()
self:placeLoots(10)
self:placeDecor(30)
self:placeThings(Key,5)
--self:placeChests(8)
self:placeWayDown()
self:setupMonsters()
end
Let’s remove that line, and the function. Test. It’s all good. Dare we remove the Chest class? Look for references. There are a bunch of tests. No other references. The tests mostly test the TileArbiter move logic.
I’m going for it. Let’s first commit, for a save point: Chests created as Decor. No more Chest objects in Dungeon. Tests remain.
Now let’s rip. I remove all the tests that check to see if you can step onto a Chest. This is a bit dangerous as there will now be fewer tests for TileArbiter. I’ll make a card.
TileArbiter needs more tests for entry
The TileArbiter createTable has this at the top:
function TileArbiter:createTable()
-- table is [resident][mover]
if TA_table[Chest] then return end
local t = {}
t[Announcer] = {}
t[Announcer][Monster] = {moveTo=TileArbiter.acceptMove}
t[Announcer][Player] = {moveTo=TileArbiter.acceptMove, action=Player.startActionWithAnnouncer}
That’s just checking to see if the table has contents, to avoid creating it multiple times. I’ll change that to reference Decor, and remove the Chest-related lines:
t[Chest] = {}
t[Chest][Monster] = {moveTo=TileArbiter.refuseMove}
t[Chest][Player] = {moveTo=TileArbiter.refuseMove, action=Player.startActionWithChest}
Now the only occurrences of “Chest” are in the Chest class, and the word “Chest” which is used in Decor but not referring to the class. I’m going to test once more before removing the Chest class. All seems good. Commit: No remaining references to Chest class.
Remove Chest tab. Test. All good. Commit: Remove Chest class tab.
Let’s sum up.
Summary
The big question now is how much am I going to go back and edit this article so that it looks like I did what I intended to do. The answer is, not at all, but I am going to back and put in some foreshadowing for you, my faithful reader. And for you also, my unfaithful reader.
I was really looking forward to making Chest and Decor look alike, and expected that at that point, folding the creation of Chests into Decor would be easy. But instead, upon looking at Chest and Decor, it seemed to me that Decor was quite close to supporting the Chest, except for the bit about opening.
It seemed to make sense to switch to a simpler plan, which amounted to extending Decor to support Chest logic.
I think things would have gone similarly with the “remove duplication” idea, where we might have looked at each common method and made them the same, until all the methods of Chest were duplicated in Decor.
But Decor was so close, since it’s basically a picture containing a bit of Loot, that just adding Chest into it seemed simpler.
At a guess it took about the same number of steps, since in the end we had to modify all the same notions, the picture, the new “open” picture, the scale, and so on. But the flow of events was different. Instead of ticking through the methods of the two classes, I was driven by seeing what didn’t work and making it work.
You could argue that it was more ad hoc the way I did it, and I wouldn’t deny it. I was driven by visible failures rather than test failures. Had I been driven by test failures, we’d all nod wisely about TDD. Driven by visible things like tiny chests and such, we are more inclined to look askance.
All that said, it went quite smoothly. I don’t recall being confused at any point. I do confess to being pleasantly surprised when the Sprites:sprite call just worked, and someday maybe I’ll look to see why. But if it worked in Chest it seemed it had to work in Decor, and it did.
The overall design of Decor seems to me to be a bit questionable. In particular, the decisions about scaling and flipping are just there in the code, and we may (or may not) wish to control them as part of our larger story. But Decor is rather nicely table-driven, which is arguably admirable.
I’m sure we’ll be working here again, as we work on controlling the dungeon population. If there are infelicities in the code, we’ll encounter them.
We did inherit a “feature”. Now a Chest can do you damage when you open it. I think that’s a good thing, but if we decided otherwise, I’m sure we could adjust the code or tables to change it.
I think the only thing I like better than a plan coming together is when a plan falls apart in the face of a better idea. Today was a day like that.