Improving and Unifying bossmod callbacks
MysticalOS opened this issue · 16 comments
This isn't an overnight thing and something to be worked on by next tier or at least by next expansion. I have many notes on it here:
DeadlyBossMods/DBM-Unified#231 (comment)
But basic gist is, BW and DBM callbacks are mostly cross compatible as is, and for majority of auras that aren't using a count or stupidly using msg text scanning when they could be using spellID, they work 1 for 1 just fine. However, when an author writes a weak aura, they target either bw or DBM and not both. It results in a weak aura that COUNT work with both mods, but doesn't.
There are a few steps to be able to unify the callbacks and they are basically
- Get on same page with stage callback (bw and DBM problem not WA)
- Support count arg in timer and announce objects so spellid + count can be used instead of string matching (DBM just did this, so did BW. It's better this is done in boss mods and not as reliant for weak aura's to do it in your handler cause this also just benefits all addons better or WAs using custom code instead of built in trigger parsing.)
- BW needs to improve type filter. DBM lets you filter by ALL timer types, but bw is just "cast" and "cd" which means an author of a weak aura still needs to scan timer text itself for any timer object not using a spellID (such as a stage timer). This is the reason why the DBM type filter lets you choose a multitude of types. A good weak aura should be able to avoid requiring localized text scanning.
What needs doing on WA short term:
- Adding support for the recently added count filter i just pushed to DBM. (DeadlyBossMods/DBM-Unified@a9dae8e)
- More stuff as BW adds it?
- Scrap the "timer id" field on DBM timer start config. It's confusing and should never be used here. That arg is for custom code handlers only such as timer replacement weak auras or other addons like player which need unique timer guids (that's what timerId is basically). for WA general triggers though. Just need spellid and message like BW timer start
Long term:
- After BW and DBM have gotten more unified (on same page), merge callbacks together
- Create an even bigger emphasis to stop using msg text scanning when we're both providing ways to avoid doing msg text scanning. There will probably still be super niche cases it's still needed but typically, they're over used when they shouldn't be and not only does it cause conflicts if boss mods don't match timer/alert text, but also in multi language support.
Having one trigger for both would indeed be better for users.
Create an even bigger emphasis to stop using msg text scanning when we're both providing ways to avoid doing msg text scanning
The solution to make them stop using text scanning (and spellid which is not straightforward to get if you don't know where to get it) is to offer a way to select the boss/ability from wa's gui
Which would ideally be done by:
- select extension
- raid or dungeon?
- select raid (or dungeon)
- select boss
- select ability
but for that to work for both is .. another story
I just for most part most WA authors know how to make a spellid weak aura. in fact most already do this, which is again why most simply "just work" by changing DBM to BW or BW to DBm in trigger handler.
The largest issue i saw in all weak auras i reviewed this tier was almost exclusively tied to counts. A weak aura that was looking for heart 4, darkness 3, and only way to do that was to scan the text cause spellid alone wasn't enough. Adding a count filter solves this almost entirely. The remaining niche cases of using text scanning is for a non spellid timer like phase. there is already a work around for that in DBM callbacks/auras but not in BW currently.
I reviewed the callbacks from both mods and they genuinely could be merged quite easily solving the type filter and stage callbacks to be on same page. After that, BW and DBM just make sure to collaborate pretty well on ensuring we use same spellids in option keys and stage denominators, etc and a unified trigger will always work as expected.
But yeah solving the "just don't use text" is harder from a non skilled dev. the pack authors know how to do things and will properly use combined triggers but your average joe shmoe will still just plugin text, but in that regard it doesn't matter too much cause they're less likely to be uploading it to wago and having thousands download it. So I think mostly if we make it POSSIBLE not to need text, and at least point them to how, they'll figure it out. They're already pretty good about using spellId when they can (at least in all the major packs).
Your gui idea would work though if again you knew both boss mods were using same spellId and stage denominators. Which is why i say is let WAs solve WA parts and let DBM and BW worry about "being on the same page".
EDIT:
Didn't realize you actually already supported count, then i'm really baffled why so many people used string matching. you already parsed out the count yourselves with bar.count = msg:match("%((%d+)%)") or msg:match("((%d+))") or "0"
. either way it can be done cleaner now I suppose :D
Oh, and you should just scrap DBM "timer ID". that's really not for use for weak aura triggers and almost purely for custom code handlers to identify timers by guid such as the timeline weak aura. but that should match bigwigs in only showing spellid and message. That'll solve some confusion too. nobody knows what it is and confuse it for spellid and it just also one less obstacle for unification.
Oh, and you should just scrap DBM "timer ID". that's really not for use for weak aura triggers and almost purely for custom code handlers to identify timers by guid such as the timeline weak aura. but that should match bigwigs in only showing spellid and message. That'll solve some confusion too. nobody knows what it is and confuse it for spellid and it just also one less obstacle for unification.
I don't want to break any aura made with this "Timer ID" right now, but if a new unified trigger is to be made then some options will have to be dropped, this would be one of them.
A possible migration plan, similar to what we did from old Aura trigger to Aura2/BuffTrigger2
- make new BossMod Timer (+stage+message) triggers
- automatically migrate from DBM/BW triggers to new ones when they do not use any option not handle by new one (may exclude all auras matching text)
- use warning system to mark DBM/BW specific ones as deprecate
- remove old triggers after 1 or 2 xpac
I think a unified trigger should not include a text matching
BigWigs added recently a grey "Key" text in their option to show matching spellId, i don't know if DBM has an indicator for this too, but i stand for the idea to make a tiny wizard to select spells
Oh, and you should just scrap DBM "timer ID". that's really not for use for weak aura triggers and almost purely for custom code handlers to identify timers by guid such as the timeline weak aura. but that should match bigwigs in only showing spellid and message. That'll solve some confusion too. nobody knows what it is and confuse it for spellid and it just also one less obstacle for unification.
I wanted to remove this field 4 years ago and you asked to re-add #1323 (comment)
Yeah I added ability to just have a spellId arg send with text only based timers already so I can just put any Id I want into object and the spellId field will be filled in with an ID even on text based timers. But we'll probably add whatever bw did too for parity cause it's likely what top guild weak auras will target.
and yeah at the time users were being dumb and using that field and I didn't want to break their weak auras, but at this point if they're still using it when spellid has been available for so many years, then their auras deserve to break.
Having some way of the trigger to differentiate between "nameplate bars" and normal bars would be helpful. For dbm iirc this can be achieved by checking if the bar has the guid property set for bw i do not know if those even show up in the normal callbacks (and therefore in the trigger)
BW uses an exclusive nameplate api for their bars. I'm not sure it even uses callbacks yet for 3rd party nameplates. But I guess you want to filter DBM timers that have a GUID from showing in timeline? I think for your weak aura you might still need to use separated DBM/BW events since you specifically are in fact replacing actual timer objects in each mod so. you need to parse extra args untypical of anyone using unified aura. Far as I know this PR isn't removing individual callbacks, just making them not the default since they are for niche cases like your own.
I do not see them adding a guid filter or arg to unified unless BWs starts supporting GUID in it's timer callback too (that'd be kinda nice but i don't see them needing to since again they have a separate api for adds/nameplate bars).
BTW, at some point DBM will move any nameplate based bar to it's own API too, it's just waiting on someone to have time to write the nameplate bar support for DBMs internal handler (when running on nameplate mods not supporting it like plater does). so your issue will resolve on it's own, in time.
there are plenty of people using wa as custom frontend to their timers (not only timeline style auras) but causese has some bare replacements aswell but if it's a different api then it would be even more needed to add said checkbox (or a differtent trigger that makes the other api available i guess)
btw @mrbuds i noticed in branch you had "ignore bws "cast" bars". Was that because of not supporting DBM cast/cd separation? You can do that though.Just map it to type = "cast" . The distinction exists in BW for same reason it exists in the type handler for DBM, so you can tell cooldown timer from "is casting right now" since they both use same key. DBM just takes this distinction and expands it into also " and you can also distinguish every other timer type too"
But for unified trigger, to just match BWs and unify you just continue having the "is cast"checkbox or whatever it was you were doing after mapping "if type = "cast" then cast else "cooldown" and you've mapped it to bigwigs for parity.
@Jodsderechte , any bar that's only intended for nameplates will use type = "cdnp" or "cdnpcount" soon™ then can be filtered like that instead of guid. guid will still be sent on a few bosses too but those won't use the nameplate timer type flags
It actually should be type:find("^cast")
and type:find("^cd")
because it could be castcount
, castsource
, cdcount
, cdsource
, cdspecial
too.
This is also something the bossmods will have to pay attention to - if there are multiple timers for a given key, then aura authors will use these options to tell them apart, and the two bossmods must agree on what the types are.
I noticed WeakAuras is still listening to wipe
and kill
messages from DBM to stop all timers. These haven't existed for years as they were renamed to DBM_Wipe
and DBM_Kill
(listened to by stage trigger), but they also shouldn't be needed for bars because DBM fires DBM_TimerStop
for each bar separately, and if it doesn't it's a bug that should be fixed in DBM.
well you don't need to type cd, cause for the purposes of distinction is to determine whether the timer type is an upcoming cast, or casting now. but yeah just type:find("^cast")
then cast else cd to match bw flags
While BigWigs generally uses cast and non-cast if there are two timers on the same key, there is one case in Dragonflight raids (Zskarn Shrapnel Bombs) where cd non-cast and non-cd non-cast share the key. Unless they're willing to change that, both cast-or-not and cd-or-not identification is needed or the unified trigger can't tell them apart.
Just posting here for visibility to any devs that may be tracking this. DBM already pushed full migration to key/spellid synergy with BW. it means in next DBM update every single timer and spell 100% matches BW keys for both dragonflight raids (and i'll do dungeon pass soon too).
https://github.com/DeadlyBossMods/DBM-Retail/wiki/DBM-Option-Key-Migration-List-for-Addon-WA-Devs
WA addon devs don't have to do anything with this. It just means that people who author auras can be assured that whatever Id they used in their BW only aura, will also work in their unified aura in BOTH mods. any author that did actually target DBMs Ids, will need to remap them to match IDs in this wiki (basically make them match the BW keys)
BTW just wanted to add there is literally nothing left to do on DBMs end. there wasn't much to do to begin with besides ensure matching keys/stages BW uses since they don't have any organized conventions like DBM. I even built a new system to make it even easier to do so, even on the fly mid fight as needed without even messing up DBM users saved options going forward.
I'm carrying most of burden so others don't have to. I shoulder all compatibility on my end, so long as unified exists. Things will "just work" as apple likes to say. It's same approach i've taken with chat bubbles and icons for years now.
Anyways, since mega dungeon is out, 10.1.7 has next to nothing to do, we're very free to help with whatever else you guys need for this. Just let me or @Mitalie know