Ovale Spell Priority

Ovale Spell Priority

6M Downloads

Cooldown Conditions not working

LunaEclipse73 opened this issue ยท 3 comments

commented

I was having trouble with my custom scripts, recommending wrong spells and such, I eventually narrowed it down to SpellCooldown() not returning correctly, this could possible apply to others such as SpellChargeCooldown as well.

Simple way to confirm this bug, I am using an unholy DK for this, but you can quite easily modify it for any class with a cooldown spell:

AddIcon specialization=unholy
{
    if SpellCooldown(death_and_decay) Spell(death_and_decay text=true)
    if not SpellCooldown(death_and_decay) Spell(death_and_decay text=false)
}

This basically adds an extra icon which is a visual display of whether the condition is returning true or false. If the spell is on cooldown it should show the icon with the text true in the center of the icon, if it is not on cooldown it should show the icon with the text false in the center.

Use Death and Decay or whichever spell you use for the test, and the box should change the text, however it never does, it always shows "False" meaning the functions are returning that spells are never on cooldown. I tested this with several DK cooldown spells, so it definitely seems to be a problem with the condition itself.

commented

I was able to get it to return the correct values by changing the condition as follows, though I don't know enough about the Ovale:Future module to know if this will interfere with future best spell suggestion.

do
    --- Get the number of seconds before any of the listed spells are ready for use.
    -- @name SpellCooldown
    -- @paramsig number or boolean
    -- @param id The spell ID.
    -- @param ... Optional. Additional spell IDs.
    -- @param operator Optional. Comparison operator: less, atMost, equal, atLeast, more.
    -- @param number Optional. The number to compare against.
    -- @return The number of seconds.
    -- @return A boolean value for the result of the comparison.
    local function SpellCooldown(positionalParams, namedParams, state, atTime)
        local comparator, limit;
        local usable = (namedParams.usable == 1);
        local target = ParseCondition(positionalParams, namedParams, state, "target");
        local returnValue = 0;

        for counter, spellID in ipairs(positionalParams) do
            if OvaleCondition.COMPARATOR[spellID] then
                comparator, limit = spellID, positionalParams[counter + 1];
                break;
            elseif not usable or state:IsUsableSpell(spellID, atTime, OvaleGUID:UnitGUID(target)) then
                local spellStart, spellDuration = state:GetSpellCooldown(spellID);
                local newReturnValue = (spellStart + spellDuration) - GetTime();

                if newReturnValue < returnValue or returnValue == 0 then
                    returnValue = newReturnValue;
                end
            end     
        end

        if returnValue < 0 then
            returnValue = 0;
        end

        return Compare(returnValue, comparator, limit);
    end

    OvaleCondition:RegisterCondition("spellcooldown", true, SpellCooldown)
end

To confirm that my new code works I changed the test icon code to the following:

AddIcon specialization=unholy { 
    if SpellCooldown(death_and_decay) >= 25 Spell(death_and_decay text=25)
    if SpellCooldown(death_and_decay) >= 20 Spell(death_and_decay text=20)
    if SpellCooldown(death_and_decay) >= 15 Spell(death_and_decay text=15)
    if SpellCooldown(death_and_decay) >= 10 Spell(death_and_decay text=10)
    if SpellCooldown(death_and_decay) >= 5 Spell(death_and_decay text=5)
    if SpellCooldown(death_and_decay) >= 1 Spell(death_and_decay text=1)

    if not SpellCooldown(death_and_decay) Spell(death_and_decay text=false)
}

This not only confirms the True/False part works, but also confirms that its returning a valid time remaining in seconds.

commented

Your fix breaks how Ovale works. It all works on time spans.

As far as Ovale is concerned, the first line can never be true. Spells have their own checks to know whether or not they'll be castable and when (from cooldown, resources or other things). So if you cast DnD at time 0, your code does the following (ignoring other constraints such as resources):

SpellCooldown(death_and_decay) = true from 0-30, false after 30
Spell(death_and_decay text=true) = false from 0-30, true after 30
First if evaluates can never be cast because those time spans are opposite one another.

not SpellCooldown(death_and_decay) = false from 0-30, true after 30
Spell(death_and_decay text=false) = false from 0-30, true after 30
Second if evaluates to true at time 30.

Now since Ovale shows the next upcoming spell, and the text=false DnD is the only valid spell, it'll show DnD as being ready at time 30. If you change the spell being cast to a different spell...

AddIcon specialization=unholy
{
    if SpellCooldown(death_and_decay) Spell(raise_dead text=true)
    if not SpellCooldown(death_and_decay) Spell(raise_dead text=false)
}

... you'll notice that the true and false thing works as you're expecting it (true when DnD is on cooldown, false when it's off cooldown). Usually, when testing to see if something is resulting in what I think it is, I'll just do:

AddIcon
{
    if SpellCooldown(death_and_decay) 1 0
}

That will display a 1 if the condition (in this case SpellCooldown(death_and_decay)) is true, 0 if it's false.

Hopefully this'll help you figure out what's going wrong in your scripts.

commented

I had been away for a long time, and came back and am writing my scripts, I realize what the problem was, before when I used to check like that I used a Texture, much like the not in melee combat, not that actual spell.

I am closing this issue, it is working correctly after all.