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

Legion runebar #269

Merged
merged 8 commits into from Sep 10, 2016
Merged

Legion runebar #269

merged 8 commits into from Sep 10, 2016

Conversation

p3lim
Copy link
Member

@p3lim p3lim commented May 31, 2016

This PR contains changes to the runebar element for Legion.
It also eliminates the need for #112.

These commits are compatible with both live and beta clients.

@p3lim p3lim mentioned this pull request May 31, 2016
@ls-
Copy link
Member

ls- commented May 31, 2016

Could you address the issue I described here?

@p3lim
Copy link
Member Author

p3lim commented May 31, 2016

Sure, but it'll have to wait until tomorrow.

@ls-
Copy link
Member

ls- commented May 31, 2016

No problem there. I decided to remind about it, cuz I wasn't sure whether or not you saw that comment. As you said, that PR had become too big, so it's quite possible, that you missed it 😅

No need to hide this frame manually any more
@Blazeflack
Copy link

Blazeflack commented Jul 10, 2016

I think you may have unintentionally made some find&replace errors in the "Add Override support and rename the PostUpdate hook" commit.

All references to the "UpdateRune" function was replaced with "Update", most likely during your replacement of "PostUpdateRune" with "PostUpdate".

@Blazeflack
Copy link

I forgot to mention that this probably breaks the element, as there is now two functions named "Update", so one replaces the other.

@p3lim
Copy link
Member Author

p3lim commented Jul 11, 2016

@Blazeflack You're right, must have slipped past when I rebased the branch.

@Blazeflack
Copy link

Blazeflack commented Jul 16, 2016

How hard would it be to merge Runes into the ClassIcons element?

@p3lim
Copy link
Member Author

p3lim commented Jul 16, 2016

Every single classicon is handled by a single texture/frame group that all use the same API/system for updates (sans vehicles because of buggy API).
Runes use their own API, and they also have statusbars that needs to be managed and can sometimes proc etc.

In theory it's not hard to merge, but in practicality it's best to leave them separate, same with totems.

@Blazeflack
Copy link

That makes sense.

@p3lim
Copy link
Member Author

p3lim commented Jul 29, 2016

Removed the compatibility code since 7.0 is already out.

@p3lim p3lim mentioned this pull request Jul 29, 2016
@ls-
Copy link
Member

ls- commented Jul 31, 2016

Hi!

There's a bug here. Please, replace r, g, b with colors[1], colors[2], colors[3].

rune.bg:SetVertexColor(colors[1] * mu, colors[2] * mu, colors[3] * mu)

@p3lim
Copy link
Member Author

p3lim commented Jul 31, 2016

There's a bug here. Please, replace r, g, b with colors[1], colors[2], colors[3].

Fixed.

doadin pushed a commit to doadin/Elvui that referenced this pull request Aug 6, 2016
doadin pushed a commit to doadin/Elvui that referenced this pull request Aug 17, 2016
@haste haste merged commit 064db96 into oUF-wow:master Sep 10, 2016
@p3lim p3lim mentioned this pull request Sep 11, 2016
@p3lim p3lim deleted the legion-runebar branch September 11, 2016 09:54
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

5 participants