Autopass trinket Alternate solution: Hardcode trinket classes

evil-morfar / RCLootCouncil2

RCLootCouncil - addon for World of Warcraft

https://rclootcouncil.com

GNU Lesser General Public License v3.0

19 stars 29 forks source link

Autopass trinket Alternate solution: Hardcode trinket classes #122

Closed SafeteeWoW closed 6 years ago

SafeteeWoW commented 6 years ago

See Also PR #121

Add developer command:
- /rc exporttrinketdata to export the class info of trinkets to a window. Copy and paste this code to Util/trinketData.lua
Trinket data is hardcoded in Util/TrinketData.lua
Add command "/rc trinkettest" or "/rc ttest" for testing.
All 26 Antorus trinkets are added to the test.
The exports includes all trinkets in all difficulties in all instances in the EJ of the current expansion.
- No need to hardcode trinkets from earlier expansion. Old trinkets equals to gold for everyone.
There is an option autoPassTrinket to toggle this on/off.
- Maybe it's possible that people use trinket not for their spec, even if it's half effectiveness.

SafeteeWoW commented 6 years ago

This is ready. Many of the changes are enclosed by @debug

evil-morfar commented 6 years ago

Seems fine, not sure it really requires a seperate command for trinket test, but fine. I just didn't want to concentrate on new features with huge bugs going on.

SafeteeWoW commented 6 years ago

It shouldn't have much issues and can be easily tested. The only stuffs released to the end users are autopass trinket options, two more lines in :AutoPassCheck and a trinket data table. Other changes are intended to be used by developers only (enclose by @debug). You can also enclose trinkettest inside debug if you want. It mainly just for my testing purposes.

Dont forget to reboot game when testing this PR. toc is changed.

SafeteeWoW commented 6 years ago

trinkettest are moved to debug section format of /rc trinkettest: /rc trinkettest WARRIOR /rc trinkettest DEMONHUNTER etc

Tested with all antorus trinkets for all 12 classes. No issues are found

SafeteeWoW commented 6 years ago

I just didn't want to concentrate on new features with huge bugs going on.

I know, and this is why I didn't write anything for #88 recently. But compared to that, this feature is not complex, easily tested and unrelated to any other features.

Also suggest people to do full client restart after the update to enable this feature in the changelog

evil-morfar commented 6 years ago

This won't go into 2.7.2, but could probably be released before 2.8

jokeyrhyme commented 6 years ago

I checked out master, then merged in your PR, and I've killed a boss (The Coven of Shivarra: Heroic) and nothing exploded or crashed :D No trinkets dropped, so I didn't get to see that part working, however. The rest of the existing functionality in the add-on seems unaffected.

SafeteeWoW commented 6 years ago

Can this be merged in the next version?

evil-morfar commented 6 years ago

Yes. I wanted to test it in a raid before releasing, and I found no issues.