View Single Post
12-28-12, 04:11 PM   #9
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
A couple thoughts on your final code:

1) There is no reason to create a separate frame for the backdrop vs. the bar. You can apply backdrops to a StatusBar object, as well as add ordinary textures to it, register events on it, set scripts on it, etc. StatusBar objects are just Frame objects with some extra features; they don't lose any features.

2) Even if you do want to create separate frames for some reason, you don't need to store them in separate tables; you should attach the child to the parent (eg. frames[i] = frame and then frame.bar = bar, instead of frames[i] = frame and bars[i] = bar).

3) You're parenting your bar to "self" but "self" is not defined in that scope, so your bars are actually not being parented to anything, which means they are not properly attached to the UI, and won't get hidden with Ctrl+Z. Again, if you really want separate frames for some reason, the bar frame should be parented to the regular frame.

4) For ease of maintenance, I'd suggest putting all of your variables/constants (like repframes and repTable) at the top of your file, so you can easily find them without having to scroll through a bunch of functions. It'll also make it easier to add more factions.

On a side note, the reason the OnEvent function is defined outside of the for-loop is because you only need one copy of it, and can attach the same function to multiple frames; defining the function inside the for-loop would create multiple copies of the function, taking up -- in your case -- 5 times as much memory but doing the same thing.

On another side note, while it doesn't affect functionality, it is conventional to use k/v only with pairs; when you're using ipairs, you use i/v instead. It just helps improve code readability, so it's more obvious that you're inside an ipairs loop (indexed values processed in sequential order) instead of a pairs loop (values processed in undefined order). It may not seem important now, but if you come back in a month or 6 months and want to change something, little helpers like this will add up and make it much easier to remember what everything does.

Revised version:
Code:
local repTable = { 1341, 1269, 1270, 1337, 1359 }

local repframes = {}

local function OnEvent(self, event, ...)
	local name, _, standing, barMin, barMax, barValue = GetFactionInfoByID(self.factionID)
	self:SetMinMaxValues(barMin, barMax)
	self:SetValue(barValue)
	self.text:SetText(name)

	local color = FACTION_BAR_COLORS[standing]
	self:SetStatusBarColor(color.r, color.g, color.b)

	if standing == 8 then
		self.text:SetTextColor(1, 0.6, 0.33)
	else
		self.text:SetTextColor(1, 1, 1)
	end
end

for i, v in ipairs(repTable) do
	local repframe = CreateFrame("StatusBar", nil, UIParent)
	repframe:SetPoint("TOP", i > 1 and repframe[i-1] or Minimap,"BOTTOM", 0, i > 1 and -2 or -25)
	repframe:SetSize(140,15)
	repframe:SetStatusBarTexture(C.media.texture)
	repframes[i] = repframe

	F.CreateBD(bg)

	local text = F.CreateFS(repframe, 8)
	text:SetPoint("CENTER")
	repframe.text = text

	repframe.factionID = v
	repframe:SetScript("OnEvent", OnEvent)
	repframe:RegisterEvent("UPDATE_FACTION")
end
__________________
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