vendethiel / GladiusEx

13 stars 19 forks source link

Legion update #1

Closed vendethiel closed 6 years ago

vendethiel commented 7 years ago

Hey,

I just wanted to know if you took a look at legion, and if you might know what it takes to update the addon to the new expac.

I'm fine with updating myself if you don't want to look into it, although in this case I'd appreciate some pointers :).

Thanks!

slaren commented 7 years ago

I have not played legion and have no idea what needs to be updated, sorry. Somebody pushed a fix to the wowace repository that may or may not work, I have pushed the changes here just in case it is useful.

vendethiel commented 7 years ago

Okay, well, first will probably be to update all the spells. I'm not sure Blizzard mage a page to list the changes between wod and legion API-wise and all...

vendethiel commented 7 years ago

I'm done updating the spells, but I need to clean what I did. Are you interesting in such a commit?

vendethiel commented 7 years ago

It's more of a LibCooldownTracker update I guess.

slaren commented 7 years ago

Sure, just keep in mind that I won't be testing anything or updating the project in wowace/curse. I can add you to the wowace project if you want to update it yourself.

vendethiel commented 7 years ago

I didn't expect you to, being able to provide it to other players is good enough. I just wrote a small script to fetch the important CDs for each classes (including DH) and generated some lua from that.

vendethiel commented 7 years ago

I didn't want to fork all the different repos, so I "forked" it manually https://github.com/vendethiel/gladiusex-legion

It's not exquisite, but it works fine. Not sure what to do now, but I'm fine with publishing this to wowace (it needs more testing)

slaren commented 7 years ago

Note that wowace also requires the same repository structure (it is not acceptable to just upload a zip file). If you want to fork that's absolutely acceptable, but if you want to submit a pull request or contribute to the project in any way, you will need to rebase your changes to the appropriate repository (gladiusex or libcooldowntracker).

vendethiel commented 7 years ago

It's mostly laziness, to be frank. I couldn't "rebase", so I'd need to just fork the correct repos and paste code on top.

vendethiel commented 7 years ago

Ok. I finished completely updating the addon (I parsed the DR info from a website).

I need to PR:

vendethiel commented 7 years ago

@Ketho maybe this is interesting to you?

slaren commented 7 years ago

The libraries are usually automatically packaged by the wowace packager following the .pkgmeta file.

LibDispellable is at https://www.wowace.com/projects/libdispellable-1-0 and seems to be on github (follow the source link) so you could probably PR it here. However the repository indicated in the .pkgmeta is likely outdated and needs to be changed (I assume that pointing it to the repository at github would work fine).

DRData is at https://www.wowace.com/projects/drdata-1-0 and seems to be abandoned, but you may be able to push your changes to its repository directly.

vendethiel commented 7 years ago

I know pretty much nothing about how these work, I need to find some sort of guide. I'm sorry because some of my questions will probably sound very stupid.

I probably need a wowace account to start with...


DRData is at https://www.wowace.com/projects/drdata-1-0 and seems to be abandoned, but you may be able to push your changes to its repository directly.

How does this work? If it's abandoned I can "claim" it? When I try to get to https://repos.wowace.com/wow/drdata-1-0 it doesn't seem to point to anything :| I did find a github but lol.. https://github.com/Shadowed/CCTracker 2009

slaren commented 7 years ago

I believe it is at https://github.com/AdiAddons/DRData-1.0 (seems to be Adirelle's github) but it clearly says that it is abandoned. I guess you could try contacting them to see if they are interested in a PR, but I assume that at this point just forking it would be fair game. You can just upload your own fork to github and update the .pkgmeta to point at it.

vendethiel commented 7 years ago

sounds good. thanks. I'll try to get that done

vendethiel commented 7 years ago

Actually it seems the great @rainrider has them updated for Legion, so I'll just be able to use their fork!

Rainrider commented 7 years ago

DRData has always failed on its own promise. You cannon reliably detect diminishing returns by just using UnitAura. Knockbacks and Silences often do not have associated debuffs. Also trying to apply further CC during the DR window resets it, which DRData is not able to track. This is why it was discontinued,

You can pull DR spells with associated auras from LibPlayerSpells-1.0 however and it is way more up-to-date in this regard than DRData. A lot of honor talents are not yet included though, feel free to contribute. If you need a usage example, look here

vendethiel commented 7 years ago

A lot of honor talents are not yet included though, feel free to contribute.

I parsed http://api.dr-wow.com/spells/spell-list : https://gist.github.com/vendethiel/49f6735e0c27dce96a765c0fc16d3e6a

Those are in DRData's format though, but I can regenerate to some other format, or only keep those with "honor talent" in the desc.

Ketho commented 7 years ago

I also have no idea what needs updating but good luck

Rainrider commented 7 years ago

I parsed http://api.dr-wow.com/spells/spell-list : https://gist.github.com/vendethiel/49f6735e0c27dce96a765c0fc16d3e6a

This list is not correct as it lacks the corresponding debuffs. Solar Beam's spell id is 78675, the associated debuff is 81261 (this is one of the few (or the only one?) silence effects that also applies a debuff). IIRC the right way to add it to DRData would be [78675] = 81261. Bursting Shot also has a debuff with an id different than the spell id - 224729 for the debuff, 186387 for the spell (wowhead has this wrong too btw).

Basically, if you want to contribute to LPS, you have to verify the corresponding auras in-game. LPS won't give you much in regard of DR though, as it provides information for addons that rely on UnitAura, so the same limitations as with DRData would apply. AdiButtonAuras uses this information to show the aura duration of abilities in the same DR category on the spells on your action bars. However this only works for spells that have associated debuffs and are listed in LPS. The combat log has more information and in some cases different spell ids, but LPS does not have them as they are not needed for what it aims to achieve.

IMHO, tracking DR beyond "do not root if the target is already rooted" is not worth the effort. Serious arena teams have better means of communication regarding DR immunity windows and in rated BGs it is too much of a hassle.

slaren commented 6 years ago

I would be ok just removing the DR module if it can't be made to work reliably.

vendethiel commented 6 years ago

Seems like Retail gladius uses a slightly "fixed" version of DRData.

That's what people have been known to expect, and while I agree that "serious arena teams" have better ways, most people doing pvp are you playing skirms or 2s with random people, without voice.

I'll switch to Gladius' DRData for now, and take a look at LPS in the future to get a more reliable handling of DR.

It's not a perfect solution, it's a status quo to provide "at least as much as gladius".

Rainrider commented 6 years ago

LPS is entirely to be used with the UnitAura API, it does not contain stuff that can be found in the combat log. If Gladius parses the combat log, then LPS is probably of limited use. Further maintaining our fork of DRData didn't make sense as it just introduced another dependency that could be easily resolved by LPS. I see DRData could be of value to Gladius though as it more specific than LPS, so maybe your effort is better pointed in that direction.

vendethiel commented 6 years ago

If Gladius parses the combat log, then LPS is probably of limited use.

Gladius uses DRData as-is (except for the actual DR categories), which is hooking into COMBAT_LOG_EVENT_UNFILTERED, which you recommended not to (because you can't track whether a DR was immune'd).

So, yeah. I'll leave the DR module as-is as a stopgap measure (just using the data from retail gladius), and consider using LPS in the future.

Rainrider commented 6 years ago

If the SPELL_MISSED subevent fires with the "IMMUNE" argument when you try to apply CC during a DR window, this could be used as an indicator to reset the DR window for destGUID. I don't know if it works like that, but if it does, then I was wrong that DRData is of no use for that.

There were occasions in the past where some spells were only visible in the combat log and had different spells ids than the ones you could get with UnitAura. Maybe this is a case to consider for i.e knockbacks.

Basically you can use LPS to prepare the list of spells you want to track but DRData is more specific and may be of more value if it is properly maintained.

vendethiel commented 6 years ago

@Rainrider thank you for being so thorough in your explanations.

I have finished updating the addon to legion (using some data from gladius) and added an interrupt module.

I'll be PR-ing:

I'll consider using LPS to generate DRData data, but it seems a bit like overkill to do it on every login.

Fixing links:

Seems like LibRangeCheck and LibGroupInSpec might need a legion update tho...

@slaren I see that many a lib in .pkgmeta use wowace svn/git that no longer exist. Am I just toast for those? Even when going through the project page for e.g. AceSerializer, it gives me https://repos.wowace.com/wow/ace3/trunk which doesn't load :(.

I am seriously starting to consider disregarding .pkgmeta for those, only for the most recent git-based ones.

Rainrider commented 6 years ago

@vendethiel just fork DRData for now. Having a complete history is nice :)

vendethiel commented 6 years ago

Moving to #2.