More work on improving the ability to place Dungeon Objects. Today I want to look at Decor and Loot. What will really happen? I never know until I get into the code.

Readers will have noticed that I don’t do a lot of detailed planning, nor much large-scale design. By choice, I like to let the features come along pretty randomly, and to let the desig evolve, mostly in the small. To me, this better simulates what I’ve seen on real product development efforts, where features blindside us and if there is a grand design to the code, it has become so barnacle-encrusted as to be nearly invisible.

In short, I do it this way because it seems more real. Also, I enjoy getting out of trouble, and the only way to get out of trouble is to get into trouble.

But I do think about where we might go, and about issues of design, often at random moments during the day or night. This morning, I was thinking about Loot, Decor, Chests, and InventoryItems. As things stand today:

  • Loot is an invisible wrapper for a treasure that will appear in the dungeon as itself. You see the vase or vial or whatever, just lying there on the floor. When you step onto it, you get the item.
  • Decor is a visible wrapper for a treasure. It looks like bones or a box or barrel. When you step onto the Decor, you may be injured, and if the Decor contains a treasure, you get it. We want to change Decor to act like Chest.
  • Chest is a visible wrapper for a treasure. It looks like a chest and has two states, closed and open. When you try to step onto it, it opens if it is closed, and if it is open, it displays the treasure inside it. When you try to step onto it again, you get the treasure.
  • InventoryItem is a class defining a treasure. It knows how to display itself in the Inventory bar, and how to be used, providing you with power, or taking some other action in the dungeon.

There seems to me to be substantial similarity between Decor and Chest. I am inclined to make them more similar. It also seems to me that Loot could be thought of as invisible Decor. But today they are separate classes, and somewhat different:

  • With Loot, you always see the treasure, right there on the floor. You can step onto the tile at any time.
  • With Decor, you never see the treasure, it is just given to you. You can step onto the tile at any time.
  • With a Chest, you see the treasure if the Chest is open. You can’t step onto the tile, you only bump it. The Chest makes its contents visible by creating a Loot instance on its own tile.

I like the effect of inspecting the Chest, seeing the treasure, and then taking it. I like the effect that you can’t step onto the Chest while this is going on, you just bump it. It kind of feels like poking around.

I’d like Decor to behave similarly to Chest. You bump it. If it has a treasure, the treasure appears. You bump again to get it. But there is the issue of damage. Decor can do damage to you. Presently, you step onto the Decor and it does the damage. We need to come up with the right sequence for that.

I think it should be possible to step onto Decor, or a Chest, once the byplay of opening and giving the Loot is done. I think that TileArbiter, which decides what tiles you can enter, has enough flexibility to handle that.

What’s the Plan, Man?

My rough plan is to make Decor act like Chest. It’ll have two states, open and closed, and when it goes from closed to open, it’ll create a Loot on top of itself. That will show the vase or whatever, right on top of the skeleton or whatever. Decor will not allow you to enter the tile unless it is open. When you do enter the tile, it will give you the item.

But what about damage? We can’t wait to do the damage until you step in. If we do that, you could inspect a crate, determine that it is empty, and avoid its damage. So we’ll apply the damage on the bump, if the Decor is closed. If the Decor is open and you step in, it should not do damage again. But if you come back later and step in again, it should do damage again. That sequence might be a bit tricky but I’m sure we can handle it.

It seems pretty clear that when this is done, Chest could probably be a kind of Decor or vice versa. It is tempting to work to converge the two right now. But I am not going to fall into that trap. I’m sure we could do it, but it seems that there are two steps that can add up to the convergence: First enhance Decor, taking guidance from Chest, probably resulting in some duplication. Then, look at the duplication and resolve it if it seems wise.

Let’s get to it.

Getting Ready

Let’s review Chest, since we expect, and even desire, duplication.

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:open()
    if self:isOpen() then return end
    self.opened = true
    self.pic = self.openPic
    self:currentTile():moveObject(self.loot)
end

How does that call to open happen? TileArbiter has the basic control:

    t[Chest][Player] = {moveTo=TileArbiter.refuseMove, action=Player.startActionWithChest}

And in Player:

function Player:startActionWithChest(aChest)
    aChest:open()
end

And, while we’re at it, Decor is like this:

    t[Decor][Player] = {moveTo=TileArbiter.acceptMove, action=Player.startActionWithDecor}

function Player:startActionWithDecor(aDecor)
    aDecor:actionWith(self)
end

I did glance at making the moveTo optional, but we’ll come back to that.

The fact that Player knows to open the Chest but just says actionWith to the Decor is a bit lopsided. We’ll probably want to normalize that.

OK, now it’s time to review Decor and see about making it work like Chest.

Decor Review and Changes

function Decor:createRequiredItemsInEmptyTiles(items, runner)
    return ar.map(items, function(item)
        local tile = runner:randomRoomTileAvoidingRoomNumber(666)
        return Decor(tile,item)
    end)
end

function Decor:init(tile, item, 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.item = item
    -- self.tile = nil -- tile needed for TileArbiter and move interaction
    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)]
end

function Decor:actionWith(aPlayer)
    local round = CombatRound(self,aPlayer)
    self.danger(self,round)
    self:giveItem()
end

We don’t need to explore how the danger feature works yet.

It seems to me that we want to change Decor so that we provide it with a loot, instead of an “item”, which I suppose is an InventoryItem. It appears that the only way that happens is via the createRequiredItemsInEmptyTiles class method, but I’m sure there are lots of direct creations in the tests, because Decor has a lot of tests.

Let’s at least look at senders of that create method.

function DungeonBuilder:placeDecor(n)
    local sourceItems = {
        InventoryItem("cat_persuasion"),
        --
        InventoryItem("curePoison"),
        InventoryItem("pathfinder"),
        InventoryItem("rock"),
        --
        InventoryItem("nothing"),
        InventoryItem("nothing"),
        InventoryItem("nothing")--]]
    }
    local items = {}
    for i = 1,n or 10 do
        table.insert(items, sourceItems[1 + i%#sourceItems])
    end
    Decor:createRequiredItemsInEmptyTiles(items,self)
end

I was thinking that I might start by creating the Loots at the last minute, in Decor:init, but the code for Chest is right above this method:

function DungeonBuilder:placeChests(numberToCreate)
    local sourceItems = {
        "curePoison",
        "strength",
        "health",
        "pathfinder",
        "speed"
    }
    for i = 1,numberToCreate do
        local kind = sourceItems[1+ i%#sourceItems]
        local loot = Loot(nil, kind, 4,10)
        local tile = self:randomRoomTileAvoidingRoomNumber(self.playerRoomNumber)
        Chest(tile,loot)
    end
end

Let’s replicate this for Decor, and drive down to make it work. It’s just about three steps. Should be safe enough.

function DungeonBuilder:placeDecor(n)
    local sourceItems = {
        "cat_persuasion",
        "curePoison",
        "pathfinder",
        "rock",
        "nothing",
        "nothing",
        "nothing"
    }
    local loots = {}
    for i = 1,n or 10 do
        local kind = sourceItems[1 + i%#sourceItems]
        local loot = Loot(nil, kind, 4,10)
        table.insert(items, loot)
    end
    Decor:createRequiredItemsInEmptyTiles(loots,self)
end

Here I “just” create a table of loots instead of items, and pass the table to the Decor factory method. A bit different from Chests, and perhaps these two methods should be improved, but for now, let’s move on to the Decor.

function Decor:createRequiredItemsInEmptyTiles(loots, runner)
    return ar.map(items, function(loot)
        local tile = runner:randomRoomTileAvoidingRoomNumber(666)
        return Decor(tile,loot)
    end)
end

This method just maps an array of Loots to an array of Decor. We actually ignore the return array. It might be better if our array functions included apply or something like that.

Now we need to make Decor want a loot instead of an InventoryItem:

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)]
end

So far so good. We now have a loot member variable and no item. So we need to deal with that … oh, and we need an open flag. Add that to init:

    self.open = false

Now in actionWith:

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

I think this nearly works but we need to refuse the move for now:

    t[Decor][Player] = {moveTo=TileArbiter.refuseMove, action=Player.startActionWithDecor}

I am going to test this “in world”, and you should call me out on it, but I want to see what happens. I do expect a bunch of tests may fail. We’ll see.

DungeonBuilder:280: bad argument #1 to 'insert' (table expected, got nil)
stack traceback:
	[C]: in function 'table.insert'
	DungeonBuilder:280: in method 'placeDecor'
	DungeonBuilder:202: in method 'customizeContents'
	DungeonBuilder:55: in method 'buildLevel'
	GameRunner:77: in method 'createLevel'
	Main:66: in function 'setup'

Did I forget a rename? Sure did:

    for i = 1,n or 10 do
        local kind = sourceItems[1 + i%#sourceItems]
        local loot = Loot(nil, kind, 4,10)
        table.insert(items, loot)
    end

Should be:

    for i = 1,n or 10 do
        local kind = sourceItems[1 + i%#sourceItems]
        local loot = Loot(nil, kind, 4,10)
        table.insert(loots, loot)
    end

Oh for smarter refactoring (or smarter programmer). Test again.

fp:70: bad argument #1 to 'for iterator' (table expected, got nil)
stack traceback:
	[C]: in function 'next'
	fp:70: in function <fp:68>
	(...tail calls...)
	DungeonBuilder:282: in method 'placeDecor'
	DungeonBuilder:202: in method 'customizeContents'
	DungeonBuilder:55: in method 'buildLevel'
	GameRunner:77: in method 'createLevel'
	Main:66: in function 'setup'

Did same thing here:

function Decor:createRequiredItemsInEmptyTiles(loots, runner)
    return ar.map(items, function(loot)
        local tile = runner:randomRoomTileAvoidingRoomNumber(666)
        return Decor(tile,loot)
    end)
end

Note to self. Be more careful with renames.

function Decor:createRequiredItemsInEmptyTiles(loots, runner)
    return ar.map(loots, function(loot)
        local tile = runner:randomRoomTileAvoidingRoomNumber(666)
        return Decor(tile,loot)
    end)
end

Test. When I try to move into a Decor, I get this:

Loot:49: expect userdata, got nil
stack traceback:
	[C]: in function 'sprite'
	Loot:49: in method 'draw'
	DungeonContentsCollection:113: in method 'draw'
	GameRunner:164: in method 'drawMapContents'
	GameRunner:130: in method 'draw'
	Main:102: in function 'draw'

I suspect that’s the “nothing”. Let’s fix Loot:draw:

function Loot:draw(tiny, center)
    if self.icon then
        sprite(Sprites:sprite(self.icon),center.x,center.y+10)
    end
end

Test. It works, mostly.

vase

The Loot sometimes shows up behind the Decor. But it does show up, and we do get it. This is good.

I want to commit, by way of saving state. But I have a broken test. Let’s see what it has to say:

5: Decor gives item only once  -- Actual: nil, Expected: item

Test is:

        _:test("Decor gives item only once", function()
            local tile = FakeTile()
            local item = FakeItem()
            local decor = Decor(tile,item)
            decor:giveItem()
            _:expect(receivedItem).is("item")
            receivedItem = "nothing"
            decor:giveItem()
            _:expect(receivedItem).is("nothing")
        end)

We’ve actually obsoleted the giveItem method, and should have removed it. i’m going to ignore this test. I’m tempted to remove it, but let’s let it irritate us for a while. I’ve declared it legal to commit with an ignored test. Commit: Decor now takes a Loot and displays it on first bump. Needs zLevel adjustment.

OK, we have a save point. Does zLevel actually do anything? It appears that we do sort on it. The default zLevel is 5, in DungeonObject, and neither Loot nor Decor define their own. Chest has zLevel 1. Let’s set Decor similarly.

function Decor:zLevel()
    return 1
end

Test. Good news, and bad news. I think that some of our Loots do not know how to display their item. In particular, the Rock and Amulet of Cat Persuasion do not seem to display anything. The things that do display seem to be on top now.

Let’s see how this happens.

function Loot:draw(tiny, center)
    if self.icon then
        sprite(Sprites:sprite(self.icon),center.x,center.y+10)
    end
end

We get the icon here:

    self.icon = self:getIcon(self.kind)

From this table:

local LootIcons = {strength="blue_pack", health="red_vial", speed="green_flask",
pathfinder="blue_jar", curePoison="red_vase"}

This is pretty weak. Loot creation needs to be a lot stronger than this. Let’s do this …

function Loot:getIcon(kind)
    local icon = LootIcons[kind]
    if not icon then print("No icon for ",kind) end
    return icon
end

Now I should get at least a printout. And I do, for rock, catPersuasion, and nothing. Nothing is OK. Let’s add the others. Must have those pictures somewhere …

local LootIcons = {strength="blue_pack", health="red_vial", speed="green_flask",
pathfinder="blue_jar", curePoison="red_vase", catPersuasion="cat_amulet", rock="rock"}

The rock shows up, but is very large. The amulet seems not to show up. There’s something odd going on. Now it shows up on the display but does not go into inventory. Something about the naming, I think.

The difference is between “cat_persuasion” and “catPersuasion”, I think.

Yes. I’ll spare you the printouts. Some places I had the one spelling, and some the other. Making them all the same provides the right icon.

I’m going to commit, but I am unsatisfied with where we are. Commit: zLevel works, cat amulet still displays poorly. Cannot enter Decor.

Raising Head

One thing that dissatisfies me is that there are two sources for icons and descriptions:

local LootIcons = {strength="blue_pack", health="red_vial", speed="green_flask",
pathfinder="blue_jar", curePoison="red_vase", catPersuasion="gray_amulet", rock="rock"}

local LootDescriptions = {strength="Pack of Steroidal Strength Powder", health="Potent Potion of Health", speed="Potion of Monstrous Speed" }

And

local ItemTable = {
    pathfinder={ 
        icon="blue_jar", 
        name="Magic Jar", 
        attribute="spawnPathfinder", 
        description="Magic Jar to create a Pathfinding Cloud Creature" },
    rock={ 
        icon="rock", 
        name="Rock", 
        description="Mysterious Rock of Dullness", 
        attribute="dullSharpness", 
        used="You have mysteriously dulled all the sharp objects near by." },
    curePoison={ 
        icon="red_vase", 
        name="Poison Antidote", 
        description="Red Vase containing a cure for poison.",
        attribute="curePoison" },
    health={
        icon="red_vial", 
        name="Health", 
        description="Potent Potion of Health", 
        attribute="addPoints", 
        value1="Health", 
        value2=1},
    strength={
        icon="blue_pack", 
        name="Strength", 
        description="Pack of Steroidal Strength Powder", 
        attribute="addPoints", 
        value1="Strength", 
        value2=1},
    speed={
        icon="green_flask", 
        name="Speed", 
        description="Spirits of Substantial Speed", 
        attribute="addPoints", 
        value1="Speed", 
        value2=1},
    catPersuasion = {
        icon="gray_amulet", 
        name="Precious Amulet of Feline Persuasion", 
        description="Precious Amulet of Feline Persuasion", 
        attribute="catPersuasion"},
    testGreen={
        icon="green_staff",
        name="green staff", 
        description="green staff"},
    testSnake={
        icon="snake_staff",
        name="snake staff",
        description="snake staff"},
}

It seems to me that a Loot has an InventoryItem in it and that it should be accessing that item directly to get the icons and messages and such.

Let’s see if we can make it do that.

At present, Loot doesn’t hold on to the item:

function Loot:actionWith(aPlayer)
    local item
    self:currentTile():removeContents(self)
    item = self:createInventoryItem(aPlayer)
    item:addToInventory()
end

function Loot:createInventoryItem(aPlayer)
    local value
    if self.min and self.max then 
        value = math.random(self.min,self.max) 
    end
    return InventoryItem(self.kind, value)
end

Let’s do that in init:

function Loot:init(tile, kind, min, max)
    self.kind = kind
    self.icon = self:getIcon(self.kind)
    self.min = min
    self.max = max
    self.desc = LootDescriptions[self.kind] or self.kind
    self.message = "I am a valuable "..self.desc
    if tile then tile:moveObject(self) end
    self.item = self:createInventoryItem()
end

Then fix the other methods to call to the item …

function Loot:getIcon(kind)
    return self.item:getIcon()
end

function Loot:draw(tiny, center)
    if self:getIcon() then
        sprite(Sprites:sprite(self:getIcon()),center.x,center.y+10)
    end
end

So far so good. Then …

function Loot:actionWith(aPlayer)
    self:currentTile():removeContents(self)
    self.item:addToInventory()
end

Test … all seems well. I do sort of wonder what happens with “nothing”.

function Loot:createInventoryItem(aPlayer)
    local value
    if self.min and self.max then 
        value = math.random(self.min,self.max) 
    end
    return InventoryItem(self.kind, value)
end

self.kind will be “nothing”, so …

function InventoryItem:init(aString, aValue2)
    local entry = InventoryTable:getTable(aString)
    self.icon = entry.icon
    self.name = entry.name or self.icon
    self.description = entry.description or self.name
    self.used = entry.used or nil
    self.attribute = entry.attribute or nil
    self.value1 = entry.value or entry.value1 or nil
    self.value2 = aValue2 or entry.value2 or nil
end

The entry will come back as an empty table … which will mean that everything in the InventoryItem object will be nil.

I’m not entirely happy with that. I suggest that we should have an item named “nothing” and that it should at least have a name and description. I add this table entry:

    nothing = {
        name="nothing",
        description = "nothing at all"},

Now I think we can refer to the item for name and description in Loot.

function Loot:init(tile, kind, min, max)
    self.kind = kind
    self.min = min
    self.max = max
    self.item = self:createInventoryItem()
    self.desc = self.item:getDescription()
    self.message = "I am a valuable "..self.desc
    if tile then tile:moveObject(self) end
end

function InventoryItem:getDescription()
    return self.description
end

This is nearly good but the nothing thing is wrong. We’ll deal with that in a moment. Test this. Works. Commit: No, wait, remove those tables from Loot. Now commit: Loot gets all its necessary info from its contained Inventory Item.

We still have that one ignored test, but I’m whipped and it is time to take a break. I’ll tell the team not to worry about the test and assure them that the game is better than ever, with more things to find.

Cards for the next few times:

“nothing” Loot should not exist, make it nil?

Fix ignored Decor test.

Allow entry for open Decor and Chest?

Deal with missing InventoryItem kind in Loot

Smaller rock icon

Let’s sum up.

Summary

Proceeding to make Decor work “like” Chests has gone smoothly. And along the way, we’ve improved Loot to refer to its InventoryItem, and eliminated that separate table that had to match up (and sometimes didn’t).

I didn’t always work bottom up, instead sometimes expressing what I wanted to happen, and pushing the capability down until it worked, usually only one or two levels.

Overall, I think we’re moving in the right direction. The code is converging and becoming more similar where it isn’t converged. We can probably combine Chest and Decor if we want to.

I’m feeling the desire to take a broader look at the Decor, Chest, Loot, InventoryItem nest, but so far, incremental change is working fine. It’s not that I think there’s anything particularly wrong in there … but there may be loopholes, and I’m sure there are opportunities for improvement.

A good morning. Peace to you all.