FGU not initializing allocating memory/tables in 'record_spell_entry.xml'

[bratch9]

Hi,

I'm getting a memory issue with just FG Unity, works fine in FG Classic.. When adding to the 'record_spell_enrtry.xml', normally I do this with an extensions but wanted to show it still has the same issue when done as a direct entry to the rule set and using the most basic standard element available. ( So no confusion with any of my code or any code at all, as none is been used !! )

In the 'issues.zip', ive included a 'campaign' and a ruleset 3.5E edited file. ( see changes.png, for the extent of the changes... not much.. )

This issue happens, ONLY on the first time LOAD/RELOAD, when you add a spell to the character sheet spell class and assign it a level. At this point you will get the error message shown in 'view.png', you can also see that on the character sheet the 'correct' change with the added number happened and also you can see that in the db.xml this also was added correctly. Just this 'odd' error on the first case showing up !!

The error comes from memory not been set up properly when the 'spell_class.lua' line 64 is run, see 'issue.png', because in FG unity the 'subwindow' is 'nil' and hence this code can not access the 'actionsmini.getwindows()'.

In FG classis all this is configured and works without any issues, and the 'subwindow' exists as do 'actionsmini', while the 'getwindows()' should return 'nil' but because the references exist it does actually exists to return 'nil' or the list of spells at that level on the character.

While I could suggest 'nil' protecting this data line, I feel this is not the correct solution. Due to it been perfectly valid code in FG classic, and only an issue in FG Unity.

So its showing some difference for process between the 2 versions of FG.

From what I can see it looks like the [c] createChild call that starts this process, when it populates the lua stack functions to then call back into the lua process code that is running is not fully populated. ( But its hard to tell from my side of the lua.. )

This could be caused by FGU calling the lua function too early in the setup process and hence has not configured all the parts (subwindow) of the memory yet, or it could be that its 'failing' to fully setup the lua state before it transitions back into lua.

While the 'record_spell_entry.xml' is configured to generate a 'numberfield', a 'stringfield', 'tokenfield' etc.. all create the same issue... basically any of these that have a 'database' reference cause the error.

If you swap over to using the 'numbercontrol' version by changing the tags at 383&388 to 'basic_control', then you will not see the error in FG Unity. ( or the DB reference, as 'control' items dont create a DB item.. )

Its something to do with having a database reference that is causing a setup issue in the table structure been generated when FG Unity boots/reloads.

This could be some local variable that is not initialised to a known value and after the first spell add it is set to a know state? ( maybe a 'local value', instead of a 'local value = {}, type thing down in coreRPG... I just dont know the full interaction of the error... because its 'subwindow' I dont think its going to be a local variable issue, but more a memory order/init issue type thing... )

While the example is in 3.5e, it obviously causes the same issue in PF1.. Due to the same sort of code been used in PF2+SFRPG I've seen this same issue in multiple rulesets, and only on FG Unity.

Thanks, Pete

[Moon Wizard]

Subwindows are not automatically created until needed; which means that the subwindow variable can be nil on initialization. This is by the original API design by the original developers (prior to my involvement)

You will see many examples in the ruleset code for Core, 5E, 3.5E, and so on where there is a specific check whether subwindow exists before executing on it.

Regards,
JPG

[bratch9]

Subwindows are not automatically created until needed; which means that the subwindow variable can be nil on initialization. This is by the original API design by the original developers (prior to my involvement)

You will see many examples in the ruleset code for Core, 5E, 3.5E, and so on where there is a specific check whether subwindow exists before executing on it.

Regards,
JPG

Yes i understand that the code might have zero to 'n' subwindows for the spell list... ie when the spell level has no entries the 'getWindows()' will return nil and the 'vAction.Update()' loop will not run as no vActions exist.

What is missing is the list container under FG Unity, but it exists under FG Classis... ( Which suggests by design it should also exist in FG Unity.. unless you are saying FG classic should not have allocated and its a bug in FG classic... )

If I put it in a different code situation...

class someStuff
{
int a;
vector<int> list;
};

if I have a variable of 'someStuff myData;'... I understand that 'myData.list.size()' could be zero to any number in the list.... what I dont expect is 'mydata.list' to not exist..

in this code example I would not expect to have to protect for the list been missing... 'if (myData.list) { for ( int i = 0; i < myData.list.size();++i) myData.list[i]++; }' I'd do exactly as the spell.lua code does... and just ask for the number of 'subwindows.getWindows()' for nil or the list contents to loop over... the 'for' is doing the 'nil' check....


I did a quick look in the code for subwindow....

(coreRPG) record_ability.xml... has 'if header.subwindow then' so it does a protection check for a 'subwindow'.... but it is not protecting the 'header' which holds the 'sub_record_header', it is assumed that that header exists because it is an element of the xml at line 33 '<sub_record_header name="header">' in the <sheetdata> tag...


(coreRPG) tableimport.lua line 228 'w.header.subwindow.name.setFocus();', has no protection against 'subwindow' been nil... is this another code mistake ???


So even in the couple of examples coreRPG code has a mix of 'protected' 'subwindow' checks and areas of the code that do not protect for it been 'nil'... are all these cases also 'bugs' that you need to fix in core and other ruleset ?


So what are you saying.... in (coreRPG) tableimport.lua, is this code is missing an 'if w and w.header and w.header.subwindow and w.header.subwindow.name then' to make sure 'all the elements exist' before calling the 'w.header.subwindow.name.setFocus()' because if that is the case, then I can see many cases not just in (coreRPG and other rulesets) that are not correctly nil checked and hence this bug report is showing up a lot of code that you need to still go in an 'nil' check.. because you should be 'nil' checking for everything ?

But the code clearly uses a mix of protected and non protected and in some cases it fails and in some cases it needs to be checked...


In the example I'm listing, the 'error' comes from a call stack that includes the 'createChild' for the subwindow ( from what I can see )... its in the call stack... its 'TRYING' to allocate the 'subwindow' and then calling functions that exist on the 'subwindow' because it has been allocated and so it SHOULD exist at this stage... see 'stack.png' which I added in the 'spell_class.lua.. function onStatUpdate()' which is reporting the nil error...

the point been, that this is the callback of createChild FOR ADDING the subwindow... so I'd expect since that is the point of the createChild I'd expect the 'subwindow' to 'exist' when 'createChild' called the 'char_actions_deatils.lua:60' and the nested call to 'spell_class.lua:42'....

is that not the point of the 'createChild' in this case... caused by the 'manager_spell.lua:60 addSpell' ? to do the work of 'creating' the 'subwindow' [child] and then calling the subfunctions sequence that is later saying it does not 'exist' ? ( in the callback stack of the createChild for it ??? )

thats like saying..... what I think is happening in FG Unity, is that the 'createChild' ( in the code below... 'createChild' function...) it should not be calling callbacks on the thing it just 'made' if it FAILED to make it ?


code(int* list )
{
---- why would I nil protect.... since I'm in the call stack that created it...

// if list then
list[0] = 4;
}

createChild()
{
// make the child...
int* list = new int[20];
if list then
{
// call the callback stack for creating the child
code(list);
}
}

main()
{
createChild();

}

-pete

[Moon Wizard]

FGC and FGU have differences in the way certain controls are initialized. Subwindows use a delayed initialization to prevent instantiation, unless absolutely necessary. They subwindow reference may be nil at different times in FGC and FGU due to the engines differing slightly. It has always been the case that you need guards for any "subwindow" references to check to see if they are nil before using. This is not a bug.

Regards,
JPG

[bratch9]

FGC and FGU have differences in the way certain controls are initialized. Subwindows use a delayed initialization to prevent instantiation, unless absolutely necessary. They subwindow reference may be nil at different times in FGC and FGU due to the engines differing slightly. It has always been the case that you need guards for any "subwindow" references to check to see if they are nil before using. This is not a bug.

Regards,
JPG

It is a least a bug, the rule set is not 'nil' protecting... is the least of this issue...

Its also repeatable with 'subwindows'... which is why its very interesting... and why I reported it...

So lets take a look...

3spells.png... shows the character with 3 spells.. hence its got 'subwindows' yes ????

4spells.png... I've added a 4th spell... and this generates the 'subwindow' nill issue... ( But I've got 3 spells and hence 'subwindows' ???? )

5spells.png... I've add a 5th spell.. note no extra errors on the console... and still the character sheet has shown the correct spells and display perfectly... ( Just that 'odd' console error.. )

reload5spells.png.... lets do a 'console /reload'... and see the character sheet showing 5 spells ( its got 'subwindows' ??? )... you can also even see the previous console 'error' from 8:44:40AM..

reload6spells.png... I've add a 6th spell... LOOK its got yet another 'console' nill error.... but I've got spells and so I've got 'subwindows' ????


You get NO errors from FG classic...

And its only an error the first time you 'add' a spell after a load/reload of the campaign... I dont think its got anything to do with 'subwindows' as the character has always had spells on the sheet and hence should always have 'subwindows'... and I can clearly 'see' these on the character sheet and they have updated properly from 3-6 spells in progression.

I dont know if this is caused by FG Unity calling the error causing code 'by accident', ie FG Classic does not call it but FG Unity does and the memory is not correct at that point...

Or why its only a 'one' off issue after a reload, and you add more items to the list that already had items in the list...

Yes it could be from the 'delay' allocate of things... but a character with spells already on the list, 'should' already have allocated this 'subwindow' list when I open the character and look at the action page ???

and so why is it going 'missing' on the first add of a spell after reload, even when before I do the spell add I can see its 'rendered' spells and those 'subwindows' items on the character sheet ?

Its not a case that the I dont have spells and hence no 'subwindows'... what is failing is the process to call the 'getWindows()' which would list me those windows.

The code is already 'looking/dealing' with the possible zero->N subwindows... this is 'vSpell.header.subwindow.actionsmini.getWindows()' code to get them...

The issue is that 'one' time after reload that 'vSpell.header.subwindow' is nil, hence you can not access the 'actionsmini.getWindows()' to get the variable number of windows that the spells might have.

I'm sure during the character sheet render of the spell list... that I'd expect the list of subwindows to exist, since its used them to render it before I add the spell ?

Its also possible that something should not be calling the 'onStatUpdate()' in the char_action_details.lua, at this point... so maybe the nil protection goes into the 'updateAbility' functions.. which is also running a 'loop' for 'spellclasslist.getWindows()' which will protect against 'nil' for the higher level variable windows list...

So in both sections of 'loops' all cases ask for the variable list of windows and subwindows down the tree with the 'getWindows()' functions.. but in the 'one' off startup/reload case when you add a that first new item, even after looking at the character sheet, it suddenly becomes 'nil' for that first action case.

Yes it could be a delay init issue, but does the 'render' of the character action page that shows this list not 'init' that data well in advance of the 'addSpell' call that causes this issue ?

[bratch9]

FGC and FGU have differences in the way certain controls are initialized. Subwindows use a delayed initialization to prevent instantiation, unless absolutely necessary. They subwindow reference may be nil at different times in FGC and FGU due to the engines differing slightly. It has always been the case that you need guards for any "subwindow" references to check to see if they are nil before using. This is not a bug.

Regards,
JPG

I'm just going to code merge in a nil check in my extension to get rid of the error....

( But it sounds like you need to look over all the rule set(s) and find more 'windows' references that you should 'nil' protect if its a 'standard' now for FG Unity. )

But I'd also like to say adding a 'number' to a class, should not cause non-related subclasses ('window') issues to start to fail, that is just a world of crazy town..

I understand that 'delay' init is a thing for FG Unity, and maybe having a 'database' reference in a 'class' causes 100% of that class to be 'delayed'.. making these non-related 'subwindow' reference not initialised and hence causing the 'nil'..

But should the delayed class not initialise everything except the items that have 'database' references and so only the subclasses of 'database' items do not get initialised..

If without the 'database' item the class is considered 'quick' to initialise and so the class gets initialised, making the subclass ('window') initialised and hence not causing the 'nil' issue.

It feels like a world of pain, I could add something to a higher level part of the 'player' class structure and suddenly something on a different page of the character sheet is now not working... because I changes a class that was not considered for delayed initialise to one that now is a delayed initialise and hence now 'loads' of new subclasses do not get initialised and causes loads of nil protection required in non-related code.

I think its poor code design if adding a new 'number' to a class makes non-related subclasses not get initialised.... when they were getting initialised before adding the 'number'... should the new 'number' class be the only delay initiaised item....

[bratch9]

FGC and FGU have differences in the way certain controls are initialized. Subwindows use a delayed initialization to prevent instantiation, unless absolutely necessary. They subwindow reference may be nil at different times in FGC and FGU due to the engines differing slightly. It has always been the case that you need guards for any "subwindow" references to check to see if they are nil before using. This is not a bug.

Regards,
JPG

Hi Moon Wizard,

I'm sure its my bad description that is making your response of 'not at bug'... and as I look further into it maybe I'll end up agreeing with you.. But let me try and re-express it and maybe this will clear up my confusion of what is happening.


Lets work with your FG Unity 'requirement' to 'nil' guard 'subwindows' as these might be 'nil', these seems to be basically what you are confirming and hence classing my issue reporting that a 'subwindow' been 'nil' is not a bug. ( I agree with you... but that is not what I'm trying to express.. )

-------------------------------------------------------------------------------------------

Lets look at the exact code in the 3.5e rule set code, as it stands..

Within 'spell_class.lua' in 'function onStatUpdate()'...

for kAction, vAction in pairs(vSpell.actions.getWindows()) do
vAction.updateViews();
end
for kAction, vAction in pairs(vSpell.header.subwindow.actionsmini.getWindo ws()) do
vAction.updateViews();
end

You can see access to 'vSpell.header.subwindow.actionsmini.getWindows()' that is not 'guarded' for 'nil' access to 'subwindow' around the 'for' loop.

But this code works perfectly fine all the time, in FG Unity and FG Classic. It does not generate any errors, is not guard protected for 'subwindow' been 'nil', and works perfectly fine.

-------------------------------------------------------------------------------------------

Lets take a look at 'record_spell_entry.xml', as I think this part will cover why the ruleset code does not need a 'nil' protect around the 'for' and its access to 'subwindow'..

<subwindow name="header">
<anchored>
<top />
<left />
<right parent="rightanchor" anchor="left" relation="relative" />
</anchored>
<class>spell_header</class>
<activate />
<fastinit />
</subwindow>

As I under stand it the '<fastinit />' is key to the 'spell_class.lua' code not requiring any 'nil' protection.

https://www.fantasygrounds.com/refdoc/subwindow.xcp

<fastinit /> If specified, instantiate the windowinstance when this control is created. Otherwise, the windowinstance will not be instantiated until control is drawn on screen.

-------------------------------------------------------------------------------------------

So at this point, I sort of agree with you its not a bug in 'spell_class.lua' because the 'recored_spell_entry.xml' specifies that this 'subwindow' (header) should be 'fastinit' and so it is 'intialised' on 'create', and because of this the 'subwindow' will always exists and so does not need any 'if vSpell.header.subwindow ~= nil then' around the for loop that access this 'subwindow' as part of a longer access group.


So at this point, I've not expressed what bug I'm seeing and I'm basically agreeing with you that 'subwindow' depending on the situation might be 'nil' and I can see why in other parts of the source code you can see 'if header.subwindow then' checks before the use of the 'header.subwindow' item.


-------------------------------------------------------------------------------------------

Now, I'm going to try and express what I think the bug is and ask what in FG Unity is causing this issue that I'm seeing....

So if I add a '<numberfield>' into the '<windowclass name="spell_item">' which we have been talking about, then the bug I am seeing is 'suddenly' now the 'subwindow' can be 'nil' and I need to adjust 'spell_class.lua' to include 'nil' protection 'if vSpell.header.subwindow then' around the 'for' loop..

for kAction, vAction in pairs(vSpell.actions.getWindows()) do
vAction.updateViews();
end
if vSpell.header.subwindow then
for kAction, vAction in pairs(vSpell.header.subwindow.actionsmini.getWindo ws()) do
vAction.updateViews();
end
end

So by the looks, adding the '<numberfield>', which is not related to the '<subwindow name="header">', this is now causing the '<fastinit />' to not initialise on create as it is supposed to do... and hence now the original 3.5e rule set code has a 'bug' in it... that requires the addition of 'nil' protection to stop the 'error' been reported due to the 'nil' subwindow.


So to be clear I think the bug is this....

Adding a database reference item like <numberfield> into the class, is causing the not-related <subwindow> which is marked as <fastinit> to NOT be created on init and so in the 'createChild' call stack of this init process for the 'subwindow' is NIL, when it should have had a VALUE because of the <fastinit> tag. ( Part of the createChild call stack ends up making calls via ability update into onStatUpdate which access this 'subwindow' that is assumed to be VALID because of the <fastinit> tag.. )


Does this make it any clearer, or am I still missing something about the process flow that should be happening...

BUG... when adding a <numberfield> to the class, causes non-related <subwindow> which is marked as <fastinit>, to not actually init on create like it should in FG Unity, while it does in FG Classic..


I am very very sorry for the extra long post... but I have to work down my thought process and express all the details as I think is happening...

( So I agree the 'subwindow' could be 'nil' different between FG Unity and FG Classic... but the bug is <fastinit> is no longer doing its 'init' on create which is now making the 'subwindow' that is expected to have a value to not have a value and the net result is it looks like a 'subwindow' been 'nil' bug... when its not actually that at all... its a failure of the <fastinit> to do its job in FG Unity.. )

-pete