Something change in onShortcutDrop? Not seeing anything of note as I drill down but..

[SilentRuin]

When giving control of NPCs to my players, I have an extension which will never use the option "Token: Party Vision and Movement" because it gives undesired behaviors in our games we don't want and we allow any faction to be played because sometimes I have a player run the bad guys against the other players. This has its own functionality which still works fully as designed in TEST, but it had an additional ability to insure if the normal FGU operation of dragging an CT NPC link onto a player portrait will get all the settings required for my extension to work and it would work the same as if someone had been granted access to all the NPCs (in this case it would be just the one dragged onto portrait instead of all of them).

Hence, I have extensions that override the onShortCutDrop functionality to insure that any drop of a CT link onto a player portrait will give it full ownership to that player for the NPC. This has a lot of ramifications - all good - that allow players full control of the NPC and allow them access to the same CT action lines that a host would have without regard for faction or requiring that unwanted party vision option (Example below)...

53170

While the ability to share all NPCs to a player still work fine when using my mechanism for allowing any NPC to be controlled by player - the overloaded onShortCutDrop to trigger this same behavior - no longer works.

I'm in the process of trying to figure out why it still pops up the NPC form on player side but somehow the CT client side logic no longer gets triggered and the addholder that was applied does not seem to stick.

Normally I'd just thrash this out on my own - but my initial looking into this on TEST using winmerge to see any changes in onShortCutDrop are not showing anything obvious. I'm going to be down to throwing a whole bunch of prints to try to figure out what has changed to cause this not to work when onShortCutDrop in TEST is called - but I'm also hoping - maybe someone reading this can give me a hint where to look to find out what has changed to make this ownership setting cease to work for me when going down this particular pathway.

Anyway - if I find out what it is before someone tells me what it might likely be - I will post here. But I'm worried its going to be in some indirect not obvious place where this got changed.

[SilentRuin]

And.... I am stumped. I found out what it is. My drop handler registration is no longer getting called at all.

CharacterListManager.registerDropHandler("shortcut", onShortcutDrop);

The only thing I can see that changed is that someone has renamed the FGU one to this

CharacterListManager.registerDropHandler("shortcut", CharacterListManager.onShortcutDrop)

Now granted I wrote this eons ago (part of my first extension), can someone explain why this is or why it ever worked?

Does the registerDropHandler have to match the name exactly or it will simply not be called?

For sure, I can think of one work around to get around this, simply override CharacterListManager.onShortcutDrop - which essentially is what I've done as I call the original version in my current code.

But I'd rather understand why it no longer triggers at all the way I have it. Any ideas?

[Moon Wizard]

It's an order of operations situation.

The code was changed to try to move some items that were being overriden from onInit to onDesktopInit to allow easier overriding. After reviewing it again, it looks like anything that has an explicit registration function needs to be moved back to onInit, so that the default behavior is set up earlier in the initialization by default and so that it can be overriden using the explicit functions in extensions (which is what you are trying to do). I just did a review of all the code like that in CoreRPG and moved some stuff around.

I'll be pushing a new build today or tomorrow with those changes; and you can check again to see if it's still an issue for you. If it is still an issue, you should be able to move your drop handler override to the onDesktopInit and that should resolve.

Regards,
JPG

[SilentRuin]

It's an order of operations situation.

The code was changed to try to move some items that were being overriden from onInit to onDesktopInit to allow easier overriding. After reviewing it again, it looks like anything that has an explicit registration function needs to be moved back to onInit, so that the default behavior is set up earlier in the initialization by default and so that it can be overriden using the explicit functions in extensions (which is what you are trying to do). I just did a review of all the code like that in CoreRPG and moved some stuff around.

I'll be pushing a new build today or tomorrow with those changes; and you can check again to see if it's still an issue for you. If it is still an issue, you should be able to move your drop handler override to the onDesktopInit and that should resolve.

Regards,
JPG

No worries - I've decided to play it safe as I usually do as this was early extension - I never override a registered thing usually just the function these days. And I've changed it to just do my normal thing...

saveonShortcutDrop = CharacterListManager.onShortcutDrop;
CharacterListManager.onShortcutDrop = onShortcutDrop;

where I call the save* version in my version (with whatever I wanted to add).


All works - and is good. Not sure if I have any other register things from the old days but I've not noticed anything else in TEST so far.

[SilentRuin]

It's an order of operations situation.

The code was changed to try to move some items that were being overriden from onInit to onDesktopInit to allow easier overriding. After reviewing it again, it looks like anything that has an explicit registration function needs to be moved back to onInit, so that the default behavior is set up earlier in the initialization by default and so that it can be overriden using the explicit functions in extensions (which is what you are trying to do). I just did a review of all the code like that in CoreRPG and moved some stuff around.

I'll be pushing a new build today or tomorrow with those changes; and you can check again to see if it's still an issue for you. If it is still an issue, you should be able to move your drop handler override to the onDesktopInit and that should resolve.

Regards,
JPG

Turns out I do have some other register calls - though not sure your changes have affected any of them. Mostly in this area....

ActionsManager.register*

I'll wait to check all those until after you make whatever updates your making. I do all my setup operations usually in OnInit() with a few exceptions (because I had to do the desktop init for DB data)

[Moon Wizard]

Just a heads up that you might have to change it back to the way it was before after these changes. Again, it's an order of operations things, and completely tied to the fact that each function instance has a unique ID.

Regards,
JPG

[SilentRuin]

Just a heads up that you might have to change it back to the way it was before after these changes. Again, it's an order of operations things, and completely tied to the fact that each function instance has a unique ID.

Regards,
JPG

You were correct - now overriding the registered function (new way) does not work - likely due to ID you mentioned. And the setting it back to the way it worked in LIVE (registering it) now works in TEST. So I guess whatever you fixed - worked - thanks!

[SilentRuin]

I am curious how more than one extension can modify the same registered function. I understand how overriding a function then calling the original can stack functionality of extensions (my save stuff above) but not how me registering a function that replaces the keyword can have stacked logic. As I understand it after seeing this it’s only ever going to be the last one that executes. I guess no one else is overriding this as I’m not aware of any reported conflicts.

[SilentRuin]

Now I get this error:

[6/17/2022 10:39:13 AM] [ERROR] Script execution error: [string "scripts/manager_campaigndata.lua"]:105: attempt to call field 'handleAnyDrop' (a nil value)

I can only imagine something has been broken where multiple extensions previously had the ability to override things and put their stamp on it without wrecking any other extension.

The extension I have doing this does this in my onInit (as many other extensions do for different functions - which I'm now beginning to wonder if they all haven't been broken or how extensions can modify the same source)



-- Make sure this function can support treasureparcel sheets
savehandleAnyDrop = ItemManager.handleAnyDrop;
ItemManager.handleAnyDrop = handleAnyDrop;


Let me know if I need to make a new thread for this one - but it seems along the same lines as to what is going on here.

Gist of this current problem is I can no longer drop NPCs on the CT.

And I am worried that any ability to catch every instance to add my stuff into is now somehow being made into "there can be only one" extension modifying these type of overrides.

[SilentRuin]

Actually now I'm really confused - that error does not occur in the extension that overrides that code. I narrowed it down to an extension which as far as I can tell never touches handleAnyDrop. So not sure what is going on.

[SilentRuin]

All I can say is the function that works in LIVE and fails in TEST is failing when an NPC or PC is dropped onto the CT at this point...

[6/17/2022 10:39:13 AM] [ERROR] Script execution error: [string "scripts/manager_campaigndata.lua"]:105: attempt to call field 'handleAnyDrop' (a nil value)

which is here


if sTarget == "item" then
ItemManager.handleAnyDrop(DB.createNode("item"), draginfo);
return true;
elseif sTarget == "combattracker" then
return CombatManager.handleAnyDrop(draginfo);
else


All I can say is that the extension is overriding a number of things in its onInit() (works fine in live)... here is a sample of what its doing (no idea what is suddenly causing this problem in TEST though I suspect its this reshuffle of order of things I'm hearing about)...



-- OOB message for notifying User to update CT Assistant GM stuff.
OOBManager.registerOOBMsgHandler(OOB_MSGTYPE_USERF ILTERCT, handleUserFilterCT);

-- OOB message for notifying User to select token on their map.
OOBManager.registerOOBMsgHandler(OOB_MSGTYPE_USERS ELTOK, handleUserSelectToken);

saveupdateActiveHelper = TokenManager.updateActiveHelper;
TokenManager.updateActiveHelper = updateActiveHelper;

--Debug.console(User.getUsername() .. "(" .. tostring(Session.IsHost) .. ") -> manager_combatgroups:onInit called");
if not Session.IsHost then
return;
end

OptionsManager.registerCallback("OLD_EYES", onOptionOldEyes);

-- In order to support combat group visibility need to override these functions in CoreRPG\scripts\manager_combat.lua
saveisCTHidden = CombatManager.isCTHidden;
CombatManager.isCTHidden = isCTHidden;
savegetSortedCombatantList = CombatManager.getSortedCombatantList;
CombatManager.getSortedCombatantList = getSortedCombatantList;
savesetFactionTargets = TargetingManager.setFactionTargets;
TargetingManager.setFactionTargets = setFactionTargets;
savehandleFactionDropOnImage = CombatManager.handleFactionDropOnImage;
CombatManager.handleFactionDropOnImage = handleFactionDropOnImage;
saveupdateTooltip = TokenManager.updateTooltip;
TokenManager.updateTooltip = updateTooltip;
saveupdateTokenColor = TokenManager.updateTokenColor;
TokenManager.updateTokenColor = updateTokenColor;
saveupdateSizeHelper = TokenManager.updateSizeHelper;
TokenManager.updateSizeHelper = updateSizeHelper;
saverollInit = CombatManager2.rollInit;
CombatManager2.rollInit = rollInit;
-- Override TokenManager Token overrides to include our new type of token
if Session.IsHost then
Token.onContainerChanged = onContainerChanged;
Token.onAdd = onTokenAdd;
Token.onDelete = onTokenDelete;
Token.onDoubleClick = onDoubleClick;
end
Token.onHover = onHover;
-- when user is state changes we need to apply client CT filter
User.onIdentityStateChange = onIdentityStateChange;

-- Make sure owned NPCs can be seen by player.
savemessageResult = ActionsManager.messageResult;
ActionsManager.messageResult = messageResult;
-- Make sure we don't display turn message on client if not owned
saveshowTurnMessage = CombatManager.showTurnMessage;
CombatManager.showTurnMessage = showTurnMessage;



I have no idea why TEST is thinking the CombatManager.handleAnyDrop call is nil. As far as I can tell I'm not doing anything with that function at all.

[SilentRuin]

Found it. This is your guys bug.

Not sure why it only triggers when my extension is in play - but as I referenced CoreRPG\scripts\manager_campaigndata.lua calls CombatManager.handleAnyDrop which evidently no longer exists. Its CombatDropManager.handleAnyDrop now - or so I'm guessing. Though I admit its all very confusing.

[SilentRuin]

Apparently another extension has stopped working for its overridden functions now. Seems to not work at all for dropping an item into the parcel when my overrides are in effect. Worked in LIVE. Is this another case of some kind of weird ordering change?



-- Make sure this function can support treasureparcel sheets
savehandleAnyDrop = ItemManager.handleAnyDrop;
ItemManager.handleAnyDrop = handleAnyDrop;

-- Make sure this function can support treasureparcel sheets
saveaddItemToList = ItemManager.addItemToList;
ItemManager.addItemToList = addItemToList;

-- Make sure this function can support treasureparcel sheets
savesendItemTransfer = ItemManager.sendItemTransfer;
ItemManager.sendItemTransfer = sendItemTransfer;


I'm getting worried that a number of my overrides are no longer functioning. I guess I need a full explanation on the new rules on how to override functions (where I don't stomp other peoples overrides but just add to them as before in LIVE).

[Moon Wizard]

I've pushed a fix for the incorrect function call.

There are no new rules. It's all about order of operations.

For global scripts, there is no guarantee of initialization order, so you can't assume any action that you perform in an onInit function occurs before/after any other onInit function. (Similarly for onDesktopInit).

For CoreRPG, I moved all the initialization calls which involved calling a built-in registration function into the onInit function; and I moved all the initialization calls for handlers (i.e. User.onLogin = fn) into the onDesktopInit. By doing this, it allows extensions to override with the built-in registrations in most situations; while allowing the handlers to be overriden by onInit calls in extensions.

When in doubt, you need to just step back and look at the ordering, and add simple debugs to make sure whether your version of the functions is being called or not.

Regards,
JPG

[SilentRuin]

I've pushed a fix for the incorrect function call.

There are no new rules. It's all about order of operations.

For global scripts, there is no guarantee of initialization order, so you can't assume any action that you perform in an onInit function occurs before/after any other onInit function. (Similarly for onDesktopInit).

For CoreRPG, I moved all the initialization calls which involved calling a built-in registration function into the onInit function; and I moved all the initialization calls for handlers (i.e. User.onLogin = fn) into the onDesktopInit. By doing this, it allows extensions to override with the built-in registrations in most situations; while allowing the handlers to be overriden by onInit calls in extensions.

When in doubt, you need to just step back and look at the ordering, and add simple debugs to make sure whether your version of the functions is being called or not.

Regards,
JPG

As I did - but its now changed from LIVE to TEST. So looking for a hint as to what I do to solve the ones that no longer work. Am I to move them to the desktopinit? I mean the whole purpose is that they get added onto whatever is triggered. We need to know the rules for compensating for things you've shifted around making our stuff no longer work. The whole point of extensions is that there IS an order. Load order between extensions - and as far as ruleset and engine it was always supposed to be overriden. Now it appears things that were overriden no longer are.

If I'm understanding you are you saying things that work leave them be and things that don't work move them from onInit where everything function override was - to desktop init where DB override stuff was? The problem is - as you pointed out in the first fix - I don't understand the new rules in TEST.

If my functions - in onInit - were able to override the old functions by doing things like

savehandleAnyDrop = ItemManager.handleAnyDrop;
ItemManager.handleAnyDrop = handleAnyDrop;

And now don't seem to work (this is example I have no idea if this one still works or not) - I need a solution for what to do now. I mean its really throwing me for a loop - the idea is that all extensions can override some function to have their stuff happen and still allow other peoples stuff to happen. I used to think I knew the rules - drop function overrides in onInit and if you play nice calling the original code still you and anyone's else stuff is still workable.

Now I fix one thing to move from an onInit register override (which I can see as only working ever for one extension - bad stuff to me) to do the above function override - which works - then you fix again - and that ceases to work making me call the register function again. And now I also find - some - not all - overrides no longer work at all.

I need some kind of pattern to understand what is going on so I can play nice with others as I do in LIVE. As I can't believe we are making TEST not allow that anymore. I figure I'm missing something simple here as you moving things to onDesktopInit - I'm not sure how to solve that. Do I now have to move all my overrides to onDesktopInit? Will that insure everything is done properly and nothing in onInit sneaks in some operation that is using pre-my stuff modifications?

[Moon Wizard]

I think you need to step back, and focus on one item at a time. You are making this out to be some larger "process" than it really is. If you are overriding built-in functions in a global script, then that needs to happen in onInit.

If you want to work through items, let's focus on one at a time with very narrow focus on the particular problem. I find writing out extensive posts does not help narrow things down, and often muddies the water.

So, let's start with ItemManager.handleAnyDrop.
* There is no reason why your ItemManager.handleAnyDrop would not override the function. Because it is not used in any registration functions nor saved off anywhere, it is only called when needed.
* I have attached a simple extension that performs a simple replacement like you are doing; and it works fine.

Regards,
JPG

[SilentRuin]

I think you need to step back, and focus on one item at a time. You are making this out to be some larger "process" than it really is. If you are overriding built-in functions in a global script, then that needs to happen in onInit.

If you want to work through items, let's focus on one at a time with very narrow focus on the particular problem. I find writing out extensive posts does not help narrow things down, and often muddies the water.

So, let's start with ItemManager.handleAnyDrop.
* There is no reason why your ItemManager.handleAnyDrop would not override the function. Because it is not used in any registration functions nor saved off anywhere, it is only called when needed.
* I have attached a simple extension that performs a simple replacement like you are doing; and it works fine.

Regards,
JPG

Fair enough. I'll find what specifically is not working (right now dropping Item into my Parcel sheet - modified - in TEST). And make sure I retest the functionality in each of my extensions. Right now I'm only aware of issues with Map Parcels (what I just mentioned) and Death Indicators (the "overview" will need to be compensated for in tokenfield placement).

If I find an issue that appears to be an error I'll post back here. Will also post back whatever my solution/find is for the Item->Parcel placement failure (and any other anomalies I've not yet tested).

And your fix solved the PC/NPC dropping into CT issue - so thanks.

[SilentRuin]

To keep anyone else reading this for similar test issues - my Death Indicators solution was to rip out all the old setAnchor code buried in lua and after much trial and error (anchors set in reverse order are truly weird) I will be solving my insert of the tokenfield into the header with this...



<windowclass name="charsheet_overview" merge="join">
<sheetdata>

<!-- Define the death indicator token -->
<token_death_indicator name="death_indicator_token" insertbefore="token">
<anchored to="rightanchor" width="45" height="45">
<top offset="1" />
<right anchor="left" relation="relative" offset="-2" />
</anchored>
</token_death_indicator>


</sheetdata>
</windowclass>


This appeared to solve my issues. Next solution I post will be whatever is causing my item->parcel drop ceased working. When I solve it.

[SilentRuin]

Appears that ItemManager.addItemToList has been rewritten - as I overloaded this function - I'm guessing somehow it ends up no longer doing anything in my version. Will have to figure out what was changed - and how to reinsert my requirements back into the code and where it will need to go as the functionality has been broken out into a helper function.

In fact the entire item to list functionality has been broken apart and spread all over the place. I'll have to redo the logic completely from scratch. Before it was all in one place and could be overridden there now - not so sure I'll have to figure out line by line what things are being done and where.

[SilentRuin]

Looks like addItemToList2 in 5E was removed - and that gets triggered in my override when I'm doing my stuff - calls original addItemToList when its not my stuff. As I originally processed this the way LIVE did for my side of things I guess I have to figure out why addItemToList2 has been removed from 5E or what is done now instead of using it.

Fixed. As addItemToList2 is no longer supported in 5E ruleset you have to check the libarary reference yourself...



if sClass == "item" then
DB.copyNode(nodeSource, nodeTemp);
bCopy = true;
elseif ItemManager2 and ItemManager2.addItemToList2 then
bCopy = ItemManager2.addItemToList2(sClass, nodeSource, nodeTemp, nodeList);
elseif LibraryData.isRecordDisplayClass("item", sClass) then
DB.copyNode(nodeSource, nodeTemp);
bCopy = true;
end

[SilentRuin]

I'd also like to request that we get a heads up on when TEST is about to go LIVE so we have an idea when to be ready to drop extensions (for me 5 of 7) that we maintain which need to contain fixes for this new version. As once it drops - extensions will be busted in varying degrees until the extensions are updated on DMsG (legacy) and FORGE.

[Moon Wizard]

The ItemManager already has that code in ItemManager.helperAddItemCopyToTempNode (from post #20)

The target date for ruleset update release is week of July 12.

JPG

[SilentRuin]

The ItemManager already has that code in ItemManager.helperAddItemCopyToTempNode (from post #20)

The target date for ruleset update release is week of July 12.

JPG

Thanks for the date.

As far as that code reference, I had to Debug.console my way into determining that the 5E implementation of addtoitemlist2 was removed. Keep in mind I had no problem with the original code I called for the original behavior - it was my override for things I do that did the same sort of thing was busted because I had to discover on my own that that code reference line you mention had been changed. That code reference with the difference between LIVE and TEST would have been handy to know before I went through all that to find it. Obviously, I had already (eventually after lots of pain searching) found it to make that highlighted part above in red change I made on my side of things.