Jass Help w/leaks and bugs

[graystuff111]

Ok, I made a working Jass spell, it works quite nicely, but I"m just wondering if it has any leaks in it. I'm quite confused about jass leaks and locals, so help would be much apprecated. I'm kinda a jass noob right now XD

globals
//Below is the customizable section of the spell----------------------------------------------------------------------------------
integer Id = 'A000'//This is the rawcode of the ability you want to add.
real TimeStill = 2.//this is the amount of time the unit needs to be standing still before the ability is added.  It is in seconds
//Dont Touch These, or anything below this line!!!--------------------------------------------------------------------------------
integer array UnitUsed
unit array Units
real array UnitTimeStill
real array UnitX
real array UnitY
integer array UnitHasAbility
integer maxunits = 0
endglobals

function AddUnit_TimedAbility takes unit u returns nothing
local integer i = 0
loop
exitwhen UnitUsed[i] == 0
set i = i+1
endloop
set Units[i] = u
set UnitHasAbility[i] = 0
set UnitUsed[i] = 1
if i > maxunits then
set maxunits = i
endif
set u = null
endfunction

function RemoveUnit_TimedAbility takes unit u returns nothing
local integer i = 0
loop
exitwhen Units[i] == u
set i = i+1
endloop
set UnitUsed[i] = 0
set Units[i] = null
set UnitTimeStill[i] = 0.
set UnitX[i] = 0.
set UnitY[i] = 0.
endfunction


function Trig_Adding_Abilitys_Actions takes nothing returns nothing
local integer i = 0
local real X
local real Y
local location p
loop
exitwhen i > maxunits
if UnitUsed[i] == 1 then
    set p = GetUnitLoc(Units[i])
    set X = GetLocationX(p)
    set Y = GetLocationY(p)
    if X == UnitX[i] and Y == UnitY[i] then
    set UnitTimeStill[i] = UnitTimeStill[i] + 0.2
    else
    set UnitTimeStill[i] = 0.
    set UnitX[i] = X
    set UnitY[i] = Y
    set UnitHasAbility[i] = 0
    call UnitRemoveAbility(Units[i], Id)
    endif
    call RemoveLocation(p)
        if UnitTimeStill[i] >= TimeStill then
            if UnitHasAbility[i] == 0 then
            call UnitAddAbility(Units[i], Id)
            set UnitHasAbility[i] = 1
            endif
        endif
endif
set i = i+1
endloop
endfunction

//===========================================================================
function InitTrig_Adding_Abilitys takes nothing returns nothing
    set UnitUsed[0] = 1
    set gg_trg_Adding_Abilitys = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_Adding_Abilitys, 0.20 )
    call TriggerAddAction( gg_trg_Adding_Abilitys, function Trig_Adding_Abilitys_Actions )
endfunction

 

[aznricepuff]

No leaks, but you don't need to use a location in Trig_Adding_Ability_Actions, just use GetUnitX() and GetUnitY().

You also don't need to iterate through your array to add a unit. Just save the number of units you have in the array and just add new units to the end. When you remove units, just put the unit at the last used index in the index that you removed.

There's also a btr way to do this using structs rather than a bunch of arrays, but if you don't really understand JASS then I guess it's better not to get in over your head.

EDIT: Well, I just spotted a leak 2 seconds after I posted...you don't null the location variable in Trig_Adding_Ability_Actions. But if you just use GetUnitX() and GetUnitY() you won't need to use a location anyway.

 

[graystuff111]

New Trigger

globals
integer Id = 'A000'//This is the rawcode of the ability you want to add.
real TimeStill = 2.//this is the amount of time the unit needs to be standing still before the ability is added.  It is in seconds
unit array Units
real array UnitTimeStill
real array UnitX
real array UnitY
integer array UnitHasAbility
integer maxunits = 0
endglobals

function AddUnit_TimedAbility takes unit u returns nothing
local integer i = 0
set i = maxunits + 1
set Units[i] = u
set UnitHasAbility[i] = 0
set maxunits = maxunits + 1
set u = null
endfunction

function RemoveUnit_TimedAbility takes unit u returns nothing
local integer i = 0
local integer j = 0
local integer k = 0
loop
exitwhen Units[i] == u
set i = i+1
endloop
    set j = i
    loop
    exitwhen k > maxunits
    set k = j + 1
    set Units[j] = null
    set Units[j] = Units[k]
    set UnitTimeStill[j] = UnitTimeStill[k]
    set UnitX[j] = UnitX[k]
    set UnitY[j] = UnitY[k]
    set UnitHasAbility[j] = UnitHasAbility[k]
    set j = j + 1
    endloop
    set maxunits = maxunits - 1
endfunction


function Trig_Adding_Abilitys_Actions takes nothing returns nothing
local integer i = 0
local real X
local real Y
loop
exitwhen i > maxunits
    set X = GetUnitX(Units[i])
    set Y = GetUnitY(Units[i])
    if X == UnitX[i] and Y == UnitY[i] then
    set UnitTimeStill[i] = UnitTimeStill[i] + 0.2
    else
    set UnitTimeStill[i] = 0.
    set UnitX[i] = X
    set UnitY[i] = Y
    set UnitHasAbility[i] = 0
    call UnitRemoveAbility(Units[i], Id)
    endif
        if UnitTimeStill[i] >= TimeStill then
            if UnitHasAbility[i] == 0 then
            call UnitAddAbility(Units[i], Id)
            set UnitHasAbility[i] = 1
            endif
        endif
set i = i+1
endloop
endfunction

//===========================================================================
function InitTrig_Adding_Abilitys takes nothing returns nothing
    local trigger gg_trg_Adding_Abilitys = CreateTrigger()
    call TriggerRegisterTimerEventPeriodic( gg_trg_Adding_Abilitys, 0.20 )
    call TriggerAddAction( gg_trg_Adding_Abilitys, function Trig_Adding_Abilitys_Actions )
    set gg_trg_Adding_Abilitys = null
endfunction

Is this better?
I made it so the array collapses. Also, thanks for the help

 

[Dreadnought[dA]]

Use scope.

scope SpellOfDeath

private function Conditions
//Add *private* in front of your function names inside a scope.
//This allows you to have 5000 functions named the same thing, as long as 
//they are inside different scopes.
//Very useful when you have a lot of custom spells.
endfunction

private function MoreActions
//
//
endfunction

private function Actions
//
//
endfunction

endscope