View Single Post
12-11-14, 04:31 PM   #5
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
The biggest problem with duplication is that it makes things harder to debug and maintain in the long run. If you decide to change anything about your health bar, for example, you have to make the same change 4 times in 4 different places, which is boring, a waste of time, and easy to screw up. If you only have to make the change in one place, it's faster and less error-prone.

Also just noticed this:
Code:
self.HealthBG = HealthBG
You should stick with oUF convention and set this as self.Health.bg instead of self.HealthBG. It won't make any functional difference, but it will help other oUF layout authors looking at your code, and it will help you if you're looking at other people's layout code, since it'll be easier to recognize what's what.

Originally Posted by sirann View Post
As for the reason I didn't make my own tags in ouf. That code has to be put somewhere in the ouf folder, be it a tags file in my layout folder, or editing the ouf tags. while it would look cleaner if I just had tags in my layout, rather than functions and frame creations, it would ultimately be messy somewhere.
But it would be a lot less messy than creating frames and setting event handlers in your code, especially when you consider that you're currently duplicating event handlers 4 times. For example, here are tags for your health value and percent strings:

Code:
oUF.Tags.Events.healthtext = 'UNIT_HEALTH'
oUF.Tags.Methods.healthtext = function(unit)
	if UnitIsGhost(unit) then
		return '|cffb40000Ghost'
	elseif UnitIsDead(unit) then
		return '|cffb40000Dead'
	end
	local r, g = 0, 180
	local v, vmax = UnitHealth(unit), UnitHealthMax(unit)
	if v < vmax then
		r, g = g, r
	end
	if v > 1000000 then
		return format('|cff%02x%02x00%.1fm', r, g, v / 1000000)
	elseif v > 1000 then
		return format('|cff%02x%02x00%.0fk', r, g, v / 1000)
	else
		return format('|cff%02x%02x00%d', r, g, v)
	end
end

oUF.Tags.Events.healthpercent = 'UNIT_HEALTH'
oUF.Tags.Methods.healthpercent = function(unit)
	if UnitIsDead(unit) then
		return
	end
	local r, g = 0, 180
	local v, vmax = UnitHealth(unit), UnitHealthMax(unit)
	if v < vmax then
		r, g = g, r
	end
	return format('|cff%02x%02x00%.0f%%', r, g, 100 * (v / vmax))
end
Then you can just do this (example for the player frame):

Code:
local HealthText = Health:CreateFontString(nil, 'OVERLAY')
HealthText:SetPoint('RIGHT', 5, -10)
HealthText:SetFont(FONT, 10, 'OUTLINE')
HealthText:SetJustifyH('LEFT')
Health.Text = HealthText

self:Tag(HealthText, '[healthvalue]')

local HealthPercent = playerhealthtextframe:CreateFontString(nil, 'OVERLAY')
HealthPercent:SetPoint('LEFT', -5, -10)
HealthPercent:SetFont(FONT, 10, 'OUTLINE')
HealthPercent:SetJustifyH('RIGHT')
Health.Percent = HealthPercent

self:Tag(HealthPercent, '[healthpercent]')
Then you can reuse those tags on the other frames. You don't need to specifically listen for vehicle changes or PLAYER_ENTERING_WORLD since oUF automatically updates all elements and tagged fontstrings when those events occur, and passes the correct unit to the tag method.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote