oUF

97.2k Downloads

oUF.colors.power usage inconsistencies

myzb opened this issue ยท 5 comments

commented

With the new color path changes most of the elements that use the oUF.colors.power table retrieve the color information by passing the color index (aka 'powerType' ID) i.e:

https://github.com/oUF-wow/oUF/blob/master/elements/alternativepower.lua#L89
https://github.com/oUF-wow/oUF/blob/master/elements/additionalpower.lua#L73

For power however color retrieval is being done using the actual token (MANA, RAGE, etc) first:
t = self.colors.power[ptoken or ptype]

See:
2ca03b7#diff-2d6dd4967e1f58a5832a8c6702fa31c9R125

This means that to i.e override the mana color 2 declarations are now needed:

oUF.colors.power[0] = { 1/255, 121/255, 228/255 }       -- for alternativepower and additionalpower
oUF.colors.power['MANA'] = { 1/255, 121/255, 228/255 }  -- for power

Is this done on purpose? It seems we want the following with ptype first instead?

t = self.colors.power[ptype or ptoken]

Also, documentation would need to get adjusted
https://github.com/oUF-wow/oUF/blob/master/elements/power.lua#L35

OR:

perhaps use ADDITIONAL_POWER_BAR_NAME instead of ADDITIONAL_POWER_BAR_INDEX for color retrieval in alternativepower and additionalpower?

commented

Thanks for the info

That's always the case, in oUF the default values we use come from Blizz's PowerBarColor table which contains both types and tokens, so we use the same structure. Also, in this new release I added alt power to our colour table.

This makes sense yes.

I really don't mind doing 2 declarations for each override, it just caused some confusion, since I was using the following to override mana color, which worked for additionalpower and power.:

oUF.colors.power['MANA'] = { 1/255, 121/255, 228/255 } 

Once I updated it stopped doing so for additionalpower because of this: e3465e3#diff-96acc97d9b9ff92537cf4041bfc93f63L73

So now I have to additionally do:

oUF.colors.power[0] = { 1/255, 121/255, 228/255 }       -- for alternativepower and additionalpower

I just wanted to let you know that having alternativepower and additionalpower retrieving color by index (ptype) while power does so by name (ptoke) may have been overlooked?

commented

You could do oUF.colors.power[0] = oUF.colors.power.MANA btw.
And alternative power is different from additional power. The former is mostly encounter based extra resource while the latter is just an additional resource based on spec (it is always mana currently). The power index for mana is 0, while the power index for alternative power is 10. The default ui calls it alternate power.

commented

I just wanted to let you know that having alternativepower and additionalpower retrieving color by index (ptype) while power does so by name (ptoke) may have been overlooked?

Well, I could do that for the sake of consistency, but if it ain't broke, you know the drill. Nevertheless, you still are expected to override each colour twice, like so

oUF.colors.power['MANA'] = { 1/255, 121/255, 228/255 }
oUF.colors.power[0] = oUF.colors.power['MANA']

primarily because we can't guarantee that UnitPowerType will always return proper values, as I showed you previously, even Blizz weren't confident in it :D

commented

Thanks for the tips. Ill def override colour twice now. The consistency thing probably was just a bit of nitpicking from my side :D
I was gonna make a patch but wasn't sure what behaviour was the proper one.

commented

Sup!

t = self.colors.power[ptoken or ptype]

It's done this way because of this. It's a very old piece of code that's added back in Cata I think and no one really bothered to change it. Technically, it should be

t = self.colors.power[ptoken] or self.colors.power[ptype]

As for

This means that to i.e override the mana color 2 declarations are now needed

That's always the case, in oUF the default values we use come from Blizz's PowerBarColor table which contains both types and tokens, so we use the same structure. Also, in this new release I added alt power to our colour table.