[Bug] UpdateUnitResource function in resourcebar.lua

[Elnarfim]

Describe the bug You're using GetSpecialization() for detecting unit's specs but it won't work That function is for player only. not accept unitid There's a only way to detect whether unit is healer or not Check powertype 0 and assigned role

and I need to suggest to use event ROLE_CHANGED_INFORM instead of GROUP_ROSTER_UPDATE Using ROLE_CHANGED_INFORM event is good for performance In this case you must make delay about 5 sec to use C_Timer.After(). The event takes time to send information about unit role It also has no arguments so you should use allupdate function

[doadin]

That is the intended point units not in a group dont have a role from UnitGroupRolesAssigned. so when a player is solo this wont match for the only show healer. and since that statically works for player that code will ulways update for the player. Then the other part of the code (members ~= 0 or subGroupMembers ~= 0) and UnitGroupRolesAssigned(unitid) ~= "HEALER") will work for group members as we will have a unitid fed to us and UnitGroupRolesAssigned will return a role.

[doadin]

As far as GROUP_ROSTER_UPDATE and ROLE_CHANGED_INFORM will ROLE_CHANGED_INFORM when a player joins ROLE_CHANGED_INFORM wont fire so resource bar wont update till the units power changes which I think is just a uneccessary small delay? Also not helpful for when a player is trying to setup their frames with more than themselves. Which maybe doesnt matter but is our function that intensive that updating it more would hurt?

[doadin]

Honestly I don't think either are necessary UNIT_POWER_UPDATE is going to be as often as we need. What I am going to do is change it so it updates on Plexus_UnitJoined Plexus_UnitLeft and only for that specific unit that way we only update that one unit instead of all. could also rewrite the current code to pull out the spec check into a seperate function that only updates on ROLE_CHANGED_INFORM but then we would have to make a table for it and then to a table lookup, at that point im not sure its any more efficient.

[doadin]

1d649cf188478e159dc963ac66123e96d51d2c98 should be a step in the right direction

[Elnarfim]

Doesn't Plexus use RegisterMessage("Grid_UnitJoined")? It can replace GROUP_ROSTER_UPDATE event

[Elnarfim]

Plexus now checks specs in UNIT_POWER_UPDATE event. That's not good for performance. It should be done in Plexus_UnitJoined, Left, ROLE_CHANGED_INFORM event

[doadin]

close plexus uses RegisterMessage("Plexus_UnitJoined") and RegisterMessage("Plexus_UnitJoined") has now replaced GROUP_ROSTER_UPDATE event. 1d649cf188478e159dc963ac66123e96d51d2c98 .

And if I don't check spec in UNIT_POWER_UPDATE then when UNIT_POWER_UPDATE is called it sill send status gained for healers even if EnableForHealers is set as we dont check for role. The only other option would be to use role to make a table of healers then check if the unit from UNIT_POWER_UPDATE is a healer or not.

flow of code as is now: UNIT_POWER_UPDATE>PlexusResourceBar:UpdateUnitResource > checks role > Sends lost or gained flow if I removed check role: UNIT_POWER_UPDATE>PlexusResourceBar:UpdateUnitResource > Sends gained(even though we don't want gained because unit is healer)

Does that make sense now?

[doadin]

Would need mroe re-write than current. Can be done but simply removing the check is not the solution so yes and no.

[doadin]

we need some kind of role check in UNIT_POWER_UPDATE otherwise how do we SendStatusGained for only healer when UNIT_POWER_UPDATE occurs? If you have an idea im open to a pull request.

[yoshimo]

isn't https://www.curseforge.com/wow/addons/libgroupinspect useful here as you care for recent talent info?

[doadin]

@yoshimo not really? Yes we could use that but no where else is talents used in plexus so adding that whole lib just to replace the like 10 lines of code we use now is not worth.

Unless something like LibGroupInSpecT worked in classic since there is no roles in classic then that would work as well but as far as I know it doesn't?

[doadin]

the best I can come up with is using ROLE_CHANGED_INFORM and PLAYER_SPECIALIZATION_CHANGED to manage a table of healers then when UNIT_POWER_UPDATE is fired UpdateUnitResource checks if EnableForHealers setting is enabled and unit is not in table of healers. if EnableForHealers setting is enabled and unit is not in table of healers it removes the status incase it was set somewhere before then stops. Otherwise it continues on to send status.

so before it would do a role check for group and player every time a resource changed. now when a resource change occurs it just checks the table. 1 table lookup per UNIT_POWER_UPDATE faster then many GetSpecializationInfo, GetSpecialization, UnitGroupRolesAssigned, GetNumGroupMembers, GetNumSubgroupMembers calls per UNIT_POWER_UPDATE?

https://github.com/doadin/Plexus/commit/8f3715e04a087e8ac2fdac010b6546f8854896d7

[Elnarfim]

I misunderstood your code. you're right. role checking should be done in UNIT_POWER_UPDATE

[doadin]

All good, defiantly open to ideas. Thanks!