Thread Tools Display Modes
02-28-14, 05:14 AM   #1
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
Health Text Code Optimization

Good morning,

the following segment of code is working exactly as intended, it just feels rather awkward, particularly the hcur = UnitHealth('player') or UnitHealth('vehicle') in the UNIT_HEALTH event. I imagine there's a better way to do this, any help would be greatly appreciated:

Lua Code:
  1. local ht = CreateFrame('Button', 'PlayerHealthTextFrame', self)
  2.             ht:SetSize(50,10)
  3.             ht:SetPoint('LEFT', self, 'RIGHT', 7, 3)
  4.             ht:EnableMouse(false)
  5.             ht:RegisterEvent('PLAYER_ENTERING_WORLD')
  6.             ht:RegisterEvent('PLAYER_DEAD')
  7.             ht:RegisterEvent('PLAYER_ALIVE')
  8.             ht:RegisterUnitEvent('UNIT_HEALTH', 'player', 'vehicle')
  9.             ht:RegisterUnitEvent('UNIT_ENTERING_VEHICLE', 'player')
  10.             ht:RegisterUnitEvent('UNIT_ENTERED_VEHICLE', 'player')
  11.             ht:RegisterUnitEvent('UNIT_EXITING_VEHICLE', 'player')
  12.             ht:RegisterUnitEvent('UNIT_EXITED_VEHICLE', 'player')
  13.    
  14.         local htt = ht:CreateFontString('PlayerHealthText', 'OVERLAY')
  15.             htt:SetAllPoints(true)
  16.             htt:SetFont(cfg.font, 11)
  17.             htt:SetShadowColor(0,0,0,1)
  18.             htt:SetShadowOffset(-1,-1)
  19.             htt:SetJustifyH('LEFT')
  20.    
  21.         ht:SetScript('OnEvent', function(self, event, unit)
  22.             if event == 'PLAYER_DEAD' or event == 'PLAYER_ENTERING_WORLD' and UnitIsDead('player') then
  23.                 htt:SetFormattedText('|cff%02x%02x%02xDead', 180, 0, 0)
  24.             elseif event == 'PLAYER_ALIVE' and UnitIsGhost('player') or event == 'PLAYER_ENTERING_WORLD' and UnitIsGhost('player') then
  25.                 htt:SetFormattedText('|cff%02x%02x%02xGhost', 180, 0, 0)
  26.             else
  27.                 local r, g, l, hcur, hmax
  28.                 if event == 'PLAYER_ENTERING_WORLD' and UnitInVehicle('player') or event == 'UNIT_ENTERED_VEHICLE' or event == 'UNIT_ENTERING_VEHICLE' then
  29.                     hcur, hmax = UnitHealth('vehicle'), UnitHealthMax('vehicle')
  30.                 elseif event == 'PLAYER_ENTERING_WORLD' or 'UNIT_EXITING_VEHICLE' or 'UNIT_EXITED_VEHICLE' then
  31.                     hcur, hmax = UnitHealth('player'), UnitHealthMax('player')
  32.                 elseif event == 'UNIT_HEALTH' then
  33.                     hcur = UnitHealth('player') or UnitHealth('vehicle')
  34.                     hmax = UnitHealthMax('player') or UnitHealthMax('vehicle')
  35.                 end
  36.                 if hcur < hmax then
  37.                     r, g = 180, 0
  38.                 else
  39.                     r, g = 0, 180
  40.                 end
  41.                
  42.                 if hcur > 1000000 then
  43.                     l = 'm'
  44.                     hcur = round(hcur/1000000,1)
  45.                 elseif hcur > 1000 then
  46.                     l = 'k'
  47.                     hcur = round(hcur/1000)
  48.                 else
  49.                     l = ''
  50.                 end
  51.                 htt:SetFormattedText('|cff%02x%02x%02x%s'..l, r, g, 0, hcur)
  52.             end
  53.         end)
  Reply With Quote
02-28-14, 09:31 AM   #2
semlar
A Pyroguard Emberseer
 
semlar's Avatar
AddOn Author - Click to view addons
Join Date: Sep 2007
Posts: 1,060
First things first, you aren't grouping your conditions.
Lua Code:
  1. elseif event == 'PLAYER_ALIVE' and UnitIsGhost('player') or event == 'PLAYER_ENTERING_WORLD' and UnitIsGhost('player')
Should be written like this using parentheses to group statements that should be evaluated together
Lua Code:
  1. elseif (event == 'PLAYER_ALIVE' or event == 'PLAYER_ENTERING_WORLD') and UnitIsGhost('player')

I would personally replace
Lua Code:
  1. if event == 'PLAYER_ENTERING_WORLD' and UnitInVehicle('player') or event == 'UNIT_ENTERED_VEHICLE' or event == 'UNIT_ENTERING_VEHICLE' then
  2.     hcur, hmax = UnitHealth('vehicle'), UnitHealthMax('vehicle')
  3. elseif event == 'PLAYER_ENTERING_WORLD' or 'UNIT_EXITING_VEHICLE' or 'UNIT_EXITED_VEHICLE' then
  4.     hcur, hmax = UnitHealth('player'), UnitHealthMax('player')
  5. elseif event == 'UNIT_HEALTH' then
  6.     hcur = UnitHealth('player') or UnitHealth('vehicle')
  7.     hmax = UnitHealthMax('player') or UnitHealthMax('vehicle')
  8. end
with something like this, since checking what event is firing doesn't really matter here
Lua Code:
  1. local unit = UnitInVehicle('player') and 'vehicle' or 'player'
  2. local hcur, hmax = UnitHealth(unit), UnitHealthMax(unit)

The main time you would want to check the event is if you're watching something like UNIT_HEALTH_FREQUENT which fires very frequently and want to reduce how much you're doing in response to that event.
  Reply With Quote
02-28-14, 07:40 PM   #3
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
As a follow up I have found one thing that doesn't work:

I've tried debugging but that only makes me even more confused. I am testing this by falling to my death.

When I hit the ground my chat frame will print: (dead "PLAYER_DEAD") yet the health text remains '0'

If I reload ui while dead (without releasing) it will print (dead "PLAYER_ENTERING_WORLD") and the health text will show 'Dead'

I don't get how the event can fire and the text isn't updated, requiring a reloadui to truly update the text.

The entire function now reads:

Lua Code:
  1. ht:SetScript('OnEvent', function(self, event, unit)
  2.             if (event == 'PLAYER_ENTERING_WORLD' and UnitIsDead('player')) or event == 'PLAYER_DEAD' then
  3.                 print('dead '..event)
  4.                 htt:SetFormattedText('|cff%02x%02x%02xDead', 180, 0, 0)
  5.             elseif (event == 'PLAYER_ALIVE' or 'PLAYER_ENTERING_WORLD') and UnitIsGhost('player') then
  6.                 print('ghost '..event)
  7.                 htt:SetFormattedText('|cff%02x%02x%02xGhost', 180, 0, 0)
  8.             else
  9.                 local r, g, l, hcur, hmax
  10.                 if event == 'PLAYER_ENTERING_WORLD' and UnitInVehicle('player') or event == 'UNIT_ENTERED_VEHICLE' or event == 'UNIT_ENTERING_VEHICLE' then
  11.                     hcur, hmax = UnitHealth('vehicle'), UnitHealthMax('vehicle')
  12.                 elseif event == 'PLAYER_ENTERING_WORLD' or 'UNIT_EXITING_VEHICLE' or 'UNIT_EXITED_VEHICLE' then
  13.                     hcur, hmax = UnitHealth('player'), UnitHealthMax('player')
  14.                 elseif event == 'UNIT_HEALTH' then
  15.                     hcur = UnitHealth('player') or UnitHealth('vehicle')
  16.                     hmax = UnitHealthMax('player') or UnitHealthMax('vehicle')
  17.                 end
  18.                 if hcur < hmax then
  19.                     r, g = 180, 0
  20.                 else
  21.                     r, g = 0, 180
  22.                 end
  23.                
  24.                 if hcur >= 1000000 then
  25.                     l = 'm'
  26.                     hcur = round(hcur/1000000,1)
  27.                 elseif hcur >= 1000 then
  28.                     l = 'k'
  29.                     hcur = round(hcur/1000)
  30.                 else
  31.                     l = ''
  32.                 end
  33.                 htt:SetFormattedText('|cff%02x%02x%02x%s'..l, r, g, 0, hcur)
  34.             end
  35.         end)
  Reply With Quote
02-28-14, 10:38 PM   #4
Phanx
Cat.
 
Phanx's Avatar
AddOn Author - Click to view addons
Join Date: Mar 2006
Posts: 5,617
That's an awful lot of unnecessary event checking.

You don't need to listen for PLAYER_DEAD and PLAYER_ALIVE at all, as UNIT_HEALTH will fire for the player unit at those times anyway. You don't need UNIT_ENTERING_VEHICLE and UNIT_EXITING_VEHICLE, as you only care when those transitions end, not when they begin.

Lua Code:
  1. ht:SetScript("OnEvent", function(self, event, unit)
  2.     if UnitIsGhost("player") then
  3.         return htt:SetText("|cffb40000Dead")
  4.     elseif UnitIsDead("player") then
  5.         return htt:SetText("|cffb40000Ghost")
  6.     end
  7.  
  8.     local hcur, hmax = UnitHealth(unit or "player"), UnitHealthMax(unit or "player")
  9.  
  10.     if hcur >= 1000000 then
  11.         hcur = format("%.1fm", hcur / 1000000)
  12.     elseif hcur >= 1000 then
  13.         hcur = format("%.0fk", hcur / 1000)
  14.     end
  15.  
  16.     if hcur == hmax then
  17.         htt:SetFormattedText("|cff00b400%s", hcur)
  18.     else
  19.         htt:SetFormattedText("|cffbf0000%s", hcur)
  20.     end
  21. end)

I'm fairly sure, but not 100% sure, that UNIT_HEALTH will fire anyway when you enter or leave a vehicle, so you don't actually need to register for those events. However, if that's not the case, just register ENTERED and EXITED to tell you when those transitions end, and adjust the event handler as follows:

Code:

+	if event == "UNIT_ENTERED_VEHICLE" then
+		unit = "vehicle"
+	end

	local hcur, hmax = UnitHealth(unit or "player"), UnitHealthMax(unit or "player")
__________________
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
03-01-14, 11:16 AM   #5
sirann
A Flamescale Wyrmkin
Join Date: Mar 2007
Posts: 142
I thank you, yet again, for your assistance, there were a few things I needed to change:

the unitisghost had to be changed to say ghost and not dead, and the unitisdead had to be changed to dead and not ghost :P

second, the previous code was comparing a formatted text to a number It would add k or m and round the number and then compare, resulting it always giving a red number as they could never be the same

upon entering a vehicle the UNIT_HEALTH event does not suffice unfortunately and the event check had to be added making the end code this:

Lua Code:
  1. ht:SetScript("OnEvent", function(self, event, unit)
  2.             local r, g
  3.             if UnitIsGhost("player") then
  4.                 return htt:SetText("|cffb40000Ghost")
  5.             elseif UnitIsDead("player") then
  6.                 return htt:SetText("|cffb40000Dead")
  7.             end
  8.            
  9.             if event == 'UNIT_ENTERED_VEHICLE' then
  10.                 unit = 'vehicle'
  11.             end
  12.          
  13.             local hcur, hmax = UnitHealth(unit or "player"), UnitHealthMax(unit or "player")
  14.            
  15.             if hcur < hmax then
  16.                 r, g = 180, 0
  17.             else
  18.                 r, g = 0, 180
  19.             end
  20.          
  21.             if hcur >= 1000000 then
  22.                 hcur = format("%.1fm", hcur / 1000000)
  23.             elseif hcur >= 1000 then
  24.                 hcur = format("%.0fk", hcur / 1000)
  25.             end
  26.            
  27.             htt:SetFormattedText("|cff%02x%02x%02x%s",r, g, 0, hcur)
  28.         end)
  Reply With Quote

WoWInterface » Developer Discussions » General Authoring Discussion » Health Text Code Optimization

Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off