Zidras / DBM-Warmane

DBM for Warmane (Icecrown, Lordaeron, Frostmourne, Onyxia)
https://discord.gg/CyVWDWS
152 stars 71 forks source link

Magtheridon: Update Timers #220

Closed ForestJ316 closed 1 week ago

ForestJ316 commented 1 week ago

Added Conflag and Quake timers, also updated the Blast Nova going into Phase 3, at least to how it appears to be working based on transcriptor logs.

Note that the Quake spell according to wowhead guide is https://www.wowhead.com/tbc/spell=30576/quake but this doesn't exist in Warmanes current database I guess, since GetSpellInfo(30576) just returns nil. So I used some other spell with the same name and icon, but the description of the spell is incorrect of course, concerning what the actual ability does.

Transcriptor logs: Transcriptor 1st pull.txt Transcriptor 2nd pull.txt Transcriptor 3rd pull.txt

ForestJ316 commented 1 week ago

As for Quake, perhaps it is better to make it either a locale-specific custom description (I would be able to provide only English though) or just have no description in the DBM options, rather than the current option? Although it probably doesn't matter that much.

Zidras commented 1 week ago

As for Quake, perhaps it is better to make it either a locale-specific custom description (I would be able to provide only English though) or just have no description in the DBM options, rather than the current option? Although it probably doesn't matter that much.

What you mean by custom description?

ForestJ316 commented 1 week ago

quakecurrentdescrip I meant this part, but now that I'm looking at the code, perhaps this functionality is not available. Or at least I didn't find any examples of it.

Zidras commented 1 week ago

quakecurrentdescrip I meant this part, but now that I'm looking at the code, perhaps this functionality is not available.

That description comes from the spellId you use in the NewCDTimer constructor. It corresponds to the tooltip spell description, thus it cannot be changed. Not unless you use a different spellId (but what would be the point? Why would the custom description be necessary? What would you want to see written?)

Zidras commented 1 week ago

Blind me for not reading the PR description -.- 30657 is the correct ID, you had a typo there

ForestJ316 commented 1 week ago

Quake New ID Using the correct ID for the timer, in DBM options there is a missing icon and description. Although the description is not that big of a deal I guess.

Zidras commented 1 week ago

Quake New ID Using the correct ID for the timer, in DBM options there is a missing icon and description. Although the description is not that big of a deal I guess.

that's intended. the spell itself does not have tooltip. The ID does need to be correct in the constructor for callback compat (weakauras for example)

ForestJ316 commented 1 week ago

Looks like they updated some voice line condition and blast nova timer yesterday/today. Seems to be yelling "I... am... unleashed!" every time he is banished with cubes now, but since I don't know if it's still a voice line for P2 start I just added a check for it instead.

Here are the new transcriptor logs: Transcriptor new 1st pull.txt Transcriptor new multiple pulls.txt

Zidras commented 1 week ago

Looks like they updated some voice line condition and blast nova timer yesterday/today. Seems to be yelling "I... am... unleashed!" every time he is banished with cubes now, but since I don't know if it's still a voice line for P2 start I just added a check for it instead.

Here are the new transcriptor logs: Transcriptor new 1st pull.txt Transcriptor new multiple pulls.txt

https://www.warmane.com/bugtracker/report/124104

ForestJ316 commented 1 week ago

Then it seems it's the correct approach. He will yell it on P2 start if u kill adds before 2 min AND each time 5 cubes clicked.

Zidras commented 1 week ago

LGTM. Sadly, I did not review the logs, but with the volatility of PTR, I prefer to have it updated with the data from your tests and code. Lmk if there is anything pending from PTR testing

ForestJ316 commented 1 week ago

Not as of right now.