Dungeon 150
Let’s drive the bus a bit further. And, I’ve seen a suggestion for a slightly different angle. We should consider that. Ohhh and a garbage problem!
The Angle
I saw some suggested code, somewhere, for a listener kind of thing, which is of course the subscriber half of our event bus thing.
Our calling sequence for subscribing is this:
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
We subscribe passing in self
, which is the object that wants to receive the notification, a method, self.addTextToCrawl
, and the event name, which is also “addTextToCrawl”. (Arguably it should be a more generic name, like “publishText”, or “notify”. There’s no guarantee that the crawl is the only publisher, or even that there is a crawl at all.)
When the EventBus notifies subscribers, it does this:
function EventBus:publish(event, optionalSender, optionalInfo)
for subscriber,method in pairs(self:subscriptions(event)) do
method(subscriber, event, optionalSender, optionalInfo or {})
end
end
The method (a function) is called, passing the subscriber, event, sender if any, and info if any. I could argue that the info should precede the sender, since info is more likely to be provided than sender. We’d only offer the sender if we are offering to enter into a conversation with the subscribers, which seems unlikely.
The example I read passed a function explicitly, rather like this:
Bus:subscribe(self, "addTextToCrawl", function(info) self.addItem(CombatRound():displayText(info.text)) end)
OK, one look at that makes me prefer my scheme, since those embedded function definitions tend to be hard to read and we could always do it with our current scheme. If I found myself wanting to include the functions explicitly like that, I might order the parameters to put the function last, which is more readable and more “Lua standard”.
But we’re doing OO, so I think we’ll stick with passing in a method, even if we write a special one.
OK, we considered that. I’m glad we had this little chat. Let’s do some “work”.
Looking for Text Publishers
We’re set up to allow anyone who wants to display text to just publish it, so let’s see who else calls the runner’s addTextToCrawl
and convert them.
function Player:curePoison()
self.runner:addTextToCrawl("You have been cured of poison!")
self.characterSheet:stopPoison()
end
function Player:poison()
self.runner:addTextToCrawl("You have been poisoned!")
self.characterSheet:startPoison()
end
function Player:doCrawl(kind, amount)
local msg = string.format("+%d "..kind.."!!", amount)
self.runner:addTextToCrawl(msg)
end
function Player:inform(message)
self.runner:addTextToCrawl(message)
end
What we have here, friends, is duplication. Let’s first remove it:
function Player:curePoison()
self:inform("You have been cured of poison!")
self.characterSheet:stopPoison()
end
function Player:poison()
self:inform("You have been poisoned!")
self.characterSheet:startPoison()
end
function Player:doCrawl(kind, amount)
local msg = string.format("+%d "..kind.."!!", amount)
self:inform(msg)
end
One more thing, a nicety. I’m going to put those informs last in the method, No deep reason but I know that Lua does tail calls, so this will be a tiny bit faster and less deep in the call stack. Just a nicety, hardly worth doing.
function Player:curePoison()
self.characterSheet:stopPoison()
self:inform("You have been cured of poison!")
end
function Player:poison()
self.characterSheet:startPoison()
self:inform("You have been poisoned!")
end
Now we’ll fix inform
:
function Player:inform(message)
Bus:publish("addTextToCrawl", self, {text=message})
end
A quick test, and we’re good to go.
And that works. Commit: Player uses EventBus to publish messages.
Are there other callers to runner’s addTextToCrawl
? There are not. We can remove that method now, reducing the face of GameRunner
a bit.
Commit: remove addTextToCrawl method from GameRunner.
Let’s do more on the floater
The GameRunner also has this method:
function GameRunner:addToCrawl(array)
self.cofloater:addItems(array)
end
This is sent here:
function GameRunner:initiateCombatBetween(attacker, defender)
if attacker:willBeDead() or defender:willBeDead() then return end
local co = CombatRound(attacker,defender)
self:addToCrawl(co:attack())
end
function Spikes:actionWith(player)
local co = CombatRound(self,player)
co:appendText("Spikes ".. self.verbs[self.state]..player:name().."!")
local damage = math.random(self:damageLo(), self:damageHi())
co:applyDamage(damage)
self.tile.runner:addToCrawl(co:getCommandList())
end
function Decor:damage(aPlayer)
local co = CombatRound(self,aPlayer)
co:appendText("Ow! Something in there is sharp!")
local damage = math.random(1,5)
co:applyDamage(damage, "_speed")
self.tile.runner:addToCrawl(co:getCommandList())
end
These are … interesting. CombatRound:attack
returns the command list:
function CombatRound:attack()
if self.attacker:willBeAlive() and self.defender:willBeAlive() then
self:append(self:display(" "))
local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
self:append(self:display(msg))
self:attemptHit()
end
return self:getCommandList()
end
The applyDamage
function does not return the list:
function CombatRound:applyDamage(damage, kind)
self.defender:accumulateDamage(damage, kind)
local op = { op="extern", receiver=self.defender, method="damageFrom", arg1=damage, arg2=kind }
self:append(op)
self:append(self:display(self.defender:name().." takes "..damage..self:wordFor(kind).." damage!"))
if self.defender:willBeDead() then
local op = OP("extern", self.defender, "youAreDead", self)
self:append(op)
local msg = self.defender:name().." is down!"
self:append(self:display(msg))
self.defender:playerCallback(self, "playerIsDead")
end
end
We only have the one call to attack outside of tests. We can leave the attack method returning the list, or we can change the tests to demand it. (Or maybe something more weird.)
If we add a publish
to CombatRound, we can cause attack
to do it unconditionally, since that’s what happens anyway, and we can call the publish
method from the two guys who are calling applyDamage
… or provide another method for them to call that does publish. That would leave all the callers uninterested in how the information comes out, just focusing on what they want to happen, i.e. a battle, or damage.
First, we’ll need to expose Floater to a new event. We want to wind up here:
function Floater:addItems(array)
self.provider:addItems(array)
end
Let’s subscribe:
function Floater:init(runner, yOffsetStart, lineSize, lineCount)
self.runner = runner
self.provider = Provider("")
self.yOffsetStart = yOffsetStart
self.lineSize = lineSize
self.lineCount = lineCount
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
Bus:subscribe(self,self.addItems, "addItemsToCrawl")
end
Note that we’re just going to drop right into the addItems
, which means we want the info
packet to be the array of items to be displayed.
Now in CombatRound:
function CombatRound:publish()
Bus:publish("addItemsToCrawl", self, self.commandList)
end
I suspect we’d do well to nil that variable. I chose to use the variable, by the way, because I foresee removing the getter for the list.
Now as a test, let’s try the Spikes with the new scheme:
function Spikes:actionWith(player)
local co = CombatRound(self,player)
co:appendText("Spikes ".. self.verbs[self.state]..player:name().."!")
local damage = math.random(self:damageLo(), self:damageHi())
co:applyDamage(damage)
self.tile.runner:addToCrawl(co:getCommandList())
end
We change that:
function Spikes:actionWith(player)
local co = CombatRound(self,player)
co:appendText("Spikes ".. self.verbs[self.state]..player:name().."!")
local damage = math.random(self:damageLo(), self:damageHi())
co:applyDamage(damage)
co:publish()
end
Now spikes should go through the new scheme. This may be a crash, but I am optimistic.
Also we need to think about better testing for this.
Stepping on the spikes gives me this:
Provider:17: did not get a table in addItems
stack traceback:
[C]: in function 'assert'
Provider:17: in method 'addItems'
Floater:20: in local 'method'
EventBus:89: in method 'publish'
CombatRound:186: in method 'publish'
Spikes:63: in method 'actionWith'
Player:255: in local 'action'
TileArbiter:27: in method 'moveTo'
Tile:110: in method 'attemptedEntranceBy'
Tile:390: in function <Tile:388>
(...tail calls...)
Player:192: in method 'moveBy'
Player:140: in method 'executeKey'
Player:186: in method 'keyPress'
GameRunner:327: in method 'keyPress'
Main:39: in function 'keyboard'
This is because I can’t just call directly to addItems, there are all those things that come back from the Bus. Got to do this:
function Floater:init(runner, yOffsetStart, lineSize, lineCount)
self.runner = runner
self.provider = Provider("")
self.yOffsetStart = yOffsetStart
self.lineSize = lineSize
self.lineCount = lineCount
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
Bus:subscribe(self,self.addItemsFromBus, "addItemsToCrawl")
end
function Floater:addItemsFromBus(event, sender, info)
self:addItems(info)
end
This should be better. And it is:
Now let’s change CombatRound to do the publish, but continue to return the array, for the tests:
function CombatRound:attack()
if self.attacker:willBeAlive() and self.defender:willBeAlive() then
self:append(self:display(" "))
local msg = string.format("%s attacks %s!", self.attacker:name(), self.defender:name())
self:append(self:display(msg))
self:attemptHit()
end
self:publish()
return self:getCommandList()
end
And change GameRunner not to publish:
function GameRunner:initiateCombatBetween(attacker, defender)
if attacker:willBeDead() or defender:willBeDead() then return end
local co = CombatRound(attacker,defender)
self:addToCrawl(co:attack())
end
That becomes:
function GameRunner:initiateCombatBetween(attacker, defender)
if attacker:willBeDead() or defender:willBeDead() then return end
CombatRound(attacker,defender):attack()
end
Now combat should work, without going through GameRunner to publish. And it does.
I think we can commit now, though we’re not quite done with addToCrawl. Commit: Combat publishes attack results itself.
I think there’s one more reference to addToCrawl
:
function Decor:damage(aPlayer)
local co = CombatRound(self,aPlayer)
co:appendText("Ow! Something in there is sharp!")
local damage = math.random(1,5)
co:applyDamage(damage, "_speed")
self.tile.runner:addToCrawl(co:getCommandList())
end
We convert that as before with Spikes:
function Decor:damage(aPlayer)
local co = CombatRound(self,aPlayer)
co:appendText("Ow! Something in there is sharp!")
local damage = math.random(1,5)
co:applyDamage(damage, "_speed")
co:publish()
end
That’s the last call to the addToCrawl
in GameRunner. We remove the method. Commit: combat publication does not use GameRunner.
Now the GameRunner does hold on to the Floater. It uses it twice, once to draw it, which we need to have somewhere, and once to publish the original text messages for the level:
function GameRunner:createLevel(count)
self.dungeonLevel = self.dungeonLevel + 1
if self.dungeonLevel > 4 then self.dungeonLevel = 4 end
TileLock=false
self:createTiles()
self:clearLevel()
self:createRandomRooms(count)
self:connectRooms()
self:convertEdgesToWalls()
self:placePlayerInRoom1()
self:placeWayDown()
self:placeSpikes(5)
self:setupMonsters(6)
self.keys = self:createThings(Key,5)
self:createThings(Chest,5)
self:createLoots(10)
self:createDecor(30)
self:createButtons()
self.cofloater:runCrawl(self:initialCrawl(self.dungeonLevel))
self:startTimers()
self.playerCanMove = true
TileLock = true
end
You’d think we could use a publish to make this happen, but it is going through a different Floater method, runCrawl
. Let’s inspect that.
function Floater:runCrawl(array)
self.provider:addItems(array)
self:startCrawl()
end
function Floater:startCrawl()
self.yOff = self.yOffsetStart
self.buffer = {}
self:fetchMessage()
end
I suspect we can start the crawl with nothing in the provider, and then just use addItems from GameRunner. Certainly the provider can run dry during the game.
I’ll move the startCrawl
to the init:
function Floater:init(runner, yOffsetStart, lineSize, lineCount)
self.runner = runner
self.provider = Provider("")
self.yOffsetStart = yOffsetStart
self.lineSize = lineSize
self.lineCount = lineCount
self:startCrawl()
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
Bus:subscribe(self,self.addItemsFromBus, "addItemsToCrawl")
end
And remove it from runCrawl
:
function Floater:runCrawl(array)
self.provider:addItems(array)
end
If this works, I can use publish in GameRunner. An odd thing happens: the crawl is jerky, pausing a bit between lines. I have a plan for that but first I will reverse out those last two changes and see if it goes smoothly again.
This is curious. It’s still going in a jerky fashion.
Some judicious checkouts using Working Copy, and I find that the jerky behavior went in on the commit named WayDown publishes messages via Bus.
Best look and see what changed. In Floater we have this:
function Floater:init(runner, yOffsetStart, lineSize, lineCount)
self.runner = runner
self.provider = Provider("")
self.yOffsetStart = yOffsetStart
self.lineSize = lineSize
self.lineCount = lineCount
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
end
function Floater:addTextToCrawl(event, sender, info)
self:addItem(CombatRound():display(info.text or ""))
end
Prior to that, we went through GameRunner:
function GameRunner:addTextToCrawl(aString)
self.cofloater:addItem(CombatRound():display(aString))
end
I wonder whether we’re getting empty lines.
Check out current version. A quick test says we’re not asking for any blank lines or empty lines.
In the first version with the jerkiness, If I comment out one line, it goes away:
function Floater:init(runner, yOffsetStart, lineSize, lineCount)
self.runner = runner
self.provider = Provider("")
self.yOffsetStart = yOffsetStart
self.lineSize = lineSize
self.lineCount = lineCount
--Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
end
I would have said that in that version, no one was even doing publishes, except for WayDown, and I’m not interacting with it.
I put a print in every method of EventBus, and other than the tests, there’s just one init and one subscribe, the one from Floater. Still jerky.
I’ll early exit the subscribe.
function EventBus:subscribe(listener, method, event)
print("EB Subscribe")
if true then return end
local subscriptions = self:subscriptions(event)
subscriptions[listener] = method
end
It’s not jerky. Let me double check that. Comment out the if, it’s jerky.
Most peculiar.
OK, this is curious. I added a dump
method to EventBus, to dump out the events table. It contains an entry for each event that is subscribed, indexed by event string, and containing a table for each subscriber, indexed by the subscriber (table) and method.
I dump here:
print("Floater before")
Bus:dump()
Bus:subscribe(self,self.addTextToCrawl, "addTextToCrawl")
print("Floater after")
Bus:dump()
And also after subscribe. Before the subscribe in Floater, there are 31 entries in the Bus.events. After the subscribe, there are 32. Oh.
One issue is that I found it necessary to create the bus before running the tests. We should at least unsubscribe all before running.
That’s this:
function EventBus:unsubscribeAll(object)
print("EB unsubs")
for event,subs in pairs(self.events) do
subs[object] = nil
end
end
I think that can just clear the events table. What was I thinking?
And in Main:
function setup()
Bus = EventBus()
if CodeaUnit then
runCodeaUnitTests()
CodeaTestsVisible = true
end
local seed = math.random(1234567)
print("seed=",seed)
math.randomseed(seed)
showKeyboard()
TileLock = false
Bus:unsubscribeAll()
Runner = GameRunner()
Runner:createLevel(12)
TileLock = true
if false then
sprite(xxx)
end
end
That speeds up the crawl. But why is it slowing down? No one is even using it.
I’m wrong about the unsubscribe all. It needs to do a single object. But we need some kind of a reset. Let me replace the line in Main:
Bus = EventBus()
Bizarre. It’s jerky.
After further play, I have a clue, and some confusion. When I backed out the change above, and went back to unsubscribeAll
, it didn’t go back to smooth. Further, I’m seeing some instances where it’s jerky for a while and then smooths out.
I’m starting to think that the tests are creating too much garbage and causing the game to choke until collection is completed. That would be a problem for a long time if we were holding on to a lot of storage.
And we are. The Floater has a GameRunner. That makes me think that when those 32 objects show up in the subscriptions, there are 32 full games, full of tiles and all, hanging down there. That would slow anyone down.
I’m going to take a break. This has tired me out, but I think I have identified the issue.
Much Later
I took most of the rest of the day off. I’m quite sure that the issue is that we’re creating 30 dungeons and they’re stuck in the Bus. We’ve probably filled the iPad up with tiles and other stuff.
The question, of course, is what to do. One will be to be sure to clear out the Bus more often. Another might be to make it a weak table so that otherwise unreferenced things get dropped. That could backfire, but it’s not likely that the only object holding onto important stuff is the Bus.
Another possibility is to provide a fake Bus during testing, or turn off the Bus altogether.
A quick change to the tests, the one that creates lots of GameRunners, plus making sure to create a new Bus right before the “real” GameRunner, resolves the problem
I’m going to wrap right here and take this up in the morning.
See you then!