UnitPowerMax returns 0 for units that do not use mana/rage/etc, such as many "warrior" type NPCs.
On a side note, your code is really inefficient.
(1) Instead of calling, for example, UnitPowerType 5 times in a row to check each power type, you should call it just once, and assign its return value to a variable, and then check that variable instead of calling the function again. You do the same thing with UnitPower, UnitPowerMax, etc.
(2) Instead of doing both string concatention (eg. a .. b) and string formatting (eg. string.format), you should just use one or the other. If you need formatting (eg. %.0f) put everything into string.format.
(3) Instead of formatting text yourself with string.format and then passing the result to :SetText, you should pass all of the arguments directly to :SetFormattedText, which does the same thing but offloads the work into C code, which is (presumably) faster.
(4) Less an efficiency issue and more a logic issue, but there are many power types other than the 0 for mana, 3 for energy, 1 for rage, 5 for runes, and 6 for runic power that you check for. Your "else" check will catch not only focus, but also soul shards, eclipse, holy power, chi, shadow orbs, burning embers, demonic fury, and any "alternate power" used by vehicles or special encounter mechanics. Also, you probably don't want to support runes here, as death knights will constantly see their mana bar switching between runic power (which is what they expect a mana bar to show) and runes (which I'm actually not sure how a single bar displays anyway). You should explicitly check for the power types that make sense to show on the mana bar, and ignore the others. Oh, and happiness was removed in Cataclysm, so it no longer exists as a power type.
(5) Another logic error, you only color the name text if the unit has a class. This means that if you target a unit without a class the name will remain colored by the class of the previous target. Also, the classes returned for NPCs are usually not relevant, so it would make more sense to color their names by reaction.
(6) Finally, you wrongly assume that a unit with energy is a rogue (or cat druid) with combo points, and you are using the
GetComboPoints function incorrectly. Monks use energy, but do not have combo points. As for GetComboPoints, it has taken
two arguments for several years now, as you can build combo points on more than one target separately. The usual use is GetComboPoints("player", "target") to find out how many CP the player has on the target.
Compare this with the snippet you posted:
Lua Code:
local firstload = true
-- upvalue frequently accessed globals for faster access:
local db = TextUnitFramesLiteDB
local format = string.format
-- store the player's class to check stuff later:
local _, playerClass = UnitClass("player")
function TUF:TargetUpdate()
if firstload and not db.general.combat then
firstload = false
target:SetPoint(db.target.loc.p, UIParent, db.target.loc.rP, db.target.loc.x, db.target.loc.y)
end
if UnitExists("target") then
self:LockTargetFrame()
local fg, fn = UnitFactionGroup("target")
if fg then -- explicitly checking ~= nil is unnecessary and wastes time
target.faction:SetTexture("Interface\\GROUPFRAME\\UI-Group-PVP-"..fg)
else
target.faction:SetTexture("Interface\\PVPFrame\\PVP-ArenaPoints-Icon")
end
target.faction:Show()
target.level:SetPoint("TOPLEFT", target, "TOPLEFT", 5, -5)
target.name:SetPoint("LEFT", target.level, "RIGHT", 5, 0)
if db.target.orientation then
target.health:ClearAllPoints()
target.mana:ClearAllPoints()
target.health:SetPoint("TOPLEFT", target.level, "BOTTOMLEFT", 0, 0)
target.mana:SetPoint("TOPLEFT", target.health, "BOTTOMLEFT", 0, 0)
else
target.health:ClearAllPoints()
target.mana:ClearAllPoints()
target.health:SetPoint("LEFT", target.name, "RIGHT", 5, 0)
target.mana:SetPoint("LEFT", target.health, "RIGHT", 0, 0)
end
target.level:SetFontObject(GameFontNormalLarge)
target.name:SetFontObject(GameFontNormalHuge)
target.health:SetFontObject(NumberFontNormalLarge)
target.mana:SetFontObject(NumberFontNormalLarge)
target.level:SetText(UnitLevel("target"))
target.name:SetText(UnitName("target"))
local _, cls = UnitClass("target")
if cls and UnitIsPlayer("target") then
-- support the CUSTOM_CLASS_COLORS standard used by !ClassColors and some others:
target.name:SetTextColor((CUSTOM_CLASS_COLORS or RAID_CLASS_COLORS)[cls].r, (CUSTOM_CLASS_COLORS or RAID_CLASS_COLORS)[cls].g, (CUSTOM_CLASS_COLORS or RAID_CLASS_COLORS)[cls].b)
elseif UnitIsFriend("target", "player") then
-- friendly, color it green
target.name:SetTextColor(0, 1, 0)
else
-- hostile, color it red
target.name:SetTextColor(1, 0, 0)
end
-- get these values once, and reuse them:
local hp, hpmax = UnitHealth("target"), UnitHealthMax("target")
local hpperc = hp / hpmax * 100
local htxtclr = TUF:SetTextColor(hpperc)
target.health:SetTextColor(htxtclr.r, htxtclr.g, htxtclr.b, 1)
-- get these values once, and reuse them:
local pp, ppmax, pptype = UnitPower("target"), UnitPowerMax("target"), UnitPowerType("target")
local mtxtclr = TUF:SetTextColor(pp / ppmax * 100)
target.mana:SetTextColor(mtxtclr.r, mtxtclr.g, mtxtclr.b, 1)
-- concatenation + string.format is inefficient; do it all in string.format!
-- better yet, use :SetFormattedText instead of string.format + :SetText to offload the work into faster C code:
if db.target.percent then
target.health:SetFormattedText("|cff00ff00H|r-%.0f%%", hpperc)
else
target.health:SetFormattedText("|cff00ff00H|r- %d|Cff666666 /|r%d", hp, hpmax)
end
-- also, since most of these are the same regardless of whether "db.target.percent" is true, no need to duplicate:
if pptype == 0 then -- mana
if db.target.percent then
target.mana:SetFormattedText("|cff3300cc M|r-%.0f%%", pp / ppmax * 100)
else
target.mana:SetFormattedText("|cff3300cc M|r-%d|Cff666666 /|r%d", pp, ppmax)
end
elseif pptype == 1 then -- rage
target.mana:SetFormattedText("|cffff3333Rage|r- %d|cff666666 /|r%d", pp, ppmax)
elseif pptype == 2 then -- focus
target.mana:SetFormattedText("|cffffcc00F|r- %d|cff666666 /|r%d", pp, ppmax)
elseif pptype == 3 then -- energy
if playerClass == "ROGUE" or playerClass == "DRUID" then
-- can have combo points
target.mana:SetFormattedText("|cffffff00E |r-%d |cff990099 Combo|r: %d", mp, GetComboPoints("player", "target"))
else
-- probably a monk, no combo points
target.mana:SetFormattedText("|cffffff00E |r-%d|cff666666 /|r%d", pp, ppmax)
end
elseif pptype == 6 then -- runic power
target.mana:SetFormattedText("|cff99ccffRP|r- %d|cff666666 /|r%d", pp, ppmax)
end
-- ignore other power types since they are additional resources, not mana equivalents.
-- instead of getting a bunch of values and then not using them, check first:
if not db.general.combat then
local ll = target.level:GetStringWidth()
local nl = target.name:GetStringWidth()
local hl = target.health:GetStringWidth()
local ml = target.mana:GetStringWidth()
-- instead of checking "if not x then A, elseif x then B" check "if x then B, else A":
if db.target.orientation then
local len = math.max(ll + nl, ml, hl)
target:SetWidth(len + 20 )
local lh = target.level:GetHeight()
local hh = target.health:GetHeight()
local mh = target.mana:GetHeight()
target:SetHeight(lh + hh + mh + 20)
else
local len = ll + nl + hl + ml + 20
target:SetWidth(len)
target:SetHeight(target.level:GetHeight() + 20)
end
end
if UnitExists("playertargettarget") then
if db.target.targettop then
target.target:ClearAllPoints()
target.target:SetPoint("TOPLEFT", target, "BOTTOMLEFT", 10, 10)
else
target.target:ClearAllPoints()
target.target:SetPoint("LEFT", target, "RIGHT", -10, 0)
end
target.target.level:SetFontObject(GameFontNormal)
target.target.level:SetText(UnitLevel("playertargettarget"))
target.target.level:SetPoint("TOPLEFT", target.target, "TOPLEFT", 5, -5)
target.target.name:SetFontObject(GameFontNormal)
target.target.name:SetText(UnitName("playertargettarget"))
target.target.name:SetPoint("LEFT", target.target.level, "RIGHT", 5, 0)
local ttlw = target.target.level:GetStringWidth()
local ttnw = target.target.name:GetStringWidth()
target.target:SetHeight(target.target.name:GetHeight()+12)
target.target:SetWidth(ttlw + ttnw + 10)
end
if not db.general.combat then
if db.target.enabled then target:Show() else target:Hide() end
if UnitExists("target") then target:Show() else target:Hide() end
if db.target.target and UnitExists("playertargettarget") then target.target:Show() else target.target:Hide() end
end
else
target:Hide()
end
end
There are probably a lot of other improvements to be made, just in that section alone, but that's all I have time for right now. Hopefully you can look over that and get a feel for what to work on.