View Single Post
12-01-14, 01:21 PM   #13
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
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.
__________________
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