Let’s look at some of the “story” cards I’ve saved up and see what we might work on.

I have one luxury that many programmers do not have: I can work on anything I want to. For my readers, all three of them, I hope this is good news, because I always manage to learn some kind of lesson, so my readers, both of them, can at least say “I’d never do that”.

Today, I’m going to page through some of the cards I’ve saved with ideas for the Dung program. I’ll list a few of them here, but mostly I plan to pick one and work on it.

  • InventoryItem “should” be a button or held by a Button;
  • Gold, and a Store
  • Torches, variable lighting, flickering, dark rooms, lighting spell;
  • Review and Improve levels of dungeon and inventory;
  • InventoryItem touch capture is clunky, saves pos; what about other Buttons?
  • Test for cancelling capture on InventoryItem given. No test for this case.
  • Manhattan check too conservative in InventoryItem touched. Also Button?
  • Make messages rise over sender? Some other way to id sender?
  • It’s possible to create a Loot with no icon, if [illegible].
  • Bump NPC DFWTNPC try Query. [No idea what that is]
  • Floor Switch Tile (do something with it)
  • InventoryItem with no name just melts away
  • How do no-op InventoryItems work? Fix it.
  • Should event name = method to call in Bus:Subscribe? May save code, be more clear. (But as safe as it is now?)
  • Validation in InventoryItem:init, Object ~= nil?
  • Puzzles
  • Advance the Learning Level
  • Poison from what sources? Needs improvement.

That’s about half the cards I have. I understand almost all of them. Only one was illegible and I sure wonder what DFWTNPC stands for.

Oh … bump NPC refers to what happens if you bump into the NPC. Presently nothing really happens. I still don’t know what DFWT means but NPC is NPC. Oh. Don’t Fool With The NPC. The idea was that when you bump the NPC it says to try query. I think that’s not the right idea. I think when you bump the NPC it should work like the Query button, and trigger the next NPC state. Let’s do that.

If you track through how bumping works, it comes down to this:

function NPC:actionWithPlayer(aPlayer)
    print("NPC saw that")
end

I just printed something out back when I built the NPC. We want it to do query. That’s this:

function NPC:actionWithPlayer(aPlayer)
    self:query()
end

That’s a call to the existing method:

function NPC:query()
    self.fsm:event("query")
end

Now when we bump the NPC she should do her next state.

horn girl interaction works

Super, just right. Commit: Bumping NPC triggers query.

As you can see in the little movie above, the Horn Girl’s remarks come out in the crawl, which is always over the player’s head. There was this card:

  • Make messages rise over sender? Some other way to id sender?

We have a similar issue when we do a query while standing near a few objects. The messages all come out right over our head, and it’s not clear which object is being queried if there’s more than one.

I have two ideas. One is to reposition the individual message over the “speaker”. Since the messages are centered, I’m not sure whether this will be clear or not. Another possibility would be to highlight the tile that is being queried, when the message appears.

Let’s try some stuff.

Identifying Query Output

OK. Our mission, until we decide not to accept it, is to do something that makes it more clear what a scrolling message refers to. Let’s generate some ideas:

  1. Give each message a speaker and prefix it with the speaker name.
  2. Position the message individually centered over its speaker.
  3. Highlight the speaker’s tile somehow, when the message comes out.

Recall that the message crawl moves slowly up the screen, so that when a given message is added to the crawl, the actual display of the message can come a long time after the message was requested. A long time in computer time anyway. So, however we actually implement this, we’ll need to associate something with the message, so that we can do the right thing when the message comes out. Based on our list above, the common element is the speaker, so let’s assume that if we can associate a speaker with a message, we’re part way there.

How do these things get put into the crawl? Let’s start with the NPC, since we’re already looking at her.

function NPC:query()
    self.fsm:event("query")
end

A quick look at the FSM (Finite State Machine) tells us that there’s nothing there about messages. Those will be in the objects running the FSM. We find this in NPC:

function NPC:publishTale(fsm, taleName)
    local tale = fsm.tab.data[taleName]
    local done, msg = tale:next()
    Bus:informDotted(msg)
    return done
end

That’ll be the call to the bus to put out a message with “…” before or after it. I forget which. Anyway:

function EventBus:informDotted(message)
    Bus:publish("addTextToCrawl", self, {text="..."})
    self:inform(message)
end

function EventBus:inform(message)
    if type(message) == "string" then
        message = splitStringToTable(message)
    end
    for i,msg in ipairs(message) do
        Bus:publish("addTextToCrawl", self, {text=msg})
    end
end

That leads here:

function Provider:init(default)
    self.default = default or "default"
    self.items = {}
    Bus:subscribe(self,self.privAddTextToCrawl, "addTextToCrawl")
    Bus:subscribe(self,self.privAddItemsFromBus, "addItemsToCrawl")
end

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text}
    self:privAddItem(item)
end

function Provider:privAddItem(item)
    table.insert(self.items, item)
end

The important learning here is that if we want to associate the sender with the message, it’s a long way around. In the end, though, it seems that we create an item in the Provider that has op=”display” and text consisting of the message to display. If we can get the NPC or other sender passed on down, we could add it to the item with the name “speaker”. Let’s see if we can make that happen.

This is an experiment. If it doesn’t work, I’m prepared to revert. However, I feel that it may well work, and if it does, we might keep it. Hang loose.

We’ll pass ourself to the informDotted:

function NPC:publishTale(fsm, taleName)
    local tale = fsm.tab.data[taleName]
    local done, msg = tale:next()
    Bus:informDotted(msg, self)
    return done
end

We’ll field it there:

function EventBus:informDotted(message, speaker)
    Bus:publish("addTextToCrawl", self, {text="..."})
    self:inform(message, speaker)
end

And deal with it here:

function EventBus:inform(message, speaker)
    if type(message) == "string" then
        message = splitStringToTable(message)
    end
    for i,msg in ipairs(message) do
        Bus:publish("addTextToCrawl", self, {text=msg})
    end
end

Ah. Here, I see two options. One is to pass the speaker on down as a new parameter to addTextToCrawl. But what if we just put it into the table instead? Let’s try that …

function EventBus:inform(message, speaker)
    if type(message) == "string" then
        message = splitStringToTable(message)
    end
    for i,msg in ipairs(message) do
        Bus:publish("addTextToCrawl", self, {text=msg, speaker=speaker})
    end
end

And then, here …

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text}
    self:privAddItem(item)
end

We can copy the speaker over. Perhaps it would be better to create the item by copying whatever is in the info? Let’s do that.

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display"}
    for k,v in pairs(info) do
        item.k = v
    end
    self:privAddItem(item)
end

Now speaker is going to be in the Provider’s returned item … and here …

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        return item.text
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

Ah. We return the text, and deal with it here:

function Floater:fetchMessage()
    local msg = self.provider:getItem()
    table.insert(self.buffer, msg)
end

Let’s try returning the whole table:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        return item
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

Then we’ll have to decode it. First attempt:

function Floater:fetchMessage()
    local msgTable = self.provider:getItem()
    local msg = msgTable.text
    local speaker = msgTable.speaker
    if speaker then
        message = tostring(speaker)..": "..msg
    end
    table.insert(self.buffer, msg)
end

I think this might do something nearly good.

But it’s only nearly good, I think because we don’t always pass in a table yet. Let me try to beef up that last method a bit. First I’ll put it back the way it was:

function Floater:fetchMessage()
    local msg = self.provider:getItem()
    table.insert(self.buffer, msg)
end

Then try again …

OK, I give up. The Provider / Floater interplay is just weird enough that I can’t seem to make this work.

Well … I almost give up. Not quite yet.

No. Gotta revert, but I liked where this was going. I dropped the ball somewhere near the end.

Revert.

Pull Head Out

OK, let’s think about what happened here. Things were going well enough passing a new “speaker” field into the table, but then got out of control somewhere quite near the end. I argue that the bug is the table. We should be passing an object. But we have a lot of callers who are sending this table.

Let’s start inquiring near here:

function Floater:draw(playerCenter)
    local n = self:numberToDisplay()
    if n == 0 then return end
    pushStyle()
    fill(255)
    fontSize(20)
    local y = self:yOffset()
    local pos = playerCenter + vec2(0,self.yOff)
    for i = 1,n do
        text(self.buffer[i], pos.x, pos.y)
        pos = pos - vec2(0,self.lineSize)
    end
    self:increment()
    popStyle()
end

function Floater:fetchMessage()
    local msg = self.provider:getItem()
    table.insert(self.buffer, msg)
end

A little known fact: the floater is always displaying and scrolling up. It’s just that it defaults to a blank line. We see in the code above that the text call expects that the buffer will include the message as it is supposed to be displayed.

So we could accept that when we put something into the buffer, via fetchMessage, it has to be a string. However, suppose that we want to adjust the position of the text based on the speaker. We need the buffer item to contain additional information.

I think I need to look at all the publishers of “addTextToCrawl”.

They all seem to wind down to this:

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text}
    self:privAddItem(item)
end

So why can’t I assume there’s a speaker item right there? It’ll be ignored and should therefore be harmless.

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text, speaker=info.speaker}
    self:privAddItem(item)
end

I try this:

function EventBus:inform(message,speaker)
    if type(message) == "string" then
        message = splitStringToTable(message)
    end
    for i,msg in ipairs(message) do
        Bus:publish("addTextToCrawl", self, {text=msg, speaker=speaker})
    end
end

function EventBus:informDotted(message, speaker)
    Bus:publish("addTextToCrawl", self, {text="..."})
    self:inform(message,speaker)
end

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text, speaker=info.speaker}
    self:privAddItem(item)
end

Now the speaker needs to be a string if it isn’t nil. I can do this:

function NPC:publishTale(fsm, taleName)
    local tale = fsm.tab.data[taleName]
    local done, msg = tale:next()
    Bus:informDotted(msg, "Horn Girl")
    return done
end

I expect this to display “Horn Girl:” ahead of her messages. And it does!

horn girl name ahead of messages

I’m going to commit this, on the grounds that it addresses the story: Horn Girl messages are prefixed “Horn Girl: “.

Let’s raise our heads again and think about what has happened.

Retrospective

The bad news is that I’ve been at this for ninety minutes and I’ve committed once in that period. Well, I was learning. From my mistakes.

What have I learned? Well, in the current scheme, I’ve learned a path directly down from the NPC all the way to the display of a line. It should be fairly easy to change things so that instead of passing the string down as speaker, we pass something more structured.

What went wrong the first time? Well, if I knew exactly, I’d have fixed it. But one thing was that I tried to push the table further down, and to decode it further down. Now it’s being decoded in the Provider:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        if item.speaker then
            return item.speaker..": "..item.text
        else
            return item.text
        end
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

And, let’s accept it, the current decoding is rather ad hoc and surely not in the right place. However, the current rules for provider are that it must return a string. We’ll need to improve that, but it was when we tried to push that decision down further that we got in trouble.

I have no big learning here. Smaller steps would have helped … first time around I changed a lot of methods, pushing down and down. Somewhere in there I went off track.

I do think that we have an object trying to be discovered here. We used to have a table with a guaranteed field text. Now we have on with an additional optional speaker. That’s sure looking like an object to me.

It’s still not clear to me just where the thing goes nor what it is.

But now … we have a solid place to stand. Let’s see what we want to have happen.

Extending the Speaker Notion

Instead of passing in the string, we’d like a speaker to pass in itself, so that we can get whatever information we want from it. So we want, not this:

function NPC:publishTale(fsm, taleName)
    local tale = fsm.tab.data[taleName]
    local done, msg = tale:next()
    Bus:informDotted(msg, "Horn Girl")
    return done
end

But this:

function NPC:publishTale(fsm, taleName)
    local tale = fsm.tab.data[taleName]
    local done, msg = tale:next()
    Bus:informDotted(msg, self)
    return done
end

And I think we want a name to use, so :

function NPC:name()
    return "Horn Girl"
end

This will fail trying to concatenate the name, I expect.

Provider:53: attempt to concatenate a table value (field 'speaker')
stack traceback:
	Provider:53: in method 'getItem'
	Floater:36: in method 'fetchMessage'
	Floater:47: in method 'increment'
	Floater:31: in method 'draw'
	GameRunner:171: in method 'drawMessages'
	GameRunner:134: in method 'draw'
	Main:102: in function 'draw'

Right. This has to change:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        if item.speaker then
            return item.speaker..": "..item.text
        else
            return item.text
        end
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

To this:

,,,
        if item.speaker then
            return item.speaker:name()..": "..item.text
        else
            return item.text
        end

Should work again. And it does. Commit this tiny step forward: speaker in crawl messages is an object. must answer ‘name`.

This might be a good time to get rid of that if statement:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        if item.speaker then
            return item.speaker:name()..": "..item.text
        else
            return item.text
        end
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

If the item were an object it might be better. We do have an issue, which is probably what drove me to settle for the table: there are a few different ops and they are all different.

The provider subscribes to two published messages, “addItemsToCrawl” and “addTextToCrawl”. When it’s a text, we package it here:

function Provider:privAddTextToCrawl(event, sender, info)
    local item = {op="display", text=info.text, speaker=info.speaker}
    self:privAddItem(item)
end

When it’s another kind of item, we just add it:

function Provider:privAddItem(item)
    table.insert(self.items, item)
end

Let’s check the callers of the addItems case.

function Floater:testOnlyRunCrawl(array)
    Bus:publish("addItemsToCrawl", self, array)
    self:startCrawl()
end

function CombatRound:publish()
    Bus:publish("addItemsToCrawl", self, self.commandList)
end

What’s in the commandList? In the case of the CombatRound, the items are all instances of OP:

OP = class()

function OP:display(aString)
    local op = OP("display")
    op.text = aString
    return op
end

function OP:op(combatRound, method)
    return OP("op", combatRound, method)
end

function OP:init(op, receiver, method, arg1, arg2)
    self.op = op
    self.receiver = receiver
    self.method = method
    self.arg1 = arg1
    self.arg2 = arg2
end

Pretty generic. I don’t think we want to push the CombatRound issue, but the display op is interesting.

The rule is kind of loose. Whatever is found in the Provider needs to have an item op. CombatRound passes in an actual OP class instance. But addTextToCrawl is rolling its own. I think we can do this and have it still work:

function Provider:privAddTextToCrawl(event, sender, info)
    local op = OP:display(info.text)
    op.speaker = info.speaker
    self:privAddItem(op)
end

OK, now we’re at least passing in an object rather than a complete nothing. Now back to that if statement:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        return item:message()
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

I’m asserting here that the OP will understand message. It will not, of course:

Provider:53: attempt to call a nil value (method 'message')
stack traceback:
	Provider:53: in method 'getItem'
	Floater:36: in method 'fetchMessage'
	Floater:47: in method 'increment'
	Floater:31: in method 'draw'
	GameRunner:171: in method 'drawMessages'
	GameRunner:134: in method 'draw'
	Main:102: in function 'draw'

I may be getting in trouble here. I’m not sure everyone is going through OP. We’ll see. Let’s update OP:

function OP:message()
    if not self.speaker then
        return self.text
    else return self.speaker:name()..": "..self.text
    end
end

I think this likely works but fails some tests. We’ll see.

Works and does not fail tests. Horn Girl displays her name. Other messages as usual. Sweet.

OP is a pretty weird object but an object it is. We can commit: CrawlMessages use OP to create a display message. Speaker can be an object responding to name.

Now suppose we want to cause the Horn Girl’s message to come from over the Horn Girl’s head rather than the Player’s. To accomplish that, we’d have to pass the object further down, into the floater display:

function Floater:draw(playerCenter)
    local n = self:numberToDisplay()
    if n == 0 then return end
    pushStyle()
    fill(255)
    fontSize(20)
    local y = self:yOffset()
    local pos = playerCenter + vec2(0,self.yOff)
    for i = 1,n do
        text(self.buffer[i], pos.x, pos.y)
        pos = pos - vec2(0,self.lineSize)
    end
    self:increment()
    popStyle()
end

How was buffer loaded?

function Floater:fetchMessage()
    local msg = self.provider:getItem()
    table.insert(self.buffer, msg)
end

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        return item:message()
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

We’d have to return the item, not its message. Let’s do that and fix the crash:

function Provider:getItem()
    if #self.items < 1 then return self.default end
    local item = table.remove(self.items,1)
    if item.op == "display" then 
        return item -- <===
    elseif item.op == "extern" then
        self:privExecute(item)
    elseif item.op == "op" then
        self:privAddItems(self:execute(item))
    else
        assert(false, "unexpected item in Provider array "..(item.op or "no op"))
    end
    return self:getItem()
end

And … that breaks some tests and crashes. I change to this:

function Floater:draw(playerCenter)
    local n = self:numberToDisplay()
    if n == 0 then return end
    pushStyle()
    fill(255)
    fontSize(20)
    local y = self:yOffset()
    local pos = playerCenter + vec2(0,self.yOff)
    for i = 1,n do
        local txt = self.buffer[i]
        if type(txt) == "table" then
            txt = txt:message()
        end
        text(txt, pos.x, pos.y)
        pos = pos - vec2(0,self.lineSize)
    end
    self:increment()
    popStyle()
end

Now I have some tests failing but the crawl works as intended. But it’s time to stop.

Revert. We’re back to green. And I see a problem, the Lever was blocking the door. I thought I had fixed that. Made a note.

Sum up.

Summary

We’re closer to what we want, which is to pass a speaker object all the way down into the Floater, so that it can do what we want relative to display.

I plan to try out the idea of offsetting the message to appear to come from the speaker. I really doubt that I’ll like it, but even so, we should pass the speaker info clear down. That will require fixing some tests, which are assuming that the getItem returns a string, which is a convention we plan to change.

I’m three hours in, and have only made just a tiny bit of code work. I’ve probably changed maybe 25 lines of code that went into the commits, though I probably did two or three times that many that didn’t go in. Call it ten lines an hour, that’s 80 LoC per day. Not fantastically good.

Looking on the bright side, we have a story running:

Horn Girl messages come out identified as hers.

In my defense, this is one of the most inherently complicated areas of the system, combining a connection with Combat, which takes place in an instant but displays asynchronously, with the OP operation notion, the NPC, driven by the Finite State Machine, going through the Provider, which delivers operations and messages to the Floater, which displays text and dispatches things like damage reports to affected objects.

I could tell the boss a good story about how necessarily complicated all this is. But we have seen in the code that the necessary changes were just a few lines. How much of my difficulty was due to the complexity, and how much to my general sloth and incompetence?

To tell the truth, I think this actually went pretty well. There was a lot of trekking around through a lot of objects, and some questionable legacy code, with strings and tables being passed where objects would have been better.

Tomorrow will tell the tale … or whatever day I get back to this. I might not work the weekend. We’ll find out.

A decent morning, lots of discoveries, from which I’ll take some lessons about overly general, and overly specific, calling sequences. Can I make it work? I’m sure I can. Can I make it better? Quite likely.

Stop by next time to find out.