Dungeon 322
Unable to do nothing, I made a ‘simple’ change to the Dungeon program. It works … and I don’t like it. Later: Sometimes doing the right thing doesn’t work out.
I wanted to do something with a little code this morning, and it occurred to me that there’s a capability in the scrolling messages to point at the entity issuing the message. As of last night, only the NPC used that capability:
I thought it would be nice if everything that issues a message like that, responding to the query [??] button, would do that.
I easily found where the NPC does the trick:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
Bus:informDotted(msg, self)
return done
end
I happened to “know” that the self parameter is optional. I didn’t review any further code: I just went around and plugged , self into all the other senders of informDotted.
It didn’t work. When I queried the Lever, the first object one encounters, it blew up here:
function OP:message()
if not self.speaker then
return self.text
else return
self.speaker:name()..": "..self.text
end
end
The Lever doesn’t have a function name and in fact it has a string member .name. I tried a few things to get around that, and finally wound up with this:
function OP:message()
if not self.speaker then
return self.text
elseif type(self.speaker.name) ~= "function" then
return self.text
else
return self.speaker:name()..": "..self.text
end
end
That took a few tries and I quickly reverted and decided to do the objects one at a time. I quickly found another problem, which is … well rather than explain all this, let me revert again and show you.
OK, we’re reverted. Let’s change all the informDotted to have , self. I’ll spare you the printouts, they’ll turn up as we get into trouble.
First, I’ll test Lever, because it’s the first thing we encounter. And we get the error that I just described:
CombatRound:232: attempt to call a string value (method 'name')
stack traceback:
CombatRound:232: in method 'message'
Floater:38: in method 'drawMessage'
Floater:30: in method 'draw'
GameRunner:177: in method 'drawMessages'
GameRunner:140: in method 'draw'
Main:108: in function 'draw'
So, after a few tries, I hack the message function this way, as also shown above:
function OP:message()
if not self.speaker then
return self.text
elseif type(self.speaker.name) ~= "function" then
else
return self.speaker:name()..": "..self.text
end
end
Now if I were even a half-way reasonable person, I’d see that that was an egregious hack, checking the type of something. But I guess I was just playing around, so I went on. My next manual test, still on Lever, netted me this:
CombatRound:225: attempt to call a nil value (method 'graphicCenter')
stack traceback:
CombatRound:225: in method 'speechOrigin'
Floater:39: in method 'drawMessage'
Floater:30: in method 'draw'
GameRunner:177: in method 'drawMessages'
GameRunner:140: in method 'draw'
Main:108: in function 'draw'
Lever does not know its graphic center. Well, fine. I looked up how people implement that:
function Player:graphicCenter()
return self:currentTile():graphicCenter()
end
OK, well, it makes sense that all the DungeonObjects could do that, so I implement it on DungeonObject. This is more reasonable than the previous change, I think. I remove the implementation from the other objects. This should get us further.
Now, still testing the Lever, I get:
Floater:44: bad argument #1 to 'text' (string expected, got nil)
stack traceback:
[C]: in function 'text'
Floater:44: in method 'drawMessage'
Floater:30: in method 'draw'
GameRunner:177: in method 'drawMessages'
GameRunner:140: in method 'draw'
Main:108: in function 'draw'
This throws me for a loop (and it didn’t happen the last time I did this). Browsing the code, I don’t see what I did wrong. The changes I made couldn’t have affected that. Well except that they did, because I didn’t put anything inside the elseif. Duh.
function OP:message()
if not self.speaker then
return self.text
elseif type(self.speaker.name) ~= "function" then
return self.text
else
return self.speaker:name()..": "..self.text
end
end
Lever should work now … and it does. Decor also work, they are inheriting graphic center properly. Now I decide to test a free-hanging Loot:
CombatRound:225: attempt to call a nil value (method 'graphicCenter')
stack traceback:
CombatRound:225: in method 'speechOrigin'
Floater:39: in method 'drawMessage'
Floater:30: in method 'draw'
GameRunner:177: in method 'drawMessages'
GameRunner:140: in method 'draw'
Main:108: in function 'draw'
OK, Loot defers its message to the InventoryItem:
function Loot:query()
self.item:query()
end
And an InventoryItem doesn’t have a graphic center at all. So then what the fool did was implement a fake graphicCenter message on InventoryItem:
function InventoryItem:graphicCenter()
return vec2(0,0)
end
This makes everything work. All the Decor work, the WayDown and Spikes work. Keys work.
I could commit this. But, should I? I’ve done two questionable things: I’ve returned a fake graphics center from an InventoryItem, and I can imagine that method confusing someone someday. And, even worse, I’ve got an if statement in the middle of the code for issuing a message that interrogates its objects about their idea of name before calling name on them.
I think that’s not righteous. Furthermore, I think that even allowing the message call to ask for name at all, even via function, and the drawing to ask for graphic center, isn’t righteous. These objects assume too much about what is going on.
Despite that I’ve done this now three times this morning, and am starting to feel like “it’s working, back away slowly” or even “it’s not so bad, I’ve seen and done worse”, this is just not OK.
Let’s think a bit, that’s often helpful.
Thinking
The text-writing code is in Floater. It is processing an array of instances of OP, from the Provider. The OPs can contain something to display, or a deferred action to be done when the item shows up in the Provider. This makes damage get deferred until the corresponding message shows up.
The drawing goes like this:
function Floater:drawMessage(op, pos)
local txt = op:message()
local origin = op:speechOrigin(pos)
strokeWidth(1)
if origin.x ~= 0 then
line(origin.x, origin.y, pos.x,pos.y-8)
end
text(txt, pos.x, pos.y)
end
The OP returns the speechOrigin:
function OP:speechOrigin(pos)
if not self.speaker then return vec2(0,0) end
local speakerPos = self.speaker:graphicCenter()
return speakerPos + vec2(0,32)
end
I observe that that method doesn’t need pos. I think at one point I thought I’d return a relative position, but the actual graphic center is what we need, offset upward.
So here we are calling the speaker, requiring that anything passed to us as a speaker must understand graphicCenter.
There is a rule of OO, “tell, don’t ask”. I tend to break it, and I’ve broken it here. My objects are asking intimate questions about anonymous objects that are passed to them: What’s your name? What’s your graphic center? We should be telling them what they need to know, not having them ask us questions that in some cases we can’t even answer.
Let’s revert and see if we can do a better job.
A Better Job (We Intend)
Now that we know that the message drawing objects are rummaging around too intimately in our objects, and that we want the message creators to be more explicit about what to say and where to show it coming from, let’s start with the NPC, who works correctly (even though we now do not like her implementation).
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
Bus:informDotted(msg, self)
return done
end
I propose that we create a new rule: If your message should have the speaker in front of it, as NPC’s does, you should format it that way.
Therefore, here:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
msg = self:name()..": "..msg
Bus:informDotted(msg, self)
return done
end
Now when we query her, we’ll see her name twice. Let’s verify that. And then let’s talk about whether we can test this other than by looking.
It does come out twice, and in addition some Horn Girl tests do fail:
1: Recognizes cat amulet --
Actual: Horn Girl: Thank you so much!
Please accept this useful gift!,
Expected: Thank you so much!
Please accept this useful gift!
Now that we’re prefixing her name to the message before publishing it, our expectations are not met. Let’s view the test:
_:test("Recognizes cat amulet", function()
local npc = NPC(FakeTile())
_:expect(Bus.subscriber, "did not subscribe").is(npc)
Bus:expect("Thank you so much!\nPlease accept this useful gift!")
npc:catPersuasion()
end)
I think we’ll just consider these to be Approval Tests and fix the strings to expect “Horn Girl: “.
OK, tests run, but the output has Horn Girl twice. We fix that here:
function OP:message()
if not self.speaker then
return self.text
else return self.speaker:name()..": "..self.text
end
end
We no longer need to do anything clever:
function OP:message()
return self.text
end
Test in game. Works as intended. We can commit: OP:message no longer adds speaker. NPC messaged prefixed by NPC name.
Now the speaker origin thing. That’s done way down in that drawMessage method:
function Floater:drawMessage(op, pos)
local txt = op:message()
local origin = op:speechOrigin(pos)
strokeWidth(1)
if origin.x ~= 0 then
line(origin.x, origin.y, pos.x,pos.y-8)
end
text(txt, pos.x, pos.y)
end
I think that’s OK, we just don’t like how OP does that:
function OP:speechOrigin(pos)
if not self.speaker then return vec2(0,0) end
local speakerPos = self.speaker:graphicCenter()
return speakerPos + vec2(0,32)
end
OP is a bit of an odd duck, because everything in the Floater is an OP, but an OP can send messages to objects or can be used to display text. I think we need to see a bit more about how it works for the NPC:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
msg = self:name()..": "..msg
Bus:informDotted(msg, self)
return done
end
function EventBus:informDotted(message, speaker)
Bus:publish("addTextToCrawl", {text="..."})
self:inform(message,speaker)
end
function EventBus:inform(message,speaker)
if type(message) == "string" then
message = du.splitStringToTable(message)
end
for i,msg in ipairs(message) do
Bus:publish("addTextToCrawl", {text=msg, speaker=speaker})
end
end
function Provider:addTextToCrawl(info)
local op = OP:display(info.text)
op.speaker = info.speaker
self:privAddItem(op)
end
That naked table is a bit naff, but It relates to (assumed) limitations in Bus:publish. I don’t think we want to tamper with that but we can certainly change the expectations of addTextToCrawl. In particular we can remove the expectation that speaker will be in there, because we’re not looking at it any more. Let’s change this method to expect speakerPos and default it to vec2(0,0) which is what we use to skip drawing the line.
function Provider:addTextToCrawl(info)
local op = OP:display(info.text)
op.speakerPos = info.speakerPos or vec2(0,0)
self:privAddItem(op)
end
Now let’s go back to OP:speechOrigin:
function OP:speechOrigin(pos)
if not self.speaker then return vec2(0,0) end
local speakerPos = self.speaker:graphicCenter()
return speakerPos + vec2(0,32)
end
function OP:speechOrigin(pos)
return self.speakerPos or vec2(0,0)
end
I panicked there and set the origin to 0,0 here, even though I expect it’s already set. I think at this point we should find that no dialog lines come out. Correct. Now back to NPC:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
msg = self:name()..": "..msg
Bus:informDotted(msg, self)
return done
end
Let’s pass in our position as the optional second argument:
function NPC:publishTale(fsm, taleName)
local tale = fsm.tab.data[taleName]
local done, msg = tale:next()
msg = self:name()..": "..msg
Bus:informDotted(msg, self:graphicCenter())
return done
end
And change there:
function EventBus:informDotted(message, speakerPos)
Bus:publish("addTextToCrawl", {text="..."})
self:inform(message,speakerPos)
end
function EventBus:inform(message,speakerPos)
if type(message) == "string" then
message = du.splitStringToTable(message)
end
for i,msg in ipairs(message) do
Bus:publish("addTextToCrawl", {text=msg, speakerPos=speakerPos})
end
end
Now I expect that the NPC’s dialog line will show up again. It does:
Commit: inform and informDotted take optional speakerPos. PC uses it.
No! I didn’t notice a red bar. Tests broke:
3: Horn Girl scenario -- Entity:8: attempt to call a nil value (method 'query')
I have no idea what that is. Someone is here:
function DungeonObject:currentTile()
return Bus:query("tileContaining", self)
end
Ah it must be the FakeBus in the tests.
I don’t care. I’ve lost the thread. OK, steady. Think. OK, we’ve called self:graphicCenter seriously early in the proceedings, and that’s going to ask for the currentTile, and the FakeBus just happens not to have had that implemented.
I thought this might work:
function FakeEventBus:query(...)
return nil
end
But now I get this:
3: Horn Girl scenario -- NPC:60: attempt to index a nil value
From here:
function NPC:graphicCenter()
return self:currentTile():graphicCenter()
end
Our test hasn’t got that much context set up. I think I can make the FakeEventBus return something close enough:
function FakeEventBus:query(...)
local fakeTile = {}
fakeTile.graphicCenter = function()
return vec2(0,0)
end
return fakeTile
end
I just created an on-the-fly object with a graphicCenter method. Tests run. Commit: Fix NPC tests to run green. Previous commit had red bar :(
Let’s take a breath. I think we’re at a good place.
Breathe …
I think we’ve done good things here. We’ve simplified the inform code so that it just displays the messages we give it, instead of making assumptions about a “speaker” parameter. And we’ve replaced the speaker parameter with a speakerPos that tells the inform to draw a line from that speaker position to the message, as a dialog line showing who or what is speaking.
We had to beef up a Fake object a bit, because this change resulted in a test object needing to come up with a tile with a graphic center. That change was local to the FakeEventBus object and doesn’t affect run time.
Now we ought to be able to make any Dungeon object show a dialog line for its query response. Let’s go ahead and do that.
But first … what about testing? The line drawing is deep in the drawing of the Provider/Floater/OP/CombatRound collaboration. What would we want to test?
I guess we’d want to test something like:
- for this dungeonObject, when it is queried, its message table includes the position of the object, i.e. the graphicCenter of its tile.
OK, having said it, I have to do it. Let’s do it in Lever, which is a nice simple object. I start with the standard frame:
function testLever()
CodeaUnit.detailed = false
_:describe("Lever", function()
_:before(function()
end)
_:after(function()
end)
_:test("First Test", function()
_:expect(2).is(3) -- change this to fail
end)
end)
end
Test is red. Yay. I think I want a fake event bus. I’m not sure what I want, so I write a silly test:
_:test("First Test", function()
local lever = Lever("no tile", "no name")
lever:query()
_:expect(someone).is("something")
end)
Run it.
1: First Test -- Lever:39: attempt to call a nil value (method 'moveObject')
Tile has to understand moveObject. We have a FakeTile, use it:
_:test("First Test", function()
local lever = Lever(FakeTile(), "no name")
lever:query()
_:expect(someone).is("something")
end)
Test. I think it’ll run to the expect but am far from certain.
1: First Test --
Actual: At first glance, this appears to be a lever.
Do you think it does anything?,
Expected: nil
1: First Test --
Actual: nil,
Expected: something
Hm. apparently FakeEventBus knows to check a message:
_:test("First Test", function()
local lever = Lever(FakeTile(), "no name")
Bus:expect("At first glance, this appears to be a lever.\nDo you think it does anything?")
lever:query()
_:expect(someone).is("something")
end)
Now I get my “something” error. And if I stir my bones to look at FakeEventBus, I find that it’s well set up to help me out:
function FakeEventBus:subscribe(object, event)
self.checkMessage = "Thank you!"
self.subscriber = object
end
function FakeEventBus:expect(msg)
self.checkMessage = msg
end
function FakeEventBus:inform(message)
_:expect(message).is(self.checkMessage)
end
function FakeEventBus:informDotted(message)
return self:inform(message)
end
function FakeEventBus:publish(...)
-- nothing
end
We can have it check the second parameter to informDotted. The test becomes:
_:test("First Test", function()
local lever = Lever(FakeTile(), "no name")
Bus:expect("At first glance, this appears to be a lever.\nDo you think it does anything?", vec2(99,99))
lever:query()
end)
And FakeEventBus gets:
function FakeEventBus:expect(msg, optionalPos)
self.checkMessage = msg
self.checkPos = optionalPos
end
function FakeEventBus:inform(message, optionalPos)
_:expect(self.checkPos).is(optionalPos)
_:expect(message).is(self.checkMessage)
end
function FakeEventBus:informDotted(message, optionalPos)
return self:inform(message)
end
Test fails as expected:
1: First Test --
Actual: (99.000000, 99.000000),
Expected: nil
Because query doesn’t provide the center yet, so we add it:
function Lever:query()
Bus:informDotted("At first glance, this appears to be a lever.\nDo you think it does anything?", self:graphicCenter())
end
This will fail because it’s not implemented up high enough:
1: First Test -- Lever:57: attempt to call a nil value (method 'graphicCenter')
Implement in DungeonObject. Seems I’ve done this a few times already today.
Even with that in place, I get this error:
1: First Test --
Actual: (99.000000, 99.000000),
Expected: nil
I’ve not gotten thru to expected. What have I done wrong here?
Oh. I’ve reversed the sense of the expect.
function FakeEventBus:inform(message, optionalPos)
_:expect(optionalPos).is(self.checkPos)
_:expect(message).is(self.checkMessage)
end
That’ll get me closer but it’s still gonna say nil. I want to check what graphicCenter returns:
_:test("First Test", function()
local lever = Lever(FakeTile(), "no name")
_:expect(lever:graphicCenter()).is(vec2(0,0))
Bus:expect("At first glance, this appears to be a lever.\nDo you think it does anything?", vec2(99,99))
lever:query()
end)
The first expect does pass. Let’s look more carefully, I’ve messed up somewhere:
function Lever:query()
Bus:informDotted("At first glance, this appears to be a lever.\nDo you think it does anything?", self:graphicCenter())
end
I’ve changed the message and that query does error. But the other check isn’t working.
Ah. Included the parm then didn’t wire it up:
function FakeEventBus:informDotted(message, optionalPos)
return self:inform(message)
end
Fix:
function FakeEventBus:informDotted(message, optionalPos)
return self:inform(message, optionalPos)
end
Now I expect a fail with expected 99,99 and actual 0,0:
Well, the good news is that our current test fails as expected:
1: First Test --
Actual: (0.000000, 0.000000),
Expected: (99.000000, 99.000000)
But some NPC tests have started to fail.
1: Recognizes cat amulet --
Actual: (0.000000, 0.000000),
Expected: nil
That’s because those tests are not providing a checkPos value. Let’s enhance the bus not to check for nil:
function FakeEventBus:inform(message, optionalPos)
if self.checkPos then
_:expect(optionalPos).is(self.checkPos)
end
_:expect(message).is(self.checkMessage)
end
I expect green except for my 99 not equalling 0: That’s the case. Fix the expectation:
_:test("Lever puts graphic center into message", function()
local lever = Lever(FakeTile(), "no name")
_:expect(lever:graphicCenter()).is(vec2(0,0))
Bus:expect("At first glance, this appears to be a lever.\nDo you think it does anything?", vec2(0,0))
lever:query()
end)
OK! We have a fairly easy way to test these guys. Commit lever: Lever sets dialog line. Has test for informDotted..
Now then. We want Decor to do this same thing. And it only seems fair to test it similarly.
_:test("Decor sets graphicCenter into informDotted", function()
Bus = FakeEventBus()
Bus:expect("",vec2(0,0))
local tile = FakeTile()
local item = "item"
local decor = Decor(tile, item, "Skeleton1")
decor:query() -- expectations are on the Bus
end)
This will fail both the message and the coordinates, because I told it to expect a empty message, and because we aren’t sending in the coordinates.
16: Decor sets graphicCenter into informDotted --
Actual: nil,
Expected: (0.000000, 0.000000)
16: Decor sets graphicCenter into informDotted --
Actual: There's a skull. Could be a dead'un. Not sure ...,
Expected:
I think that message is supposed to be random. Let’s have a new rule, that if the message is nil in the Bus, we don’t check it, as with the coordinates.
function FakeEventBus:inform(message, optionalPos)
if self.checkPos then
_:expect(optionalPos).is(self.checkPos)
end
if self.checkMessage then
_:expect(message).is(self.checkMessage)
end
end
OK, now I should just get the coordinate error:
16: Decor sets graphicCenter into informDotted --
Actual: nil,
Expected: (0.000000, 0.000000)
Fix the line of code:
function Decor:query()
Bus:informDotted(self:queryMessage(self.kind), self:graphicCenter())
end
Should be green. Is. Commit: Decor includes dialog line in query response.
What’s left? Loot, Spikes, WayDown … Monsters …
Honestly, this is taking forever and the tests aren’t serving me much at all. I’m going to go ahead and find all the informDotted and inform messages and fix them.
Ha! That’ll show me. I broke the key. In my zeal for speed, I wrote this:
function Key:query()
Bus:informDotted("Could I possibly be ... a key???", graphicCenter())
end
Instead of this:
function Key:query()
Bus:informDotted("Could I possibly be ... a key???", self:graphicCenter())
end
Dammit. Try to slide by and get bitten, when you really should know better. I’ve fixed key but do I owe the world a test?
I’m going to devise a better way to test this. All I really want to know is whether each of these objects, when you call query, calls Bus:informDotted with a string and a vec2(0,0) (when quiescent … it’ll be a real vec2 at game time).
Let’s take the time to rig something up to check that:
function testDungeonObjectsIncludeCoordinates()
CodeaUnit.detailed = false
local _bus
_:describe("DungeonObjects include coords in query", function()
_:before(function()
_bus = Bus
Bus = FakeCoordBus()
end)
_:after(function()
Bus = _bus
end)
_:test("Check Key", function()
_:expect(2).is(3) -- change this to fail
end)
end)
end
Fails for want of the new FakeCoordBus.
I’ve gotten in some bizarre kind of trouble. Can’t even try to explain it. Revert. I should never have tried to “just do it”. But it really does seem wasteful to test a change like this:
function Monster:query()
Bus:informDotted(self.behavior.query(self))
end
To this:
function Monster:query()
Bus:informDotted(self.behavior.query(self), self:graphicCenter())
end
That works, by the way. I feel that writing these tests is wasteful but I’ve done this wrong so many times. I tried to write a more clever test to check the objects directly, but I couldn’t seem to make it work. Let me try that again, with Key.
OK, this is a bit of code but it works, so let me explain:
local _msg
local _gc
function testKeySendsDialogLineCoordinates()
CodeaUnit.detailed = false
local _bus
_:describe("Key sends coords", function()
_:before(function()
_bus = Bus
Bus = KeyFakeBus
end)
_:after(function()
Bus = _bus
end)
_:test("First Test", function()
local key = Key()
_:expect(key:is_a(Key)).is(true)
key:query()
_:expect(_msg).is("Could I possibly be ... a key???")
_:expect(_gv).is(vec2(0,0))
end)
end)
end
KeyFakeBus = class()
function KeyFakeBus:init()
end
function KeyFakeBus:informDotted(msg, gc)
_msg = msg
_gc = gc
end
The test starts with a couple of temps, _msg and _gc, that I’m using to capture the parameters from query’s call to informDotted. The KeyFakeBus just implements informDotted and captures the results. The test calls query and expects the right answer to have been saved.
The string test passes, the vector one, not yet.
function Key:query()
Bus:informDotted("Could I possibly be ... a key???", self:graphicCenter())
end
This will barf for want of current tile, I expect. But it doesn’t! Instead it says:
1: First Test -- Entity:8: attempt to call a nil value (method 'query')
Somehow, the call to self:graphicCenter has caused our local key to no longer be a key: the method is missing.
Let me return a literal.
function Key:query()
Bus:informDotted("Could I possibly be ... a key???", vec2(9,9))
end
Curiously, I get this:
1: First Test --
Actual: nil,
Expected: (0.000000, 0.000000)
Typo in the test, typed gv, not gc. Fixing that I expect error on 9,9 instead of 0,0.
1: First Test --
Actual: (9.000000, 9.000000),
Expected: (0.000000, 0.000000)
Good. So what went wrong when we called self:graphicCenter()?
function Key:query()
Bus:informDotted("Could I possibly be ... a key???", self:graphicCenter())
end
Where is that implemented?
function DungeonObject:graphicCenter()
return self:currentTile():graphicCenter()
end
function DungeonObject:currentTile()
return Bus:query("tileContaining", self)
end
We’d best implement query on our fake bus.
function KeyFakeBus:query(_ignored)
local t = {}
t.graphicCenter = function()
return vec2(0,0)
end
return t
end
That’s similar to the other test but a bit less sensitive. Now let’s see if I can test other objects this way.
_:test("Loot Test", function()
local loot = Loot(nil,"health", 0,0)
loot:query()
_:expect(_gc).is(vec2(0,0))
end)
That fails:
2: Loot Test --
Actual: nil,
Expected: (0.000000, 0.000000)
And if we look at Loot’s implementation of query we get this:
function Loot:query()
self.item:query()
end
function InventoryItem:query()
if self.name ~= "nothing" then
Bus:informDotted(self.message)
end
end
Let’s pass in an optional parameter to InventoryItem query. I’m not sure whether that might be called from elsewhere.
function Loot:query()
self.item:query(self:graphicCenter())
end
function InventoryItem:query(optionalPos)
if self.name ~= "nothing" then
Bus:informDotted(self.message, optionalPos)
end
end
And test.
1: Loot messages -- Entity:8: attempt to call a nil value (method 'query')
2: Can override a Loot's Item's message -- Entity:8: attempt to call a nil value (method 'query')
I am honestly not sure what has happened there. I think I’m getting into trouble with my overly simple replacement Bus.
If I leave the implementation in and remove the test, I do get another couple of test failures:
1: Loot messages -- Entity:8: attempt to call a nil value (method 'query')
2: Can override a Loot's Item's message -- Entity:8: attempt to call a nil value (method 'query')
The first of these is here:
_:test("Loot messages", function()
local speaking = Loot(nil, "health", 0,0)
local silent = Loot(nil, "nothing", 0,0)
speaking:query()
_:expect(busMessage).is("I am a valuable Potent Potion of Health")
busMessage = nil
silent:query()
_:expect(busMessage).is(nil)
end)
And the second is here:
_:test("Can override a Loot's Item's message", function()
local overridden = Loot(nil, "health", 0,0)
overridden:setMessage("A new message")
overridden:query()
_:expect(busMessage).is("A new message")
end)
I think that if I reset the Loot to a fixed value in the position, the errors go away.
function Loot:query()
self.item:query(vec2(91,91))
end
Test. Tests run green. So what has happened in these tests? I’m sure it’s these bizarre fake objects I’m using.
The defect occurs when I call self:graphicCenter in Loot: query. That gets us here:
function DungeonObject:currentTile()
return Bus:query("tileContaining", self)
end
function DungeonObject:graphicCenter()
return self:currentTile():graphicCenter()
end
And we call query on our bus with the message ‘tileContaining`. Our Bus in those tests doesn’t include a query. Meh. This is truly irritating.
Still, I have to make the old tests work even if I don’t provide a new one.
function FakeBus:query(_tileContaining)
local t = {}
t.graphicCenter = function()
return vec2(0,0)
end
return t
end
This passes my old tests. And the loot dialog line works in the game. And lunch is here. I can commit without my new test, and I do.
I’m really ticked off about this testing now. I didn’t want to write tests to begin with and now they aren’t cooperating. But maybe it’s sorted now. However, I’m going to implement the remaining calls to graphicCenter and test in the game.
The only ones left were WayDown and Spikes, both trivial. But now we have Loot, WayDown, and Spikes all working, but without tests. And I am tired of trying to write these tests, which are quite a few lines of code to test one parameter of a method call.
I’m going to commit and sum up, confessing my sins of course. Commit: spikes and waydown show dialog line, no tests.
Summary
What a weird morning. I set out to do this simple task and ran into odd troubles, and of course I wasn’t testing. When I got it working, I realized the code wasn’t good, because our low-level and remote messaging objects were calling back to our dungeon objects to get their name, which shouldn’t have been done at all, and theif location. I believed that they shouldn’t ask for the position, but now that we’re here I think I might be wrong.
The way it works now, we get the position of the speaker at the time it emits its messages to the crawl. By the time those messages come out, in the case of a monster, it may have moved. With the prior implementation, if the speaker moved, the dialog line moved with it. Now the dialog line says where it started. That’s fine for static objects and a bit odd for Monsters.
That doesn’t bother me much, and maybe I’ll play with it again, though I’m trying to be done with the Dungeon. It’s just such a fertile field for code that needs improvement. I wonder who wrote this stuff …
The testing troubles me. I really tried to do the right thing, and it was tedious, difficult, and finally I just gave up. Sometimes doing the right thing just doesn’t work out.
The objects are interconnected, and it seems to me that they rather need to be somewhat connected. But if a dungeon object had a coordinate rather than a Tile, we could eliminate a lot of this setup in the tests.
Additionally, as much as I like the Bus, which allows me to disconnect objects from each other, I am not fond of needing to set up fake Bus objects to manage the testing.
Perhaps there is more to learn from this than I thought.
We’ll see. For today, the evil is sufficient. See you next time!