WoWInterface
Page 1 of 2 1 2 >
Show 60 post(s) from this thread on one page

WoWInterface (https://www.wowinterface.com/forums/index.php)
-   Lua/XML Help (https://www.wowinterface.com/forums/forumdisplay.php?f=16)
-   -   Misc issues with an addon (https://www.wowinterface.com/forums/showthread.php?t=48956)

Caellian 02-19-14 05:32 AM
Misc issues with an addon
 
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:

Code:
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 02-19-14 06:14 AM
For paladin, compare the new amount of combo points to the previous amount. Only beep if the new amount is greater.

Oppugno 02-19-14 07:36 AM
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 02-19-14 07:47 AM
Quote:
Originally Posted by Oppugno (Post 290893)
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 02-19-14 08:50 AM
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:

Lua Code:
  1. local numPrevious = 0
  2.  
  3. caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
  4.     ...
  5.     if event == "UNIT_POWER" then
  6.         ...
  7.         elseif caelUI.playerClass == "WARLOCK" then
  8.             if spec then
  9.                 if spec == SPEC_WARLOCK_AFFLICTION then
  10.                     num = UnitPower("player", SPELL_POWER_SOUL_SHARDS)
  11.                     numMax = UnitPowerMax("player", SPELL_POWER_SOUL_SHARDS)
  12.                 elseif spec == SPEC_WARLOCK_DEMONOLOGY then
  13.                     num = math.floor(UnitPower("player", SPELL_POWER_DEMONIC_FURY) / 250)
  14.                     numMax = UnitPowerMax("player", SPELL_POWER_DEMONIC_FURY) / 250
  15.                 elseif spec == SPEC_WARLOCK_DESTRUCTION then
  16.                     num = UnitPower("player", SPELL_POWER_BURNING_EMBERS)
  17.                     numMax = UnitPowerMax("player", SPELL_POWER_BURNING_EMBERS)
  18.                 end         
  19.             end
  20.         end
  21.  
  22.         if num > numPrevious then
  23.             if num ~= 0 and num ~= numMax then
  24.                 PlaySoundFile(caelMedia.files.soundCombo, "Master")
  25.             elseif num == numMax then
  26.                 PlaySoundFile(caelMedia.files.soundComboMax, "Master")
  27.             end
  28.         end
  29.         numPrevious = num
  30.     end
  31. end

Caellian 02-19-14 08:55 AM
There we go, i wasn't resetting "numPrevious"...

Now let's try.

Oppugno 02-19-14 09:19 AM
It seems UnitPower("player", SPELL_POWER_BURNING_EMBERS) returns a count of only full embers not total ember bits. Updated code.

Caellian 02-19-14 09:24 AM
Well, so far i've tried paladins and all 3 warlock specs and those work great, thank you !

Phanx 02-19-14 09:26 AM
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:

Lua Code:
  1. caelUI.powersound = caelUI.createModule("PowerSound")
  2.  
  3. -- Use RegisterUnitEvent, not RegisterEvent, to filter out irrelevant events.
  4. caelUI.powersound:RegisterUnitEvent("UNIT_COMBO_POINTS", "player")
  5. caelUI.powersound:RegisterUnitEvent("UNIT_POWER", "player")
  6.  
  7. -- Look these things up once at load; they're not going to change mid-session.
  8. local POWER_TYPE, POWER_DIVISOR
  9. if caelUI.playerClass == "MONK" then
  10.     POWER_TYPE = "CHI"
  11. elseif caelUI.playerClass == "PALADIN" then
  12.     POWER_TYPE = "HOLY_POWER"
  13. elseif caelUI.playerClass == "PRIEST" then
  14.     POWER_TYPE = "SHADOW_ORBS"
  15.     MAX_POWER = PRIEST_BAR_NUM_ORBS
  16. elseif caelUI.playerClass == "WARLOCK" then
  17.     -- Only register these events on a class that needs it.
  18.     caelUI.powersound:RegisterEvent("PLAYER_LOGIN")
  19.     caelUI.powersound:RegisterEvent("PLAYER_SPECIALIZATION_CHANGED")
  20. end
  21.  
  22. local lastCount = 0
  23. caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
  24.     -- This will quickly get out of irrelevant power updates, which are the most frequent case.
  25.     if event == "UNIT_POWER" and powerType ~= POWER_TYPE then return end
  26.  
  27.     -- This is the least frequent case, but the other events share code, so check for this one first.
  28.     if event == "PLAYER_SPECIALIZATION_CHANGED" then
  29.         local spec = GetSpecialization()
  30.         if spec == SPEC_WARLOCK_AFFLICTION then
  31.             POWER_TYPE = "SOUL_SHARDS"
  32.             POWER_DIVISOR = 1
  33.         elseif spec == SPEC_WARLOCK_DEMONOLOGY then
  34.             POWER_TYPE = "DEMONIC_FURY"
  35.             POWER_DIVISOR = 250
  36.         elseif spec == SPEC_WARLOCK_DESTRUCTION then
  37.             POWER_TYPE = "BURNING_EMBERS"
  38.             POWER_DIVISOR = 10
  39.         end
  40.         return
  41.     end
  42.  
  43.     local count, maxCount
  44.     if event == "UNIT_COMBO_POINTS" then
  45.         count = UnitComboPoints("player", "target")
  46.         maxCount = MAX_COMBO_POINTS
  47.     elseif POWER_DIVISOR then
  48.         -- Only use this code path for warlocks to avoid the 2 function calls if they're not needed.
  49.         count = floor(UnitPower(unit, powerType) / POWER_DIVISOR)
  50.         maxCount = MAX_POWER or floor(UnitPowerMax(unit, powerType) / POWER_DIVISOR)
  51.     else
  52.         count = UnitPower(unit, powerType)
  53.         maxCount = MAX_POWER or UnitPowerMax(unit, powerType)
  54.     end
  55.  
  56.     if count > lastCount then
  57.         -- count can't be < 0, so neither can lastCount, and 0 is not > 0, so no need to check count > 0
  58.         if count == maxCount then
  59.             -- Check this first so you can just "else" straight into the other thing.
  60.             PlaySoundFile(caelMedia.files.soundComboMax, "Master")
  61.         else
  62.             PlaySoundFile(caelMedia.files.soundCombo, "Master")
  63.         end
  64.     end
  65.  
  66.     lastCount = count
  67. end)

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

Caellian 02-19-14 09:36 AM
Phanx, i only copy pasted but yours doesn't seems to work :( as in, no error but doesn't do a thing.

Edit: That's because you use "SPELL_POWER_HOLY_POWER" which returns 9 instead of "HOLY_POWER"

Hmm, even like that, there are other issues.

Phanx 02-19-14 01:11 PM
Fixed the power types, but unless you can be more specific than "it has issues" there's not much I can do (other than leveling each class to the appropriate level to get the secondary resource and testing it myself, which I'm certainly not going to do)....

Caellian 02-19-14 01:13 PM
Oh no don't worry i understand, unfortunately i can't be more specific right now, i don't have a warlock and i have to log a friend's account but i can't now because he's playing it.

I'll get back to you with more details as soon as i can log it in.

Only one thing i can see right now is, you're only defining the warlock spec on "PLAYER_SPECIALIZATION_CHANGED" and not on PEW but as i said i can't test anything.

Phanx 02-19-14 01:51 PM
PLAYER_SPEC should fire shortly after login; there shouldn't be any reason to listen to PEW.

Caellian 02-19-14 01:53 PM
Quote:
Originally Posted by Phanx (Post 290917)
PLAYER_SPEC should fire shortly after login; there shouldn't be any reason to listen to PEW.
Ah, i didn't know that, anyway, i'll see by myself tomorrow and get back to you :)

Phanx 02-19-14 03:37 PM
Actually I just checked; PLAYER_SPEC only fires on login, not on reload (though on reload the spec is available immediately). So, I'd just register PLAYER_LOGIN for warlocks too. I've updated the code in my previous post again.

Caellian 02-19-14 03:50 PM
Well, something is wrong, at least on the paladin, the beeps come randomly, sometimes they do, sometimes they don't and i can't find any pattern. Also, it beeps when i use the holy power points which it shouldn't since it's a decrease and not an increase.

That's what i've noticed after a quick test.

Rainrider 02-19-14 04:26 PM
Just a note for warlock:

Those magic 10 are contained in the global MAX_POWER_PER_EMBER.
Code:
UnitPower("player", SPELL_POWER_BURNING_EMBERS)
is the same as
Code:
math.floor(UnitPower("player", SPELL_POWER_BURNING_EMBERS, true) / MAX_POWER_PER_EMBER)
The same goes for UnitPowerMax. As you only want a sound on gaining a full ember and use the first, just skip the POWER_DIVISOR code at all. It also does not make much sense for demonology either, as the abilities you use in metamorphosis have different demonic fury cost and the spells when not in metamorphosis generate different amounts of it too, so that your play style does not depend on demonic fury being gained in chunks of a set amount. You can also spend less than full embers with the T15 2-pieces bonus when Dark Soul is active, which would make your sound based implementation sub-optimal in the this scenario.

Caellian 02-19-14 06:16 PM
Phanx, i believe i have it working for everyone with your version, let me know what you think:

Code:
local _, caelUI = ...

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

local floor, numPrevious = math.floor, 0

local POWER_TYPE, SPELL_POWER_TYPE

if caelUI.playerClass == "MONK" then
        POWER_TYPE = "CHI"
        SPELL_POWER_TYPE = SPELL_POWER_CHI
elseif caelUI.playerClass == "PALADIN" then
        POWER_TYPE = "HOLY_POWER"
        SPELL_POWER_TYPE = SPELL_POWER_HOLY_POWER
elseif caelUI.playerClass == "PRIEST" then
        POWER_TYPE = "SHADOW_ORBS"
        SPELL_POWER_TYPE = SPELL_POWER_SHADOW_ORBS
        MAX_POWER = PRIEST_BAR_NUM_ORBS
elseif caelUI.playerClass == "WARLOCK" then
        caelUI.powersound:RegisterEvent("PLAYER_LOGIN")
        caelUI.powersound:RegisterEvent("PLAYER_SPECIALIZATION_CHANGED")
end

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

        if event == "PLAYER_LOGIN" or event == "PLAYER_SPECIALIZATION_CHANGED" then
                local spec = GetSpecialization()

                if spec == SPEC_WARLOCK_AFFLICTION then
                        POWER_TYPE = "SOUL_SHARDS"
                        SPELL_POWER_TYPE = SPELL_POWER_SOUL_SHARDS
--                elseif spec == SPEC_WARLOCK_DEMONOLOGY then
--                        POWER_TYPE = "DEMONIC_FURY"
--                        SPELL_POWER_TYPE = SPELL_POWER_DEMONIC_FURY
                elseif spec == SPEC_WARLOCK_DESTRUCTION then
                        POWER_TYPE = "BURNING_EMBERS"
                        SPELL_POWER_TYPE = SPELL_POWER_BURNING_EMBERS
                end

                return
        end

        local num, numMax

        if event == "UNIT_COMBO_POINTS" then
                num = GetComboPoints("player", "target")
                numMax = MAX_COMBO_POINTS
        else
                num = UnitPower(unit, SPELL_POWER_TYPE)
                numMax = MAX_POWER or UnitPowerMax(unit, SPELL_POWER_TYPE)
        end

        if num > numPrevious then
                if num == numMax then
                        PlaySoundFile(caelMedia.files.soundComboMax, "Master")
                else
                        PlaySoundFile(caelMedia.files.soundCombo, "Master")
                end
        end

        numPrevious = num
end)

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

Phanx 02-19-14 07:01 PM
First and foremost, stop doing this:
Code:
for _, event in next, {
        "UNIT_COMBO_POINTS",
        "UNIT_POWER",
} do
        caelUI.powersound:RegisterUnitEvent(event, "player")
end
There is absolutely no benefit to registering events this way. You're just wasting memory on a table and CPU on a loop. If you were dynamically altering the list of events to register this might make sense, but for a fixed list of 2 events, just do it normally:
Code:
caelUI.powersound:RegisterUnitEvent("UNIT_COMBO_POINTS", "player")
caelUI.powersound:RegisterUnitEvent("UNIT_POWER", "player")
Other than that, my only complaints are stylistic:
(1) Rather than POWER_TYPE and SPELL_POWER_TYPE, I'd use variable names that more clearly identified what they are, such as POWER_TYPE_NAME and POWER_TYPE_ID.
(2) Mixing upvalues and local variables like this seems sloppy:
Code:
local floor, numPrevious = math.floor, 0
Plus you missed a few upvalues:
Code:
local floor, UnitPower, UnitPowerMax = math.floor, UnitPower, UnitPowerMax
local numPrevious = 0
Actually there is an actual problem; you forgot to define MAX_POWER as a local variable at the top there, so you're leaking a global. :(

Lua Code:
  1. local _, caelUI = ...
  2.  
  3. -- Upvalues
  4. local floor, UnitPower, UnitPowerMax = math.floor, UnitPower, UnitPowerMax
  5.  
  6. -- Variables
  7. local numPrevious = 0
  8. local POWER_TYPE_NAME, POWER_TYPE_ID, MAX_POWER
  9.  
  10. -- Create frame and register global events
  11. caelUI.powersound = caelUI.createModule("PowerSound")
  12. caelUI.powersound:RegisterUnitEvent("UNIT_COMBO_POINTS", "player")
  13. caelUI.powersound:RegisterUnitEvent("UNIT_POWER", "player")
  14.  
  15. -- Define class variables and register class events
  16. if caelUI.playerClass == "MONK" then
  17.     POWER_TYPE_NAME, POWER_TYPE_ID = "CHI", SPELL_POWER_CHI
  18. elseif caelUI.playerClass == "PALADIN" then
  19.     POWER_TYPE_NAME, POWER_TYPE_ID = "HOLY_POWER", SPELL_POWER_HOLY_POWER
  20. elseif caelUI.playerClass == "PRIEST" then
  21.     POWER_TYPE_NAME, POWER_TYPE_ID, MAX_POWER = "SHADOW_ORBS", SPELL_POWER_SHADOW_ORBS, PRIEST_BAR_NUM_ORBS
  22. elseif caelUI.playerClass == "WARLOCK" then
  23.     caelUI.powersound:RegisterEvent("PLAYER_LOGIN")
  24.     caelUI.powersound:RegisterEvent("PLAYER_SPECIALIZATION_CHANGED")
  25. end
  26.  
  27. -- Many events, all sides, handle it!
  28. caelUI.powersound:SetScript("OnEvent", function(self, event, unit, powerType)
  29.     if event == "UNIT_POWER" and powerType ~= POWER_TYPE_NAME then return end
  30.  
  31.     if event == "PLAYER_LOGIN" or event == "PLAYER_SPECIALIZATION_CHANGED" then
  32.         local spec = GetSpecialization()
  33.         if spec == SPEC_WARLOCK_AFFLICTION then
  34.             POWER_TYPE_NAME, POWER_TYPE_ID = "SOUL_SHARDS", SPELL_POWER_SOUL_SHARDS
  35. --      elseif spec == SPEC_WARLOCK_DEMONOLOGY then
  36. --          POWER_TYPE_NAME, POWER_TYPE_ID = "DEMONIC_FURY", SPELL_POWER_DEMONIC_FURY
  37.         elseif spec == SPEC_WARLOCK_DESTRUCTION then
  38.             POWER_TYPE_NAME, POWER_TYPE_ID = "BURNING_EMBERS", SPELL_POWER_BURNING_EMBERS
  39.         end
  40.         return
  41.     end
  42.  
  43.     local num, numMax
  44.     if event == "UNIT_COMBO_POINTS" then
  45.         num = GetComboPoints("player", "target")
  46.         numMax = MAX_COMBO_POINTS
  47.     else
  48.         num = UnitPower(unit, POWER_TYPE_ID)
  49.         numMax = MAX_POWER or UnitPowerMax(unit, POWER_TYPE_ID)
  50.     end
  51.  
  52.     if num > numPrevious then
  53.         if num == numMax then
  54.             PlaySoundFile(caelMedia.files.soundComboMax, "Master")
  55.         else
  56.             PlaySoundFile(caelMedia.files.soundCombo, "Master")
  57.         end
  58.     end
  59.  
  60.     numPrevious = num
  61. end)

Rainrider 02-20-14 09:00 AM
You also don't have a local reference to MAX_COMBO_POINTS. You don't check for vehicle combo points either (there was a daily rep quest back in WotLK that used that, maybe there are others too). You could also set MAX_POWER to 4 for warlocks as long you are not using this for demonology and spare a function call in that case.


All times are GMT -6. The time now is 09:25 AM.
Page 1 of 2 1 2 >
Show 60 post(s) from this thread on one page

vBulletin © 2024, Jelsoft Enterprises Ltd
© 2004 - 2022 MMOUI