Dungeon 208

Are you wondering why i don’t start a new program? There are reasons. Also: let’s take the Bus.

208 articles on a silly dungeon program seems like a lot, doesn’t it? And I have diverted from this series very briefly, but mostly not on programming topics. One day I may start a new one, but despite the surely boring topic, a dungeon crawl, the code keeps offering learning opportunities.

I believe that much programming, I’d even say most, is similar. I’m wrong about that, of course, since there are reasons why there are different niches that aren’t so much alike:

Functional programming, especially with modern languages, isn’t much like the OO programming that I do here.
Programming with containers and microservices is often more like wiring things together than like this code.
Building a web site with something like Jekyll or Sinatra isn’t much like this code. It’s usually just tiny snippets. Except that this is also mostly tiny snippets.

I guess it’s fair to say that a lot of programming is like what we see here, and a lot of what we see here is very much like what we saw in Spacewar, Asteroids, and Space Invaders, earlier articles in the Codea realm. And it’s fair to say that what we see here is very much like most of the programming that I’ve done in my long life, so I’m hopeful that what happens here applies fairly often “out there in the world”.

Since we so rarely get to start with a blank sheet of screen, working on a relatively long-term “legacy” program seems like a perfectly reasonable thing to do.

Enough rationale. You can be sure that when I tire of this venue, I’ll switch so fast your head will spin. But for now, there seems to be plenty to look at and talk about.

So let’s start doing that.

Looking and Talking

We’ve been working with the InventoryItem and the objects associated with it, notably Loot and Decor. There are things to like, and things not to like. We prefer the latter, because our exploration is mostly about how to take existing code and change it so that it is better and more capable, while all the time not breaking things.

Well, almost all the time. We do break things, inevitably, and what we try to do is to break them as seldom as possible, and to detect that we have done so as quickly as possible.

Today, I want to think about and possibly address some issues with the current InventoryItem and friends. These include:

InventoryItem has a few iron-clad requirements, and they are not well validated.
Creation of InventoryItem instances occurs in a number of places in the code, making changes to their form a bit harder to deal with.
InventoryItems can be configured to send any message to any object when they are selected from Inventory. This generality comes with built-in complexity and a level of confusion, because different things are done different ways, even if they could be more similar.
I have the vague feeling that the textual descriptions of the Item could be both simpler and more powerful, enabling them to see a bit more “alive”, with varying messages at varying times in their lifetime.
I’d like to have the ability for the user to query an item, to remind herself what an item is. A double-tap or touch and drag in the Inventory space might suffice for this.
Monsters are beginning to develop different behaviors with regard to different InventoryItems. The Ankle Biter becomes dull when the Rock is deployed, for example. There should be other such behaviors, and we’d like those to be done in an easy and consistent way.

I have one specific idea that seems to me to be likely to be helpful. I think that if we changed the InventoryItem so that it always uses Bus:publish to indicate that it has been used, that would lead to somewhat simpler and more consistent code.

Let’s try it.

I’m Goin In ..

Let’s try to do this incrementally. We’re on a clean commit, nothing waiting in the queue.

First, let’s break Player:addPoints so that the power-ups don’t work any more.

No, let’s not. We actually like that code, we just don’t like that InventoryItem is calling it.

Instead, and this is a bit too extreme, let’s not call the object-message at all in Item deployment, instead doing our proposed publish.

I got this far only to hit a bump:

function InventoryItem:touched(aTouch, pos)
    if aTouch.state == ENDED and manhattan(aTouch.pos,pos) < ItemWidth//2 then
        Inventory:remove(self)
        self:informObjectRemoved()
        self:usedMessage()
        --self.object[self.message](self.object, self.attribute, self.value)
        Bus:publish("ItemUsed", self, ...
    end
end

The Item hasn’t kept its input table, it has decoded the table into the detailed items it wants. I think we’re going to wind up not wanting to do that. For now, we’ll pass ourselves into the publish and see what happens.

function InventoryItem:touched(aTouch, pos)
    if aTouch.state == ENDED and manhattan(aTouch.pos,pos) < ItemWidth//2 then
        Inventory:remove(self)
        self:informObjectRemoved()
        self:usedMessage()
        --self.object[self.message](self.object, self.attribute, self.value)
        Bus:publish("ItemUsed", self, self)
    end
end

I’m passing the Item twice, once in the sender field and once in the info. We’ll want to sort that out. Making a card for that.

If we were to run now, deploying a Health etc will have no effect. And perhaps some tests will break. Let’s find out.

health is used to no avail

We get the message that we used the Health, and it disappears from Inventory, but our health does not improve.

OK, let’s adjust Player and see whether we can decode a generic Item deployment.

Oh my. Player doesn’t currently subscribe to anything. The InventoryItems currently just send messages to provided objects willy-nilly, whether anyone has subscribed or not.

No matter. We’ll move forward, get to the end if we can, and then decide how we feel about what we’ve done. No sense speculating about how it might turn out when we can see how it does turn out.

function Player:init(tile, runner)
    self.runner = runner
    self:initAttributes(self:retainedAttributes())
    self:initSounds()
    self:initVerbs()
    Bus:subscribe(self,"itemUsed", "ItemUsed")
    tile:moveObject(self)
    tile:illuminate()
end

I’m wondering whether we’d like to have a convention that the event name is the method to be called. Might make some sense. Card made.

Let’s get to using publish:

InventoryItem Uses Publish

I guess that’s the story card up there. Let’s begin by reviewing the EventBus cycle:

function EventBus:subscribe(listener, method, event)
    assert(method, "no method")
    local subscriptions = self:subscriptions(event)
    subscriptions[listener] = method
end

function EventBus:publish(event, optionalSender, optionalInfo)
    for subscriber,method in pairs(self:subscriptions(event)) do
        method(subscriber, event, optionalSender, optionalInfo or {})
    end
end

When you subscribe, you name the event that you want to subscribe to. This is a shared name between the publisher and subscriber. You have to know what event the publisher will publish. You also tell the EventBus what object should receive the publication, typically self, and what method should be called on that object.

This arrangement lets a subscriber specify as many different methods, or as few, to receive publications.

When an object publishes an event, it names the event, and optionally, the publishing object, and an argument, info, which can be any object. There are a couple of patterns used so far. Here are two that occur right together:

function WayDown:actionWith(aPlayer)
    if aPlayer:provideWayDownKey() then
        Bus:publish("createNewLevel")
    else
        Bus:publish("addTextToCrawl", self, {text="You must have a key to go further!"})
    end
end

Here we publish just the event “createNewLevel” if the player has a key, and otherwise we publish an “addTextToCrawl” event, providing ourself and a table containing a text item.

Here’s another example, showing what seems like a reasonably general technique:

function Lever:nextPosition()
    self.position = self.position + 1
    if self.position > 4 then self.position = 1 end
    Bus:publish("Lever", self, {name=self.name, position=self.position})
end

Here the Lever changes position (when stepped on), and then publishes “Lever”, together with a table showing its name and current position, a string and a number. Objects interested in Levers are set up to check the name, to see if it is “their” Lever, and if it is, they use the setting to decide what to do:

function Spikes:leverHandler(event, sender, info)
    self.stayDown = false
    local pos = info.position or 1
    if pos == 4 then
        self.stayDown = true
    end
end

(Currently, Spikes are the only users of Lever, and they don’t check the name. They should.)

I see no cases where the subscriber uses the sender, but two where the info is used. That suggests to me that sender should at least be last in the publish calling sequence. Arguably it shouldn’t be provided as a separate element at all, because it can always be provided in the info if info is provided.

I’ve just written a card to the effect that we should do that. Now’s not the time, we’re thinking about InventoryItem, and exploring the Bus as part of that.

If we were to make InventoryItem conventionally use publish when used, it seems to me to make sense to publish a single event, maybe “ItemUsed”, and to include the entire Item as the ‘info’ parameter. It’s already a table and has in it anything one could possibly want to know.

Let’s see how that might work.

function InventoryItem:touched(aTouch, pos)
    if aTouch.state == ENDED and manhattan(aTouch.pos,pos) < ItemWidth//2 then
        Inventory:remove(self)
        self:informObjectRemoved()
        self:usedMessage()
        self.object[self.message](self.object, self.attribute, self.value)
    end
end

It’s that last operative line that we’ll be replacing. Currently the Item contains a field “object”, a field “message”, and optional fields attribute and value, and it sends that message to the object with those parameters.

Here’s an Item table showing an example.

function Loot:createInventoryItem(aPlayer)
    if self.kind == "Pathfinder" then
        return InventoryItem{icon="blue_jar", name="Magic Jar", object=aPlayer, method="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature"}
    elseif self.kind == "Antidote" then
        return InventoryItem{icon="red_vase", name="Poison Antidote", object=aPlayer, method="curePoison"}
    else
        return InventoryItem{icon=self.icon, name=self.kind, description=self.desc, object=aPlayer, attribute=self.kind, value=math.random(self.min,self.max), method="addPoints"}
    end
end

That last bit creates an inventory item that will invoke the method addPoints on the Player, passing an attribute of “Health”, “Strength” or “Speed” and a random value which is calculated at the time of creation, based on the limits sent to the Loot object.

This seems a bit around the horn, doesn’t it? I think we may be getting rid of the loose Loot idea. Even if we want to have a random bottle lying there, we might use Decor to do it. So we’ll let this ride for now, perhaps.

What we see in the above is that when the Item is deployed, we’ll hit this method in Player:

function Player:addPoints(kind, amount)
    self.characterSheet:addPoints(kind,amount)
    self:doCrawl(kind, amount)
end

function CharacterSheet:addPoints(kind,amount)
    local name = self:pointsTable(kind)
    self[name]:addPoints(amount)
end

And voils! The points are added.

What’s not to like about this?

The individual InventoryItem objects are, I suggest, too general. They know details of other objects that they needn’t know. The Loot one knows that the player should be informed, and that the message to send should be “addPoints”. If we want to rename that method in Player, we have to find that string in Loot creation and edit it.

If InventoryItems were to just cry out that they are used, all the interesting logic would be in whatever object wants to know. There’s still some coupling that would have to go on, though, since we have to agree on what will be included in the Item’s table, which is to be sent to the subscriber.

OK, I think I’m ready.

Not quite. We have to pass a method to the subscribe:

function Player:init(tile, runner)
    self.runner = runner
    self:initAttributes(self:retainedAttributes())
    self:initSounds()
    self:initVerbs()
    Bus:subscribe(self, self.itemUsed, "ItemUsed")
    tile:moveObject(self)
    tile:illuminate()
end

That works. When we tap a Health Item in Inventory, we get the messages and gain the health.

Let’s enhance the subscribe to require a function.

function EventBus:subscribe(listener, method, event)
    assert(type(method)=="function", "method parameter must be function")
    local subscriptions = self:subscriptions(event)
    subscriptions[listener] = method
end

Darn. thing is, now I really want to commit that change. I’ve got running tests and the loots work. But surely there are other items that do not work. Let’s look at others that use the original scheme:

        InventoryItem{ icon="red_vase", name="Poison Antidote", object=self.player, method="curePoison" },
...
        return InventoryItem{icon="blue_jar", name="Magic Jar", object=aPlayer, method="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature"}

These are too irregular. How will we sort those out in this method?

function Player:itemUsed(event, sender, info)
    -- sender and info are both InventoryItem
    local name = info.attribute
    if name=="Health" or name=="Strength" or name=="Speed" then
        self:addPoints(info.attribute, info.value)
    end
end

Looks to me as if the method value is the closest to a common one.

I’m not happy with this path. I’m going to revert, make that one improvement to check for a function, and commit that.

Commit: EventBus:subscribe checks to be sure method is a function.

Let’s look back and see how to regroup.

Retro

The basic idea here is to have InventoryItems just publish their being used to the EventBus, and to have the objects who need to know things subscribe to the Items’ event or events.

I think that idea has merit, because it should simplify the InventoryItem object. However, I may not be correct.

As it stands, an Item knows who is interested in its being used, and it calls that objects with a defined method. An objection to that is that InventoryItems need to be linked to the applicable objects early on, when created. We get around that, for example, with the Rock of Dullness, by having it tell the Player it has been used and then the Player publishes another event, “becomeDull”.

But that construction links inventory items directly to essentially any object in the system, which is a rather tight and unpredictable coupling.

The scheme that I just tried used a single published message, “ItemUsed”, for all InventoryItems. The subscriber will therefore be exposed to the use of all Inventory items, whether or not they are interested. Take the current situation and play it forward.

The Player wants to know whether any items have been used that add points to attributes, so it must receive the “ItemUsed” event. The Ankle Biter should want to know whether the Rock has been deployed, so it should receive the event as well. So both Ankle Biter and Player have to parse out the Item uses that they care about, discarding the others.

The current scheme is better, in that the Items can be configured differently. Health ones know to call the Player, and Rock ones know to announce “dullSharpness” on the Bus.

We are in a rather common di- or multi-lemma situation that we encounter in “legacy” code: we have similar things being done in different ways. We can see that we’d like to normalize them, but the different ways are just different enough that we can’t see clearly how to get them all alike, so that we can do it all in just one way.

It is tempting to sit down and make a table of all the things we publish and subscribe, and to devise an overall plan, and then commit ourselves to finding a way to implement that overall plan.

We might ask for a week or a month to refactor InventoryItems. We might devise a way to change them one at a time to the new scheme. The latter is better, if we can do it, but as we’ve just seen, sometimes doing it incrementally becomes confusing and we have to back away slowly, hoping that the tower of cards doesn’t collapse.

I’d like to do something incremental, and I’d like to do some of it today, in a way that I think can continue to go forward. And I’d like to think a bit about the overall scheme, but I’d like not to devise much of an overall plan. Why? Because usually, in real life, we don’t have the time to sit back and think a lot about what to do. We have to think as we go, based on a bit of initial thinking, but not so much that our apparent progress slows down.

I once saw a team ask for permission to do a “big refactoring”, and they were given permission. They thought it might take a week or two. Forty-two days later, they weren’t quite done.

That’s not a good thing, and many teams would find themselves in big trouble over something like that. So I try never to take days to do anything. Minutes and hours are the units that I prefer. Small numbers of them.

Working toward what to do …

It’s well, when looking back, to reflect on what happened, and what’s hard about going forward, especially when what we just tried didn’t work. But we need to work toward a solution.

Let’s review publish-subscribe yet again. For a given t=event, the publish loop sends, to each subscriber, the message the subscriber asked to have sent for that event, and whatever other information the publisher includes, typically the publisher’s own id, and an arbitrary information object, which could be a value but is usually a table of info.

The subscriber subscribes by naming an event it’s interested in, its own self, and the method to be called when the event occurs (not in that order, perhaps unfortunately).

So it’s basically “when this happens, call me on this number, passing me whatever goes with what happens, and “this has happened, here’s what goes with it”.

InventoryItems can use this mechanism but are not required to. Maybe we want to adjust them individually to do that, and then consolidate.

Hey, that sounds good. There are already Items that do a Bus:publish. And there are some that do not. If we changed them individually until they all do Bus:publish, we can (probably) move the Bus:publish bit down into the object, and out of its table.

This makes me think that each “kind” of InventoryItem should have its own event to publish, rather than just “ItemUsed” as I did above. We have one such already:

        InventoryItem{ icon="rock", name="Rock", object=Bus, method="publish", attribute="dullSharpness", description="Mysterious Rock of Dullness", used="You have mysteriously dulled all the sharp objects near by." },

So long as this is only partly done, we have to leave the Item’s object and method alone. And we know that, so far, the attribute and value are sufficient to our needs, because everything works.

However, users are currently decoding on method and then perhaps looking at attribute and value. I think we need another element in the InventoryItem, event, which will be the event to publish, so that attribute and value are still available for use.

That’s tricky because of how we use an item:

function InventoryItem:touched(aTouch, pos)
    if aTouch.state == ENDED and manhattan(aTouch.pos,pos) < ItemWidth//2 then
        Inventory:remove(self)
        self:informObjectRemoved()
        self:usedMessage()
        self.object[self.message](self.object, self.attribute, self.value)
    end
end

In the above, self.message is actually the method parameter. Confusing? Arguably yes. We’re not quite here to fix that, however. Our two cases don’t line up:

Item	object	method	attribute	value
Health	player	addPoints	“Health”	aNumber
Rock	Bus	publish	dullSharpness	nil

We’d kind of like for the event and attribute to have a single meaning, but right now, they just don’t.

Let’s see whether we can just gut this through.

One way we could do it would be to provide an additional value parameter, value2. Let’s do that. We can allow for it without every providing one at first.

function InventoryItem:init(aTable)
    self.icon = aTable.icon
    self.name = aTable.name or self.icon
    self.object = aTable.object or self
    self.message = aTable.method or "print"
    self.description = aTable.description or self.name
    self.used = aTable.used or nil
    self.attribute = aTable.attribute or nil
    self.value1 = aTable.value or aTable.value1 or nil
    self.value2 = aTable.value2 or nil
end

Now we need to be sure to use those:

function InventoryItem:touched(aTouch, pos)
    if aTouch.state == ENDED and manhattan(aTouch.pos,pos) < ItemWidth//2 then
        Inventory:remove(self)
        self:informObjectRemoved()
        self:usedMessage()
        self.object[self.message](self.object, self.attribute, self.value1, self.value2)
    end
end

So far, no change. It’s just that anyone using the direct call method will get another parameter.

No, wait. Won’t work. Publish doesn’t expect two parameters, it expects a table. We could “fix” that.

Let’s explore how we might do that but probably not do it:

function EventBus:publish(event, optionalSender, optionalInfo)
    for subscriber,method in pairs(self:subscriptions(event)) do
        method(subscriber, event, optionalSender, optionalInfo or {})
    end
end

Well, we don’t really know what’s being passed into us. We could add an additional parm here as well. Or, for a more fancy approach, Lua does have “…” parameters. I don’t know quite how those work. Too fancy. Let’s change publish to allow two infos, not just one.

function EventBus:publish(event, optionalSender, optionalInfo1, optionalInfo2)
    for subscriber,method in pairs(self:subscriptions(event)) do
        method(subscriber, event, optionalSender, optionalInfo1 or {}, optionalInfo2 or {})
    end
end

I’ll default them both to an empty table. The first already was, and though I don’t know that anyone cars, it seemed best to preserve that behavior for now. I made a card to look into whether that’s a good idea.

Let’s test this much. Results are good. Commit: added additional parameter value2 to InventoryItem and also optionalInfo2 to publish.

Now we should be in shape to change how other InventoryItems work. Let’s change the Pathfinder, as I think it might be simpler. There are, however, two cases:

function Loot:createInventoryItem(aPlayer)
    if self.kind == "Pathfinder" then
        return InventoryItem{icon="blue_jar", name="Magic Jar", object=aPlayer, method="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature"}
    elseif self.kind == "Antidote" then
        return InventoryItem{icon="red_vase", name="Poison Antidote", object=aPlayer, method="curePoison"}
    else
        return InventoryItem{icon=self.icon, name=self.kind, description=self.desc, object=aPlayer, attribute=self.kind, value=math.random(self.min,self.max), method="addPoints"}
    end
end

function GameRunner:createDecor(n)
    local sourceItems = {
        --
        InventoryItem{ icon="red_vase", name="Poison Antidote", object=self.player, method="curePoison" },
        InventoryItem{ icon="blue_jar", name="Magic Jar", object=self.player, method="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature" },--]]
        InventoryItem{ icon="rock", name="Rock", object=Bus, method="publish", attribute="dullSharpness", description="Mysterious Rock of Dullness", used="You have mysteriously dulled all the sharp objects near by." },
        --

I hate that duplication, by the way, though I’ve been tolerating it.

Anyway we want to change this so that object is Bus, method is publish, the attribute is spawnPathfinder, which will be taken as the event, as it is with the Rock below it.

        InventoryItem{ icon="blue_jar", name="Magic Jar", object=Bus, method="publish", attribute="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature" },

I think now that everything should run, but that Pathfinders won’t spawn. Excuse me while I go test that. Correct. Now let’s have the Player subscribe, because she knows how to do the work:

function Player:init(tile, runner)
    self.runner = runner
    self:initAttributes(self:retainedAttributes())
    self:initSounds()
    self:initVerbs()
    Bus:subscribe(self, self.spawnPathfinder, "spawnPathfinder")
    tile:moveObject(self)
    tile:illuminate()
end

Again, this makes me think that maybe publish-subscribe should just call the event name on subscribers. Again, that’s not something to do just now.

I think that Pathfinder should work again. It takes me forever to find out, but in fact it does work.

Commit: Decor Pathfinder uses Bus:publish.

Now let’s do the other one the same way.

function Loot:createInventoryItem(aPlayer)
    if self.kind == "Pathfinder" then
        return InventoryItem{ icon="blue_jar", name="Magic Jar", object=Bus, method="publish", attribute="spawnPathfinder", description="Magic Jar to create a Pathfinding Cloud Creature" }
    elseif self.kind == "Antidote" then
        return InventoryItem{icon="red_vase", name="Poison Antidote", object=aPlayer, method="curePoison"}
    else
        return InventoryItem{icon=self.icon, name=self.kind, description=self.desc, object=aPlayer, attribute=self.kind, value=math.random(self.min,self.max), method="addPoints"}
    end
end

Again this should work. I’m not sure just how we can test it with a microtest. We’ll talk about that. For now I’ll just wander until I find one.

Hard to find, but does work. Commit: Pathfinder InventoryItem always uses Bus:publish.

Now let’s see. I’ve messed up the article flow and have to fix that. I’ve already done the addPoints version once, so arguably it should be easy. And it’s 1300 hours, and time for lunch and a break.

Let’s leave well enough alone, sum up, and come back next time all bright-eyed and bushy-tailed.

Summary

We have a but of a Legacy situation in our InventoryItems, which are a bit ad-hoc and perhaps too general, with essentially the ability to call any object out of the blue. We think a publish-subscribe model would be better. The InventoryItems can publish that they’re being used, and if anyone cares, they’ll sign up to be told.

First time through, I lost the thread and decided to back out. I was juggling one more idea than my brain could handle i.e. two ideas, so I reverted to start over.

Somewhere in there, I also lost the thread in the article, resulting in an orphaned section that I’ll have to remove or plaster the edges to make it fit in.

So I started over, and I started in a new place. Instead of doing the addPoints thing, I did the spawnPathfinder thing. I thought it might be simpler, and I guess it was, but mostly I wanted to start somewhere else, because the previous path had been weedy and full of thorns. I often try another path when I lose the thread, and I’d guess that I’d be wise always to do so. I do at least try to approach the same thread a different way, almost always.

I’m trying to keep my mind fresh, and not too contaminated by the thoughts and fears and confusion of the prior approach. I think fresh is better.

The second time through, we got the Pathfinder to work via the Bus without much excitement.

Predicting is difficult, someone said, especially when it involves the future, but I predict that we’ll get the addPoints thing to work readily, and that then we’ll be able to consolidate the InventoryItem so that it always uses Bus:publish, by design, not but virtue of what we plug into its tables. I think we’ll find that the protocol is simplified.

I’d like to get away from the table idea that’s being used in publish-subscribe as well, and instead just pass individual parms, which, I suppose, might be tables for some events.

We’ll see about that. It would perhaps be best to avoid the variable argument list feature of Lua, but we’ll use it if it makes things better. If we do, it should be transparent to users of InventoryItems and the Bus.

All that’s for another day. I hope to see you then. Now let me go see if I can patch up the front of this article. If you didn’t notice any problems, I was successful.

D2.zip