[JASS] Activatable Spell won't level up when active

[Titanhex]

Alright so a long time ago watermelon1234 showed me an example on how to do Activatables, as they're a rather complex thing from what I can tell.

Well, what I've been using is far from perfect, and has a lot of issues. (This set-up has been giving me headaches forever. When Silenced the spell deactivates without the icon deactivating. If you try to level the spell up when active, it doesn't level up. Fortunately those are the two issues where it's been narrowed down to.)

Perhaps someone can give me a better way, or help me fix this current version.

Uses Table.

Activatable

scope Euphoria initializer Init

globals    
    private constant integer SPELLID    = 'Euph'
    private constant string TurnOn      = "immolation"
    private constant string TurnOff     = "unimmolation"
    private constant real tick          = 0.1
    private HandleTable info
    private unit TEMP
endglobals

private struct Data

    unit c
    boolean deactivate = false
    timer tim

    static method GroupEm takes nothing returns boolean
     local integer lvl = GetUnitAbilityLevel(TEMP, SPELLID)
        if IsUnitAlly(GetFilterUnit(), GetOwningPlayer(TEMP)) and IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) != true and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) != true then
            call SetWidgetLife(GetFilterUnit(), GetWidgetLife(GetFilterUnit())+((GetHeroInt(TEMP,true)*lvl*0.07+(0.3+0.8*lvl)+1.5)*tick))
            call DestroyEffect(AddSpecialEffectTarget(GetAbilityEffectById(SPELLID, EFFECT_TYPE_TARGET, 0), GetFilterUnit(), "origin"))
        endif
     return false
    endmethod
    
    static method onLoop takes nothing returns nothing
        local group g = NewGroup()
        local Data D = Data(GetTimerData(GetExpiredTimer()))
        if not(IsUnitType(D.c,UNIT_TYPE_DEAD)) and not(D.deactivate) and GetUnitState(D.c, UNIT_STATE_MANA) >= 10 then
            //Start timer
            set TEMP = D.c
            call GroupEnumUnitsInArea(g, GetUnitX(D.c), GetUnitY(D.c), 300, Filter(function Data.GroupEm))
            call SetUnitState(D.c, UNIT_STATE_MANA, GetUnitState(D.c, UNIT_STATE_MANA) - (0.3 + 0.1 * GetHeroInt(D.c, true)*tick))
            call ReleaseGroup(g)
        else
            //Cleanup
            call IssueImmediateOrder(D.c, TurnOff)
            call info.flush(D.c)
            set D.c = null
            call ReleaseTimer(D.tim)
            set D.tim = null
            call D.destroy()
        endif
    endmethod
    
    static method create takes unit caster returns Data
    local Data D =  Data.allocate()
        set D.c = caster
        //Timer
        set D.tim = NewTimer()
        call SetTimerData(D.tim,D)
        call TimerStart(D.tim, tick, true, function Data.onLoop)
        set info[D.c] = D
        return D
    endmethod
endstruct
 
private function Deactivate takes nothing returns boolean
 local Data D
    if GetIssuedOrderId() == OrderId(TurnOff) and info.exists(GetTriggerUnit()) then
        set D = info[GetTriggerUnit()]
        set D.deactivate = true
    endif
 return false
endfunction

private function Activation takes nothing returns boolean
    if GetIssuedOrderId() == OrderId(TurnOn) and GetUnitAbilityLevel(GetTriggerUnit(), SPELLID) > 0 then
        call Data.create(GetTriggerUnit())
    endif
 return false
endfunction

 //===========================================================================
public function Init takes nothing returns nothing
 local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_ISSUED_ORDER )
    call TriggerAddCondition(t, function Activation )
    set t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_ISSUED_ORDER )
    call TriggerAddCondition( t, function Deactivate )
    set info = HandleTable.create()
 call XE_PreloadAbility(SPELLID)
endfunction

endscope

 

[Bribe]

1. Using a Unit Indexer is much better than hashing to each unit with a HandleTable
2. Vexorian's Table is bad because it fails with module initializers
3. This only works with immolation
4. Abilities like immolation don't level up while active. I think the same is true with any togglable ability.
5. TimerQueue is much better than TimerUtils in this case. TimerUtils also fails with struct/module initializers.
6. You register two identical sets of events for two different triggers which is totally unnecessary. This can be avoided using a single condition and check what the issued order id is, then go from there.
7. Constants, locals have bad form.
8. If a scope has requirements, it needs to be a library or at least list those requirements.
9. GroupUtils is totally unneeded. Use a static enum group, instead. bj_lastCreatedGroup is a perfect candidate.
10. This:

library Euphoria requires UnitIndexer, Tq, optional xepreload

globals    
    private constant integer SPELL_ID   = 'Euph'
    private constant integer TURN_ON    = OrderId("immolation")
    private constant integer TURN_OFF   = OrderId("unimmolation")
    private constant real TICK          = 0.1
    private constant real AOE           = 300
    private unit tU
endglobals

private module Init
    boolean deactivate = false
    thistype c
    thistype d
    
    static constant real INTERVAL = TICK
    
    static method enum takes nothing returns boolean
        local integer lvl = GetUnitAbilityLevel(tU, SPELLID)
        local unit u = GetFilterUnit()
        if IsUnitAlly(u, GetOwningPlayer(tU)) and not IsUnitType(u, UNIT_TYPE_STRUCTURE) and not IsUnitType(u, UNIT_TYPE_DEAD) then
            call SetWidgetLife(u, GetWidgetLife(u)+((GetHeroInt(tU, true) * lvl * 0.07 + (0.3 + 0.8 * lvl) + 1.5) * TICK))
            call DestroyEffect(AddSpecialEffectTarget(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_TARGET, 0), u, "origin"))
        endif
        set u = null
        return false
    endmethod
    
    method expire takes nothing returns nothing
        if not(IsUnitType(tU, UNIT_TYPE_DEAD)) and not(this.deactivate) and GetUnitState(tU, UNIT_STATE_MANA) >= 10 then
            set tU = GetUnitById(this.c)
            call SetUnitState(tU, UNIT_STATE_MANA, GetUnitState(tU, UNIT_STATE_MANA) - (0.3 + 0.1 * GetHeroInt(tU, true)*tick))
            call GroupEnumUnitsInArea(bj_lastCreatedGroup, GetUnitX(tU), GetUnitY(tU), AOE, Filter(function Data.enum))
        else
            call IssueImmediateOrder(GetUnitById(this.c), TurnOff)
            call Tq_D(this)
            set this.c.d = 0
            set this.c = 0
        endif
    endmethod
    
    implement Tq
    
    static method create takes unit caster returns Data
        local Data d = Data.allocate()
        set d.c = GetUnitUserData(caster)
        set d.c.d = d
        return d
    endmethod
    
    static method toggle takes nothing returns boolean
        if (GetUnitAbilityLevel(GetTriggerUnit(), SPELL_ID) > 0) then
            if (GetIssuedOrderId() == TURN_OFF) then
                set thistype(GetUnitUserData(GetTriggerUnit())).d.deactivate = true
            elseif (GetIssuedOrderId() == TURN_ON) then
                call Data.create(GetTriggerUnit())
            endif
        endif
        return false
    endmethod

    private static method onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_ISSUED_ORDER)
        call TriggerAddCondition(t, Condition(function thistype.toggle))
        static if (LIBRARY_xepreload) then
            call XE_PreloadAbility(SPELLID)
        endif
        set t = null
    endmethod
    
endmodule

private struct Data extends array
    implement Init
endstruct

endlibrary