Add Classic stance condition #69
Conversation
There was a problem hiding this comment.
Wow. That's great, amazing work.
I agree with you on the dedicated functions for Classic, though I can't really think of a good solution there. One option would be to put if Addon:IsClassic() in all the existing functions but since they also require different signatures (classes instead of specs) that might be more confusing and harder to maintain. Similar to what you're suggesting with "fake" spec IDs.
Regarding Druids specifically, on Retail it was a giant pain in the ass to get the form indices right with all the glyphs and talents, does Classic/TBC not have something like that? Is, for example, the index of Moonkin form and Tree of Life stable?
|
|
||
| return IsSpellKnown(spellId) | ||
| else | ||
| local forms = Addon:GetClassicShapeshiftFormsForClass("DRUID") |
There was a problem hiding this comment.
Shouldn't this use select(2, UnitClass("player"))?
|
|
||
| if classNames == nil then | ||
| classNames = {} | ||
| -- TODO: Set to player's current class |
There was a problem hiding this comment.
Can use classNames[1] = select(2, UnitClass("player")) here
| { 9634, 5487 }, -- Dire Bear Form, Bear Form | ||
| { 1066 }, -- Aquatic Form | ||
| { 768 }, -- Cat Form | ||
| { 783 }, -- Travel Form | ||
| { 24858, 33891 }, -- Moonkin Form, Tree of Life Form | ||
| { 40120, 33943 } -- Swift Flight Form, Flight Form |
There was a problem hiding this comment.
Is there a reason these have to be in their own tables? Would { 9634, 5487, 1066, 768, ... } not work?
There was a problem hiding this comment.
It's because we have ranked spell on classic, so Bear Form (5487) is replaced by Dire Bear Form (9634) when the character gains this new spell; the same for Flight Form and Swift Flight Form as I understood (but I don't have a character to check for now). About Moonkin Form and Tree of Life Form, you can't have both at the same time, so they share the same form number.
|
Thanks for your review, and sorry for my late reply - with the release of BC I was focus on leveling ^^
I cannot find again my source about that, but yes these number are stable as you can't have both. I need to ask a friend that has a level 70 Druid to run the command to get the list of form numbers to check that - I have a doubt about flying number when Moonkin or Tree of Life is not available, maybe it's number is not stable. |
|
Heya, I implemented your PR and added the remaining classes. Thanks for working on this! It's been long overdue :) |
|
Thanks a lot, I saw my name in the changelog today. I didn't play my druid during BC and I'm sorry that I didn't finish working on this PR, but I'm glad if my code helps you to implement this new feature that I will be happy to use as I still use your awsome addon and play my Druid now. |
Here a Pull Request draft that enables the stance condition in BC classic (should work in Era too, but not tested). As I'm not fully happy with how I implemented it, I open a draft pull request to have your feedback before working more on it - this version has only Druid forms (tested for a week in-game now) and I will add other classes stances if you are interested to merge this code.
I added a few methods dedicated to Classic and I'm not happy with that, the other solution could be to create a "fake" spec id for Classic to use the same code, but that seems harder to maintain than dedicated methods.
thanks for your feedback about the implementation itself, and also about the idea to add this feature to Clicked.
THis PR is related to issue #68