ascott18 / TellMeWhen

TellMeWhen is a combat tracking AddOn for World of Warcraft Retail and Classic
https://wow.curseforge.com/projects/tellmewhen
GNU General Public License v3.0
86 stars 11 forks source link

BigDebuffs and Threath Plates interfering #2064

Closed JohnnyUndercoverr closed 1 year ago

JohnnyUndercoverr commented 1 year ago

WoW Version

Retail

TellMeWhen Version

10.0.8

Describe the bug

While using the addons BigDebuffs and ThrethPlates together with TellMeWhen, BigDebuffs is not working, disabling TellMeWhen makes those two addons work together just fine.

Export Strings

N/a
JohnnyUndercoverr commented 1 year ago

It also shows 'Out of date' in the addon control panel although it's the latest version.

Wow_eXpAi32mft

aurelx commented 1 year ago

+1

ascott18 commented 1 year ago

This is a bug with BigDebuffs, not TellMeWhen. BigDebuffs is making assumptions about event registration ordering that won't always be true.

Consider the sequence of events when TellMeWhen is disabled:

  1. BigDebuffs loads, but does nothing until its OnEnabled hook fires (which doesn't happen until PLAYER_LOGIN).
  2. TidyPlates_ThreatPlates loads, registering for NAME_PLATE_UNIT_ADDED during its initial script execution (i.e. it executes frame:RegisterEvent as a top level statement).
  3. BigDebuffs's OnEnabled hook is called, at which point is registers for NAME_PLATE_UNIT_ADDED via AceEvent-3.0.
  4. NAME_PLATE_UNIT_ADDED fires. ThreatPlates handles the event first since it registered it first. BigDebuffs handles it second, and so since ThreatPlates runs first and has a chance to setup its nameplate frames on the nameplate, frame.TPFrame:IsShown() returns true by the time BigDebuff's event handler runs and so BigDebuffs is able to hook into the nameplate.

Now, consider the sequence of events when TellMeWhen is enabled:

  1. BigDebuffs loads, but does nothing until its OnEnabled hook fires (which doesn't happen until PLAYER_LOGIN).
  2. TellMeWhen loads, and as part of its top level script executions, registers NAME_PLATE_UNIT_ADDED with AceEvent-3.0. AceEvent now has the first registration of NAME_PLATE_UNIT_ADDED.
  3. TidyPlates_ThreatPlates loads, registering for NAME_PLATE_UNIT_ADDED during its initial script execution (i.e. it executes frame:RegisterEvent as a top level statement). TheatPlates' registration is now second in the execution order, with AceEvent's registration being first.
  4. BigDebuffs's OnEnabled hook is called, at which point is registers for NAME_PLATE_UNIT_ADDED via AceEvent-3.0. BigDebuff's registration is first in the execution order since its using AceEvent. (NB: AceEvent's events are not stored in an ordered table, so all registrations with AceEvent can be fired in a nondeterministic order, but this doesn't actually matter here since ThreatPlates doesn't use AceEvent).
  5. NAME_PLATE_UNIT_ADDED fires. AceEvent handles it first, delegating to both TellMeWhen and BigDebuffs (in no particular order). ThreatPlates handles the event second. Because ThreatPlates handles it second, BigDebuffs sees frame.TPFrame:IsShown() == false when it runs first and so does nothing.

This could happen with any addon that registers NAME_PLATE_UNIT_ADDED with AceEvent. It just so happens that TellMeWhen does this, and that "Te" is alphabetically before "Ti".

Ultimately its the responsibility of BigDebuffs to ensure that when it is examining or interacting with the state of another addon, that it is always doing that examination of that state after the third party addon has had a chance to actually update that state.

If BigDebuffs wants to eliminate possible issues with nondeterministic event registration ordering, it should start by removing its usage of AceEvent and instead manually register events with its own dedicated frame instance.

See also: https://github.com/jordonwow/bigdebuffs/issues/520

aurelx commented 1 year ago

Thanks for the detailed explanation. I see you've posted in https://github.com/jordonwow/bigdebuffs/issues/520 as well. Cheers!

ascott18 commented 1 year ago

A workaround for now would be to add TidyPlates_ThreatPlates to BigDebuffs.toc 's OptionalDeps:

## OptionalDeps: Masque, TidyPlates_ThreatPlates 

Should generally always work, unless another addon with a name alphabetically before BigDebuffs also registers NAME_PLATE_UNIT_ADDED with AceEvent (pretty unlikely that there are any such addons in existence).

cobesy commented 1 year ago

A workaround for now would be to add TidyPlates_ThreatPlates to BigDebuffs.toc 's OptionalDeps:

## OptionalDeps: Masque, TidyPlates_ThreatPlates 

Should generally always work, unless another addon with a name alphabetically before BigDebuffs also registers NAME_PLATE_UNIT_ADDED with AceEvent (pretty unlikely that there are any such addons in existence).

Is there a way to do this if you're not the developer? Like, can I do this by editing something in the addon file I download?

Backupiseasy commented 6 months ago

This issue should be fixed with this PR for BigDebuffs https://github.com/jordonwow/bigdebuffs/pull/715 and version 11.2.4 of Threat Plates.