[vJASS] Need to check this

[Starquizer]

I need someone to check this trigger for any leak problem or some useless stuff. Its a Damage Over Time (DoT) spell,
the spell worked for me without any bugs but who knows may be somes flaws will be found.

~thanks in advance~

scope Immo initializer Init2

globals

    Immolation filter
    Immolation dmg
    
    attacktype AT   = ATTACK_TYPE_NORMAL
    damagetype DT   = DAMAGE_TYPE_FIRE

    integer array temp_dat
    integer ABIL_ID = 'A003'
    integer index   = 0

    real DAMAGE     =   0.50
    real DURATION   =  10.00
    real INTERVAL   =   0.10
    real R          = 300.00
    
    string mdl      = "Abilities\\Spells\\Other\\BreathOfFire\\BreathOfFireDamage.mdl"
    string AttPoint = "chest"
  
    timer interval = CreateTimer()

endglobals

struct Immolation

    unit     caster
    location castpoint
    timer    duration
    group    targets
    boolean  done
    integer  Ntargets
    integer  int
    effect   array burning[30]
    boolean  array effectOnce[30]

    static method Interval takes nothing returns nothing
        local thistype dat = thistype.create()
        local integer i = 0
     
        loop
        exitwhen i >= index
            set dat = temp_dat[i]
            set dmg = dat

            if dat.done == false then
                set dat.int = 0
                call ForGroup(dat.targets, function Immolation.Damage)
            endif
            
        set i = i + 1
        endloop

    endmethod
    
    static method Damage takes nothing returns nothing
        local unit u = GetEnumUnit()

        call UnitDamageTarget(dmg.caster,u,DAMAGE,true,false,AT,DT,null)

            if dmg.effectOnce[dmg.int] == false then
                set dmg.effectOnce[dmg.int] = true
                set dmg.burning[dmg.int] = AddSpecialEffectTarget(mdl,u,AttPoint)
            endif
        set dmg.int = dmg.int + 1

    endmethod


    static method Duration takes nothing returns nothing
        local thistype dat = thistype.create()
        local integer i = 0
        
        loop
        exitwhen i >= index
            set dat = temp_dat[i]
            if TimerGetRemaining(dat.duration) <= 0.0 then
                set index = index - 1
                set temp_dat[i] = temp_dat[index]
                set i = i - 1

                set dat.done = true
                call DestroyGroup(dat.targets)
                call DestroyTimer(dat.duration)
                set dat.caster = null

                loop
                exitwhen i >= dat.Ntargets
                    call DestroyEffect(dat.burning[i])
                set i = i + 1
                endloop

                call dat.destroy()

                if index == 0 then
                    call PauseTimer(interval)
                endif

            endif
        set i = i + 1
        endloop
    endmethod


    static method Setup takes unit u,location loc returns nothing
        local thistype dat = thistype.create()
        local real X
        local real Y
        local integer i = 0

        set temp_dat[index] = dat
        set dat.caster      = u
        set dat.castpoint   = loc
        set X               = GetLocationX(loc)
        set Y               = GetLocationY(loc)
        call RemoveLocation(dat.castpoint)
        set dat.duration    = CreateTimer()
        set dat.targets     = CreateGroup()
        set dat.done        = false
        set dat.Ntargets    = 0
        set dat.int         = 0

        set filter = dat
        call GroupEnumUnitsInRange(dat.targets,X,Y,R,Filter(function thistype.Match))
        
        loop
        exitwhen i >= dat.Ntargets
            set dat.effectOnce[i]  = false
        set i = i + 1
        endloop

        call TimerStart(dat.duration, DURATION , false , function thistype.Duration )
   
        if index == 0 then
            call TimerStart(interval, INTERVAL , true  , function thistype.Interval )
        endif

        set index = index + 1

    endmethod

    static method Match takes nothing returns boolean

        if (IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(filter.caster))) then
            set filter.Ntargets = filter.Ntargets + 1
            return true
        else
            return false
        endif

    endmethod

endstruct

function t_Actions takes nothing returns nothing
//*
    if (GetSpellAbilityId() == ABIL_ID) then
        call Immolation.Setup(GetTriggerUnit(),GetSpellTargetLoc())
    endif
//*/

/*
// MUI Testing
local integer i = 0
if (GetSpellAbilityId() == ABIL_ID) then
loop
exitwhen i >= 1
call Immolation.Setup(GetTriggerUnit(),GetSpellTargetLoc())
set i = i + 1
endloop
endif
*/

endfunction

function Init2 takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddAction( t, function t_Actions )
endfunction

endscope

 

[Pharaoh_]

Since you merge actions with conditions, merge them to a conditions block, which runs before the actions' one and return false.
GetSpellTargetLoc() leaks. Use coordinates; get GetSpellTargetX() and GetSpellTargetY() as parameters.

Merge interval with duration, checking if interval exceeds MaxCount (Where MaxCount = duration of DpS, which increases by INTERVAL amount).

 

[Starquizer]

I found a problem with special effects, when the spell is cast frequently in short time (like holding SHIFT and left-clicking the location) some special effects are not destroyed. Is it something related to having many timer variables ( duration[] ) ?

 

[Maker]

I recommed you look into constant and private keywords.

private timer interval = CreateTimer()
^The timer should be private if you don't want it to be accessible outside this scope. Adding private key word also avoids clashes with other global variables with the same name.

private constant real DAMAGE = 0.50
Âdd private since the value most likely should not be accessible from outside the library. Constant prevents the value from being modified inside the functions/methods.

Init2 should also be private and maybe the whole struct or atleast some methods.

 

[Luorax]

static method Setup takes unit u,real x,real y returns nothing
    // No more local reals needed, nor stupid, stupid locations, which are 
    // slow as hell
    call GroupEnumUnitsInRange(dat.targets,x,y,R,Filter(function thistype.Match))
endmethod

....

call Immolation.Setup(GetTriggerUnit(),GetSpellTargetX(),GetSpellTargetY())

// or even more inlined:

static method Setup takes unit ureturns nothing
    call GroupEnumUnitsInRange(dat.targets,GetSpellTargetX(),GetSpellTargetY(),R,Filter(function thistype.Match))
endmethod

I've also taught you how to use arrays in array structs; take advantage of it.

 

[Starquizer]

I recommed ... methods.

I know about those things and I will add them once I finish it. (but thanks anyway)

My concern now is the special effect: it is not working well and some effects don't get destroyed when the duration ends.

I also made a few changes concerning the duration timer.

scope Immo2 initializer Init2

globals

    private Immolation filter
    private Immolation dmg
    
    private attacktype AT   = ATTACK_TYPE_NORMAL
    private damagetype DT   = DAMAGE_TYPE_FIRE

    private integer array temp_dat
    private integer ABIL_ID = 'A003'
    private integer index   = 0

    private real DAMAGE     =   0.10
    private real DURATION   =  05.00
    private real INTERVAL   =   0.10
    private real R          = 300.00
    
    private string mdl      = "Abilities\\Spells\\Other\\BreathOfFire\\BreathOfFireDamage.mdl"
    private string AttPoint = "chest"
  
    private timer interval = CreateTimer()

endglobals

struct Immolation

    unit     caster
    location castpoint
    //timer    duration
    integer  executecount
    group    targets
    boolean  done
    integer  Ntargets
    integer  int
    effect   array burning[64]
    boolean  array effectOnce[64]

    static method Interval takes nothing returns nothing
        local thistype dat = thistype.create()
        local integer i = 0

        loop
        exitwhen i >= index
            set dat = temp_dat[i]
            set dmg = dat
            set dat.executecount = dat.executecount + 1
            if dat.executecount > R2I(DURATION/INTERVAL) then
                set index = index - 1
                set temp_dat[i] = temp_dat[index]
                set i = i - 1
                call DestroyGroup(dat.targets)
                set dat.caster = null

                loop
                exitwhen i >= dat.Ntargets
                    call DestroyEffect(dat.burning[i])
                set i = i + 1
                endloop
                
                call dat.destroy()

                if index == 0 then
                    call PauseTimer(interval)
                endif

            else
                set dat.int = 0
                call ForGroup(dat.targets, function Immolation.Damage)
            endif

        set i = i + 1
        endloop

    endmethod
    
    static method Damage takes nothing returns nothing
        local unit u = GetEnumUnit()
        local integer i = dmg.int

        call UnitDamageTarget(dmg.caster,u,DAMAGE,true,false,AT,DT,null)

            if dmg.effectOnce[i] == false then
                set dmg.effectOnce[i] = true
                set dmg.burning[i] = AddSpecialEffectTarget(mdl,u,AttPoint)
                set dmg.int = dmg.int + 1
            endif

    endmethod

    static method Match takes nothing returns boolean

        if (IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(filter.caster))) then
            set filter.Ntargets = filter.Ntargets + 1
            return true
        else
            return false
        endif

    endmethod

    static method Setup takes unit u,real x,real y returns nothing
            local thistype dat = thistype.create()
            local real X
            local real Y
            local integer i = 0

        set temp_dat[index] = dat
        set dat.caster      = u
        set X               = x
        set Y               = y
        set dat.executecount= 0
        set dat.targets     = CreateGroup()
        set dat.done        = false
        set dat.Ntargets    = 0
        set dat.int         = 0

        set filter = dat
        call GroupEnumUnitsInRange(dat.targets,X,Y,R,Filter(function thistype.Match))
        
        loop
        exitwhen i >= dat.Ntargets
            set dat.effectOnce[i]  = false
            // just a fail safe
            call DestroyEffect(dat.burning[i])
        set i = i + 1
        endloop

        if index == 0 then
            call TimerStart(interval, INTERVAL , true  , function thistype.Interval )
        endif

        set index = index + 1

    endmethod

endstruct

function t_Actions takes nothing returns nothing
        local location loc = GetSpellTargetLoc()
    if (GetSpellAbilityId() == ABIL_ID) then
        call Immolation.Setup(GetTriggerUnit(),GetLocationX(loc),GetLocationY(loc))
        call RemoveLocation(loc)
    endif
endfunction

function Init2 takes nothing returns nothing
        local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddAction( t, function t_Actions )
endfunction

endscope

I was hoping someone tell me whats wrong with the special effect.

Damn it, I solved the special effect problem to find another.

I am going through the spell line by line and I don't know what's the problem

Problem

When I hold SHIFT and press SPELL HOTKEY then L-clicks; lets say spell damage is 5 and I clicked quickly 5 times then the total is 25, what really happen is less than that)

Spell

scope Immo2 initializer Init2

globals

    private Immolation filter
    private Immolation dmg
    
    private attacktype AT   = ATTACK_TYPE_NORMAL
    private damagetype DT   = DAMAGE_TYPE_FIRE

    private integer array temp_dat
    private integer ABIL_ID = 'A003'
    private integer index   = 0

    private real DAMAGE     =   0.10
    private real DURATION   =  05.00
    private real INTERVAL   =   0.10
    private real DIV = DURATION/INTERVAL
    private real R          = 300.00
    
    private string mdl      = "Abilities\\Spells\\Other\\BreathOfFire\\BreathOfFireDamage.mdl"
    private string AttPoint = "chest"
  
    private timer interval = CreateTimer()

endglobals

struct Immolation

    unit     caster
    location castpoint
    //timer    duration
    integer  executecount
    group    targets
    integer  Ntargets
    integer  int
    effect   array burning[64]
    boolean  array effectOnce[64]

    static method Interval takes nothing returns nothing
        local thistype dat = thistype.allocate()
        local integer i = 0
        local integer j = 0

        loop
        exitwhen i >= index
            set dat = temp_dat[i]
            set dat.executecount = dat.executecount + 1
            if dat.executecount > DIV then
                set index = index - 1
                set temp_dat[i] = temp_dat[index]
                set i = i - 1
                call DestroyGroup(dat.targets)
                set dat.caster = null

                loop
                exitwhen j >= dat.Ntargets
                    call DestroyEffect(dat.burning[j])
                set j = j + 1
                endloop
                
                call dat.destroy()

                if index == 0 then
                    call PauseTimer(interval)
                endif

            else
                set dmg = dat
                set dat.int = 0
                call ForGroup(dat.targets, function Immolation.Damage)
            endif

        set i = i + 1
        endloop

    endmethod
    
    static method Damage takes nothing returns nothing
        local unit u = GetEnumUnit()
        local integer i = dmg.int

        call UnitDamageTarget(dmg.caster,u,DAMAGE,true,false,AT,DT,null)

            if dmg.effectOnce[i] == false then
                set dmg.effectOnce[i] = true
                set dmg.burning[i] = AddSpecialEffectTarget(mdl,u,AttPoint)
                set dmg.int = dmg.int + 1
            endif

    endmethod

    static method Match takes nothing returns boolean

        if (IsUnitEnemy(GetFilterUnit(),GetOwningPlayer(filter.caster))) then
            set filter.Ntargets = filter.Ntargets + 1
            return true
        else
            return false
        endif

    endmethod

    static method Setup takes unit u,real X,real Y returns nothing
            local thistype dat = thistype.allocate()
            local integer i = 0

        set temp_dat[index] = dat
        set dat.caster      = u
        set dat.executecount= 0
        set dat.targets     = CreateGroup()
        set dat.Ntargets    = 0
        set dat.int         = 0

        set filter = dat
        call GroupEnumUnitsInRange(dat.targets,X,Y,R,Filter(function thistype.Match))
        
        
        loop
        exitwhen i >= dat.Ntargets
            set dat.effectOnce[i]  = false
            // just a fail safe
            call DestroyEffect(dat.burning[i])
        set i = i + 1
        endloop

        if index == 0 then
            call TimerStart(interval, INTERVAL , true  , function thistype.Interval )
        endif

        set index = index + 1

    endmethod

endstruct

function t_Actions takes nothing returns nothing
    if (GetSpellAbilityId() == ABIL_ID) then
        call Immolation.Setup(GetTriggerUnit(),GetSpellTargetX(),GetSpellTargetY())
    endif
endfunction

function Init2 takes nothing returns nothing
        local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddAction( t, function t_Actions )
endfunction

endscope

<<< EDIT >>>
sorry for posting 2 consecutive times

 

[Bribe]

You should also set "dat.targets = null" if you are going to bother with setting "dat.caster = null"

 

[Maker]

Generally EVENT_PLAYER_UNIT_SPELL_EFFECT event is used for spells, not EVENT_PLAYER_UNIT_SPELL_CAST

Cooldown and mana cost take place at EFFECT. The time between CAST and EFFECT is defined by the unit's object editor value of Art - Animation - Cast Point.

 

[Starquizer]

Cooldown and mana cost take place at EFFECT. The time between CAST and EFFECT is defined by the unit's object editor value of Art - Animation - Cast Point.

I am assuming worst case scenario; which is the user setting cooldown and mana to 0 and begin casting quickly.

Also, I am making Damage Over Time because once it is done I will implement it to my Firenova spell but the problem is that I can't make this work properly.

 

[Maker]

Activate ability - give stop order. Ability will trigger with CAST event with no mana cost or cooldown. With EFFECT, it is not possible.

 

[Starquizer]

How about this, instead of using struct array members I used hashtable to store effects and booleans but I am still using unit groups.

// Damage Over Time using Unit Group

scope Immolation initializer Init2

globals

    private Immolation dmg
    
    private attacktype AT   = ATTACK_TYPE_NORMAL
    private damagetype DT   = DAMAGE_TYPE_FIRE

    private integer array temp_dat
    private integer ABIL_ID = 'A003'
    private integer index   = 0

    private real DAMAGE     =   0.10
    private real DURATION   =  10.00
    private real INTERVAL   =   0.10
    private real R          = 300.00
    
    private string mdl      = "Abilities\\Spells\\Other\\BreathOfFire\\BreathOfFireDamage.mdl"
    private string AttPoint = "chest"
  
    private timer interval = CreateTimer()
    
    private hashtable fxTable = InitHashtable()

endglobals

struct Immolation

    unit     caster
    location castpoint
    integer  executecount
    group    targets
    integer  Ntargets


    static method Interval takes nothing returns nothing
        local thistype dat
        local integer i = 0
        local integer j = 0
        local unit unt
        local group temp_g2 = CreateGroup()

        loop
        exitwhen i >= index
            set dat = temp_dat[i]
            set dat.executecount = dat.executecount + 1

            if dat.executecount > DURATION/INTERVAL then

call BJDebugMsg("|cFFffcc00Duration Ended|r")

                set index = index - 1
                set temp_dat[i] = temp_dat[index]
                set i = i - 1
                call DestroyGroup(dat.targets)
                set dat.caster = null
                set dat.executecount = 0

                set j = 0
                loop
                exitwhen j >= dat.Ntargets
                    call DestroyEffect(LoadEffectHandle(fxTable,dat,j))
                set j = j + 1
                endloop

                set dat.Ntargets     = 0
                call FlushChildHashtable(fxTable,dat)
                call dat.destroy()

                if index == 0 then
                    call PauseTimer(interval)
                endif

            else

            set j = 0
            loop
                set unt = FirstOfGroup(dat.targets)
            exitwhen j >= dat.Ntargets

                call UnitDamageTarget(dat.caster,unt,DAMAGE,false,false,AT,DT,null)

                if LoadBoolean(fxTable,dat,j) == false then
                call SaveBoolean(fxTable,dat,j,SaveEffectHandle(fxTable,dat,j,AddSpecialEffectTarget(mdl,unt,AttPoint)))
                endif

                call GroupAddUnit(temp_g2,unt)
                call GroupRemoveUnit(dat.targets,unt)
            set j = j + 1
            endloop
            
            loop
                set unt = FirstOfGroup(temp_g2)
            exitwhen unt == null

                call GroupAddUnit(dat.targets,unt)
                call GroupRemoveUnit(temp_g2,unt)
            endloop
            
            endif

        set i = i + 1
        endloop
        call DestroyGroup(temp_g2)

    endmethod

    static method Setup takes unit u,real X,real Y returns nothing
        local thistype dat    = thistype.allocate()
        local integer  i      = 0
        local group    temp_g = CreateGroup()
        local unit     temp_u

        set temp_dat[index]   = dat
        set dat.caster        = u
        set dat.executecount  = 0
        set dat.Ntargets      = 0
        set dat.targets       = CreateGroup()

        call GroupEnumUnitsInRange(temp_g,X,Y,R,null)

        loop
        set temp_u = FirstOfGroup(temp_g)
        exitwhen temp_u == null

        if (IsUnitEnemy(temp_u,GetOwningPlayer(u))) == true then
            call GroupAddUnit(dat.targets,temp_u)
            set dat.Ntargets = dat.Ntargets + 1
        endif
        
        call GroupRemoveUnit(temp_g,temp_u)
        endloop

        call DestroyGroup(temp_g)

        if index == 0 then
            call TimerStart(interval, INTERVAL , true  , function thistype.Interval )
        endif

        set index = index + 1
//call BJDebugMsg(I2S(dat))
    endmethod

endstruct

function t_Actions takes nothing returns nothing
    if (GetSpellAbilityId() == ABIL_ID) then
        call Immolation.Setup(GetTriggerUnit(),GetSpellTargetX(),GetSpellTargetY())
    endif
endfunction

function Init2 takes nothing returns nothing
        local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddAction( t, function t_Actions )
endfunction

endscope

<<< EDIT >>>
I will try to make a version but using hashtable for units instead of unit groups