search

Backupiseasy / ThreatPlates

Nameplate addon for World of Warcraft
https://wow.curseforge.com/projects/tidy-plates-threat-plates
35 stars 18 forks source link

SOD Nameplate Castbar Fixes #498

Closed fbrusch-ik closed 5 months ago

fbrusch-ik commented 5 months ago

Hey ! In SOD many of the spell ids got switched around which confuses LibClassicCasterino so it does not show all casts.

The good news is that technically you dont need LibClassicCasterino at all anymore since you can just listen to the events and use the UnitCastingInfo / UnitChannelInfo directly from the WoW API.

The bad news is that the UnitChannelInfo Event is bugged and only returns a bunch of nils, so we still need LibClassicCasterino for that.

I added penance as a new channel to show show how it is done.

danjohnso commented 5 months ago

Can confirm this fixes what I am seeing in https://github.com/Backupiseasy/ThreatPlates/issues/493, looks good in a battleground, will try in raid later

danjohnso commented 5 months ago

Been running with this PR for 2 weeks now, no issues that I have noticed, seems like all the castbars are appearing as expected

Backupiseasy commented 5 months ago

Target branch should be devel/111/main - I already changed that. There seems to be some changes in your PR that are already in devel/111/main. From which branch did you fork it or maybe these are changes added after your fork. Can you update your PR with the most recent version of devel/111/main?

Backupiseasy commented 5 months ago

I also think that having separate parts for Addon.IS_CLASSIC_SOD and Addon.IS_CLASSIC is necssary. There is no Addon.IS_CLASSIC without SOD when it comes to funcionality, i.e., the changes you made to Addon.IS_CLASSIC_SOD should also work on plain Classic servers. So, you can merge that I think.

Backupiseasy commented 5 months ago

See my comment here: https://github.com/Backupiseasy/ThreatPlates/issues/493

I did not merge your PR as it is behind the current branch (devel/111/main), but used your changes.

fbrusch-ik commented 5 months ago

Hey! Dont worry, that was my main intention behind this anyway