WoWInterface - Reusing frames the proper way

WoWInterface (https://www.wowinterface.com/forums/index.php)

- General Authoring Discussion (https://www.wowinterface.com/forums/forumdisplay.php?f=20)

- - Reusing frames the proper way (https://www.wowinterface.com/forums/showthread.php?t=53265)

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:

----------------------------------------------
-- Disable dps addon functionality
----------------------------------------------
function TDDps_DisableAddon()
    if _TD['DPS_Enabled'] == 0 then
        return;
    end
    TDButton_DestroyAllOverlays();
    print(_tdInfo .. TDDpsName .. ': Disabling');
    TDDps_Frame:SetScript('OnUpdate', nil);
    DPS_Skill = nil;
    TDDps_Frame.rotationEnabled = false;
    _TD['DPS_Enabled'] = 0;
end
 
----------------------------------------------
-- Initialize dps addon functionality
----------------------------------------------
function TDDps_InitAddon()
    TDDps_Frame:Show();
 
    TDDps_Frame:RegisterEvent('PLAYER_TARGET_CHANGED');
    TDDps_Frame:RegisterEvent('PLAYER_TALENT_UPDATE');
    TDDps_Frame:RegisterEvent('PLAYER_REGEN_DISABLED');
    TDDps_Frame:RegisterEvent('PLAYER_REGEN_ENABLED');
 
    TDDps_Frame:SetScript('OnEvent', TDDps_OnEvent);
 
    print(_tdInfo .. TDDpsName .. ': Initialized');
end
 
----------------------------------------------
-- Enable dps addon functionality
----------------------------------------------
function TDDps_EnableAddon(mode)
    print(_tdInfo .. TDDpsName .. ': Enabling');
    
    if _TD['DPS_NextSpell'] == nil then
        print(_tdError .. TDDpsName .. ': No addon selected, cannot enable');
        return;
    end
    
    if _TD['DPS_Enabled'] == 1 then
        return;
    end
    
    _TD['DPS_Mode'] = mode;
 
    TDButton_Fetch();
    
    if _TD['DPS_OnEnable'] then
        _TD['DPS_OnEnable']();
    end
 
    TDDps_Frame:SetScript('OnUpdate', TDDps_OnUpdate);
    
    _TD['DPS_Enabled'] = 1;
    print(_tdSuccess .. TDDpsName .. ': Enabled');
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:

TDButton_FramePool = {};
TDButton_Frames = {};
 
function TDButton_CreateOverlay(parent, id, texture, r, g, b)
    local frame = tremove(TDButton_FramePool);
    if not frame then
        frame = CreateFrame('Frame', 'TDButton_Overlay_' .. id, parent);
    else
        frame:SetAttribute('name', 'TDButton_Overlay_' .. id);
    end
 
    frame:SetParent(parent);
    frame:SetFrameStrata('HIGH');
    frame:SetPoint('CENTER', 0, 0);
    frame:SetWidth(parent:GetWidth() * 1.4);
    frame:SetHeight(parent:GetHeight() * 1.4);
 
    local t = frame.texture;
    if not t then
        t = frame:CreateTexture('GlowOverlay', 'OVERLAY');
    end
 
    t:SetTexture(texture or TDDps_Options_GetTexture());
    t:SetBlendMode('ADD');
    t:SetAllPoints(frame);
 
    t:SetVertexColor(
        r or TDDps_Options.highlightColor.r,
        g or TDDps_Options.highlightColor.g,
        b or TDDps_Options.highlightColor.b,
        TDDps_Options.highlightColor.a
    );
    frame.texture = t;
 
    tinsert(TDButton_Frames, frame);
    return frame;
end
 
function TDButton_DestroyAllOverlays()
    local frame;
    for key, frame in pairs(TDButton_Frames) do
        frame:Hide();
    end
    for key, frame in pairs(TDButton_Frames) do
        tinsert(TDButton_FramePool, frame);
        TDButton_Frames[key] = nil;
    end
end

Any idea what I might be doing wrong here?

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

Quote:

Originally Posted by Fizzlemizz (Post 313812)

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.

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 :o

Turned out i'm blind and forgot to remove layers from parent button:

Lua Code:

function TDButton_DestroyAllOverlays()
    local frame;
    for key, frame in pairs(TDButton_Frames) do
        frame:GetParent().tdOverlays = nil;
        frame:ClearAllPoints();
        frame:Hide();
        frame:SetParent(UIParent);
        frame.width = nil;
        frame.height = nil;
    end
    for key, frame in pairs(TDButton_Frames) do
        tinsert(TDButton_FramePool, frame);
        TDButton_Frames[key] = nil;
    end
end

that line did the job: frame:GetParent().tdOverlays = nil;
Because at the begining of the showing overlay i had:

Lua Code:

if button.tdOverlays and button.tdOverlays[id] then
        button.tdOverlays[id]:Show();
    else

Is there a reason why you can't combine both loops to make it easier on the CPU?

Lua Code:

function TDButton_DestroyAllOverlays()
    local frame;
    for key, frame in pairs(TDButton_Frames) do
        frame:GetParent().tdOverlays = nil;
        frame:ClearAllPoints();
        frame:Hide();
        frame:SetParent(UIParent);
        frame.width = nil;
        frame.height = nil;
 
        tinsert(TDButton_FramePool, frame);
        TDButton_Frames[key] = nil;
    end
end

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.

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.

Quote:

Originally Posted by SDPhantom (Post 313859)

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.

Again, it's a judgement call. There are plenty of pros and cons on both sides.