Health Bar Smoothing Addon (Code Critique Please)

[Aizhia]

Hi All,

I'm working on a number of small addon projects to teach myself the basics before moving on to a larger project. What I'm doing is starting with the default UI and going through it making small "improvements" here and there.

The fist one I've done is an addon to smooth the health bars of the default unit frames. I'd appreciate some feedback.

local _HealthBar_OnValueChanged = HealthBar_OnValueChanged

HealthBar_OnValueChanged = function(healthBar, value, ...)
	_HealthBar_OnValueChanged(healthBar, value, true)
end

local _UnitFrameHealthBar_Update = UnitFrameHealthBar_Update

UnitFrameHealthBar_Update = function(statusbar, unit)
	_UnitFrameHealthBar_Update(statusbar, unit)

	if (not statusbar or statusbar.disconnected) then return end

	local min, max = statusbar:GetMinMaxValues();
	if (max <= min) then return end

	local value = statusbar:GetValue()
	if ( (value < min) or (value > max) ) then return end

	value = (value - min) / (max - min);

	local r, g
	if(value > 0.5) then
		r = (1.0 - value) * 2;
		g = 1.0;
	else
		r = 1.0;
		g = value * 2;
	end
	statusbar:SetStatusBarColor(r, g, 0.0);
end

I'm concerned about the second function I've hooked. It gets called a lot (presumably once per frame for every UnitFrameHealthBar). Am I right to be?

Note: I originally wrote this using events but frames like focustarget and targettarget weren't getting updated because changes to those don't trigger UNIT_HEALTH events.

[Slakah]

My only major qualms are that you don't properly hook and if Blizzard were to change those function in anyway by adding extra arguments or using return values you could muck stuff up in a fairly major way.

Heres an quick example of how to make it more "future proof".

local _HealthBar_OnValueChanged = HealthBar_OnValueChanged
 
HealthBar_OnValueChanged = function(healthBar, value, bol, ...)
    return _HealthBar_OnValueChanged(healthBar, value, true, ...)
end
 
local _UnitFrameHealthBar_Update = UnitFrameHealthBar_Update
 
local function UpdateHelper(statusbar, unit, ...)
    if (not statusbar or statusbar.disconnected) then return ... end
 
    local min, max = statusbar:GetMinMaxValues();
    if (max <= min) then return end
 
    local value = statusbar:GetValue()
    if ( (value < min) or (value > max) ) then return ... end
 
    value = (value - min) / (max - min);
 
    local r, g
    if(value > 0.5) then
        r = (1.0 - value) * 2;
        g = 1.0;
    else
        r = 1.0;
        g = value * 2;
    end
    statusbar:SetStatusBarColor(r, g, 0.0);
    return ...
end
 
 
UnitFrameHealthBar_Update = function(statusbar, unit, ...)
    return UpdateHelper(statusbar, unit, _UnitFrameHealthBar_Update(statusbar, unit, ...))
end

[Aizhia]

Cool. That makes sense.

Many thanks.

[xConStruct]

Since you are not modifying the arguments of UnitFrameHealthBar_Update and your code is called after the original function, you could also just use hooksecurefunc and don't care about the whole argument-passing. This approach also avoids any possible tainting.

hooksecurefunc("UnitFrameHealthBar_Update", function(statusbar, unit)

	if (not statusbar or statusbar.disconnected) then return end

	local min, max = statusbar:GetMinMaxValues();
	if (max <= min) then return end

	local value = statusbar:GetValue()
	if ( (value < min) or (value > max) ) then return end

	value = (value - min) / (max - min);

	local r, g
	if(value > 0.5) then
		r = (1.0 - value) * 2;
		g = 1.0;
	else
		r = 1.0;
		g = value * 2;
	end
	statusbar:SetStatusBarColor(r, g, 0.0);
end)

[Aizhia]

Even better

I've incorporated both suggestions. Thanks.