Changing hotkey colour

[ObbleYeah]

The latest in my ongoing series of very annoying and relatively minor issues!

I'm trying to change the colour of the action button hotkeys so they're a little more readable on my bar, and finding them a little more resistant to change than i was expecting. The best i've managed is:

hooksecurefunc("ActionButton_Update", function(self)
 
            local hotkey = _G[self:GetName().."HotKey"]
            hotkey:ClearAllPoints()
            hotkey:SetPoint("TOPRIGHT", self, -2, 14.5)
            local hkfont = CreateFont("HotkeyFontForModernists")
            hkfont:SetFont("Fonts\\FRIZQT__.ttf", 12.5, "THINOUTLINE")
            NumberFontNormalSmallGray:SetFontObject(hkfont)
            NumberFontNormalSmallGray:SetTextColor(255/255, 209/255, 0)
            if (showkeybinds==false) then
                hotkey:Hide()
            end
    end)

which changes the colour of all the hotkeys apart from those on the main action buttons (ie. petbar, stances etc.)

[Phanx]

Dear god don't create a new font object every time a hotkey fontstring is updated. >_<

Also:

- SetFontObject only works on fontstrings that have never had SetFont called on them; I haven't checked, but I suspect this is the problem here. Just use SetFont instead.

- "THINOUTLINE" is not technically a valid flag value. The only valid flags are "OUTLINE", "THICKOUTLINE", and "MONOCHROME". If "THINOUTLINE" is producing the desired result, it's only because internally WoW is doing the C equivalent of string.match(flags, "OUTLINE").

- If you're going to hide the hotkey under some circumstance, check that first before wasting CPU cycles making other visual modifications.

- I see you're a fan of the Gratuitous Overuse Of Explicit Checks On Boolean Values (tm) technique. Just do "if not x" instead of "if x == false" unless you actually need to distinguish between nil and false.

- The only property changed in ActionButton_Update is the color of the fontstring, so that's all you should be changing in your hook; however, the color is also changed in ActionButton_OnUpdate, so you'll need to hook that as well. Both ActionButton_OnUpdate and ActionButton_UpdateHotkeys hide/show the text, so you'll need to hook both of those to hide it. The position and font are never updated after creation, so you can just loop over all the buttons and do that stuff once when your addon loads.

for i = 1, NUM_ACTIONBAR_BUTTONS do
   local hotkey = _G["ActionButton"..i.."HotKey"]
   hotkey:ClearAllPoints()
   hotkey:SetPoint("TOPRIGHT", self, -2, 14.5)
   hotkey:SetFont("Fonts\\FRIZQT__.ttf", 12.5, "OUTLINE")
end
-- repeat for buttons on other bars

local function CustomizeHotkey(self)
   local hotkey = _G[self:GetName().."HotKey"]
   if (not showkeybinds) then
      return hotkey:Hide()
   end
   hotkey:SetTextColor(1, 0.8, 0)
end
hooksecurefunc("ActionButton_Update", CustomizeHotkey) -- color
hooksecurefunc("ActionButton_OnUpdate", CustomizeHotkey) -- color, hide/show
hooksecurefunc("ActionButton_UpdateHotkeys", CustomizeHotkey) -- hide/show

[ObbleYeah]

Thanx Phanx, as always. Every time I think i'm getting a little more adept you push me back on my ass - but it's the only way i'll learn right?

Originally Posted by Phanx - "THINOUTLINE" is not technically a valid flag value. The only valid flags are "OUTLINE", "THICKOUTLINE", and "MONOCHROME". If "THINOUTLINE" is producing the desired result, it's only because internally WoW is doing the C equivalent of string.match(flags, "OUTLINE").

This is interesting, i'd never seen any documentation that made that obvious. What's the reason it even exists then? I did always wonder why the difference between THINOUTLINE and OUTLINE seemed so minuscule!

[Phanx]

Originally Posted by ObbleYeah This is interesting, i'd never seen any documentation that made that obvious. What's the reason it even exists then?

It doesn't exist. I suspect that internally WoW is doing this:

   if string.match(flags, "THICKOUTLINE") then
      -- do a thick outline
   elseif string.match(flags, "OUTLINE") then
      -- do a regular/thin outline
   else
      -- do no outline
   end

...which is why you can write stuff like "THINOUTLINE" to get an outline, and "NONE" to get no outline, instead of getting an error about your made-up values.

See also:
- http://wowpedia.org/API_FontString_SetFont
- http://wowprogramming.com/docs/widge...stance/SetFont

[zork]

That would explain why using THINOUTLINE always worked for me.

If you only want to change the color of the hotkey you can hook the color function of the hotkey. By default Blizzard is using the hotkey as your range indicator and thus rewriting the hotkey color for out of range targets and such.

Blizzard is doing this by calling SetVertexColor on the hotkey.
https://github.com/tekkub/wow-ui-sou...utton.lua#L324
https://github.com/tekkub/wow-ui-sou...utton.lua#L672

You can prevent an infinite loop by either using a condition that checks for colors or by applying your new color as SetTextColor as Phanx wrote. That way a different (non-hooked) function is called.

for i = 1, NUM_ACTIONBAR_BUTTONS do
   local hotkey = _G["ActionButton"..i.."HotKey"]
   hooksecurefunc(hotkey, "SetVertexColor ", function(...)
      print(...) --should be self,r,g,b,(a)
      --do stuff based on r,g,b
      self:SetTextColor(1, 0.8, 0)
   end)
end

[Phanx]

Creating 12 copies of the same function = bad.

local function FixColor(...) -- ONE copy of the function!
   print(...) --should be self,r,g,b,(a)
   --do stuff based on r,g,b
   self:SetTextColor(1, 0.8, 0)
end
 
for i = 1, NUM_ACTIONBAR_BUTTONS do
   local hotkey = _G["ActionButton"..i.."HotKey"]
   hooksecurefunc(hotkey, "SetVertexColor ", FixColor) -- reuse it!
end

[zork]

Thanks Phanks.

I got a follow up question on that part that I need to get answered for my current project.

I have a model frame. Each model lives on the same canvas. The canvas can hold a lot of models (200+) in extreme cases.

How should I define functions that work on my model frame? Should they be defined on the canvas or can I add them to each model.

Currently my implementation is like this:

--canvas
  local canvas = CreateFrame("Frame", nil, UIParent)
  canvas.models = {}
  
  function canvas:ModelOnMouseDown(model,...)
    print(model.name,...)
    model:Reset()
  end
  
  function canvas:ModelOnMouseUp(model,...)
    print(model.name,...)
  end
  
  function canvas:CreateModel(id)
  
    local m = CreateFrame("PlayerModel", nil, self)
    m:EnableMouse(true)
    m.id = id
    m.name = "model"..id
    
    function m:Reset()
      --stuff to reset model data
      self:PortraitZoom(0)
      print(self.name,"reseting values")
    end    
    
    m:SetScript("OnMouseDown", self.ModelOnMouseDown)
    m:SetScript("OnMouseUp", self.ModelOnMouseUp)   
  
    return m
  
  end
  
  for i=1, 50 do
    canvas.models[i] = canvas:CreateModel(i)
  end

My question is about model specific functions like m:Reset().

Should they be defined on the canvas aswell?

Currently I would define model specific funtions directly on the model. But I could create them on the canvas instead.

So the above would change to sth like this

--canvas
  local canvas = CreateFrame("Frame", nil, UIParent)
  canvas.models = {}
  
  function canvas:ModelOnMouseDown(model,...)
    print(model.name,...)
    self:ResetModel(model,...) --not sure if self is available in this context otherwise model:GetParent()
  end
  
  function canvas:ResetModel(model,...)
    --stuff to reset model data
    model:PortraitZoom(0)
    print(model.name,"reseting values")
  end    
  
  function canvas:ModelOnMouseUp(model,...)
    print(model.name,...)
  end
  
  function canvas:CreateModel(id)
  
    local m = CreateFrame("PlayerModel", nil, self)
    m:EnableMouse(true)
    m.id = id
    m.name = "model"..id
    
    m:SetScript("OnMouseDown", self.ModelOnMouseDown)
    m:SetScript("OnMouseUp", self.ModelOnMouseUp)   
  
    return m
  
  end
  
  for i=1, 50 do
    canvas.models[i] = canvas:CreateModel(i)
  end

Which version is better and why?

[Phanx]

You can reuse all of the functions, the reset function included. However, the way you're doing it is a bit odd, and actually your OnMouseUp/OnMouseDown scripts won't work; you've got an implied "self" as the first argument, and "model" as the first explicit argument, but the value passed in that place will actually be a string describing which mouse button was pressed/released. If you want "model" to work as you seem to want, you need to define those methods with a dot instead of a colon.

However, to keep things cleaner, I'd put all those functions in their own table, rather than attach them to the canvas (where they don't make any sense). Also, rather than creating 50 models up front, with (I assume) the possibility of more being created later, I'd just use a metatable to create them all on demand.

local canvas = CreateFrame("Frame", nil, UIParent)

local modelPrototype = {}

function modelPrototype:Reset()
	print(self.name, "resetting values")
	self:PortraitZoom(0)
end

function modelPrototype:OnMouseDown(button)
	print(self.name, button)
	self:Reset()
end

function modelPrototype:OnMouseUp(button)
	print(self.name, button)
end

canvas.models = setmetatable({}, { __index = function(t, id)
	local m = CreateFrame("PlayerModel", nil, canvas)
	m:EnableMouse(true)

	m.name = "model"..id
	m.id = id

	for k, v in pairs(modelPrototype) do
		m[k] = v
	end

	m:SetScript("OnMouseDown", m.OnMouseDown)
	m:SetScript("OnMouseUp", m.OnMouseUp)

	t[id] = m
	return m
end })

[zork]

That setmetatable syntax has always eluded me.

How would I add a new model?

tinsert(canvas.models,1)

That way?

p.s. I do not create 50 models upfront. That was just for the example.

Additionally. In the end...does the setmetatable syntax produce a different result than this?

local m = CreateFrame("PlayerModel", nil, canvas)
    m:EnableMouse(true)
 
    m.name = "model"..id
    m.id = id
 
  function m:Reset()
    print(self.name, "resetting values")
    self:PortraitZoom(0)
  end
  
  function m:OnMouseDown(...)
    print(self.name, ...)
    self:Reset()
  end
  
  function m:OnMouseUp(...)
    print(self.name, ...)
  end
  
  m:SetScript("OnMouseDown", m.OnMouseDown)
  m:SetScript("OnMouseUp", m.OnMouseUp)

If not. Why the hassle?

[ravagernl]

Originally Posted by zork That setmetatable syntax has always eluded me. How would I add a new model?

local model1 = canvas.models[1]
print(model1.id == 1) -- true

This will make the playermodel when you attempt to get it by index. In other words, when a table value equals nil, the __index function is called and the return value is passed.

[Phanx]

Originally Posted by zork That setmetatable syntax has always eluded me. How would I add a new model?

You just use it as if it already exists. If it doesn't already exist, the metatable will create it for you, and you'll never be the wiser.

local tbl = setmetatable({}, {
	__index = function(tbl, key)
		print("You looked up a nonexistent key:", key)
		tbl[key] = "o hai"
		return "o hai"
	end,
	__newindex = function(tbl, key, value)
		print("You wrote a new key/value pair:", key, "->", value)
		tbl[key] = value
	end,
	__call = function(arg1, arg2)
		print("You called the table like a function with arguments:", arg1, arg2)
	end,
end })

print(tbl["hi2u"])
=> "You looked up a nonexistent key: hi2u"
=> "o hai"

print(tbl["hi2u"])
=> "o hai"

tbl["o rly"] = "ya rly"
=> "You wrote a new key/value pair: o rly -> ya rly"

tbl["o rly"] = "no wai"
<no result>

tbl("cat")
=> "You called the table like a function with arguments: cat"

LibStub uses a metatable with __call so you can do this:

local libref = LibStub("LibSomething-1.0")

...instead of having to do this:

local libref = LibStub:GetLibrary("LibSomething-1.0")

...and LibDataBroker uses __index and __newindex to hide the real data object table from outside, eg:

local internal = {
	cat = "meow",
	dog = "woof",
}

local external = setmetatable({}, {
	__index = function(external, key)
		-- note nothing is written to this table, so
		-- every lookup will trigger the __index function
		return internal[key]
	end,
	__newindex = function(external, key, value)
		-- again, since nothing is ever written to
		-- this table, no keys will ever exist in it,
		-- so all writes will trigger __newindex
		if type(value) == type(internal[key]) then
			internal[key] = value
		else
			print("Wrong value type!")
		end
	end
})

[zork]

Very informative. Thanks.