Dungeon 218
One nice thing. One at a time. One nice thing at a time is enough.
The FSM definition for Horn Girl looks like this:
{
initial="unsat",
events = {
{event="query", from="unsat", to="wait", arg="unsat"},
{event="query", from="wait", to="wait",arg="wait"},
{event="received", from={"unsat","wait"}, to="thanks", arg="thanks"},
{event="query", from="thanks", to="satisfied", arg="thanks"},
{event="query", from="satisfied", to="satisfied", arg="satisfied"},
},
callbacks = {
on_leave_unsat = function(fsm,event,from,to, arg)
if event ~= "query" then return end
return self:publishMessages(arg, fsm,event,from,to)
end,
on_leave_wait = function(fsm,event,from,to, arg)
if event ~= "query" then return true end
return self:publishMessages(arg, fsm,event,from,to)
end,
on_enter_thanks = function(fsm,event,from,to, arg)
return self:publishMessages(arg, fsm,event,from,to)
-- give something
end,
on_enter_satisfied = function(fsm,event,from,to, arg)
self:publishMessages(arg, fsm,event,from,to)
end,
},
data = {
unsat_messages = {
"I am your friendly neighborhood Horn Girl!\nAnd do I have a super quest for you!",
"I am looking for my lost Amulet of Cat Persuasion,\nso that I can persuade the Cat Girl to let me pet her kitty.",
"If you can find it and bring it to me,\nI shall repay you generously.",
},
wait_messages = {
"I sure hope you can find the Amulet for me!\nI'll be so grateful!",
},
thanks_messages = {
"Thank you so much!\nPlease accept this useful gift!",
},
satisfied_messages = {
"Hello, friend!",
},
}
}
The FSM callbacks rely on this method in NPC (Non-Player Character), the class that defines Horn Girl and other NPCs in due time:
function NPC:publishMessages(element, fsm,event,from,to)
local data = fsm.tab.data
local msg_name = element.."_messages"
local idx_name = element.."_index"
local msgs = data[msg_name]
local index = math.min(data[idx_name] or 1, #msgs)
Bus:informDotted(msgs[index])
index = index + 1
data[idx_name] = index
return index > #msgs
end
This hookup relies on a moderately nasty bit of business. We’ve allowed each FSM transition to include an optional value arg
, which can be anything.
In this situation, we’re letting it be a string that we then concatenate with a literal string:
local msg_name = element.."_messages"
local idx_name = element.."_index"
And those two strings are keys back into the data
section of the FSM, where we find our tables of things to publish, and where we create and use a persistent index to allow the publishMessages
method to loop over the messages.
It’s all a bit intricate. Good enough for a couple of days ago, but I don’t think it’s suitable for the long term. We could refactor the publish method to make its tricksy behavior more clear, but we still have this odd connection between the input string and names that had better be found in the data section.
I think we can do better, and I’m going to try an experiment to find out. The idea is that we’ll have a new object, a Tale
, consisting of one or more publishable strings. (Remember, we can publish a multi-line string, and often do.) We’ll use the arg
feature to provide the exact name of a Tale to each callback, and it will just forward a message to the Tale to make it publish its next line.
I believe we’ll like it better. If so, we’ll finalize it, and if not, we’ll revert as if it had never happened.
I’m going to TDD the Tale
, because while this is an experiment, I expect to like the result well enough to keep it. If I don’t TDD it, I won’t feel as good about keeping it. So here we go.
Tale
I think I know what I want, so I write out the whole test:
_:test("Tale steps properly", function()
local tale = Tale{"m1","m2","m3"}
_expect(tale:next()).is("m1")
_expect(tale:next()).is("m2")
_expect(tale:next()).is("m3")
_expect(tale:next()).is("m3")
end)
We create a Tale of three messages, and every time we call next, it gives us the obvious message, continually looping over the third.
That fails with no Tale, and so on. We know the drill, so we code until we’re ready to see what we did wrong.
OK, I just typed this in all in one go, so it will serve me right if the test fails:
Tale = class()
function Tale:init(messages)
self.messages = messages
self.index = 0
end
function Tale:next()
self.index = math.min(self.index+1,#self.messages)
return self.messages[self.index]
end
LOL:
1: Tale steps properly -- Tale:17: attempt to call a nil value (global '_expect')
Yeah, right, had to be something. Needs _:expect.
Test runs green! Take that, Fate!
Now, we want our callback to know whether we’ve given it the last one. We do keep repeating if called again, as with “Hello, friend!”, but our tale-telling callbacks want a false return if we’re not done, true if we are.
Now I’be been thinking I’d publish directly from the Tale. But maybe that’s not the best thing to do. Maybe we’d be better to leave it just returning the string and letting the caller decide what to do with it. It’s a bit more flexible and not much harder to use. So let’s assume that, and let’s allow ourselves the luxury of a two-element return.
We want the normal return, i.e. the first two in our case here, to be false, and the final one, true.
Expand the test. I think this is right:
_:test("Tale steps properly", function()
local done,msg
local tale = Tale{"m1","m2","m3"}
done,msg = tale:next()
_:expect(done).is(false)
_:expect(msg).is("m1")
done,msg = tale:next()
_:expect(done).is(false)
_:expect(msg).is("m2")
done,msg = tale:next()
_:expect(done).is(true)
_:expect(msg).is("m3")
done,msg = tale:next()
_:expect(done).is(true)
_:expect(msg).is("m3")
end)
Change next()
:
function Tale:next()
self.index = math.min(self.index+1,#self.messages)
return self.index==#self.messages, self.messages[self.index]
end
Tests run. Whoopee!!
Let’s plug in Tale, on the hardest case first:
data = {
unsat = Tale{
"I am your friendly neighborhood Horn Girl!\nAnd do I have a super quest for you!",
"I am looking for my lost Amulet of Cat Persuasion,\nso that I can persuade the Cat Girl to let me pet her kitty.",
"If you can find it and bring it to me,\nI shall repay you generously.",
},
wait_messages = {
"I sure hope you can find the Amulet for me!\nI'll be so grateful!",
},
thanks_messages = {
"Thank you so much!\nPlease accept this useful gift!",
},
satisfied_messages = {
"Hello, friend!",
},
}
I give it the name “unsat” because I won’t have to do the fancy publish stuff. Instead:
callbacks = {
on_leave_unsat = function(fsm,event,from,to, arg)
if event ~= "query" then return end
local done, msg = fsm.tab.data[arg]:next()
Bus:informDotted(msg)
return done
end,
This works as intended. It’s shorter than the publishMessages
method, but longer than what we called before. Easily fixed: we are happy to have supporting methods on NPC, we just want them to be as simple as possible.
I’m wondering what the calling sequence should be to a new publishFromTale method. I guess when I call it that, it’s pretty clear it doesn’t need much more info. Shouldn’t we just pass in the name, to save duplication? I think so. Let’s assume it can be simple and deal with making it more complex only if we need to.
Ah, we do need either the fsm or the data table. Give it the fsm. I decide on this:
on_leave_unsat = function(fsm,event,from,to, arg)
if event ~= "query" then return end
return self:publishTale(fsm, arg)
end,
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
Bus:informDotted(msg)
return done
end
Still all good. I think I like this. Let’s put it fully in place and look at it. Maybe commit first: NPC uses Tale in initial unsat long message.
Now the rest:
{
initial="unsat",
events = {
{event="query", from="unsat", to="wait", arg="unsat"},
{event="query", from="wait", to="wait",arg="wait"},
{event="received", from={"unsat","wait"}, to="thanks", arg="thanks"},
{event="query", from="thanks", to="satisfied", arg="thanks"},
{event="query", from="satisfied", to="satisfied", arg="satisfied"},
},
callbacks = {
on_leave_unsat = function(fsm,event,from,to, arg)
if event ~= "query" then return end
return self:publishTale(fsm, arg)
end,
on_leave_wait = function(fsm,event,from,to, arg)
if event ~= "query" then return true end
return self:publishTale(fsm, arg)
end,
on_enter_thanks = function(fsm,event,from,to, arg)
return self:publishTale(fsm, arg)
-- give something
end,
on_enter_satisfied = function(fsm,event,from,to, arg)
return self:publishTale(fsm, arg)
end,
},
data = {
unsat = Tale{
"I am your friendly neighborhood Horn Girl!\nAnd do I have a super quest for you!",
"I am looking for my lost Amulet of Cat Persuasion,\nso that I can persuade the Cat Girl to let me pet her kitty.",
"If you can find it and bring it to me,\nI shall repay you generously.",
},
wait = Tale{
"I sure hope you can find the Amulet for me!\nI'll be so grateful!",
},
thanks = Tale{
"Thank you so much!\nPlease accept this useful gift!",
},
satisfied = Tale{
"Hello, friend!",
},
}
}
When I tested this, an odd thing happened. I gave the amulet to the Horn Girl, and got the thank you message as expected. Then I did a ?? and got the thank you message again. After that, ?? operated as I expected, saying “Hello, friend!”.
To do this testing, I have to actually play the game, find an amulet, go back to the Horn Girl, etc. I really need some way to test a scenario like this.
But right now, what about this problem? I may have to revert, but this is nearly good, and I don’t think our Tale could have changed this behavior, so I’m going to commit this work. I think if I understood stash, maybe I’d stash it.
Commit: NPC uses Tale throughout. Double Thanks message?
Now let’s first look for the defect. I’ll revert if need be but let’s see if we can see it.
OK, we were clearly in the unsat
state when we gave her the amulet. Let’s trace the machine:
events = {
{event="query", from="unsat", to="wait", arg="unsat"},
{event="query", from="wait", to="wait",arg="wait"},
{event="received", from={"unsat","wait"}, to="thanks", arg="thanks"},
{event="query", from="thanks", to="satisfied", arg="thanks"},
{event="query", from="satisfied", to="satisfied", arg="satisfied"},
},
callbacks = {
on_leave_unsat = function(fsm,event,from,to, arg)
if event ~= "query" then return end
return self:publishTale(fsm, arg)
end,
on_leave_wait = function(fsm,event,from,to, arg)
if event ~= "query" then return true end
return self:publishTale(fsm, arg)
end,
on_enter_thanks = function(fsm,event,from,to, arg)
return self:publishTale(fsm, arg)
-- give something
end,
on_enter_satisfied = function(fsm,event,from,to, arg)
return self:publishTale(fsm, arg)
end,
},
So … we get “received” in “unsat”, we enter “thanks”. On entering “thanks” we give the message, and we stay in thanks.
When we get a “query” in “thanks”, we transition to “satisfied”, and we pass the arg of “thanks”. That will publish the thank you again. The transition table is wrong. Should be:
{event="query", from="thanks", to="satisfied", arg="satisfied"},
Did I just change that this morning? No, it was that way when we came in. This defect must have crept in (by which I mean I must have typed it in) in the previous version. Just to be sure, I’ll go look.
A quick revert assures me that the defect was there when I came in. Clearly we need a way to write a scenario test for our NPCs.
Well, heck, let’s just try to write one and see what it would look like.
_:test("Horn Girl scenario", function()
local npc = NPC(FakeTile())
Bus:expect("I am your friendly neighborhood Horn Girl!\nAnd do I have a super quest for you!")
npc:query()
Bus:expect("Thank you so much!\nPlease accept this useful gift!")
npc:catPersuasion()
Bus:expect("Hello, friend!")
npc:query()
end)
This passes. Just to test the test, I’ll put that defect back in.
3: Horn Girl scenario -- Actual: Thank you so much!
Please accept this useful gift!, Expected: Hello, friend!
Super. Tests improved.
Refix the fix. Commit: added longer scenario test to NPC.
Let’s sum up.
Summary
We had a rather odd method in NPC:
function NPC:publishMessages(element, fsm,event,from,to)
local data = fsm.tab.data
local msg_name = element.."_messages"
local idx_name = element.."_index"
local msgs = data[msg_name]
local index = math.min(data[idx_name] or 1, #msgs)
Bus:informDotted(msgs[index])
index = index + 1
data[idx_name] = index
return index > #msgs
end
We replaced that with the much simpler:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
Bus:informDotted(msg)
return done
end
We made that work by creating, complete with tests, a very simple two-method class, Tale:
Tale = class()
function Tale:init(messages)
self.messages = messages
self.index = 0
end
function Tale:next()
self.index = math.min(self.index+1,#self.messages)
return self.index==#self.messages, self.messages[self.index]
end
That allowed us to make the connection between the event table and the callbacks simpler. Instead of using the argument string to create a name in the data, we used the actual name. Saved a couple of lines of code, but also it’s much more clear.
(I think we could probably even pass in the actual Tale, but the code would look nasty. Maybe we’ll try that someday, but not today. Today I don’t like the idea, though I would prefer a more direct connection than the data table lookup.)
So, here’s the bottom line, at least for me.
Just a few minutes’ work lets us improve the process of building up NPC tables. It’s still work for a programmer, but as we do a few more, we’ll see if we can extract more and more out into some kind of input that a human could use. Truth is, I think I could help a human learn how to use the tables as they stand.
Small steps, nice improvement, little jolt of happiness. Join me next time!