Misc issues with an addon

[Caellian]

Hi, so i've had this little addon issuing a sound for each combo point and then another sound to warn me when i reached 5 (some people use bars on their screen for that, i prefer sounds) but then i decided i wanted to level a paladin and thought, why not make this work for each classes that this kind of "points".

So i came up with this:

local _, caelUI = ...

caelUI.powersound = caelUI.createModule("PowerSound")

local num, numMax, power, spec

local powers = {
	["MONK"]		=	"CHI",
	["PALADIN"]	=	"HOLY_POWER",
	["PRIEST"]	=	"SHADOW_ORBS",
	["WARLOCK"] = {
		[1]		=	"SOUL_SHARDS",
		[2]		=	"DEMONIC_FURY",
		[3]		=	"BURNING_EMBERS",
	}
}

caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
	if unit ~= "player" then return end

	if event == "UNIT_COMBO_POINTS" then
		num = GetComboPoints("player", "target")
		numMax = MAX_COMBO_POINTS

		if num ~= 0 and num ~= numMax then
			PlaySoundFile(caelMedia.files.soundCombo, "Master")
		elseif num == numMax then
			PlaySoundFile(caelMedia.files.soundComboMax, "Master")
		end
	end

	if event == "UNIT_POWER" then
		spec = GetSpecialization()

		if powerType ~= powers[caelUI.playerClass] and powerType ~= powers[caelUI.playerClass][spec] then return end

		if caelUI.playerClass == "MONK" then
			num = UnitPower("player", SPELL_POWER_CHI)
			numMax = UnitPowerMax("player", SPELL_POWER_CHI)
		elseif caelUI.playerClass == "PALADIN" then
			num = UnitPower("player", SPELL_POWER_HOLY_POWER)
			numMax = UnitPowerMax("player", SPELL_POWER_HOLY_POWER)
		elseif caelUI.playerClass == "PRIEST" then
			num = UnitPower("player", SPELL_POWER_SHADOW_ORBS)
			numMax = PRIEST_BAR_NUM_ORBS -- UnitPowerMax("player", SPELL_POWER_SHADOW_ORBS)
--[[
		elseif caelUI.playerClass == "WARLOCK" then
			if spec then
				if spec == SPEC_WARLOCK_AFFLICTION then
					num = UnitPower("player", SPELL_POWER_SOUL_SHARDS)
					numMax = UnitPowerMax("player", SPELL_POWER_SOUL_SHARDS)
				elseif spec == SPEC_WARLOCK_DEMONOLOGY then
					num = UnitPower("player", SPELL_POWER_DEMONIC_FURY)
					numMax = UnitPowerMax("player", SPELL_POWER_DEMONIC_FURY)
				elseif spec == SPEC_WARLOCK_DESTRUCTION then
					num = UnitPower("player", SPELL_POWER_BURNING_EMBERS)
					numMax = UnitPowerMax("player", SPELL_POWER_BURNING_EMBERS)
				end
			end
--]]
		end

		if num ~= 0 and num ~= numMax then
			PlaySoundFile(caelMedia.files.soundCombo, "Master")
		elseif num == numMax then
			PlaySoundFile(caelMedia.files.soundComboMax, "Master")
		end
	end
end)

for _, event in next, {
	"UNIT_COMBO_POINTS",
	"UNIT_POWER",
} do
	caelUI.powersound:RegisterEvent(event)
end

Obviously, for combo points it works perfectly, but for other classes not so much.

The issues (that i need help with) are the following:

Paladin: Over time, the Holy power decreases and the problem is that each time i loose one point, it beeps, it shouldn't.
Warlock: This one is the worst, different power source for each spec, the power does decrease over time, burning embers seems to beep for every spell i cast, not just when i gain one ember, in fewer words, warlocks are a mess.

Anyone would have any idea how i could improve this little addon ?

[Haleth]

For paladin, compare the new amount of combo points to the previous amount. Only beep if the new amount is greater.

[Oppugno]

As Haleth suggested the same can be done for warlocks with a slight adjustment. The only difference being that Warlock resources (possibly with the exception of Soul Shards) are generated fractionally. It takes 10 Ember Bits to fill a Burning Ember and 1000 Demonic Fury to fill the bar (I've no idea about Soul Shards as I've never played Affliction).

Dividing by 10 on Burning Ember power will tell you how many Burning Embers you have. I don't know how you'd want to handle Demonic Fury as that's not really a combo point system (perhaps every 250?).

Although you asked for improvements I'm not sure you meant efficiency/superfluous code issues. There are a few but they could just be individual to me.. e.g. using RegisterEvent() instead of RegisterUnitEvent(), registering events for classes that don't need them, UnitMaxPower() variants on each UNIT_POWER even though most of them are constant (and those that aren't fire an event for it), etc.

[Caellian]

Originally Posted by Oppugno As Haleth suggested the same can be done for warlocks with a slight adjustment. The only difference being that Warlock resources (possibly with the exception of Soul Shards) are generated fractionally. It takes 10 Ember Bits to fill a Burning Ember and 1000 Demonic Fury to fill the bar (I've no idea about Soul Shards as I've never played Affliction). Using a modulus of 10 on Burning Ember power will tell you how many Burning Embers you have. I don't know how you'd want to handle Demonic Fury as that's not really a combo point system (perhaps every 250?).

I know all that, the issue is to write it, or else i wouldn't ask here

[Oppugno]

I didn't realise you wanted a solution and not a hint (which is a good thing as I meant divide by 10 not modulo by it).

With changing as little as possible:

local numPrevious = 0
 
caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
    ...
    if event == "UNIT_POWER" then
        ...
        elseif caelUI.playerClass == "WARLOCK" then
            if spec then
                if spec == SPEC_WARLOCK_AFFLICTION then
                    num = UnitPower("player", SPELL_POWER_SOUL_SHARDS)
                    numMax = UnitPowerMax("player", SPELL_POWER_SOUL_SHARDS)
                elseif spec == SPEC_WARLOCK_DEMONOLOGY then
                    num = math.floor(UnitPower("player", SPELL_POWER_DEMONIC_FURY) / 250)
                    numMax = UnitPowerMax("player", SPELL_POWER_DEMONIC_FURY) / 250
                elseif spec == SPEC_WARLOCK_DESTRUCTION then
                    num = UnitPower("player", SPELL_POWER_BURNING_EMBERS)
                    numMax = UnitPowerMax("player", SPELL_POWER_BURNING_EMBERS)
                end         
            end
        end
 
        if num > numPrevious then
            if num ~= 0 and num ~= numMax then
                PlaySoundFile(caelMedia.files.soundCombo, "Master")
            elseif num == numMax then
                PlaySoundFile(caelMedia.files.soundComboMax, "Master")
            end
        end
        numPrevious = num
    end
end

[Caellian]

There we go, i wasn't resetting "numPrevious"...

Now let's try.

[Oppugno]

It seems UnitPower("player", SPELL_POWER_BURNING_EMBERS) returns a count of only full embers not total ember bits. Updated code.

[Phanx]

In the interests of reducing code duplication, and reducing the number of function calls and global lookups (checking the class and spec on every power update is quite wasteful) I would rewrite that like so:

caelUI.powersound = caelUI.createModule("PowerSound")
 
-- Use RegisterUnitEvent, not RegisterEvent, to filter out irrelevant events.
caelUI.powersound:RegisterUnitEvent("UNIT_COMBO_POINTS", "player")
caelUI.powersound:RegisterUnitEvent("UNIT_POWER", "player")
 
-- Look these things up once at load; they're not going to change mid-session.
local POWER_TYPE, POWER_DIVISOR
if caelUI.playerClass == "MONK" then
    POWER_TYPE = "CHI"
elseif caelUI.playerClass == "PALADIN" then
    POWER_TYPE = "HOLY_POWER"
elseif caelUI.playerClass == "PRIEST" then
    POWER_TYPE = "SHADOW_ORBS"
    MAX_POWER = PRIEST_BAR_NUM_ORBS
elseif caelUI.playerClass == "WARLOCK" then
    -- Only register these events on a class that needs it.
    caelUI.powersound:RegisterEvent("PLAYER_LOGIN")
    caelUI.powersound:RegisterEvent("PLAYER_SPECIALIZATION_CHANGED")
end
 
local lastCount = 0
caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
    -- This will quickly get out of irrelevant power updates, which are the most frequent case.
    if event == "UNIT_POWER" and powerType ~= POWER_TYPE then return end
 
    -- This is the least frequent case, but the other events share code, so check for this one first.
    if event == "PLAYER_SPECIALIZATION_CHANGED" then
        local spec = GetSpecialization()
        if spec == SPEC_WARLOCK_AFFLICTION then
            POWER_TYPE = "SOUL_SHARDS"
            POWER_DIVISOR = 1
        elseif spec == SPEC_WARLOCK_DEMONOLOGY then
            POWER_TYPE = "DEMONIC_FURY"
            POWER_DIVISOR = 250
        elseif spec == SPEC_WARLOCK_DESTRUCTION then
            POWER_TYPE = "BURNING_EMBERS"
            POWER_DIVISOR = 10
        end
        return
    end
 
    local count, maxCount
    if event == "UNIT_COMBO_POINTS" then
        count = UnitComboPoints("player", "target")
        maxCount = MAX_COMBO_POINTS
    elseif POWER_DIVISOR then
        -- Only use this code path for warlocks to avoid the 2 function calls if they're not needed.
        count = floor(UnitPower(unit, powerType) / POWER_DIVISOR)
        maxCount = MAX_POWER or floor(UnitPowerMax(unit, powerType) / POWER_DIVISOR)
    else
        count = UnitPower(unit, powerType)
        maxCount = MAX_POWER or UnitPowerMax(unit, powerType)
    end
 
    if count > lastCount then
        -- count can't be < 0, so neither can lastCount, and 0 is not > 0, so no need to check count > 0
        if count == maxCount then
            -- Check this first so you can just "else" straight into the other thing.
            PlaySoundFile(caelMedia.files.soundComboMax, "Master")
        else
            PlaySoundFile(caelMedia.files.soundCombo, "Master")
        end
    end
 
    lastCount = count
end)

Also you may want to upvalue UnitPower, UnitPowerMax, and math.floor to speed things up.