Lua code and Closures - or, why aren't my widgets updating ?!

[phantomwhale]

I wanted to share a problem I'm having, and see if any of the Lua wizzes (or novices, like me !) might hazard a guess what's happen here - other then I'm being far too fancy for my own good, possibly.

I have a problem with my widgets - they stop updating ! And I have narrowed down why. I created the following "Widget Factory" as a named Lua script:



function createBennyWidget(control, identity, posx, posy)
local bennywidget = control.addBitmapWidget("benny_stack1")
bennywidget.setPosition("topleft", posx, posy)
bennywidget.setVisible(false)

local bennytextwidget = control.addTextWidget("sheetlabelsmallbold", "")
bennytextwidget.setPosition("topleft", posx-1, posy+1)
bennytextwidget.setVisible(false)

local nodeChar = DB.findNode("charsheet." .. identity)

local updateBennies = function()
local nBennies = DB.getValue(nodeChar, "main.bennies", 0)
if nBennies <= 0 then
bennywidget.setVisible(false)
bennytextwidget.setVisible(false)
else
bennywidget.setBitmap("benny_stack" .. nBennies)
bennywidget.setVisible(true)
bennytextwidget.setVisible(false)
end
end

local bennyNode = nodeChar.createChild("main.bennies", "number")
bennyNode.onUpdate = updateBennies
updateBennies()
end


This method simply takes in a windowcontrol, adds a bitmap and text widget onto the control, and then creates and registers a local update method to ensure the widget updates whenever the database node in question changes.

And this basically works - I create a characterlist_entry windowcontrol - this is the portrait you see at the top of each ruleset, representing one player-controlled character. This is created when a player picks up a character (or identity) in the following method:



function onIdentityActivation(identity, username, activated)
if activated then
if not findControlForIdentity(identity) then
CharacterListEntryFactory.create(self, identity)
layoutControls()
end
...
end


You'll note this calls a CharacterListEntryFactory - this is another named lua script, quite small, and the create method, called above, looks a bit like this:



function create(characterList, identity)
local userctrl = characterList.createControl(controlClass, "ctrl_" .. identity)
userctrl.setIdentity(identity)
userctrl.createNameWidget()

if User.isHost() or User.isOwnedIdentity(identity) then
WidgetFactory.createBennyWidget(control, identity, 8, 49)
end
end


So, in summary, the characterlist creates a new entry, passes it into the factory which adds on some basic widgets, and then passes it (again) onto the WidgetFactory, which adds on the benny widget.

Why is it done this way ? Well, we also include the benny widgets onto the character selection screen (for the GM only), so this also calls the following code:



function overlayBennyStack(win, control)
local bennypanel = win.createControl("bennyoverlaypanel", "bennypanel")
WidgetFactory.createBennyWidget(bennypanel, win.getDatabaseNode().getName(), 10, 10)
return bennypanel
end


Nothing vital to understand here, other than there is ANOTHER bit of code somewhere that calls into the WidgetFactory.

The issue I'm having is... whenever I call the WidgetFactory.createBennyWidget method, the previous benny widget created for that database node STOPS updating.

So if a user logs in and takes a character, it creates the benny widget on that character portrait entry at the top of the screen. The benny widget then works... UNTIL I open up the character selection screen as the GM, which creates another benny widget, for a totally different contorl, but somehow STOPPED the updates happening on the original widget.

This does NOT happen when multiple players all log in and select characters; each creates a portrait control with the benny widget, and they all update correctly, it's only when I open up the other window and create widgets against database nodes that already have widgets listening to them, and then the updates stop.

As far as I am aware, you can set the onUpdate handler multiple times against a databasenode and it will call all listener functions. So I don't think that's why. But somehow, calling the WidgetFactory.createBennyWidget is killing the old one.

I thought everything would work, as this method ONLY creates local variables (no globals) and then references them within a function, which it then passes to the FGII database node onUpdate handler. So I thought all these local variables would be locked up in the closure scope, and the stateless createBennyWidget function would just return, meaning multiple calls to it would just generate multiple closures.

Might be a bit heavy for a forum question, but if anyone has any thoughts, I'd be keen to know ! I could just copy and paste that widget code in two places, but it would be a shame to do that, as I'd quite like to figure out the secret to making advanced widget factories that can make widgets to add to ANY control !

Cheers,
Ben (-PW-)

[Valarian]

Sounds like you might be passing a reference rather than separate objects at some point, so that the controls are using the same instance of the widget.

What's the code in control.addBitmapWidget("benny_stack1")? Does this create a new instance of a Benny widget?

Can't see a control variable in the CharacterListEntryFactory create method. Do you mean userctrl there?

[phantomwhale]

Sounds like you might be passing a reference rather than separate objects at some point, so that the controls are using the same instance of the widget.


Although in this case there are two widgets, but only one is getting updated and the other is not, so I suspect they are different. But I think the answer will be in that sort of template, yeah.


What's the code in control.addBitmapWidget("benny_stack1")? Does this create a new instance of a Benny widget?


This is out of the core API: https://www.fantasygrounds.com/refdoc/widgetcontainer.xcp#addBitmapWidget


Can't see a control variable in the CharacterListEntryFactory create method. Do you mean userctrl there?


Yep, that would be userctrl - I've simplified all the code a little where I thought it was harmless to do so - there is a middle-method I took out here, and forgot to change the variable name to match.

[Moon Wizard]

It would take quite a while to debug exactly why this is happening, since the Lua/C and closure interfaces are fairly obfuscated when debugging. Plus, you are adding in a lambda function into the equation, and I'd have to research how scoping works in that case.

A couple guesses:
* I think that the onUpdate handler uses the LUA "environment" as the key to index the handlers, so you may be overwriting the same handler.
* The bennywidget and bennytextwidget variables are going out of scope or are being overwritten on subsequent calls.

A couple suggestions:
* You can try using named widgets (similar to 3.5E and 4E token effect widgets), instead of trying to save widget variables.
* You can try using DB.addHandler("charsheet.*.main.bennies", "onUpdate", <fn>) to the character list (upper desktop and within character selection window), and then just iterate through each window in the list to update.

Regards,
JPG

[phantomwhale]

Had a quick play with DB.addHandler - is that an undocumented feature; I don't think I've seen it in the refdoc (https://www.fantasygrounds.com/refdoc/DB.xcp) ?

I maintained a table of widgets, and hit them all with an update on any character benny node update. This works somewhat, but as controls (and associated widgets) get destroyed, this table ends up full of stale widgets.

I couldn't seem to find a reliable way of asking a widget reference if it still exists - any test either throws an error, or doesn't ever return false. This might be due to the destroyed widget now being a "broken" reference ? In any case, I'm not too happy with having to do that anyway, but equally don't fancy controls needed to "unregister widgets" when they close down.

Will have a looked at "named widgets" too, whatever they are :)

Finally, I think your point about the onUpdate handler using the LUA "enviornment" as the handler key is quite likely the core of the problem. Will have a think if I can code around that, if the above techniques don't yield an answer.

[Moon Wizard]

The named widgets keep you from having to track the widgets at all. Just set the name of the widget using the widget.setName(<name>) function, and then query for the existence of a widget using the windowcontrol.findwidget(<name>) function.

Cheers,
JPG

[S Ferguson]

It would take quite a while to debug exactly why this is happening, since the Lua/C and closure interfaces are fairly obfuscated when debugging. Plus, you are adding in a lambda function into the equation, and I'd have to research how scoping works in that case.

Regards,
JPG

Just as a brief aside, functions are first-class members in Lua, the inline lambda function being the quintessential "bare-minimum" function call. Scoping is lexical in all cases (in est lambda functions are local to the function it's defined in). That's probably why the closure interfaces are so obfuscated (within the FG-Lua sandbox that is); the meta-functional aspects of the language are hidden a layer under, it's often more difficult to fully commit to lambda functions and determine their results. It makes me wonder how those crafty Brazilians ever made it a language in the first place.

Cheers,
SF

[phantomwhale]

So I went with named widgets, and the as-yet-undocumented addHandler function on the DB package.

This avoids issues where a client doesn't have the database node during creation. I have to track when controls are deleted, and remove them from the table of registered controls (otherwise things can slow down over time)

Overall, I ended up with this:



local widgetControls = {}

function onInit()
DB.addHandler("charsheet.*.main.bennies", "onUpdate", updateBennies)
end

function updateBennies(nodeBennies)
updateBennyWidgets(nodeBennies.getChild("...").getName())
end

function updateBennyWidgets(identity)
local compactTable = false
local controlsForIdentity = widgetControls[identity] or {}
for index, control in ipairs(controlsForIdentity) do
if control.isVisible then
local bennywidget = control.findWidget("benny")
local bennytextwidget = control.findWidget("bennytext")
local nBennies = DB.getValue("charsheet." .. identity .. ".main.bennies", 0)
if nBennies <= 0 then
bennywidget.setVisible(false)
bennytextwidget.setVisible(false)
elseif nBennies > 4 then
bennywidget.setBitmap("benny_stackn")
bennytextwidget.setText(nBennies)
bennywidget.setVisible(true)
bennytextwidget.setVisible(true)
else
bennywidget.setBitmap("benny_stack" .. nBennies)
bennywidget.setVisible(true)
bennytextwidget.setVisible(false)
end
else
compactTable = true
controlsForIdentity[index] = nil
end
end
if compactTable then
TableUtil.compact(controlsForIdentity)
end
end

--
-- Adds a Benny Widget to the supplied windowcontrol
--
function createBennyWidget(control, identity, posx, posy)
local bennywidget = control.addBitmapWidget("benny_stack1")
bennywidget.setPosition("topleft", posx, posy)
bennywidget.setVisible(false)
bennywidget.setName("benny")

local bennytextwidget = control.addTextWidget("sheetlabelsmallbold", "")
bennytextwidget.setPosition("topleft", posx-1, posy+1)
bennytextwidget.setVisible(false)
bennytextwidget.setName("bennytext")

local controlsForIdentity = widgetControls[identity] or {}
table.insert(controlsForIdentity, control)
widgetControls[identity] = controlsForIdentity
updateBennyWidgets(identity)
end


I wonder if DB.addHandler should be used more to get around the problem of client controls being initialized, but the databasenode hasn't yet been created ? In any case, this has fixed up my problem, by keeping a single event function rather than trying to create an anonymous closure per control (which never went out of scope either)

Cheers,
Ben (-PW-)

[Moon Wizard]

That's exactly why I created the DB.addHandler and DB.removeHandler functions, specifically to deal with node existence issues as well as a way to allow wildcards.

If your object actually closes, you can use the removeHandler call in onClose to remove the handler you added in the onInit function. (same parameters)

Also, you can use an asterosk as a node in the database path to specify as a wildcard. This works great for things like encumbrance calculations. Ex: Only 3 addHandler/removeHandler calls, instead of having every field in the inventory call back to the top to force encumbrance calculation.

Cheers,
JPG

[kalmarjan]

No worries Phantomwhale... this is way beyond over my head. Like looking at a spaceship orbiting the moon while I row my boat on the dead sea.

I look forward to learning with you lads!

[S Ferguson]

That's exactly why I created the DB.addHandler and DB.removeHandler functions, specifically to deal with node existence issues as well as a way to allow wildcards.

If your object actually closes, you can use the removeHandler call in onClose to remove the handler you added in the onInit function. (same parameters)

Also, you can use as a node in the database path you specify as a wildcard. This works great for things like encumbrance calculations. Ex: Only 3 addHandler/removeHandler calls, instead of having every field in the inventory call back to the top to force encumbrance calculation.

Cheers,
JPG

Could you be so kind as to give an example of how exactly the encumbrance calculations would appear code-wise? I'm dealing with an issue like that now.

[Moon Wizard]

This is the relevant code from the 3.5E reboot I am working on. This script code is linked to the windowlist object that holds the items on the character inventory page.



function onInit()
local sNode = getDatabaseNode().getNodeName();
DB.addHandler(sNode .. ".*.carried", "onUpdate", onEncumbranceChanged);
DB.addHandler(sNode .. ".*.weight", "onUpdate", onEncumbranceChanged);
DB.addHandler(sNode .. ".*.count", "onUpdate", onEncumbranceChanged);
end

function onClose()
local sNode = getDatabaseNode().getNodeName();
DB.removeHandler(sNode .. ".*.carried", "onUpdate", onEncumbranceChanged);
DB.removeHandler(sNode .. ".*.weight", "onUpdate", onEncumbranceChanged);
DB.removeHandler(sNode .. ".*.count", "onUpdate", onEncumbranceChanged);
end

function onEncumbranceChanged()
CharManager.updateEncumbrance(window.getDatabaseNo de());
end


Let me know if you want to see the details of the updateEncumbrance function, but this is how it gets called.

Cheers,
JPG

[S Ferguson]

This is the relevant code from the 3.5E reboot I am working on. This script code is linked to the windowlist object that holds the items on the character inventory page.



function onInit()
local sNode = getDatabaseNode().getNodeName();
DB.addHandler(sNode .. ".*.carried", "onUpdate", onEncumbranceChanged);
DB.addHandler(sNode .. ".*.weight", "onUpdate", onEncumbranceChanged);
DB.addHandler(sNode .. ".*.count", "onUpdate", onEncumbranceChanged);
end

function onClose()
local sNode = getDatabaseNode().getNodeName();
DB.removeHandler(sNode .. ".*.carried", "onUpdate", onEncumbranceChanged);
DB.removeHandler(sNode .. ".*.weight", "onUpdate", onEncumbranceChanged);
DB.removeHandler(sNode .. ".*.count", "onUpdate", onEncumbranceChanged);
end

function onEncumbranceChanged()
CharManager.updateEncumbrance(window.getDatabaseNo de());
end


Let me know if you want to see the details of the updateEncumbrance function, but this is how it gets called.

Cheers,
JPG

Actually I wouldn't mind seeing the encumbrance code. If it's too long to post, you could e-mail or PM me it.

[Moon Wizard]

Here it is:



function updateEncumbrance(nodeChar)
local nEncTotal = 0;

local nCount, nWeight;
for _,vNode in pairs(DB.getChildren(nodeChar, "inventorylist")) do
if DB.getValue(vNode, "carried", 0) == 1 then
nCount = DB.getValue(vNode, "count", 0);
if nCount < 1 then
nCount = 1;
end
nWeight = DB.getValue(vNode, "weight", 0);

nEncTotal = nEncTotal + (nCount * nWeight);
end
end

DB.setValue(nodeChar, "encumbrance.load", "number", nEncTotal);
end


Cheers,
JPG

[Moon Wizard]

This code will actually work better for you in the inventory list. It triggers more often to recalculate encumbrance whenever any item field is changed, but it also captures item deletion events. I'm adding a separate onChildDeleted event in v3.0.



function onInit()
local sNode = getDatabaseNode().getNodeName();
DB.addHandler(sNode, "onChildUpdate", onEncumbranceChanged);
end

function onClose()
local sNode = getDatabaseNode().getNodeName();
DB.removeHandler(sNode, "onChildUpdate", onEncumbranceChanged);
end

function onEncumbranceChanged()
CharManager.updateEncumbrance(window.getDatabaseNo de());
end


Cheers,
JPG