baaron4 / GW2-Elite-Insights-Parser

Binary parser for the .evtc files that arcdps generates after a boss encounter. This will generate a .html file where the results can be easily reviewed.
MIT License
136 stars 48 forks source link

Skill icons and some new buffs #780

Closed Linkaaaaa closed 1 year ago

Linkaaaaa commented 1 year ago

5626

EliphasNUIT commented 1 year ago

Why the XXXBuff naming instead of XXXEffect for certain things and not the others?

Linkaaaaa commented 1 year ago

we had decided some time back with zerthox to rename effect to buff to go along with all the buff related classes

EliphasNUIT commented 1 year ago

Yes and I am perfectly fine with that. But that does not answer the question, my understanding was that the naming convention would change, not that we would have the two conventions co-existing. Is there a plan to rename all of them?

Linkaaaaa commented 1 year ago

I can work on renaming all of them for this pr then

EliphasNUIT commented 1 year ago

That'd be perfect!

Linkaaaaa commented 1 year ago

done that, should i rename XXXXHit to XXXXDamage too? both are mixed, do you have preference on which word?

EliphasNUIT commented 1 year ago

I believe that is more a case by case basis, if something has been wrongly named Hit instead of Damage, yes. But there are a lot of skill ids that represent secondary hits or multi hit components of skills. I don't think you need to go through that in this PR (not sure if it is even necessary).

Linkaaaaa commented 1 year ago

Alright then i believe it's good to go, won't touch it unless you notice something wrong

EliphasNUIT commented 1 year ago

Thanks!