Dungeon 171

Planning the Learning Level. We need to do a bit better than our usual small feature ideas here. Get some shape up in this b- baby.

Our “learning level” has two main purposes. First, it provides a way for a new user to get some training and experience with the game before they get in there for real. It will give us a chance to be sure that they understand the basics,so that they don’t have to just poke around and perhaps never figure out some key aspect.

Second, it’s a basis for our notion of “set pieces”, levels, or parts of levels, that have some designed puzzle or events, unlike the random aspects that have been most of what we have provided to date.

Third–I lied about the two–it’s an opportunity to test my belief that a system with a decently clean design can readily support even substantial changes without the need for a rewrite or help from demons.

Let’s look at what we want in the learning level, with the understanding that it’s early days and we’ll surely change our minds as we learn.

Custom Initial Message

Each level has its own starting message in the crawl. The learning level is currently identified as level 1, so it gets the standard message. We need our own.

Repeat Initial Message

We might decide that if you hit the ?? button with nothing nearby, you get the initial message again.

Custom Element Messages

We’ll want to place decor, loot, chests, and monsters in the learning level, and to encourage the user to hit the ?? to find out about things. The messages in the learning level should be educational, and different from the regular ones.

For example: I am a crate. When you press the ?? button near any object in the dungeon, you will get a possibly helpful message about what it is. Any object has the ability to give you something of value, and any object has the ability to do you some harm. No object is fatal: you cannot die here. Move onto my square to see if I have anything to give you.

Rather wordy, and we’d probably spread this info over several objects, but it gives the idea.

Custom Element Contents

We may wish to provide the player’s original inventory by having them discover the pieces in the learning level. We may need articles that will not appear again in the dungeon.

Context-Sensitive Element Contents

A nice to have would be conditional contents, not just in the learning level. Rather than give the player a third copy of the Hitchhiker’s Guide, give them a chocolate chip cookie. Or something.

Room Descriptions

The game has a single long crawl at the beginning of each level. It might be of value to associate a message with each room. The message would come out the first time you visit the room. This might include a helpful description to go with our rudimentary graphics.

You have entered a cold, chilling room. In the center, you see a large block of what appears to be ice. There is something embedded in the ice. This might be important!

We do retain a room structure at level creation time, although we do not use it for much after drawing hallways. And each room tile knows its room number, so this feature might not be difficult.

Let’s move for a moment to a story of what the learning level might be like.

Intro Room

A message comes out describing the game in general terms, and including the fact that if you press ?? you can see the room’s message again. *When you’re ready, move to the next room.”

Attribute Room

The message directs the player’s attention to their attribute sheet. A few words about the Health, Speed, and Strength attributes. A few words about healing. Advice to keep an eye on the values. Tell them they can’t die, but they can be sent back to the beginning of the level.

Decor Room

The message indicates that there are some items in the room, and directs the player to stand next to them, but not on them, and to press ?? to get information about the item.

Probably two or three items. One of them is harmful. It would be useful if the message encourages them to observe their attribute sheet and not move on until they have healed.

Door Room

Room has a door on its exit. Message says that doors require keys. Hide a key in some item of decor in the room. Quit leaving keys just lying around.

Calm Monster Room

The message indicates that there is a monster in the room. Monsters can be calm, irritable, or angry. Calm ones may follow you but will not attack. Irritable ones will attack if you get too close. Angry ones will immediately approach and try to attack. The monster here is calm, please do not attack it.

Battle Room

Here there is another monster. It is irritable. See if you can corner it and move next to it. If you do, it may attack. You can attack it by trying to move onto its square, or by pressing the Fight button (if it works). Battle the monster. The player wins unless they are incredibly unlucky.

Poison Room

Discuss poison. Find a poison antidote. Ask player to battle the venomous monster until she is poisoned and it is down. Then, once the battle is over, touch the poison antidote in inventory and be healed.

WayDown Room

Player has been fully educated. Tell them how to get the learning level again. Tell them that the WayDown will take them to the next level. Tell them to expect to need a key to get down there.

Arrgh

I was thinking that if you don’t have a key, maybe you can attack one of the Poison Frogs that guard the WayDown and have it give you a key. I tried that, and discovered that the Poison Frog’s animations were not set up properly. In finding the WayDown, also discovered that the Pathfinder’s animations were also wrong.

Bummer. I’ve fixed them both and committed. These defects were pretty bad. I am ashamed.

One of the errors was that the monster had a single asset for hit or dead, instead of a table. We do have a check for there being something in the animation but we don’t test for it being a table. Let’s do that:

function Animator:init(mtEntry)
    self.animations = {}
    self.animations.dead = mtEntry.dead or error("need dead animation")
    self.animations.hit  = mtEntry.hit  or error("need hit animation")
    self.animations.moving = mtEntry.moving or error("need moving animation")
    self.animations.attack = mtEntry.attack or mtEntry.moving
    self.animations.hide = mtEntry.hide or mtEntry.moving
    self.animations.idle = mtEntry.idle or mtEntry.moving
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

Looking at this now, I’m not fond of a fatal error if the animations are missing. That seems like a bad way to handle the situation. Let’s instead create a default animation, or use one of the other tables if we have it. The moving is mostly likely to be there.

function Animator:init(mtEntry)
    self.animations = {}
    self.animations.moving = self:validate(mtEntry.moving)
    self.animations.dead = self:validate(mtEntry.dead)
    self.animations.hit  = self:validate(mtEntry.hit)
    self.animations.attack = mtEntry.attack or mtEntry.moving
    self.animations.hide = mtEntry.hide or mtEntry.moving
    self.animations.idle = mtEntry.idle or mtEntry.moving
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

I think we’ll probably break a test doing this. We’ll deal with that.

function Animator:validate(animationTable)
    if not animationTable then return self:defaultAnimationTable() end
    if type(animationTable) ~= table then return self:defaultAnimationTable() end
    return animationTable
end

function Animator:defaultAnimationTable()
    if self.moving then return self.moving end
    return { asset.Skeleton1 }
end

I think this may work. We’ll have some test revision to do. First the ones that throw:

5: Animator diagnoses missing dead  -- Actual: nothing thrown, Expected: need dead animation

        _:test("Animator diagnoses missing dead", function()
            local entry = {
                hit={"hit"},
                moving={"moving"},
            }
            local f = function()
                Animator(entry)
            end
            _:expect(f).throws("need dead animation")
        end)
        
        _:test("Animator diagnoses missing hit", function()
            local entry = {
                dead={"dead"},
                moving={"moving"},
            }
            local f = function()
                Animator(entry)
            end
            _:expect(f).throws("need hit animation")
        end)
        
        _:test("Animator diagnoses missing moving", function()
            local entry = {
                dead={"dead"},
                hit={"hit"},
            }
            local f = function()
                Animator(entry)
            end
            _:expect(f).throws("need moving animation")
        end)

We won’t need to do that fancy throws checking.

The new test is rather invasive but works:

        _:test("Animator diagnoses missing moving", function()
            local entry = {
                dead={"dead"},
                hit={"hit"},
            }
            local animator = Animator(entry)
            animator:cycle("moving")
            local t = animator.animation
            _:expect(t.frames[1]).is(asset.Skeleton1)
        end)

Animation has a method firstFrameStep. We can use that:

        _:test("Animator diagnoses missing moving", function()
            local entry = {
                dead={"dead"},
                hit={"hit"},
            }
            local animator = Animator(entry)
            animator:cycle("moving")
            local frame = animator.animation:firstFrameStep()
            _:expect(frame).is(asset.Skeleton1)
        end)

That works. My tests are taking about 8 seconds to run. That’s way too long to be tolerable when I’m testing.

But I’m on a mission to fix those other two tests. I thought this would do it for the next one:

        _:test("Animator diagnoses missing hit", function()
            local entry = {
                dead={"dead"},
                moving={"moving"},
            }
            local animator = Animator(entry)
            animator:cycle("hit")
            local frame = animator.animation:firstFrameStep()
            _:expect(frame).is("moving")
        end)

But no:

6: Animator diagnoses missing hit  -- Actual: Asset Key: Skeleton1.png (path: "/private/var/mobile/Containers/Data/Application/8237698C-306E-444D-BA10-95B7089EF5DA/Documents/D2.codea/Skeleton1.png"), Expected: moving

I’m sure glad I have these tests. Why didn’t we return moving?

function Animator:defaultAnimationTable()
    if self.moving then return self.moving end
    return { asset.Skeleton1 }
end

That should be:

function Animator:defaultAnimationTable()
    if self.animations.moving then return self.moving end
    return { asset.Skeleton1 }
end

Test runs. One more:

        _:test("Animator diagnoses missing dead", function()
            local entry = {
                hit={"hit"},
                moving={"moving"},
            }
            local animator = Animator(entry)
            animator:cycle("dead")
            local frame = animator.animation:firstFrameStep()
            _:expect(frame).is("moving")
        end)

OK. Now if a monster doesn’t have the core animations, it will display as a moving skeleton1. Weird, but not fatal. I suppose I could send myself an email if it happens … but no.

I think I recall making that self.moving should be self.animations.moving mistake before. Let’s look at this code and see whether it needs improvement.

function Animator:init(mtEntry)
    self.animations = {}
    self.animations.moving = self:validate(mtEntry.moving)
    self.animations.dead = self:validate(mtEntry.dead)
    self.animations.hit  = self:validate(mtEntry.hit)
    self.animations.attack = mtEntry.attack or mtEntry.moving
    self.animations.hide = mtEntry.hide or mtEntry.moving
    self.animations.idle = mtEntry.idle or mtEntry.moving
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

We might as well use the validate method throughout, since we have it.

function Animator:init(mtEntry)
    self.animations = {}
    self.animations.moving = self:validate(mtEntry.moving)
    self.animations.dead = self:validate(mtEntry.dead)
    self.animations.hit  = self:validate(mtEntry.hit)
    self.animations.attack = self:validate(mtEntry.attack)
    self.animations.hide = self:validate(mtEntry.hide)
    self.animations.idle = self:validate(mtEntry.idle)
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

The Animator has confusing method names defaultAnimationTable and defaultAnimation:

function Animator:defaultAnimation()
    return self.animations.moving
end

function Animator:getAnimation(name)
    return self.animations[name] or self:defaultAnimation()
end

The fetch by name can’t fail now, we’ve put a table into every entry directly. We can remove the or clause and the defaultAnimation method.

Well, except that five (!) tests fail.

4: Animator defaults  -- Actual: nil, Expected: table: 0x294e14b00

        _:test("Animator defaults", function()
            local entry = {
                dead={"dead"},
                hit={"hit"},
                moving={"moving"},
            }
            local animator = Animator(entry)
            _:expect(animator:getAnimation("attack")).is(entry.moving)
            _:expect(animator:getAnimation("hide")).is(entry.moving)
            _:expect(animator:getAnimation("idle")).is(entry.moving)
        end)

Well. So that tells me that the ‘or’ and default fetch were necessary. But I was sure they wouldn’t be. Could put them back. But if the tables are right, how did we ever get the failure?

function Animator:getAnimation(name)
    return self.animations[name] or error("wtf")
end

Ha! Look at this:

function Animator:defaultAnimationTable()
    if self.animations.moving then return self.moving end
    return { asset.Skeleton1 }
end

Again that same error!

function Animator:defaultAnimationTable()
    if self.animations.moving then return self.animations.moving end
    return { asset.Skeleton1 }
end

Tests are green again.

But how can I improve this code so that I don’t make the mistake again, of typing

self.moving

When what I should type is:

self.animations.moving

I think we should change all those references to use our getter:

function Animator:getAnimation(name)
    return self.animations[name]
end

We’ll have to use the strings, but that’s probably best throughout.

function Animator:defaultAnimationTable()
    local movingAnim = self:getAnimation("moving")
    return movingAnim or { asset.Skeleton1 }
end

That actually improves the method a bit as well. Test. Still green. That 8 seconds is really bugging me.

Now this:

function Animator:init(mtEntry)
    self.animations = {}
    self.animations.moving = self:validate(mtEntry.moving)
    self.animations.dead = self:validate(mtEntry.dead)
    self.animations.hit  = self:validate(mtEntry.hit)
    self.animations.attack = self:validate(mtEntry.attack)
    self.animations.hide = self:validate(mtEntry.hide)
    self.animations.idle = self:validate(mtEntry.idle)
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

We’ll do this:

function Animator:init(mtEntry)
    self.animations = {}
    self:putAnimation("moving", mtEntry)
    self:putAnimation("dead", mtEntry)
    self:putAnimation("hit", mtEntry)
    self:putAnimation("attack", mtEntry)
    self:putAnimation("hide", mtEntry)
    self:putAnimation("idle", mtEntry)
    self.animationTimes = mtEntry.animationTimes or { 0.23, 0.27 }
    self.animation = Animation()
end

Then this:

function Animator:putAnimation(name, mtEntry)
    local entry = mtEntry[name]
    self:rawPut(name,self:validate(entry))
end

function Animator:rawPut(name, animationTable)
    self.animations[name] = animationTable
end

That’s better, I think. Even better would be if the animation table could help us a bit, but that’s probably overkill for this deep in the code.

Tests are green, game works. Commit: improved animation validation, removed fatal error call.

We’ve done a couple of hours of planning and coding, though the coding was a bit of a surprise. Let’s sum up.

Summary

I think the planning was of value, and I’m a bit happy that we didn’t start on implementing it right away. Give it 24 hours to rise. Start baking tomorrow.

The defect with the animations was quite serious. The pathfinder was entirely broken, and if you happened to attack the poison frogs, they would crash as well.

But we didn’t just patch the code. I did do that first and could, in principle, have shipped an emergency fix version right then. But then we went after a more basic cause, and first improved the operation of the Animator, but then went further to improve the code.

Interestingly enough, improving the code discovered another issue, which was two references to self.moving that should have been self.animations.moving. I spotted the first one, and further code improvement uncovered the other. And I had already made that mistake a few days ago, I’m almost certain.

With that in mind, we then changed the code’s organization in two important ways.

First, we removed dot references to a table that’s really indexed by string. Recall that in Lua x.foo is exactly equivalent to x[“foo”]. To save myself some typing, I used the dot notation and it didn’t serve me well.

Second, though, we also boiled all the references to those table entries down to a single get method and a single (raw) put method. This should signal to us if and when we change this code that we should use those methods to access our animations table using those methods.

I’ve mentioned before that when an object has accessors, I think it is best to use them, even internally. Here we see an example of why I think that. I’m sure that if we looked, we’d find cases in this program where I’ve not done that.

Which reminds me. Did we check all the tests for whether they are using the dot notation? We did not.

I’m ready to move on to the next part of my day, so I’ve made a reminder: “Check Animator tests for not using animations.moving. Use accessors.”

A quick scan of the code makes me think it’s OK, but I’ll double check sometime when I’m fresh.

So. An interesting morning. Some useful planning, and then a quick fix followed immediately by making the fixed code more nearly right.

I am pleased. See you next time!

D2.zip