checking for buff stacks in C_L_E_U

[ObbleYeah]

over the last couple of days me and Lyn have been trying to build a little mod that will display a set of text strings based on whether the player has particular buffs. So far we have this:

https://gist.github.com/anonymous/d23ffb8dfd3124659c46

which, despite obviously being pretty unfinished, is sort-of working. Getting how many stacks a buff has doesn't seem to be working despite best efforts though - the debug print only ever returning a maximum value of 1. Experiments with using UNIT_AURA instead provided the correct # of stacks, but we couldn't get the thing to work otherwise. Doing something wrong here?

[semlar]

Is there some reason you're trying to use the combat log instead of UNIT_AURA to track a buff on the player?

[ObbleYeah]

Well, tried this too and couldn't get anything at all (though it does pass stacks correctly):

https://gist.github.com/anonymous/55cbe42ae6a68b3de58b

if it's easier/better to use UNIT_AURA then great, just struggling to make it work atm

edit: actually it does seem to be working when brain freeze procs: but not for fingers of frost. hmn

[semlar]

You're calling self:Hide() for each item in your table, so the only possible time it would be visible is if you have the last buff in the table active.

[ObbleYeah]

neat, that gives me something to work with. Thanks!

[semlar]

A couple things I would change..

frame:RegisterEvent("UNIT_AURA")

would be better as

frame:RegisterUnitEvent("UNIT_AURA", "player")

and

if UnitBuff("player", buff) then    
    local _, _, _, stacks = UnitBuff("player", buff)

could be replaced with

local _, _, _, stacks = UnitBuff("player", buff)
if stacks then

[Phanx]

The (probable) reason CLEU didn't work is that UnitAura isn't necessarily updated yet when the aura change is reported in the combat log.

With your current code, I'd recommend using the spellID as the table key, not the name, so it can work in all languages and not just English:

local spells = {
    [1234] = {
        -- some other data here
    },
    [56789] = {
        -- some other data here
    },
}

-- add name for current locale at runtime:
for id, data in pairs(spells) do
     data.name = GetSpellInfo(id)
end

Then, when you're checking for buffs:

for id, data in pairs(spells) do
    local _, _, _, stacks = UnitBuff("player", data.name)
    if stacks then
        -- do stuff here
    end
end

Also, don't forget that stacks will be 0 for buffs that don't stack -- if you need to support those, then you'll want to do an additional "if stacks == 0 then stacks = 1 end" in the "do stuff here" area.

Oh, and there's not really any reason to use unpack here, since you know how many values are in the table. This:

local t = { 128, 128, 255 }
local r, g, b = t[1], t[2], t[3]

... is much faster than this:

local t = { 128, 128, 255 }
local r, g, b = unpack(t)

Another problem:

               for i = 1, stacks do
                    string:SetText(spellList[buff].Text)
                end

1) string is a horrible name for a variable, since it will overwrite the global string library in that context. Similarly, you should never name a variable table or math or setmetatable or any other Lua or WoW API global.

2) I'm guessing you want to show the given character n times, where n is the number of stacks of the buff, but your current code won't do that; it will just tell the fontstring n times to display one of the character. You probably want to do this instead:

    myFontString:SetText(string.rep("x", stacks)

Finally, your current method of checking if Tier is 1 or 2 will lead to problems later on if you try to support more spells, since you'll have to do a long else chain to do the positioning correctly. If you want to show the buffs in a specific order, I'd suggest using an indexed table instead of a hash table, so you can use table.sort on it:

local spells = {
    {
        id = 1234,
        priority = 1,
        -- other data here
    },
    {
        id = 56789,
        priority = 2,
        -- other data here
    },
}
for i = 1, #spells do
    local spell = spells[i]
    spell.name = GetSpellInfo(spell.id)
end

local function sortSpells(a, b)
     -- Show lower priority first, or alphabetically within same priority:
     return a.priority < b.priority or a.priority == b.priority and a.name < b.name
end

-- you can call this again later if you change priorities in-game:
table.sort(spells, sortSpells)

Then:

local shown = 0
for i = 1, #spells do
    local spell = spells[i]
    local _, _, _, stacks = UnitBuff("player", spell.name)
    if stacks then
        shown = shown + 1
        local fontstring = fontstrings[shown] -- keep them all in another indexed table
        fontstring:SetText(string.rep(spell.text, stacks > 0 and stacks or 1))
        fontstring:SetTextColor(spell.color[1], spell.color[2], spell.color[3])
        if shown > 1 then
            fontstring:SetPoint("TOP", fontstrings[shown - 1], "BOTTOM", 0, -5)
        else
            fontstring:SetPoint("TOP", UIParent, "CENTER")
        end
        fontstring:Show()
    end
    -- hide the unused ones:
    for i = shown + 1, #fontstrings do
        fontstrings[i]:Hide()
    end
end

This will ensure that your spells are always checked and displayed in order without needing to hardcode any checks.

[ObbleYeah]

Originally Posted by semlar A couple things I would change.. frame:RegisterEvent("UNIT_AURA") would be better as frame:RegisterUnitEvent("UNIT_AURA", "player")

does this mean the removal of

if unit~="player" then return end

is then possible at the start of the onevent, or is it just for general increased precision so that line doesn't have to keep running on every aura event? Considering recent troubles I've had whittling events down to just the players actions I'm assuming the latter.

Also, how would you suggest hiding the frame once the buff is no longer around? I guess that

for buff in pairs(spellList) do
    local _, _, _, stacks = UnitBuff("player", buff)
    if stacks then
        -- stuff
        self:Show()
    else
        self:Hide()
    end
end

would cause similar issues (not in an environment to just test at the minute).

[semlar]

Originally Posted by ObbleYeah does this mean the removal of if unit~="player" then return end

Yeah, it will only fire OnEvent for UNIT_AURA on the player. You won't need to check what unit it fired on.

As for hiding the frame, you could hide it before the buff loop and just show it when one is found.

[ObbleYeah]

makes sense, thanks.

[Phanx]

Edited my post while you guys were posting, in case you didn't notice the additions.

[ObbleYeah]

Well that certainly cuts down the amount of time i was expecting for this to eat into my work-day tomorrow. What am i supposed to do now?

Hugely appreciated.

[ObbleYeah]

Is there any way to get these strings to align (i'm already using SetJustifyH and SetPoint to anchor and align) or is this kind of thing just the result of quirks in each typeface symbol + different font sizes?



edit: more noticable in this image maybe

[Lombra]

Which strings?

[ObbleYeah]

the bullet point, asterisk and em dash in the center.

e: it doesn't matter anyway. I'm happy with them right-aligned now

[Lombra]

If they're anchored to each other's center and center justified there's not much more you can do, no.

[semlar]

Originally Posted by ObbleYeah Is there any way to get these strings to align

Assuming you're using phanx's script, this is what is anchoring your strings

if shown > 1 then
    fontstring:SetPoint("TOP", fontstrings[shown - 1], "BOTTOM", 0, -5)
else
    fontstring:SetPoint("TOP", UIParent, "CENTER")
end

The second SetPoint anchors the initial font string to the UIParent and the first SetPoint anchors the top of the following strings to the bottom of the previous one. To left align them you would just change their anchor points to "TOPLEFT" and "BOTTOMLEFT" and then anchor the initial string wherever you want.

if shown > 1 then
    fontstring:SetPoint("TOPLEFT", fontstrings[shown - 1], "BOTTOMLEFT", 0, -5)
else
    fontstring:SetPoint("TOPLEFT", UIParent, "CENTER")
end

This would make the list appear left-aligned with the center of the UIParent.

If you don't keep the relative string anchors the same (they're all "TOPLEFT" in this) then you'll have to call fontstring:ClearAllPoints() first.

[Phanx]

Actually, it occurred to me that you don't need to do any SetPoint work in the update routine -- you only need to set the point of each font string once, when it's created.

local lines = setmetatable({}, { __index = function(t, i)
    local line = UIParent:CreateFontString(nil, "OVERLAY", "GameFontNormal")
    if i > 1 then
        line:SetPoint("BOTTOM", lines[i-1], "TOP")
    else
        line:SetPoint("BOTTOM", UIParent, "CENTER")
    end
    -- do other stuff here, add shadow, outline, custom font, whatever
    t[i] = line
    return line
end })

All you need to do in the update routine is either SetText+Show, or Hide.

[ObbleYeah]

what do I use as a variable in the event handler for the string i've created using that method?

[Phanx]

local fontstring = fontstrings[shown] => local fontstring = lines[shown]

or just rename "lines" to "fontstrings"

Also, it's a metatable, so you just look up the index you want, and the metatable automatically creates a fontstring object at that index; you don't have to "create" anything by hand.