View Single Post
11-16-12, 03:03 AM   #3
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
You could make that more efficient by (1) using either simple string concatenation, or string.format with the standard substitution token %s, instead of using the more expensive string.gsub with the custom substitution token %f (which is actually a real Lua pattern token indicating a floating-point numeric value), and (2) only hiding the frame if it's actually supposed to be hidden, instead of unconditionally hiding it and then conditionally showing it again immediately afterward.

Also, it's a good practice to only declare variables (I'm looking at your "faction" variable) in the most limited scope where they are useful, and in this case since the value from UnitFactionGroup will only be used once, there's not even any need to store it in a variable. Same goes for the texture paths; there's no reason to assign them both to variables when only one might be used. I'm guessing this script was originally adapted from an arena macro; the authors of those are notorious for misunderstanding the purpose of variables, and assigning everything to a variable even when it's only used once (or not at all) and actually takes up more space in their macro.

OnLoad:
Code:
self:RegisterEvent("PLAYER_ENTERING_WORLD")
self:RegisterEvent("PLAYER_TARGET_CHANGED")
self:Hide()
OnEvent:
Code:
if UnitIsPVPFreeForAll("target") then
	self.bg:SetTexture("Interface\\TargetingFrame\\UI-PVP-FFA")
	self:Show()
elseif UnitIsPVP("target") then
	self.bg:SetTexture("Interface\\TargetingFrame\\UI-PVP-" .. UnitFactionGroup("target"))
	self:Show()
else
	self:Hide()
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