oUF-wow / oUF

WoW AddOn - Unit frame framework.
MIT License
222 stars 58 forks source link

GROUP_ROSTER_UPDATE, elements and headers #345

Closed p3lim closed 3 years ago

p3lim commented 7 years ago

For the following elements, GROUP_ROSTER_UPDATE is registered as one of the (if not the only) events triggering updates:

On headers this event is registered by default to update the headers themselves, which means the elements updates twice on every event fired, and this a spammy one.

I suggest we account for this when registering the events on these elements, either by checking manually for the unit (party/raid) or adding a method of checking whether the event is registered to the update function or not (adding obj:IsEventRegistered(event, func) to the event handler and using it to check on element enabling).

Also, for some reason, the event is not registered as unitless (as it should) in the core:
https://github.com/oUF-wow/oUF/blob/56660850136603f218337a50f2db49137b58d77b/ouf.lua#L246

p3lim commented 7 years ago

Also, while on the subject of GROUP_ROSTER_UPDATE, these elements that use it don't seem to be using the unit passed (from the unitless registration) and instead creating a new local reference to their object's unit instead.

I assume it's because of the last note in the above, but we really should start using it.

Examples: RaidRoleIndicator MasterLooterIndicator LeaderIndicator GroupRoleIndicator AssistantIndicator

Rainrider commented 7 years ago

Some of this is already addressed in the development branch - GRU is properly registered as unitless, though this is not an issue because of https://github.com/oUF-wow/oUF/blob/master/events.lua#L63-L66

these elements that use it don't seem to be using the unit passed (from the unitless registration) and instead creating a new local reference to their object's unit instead.

This is not wholly avoidable as we need to keep it that way for non-header units. The frame unit is passed along by UpdateAllElements but not by the OnUpdate script (it passes only the actual event args). At best we could do some unit = unit or self.unit.

To avoid the double-dipping @haste suggested:

to add a new attribute to oUF header children and check for that, then add a comment at both places about GRU

The other way would be to extend IsEventRegistered to check if a certain handler is registered with the event on the frame (UpdateAllElements in this case).

Either way the gain is probably not worth the effort.