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

"Spell Learned" condition is not correctly tracking 'replacement' spells #2214

Closed RealHawkBat closed 2 months ago

RealHawkBat commented 2 months ago

WoW Version

Retail

TellMeWhen Version

11.0.5

Describe the bug

Background: As a Prot Paladin, when "Holy Armaments" is talented, when the spell "Holy Bulwark" is used, it is replaced by the spell "Sacred Weapon". When "Sacred Weapon" is used, it becomes the "Sacred Weapon" again etc. While this example calls out specific spells, I believe this bug would be present for any spell where using it replaces the spell with another.

Steps to reproduce:

  1. Log in as a Prot Paladin with "Holy Armaments" talented
  2. Create Icon A for Spell Cooldown that tracks "Holy Bulwark" (spellid: 432459) 2a. For Icon A, set the "Spell Learned" condition to FALSE, tracking spellid: 432472 (Sacred Weapon)
  3. Create Icon B for Spell Cooldown that tracks "Sacred Weapon" (spellid: 432472) 3a. For Icon B, set the "Spell Learned" condition to TRUE, tracking spellid: 432472 (Sacred Weapon)
  4. Ensure "Holy Bulwark" spell is off cooldown and available

Expected Result Icon A is visible when Holy Bulwark is available, and is invisible when Sacred Weapon is available Icon B is visible when Sacred Weapon is available, and is invisible when Holy Bulwark is available

Actual Result Icon A is NEVER visible, regardless of which of the two spells (Holy Bulwark, Sacred Weapon) are available Icon B is visible when either of the two spells (Holy Bulwark, Sacred Weapon) are available

Export Strings

These are the icons I've been using for testing:

^1^T^SPoint^T ^Sy^F-6127105855193088 ^f-46^Sx ^N9.0045166015625^Spoint ^STOP^SrelativePoint ^STOP^t ^SGUID^STMW:group:1cqkSDk22K4j ^SIcons^T ^N1^T ^SType^Scooldown ^SConditions^T ^N1^T ^SType^SSPELL_LEARNED ^SLevel^N1 ^SName^S432472 ^t^Sn^N1 ^t^SName^S432459 ^SEnabled^B ^t^N4^T ^SType^Scooldown ^SConditions^T ^N1^T ^SType^SSPELL_LEARNED ^SName^S432472 ^t^Sn^N1 ^t^SName^S432472 ^SEnabled^B ^t^t^SName^SHoly~`Armaments~`Test ^t^N110501^S~`~| ^Sgroup^N19 ^^
ascott18 commented 2 months ago

Due to some reasons that I don't fully remember, you have to use spell names with the Spell Learned condition. It'll always be true for any extant spellID.

I'll see if I can make it work with IDs, but if I remember correctly when I added this condition, there was no API provided by Blizzard that can reliably work for this with ID inputs. If I can't fix it, I'll just make it transform the ID input into a name.

Alwies commented 2 months ago

EDIT: A discord user mentioned a condition icon using spell learned on Sacred weapon did not work. As I can't test this myself on retail I'll bow out as I'm not really helping.

Looks like I missed several solutions to track if sacred weapon can be used (you can derive holy bulwark from that as well). .

My testing, on the screenshot Holy Bulward is ready/usable. Fully filled 4x4 grid, missing icons have successfully found out Sacred weapon is not usable. https://gyazo.com/eebb00fbbf1cb6adca269dd700fa8b52 Import as file. [Bulwark2.txt](https://github.com/user-attachments/files/16846797/Bulwark2.txt) With Sacred weapon ready everything shows.

Bit unhappy that I missed this initially. Can only guess I tried with SpellID instead of name. As a note this does conflict with OP as for me spellID for Sacred weapon on a spell learned condition does work correctly.

TMW version: 11.0.5 r10 (g7a86551 Wow version Beta - (11.0.2)

Alwies commented 2 months ago

A discord user mentioned a condition icon using spell learned on Sacred weapon did not work. As I can't test this myself on retail, I'll bow out as I'm not really helping.

ascott18 commented 2 months ago

I was also testing on beta. Maybe I'll go farm out a level on my paladin just to validate that I'm not going crazy here.

I'm surprised that IsUsableSpell/ reactive checks works - I figured that Blizzard would have just left the other spell as usable (but overridden).

I'm also confused that OP found a situation where spell learned doesn't work - it does already transform its input to a spell name if given a spellid, so I suppose it might be not working if the name is changing for the given spellID? I didn't think that could ever happen...

ascott18 commented 2 months ago

The icons in the OP work perfectly for me on retail. No difference in behavior from what I saw testing on beta. Freshly logged in as a prot pally with HA talented. I just bumped the unusable opacity so I could see that they're still working when fully on cooldown.

https://github.com/user-attachments/assets/3126005d-9ae4-4995-9698-894741b79cc6

I'll leave this open for now in case someone can up with more useful information.

ascott18 commented 2 months ago

Added a Spell Overriden condition since it seems like none of the existing mechanisms seem to work for Ret/Templar/Light's Guidance