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

Feature: Arena Preparation #275

Closed
wants to merge 9 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@p3lim
Member

p3lim commented Jul 11, 2016

This adds support for arena preparation frames, a solution to #177.

By taking over the arena# frames' state driver until the unitIDs become available, this PR achieves the following:

  • Displaying the frames with bogus health/power information
  • Class color support on health/power elements
  • Class color support in [raidcolor] tag
  • New tag [arenaspec] which also work on the normal arena# frames

That should cover most of the available features with the arena prep system, and it requires no additional setup for the layout (plug-n-play).

Please test!

core: Add support for arena preparation frames
These will piggyback on the arena# frames, disabling the state driver until the units are available.

@p3lim p3lim changed the title from Feature arenaprep to Feature: Arena Preparation Jul 11, 2016

p3lim added a commit to p3lim-wow/oUF_P3lim that referenced this pull request Jul 29, 2016

@Sticklord

This comment has been minimized.

Show comment
Hide comment
@Sticklord

Sticklord Aug 14, 2016

Contributor

I would like request that instead of needing to change all the different elements etc, you could setup the arena frames in a single place. Because now you need to test if it's an arena preparation frame on all the elements on all units.

We only need to look for the event ARENA_PREP_OPPONENT_SPECIALIZATIONS which enables it and ARENA_OPPONENT_UPDATE which disables it. What I'm proposing is creating function which setups up the dummy frame when we got the information (ARENA_PREP_OPPONENT_SPECIALIZATIONS) and disables it when the arena starts.

By the way, I tested this and it seemed to work:)

Contributor

Sticklord commented Aug 14, 2016

I would like request that instead of needing to change all the different elements etc, you could setup the arena frames in a single place. Because now you need to test if it's an arena preparation frame on all the elements on all units.

We only need to look for the event ARENA_PREP_OPPONENT_SPECIALIZATIONS which enables it and ARENA_OPPONENT_UPDATE which disables it. What I'm proposing is creating function which setups up the dummy frame when we got the information (ARENA_PREP_OPPONENT_SPECIALIZATIONS) and disables it when the arena starts.

By the way, I tested this and it seemed to work:)

@p3lim

This comment has been minimized.

Show comment
Hide comment
@p3lim

p3lim Aug 14, 2016

Member

The reason behind the changes to all the elements is because of several reasons:

  • The units doesn't exist, so we need to fake them
  • When we fake them the elements can't get valid information and would throw errors
  • We do want to update the elements to allow the style to be applied

What you are thinking of is a new system to handle this, and that is exactly what we don't want, we want it to be handled gracefully without needing additional work in the layouts.

Member

p3lim commented Aug 14, 2016

The reason behind the changes to all the elements is because of several reasons:

  • The units doesn't exist, so we need to fake them
  • When we fake them the elements can't get valid information and would throw errors
  • We do want to update the elements to allow the style to be applied

What you are thinking of is a new system to handle this, and that is exactly what we don't want, we want it to be handled gracefully without needing additional work in the layouts.

@Sticklord

This comment has been minimized.

Show comment
Hide comment
@Sticklord

Sticklord Aug 15, 2016

Contributor

Okay, I see your point(s). I just annoying taking care of all the custom elements postupdate in my layout without knowing what event caused it. But it would be nice to have the possibility to handle all this in a single function. Is there an ok way of making the line self:UpdateAllElements('ArenaPreparation', true) instead call a custom setup function? I could always hook it but seems abit needless. The oUF code is abit above my league to understand fully:p

Or maybe add the classic:

(self.ArenaPreparation or self.UpdateAllElements) (self, 'ArenaPreparation', true)

Contributor

Sticklord commented Aug 15, 2016

Okay, I see your point(s). I just annoying taking care of all the custom elements postupdate in my layout without knowing what event caused it. But it would be nice to have the possibility to handle all this in a single function. Is there an ok way of making the line self:UpdateAllElements('ArenaPreparation', true) instead call a custom setup function? I could always hook it but seems abit needless. The oUF code is abit above my league to understand fully:p

Or maybe add the classic:

(self.ArenaPreparation or self.UpdateAllElements) (self, 'ArenaPreparation', true)

@p3lim

This comment has been minimized.

Show comment
Hide comment
@p3lim

p3lim Aug 15, 2016

Member

You could check if the unit exists (UnitExists(element.__owner.unit)), I think this should only be nil/false and get to PostUpdate in the occasion of prep updates.

Member

p3lim commented Aug 15, 2016

You could check if the unit exists (UnitExists(element.__owner.unit)), I think this should only be nil/false and get to PostUpdate in the occasion of prep updates.

@Sticklord

This comment has been minimized.

Show comment
Hide comment
@Sticklord

Sticklord Aug 15, 2016

Contributor

Okay got it working the way i wanted, thanks!

Contributor

Sticklord commented Aug 15, 2016

Okay got it working the way i wanted, thanks!

@p3lim p3lim modified the milestone: 1.7.0 Sep 11, 2016

@Rainrider Rainrider referenced this pull request Sep 11, 2016

Closed

Arena preparation frames #177

@p3lim p3lim self-assigned this Sep 12, 2016

@p3lim p3lim added the core label Sep 17, 2016

@p3lim

This comment has been minimized.

Show comment
Hide comment
@p3lim

p3lim Sep 19, 2016

Member

Need some actual tests on this, I don't do arenas so the only tests I did were while making this feature doing skirmishes.
Input from someone that actually does arenas would be nice.

Member

p3lim commented Sep 19, 2016

Need some actual tests on this, I don't do arenas so the only tests I did were while making this feature doing skirmishes.
Input from someone that actually does arenas would be nice.

@Sticklord

This comment has been minimized.

Show comment
Hide comment
@Sticklord

Sticklord Sep 21, 2016

Contributor

I tested this, seemed to work nicely. The arena prep frames where on low alpha though (like 50 or so), not sure why that is.

Those couple of changes i had was also included but it shouldn't affect it.

Edit: Nevermind the alpha, that was my layout.

Contributor

Sticklord commented Sep 21, 2016

I tested this, seemed to work nicely. The arena prep frames where on low alpha though (like 50 or so), not sure why that is.

Those couple of changes i had was also included but it shouldn't affect it.

Edit: Nevermind the alpha, that was my layout.

@p3lim p3lim modified the milestones: 2.0.0, 1.7.0 Oct 17, 2016

@Gethe

This comment has been minimized.

Show comment
Hide comment
@Gethe

Gethe May 4, 2017

Contributor

I did some testing for this with my rebased branch. It worked pretty well overall, but I noticed that the frames would not show during arena prep until a teammate enters.

This is honestly a bit of an edge case as it's only really an issue if you're queuing solo for skirmishes, but I figured I would mention it regardless.

Contributor

Gethe commented May 4, 2017

I did some testing for this with my rebased branch. It worked pretty well overall, but I noticed that the frames would not show during arena prep until a teammate enters.

This is honestly a bit of an edge case as it's only really an issue if you're queuing solo for skirmishes, but I figured I would mention it regardless.

@p3lim

This comment has been minimized.

Show comment
Hide comment
@p3lim

p3lim May 31, 2017

Member

@Gethe Might be a bug with the event actually, it worked fine prior to 7.0.

Member

p3lim commented May 31, 2017

@Gethe Might be a bug with the event actually, it worked fine prior to 7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment