token, onMove,onAdd,onDelete, OnContainerChanged call order and issues.

[bratch9]

Hi,
Find attached a quick change to coreRPG image.lua ( renamed image.txt to allow upload. ), basically it outputs some debug relating the token add/delete/move/conatainer change. The campaign used to do this is just an new one with the FG maps and an empty character named bob...

Because im in the image lua file, ive configured Token.onAdd = onTokenAdd for the debug output etc..

I've added gm chat about that is happening... on the gm side I've opened 2 maps and on the client 3 shared maps, before this sequence.

You can see that on the gm side with 2 maps you get 2 copies of the calls, one for both maps... even with just one token on one map !!!

On the client side with 3 maps open, this generates 3 copies of most calls.

Ive added a 'bTRUE', 'bFALSE' output for when the database node of the target (token) and the image (map) match.


Also note that the order of events are different for client and gm.

On the client it seems more in the correct order. When the token is added we first see onTokenAdd and I guess the 'first' onTokenMove to 'set' its position. Then a quick move by the player causes more onTokenMove, and then when the gm moves the token to a new map, we see some onTokenMove, and onTokenAdd, the 'first' onTokenMove and an onTokenDelete. I might suggest onTokenDelete before the onTokenAdd, as it seems sensible to 'delete' it from the current map before adding/moving it to the new map.

You will notice the client is missing 'onTokenContainerChanged' calls for some reason..

On the gm side, you can see the initial add of a token first generates onTokenMove before the onTokenAdd. The small move seems ok, with onTokenMove. But again the move to new map is an odd order, you can see 4 onTokenMove with the token on map id.0008, and then you get the incorrect onTokenMove for map id.0009 and the onTokenAdd follows again. You now get the onTokenContainerChanged calls and the onTokenDelete.

Again I'd probably expect onTokenDelete first then the onTokenAdd, not sure on the onTokenContainerChanged sequence probably last for me.

Clearing up the multiple calls on image that has nothing to do with the token generating the evens would be a help, or it needs to be made very very clear in the documentation that you will get calls on multiple open image maps for each token, even if its not on that map !!!

I think the sequence been in different orders between client and gm needs to be sorted, and I'd not expect to see onTokenMove calls before the onTokenAdd etc.. and why does client not get the onTokenContainerChanged calls..

The sequence is just broken in many ways.

Please let me know if/when a solution will be available.

Thanks, Pete

[LordEntrails]

MOD: moved to Workshop, it's the best location for these types of discussions :)

[superteddy57]

In my experience if something is not firing on the client it needs to be done on the GM Host side as they are the ones only able to perform or work with the API. I will pass this along to Moon Wizard to have a look, but at first glance that is what looks like it's doing. As for the documentation, that is still in transition as more features are added to images in general. Many of the APIs are still not in their final form. Thank you for the post and I'll inform Moon of it.

[bratch9]

In my experience if something is not firing on the client it needs to be done on the GM Host side as they are the ones only able to perform or work with the API. I will pass this along to Moon Wizard to have a look, but at first glance that is what looks like it's doing. As for the documentation, that is still in transition as more features are added to images in general. Many of the APIs are still not in their final form. Thank you for the post and I'll inform Moon of it.

The bigger issue is the host side calling onMove BEFORE the onAdd calls, so it could cause access to memory before the create call has completed. ( I assume the create function is responsible for calling the onAdd callback at its end.. )

On top of this calling these functions per image map, instead of on the map the token is actually on, might be causing other possible issues. Like are all open maps 'Line of Sight' been re-generated because all maps think something has moved on the map ?

I also dont think this is a 'workshop' discussion type of thing, these are bugs because they could cause memory access for non allocated data and they could be burning cpu/network usage for no reason. I dont need to talk about if this should or should not be happening like this.

I clearly think it should not be working like this, as its not 'normal' to have callbacks on objects that are not related. It would be like the web shop calling the 'addPurchase' to EVERY registered account instead of on just my account. Its more realistic that it would call updated on the single image it belongs to, then call callbacks on all images. If the dev team require it to call callbacks on all open images for some reason, then a MASSIVE NOTE in the documentation to make this non-normal-behavior clear and obvious to extension developers so we can put the required double checks in place.

-pete

[DCrumb]

NOTE: I am not a developer. I think that part of the problem you are seeing is that the program and scripting engine do not keep where things are located in memory, but in the database xml file. Therefore, many of the calls you are seeing are from the scripting engine checking the database to see if the token is in/on another object. I believe (haven't looked at the lua for image or imagecontrol recently) that the script checks to see if the token is where the call is from, if it isn't, return and continue on. The token was created when attached to a character sheet (or possibly the combat tracker). It is then moved from the CT to an image (hence the move before add).

[bratch9]

NOTE: I am not a developer. I think that part of the problem you are seeing is that the program and scripting engine do not keep where things are located in memory, but in the database xml file. Therefore, many of the calls you are seeing are from the scripting engine checking the database to see if the token is in/on another object. I believe (haven't looked at the lua for image or imagecontrol recently) that the script checks to see if the token is where the call is from, if it isn't, return and continue on. The token was created when attached to a character sheet (or possibly the combat tracker). It is then moved from the CT to an image (hence the move before add).

Hi,
Database items do not 'move' between the character/combact tracker and the image map. If it did than the database item would 'vanish' from the character sheet/combat tracker and appear on the image map. They are different items, when you drag off the combat tracker the item you dragged stays on the combat tracker ( you set up 'draginfo' that is passed around. ) and a 'new' item arrives at the location you dropped it. In fact the 'onDrop' call back for image is responsible for calling the 'addToken' for the image to add the token asked for in the drag information setup by the combat tracker drag event that started this. ( If that makes sense. )

Most 'stuff' on the display is a 'class' and the class can register for call backs. So the 'character sheet' is a class, and it has sub-classes for things like attribute number.. So you can think of them as nested class items. When you let go of the mouse from a drag over a 'class' its class gets the call back if registered. In a lot of cases the 'number' on the character sheet does not want to deal with the call, so does not register an onDrop callback, so the system move to a parent class and checks is it has an onDrop callback... and this repeats until you hit the main character class window. Each callback can set a return value to say what to do, return true and no further callbacks happen, return false tells the system to call items below this one. ( think if you dropped on some space around the attribute number, the 'frame' would get the onDrop call back and it needs to be passed to the child-class 'number' onDrop before passing it up towards the parent. returning nil, tells the default system to do its stuff. which normally will go to the children first until it runs out, and then pass it down to the parent to process. )

So when you drag a character token off the combat tracker,
1. combat tracker get an 'onDragStart' callback, and it sets up information in the drag info about the 'source' so the character info from the line you dragged from.
2. it 'would' then be combat tracker gets 'onDrag' updates as you drag the item around on the screen. ( combat tracker does not register 'onDrag', as it does not care. But a scroll bar would care what you do when you drag its little scroll marker... )

Then you let go of the mouse, over some random 'class'.

3. Let say you drop on the chat window, the chat window class will get an 'onDrop' callback to deal with. ( Or the image class 'onDrop' function is called if you drop on an image, it will check if it understands the draginfo, and do stuff if it does. )

4. the chat window 'onDrop' has a chance to 'change' the draginfo to 'return' any information, if needed..
5. the combat tracker 'onDragEnd' call with happen with the possibly changed drag info from the class that processed the onDrop.

At no point has a token 'moved', it will have been 'added' to the image map, but you have not touched it with your mouse yet to move it. So I see no reason to get an 'onMove' event call before the 'onAdd' event call.

I do see a reason to call the 'onMove' event after the 'onAdd' even to 'set' and initial placement/move position. And on the client this is what happens, but on the host it is out of order, which I consider as a bug.

The multiple calls to each open image map, may well be because I dont return any value, so it will return nil. The image framework might pass the event up/down the child/parent list to push an event about. But it should not be able to push from one image map to another. Think about call backs going up the character sheet and then over to a second character sheet if it was open. It 'should' not be doing that sort of thing.

But I will have a look to see if forcing 'return true' stops the extra events across on other images. But I'll also have to have a look at a character class to see if I could get events to pass from one character sheet to another character sheet when you drop onto a specific character sheet. It could be the 'default' event passing could consider the 'array of' image/character classes as a parent and pop the event along the array of items...

[bratch9]

So I took some time to include 'return true' at the end of the 4 test functions for the callbacks on image class for the Token.onAdd/onDelete/onMove/onContainerChanged calls and it makes no difference you still get per-open image callbacks into these systems.

This is expected because the documentation for these callbacks eg, Token.onAdd (https://fantasygroundsunity.atlassian.net/wiki/spaces/FGCP/pages/996644662/Token#onAdd) has no return section information. Unlike the Token.onDragStart (https://fantasygroundsunity.atlassian.net/wiki/spaces/FGCP/pages/996644662/Token#onDragStart) type functions which have the return true/false/nil control system for the callback system.

So I do think this is a 'bug' in the calling system.

On top the fact the 'host' gets 'onMove then onAdd' when you add a token and 'clients' get 'onAdd then onMove' for that add suggests another bug to do with the difference between host and client versions of the systems.

[Moon Wizard]

There is no bug here. Some events allow return values when it makes sense, and others don't. Also, return values get very complicated when implementing "handlers" (which can have multiple registrations) vs. "events" (which only call the local function). Tokens must use handlers for everything, since they don't have a "script" space per token like windows/controls.

Again, as I mentioned in the other thread; please focus first and foremost on posting what you are trying to do. Posting a wall of text that that is not succinct nor focused just confuses everyone that tries to help, including me.

Regards,
JPG

[bratch9]

There is no bug here. Some events allow return values when it makes sense, and others don't. Also, return values get very complicated when implementing "handlers" (which can have multiple registrations) vs. "events" (which only call the local function). Tokens must use handlers for everything, since they don't have a "script" space per token like windows/controls.

Again, as I mentioned in the other thread; please focus first and foremost on posting what you are trying to do. Posting a wall of text that that is not succinct nor focused just confuses everyone that tries to help, including me.

Regards,
JPG

1. why does the system generate a callback per open window when you move a token on a specific window ? ( it knows what window it is on. )

2. why on the host do you get onMove before the onAdd, but on the client you get the onAdd then the onMove events ? ( When you add a token to a map, most software assumes memory created in onAdd type events and would not expect onMove events on memory that has not been created yet. )

3. why on FGU when you move a token between maps, do you get onDelete/onAdd and onContainerChanged events but on FGC you only get onAdd and onContainerChanged events ? ( Inconsistent event callbacks. )


What I'm trying to do is show inconsistencies between FG versions and inconsistencies between host and client responses in location that they probably should be matching. Situations when things are been called out of expected order that might be causing unknown issues under the hood due to use of memory before its allocated.

It over complicates code and slows down the script system if I have to bullet proof and double check all the parameters to work out if a callback is actually a valid one or not. When its clear that the user added a token to a specific map, why would i even think that all open maps would get the onAdd (and other) callback(s) and add that sort of protections. ( I do not see these protections all over the ruleset code, but also the ruleset code does not hook onto these events so I assume its a code path not been debuged/tested because no ruleset activates this code flow. )

[Moon Wizard]

As I mentioned in the other thread, the token system was completely rebuilt, so it needs to go through fine-tuning. However, we do not guarantee that script event ordering behaviors will remain the same; because many of the internal workflows have changed to support the new FGU features. (so these are not inconsistencies but changing behaviors)

The onDelete/onAdd/onContainerChanged event order/behaviors are by design. This is because moving a token between maps is the same as deleting on the old map; and placing on the new map.

For the other items, I will need to look into the various events, and determine whether they are where I think they should be.

Regards,
JPG

[Moon Wizard]

Just did some research, and here are my comments:

* As mentioned above, the order/behavior of onDelete/onAdd/onContainerChanged events are by design. (due to fact that token data has always been a child of the image data, both in FGC and FGU; so moving is literally deleting from old map and adding to new map.)

* I verified the onAdd/onMove ordering. It's sort of a chicken and egg problem with ordering and our current design with regards to data value setting and consistency between clients. I built a new ordering of steps to consolidate the initial position information will be included in the onAdd event; so that the first onMove is no longer needed and won't be an ordering problem. I'll push as part of beta test soon; and hopefully, it won't cause any issues.

* For the remaining item (#1 above), the onMove events will be generated for every open window containing the token. We currently don't track which window is triggering the event through the entire workflow; so all token events which are fired will trigger the appropriate event in any control pointing at that image value. If you have multiple windows pointing at same image, you should expect to get all events. There are no plans to change at this time; due to the amount of work and possibility of introducing issues.

Regards,
JPG