1. Data that is static/constant does not need to be called each time, i.e. if you know that the player must be a specific class then you don't need to check for it each time inside the routine, instead make it so the routine does not load or run at all if the class is wrong. Unlike when we are checking other targets, the player can only be one class when he logs in.
2. When a frame has several events registered, the function you write runs each time one of the events is received by the game from the server, meaning if you have that code it will run each time you get out of combat, enter combat, power is restored, e.g. This means the "event" will always have one value at a time.
3. It's a good habit to try split up things, like when I know I will run the same code several times I put that code in it's own function and call it when I need it, the idea of CheckTotems below is to run trough the 4 totems, if it's spawned to show the frame, if not then hide it. But since we can only do this outside of combat, at the begining there is the combat check, if the variable inCombat is set then it means we are in combat and we can't work with the frames. NOTE: not all frames are like this, I don't know the addon and such, but if you like try disable the return part so the frames are shown or hidden during combat too, if it works then it's just nicer isn't it?
4. getglobal and _G are the same
5. select will select the n-th value we got 'local class, enClass = UnitClass("player")' if we only want the enClass we can just do 'select(2, UnitClass("player"))' and another way to tackle these is for example "local _, enClass = ..." if you don't bother with the first one but you want the 2nd.
6. When assuming a frame exists like 't = _G["XiTimers_Timer"..i]' it's nice to first check that the object after that actually exists, before trying to run methods like t:Show(), in case it does not exist you will get an error and the addon code will fail, it's nicer for it to not fail like this.
Here is my suggestion, what I think you tried to make:
Code:
if select(2, UnitClass("player")) ~= "SHAMAN" then
return -- does not load the file because the player is not a Shaman
end
local f = CreateFrame("Frame")
f:RegisterEvent("PLAYER_ENTERING_WORLD")
f:RegisterEvent("PLAYER_REGEN_DISABLED")
f:RegisterEvent("PLAYER_REGEN_ENABLED")
f:RegisterEvent("UNIT_POWER")
local inCombat
local function CheckTotems()
if inCombat then
return -- if in combat we cant do anything with these frames
end
local t
for i = 1, 4 do
t = _G["XiTimers_Timer"..i]
if t then
if select(2, GetTotemInfo(i)) ~= "" then
t:Show()
else
t:Hide()
end
end
end
end
local function OnEvent(f, event, unit, power, ...)
if event == "PLAYER_REGEN_DISABLED" then
inCombat = 1
elseif event == "PLAYER_REGEN_ENABLED" then
inCombat = nil
CheckTotems()
elseif event == "PLAYER_ENTERING_WORLD" then
CheckTotems()
elseif event == "UNIT_POWER" then
if unit == "player" and power == "MANA" then
CheckTotems()
end
end
end
f:SetScript("OnEvent" OnEvent)