View Single Post
08-27-16, 01:17 AM   #18
Aftermathhqt
A Molten Giant
 
Aftermathhqt's Avatar
AddOn Author - Click to view addons
Join Date: Dec 2009
Posts: 784
Originally Posted by Phanx View Post
There are several problems with your code. Let's start with this one:

Code:
hooksecurefunc("AuraButton_Update", function(self, index)
	...
end)
Everything inside that function gets re-run every time an aura button is updated. That includes this:

Code:
local BuffStatusBar = CreateFrame("StatusBar", nil, ABuffFrame)
Your statusbars aren't "jumping around" -- they're being covered up by new statusbars every time the button is updated. Eventually you'll end up with thousands of statusbars on every aura button, which will use up a lot of memory and impact your game's performance.

It also includes this:

Code:
BuffStatusBar:SetScript("OnUpdate", function(self, Elapsed)
	...
end)
When each of those thousands of statusbars per button has its own OnUpdate script running, that uses a lot of CPU time, which will impact your game's performance even more. Creating thousands of copies of the same function also uses lots of memory.

Fortunately these are easy problems to solve.

================================

First, stop creating a new copy of that OnUpdate function every time a button is update. Create one copy of the function, outside of your AuraButton_Update hook:

Code:
local function AuraButtonStatusBar_OnUpdate(self, Elapsed)
	...
end
Then inside your hook, instead of passing a new (unnamed) function to SetScript, just pass the name of the function you already have:

Code:
BuffStatusBar:SetScript("OnUpdate", AuraButtonStatusBar_OnUpdate)
Now you need to make a few minor adjustments. Your original OnUpdate function refers to local variables that only exist in the scope of the AuraButton_Update hook. Since you moved it outside of that scope, those references are now looking for and/or writing to global variables by those names. The ones it looks for (eg. "index") don't exist, so your code will fail with an error. The ones it reads will be created, and creating global variables with names like "Val" is just asking for trouble.

Code:
_,_,_,_,_, ABuffFrameDuration, Exps = UnitBuff("player", index)
1. Your underscore, "ABuffFrameDuration", and "Exps" are now being written into the global scope. This is bad. Don't do it. Add a "local" keyword at the beginning of the line.

2. There is no variable "index" in this scope. Instead of looking for one, look for a value stored on the statusbar itself under the key name "index".

3. You're always specifying "player" -- this will fail when you're using a vehicle. Instead of "player", look at PlayerFrame.unit, which will usually be "player", but gets swapped with "vehicle" automatically when you enter or leave a vehicle.

4. You're using UnitBuff, but AuraButton_Update also runs for debuffs, so you should use UnitAura here instead, and store the filter value that's passed to AuraButton_Update.

Fixed version:

Code:
local _,_,_,_,_, ABuffFrameDuration, Exps = UnitAura(PlayerFrame.unit, self.index, self.filter)
You need a "local" keyword here, too:

Code:
local Val = Exps-GetTime()
Correspondingly, in your AuraButton_Update hook, store the necessary values under the "index" and "filter" keys on the statusbar:

Code:
BuffStatusBar.index = index
BuffStatusBar.filter = filter
(This doesn't go in the one-time chunk, since it needs to happen every time the button is updated.)

You also need to update your hook function definition so it actually receives the "filter" argument:

Code:
hooksecurefunc("AuraButton_Update", function(self, index, filter)
Okay, now you're no longer creating thousands of functions!

================================

Let's move on to the "creating thousands of statusbars" problem. Instead of indiscriminately creating a new statusbar every time an aura button updates, check to see if one already exists. If it does, use it. If it doesn't, then and only then should you create a new one:

Code:
local BuffStatusBar = ABuffFrame.StatusBar
if not BuffStatusBar then
	-- It doesn't exist yet. Create it:
	BuffStatusBar = CreateFrame("StatusBar", nil, ABuffFrame)
	-- and store a reference to it so you can easily find it next time:
	ABuffFrame.StatusBar = BuffStatusBar
end
Now you have a chunk of code that's only running the first time a particular aura button is updated. Every time after that, the button will already have a statusbar, so that chunk of code gets skipped.

From here, the logical next step is to move other things that only need to happen once into that chunk of code, so they don't keep getting run over and over for no reason. Since the statusbar is something you are creating, the default UI will never touch it, so all of the lines of code that place and style the statusbar only need to happen once. Move them into that block. The only thing you need to do with your statusbar after it's created is set the "index" value.

================================

Now your code should work, but there are a few simple optimizations I'd recommend doing. Since your code is going to be running very frequently, you should do everything you can (within reason) to make it as fast as possible.

The first optimization is similar to what we did for the statusbar. The default UI isn't moving and styling icons and font strings on every button update, so you don't need to do that either -- you only need to do that once, on the first update for each button. Conveniently, you already have a way to tell if this is the first update for a given button -- if it doesn't have a statusbar yet, it's the first update. Just move all of your other one-time styling code into the same chunk of code where you create a statusbar.

================================

The second optimization would be to use just one OnUpdate function to update all the statusbars, instead of a separate OnUpdate function for each statusbar, and to use GetTime() directly as the statusbar value instead of doing math calculations. The parent frame for buffs and debuffs already has an OnUpdate function, so just piggyback on that:

Code:
BuffFrame:HookScript("OnUpdate", function()
	-- Store the current time:
	local now = GetTime()

	-- Update all the buffs:
	for i = 1, BUFF_ACTUAL_DISPLAY do
		local button = _G["BuffButton"..i]
		if button.timeLeft then
			button.StatusBar:SetValue(now)
		end
	end

	-- Update all the debuffs:
	for i = 1, DEBUFF_ACTUAL_DISPLAY do
		local button = _G["DebuffButton"..i]
		if button.timeLeft then
			button.StatusBar:SetValue(now)
		end
	end
end)
Again, this should be outside of your AuraButton_Update hook.

You don't need to worry about whether or not the statusbar exists here, because BUFF_ACTUAL_DISPLAY is a count of how many buttons have already been updated, so your AuraButton_Update hook will already have been run for every button this loop catches.

However, you need to make another adjustment inside your AuraButton_Update hook to support using times directly as statusbar values:

Code:
-- Remove these two lines; we don't need them anymore:
BuffStatusBar.index = index
BuffStatusBar.filter = filter

-- Add these ones instead:
local _, _, _, _, _, duration, expirationTime = UnitAura(PlayerFrame.unit, index, filter)
BuffStatusBar:SetMinMaxValues(expirationTime - duration, expirationTime)
Now the bar's min value is the aura's start time, and it's max value is the aura's end time, so you can just set "now" as the current value in the OnUpdate script. You've also avoided calling UnitAura inside the OnUpdate script, which adds another performance boost.

(Note that I didn't actually test any of this code in-game, so there may be typos. If you get an error and can't figure it out, post your complete updated code and the actual error message.)
Thank you again Phanx, you're a life saver!

This makes everything much more easier, when explained like this

It works, but its behaving strange, the bars fills up instead of down, and sometime it doesn't really even do anything.

Lua Code:
  1. local Timer = 0
  2. local NumBuffsSkinned = 0
  3.  
  4. local function AuraButtonStatusBar_OnUpdate(self, Elapsed)
  5.    
  6.     local _,_,_,_,_, ABuffFrameDuration, Exps = UnitAura(PlayerFrame.unit, self.index, self.filter)
  7.     local Val = Exps-GetTime()
  8.    
  9.     BuffStatusBar:SetScript("OnUpdate", function(self, Elapsed)
  10.         Timer = Timer + Elapsed
  11.         if Timer >= .1 then
  12.             if ABuffFrameDuration == 0 then
  13.                 self:SetValue(1)
  14.             else
  15.                 if Exps then
  16.                     self:SetValue(Val/ABuffFrameDuration)
  17.                 end
  18.             end
  19.            
  20.             Timer = 0
  21.         end
  22.        
  23.         NumBuffsSkinned = self.index
  24.     end)
  25. end
  26.  
  27. hooksecurefunc("AuraButton_Update", function(self, index, filter)
  28.     local ABuffFrame = _G[self..index]
  29.     if ABuffFrame then
  30.         ABuffFrame:SetSize(BuffSize, BuffSize)
  31.         A.CreateBorder(ABuffFrame, true, BuffSize, BuffSize)
  32.  
  33.         local BuffStatusBar = ABuffFrame.StatusBar
  34.         if not BuffStatusBar then
  35.             local _, _, _, _, _, duration, expirationTime = UnitAura(PlayerFrame.unit, index, filter)
  36.             BuffStatusBar = CreateFrame("StatusBar", nil, ABuffFrame)
  37.             BuffStatusBar:SetFrameStrata("HIGH")
  38.             BuffStatusBar:SetSize(32, 6)
  39.             BuffStatusBar:SetPoint("BOTTOM", ABuffFrame, 0, -8)
  40.             BuffStatusBar:SetMinMaxValues(expirationTime - duration, expirationTime)
  41.             BuffStatusBar:SetStatusBarTexture(C.Media.Texture)
  42.             BuffStatusBar:SetStatusBarColor(A.ClassColor.r, A.ClassColor.g, A.ClassColor.b)
  43.             A.Skin(BuffStatusBar)
  44.    
  45.             ABuffFrame.StatusBar = BuffStatusBar           
  46.         end
  47.     end
  48.    
  49.     local ABuffFrameIcon = _G[self..index.."Icon"]
  50.     if ABuffFrameIcon then
  51.         ABuffFrameIcon:SetTexCoord(0.03, 0.97, 0.03, 0.97)
  52.     end
  53.    
  54.     local ABuffFrameDuration = _G[self..index.."Duration"]
  55.     if ABuffFrameDuration then
  56.         ABuffFrameDuration:SetDrawLayer("OVERLAY")
  57.         ABuffFrameDuration:ClearAllPoints()
  58.         ABuffFrameDuration:SetPoint("BOTTOM", ABuffFrame, "BOTTOM", 1, -15.5)
  59.         ABuffFrameDuration:SetFont(C.Media.Font, 12, "THINOUTLINE")
  60.         ABuffFrameDuration:SetShadowOffset(1, -1)
  61.         ABuffFrameDuration:SetShadowColor(0, 0, 0)
  62.         ABuffFrameDuration:SetAlpha(0)
  63.     end
  64.    
  65.     local ABuffFrameCount = _G[self..index.."Count"]
  66.     if ABuffFrameCount then
  67.         ABuffFrameCount:SetDrawLayer("OVERLAY")
  68.         ABuffFrameCount:ClearAllPoints()
  69.         ABuffFrameCount:SetPoint("BOTTOMRIGHT", ABuffFrame, 1, 1)
  70.         ABuffFrameCount:SetFont(C.Media.Font, 12.5, "THINOUTLINE")
  71.         ABuffFrameCount:SetShadowOffset(1, -1)
  72.         ABuffFrameCount:SetShadowColor(0, 0, 0)
  73.     end
  74.    
  75.     local ABuffFrameBorder = _G[self..index.."Border"]
  76.     if ABuffFrameBorder then
  77.         local R, G, B = ABuffFrameBorder:GetVertexColor()
  78.         ABuffFrameBorder:Hide()
  79.         ABuffFrameBorder:SetTexture(nil)
  80.         A.ColorBorder(ABuffFrame, R, G, B)     
  81.     end
  82. end)
  83.  
  84. BuffFrame:HookScript("OnUpdate", function()
  85.     -- Store the current time:
  86.     local now = GetTime()
  87.  
  88.     -- Update all the buffs:
  89.     for i = 1, BUFF_ACTUAL_DISPLAY do
  90.         local button = _G["BuffButton"..i]
  91.         if button.timeLeft then
  92.             button.StatusBar:SetValue(now)
  93.         end
  94.     end
  95.  
  96.     -- Update all the debuffs:
  97.     for i = 1, DEBUFF_ACTUAL_DISPLAY do
  98.         local button = _G["DebuffButton"..i]
  99.         if button.timeLeft then
  100.             button.StatusBar:SetValue(now)
  101.         end
  102.     end
  103. end)

Last edited by Aftermathhqt : 08-27-16 at 01:41 AM.
  Reply With Quote