View Single Post
03-13-14, 10:32 PM   #3
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
1. For the love of kittens and unicorns, use some freaking variables instead of doing long table lookups over and over. Seeing c['media'].fontNormal written out 500 times is just excruciating.

2. This kind of thing causes Bad Taint, and will definitely get your addon blamed (and rightfully so) for ACTION_BLOCKED errors:

Code:
    -- Make the new locations stay
    PartyMemberFrame1.SetPoint = function() end;
    PartyMemberFrame2.SetPoint = function() end;
    PartyMemberFrame3.SetPoint = function() end;
    PartyMemberFrame4.SetPoint = function() end;
You cannot overwrite functions or (anything else) on a secure frame without breaking something. Your function is insecure, and taints the entire frame. Instead, you need to figure out which Blizzard function(s) move those frames, and post-hook them to move the frames back to the desired location after the default UI repositions them, but only if you're not in combat.

3. Globals are bad. Don't make anything a global unless you have a Really Good Reason. If you're not 100% sure something needs to be global, it doesn't need to be global, so make it local. On a similar topic, always define variables in the narrowest/strictest/lowest scope necessary. For example, here you are doing two things "wrong":

Code:
    if C["unitframes"].arena.tracker == true then
        trinkets = {};
        -- ^^ This is a global (bad) and a very generically named
        -- global at that (even worse). Add a "local" in front of it.
        local arenaFrame,trinket;
        -- ^^ These variables do not need to exist in this scope.
        -- Get rid of this line, and add a "local" in front of each one where
        -- they are defined INSIDE the loop below, since that is the only
        -- place they are used.
        for i = 1, 5 do
            arenaFrame = "ArenaEnemyFrame"..i;
            trinket = CreateFrame("Cooldown", arenaFrame.."Trinket", ArenaEnemyFrames);
(Actually you're doing a third thing "wrong" by parenting all 5 trinket icons the "ArenaEnemyFrames" frame, whatever that is, instead of parenting each one to the specific frame it's associated with. Change "ArenaEnemyFrames" to "arenaFrame" there.)

Finally, ArenaJunkies is a horrible place to get any kind of code. I've never seen any code snippet attributed to ArenaJunkies that wasn't a contender for the Worst Code Ever Written prize.
__________________
Retired author of too many addons.
Message me if you're interested in taking over one of my addons.
Don’t message me about addon bugs or programming questions.
  Reply With Quote