PDA

View Full Version : ActorManager5E getWoundPercent() bug



mattekure
February 24th, 2021, 15:58
There appears to be a bug with the getWoundPercent() function in ActorManager5E. When a change has occured to the health status such as a wound being applied, the sStatus returned can be different between Host and Client. The nPercentWounded is returned correctly.

I believe this is due to lines 47-48 where the client returns the previous status without calculating a new one based on the current values. When it is the host calling the function, it calculates the new status and returns it.

SilentRuin
February 24th, 2021, 16:08
There appears to be a bug with the getWoundPercent() function in ActorManager5E. When a change has occured to the health status such as a wound being applied, the sStatus returned can be different between Host and Client. The nPercentWounded is returned correctly.

I believe this is due to lines 47-48 where the client returns the previous status without calculating a new one based on the current values. When it is the host calling the function, it calculates the new status and returns it.

The chain around my neck (free extension Death Indicator that I volunteered to maintain) appears to have this call in it multiple times. Can you describe what behavior I would see from this bug from a user perspective? It appears most cases I've looked at in this stuff is a > 1 type of check or the status value. What status value difference behavior are you seeing specifically? (this was one of those deprecated updates I had to make)

mattekure
February 24th, 2021, 16:19
The function returns 2 values, nPercentWounded, and sStatus. In this example, I took a single NPC with 10 HP, and added damage 1 HP at a time. After each wound was received, I called the function and got these return values.

You can the results as I apply each HP of damage. Both host and client return the same value for the nPercentWounded. But the client returns the status before the wound was applied, not the new status. The host returns what the status should be.

https://imgur.com/sB9y30r.jpg

Moon Wizard
February 24th, 2021, 16:25
The client pulls the status from the CT entry when possible. There was a reason why this was added sometime in the past for other rulesets; and I applied it for consistency.

Is there are particular place where this incurs an issue?

Thanks,
JPG

mattekure
February 24th, 2021, 16:28
This issue came up to me when working on an extension that applies a bitmap status indicator to the token based on how wounded you are. It was relying on the status from that function, but I was getting inconsistent results where the DM would show the "Dead" indicator, while the Players would still only show the "Critical" indicator.

I have rewritten it to only rely on the nPercentWounded since that produces consistent results for both client and host.

Moon Wizard
February 24th, 2021, 16:34
I'm wondering if I should move to attempting to calculate status directly on client in all instances; and wait until the original reason for that hook was added.

It's possible that it's no longer needed; since the function only calculates percentages for owned PCs and CT entries anyway (which we know are available on the current client by default).

Let me think on it a bit; and possibly patch next week.

Regards,
JPG

mattekure
February 24th, 2021, 16:42
Thanks, appreciate it

kevininrussia
February 24th, 2021, 17:09
I tested this with my 4E version of Death Indicator and did not receive any errors. Seems to be working as intended (knock on wood).

SilentRuin
February 24th, 2021, 17:16
I tested this with my 4E version of Death Indicator and did not receive any errors. Seems to be working as intended (knock on wood).

Thanks it looks like it is mostly called on the host. I'll check more in depth if someone reports a problem - as it does replace those dying/stable/dead things based on "Dead" status or some logic like that in one case - for the token and in one case on the CT. I just think all the logic is relegated to the host side of things and the actual modifications get passed to client in normal ways.

Moon Wizard
February 24th, 2021, 18:10
No, token widgets (decorators) and CT font colors are all local to each client; so the token health bar and CT wound coloration would be the most likely impacted.

Regards,
JPG

SilentRuin
February 24th, 2021, 18:38
No, token widgets (decorators) and CT font colors are all local to each client; so the token health bar and CT wound coloration would be the most likely impacted.

Regards,
JPG

Not token widgets - actual overlays - which are fine. This only does things when the CT entry is dead dying/stable/dead with an overlay or outright token replacement. Who can see those tiny widgets ever? Not I for sure.

Moon Wizard
February 24th, 2021, 18:46
They're all either "tokeninstance" objects, or "widget" objects on tokens; at least at the client/ruleset level.

JPG

SilentRuin
February 24th, 2021, 18:51
They're all either "tokeninstance" objects, or "widget" objects on tokens; at least at the client/ruleset level.

JPG

I'd have to check - the stuff I do is always tokeninstance so I'm assuming that is true for that thing also. But as kevininrussia sees no issues, I'm going to assume things are not running on client side so there would be no problem with this bug - as host appears to be fine. And anything updated on host and transferred to client in the engine code I assume is also fine (or he would have seen an issue on the player side). Assuming - that he was testing the player side of course.

In any case, I did my due diligence to make sure I was not obviously burned by this bug. Will have to wait to see if I'm subtly burned by it :)

mattekure
February 24th, 2021, 18:51
Thanks it looks like it is mostly called on the host. I'll check more in depth if someone reports a problem - as it does replace those dying/stable/dead things based on "Dead" status or some logic like that in one case - for the token and in one case on the CT. I just think all the logic is relegated to the host side of things and the actual modifications get passed to client in normal ways.

I just posted an update to Matjams status indicators with fixes for how I handled all of those situations. It has similar functionality to yours in placing a death indicator. I got it working for Host/Client using OOB messages to ensure the functions were getting run properly. You can check out the changes I made
https://fantasygrounds.com/forums/showthread.php?59594-5E-Matjam-s-Status-Indicators&p=582099&viewfull=1#post582099

SilentRuin
February 24th, 2021, 18:54
I just posted an update to Matjams status indicators with fixes for how I handled all of those situations. It has similar functionality to yours in placing a death indicator. I got it working for Host/Client using OOB messages to ensure the functions were getting run properly. You can check out the changes I made
https://fantasygrounds.com/forums/showthread.php?59594-5E-Matjam-s-Status-Indicators&p=582099&viewfull=1#post582099

Will do if I see the issue - for now as I said in my simultaneous post to yours - I'm assuming I'm unaffected on host only processing (without actually digging into it). But appreciated immensely the heads up on this and the potential warning of only run this thing on host side - for now I'm assuming that thing is. [Don't make me look!]