View Single Post
Unread 01-14-13, 11:04 PM   #14
Phanx
A Pyroguard Emberseer
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 4,261
That's actually quite inefficient, for several reasons:

(1) You're using an OnUpdate script -- which runs every time a video frame is drawn, 25-100 or more times per second, depending on your system -- to check for something (the current skin) that changes rarely, or never, during any given session since most users do not sit around repeatedly changing the same option. OnUpdate scripts are very expensive in CPU terms, and should only be used for timers, animations, and other things that can't be done any other way.

(2) You're creating a bunch of extra tables -- which cost memory -- for every button created. Also, you're creating your tables in the least efficient way possible, with tons of extra table lookups. Instead of doing:

Code:
t = {}
t.alpha = 1
t.beta = 2
t.gamma = {}
t.gamma.foo = 42
t.gamma.bar = 84
just construct the table directly:
Code:
t = {
	alpha = 1,
	beta = 2,
	gamma = {
		foo = 42,
		bar = 84,
	}
}
This has the incidental advantage of being easier to read, too.

(3) You're doing a bunch of table lookups on every mousedown/mouseup event, even though the result of any given lookup will almost always be the same.

Instead of doing all this, you should either:

(a) use the default template, and use the technique outlined in my previous post to change the textures when needed, or

(b) use a custom template that starts out one color, and change the textures "by hand" instead of changing the "self.skin" value and letting an expensive OnUpdate function check if it should change the textures 25-100 times per second.

Also, if you're not already using XML to define your frames -- and you shouldn't, unless you really really love XML, or you are writing secure templates for action buttons/unit frames/state drivers/etc. -- you don't need to use XML for this either. Rather than defining a template and inheriting it, just use a "factory" function to generate your frames:

Code:
local ButtonTextures = {
	RED = {
		up = "Interface\\Buttons\\UI-Panel-Button-Up",
		down = "Interface\\Buttons\\UI-Panel-Button-Down",
		highlight = "Interface\\Buttons\\UI-Panel-Button-Highlight",
	},
	GREEN = {
		up = "Interface\\AddOns\\NickAlert\\Green-Panel-Button-Up",
		down = "Interface\\AddOns\\NickAlert\\Green-Panel-Button-Down",
		highlight = "Interface\\AddOns\\NickAlert\\Green-Panel-Button-Highlight",
	},
}

local function GetSkin(self)
	return self.__skin
end

local function SetSkin(self, skin)
	if self.__skin == skin then return end
	self.__skin = skin

	if self:IsMouseOver() and IsMouseButtonDown() then
		local down = ButtonTextures[skin].down
		self.Left:SetTexture(down)
		self.Right:SetTexture(down)
		self.Middle:SetTexture(down)
	else
		local up = ButtonTextures[skin].up
		self.Left:SetTexture(up)
		self.Right:SetTexture(up)
		self.Middle:SetTexture(up)
	end

	self:SetHighlightTexture(ButtonTextures[skin].highlight, "ADD")
	self:GetHighlightTexture():SetTexCoord(0, 80/128, 0, 22/32)
end

local function OnMouseDown(self, button)
	local down = ButtonTextures[self.__skin].down
	self.Left:SetTexture(down)
	self.Right:SetTexture(down)
	self.Middle:SetTexture(down)
end

local function OnMouseUp(self, button)
	local up = ButtonTextures[self.__skin].up
	self.Left:SetTexture(up)
	self.Right:SetTexture(up)
	self.Middle:SetTexture(up)
end

local function NewButton(parent, skin, name)
	local button = CreateFrame("Button", name, parent or UIParent)
	button:SetSize(40, 22)

	local left = button:CreateTexture(nil, "BACKGROUND")
	left:SetPoint("TOPLEFT")
	left:SetPoint("BOTTOMLEFT")
	left:SetWidth(12)
	left:SetTexCoord(0, 12/128, 0, 22/32)
	button.Left = left

	local right = button:CreateTexture(nil, "BACKGROUND")
	right:SetPoint("TOPRIGHT")
	right:SetPoint("BOTTOMRIGHT")
	right:SetWidth(12)
	right:SetTexCoord(68/128, 80/128, 0, 22/32)
	button.Right = right

	local middle = button:CreateTexture(nil, "BACKGROUND")
	middle:SetPoint("TOPLEFT", left, "TOPRIGHT")
	middle:SetPoint("BOTTOMRIGHT", right, "BOTTOMLEFT")
	middle:SetTexCoord(12/128, 68/128, 0, 22/32)
	button.Middle = middle

	local text = button:CreateFontString(nil, "OVERLAY", "GameFontNormal")
	text:SetAllPoints(true)
	text:SetJustifyH("CENTER")
	text:SetJustifyV("CENTER")
	button:SetFontString(text)
	button:SetNormalFontObject("GameFontNormal")
	button:SetHighlightFontObject("GameFontHighlight")	

	button:SetScript("OnMouseDown", OnMouseDown)
	button:SetScript("OnMouseUp", OnMouseUp)
	button:SetScript("OnShow", OnMouseUp)

	button.GetSkin = GetSkin
	button.SetSkin = SetSkin

	SetSkin(button, skin or "RED")

	return button
end
This (a) reuses tables and functions instead of creating duplicate copies for every new button, (b) does not require anything to be in the global namespace, unless you choose to give the button a global name, in which case it only adds one global per button, (c) minimizes table lookups, and (d) avoids the use of an expensive OnUpdate script. It also doesn't require any XML.

Then, instead of:
local button = CreateFrame("Button", nil, SomeFrame, "UIPanelButtonTemplate")
button.skin = "GREEN"
do:
local button = NewButton(parent, "GREEN")
When you want to change the skin, instead of doing button.skin = "RED" do button:SetSkin("RED").

If you want to check which skin the button is using, do button:GetSkin().
__________________
Author/maintainer of Grid, PhanxChat, ShieldsUp, and many more.
Troubleshoot an addonTurn any code into an addonMore addon resources
Need help with your code? Post all of your actual code! Attach or paste your files.
Please don’t PM me about addon bugs or code questions. Post a comment or forum thread instead!
Phanx is offline   Reply With Quote