Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Evoker Hover ability #204

Merged
merged 1 commit into from
Nov 17, 2022
Merged

Conversation

Faldoncow
Copy link
Contributor

No description provided.

Copy link
Collaborator

@DanSheps DanSheps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work.

I am almost ready to merge the latest code on my end that will make some major changes to the GUI itself. So i am going to mark this as needing changes. You may need to rebase at some point.

Question thought. Did you use Google Translate for the strings or did you pull them from somewhere else?

Comment on lines +64 to +66
{
{name="classes.evoker.hover",element="CheckBox",label=BeStride_Locale.Settings.Classes.Evoker.Hover,class="evoker"},
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is going to be a change to how options are done in the next version. I am going to hold off on merge until after that is completed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem on holding off. Feel free to reject this PR and add Hover yourself if you want, or I can update/remake it later after the GUI changes. I mostly did this to ensure I'd have hover for my Evoker starting today.

Also, I've got some code changes on my fork with support for Dragonriding. That is something I'd love to have in the addon on the 28th. Was waiting to PR those until after today's patch since that changed some API calls (again) in retail. Also, it may break WotLK/Classic support? Not sure, will have to see...will take a look at that this weekend maybe.

For the translations, a little bit of Google Translate, but for the spell/ability names and Evoker name, I used Wowhead and flipped between the different language versions of the site. Spell ids remain the same, so getting the name for the ability in-game is easy.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am to merge to master right now. If you want to get ahead of it, it is on develop and you can make the minor changes (I don't think it is anything too fancy so should be easy to do)

@DanSheps DanSheps changed the base branch from master to develop November 17, 2022 03:55
@DanSheps
Copy link
Collaborator

Going to merge this to develop and rework the options GUI

@DanSheps DanSheps merged commit 971f854 into Bestride:develop Nov 17, 2022
@Faldoncow Faldoncow deleted the dracthyr branch November 28, 2022 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants