Thread Tools Display Modes
03-26-16, 11:09 AM   #1
Sharji
A Deviate Faerie Dragon
AddOn Author - Click to view addons
Join Date: May 2015
Posts: 14
Reusing frames the proper way

Hi

I have a wierd bug with my addon and I have really no idea how to fix it. First how it works:
Once you get to combat addon scans your action bars and remebers them by spell that are on specific button. If external module (ex. TDDps_Warlock) calls for a glowing a button, main addon (TDDps) creates overlay on specific button spell (ex Immolate) and shows a glow on it. Everything works EXCEPT changing specializations.

I am listening on 'PLAYER_TALENT_UPDATE' event and calling TDDps_DisableAddon();. Enable/disable/init sequence is like this:

Lua Code:
  1. ----------------------------------------------
  2. -- Disable dps addon functionality
  3. ----------------------------------------------
  4. function TDDps_DisableAddon()
  5.     if _TD['DPS_Enabled'] == 0 then
  6.         return;
  7.     end
  8.     TDButton_DestroyAllOverlays();
  9.     print(_tdInfo .. TDDpsName .. ': Disabling');
  10.     TDDps_Frame:SetScript('OnUpdate', nil);
  11.     DPS_Skill = nil;
  12.     TDDps_Frame.rotationEnabled = false;
  13.     _TD['DPS_Enabled'] = 0;
  14. end
  15.  
  16. ----------------------------------------------
  17. -- Initialize dps addon functionality
  18. ----------------------------------------------
  19. function TDDps_InitAddon()
  20.     TDDps_Frame:Show();
  21.  
  22.     TDDps_Frame:RegisterEvent('PLAYER_TARGET_CHANGED');
  23.     TDDps_Frame:RegisterEvent('PLAYER_TALENT_UPDATE');
  24.     TDDps_Frame:RegisterEvent('PLAYER_REGEN_DISABLED');
  25.     TDDps_Frame:RegisterEvent('PLAYER_REGEN_ENABLED');
  26.  
  27.     TDDps_Frame:SetScript('OnEvent', TDDps_OnEvent);
  28.  
  29.     print(_tdInfo .. TDDpsName .. ': Initialized');
  30. end
  31.  
  32. ----------------------------------------------
  33. -- Enable dps addon functionality
  34. ----------------------------------------------
  35. function TDDps_EnableAddon(mode)
  36.     print(_tdInfo .. TDDpsName .. ': Enabling');
  37.    
  38.     if _TD['DPS_NextSpell'] == nil then
  39.         print(_tdError .. TDDpsName .. ': No addon selected, cannot enable');
  40.         return;
  41.     end
  42.    
  43.     if _TD['DPS_Enabled'] == 1 then
  44.         return;
  45.     end
  46.    
  47.     _TD['DPS_Mode'] = mode;
  48.  
  49.     TDButton_Fetch();
  50.    
  51.     if _TD['DPS_OnEnable'] then
  52.         _TD['DPS_OnEnable']();
  53.     end
  54.  
  55.     TDDps_Frame:SetScript('OnUpdate', TDDps_OnUpdate);
  56.    
  57.     _TD['DPS_Enabled'] = 1;
  58.     print(_tdSuccess .. TDDpsName .. ': Enabled');
  59. end

Event system seems to be working fine. I think I have a problem with reusing frames becasue when i change specialization and enter combat 'PLAYER_REGEN_DISABLED' some wierd things are going. Spells are either not overlaying or some random spells are.

Here is my reusing frames system:

Lua Code:
  1. TDButton_FramePool = {};
  2. TDButton_Frames = {};
  3.  
  4. function TDButton_CreateOverlay(parent, id, texture, r, g, b)
  5.     local frame = tremove(TDButton_FramePool);
  6.     if not frame then
  7.         frame = CreateFrame('Frame', 'TDButton_Overlay_' .. id, parent);
  8.     else
  9.         frame:SetAttribute('name', 'TDButton_Overlay_' .. id);
  10.     end
  11.  
  12.     frame:SetParent(parent);
  13.     frame:SetFrameStrata('HIGH');
  14.     frame:SetPoint('CENTER', 0, 0);
  15.     frame:SetWidth(parent:GetWidth() * 1.4);
  16.     frame:SetHeight(parent:GetHeight() * 1.4);
  17.  
  18.     local t = frame.texture;
  19.     if not t then
  20.         t = frame:CreateTexture('GlowOverlay', 'OVERLAY');
  21.     end
  22.  
  23.     t:SetTexture(texture or TDDps_Options_GetTexture());
  24.     t:SetBlendMode('ADD');
  25.     t:SetAllPoints(frame);
  26.  
  27.     t:SetVertexColor(
  28.         r or TDDps_Options.highlightColor.r,
  29.         g or TDDps_Options.highlightColor.g,
  30.         b or TDDps_Options.highlightColor.b,
  31.         TDDps_Options.highlightColor.a
  32.     );
  33.     frame.texture = t;
  34.  
  35.     tinsert(TDButton_Frames, frame);
  36.     return frame;
  37. end
  38.  
  39. function TDButton_DestroyAllOverlays()
  40.     local frame;
  41.     for key, frame in pairs(TDButton_Frames) do
  42.         frame:Hide();
  43.     end
  44.     for key, frame in pairs(TDButton_Frames) do
  45.         tinsert(TDButton_FramePool, frame);
  46.         TDButton_Frames[key] = nil;
  47.     end
  48. end

Any idea what I might be doing wrong here?
  Reply With Quote
03-26-16, 11:23 AM   #2
Fizzlemizz
I did that?
 
Fizzlemizz's Avatar
Premium Member
AddOn Author - Click to view addons
Join Date: Dec 2011
Posts: 1,871
Rather than tremove and tinsert (which don't return anything) add the frame to your table by name
Code:
TDButton_FramePool = {};
 
function TDButton_CreateOverlay(parent, id, texture, r, g, b)
	local frame = TDButton_FramePool['TDButton_Overlay_' .. id]
	if not frame then
		frame = CreateFrame('Frame', 'TDButton_Overlay_' .. id, parent);
		TDButton_FramePool['TDButton_Overlay_' .. id] = frame
		frame:SetFrameStrata('HIGH');
		frame:SetPoint('CENTER', 0, 0);
	    	frame:SetWidth(parent:GetWidth() * 1.4);
		frame:SetHeight(parent:GetHeight() * 1.4);
		frame.texture = frame:CreateTexture('GlowOverlay', 'OVERLAY');
		frame.texture:SetAllPoints(frame);
		frame.texture:SetBlendMode('ADD');
	end
	frame.texture:SetTexture(texture or TDDps_Options_GetTexture());
	frame.texture:SetVertexColor(
		r or TDDps_Options.highlightColor.r,
		g or TDDps_Options.highlightColor.g,
		b or TDDps_Options.highlightColor.b,
		TDDps_Options.highlightColor.a
	);
	return frame;
end

function TDButton_DestroyAllOverlays()
        for key, frame in pairs(TDButton_FramePool) do
            frame:Hide();
        end
end
__________________
Fizzlemizz
Maintainer of Discord Unit Frames and Discord Art.
Author of FauxMazzle, FauxMazzleHUD and Move Pad Plus.

Last edited by Fizzlemizz : 03-26-16 at 12:32 PM.
  Reply With Quote
03-26-16, 11:28 AM   #3
Torhal
A Pyroguard Emberseer
 
Torhal's Avatar
AddOn Author - Click to view addons
Join Date: Aug 2008
Posts: 1,196
Originally Posted by Fizzlemizz View Post
Rather than tremove and tinsert (which don't return anything) add the frame name to your table by name
Code:
    local frame = TDButton_FramePool['TDButton_Overlay_' .. id]
    if not frame then
        frame = CreateFrame('Frame', 'TDButton_Overlay_' .. id, parent);
        TDButton_FramePool['TDButton_Overlay_' .. id] = frame
-- You waon't need to set the name attribute
    end
Except that tremove returns the value that's been removed. In your snippet above, you're only adding the frame to the pool if it's newly-created, but the destroy function removes them all so this will be an issue.
__________________
Whenever someone says "pls" because it's shorter than "please", I say "no" because it's shorter than "yes".

Author of NPCScan and many other AddOns.
  Reply With Quote
03-26-16, 11:40 AM   #4
Fizzlemizz
I did that?
 
Fizzlemizz's Avatar
Premium Member
AddOn Author - Click to view addons
Join Date: Dec 2011
Posts: 1,871
I Changed Destroy() to just hide them.

WoWProgramming doesn't mention tremove returning a value so I always assumed it didn't. I need to start checking more sources
__________________
Fizzlemizz
Maintainer of Discord Unit Frames and Discord Art.
Author of FauxMazzle, FauxMazzleHUD and Move Pad Plus.
  Reply With Quote
03-26-16, 04:24 PM   #5
Sharji
A Deviate Faerie Dragon
AddOn Author - Click to view addons
Join Date: May 2015
Posts: 14
Turned out i'm blind and forgot to remove layers from parent button:

Lua Code:
  1. function TDButton_DestroyAllOverlays()
  2.     local frame;
  3.     for key, frame in pairs(TDButton_Frames) do
  4.         frame:GetParent().tdOverlays = nil;
  5.         frame:ClearAllPoints();
  6.         frame:Hide();
  7.         frame:SetParent(UIParent);
  8.         frame.width = nil;
  9.         frame.height = nil;
  10.     end
  11.     for key, frame in pairs(TDButton_Frames) do
  12.         tinsert(TDButton_FramePool, frame);
  13.         TDButton_Frames[key] = nil;
  14.     end
  15. end

that line did the job: frame:GetParent().tdOverlays = nil;
Because at the begining of the showing overlay i had:
Lua Code:
  1. if button.tdOverlays and button.tdOverlays[id] then
  2.         button.tdOverlays[id]:Show();
  3.     else
  Reply With Quote
03-26-16, 05:07 PM   #6
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Is there a reason why you can't combine both loops to make it easier on the CPU?
Lua Code:
  1. function TDButton_DestroyAllOverlays()
  2.     local frame;
  3.     for key, frame in pairs(TDButton_Frames) do
  4.         frame:GetParent().tdOverlays = nil;
  5.         frame:ClearAllPoints();
  6.         frame:Hide();
  7.         frame:SetParent(UIParent);
  8.         frame.width = nil;
  9.         frame.height = nil;
  10.  
  11.         tinsert(TDButton_FramePool, frame);
  12.         TDButton_Frames[key] = nil;
  13.     end
  14. end
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote
03-27-16, 05:48 AM   #7
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,290
Is this really worth it? We are only talking about some memory usage here, that you willing to trade for cpu usage. Mostly this method will only be good on things like very big addon options gui, and there is a pretty big chance that options module is LOD.

Last edited by Resike : 03-27-16 at 05:54 AM.
  Reply With Quote
03-27-16, 02:10 PM   #8
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Honestly, it's a judgement call on how many UI objects you're comfortable creating and the ratio of used and unused objects. There will always be overhead when recycling resources. It's a balance between how frequent you're going to be reallocating such resources and how much would otherwise go to waste. This code can still be optimized to be lighter on both memory and CPU.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote
03-27-16, 03:43 PM   #9
Resike
A Pyroguard Emberseer
AddOn Author - Click to view addons
Join Date: Mar 2010
Posts: 1,290
Originally Posted by SDPhantom View Post
Honestly, it's a judgement call on how many UI objects you're comfortable creating and the ratio of used and unused objects. There will always be overhead when recycling resources. It's a balance between how frequent you're going to be reallocating such resources and how much would otherwise go to waste. This code can still be optimized to be lighter on both memory and CPU.
Yes, but not just reusing the object costs resources, but recycling them. Maybe at the end of the day, it's better just to create a new object. Of course the resource usage is spread out more with the first method, but it you create so many things at the same time for this to worth doing it, you already doing something wrong.
  Reply With Quote
03-27-16, 07:59 PM   #10
SDPhantom
A Pyroguard Emberseer
 
SDPhantom's Avatar
AddOn Author - Click to view addons
Join Date: Jul 2006
Posts: 2,313
Again, it's a judgement call. There are plenty of pros and cons on both sides.
__________________
WoWInterface AddOns
"All I want is a pretty girl, a decent meal, and the right to shoot lightning at fools."
-Anders (Dragon Age: Origins - Awakening)
  Reply With Quote

WoWInterface » Developer Discussions » General Authoring Discussion » Reusing frames the proper way

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off