No. When defining variables, you should limit them to the narrowest scope necessary. Likewise, when hooking functions, you should limit your hooks to the most specific functions necessary. If you're hooking a function for the purpose of moving the level text, you should hook the actual function that moves the level text, not some other function that calls the function that moves the level text, particularly in this case where you're running different code inside each different function hook. If you hooked
UnitFrame_Update instead, you'd have to add additional logic to check which unit was being updated.
Some other things:
Code:
hooksecurefunc("PartyMemberFrame_UpdateMember", function(self)
if self:IsShown() then
for i = 1, MAX_PARTY_MEMBERS do
_G["PartyMemberFrame"..i]:SetScale(ns.unitframesPartyScale)
end
PartyMemberFrame1:SetPoint(db.unitframes.party.position.relAnchor, UIParent, db.unitframes.party.position.offSetX, db.unitframes.party.position.offSetY);
end
end)
There are several problems with this:
You cannot move unit frames while in combat, so you need to check for
InCombatLockdown() before attempting to move a unit frame.
PartyMemberFrame_UpdateMember doesn't move the party frames, so hooking it is probably not a very good way to move the party frames. I don't know that the default UI ever moves the party frames, but if it does, you should find the function that actually does it, and hook that instead. If it doesn't, then you can just move them once and not worry about it.
PartyMemberFrame_UpdateMember is called for each frame being updated, but you're updating all the frames there, leading to a lot of extra updates. Also, the default UI never calls
SetScale on the party frames, so you can just do that once, as with the boss frames.
Code:
if not ArenaEnemyFrame1 then
return true
end
The arena frames are created by the Blizzard_ArenaUI "addon" which is loaded on demand, so unless you are in an arena when you log in, they will probably never exist when your addon loads, so nothing after this check will ever execute.
Instead, if the arena frames don't exist yet, you should delay dealing with them until they are created:
Code:
local function ScaleArenaFrames()
for i = 1, MAX_ARENA_ENEMIES do
_G["ArenaPrepFrame"..i]:SetScale(ns.unitframesArenaScale)
_G["ArenaEnemyFrame"..i]:SetScale(ns.unitframesArenaScale)
end
end
if IsAddOnLoaded("Blizzard_ArenaUI") then
ScaleArenaFrames()
else
local f = CreateFrame("Frame")
f:RegisterEvent("ADDON_LOADED")
f:SetScript("OnEvent", function(self, event, addon)
if addon == "Blizzard_ArenaUI" then
self:UnregisterEvent(event)
ScaleArenaFrames()
end
end)
end
Another problem with your arena frame check is you put the code to scale the boss frames after it, meaning that isn't getting run either.
And as for the boss frame code itself:
Code:
if (event == "INSTANCE_ENCOUNTER_ENGAGE_UNIT") then
for i = 1, MAX_BOSS_FRAMES do
_G["Boss"..i.."TargetFrame"]:SetScale(ns.unitframesBossScale)
end
end
This code isn't (I hope) running inside an event handler -- it's just in the main chunk of your file. There is no "event" variable there, so even if this code was being run, that "if" check would fail since its value is effectively nil, which doesn't match "INSTANCE_ENCOUNTER_ENGAGE_UNIT".
However, the boss frames are already created when your addon loads, so you can just run your "for" loop right away; there's no need to delay or listen for any events.
Overall your coding skills are improving. You're just overthinking/overcomplicating things here.