My Balance Druid custom Rotation

Post by **Kaminaris** » 09 Sep 2018, 22:28

Nooo lol...

Caching result within local variable will not work. It will evaluate each time again.
Caching it in global variable will stay forever the same value.

Gadgetsan · Post by **Gadgetsan** » 09 Sep 2018, 23:34

it doesn't get evaluated when we don't need it cause the 'cache' function would then never get called

Post by **Kaminaris** » 10 Sep 2018, 03:30

I think you are misunderstanding how programming works:

Code: [Select all] [Expand/Collapse]

function rotation()
    local cache = {};
    function getSomething(what)
        if cache[what] then
            print('cache hit')
            return cache[what];
        end
 
        print('I am being called without cache');
        cache[what] = math.random(1,10);
    end
    -- do some other checks.
    getSomething('abc');
    getSomething('abc');
    getSomething('abc');
end
 
rotation();
rotation();
rotation();

GeSHi © Codebox Plus Extension

Is literally the same as

Code: [Select all] [Expand/Collapse]

function rotation()
    function getSomething(what)
 
        print('I am being called without cache');
        cache[what] = math.random(1,10);
    end
    -- do some other checks.
    local beforeFirstCheck = getSomething('abc');
    beforeFirstCheck;
    beforeFirstCheck;
end
 
rotation();
rotation();
rotation();

GeSHi © Codebox Plus Extension

and its even worse because you are creating a lot of functions on the fly.

I just made you some fixes:

Code: [Select all] [Expand/Collapse]

function(_, timeShift, currentSpell, gcd, talents)
    local DEBUG = false;
    --Links
    --http://maxdps.net/viewtopic.php?f=6&t=5&sid=134ea7dae4e3b8092032fb116836c364
 
    --SKILLS
    local _Moonfire = 8921;
    local _Sunfire = 93402;
    local _Starsurge = 78674;
    local _LunarEmpowerment = 164547;
    local _SolarEmpowerment = 164545;
    local _LunarStrike = 194153;
    local _SolarWrath = 190984;
    local _NewMoon = 274281;
    local _HalfMoon = 274282;
    local _FullMoon = 274283;
    local _CelestialAlignment = 194223;
    local _IncarnationChosenofElune = 102560;
    local _StellarFlare = 202347;
    local _Starfall = 191034;
    local _WarriorofElune = 202425;
    local _MoonkinMoonfire = 164812;
    local _MoonkinSunfire = 164815;
    local _WarriorofEluneAura = 202425;
 
    -------------------------------VARIABLES--------------------------------------------
    local AP = UnitPower('player', Enum.PowerType.LunarPower);
    --print(string.format("AstralPower: %s", AP)); -- dont define functions here unless you absolutely must
    --Note: we can cast 30% before the end because of pandemic dotting
    local targetHasMoonfireDot = MaxDps:TargetAura(_MoonkinMoonfire, timeShift + 5);
    local targetHasSunfireDot = MaxDps:TargetAura(_MoonkinSunfire, timeShift + 4);
    local targetHasStellarFlareDot = MaxDps:TargetAura(_StellarFlare, timeShift + 5);
 
    local solarE, solarCharges = MaxDps:Aura(_SolarEmpowerment, timeShift);
    local lunarE, lunarCharges = MaxDps:Aura(_LunarEmpowerment, timeShift);
 
    local currentMoon, nextMoon, cd, currentMoonCharges, maxCharges; -- no variable was in parent scope
    if talents[_NewMoon] then
        currentMoon = _NewMoon;
        if MaxDps:FindSpell(_NewMoon) ~= nil then
            currentMoon = _NewMoon;
        end
        if MaxDps:FindSpell(_HalfMoon) ~= nil then
            currentMoon = _HalfMoon;
        end
        if MaxDps:FindSpell(_FullMoon) ~= nil then
            currentMoon = _FullMoon;
        end
        --nextMoon is necessary in the case where we are currently casting
        nextMoon = currentMoon;
        -- horrible thing to do. You just created 4 global variables
        cd, currentMoonCharges, maxCharges = MaxDps:SpellCharges(currentMoon, timeShift);
    end
 
    --we look after the current casting to see in advance if we will have enough AP
    if currentSpell == _SolarWrath then
        AP = AP + 8;
        solarCharges = solarCharges - 1;
    elseif currentSpell == _LunarStrike then
        AP = AP + 12;
        lunarCharges = lunarCharges - 1;
    elseif currentSpell == _NewMoon then
        AP = AP + 10;
        currentMoonCharges = currentMoonCharges - 1;
        nextMoon = _HalfMoon
    elseif currentSpell == _HalfMoon then
        AP = AP + 20;
        currentMoonCharges = currentMoonCharges - 1;
        nextMoon = _FullMoon
    elseif currentSpell == _FullMoon then
        AP = AP + 40;
        currentMoonCharges = currentMoonCharges - 1;
        nextMoon = _NewMoon
    end
 
    --i once ended up with negative solar charges...
    if lunarCharges < 0 then
        lunarCharges = 0
    end
    if solarCharges < 0 then
        solarCharges = 0
    end
    --print(string.format("lunarCharges: %s solarCharges: %s ", lunarCharges, solarCharges));
 
 
    -------------------------------COOLDOWNS--------------------------------------------
    if talents[_IncarnationChosenofElune] then
        MaxDps:GlowCooldown(_IncarnationChosenofElune, MaxDps:SpellAvailable(_IncarnationChosenofElune, timeShift));
    else
        MaxDps:GlowCooldown(_CelestialAlignment, MaxDps:SpellAvailable(_CelestialAlignment, timeShift));
    end
 
    if talents[_WarriorofElune] then
        MaxDps:GlowCooldown(_WarriorofElune, MaxDps:SpellAvailable(_WarriorofElune, timeShift)
            and not MaxDps:Aura(_WarriorofEluneAura, timeShift));
    end
 
    -------------------------------PRIORITY--------------------------------------------
 
    --DOTS so they are always applied
    if not targetHasMoonfireDot then
        --print("Target does not have Moonfire!");
        return _Moonfire;
    end
 
    if not targetHasSunfireDot then
        --print("Target does not have Sunfire!");
        return _Sunfire;
    end
 
    --Stellar Flare (i don't use it...)
    if talents[_StellarFlare] and not targetHasStellarFlareDot and currentSpell ~= _StellarFlare then
        return _StellarFlare;
    end
 
 
    --New/Half/Full Moon, as suggested by HamOfMoose » 30 Aug 2018, 21:20. Requires New Moon Talent
    if talents[_NewMoon] then
        local isAvailable  = MaxDps:SpellAvailable(currentMoon, timeShift)
        --print(string.format("Found %s stakcs for moon thing", currentMoonCharges));
        if currentMoonCharges > 1 and isAvailable then
            --we look at the current moon (or the next if we are currently casting in order to decide if we cast it or not)
            if (nextMoon == _NewMoon and AP < 90) or (nextMoon == _HalfMoon and AP < 80) or (nextMoon == _FullMoon and AP < 60) then
                return currentMoon;
            end
        end
    end
 
    --if i have a lot of AP, i want to be casting Star Surge or Starfall in priority
    local targets;
    if AP > 80 then
        if targets == nil then
            targets = MaxDps:TargetsInRange(_LunarStrike);
        end
 
        if (targets > 3) then
            return _Starfall;
        else
            return _Starsurge;
        end
    end
 
 
    --Empowerments so i use spend them
    if (lunarCharges > 0 or solarCharges > 0) then
        if lunarCharges > solarCharges then
            return _LunarStrike
        else
            return _SolarWrath
        end
    end
 
    --StarSurge or StarFall to spend AP (LunarStrike and Starfall have same range)
    if targets == nil then
        targets = MaxDps:TargetsInRange(_LunarStrike);
    end
 
    if (targets > 3) then
        if AP > 50 then
            return _Starfall;
        end
    else
        if AP > 40 then
            return _Starsurge;
        end
    end
 
    --SLunarStrike to Get AP if more than 1 in range
    if (targets > 1) then
        return _LunarStrike;
    else
        return _SolarWrath;
    end
 
    return _LunarStrike;
end

GeSHi © Codebox Plus Extension

Post by **Kaminaris** » 10 Sep 2018, 03:45

If you wish to improve, general rules:
1. Dont make variables you are not using in code
2. Dont create functions unless you have to
3. If you wish to call function twice, just save result to variable
4. Don't ever make global variables inside it.

If you absolutely have to make function inside rotation function, do it like this:

Code: [Select all] [Expand/Collapse]

    local someFunction = _G.MyUniqueFunctionName;
    if not someFunction then
        someFunction = function(params)
            -- do something here
        end
        _G.MyUniqueFunctionName = someFunction;
    end

GeSHi © Codebox Plus Extension

Gadgetsan · Post by **Gadgetsan** » 10 Sep 2018, 20:34

you are mistaken in thinking the generalisation of a concept is not useful. Code reuse is an extremely important concept in software engineering which i apply daily.

the advantage of my caching function is that if i want to use targetsInRangeCache(_FullMoon) later in my code, i can still do it! i admit it's less useful with a couple dozens lines of code but it's a godsend when you have software spanning hundreds of thousand of lines of codes accross thousand files (which i program daily).

the usage of function takes up more space for the code it's true but it offers a lot of advantage for development of software and it is definetly worth it! if it was not we would all be programming using the Assembly language of our processor. Abstraction is (almost) always good.

Post by **Kaminaris** » 11 Sep 2018, 04:19

You are mistaken about few things. I never said functions are wrong. Functions are wrong inside rotation function!

1. You cannot create a ONE function inside rotation. You are creating one function each 0.15 seconds which gets garbage collected later. I bet it would create like 100 functions before it gets garbage collected.

2. This makes tremendous impact on performance which is crucial here. This function gets called each 0.15s. While it is configurable and you can make it 0.01s. Your function only lives one cycle.

So not only you are using expensive function, but you also wrap it around another function which could be replaced by one variable. Then you are making garbage collector sweat because it needs to get rid of your 100 functions. With debug function it will be 200.

So in about 15 seconds you created 200 functions on the fly. You really think this has anything to do with code reuse?

The only way to do it properly is to declare a function inside a global scope _G IF it is not defined already. And you can see how to do it in my previous post.

I have 14 years of experience with development, I see issues like this on first glance. And trust me, calling function twice where result will not change is performance issue.

If you doubt me, try other rotation addons and measure them with Addon CPU usage. So far every rotation I wrote takes from 0.1% - 1% cpu. While those with targets checking takes considerably more.

Gadgetsan · Post by **Gadgetsan** » 11 Sep 2018, 12:50

if you explained it clearly the first time i would have understood it earlier.

you however have a good point. to be honest i didn't think about the fact that this code needed to run four time every seconds. 250ms isn't that much of a delay for lower level language but i guess for LUA it is.

i will build myself a library that can be used without having to rebuild it every time (a kind of singleton if you wish)

Gadgetsan · Post by **Gadgetsan** » 14 Sep 2018, 02:46

I have updated my code to use my new library system that will (hopefully) satisfy my maintenance requirements as well as your performance concerns.

I will be updating my initial post as to make the most recent version easy to find.

i will also update my Restoration rotation with my Library system soon.

Code: [Select all] [Expand/Collapse]

function(_, timeShift, currentSpell, gcd, talents)
 
    if _GadgetsanLibrary == nil or _GadgetsanLibrary.libVersion ~= "Moonkin3.0" then
        _GadgetsanLibrary = {};
        _GadgetsanLibrary.libVersion = "Moonkin3.0";
 
    --CACHEs setup
        _GadgetsanLibrary.hitCountCache = {};
        _GadgetsanLibrary.movingCache = nil;
        _GadgetsanLibrary.hasAuraCache = {};
        _GadgetsanLibrary.meAuraCountCache = {};
 
        --a function that should be called when a new rotation has started so it can do some wuick housekeeping
        _GadgetsanLibrary.newRotation = function()            
            _GadgetsanLibrary.hitCountCache = {};
            _GadgetsanLibrary.movingCache = nil;
            _GadgetsanLibrary.hasAuraCache = {};
            _GadgetsanLibrary.meAuraCountCache = {};
        end
 
 
    --HELPER FUNCTIONS
 
        --for debugging purposes (i turn this off if i want to output some text)
        _GadgetsanLibrary.DebugToggle = false;
        _GadgetsanLibrary.debug = function(text)
            if(_GadgetsanLibrary.DebugToggle) then print(text);end;
        end
 
        --caching the value of TargetsInRange so i don't make unnecesary calls
        _GadgetsanLibrary.hitCount = function(spellId)
            if _GadgetsanLibrary.hitCountCache[spellId] == nil then
                _GadgetsanLibrary.hitCountCache[spellId] = MaxDps:TargetsInRange(spellId);
            end
            return _GadgetsanLibrary.hitCountCache[spellId];
        end
 
        --caching the value of TargetAura so i don't make unnecesary calls
        _GadgetsanLibrary.hasAura = function(spellId, shift)
            if _GadgetsanLibrary.hasAuraCache[spellId] == nil then
                _GadgetsanLibrary.hasAuraCache[spellId] = MaxDps:TargetAura(spellId, shift);
            end
            return _GadgetsanLibrary.hasAuraCache[spellId];
        end
 
        --caching the value of Aura so i don't make unnecesary calls
        _GadgetsanLibrary.meAuraCount = function(spellId)
            if _GadgetsanLibrary.meAuraCountCache[spellId] == nil then
                local trueOrFalse, count = MaxDps:Aura(spellId, timeShift);
                _GadgetsanLibrary.meAuraCountCache[spellId] = count;
            end
            return _GadgetsanLibrary.meAuraCountCache[spellId];
        end
 
 
        --function to test if character is moving (and caching the result)
        _GadgetsanLibrary.isMoving = function()
            if _GadgetsanLibrary.movingCache ~= nil then
                return _GadgetsanLibrary.movingCache
            end
            local currentSpeed, runSpeed, flightSpeed, swimSpeed = GetUnitSpeed("Player");
            if currentSpeed == 0 then
                _GadgetsanLibrary.movingCache = false;
            else
                _GadgetsanLibrary.movingCache = true;
            end
            return _GadgetsanLibrary.movingCache;
        end
        
    --SKILLS
        _GadgetsanLibrary.Moonfire = 8921;
        _GadgetsanLibrary.Sunfire = 93402;
        _GadgetsanLibrary.Starsurge = 78674;
        _GadgetsanLibrary.LunarEmpowerment = 164547;
        _GadgetsanLibrary.SolarEmpowerment = 164545;
        _GadgetsanLibrary.LunarStrike = 194153;
        _GadgetsanLibrary.SolarWrath = 190984;
        _GadgetsanLibrary.NewMoon = 274281;
        _GadgetsanLibrary.HalfMoon = 274282;
        _GadgetsanLibrary.FullMoon = 274283;
        _GadgetsanLibrary.CelestialAlignment = 194223;
        _GadgetsanLibrary.IncarnationChosenofElune = 102560;
        _GadgetsanLibrary.StellarFlare = 202347;
        _GadgetsanLibrary.Starfall = 191034;
        _GadgetsanLibrary.MasteryStarlight = 77492;
        _GadgetsanLibrary.StellarEmpowerment = 197637;
        _GadgetsanLibrary.Heroism = 32182;
        _GadgetsanLibrary.Bloodlust = 2825;
        _GadgetsanLibrary.Berserking = 26297;
        _GadgetsanLibrary.ForceofNature = 205636;
        _GadgetsanLibrary.WarriorofElune = 202425;
        _GadgetsanLibrary.AstralCommunion = 202359;
        _GadgetsanLibrary.BlessingoftheAncients = 202360;
        _GadgetsanLibrary.BlessingofElune = 202737;
        _GadgetsanLibrary.FuryofElune = 202770;
        _GadgetsanLibrary.MoonkinMoonfire = 164812;
        _GadgetsanLibrary.MoonkinSunfire = 164815;
 
 
    end
 
    local GL = _GadgetsanLibrary;
    GL.newRotation();
 
--Links
    --http://maxdps.net/viewtopic.php?f=6&t=5&sid=134ea7dae4e3b8092032fb116836c364
 
 
-------------------------------VARIABLES--------------------------------------------
    local AP = UnitPower('player', Enum.PowerType.LunarPower);
    --debug(string.format("AstralPower: %s", AP));
    --Note: we can cast 30% before the end because of pandemic dotting
    local solarE, solarCharges = MaxDps:Aura(GL.SolarEmpowerment, timeShift);
    local lunarE, lunarCharges = MaxDps:Aura(GL.LunarEmpowerment, timeShift);
    
    local currentMoonCharges = 0;
    if talents[GL.NewMoon] then
        currentMoon = GL.NewMoon;
        if MaxDps:FindSpell(GL.NewMoon) ~= nil then
            currentMoon = GL.NewMoon;
        end
        if MaxDps:FindSpell(GL.HalfMoon) ~= nil then
            currentMoon = GL.HalfMoon;
        end
        if MaxDps:FindSpell(GL.FullMoon) ~= nil then
            currentMoon = GL.FullMoon;
        end
        --nextMoon is necessary in the case where we are currently casting
        nextMoon = currentMoon;
        local cd, charges, maxCharges = MaxDps:SpellCharges(currentMoon, timeShift);
        currentMoonCharges = charges;
    end
 
    --we look after the current casting to see in advance if we will have enough AP
    if currentSpell == GL.SolarWrath then
        AP = AP + 8;
        solarCharges = solarCharges - 1;
    elseif currentSpell == GL.LunarStrike then
        AP = AP + 12;
        lunarCharges = lunarCharges - 1;
    elseif currentSpell == GL.NewMoon then
        AP = AP + 10;
        currentMoonCharges = currentMoonCharges-1;
        nextMoon = GL.HalfMoon
    elseif currentSpell == GL.HalfMoon then
        AP = AP + 20;
        currentMoonCharges = currentMoonCharges-1;
        nextMoon = GL.FullMoon
    elseif currentSpell == GL.FullMoon then
        AP = AP + 40;
        currentMoonCharges = currentMoonCharges-1;
        nextMoon = GL.NewMoon
    end
    --i once ended up with negative solar charges...
    if lunarCharges < 0 then lunarCharges = 0 end
    if solarCharges < 0 then solarCharges = 0 end
    --GL.debug(string.format("lunarCharges: %s solarCharges: %s ", lunarCharges, solarCharges));
    
 
-------------------------------COOLDOWNS--------------------------------------------
    if talents[GL.IncarnationChosenofElune] then
        MaxDps:GlowCooldown(GL.IncarnationChosenofElune, MaxDps:SpellAvailable(GL.IncarnationChosenofElune, timeShift));
    else
        MaxDps:GlowCooldown(GL.CelestialAlignment, MaxDps:SpellAvailable(GL.CelestialAlignment, timeShift));
    end
 
    if talents[GL.WarriorofElune] then
        MaxDps:GlowCooldown(GL.WarriorofElune, MaxDps:SpellAvailable(GL.WarriorofElune, timeShift)
            and not MaxDps:Aura(GL.WarriorofElune));
    end
    
-------------------------------PRIORITY--------------------------------------------
 
--DOTS so they are always applied
    if not GL.hasAura(GL.MoonkinMoonfire, timeShift + 5) then       
        GL.debug("Target does not have Moonfire!");
        return GL.Moonfire;
    end
 
    if not GL.hasAura(GL.MoonkinSunfire, timeShift + 4) then
        GL.debug("Target does not have Sunfire");
        return GL.Sunfire;
    end    
 
--if i'm moving, i want to instant cast only
    if GL.isMoving() then
        GL.debug("Currently Moving");
        if AP >= 40 then
            return GL.Starsurge;
        end
 
        --if i have warrior of elune, i wanna spend my StellarFlare instant casts        
        if GL.meAuraCount(GL.WarriorofElune) > 0 then
            return GL.LunarStrike
        end
 
        -- is i'm still moving, i wanna cast Warrior of Elune
        if talents[GL.WarriorofElune] and MaxDps:SpellAvailable(GL.WarriorofElune, timeShift) then
            return GL.WarriorofElune
        end
 
        --if nothing else, i can cast this while moving
        return GL.Sunfire;
    end
 
--Stellar Flare (i don't use it...)
    if talents[GL.StellarFlare] and not GL.hasAura(GL.StellarFlare, timeShift + 5) and currentSpell ~= GL.StellarFlare then
        return GL.StellarFlare;
    end
 
    
--New/Half/Full Moon, as suggested by HamOfMoose » 30 Aug 2018, 21:20. Requires New Moon Talent
    if talents[GL.NewMoon] then        
        GL.debug("New Moon Management");
        local isAvailable, cd = MaxDps:SpellAvailable(currentMoon, timeShift)
        --GL.debug(string.format("Found %s stakcs for moon thing", currentMoonCharges));
        if currentMoonCharges > 1 and isAvailable then
            --we look at the current moon (or the next if we are currently casting in order to decide if we cast it or not)
            if(nextMoon == GL.NewMoon and AP < 90) or (nextMoon == GL.HalfMoon and AP < 80) or (nextMoon == GL.FullMoon and AP < 60) then
                return currentMoon;
            end
        end
    end
    
--if i'm maxed on enhancement, i priorise this
    if(lunarCharges > 2 or solarCharges > 2) then
        GL.debug("Almost at max enhancement");
        if lunarCharges > solarCharges then
            return GL.LunarStrike
        else
            return GL.SolarWrath
        end
    end
    
--StarSurge or StarFall to spend AP (LunarStrike and Starfall have same range)
    if(GL.hitCount(GL.LunarStrike) > 3) then
        if AP > 90 then
            GL.debug("enough AP to cast Starfall!");
            return GL.Starfall;
        end
    else
        if AP > 80 then
            GL.debug("enough AP to cast Starsurge!");
            return GL.Starsurge;
        end
    end
 
--Empowerments so i use them
    if(lunarCharges > 0 or solarCharges > 0) then
        GL.debug("casting remaining enhancements");
        if lunarCharges > solarCharges then
            return GL.LunarStrike
        else
            return GL.SolarWrath
        end
    end
 
--SLunarStrike to Get AP if more than 1 in range
    if(GL.hitCount(GL.LunarStrike) > 1) then        
        GL.debug("casting Lunarstrike to raise AP");
        return GL.LunarStrike;
    else
        GL.debug("casting Solar Wrath to raise AP");
        return GL.SolarWrath;
    end
 
    GL.debug("I've got nothing to do!");
    return GL.LunarStrike;
end

ruchikadmore · Post by **ruchikadmore** » 26 Feb 2019, 06:19

Excellent topic. I got lots of information here. Thank you.

maxdps.net

My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation

Re: My Balance Druid custom Rotation