[JASS] will this spell leak?

[m4st4]

hi, im kinda new, can anyone plz tell me wether this spell will leak

its a supersimple teleportation spell, but if you have any suggestions how to optimize code/make it faster ect. its more than welcome!!!

function Trig_elePort_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A006' ) ) then
        return false
    endif
    return true
endfunction


function elePortFire takes nothing returns nothing
//retrieving target data
    local timer tEp=GetExpiredTimer()
    local real xxx = GetAttachedReal(tEp, "targetx")
    local real yyy = GetAttachedReal(tEp, "targety")
    local unit uEpCaster=GetAttachedUnit(tEp, "caster")
//fire effects
    call AddSpecialEffectLocBJ( GetUnitLoc(uEpCaster), "Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl" )//effect
    call AddSpecialEffectLocBJ( Location(xxx,yyy), "Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl" )//effect
    call SetUnitPositionLoc( uEpCaster, Location(xxx,yyy) )//port
//prevent leaks
    call FlushHandle(tEp)
    call DestroyTimer(tEp)
    set tEp=null
    set uEpCaster=null
    set xxx=0.00
    set yyy=0.00
endfunction


function Trig_elePort_Actions takes nothing returns nothing
//getting target data, init timer
    local real xxx=GetLocationX(GetSpellTargetLoc())
    local real yyy=GetLocationY(GetSpellTargetLoc())
    local unit uEpCaster=GetTriggerUnit()
    local timer tEp=CreateTimer()
//storing target data
    call AttachReal( tEp, "targetx", xxx)
    call AttachReal( tEp, "targety", yyy)
    call AttachObject( tEp, "caster", uEpCaster)
//waiting for spell to finish (catching cooldown) then firering
    call TimerStart(tEp, .05, false, function elePortFire)
//preventing leaks
    set uEpCaster=null
    set xxx=0.00
    set yyy=0.00    
endfunction


//===========================================================================
function InitTrig_elePort takes nothing returns nothing
    set gg_trg_elePort = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_elePort, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_elePort, Condition( function Trig_elePort_Conditions ) )
    call TriggerAddAction( gg_trg_elePort, function Trig_elePort_Actions )
endfunction

ty in advance

 

[PurplePoot]

The condition can be optimized:

function Trig_elePort_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A006'
endfunction

You leak two locations (GetSpellTargetLoc() twice) in Trig_elePort_Actions. Also, you don't need to "null" reals.

You also leak three locations and two effects in elePortFire. You can remove location usage by just using X/Y natives (Example: native SetUnitPosition takes unit whichUnit, real newX, real newY returns nothing). Effects can simply be destroyed upon creation via call DestroyEffect(AddSpecialEffect(...)).

 

[Earth-Fury]

Problem 1:

function Trig_elePort_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A006' ) ) then
        return false
    endif
    return true
endfunction

The GUI writes horrible code... you should learn how to code from scratch. Anyway, removing all the useless crap from that funtion leaves you with:

function Trig_elePort_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A006'
endfunction

Problem 2:

    call AddSpecialEffectLocBJ( GetUnitLoc(uEpCaster), "Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl" )//effect
call AddSpecialEffectLocBJ( Location(xxx,yyy), "Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl" )//effect

Ignoring the fact that you're using locations, that function is a "BJ" functions. (Functions from blizzard.j, which contains most of the functions the GUI uses, and is coded in JASS.)

Heres that functions code:

function AddSpecialEffectLocBJ takes location where, string modelName returns effect
    set bj_lastCreatedEffect = AddSpecialEffectLoc(modelName, where)
    return bj_lastCreatedEffect
endfunction

So, those two lines set up to use the native (A native is a function that is implemented in the game engine itself, as opposed to in JASS) gives:

function elePortFire takes nothing returns nothing
    local timer tEp = GetExpiredTimer()
    local real xxx = GetAttachedReal(tEp, "targetx")
    local real yyy = GetAttachedReal(tEp, "targety")
    local unit uEpCaster = GetAttachedUnit(tEp, "caster")
    
    call AddSpecialEffectLoc("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitLoc(uEpCaster))
    call AddSpecialEffectLoc("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", Location(xxx,yyy))

    call SetUnitPositionLoc( uEpCaster, Location(xxx,yyy) )
    
    call FlushHandle(tEp)
    call DestroyTimer(tEp)
    set tEp=null
    set uEpCaster=null
    set xxx=0.00
    set yyy=0.00
endfunction

Problem 3: You never destroy those two SFX. For most special effect models, the following will work:

    call DestroyEffect(AddSpecialEffectLoc("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitLoc(uEpCaster)))
    call DestroyEffect(AddSpecialEffectLoc("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", Location(xxx,yyy)))

Problem 4: You use locations in the above... Every function, except GetLocationZ() and GetSpellTargetLoc() have an equivalent or a pair of equivalent functions which return X and Y coordinates. EG: GetUnitLoc(), and GetUnitX() GetUnitY(). Fixing the code to not use locations in that function:

    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitX(uEpCaster), GetUnitY(uEpCaster))
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", xxx, yyy))

Problem 5:

    set xxx=0.00
    set yyy=0.00

I suggest you read: http://www.hiveworkshop.com/forums/showpost.php?p=688477&postcount=12

Problem 6: call SetUnitPositionLoc(uEpCaster, Location(xxx,yyy)) Another location. Considering you don't destroy any of these locations you're using, your code does leak. A lot. However, you don't have to use locations at all... Fixing that to not use locations yeilds: call SetUnitPosition(uEpCaster, xxx, yyy)

Problem 7: Your variable names suck, and your code is very... messy... generally cleaning it up:

function Trig_elePort_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A006'
endfunction

function elePortFire takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local real x = GetAttachedReal(t, "targetx")
    local real y = GetAttachedReal(t, "targety")
    local unit caster = GetAttachedUnit(t, "caster")
    
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitX(caster), GetUnitY(caster))
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", x, y))

    call SetUnitPosition(uEpCaster, x, y)
    
    call FlushHandle(t)
    call DestroyTimer(t)
    
    set t = null
    set caster = null
endfunction


function Trig_elePort_Actions takes nothing returns nothing
    local real x = GetLocationX(GetSpellTargetLoc())
    local real y = GetLocationY(GetSpellTargetLoc())
    local unit caster = GetTriggerUnit()
    local timer t = CreateTimer()
    
    call AttachReal(t, "targetx", x)
    call AttachReal(t, "targety", y)
    call AttachObject(t, "caster", caster)
    
    call TimerStart(t, .05, false, function elePortFire)
    
    set caster=null
endfunction

function InitTrig_elePort takes nothing returns nothing
    set gg_trg_elePort = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_elePort, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_elePort, Condition(function Trig_elePort_Conditions))
    call TriggerAddAction(gg_trg_elePort, function Trig_elePort_Actions)
endfunction

Problem 8:

    local real x = GetLocationX(GetSpellTargetLoc())
    local real y = GetLocationY(GetSpellTargetLoc())

Two location leaks. There is no X/Y equivalent for that function, so we actually have to use a location here. But when you create a location, you must destroy it. So:

function Trig_elePort_Actions takes nothing returns nothing
    local location l = GetSpellTargetLoc()
    local real x = GetLocationX(l)
    local real y = GetLocationY(l)
    local unit caster = GetTriggerUnit()
    local timer t = CreateTimer()
    
    call AttachReal(t, "targetx", x)
    call AttachReal(t, "targety", y)
    call AttachObject(t, "caster", caster)
    
    call TimerStart(t, .05, false, function elePortFire)
    
    call RemoveLocation(l)
    
    set caster=null
    set l = null
endfunction

Problem 9: You don't null the timer's local in that function.

Problem 10: Your handle vars should be prefixed with the spell's "name", so that they won't collide with ones set by another spell / system. So going from:

call AttachReal(t, "targetx", x)

To:

call AttachReal(t, "elePort_targetx", x)

All around gives:

function elePort_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A006'
endfunction

function elePort_Fire takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local real x = GetAttachedReal(t, "elePort_targetx")
    local real y = GetAttachedReal(t, "elePort_targety")
    local unit caster = GetAttachedUnit(t, "elePort_caster")
    
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitX(caster), GetUnitY(caster))
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", x, y))

    call SetUnitPosition(caster, x, y)
    
    call FlushHandle(t)
    call DestroyTimer(t)
    
    set t = null
    set caster = null
endfunction


function elePort_Actions takes nothing returns nothing
    local location l = GetSpellTargetLoc()
    local real x = GetLocationX(l)
    local real y = GetLocationY(l)
    local unit caster = GetTriggerUnit()
    local timer t = CreateTimer()
    
    call AttachReal(t, "elePort_targetx", x)
    call AttachReal(t, "elePort_targety", y)
    call AttachObject(t, "elePort_caster", caster)
    
    call TimerStart(t, .05, false, function elePort_Fire)
    
    call RemoveLocation(l)
    
    set caster=null
    set l = null
    set t = null
endfunction

function InitTrig_elePort takes nothing returns nothing
    set gg_trg_elePort = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_elePort, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_elePort, Condition(function elePort_Conditions))
    call TriggerAddAction(gg_trg_elePort, function elePort_Actions)
endfunction

Problem 11: Unless you will be turning the trigger on / off at run time, the InitTrig function could be done much cleaner.

Fixing anything I missed, this leaves you with:

function elePort_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A006'
endfunction

function elePort_Fire takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local real x = GetAttachedReal(t, "elePort_targetx")
    local real y = GetAttachedReal(t, "elePort_targety")
    local unit caster = GetAttachedUnit(t, "elePort_caster")
    
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", GetUnitX(caster), GetUnitY(caster)))
    call DestroyEffect(AddSpecialEffect("Abilities\\Spells\\Human\\FlakCannons\\FlakTarget.mdl", x, y))

    call SetUnitPosition(caster, x, y)
    
    call FlushHandle(t)
    call DestroyTimer(t)
    
    set t = null
    set caster = null
endfunction


function elePort_Actions takes nothing returns nothing
    local location l = GetSpellTargetLoc()
    local real x = GetLocationX(l)
    local real y = GetLocationY(l)
    local unit caster = GetTriggerUnit()
    local timer t = CreateTimer()
    
    call AttachReal(t, "elePort_targetx", x)
    call AttachReal(t, "elePort_targety", y)
    call AttachObject(t, "elePort_caster", caster)
    
    call TimerStart(t, .05, false, function elePort_Fire)
    
    call RemoveLocation(l)
    
    set caster=null
    set l = null
    set t = null
endfunction

function InitTrig_elePort takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function elePort_Conditions))
    call TriggerAddAction(t, function elePort_Actions)
endfunction

I suggest you begin using the NewGen editor, if you are not already. (Link in my sig.) It includes a lot the default WE does not, and it doesn't crash when you make certain kinds of syntax errors!

As well, if you use NewGen, you can use vJASS. (vJASS is an extension to the JASS language, which, when you save in NewGen, is converted to normal JASS. vJASS is fully compatible with default Warcraft 3.) You can learn more about vJASS by reading the vJASS manual: http://wc3campaigns.net/vexorian/jasshelpermanual.html

 

[PurplePoot]

And you said Silv needs to get laid, EF...

Also, why do you care so much about global vs local triggers in InitTrig?

 

[m4st4]

thanx a lot for taking so much time to reply, that really helped me to understand a lot more of whats going on!!!

and that local trigger doesnt count as a handle? so no set t=null?

 

[PurplePoot]

m4st4, nulling only matters when the object is destroyed, and a permanent trigger will never be.