Spell Leak Help

[sephiroth482]

This spell lags when casted too often, can someone please tell me where it is lagging? because im not sure how to fix it, here is the code:

scope ChargedArrow

globals
    private constant integer SpellAbilityId='A006'
    private constant integer ArrowUnitId='e001'
    private constant real TimerInterval=0.03
    private constant real ArrowMoveSpeed=30
    private constant real MaxArrowDist=1800
    private constant real ArrowAOE=200
    private constant string DamageEffectString="Objects\\Spawnmodels\\Critters\\Albatross\\CritterBloodAlbatross.mdl"
endglobals

private struct data
    unit Caster
    unit Arrow
    location CasterLoc
    location TargetLoc
    real FacingAngle
endstruct

private function PickedUnitConditions takes nothing returns boolean
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    set t=null
    return b1 and b2 and b3 and b4
endfunction

private function MoveArrow takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local unit Target
    local group g
    local location ArrowLoc=GetUnitLoc(d.Arrow)
    call CS_MoveUnitLoc(d.Arrow, PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle))
    set g=GetUnitsInRangeOfLocMatching(ArrowAOE,ArrowLoc,Condition(function PickedUnitConditions))
    loop
        set Target = FirstOfGroup(g)
        exitwhen Target == null
        call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
        call RemoveUnit(d.Arrow)
        call GroupRemoveUnit(g, Target)
        call DestroyGroup(g)
        set Target=null
    endloop
    call RemoveLocation(GetUnitLoc(d.Arrow))
    call RemoveLocation(ArrowLoc)
    call DestroyGroup(g)
    set Target=null
    set ArrowLoc=null
    set g=null
    set t=null
endfunction

private function ChargedArrowActions takes nothing returns nothing
    local timer t=CreateTimer()
    local data d=data.create()
    local location ArrowOrigin
    local location ArrowLoc
    local real ArrowCurrentDist
    set d.Caster=GetTriggerUnit()
    set d.CasterLoc=GetUnitLoc(d.Caster)
    set d.TargetLoc=GetSpellTargetLoc()
    set d.FacingAngle=AngleBetweenPoints(d.CasterLoc,d.TargetLoc)
    set d.Arrow=CreateUnitAtLoc(GetOwningPlayer(d.Caster),ArrowUnitId,d.CasterLoc,d.FacingAngle)
    set ArrowOrigin=GetUnitLoc(d.Caster)
    call AttachInt(t,"d",d)
    call TimerStart(t, TimerInterval,true,function MoveArrow)
    loop
        set ArrowLoc=GetUnitLoc(d.Arrow)
        set ArrowCurrentDist=DistanceBetweenPoints(ArrowOrigin,ArrowLoc)
        call RemoveLocation(GetUnitLoc(d.Arrow))
        call RemoveLocation(ArrowLoc)
        exitwhen ArrowCurrentDist>=MaxArrowDist or d.Arrow==null
        call TriggerSleepAction(0)
    endloop
    call RemoveUnit(d.Arrow)
    call RemoveLocation(GetUnitLoc(d.Caster))
    call RemoveLocation(GetSpellTargetLoc())
    call RemoveLocation(ArrowOrigin)
    call CleanAttachedVars(t)
    call DestroyTimer(t)
    call data.destroy(d)
    set ArrowOrigin=null
    set ArrowLoc=null
    set t=null
endfunction

function ChargedArrowConditions takes nothing returns boolean
    return GetSpellAbilityId()==SpellAbilityId
endfunction

function InitTrig_Charged_Arrow takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition( t, Condition( function ChargedArrowConditions))
    call TriggerAddAction(t, function ChargedArrowActions)
endfunction

endscope

Thanks

 

[Builder Bob]

You've made a good effort in sealing leaks, but there are some things that could have been done differently.

Here's a first draft on some suggestions. I've not changed a whole lot, because I am not entirely sure how you want the arrow to function.

With these changes, the arrow will go through and damage all units until it gets farther than MaxArrowDist. There are some places in your timer function where it looks like you want the arrow to only hit the first unit on it's path. If that is what you are aiming for, I would change a whole lot more to avoid double removal of units.

You should also move the code that measures the distance the arrow has traveled into your timer function.
If you're not very against using coordinates, that's also something that would improve your spell.

scope ChargedArrow

globals
    private constant integer SpellAbilityId='A006'
    private constant integer ArrowUnitId='e001'
    private constant real TimerInterval=0.03
    private constant real ArrowMoveSpeed=30
    private constant real MaxArrowDist=1800
    private constant real ArrowAOE=200
    private constant string DamageEffectString="Objects\\Spawnmodels\\Critters\\Albatross\\CritterBloodAlbatross.mdl"
endglobals

private struct data
    unit Caster
    unit Arrow
    location CasterLoc
    location TargetLoc
    real FacingAngle
endstruct

private function PickedUnitConditions takes nothing returns boolean
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    set t=null
    return b1 and b2 and b3 and b4
endfunction

private function MoveArrow takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local unit Target
    local group g
    local location ArrowLoc=GetUnitLoc(d.Arrow)
    call CS_MoveUnitLoc(d.Arrow, PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle))
    set g=GetUnitsInRangeOfLocMatching(ArrowAOE,ArrowLoc,Condition(function PickedUnitConditions))
    loop
        set Target = FirstOfGroup(g)
        exitwhen Target == null
        call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
        //call RemoveUnit(d.Arrow) //removing the arrow here will cause the next GetUnitLoc(d.Arrow) to get problems, since the arrow no longer exists
        //I realize that you might want the arrow to only hit one unit, and then be removed. If that is the case, remove the loop and stop the timer and remove arrow when FirstOfGroup(g) != null
        call GroupRemoveUnit(g, Target)
        //call DestroyGroup(g) //destroying this group here causes the next FirstOfGroup(g) call to return null, since the group no longer exists
        set Target=null
    endloop
    //call RemoveLocation(GetUnitLoc(d.Arrow))
    call RemoveLocation(ArrowLoc)
    call DestroyGroup(g)
    set Target=null
    set ArrowLoc=null
    set g=null
    set t=null
endfunction

private function ChargedArrowActions takes nothing returns nothing
    local timer t=CreateTimer()
    local data d=data.create()
    local location ArrowOrigin
    local location ArrowLoc
    local real ArrowCurrentDist
    set d.Caster=GetTriggerUnit()
    set d.CasterLoc=GetUnitLoc(d.Caster)
    set d.TargetLoc=GetSpellTargetLoc()
    set d.FacingAngle=AngleBetweenPoints(d.CasterLoc,d.TargetLoc)
    set d.Arrow=CreateUnitAtLoc(GetOwningPlayer(d.Caster),ArrowUnitId,d.CasterLoc,d.FacingAngle)
    set ArrowOrigin=GetUnitLoc(d.Caster)
    call AttachInt(t,"d",d)
    call TimerStart(t, TimerInterval,true,function MoveArrow)
    loop
        set ArrowLoc=GetUnitLoc(d.Arrow)
        set ArrowCurrentDist=DistanceBetweenPoints(ArrowOrigin,ArrowLoc)
        //call RemoveLocation(GetUnitLoc(d.Arrow)) //this creates and removes a location, and effectively does nothing
        call RemoveLocation(ArrowLoc)
        exitwhen ArrowCurrentDist>=MaxArrowDist or d.Arrow==null
        call TriggerSleepAction(0)
    endloop
    call RemoveUnit(d.Arrow)
    //call RemoveLocation(GetUnitLoc(d.Caster))
    //call RemoveLocation(GetSpellTargetLoc())
    call RemoveLocation(ArrowOrigin)
    call CleanAttachedVars(t)
    call PauseTimer(t) //pause all repeating timers before you destroy them. In some occations a repeating timer may expire even after it is destroyed.
    call DestroyTimer(t)
    call data.destroy(d)
    set ArrowOrigin=null
    set ArrowLoc=null
    set t=null
endfunction

function ChargedArrowConditions takes nothing returns boolean
    return GetSpellAbilityId()==SpellAbilityId
endfunction

function InitTrig_Charged_Arrow takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition( t, Condition( function ChargedArrowConditions))
    call TriggerAddAction(t, function ChargedArrowActions)
endfunction

endscope

 

[sephiroth482]

Thanks for the Quick Response, and for explaining it so thoroughly. I changed the code a bit, and the lag was reduced greatly, but it still lags a little. Sorry for the trouble but is this code any better and if you could help me fix the problems in this one or give me any more suggestions or advice? Oh and sorry i didnt say wat i needed the arrow to do, but i wanted the arrow to dissappear after hitting the first unit in it's path.

scope ChargedArrow

globals
    private constant integer SpellAbilityId='A006'
    private constant integer ArrowUnitId='e001'
    private constant real TimerInterval=0.03
    private constant real ArrowMoveSpeed=30
    private constant real MaxArrowDist=1800
    private constant real ArrowAOE=200
endglobals

private struct data
    unit Caster
    unit Arrow
    location CasterLoc
    location TargetLoc
    real FacingAngle
endstruct

function PickedUnitConditions takes nothing returns boolean
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    return b1 and b2 and b3 and b4
endfunction

function MoveArrow takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local unit Target
    local group g
    local location ArrowLoc=GetUnitLoc(d.Arrow)
    call CS_Error(GetOwningPlayer(d.Caster),"Blah")
    if DistanceBetweenPoints(d.CasterLoc,ArrowLoc)>=MaxArrowDist then
        call RemoveUnit(d.Arrow)
        call DestroyGroup(g)
        call PauseTimer(t)
        call CleanAttachedVars(t)
        call DestroyTimer(t)
        call data.destroy(d)
        set t=null
    else
        call CS_MoveUnitLoc(d.Arrow, PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle))
        set g=GetUnitsInRangeOfLocMatching(ArrowAOE,ArrowLoc,Condition(function PickedUnitConditions))
        set Target = FirstOfGroup(g)
        if FirstOfGroup(g) !=null then
            call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
            call RemoveUnit(d.Arrow)
            call DestroyGroup(g)
            call PauseTimer(t)
            call CleanAttachedVars(t)
            call DestroyTimer(t)
            call data.destroy(d)
            set t=null
        endif
    endif
    call RemoveLocation(ArrowLoc)
    call DestroyGroup(g)
    set Target=null
    set ArrowLoc=null
    set g=null
endfunction

function ChargedArrowActions takes nothing returns nothing
    local timer t=CreateTimer()
    local data d=data.create()
    set d.Caster=GetTriggerUnit()
    set d.CasterLoc=GetUnitLoc(d.Caster)
    set d.TargetLoc=GetSpellTargetLoc()
    set d.FacingAngle=AngleBetweenPoints(d.CasterLoc,d.TargetLoc)
    set d.Arrow=CreateUnitAtLoc(GetOwningPlayer(d.Caster),ArrowUnitId,d.CasterLoc,d.FacingAngle)
    call AttachInt(t,"d",d)
    call TimerStart(t, TimerInterval,true,function MoveArrow)
    set t=null
endfunction

function ChargedArrowConditions takes nothing returns boolean
    return GetSpellAbilityId()==SpellAbilityId
endfunction

function InitTrig_Charged_Arrow takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition( t, Condition( function ChargedArrowConditions))
    call TriggerAddAction(t, function ChargedArrowActions)
endfunction

endscope

 

[AdamGriffith]

Are you sure it works as designed?

    loop
        set Target = FirstOfGroup(g)
        exitwhen Target == null
        call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
        call RemoveUnit(d.Arrow)
        call GroupRemoveUnit(g, Target)
        call DestroyGroup(g)
        set Target=null
    endloop

Your looping but after the first 'iteration' the group is destroyed. So there is not point in the loop at all...

Unless I'm missing something.

 

[sephiroth482]

Yah it works fine now, the newer code i posted is just above your post. I fixed that mistake and changed the code to work so that the arrow would dissappear after hitting one unit, but i found that the timer keeps running even though i put it to pause. Even though i set another loop in the main function to exitwhen d.ArrowGone <-(Boolean)==true, then i pause the timer and destroy it, it still lags even though i paused and destroyed timer.

scope ChargedArrow

globals
    private constant integer SpellAbilityId='A006'
    private constant integer ArrowUnitId='e001'
    private constant real TimerInterval=0.03
    private constant real ArrowMoveSpeed=30
    private constant real MaxArrowDist=1800
    private constant real ArrowAOE=200
    private constant string DamageEffectString="Objects\\Spawnmodels\\Critters\\Albatross\\CritterBloodAlbatross.mdl"
endglobals

private struct data
    unit Caster
    unit Arrow
    location CasterLoc
    location TargetLoc
    real FacingAngle
    boolean ArrowGone
endstruct

private function PickedUnitConditions takes nothing returns boolean
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    return b1 and b2 and b3 and b4
endfunction

private function MoveArrow takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local unit Target
    local group g
    local location ArrowLoc=GetUnitLoc(d.Arrow)
    if DistanceBetweenPoints(d.CasterLoc,ArrowLoc)>=MaxArrowDist then
        set d.ArrowGone=true
    else
        call CS_MoveUnitLoc(d.Arrow, PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle))
        set g=GetUnitsInRangeOfLocMatching(ArrowAOE,ArrowLoc,Condition(function PickedUnitConditions))
        set Target = FirstOfGroup(g)
        if FirstOfGroup(g) !=null then
            call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
            set d.ArrowGone=true
        endif
    endif
    call RemoveLocation(ArrowLoc)
    call DestroyGroup(g)
    set Target=null
    set ArrowLoc=null
    set g=null
    set t=null
endfunction

private function ChargedArrowActions takes nothing returns nothing
    local timer t=CreateTimer()
    local data d=data.create()
    set d.Caster=GetTriggerUnit()
    set d.CasterLoc=GetUnitLoc(d.Caster)
    set d.TargetLoc=GetSpellTargetLoc()
    set d.FacingAngle=AngleBetweenPoints(d.CasterLoc,d.TargetLoc)
    set d.Arrow=CreateUnitAtLoc(GetOwningPlayer(d.Caster),ArrowUnitId,d.CasterLoc,d.FacingAngle)
    set d.ArrowGone=false
    call AttachInt(t,"d",d)
    call TimerStart(t, TimerInterval,true,function MoveArrow)
    loop 
        exitwhen d.ArrowGone==true
        call TriggerSleepAction(0)
    endloop
    call RemoveUnit(d.Arrow)
    call PauseTimer(t)
    call CleanAttachedVars(t)
    call DestroyTimer(t)
    call data.destroy(d)
    set t=null
endfunction

function ChargedArrowConditions takes nothing returns boolean
    return GetSpellAbilityId()==SpellAbilityId
endfunction

function InitTrig_Charged_Arrow takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition( t, Condition( function ChargedArrowConditions))
    call TriggerAddAction(t, function ChargedArrowActions)
endfunction

endscope

Even after doing this it still lags the game after a while. Am i forgetting something? or should i do something different? If so could someone show me how to do it?

 

[Builder Bob]

Here are some changes you could do to save yourself from doing the TriggerSleepAction, and seal a location leak.

You had a leak in PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle), and you were sometimes calling DestroyGroup(null). I don't know if destroying null actually is a problem or not, but I fixed it anyway.

scope ChargedArrow

globals
    private constant integer SpellAbilityId='A006'
    private constant integer ArrowUnitId='e001'
    private constant real TimerInterval=0.03
    private constant real ArrowMoveSpeed=30
    private constant real MaxArrowDist=1800
    private constant real ArrowAOE=200
    private constant string DamageEffectString="Objects\\Spawnmodels\\Critters\\Albatross\\CritterBloodAlbatross.mdl"
endglobals

private struct data
    unit Caster
    unit Arrow
    location CasterLoc
    location TargetLoc
    real FacingAngle
    boolean ArrowGone
endstruct

private function PickedUnitConditions takes nothing returns boolean
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    return b1 and b2 and b3 and b4
endfunction

private function MoveArrow takes nothing returns nothing
    local timer t=GetExpiredTimer()
    local data d=data(GetAttachedInt(t,"d"))
    local unit Target
    local group g
    local location ArrowLoc=GetUnitLoc(d.Arrow)
    local location ArrowNewLoc=PolarProjectionBJ(ArrowLoc, ArrowMoveSpeed, d.FacingAngle)
    call CS_MoveUnitLoc(d.Arrow, ArrowNewLoc)
    set g=GetUnitsInRangeOfLocMatching(ArrowAOE,ArrowLoc,Condition(function PickedUnitConditions))
    set Target = FirstOfGroup(g)
    if Target != null then
        call UnitDamageTargetBJ(d.Caster,Target,GetRandomReal(400.00, 1200.00), ATTACK_TYPE_HERO, DAMAGE_TYPE_MAGIC)
    endif
    if Target != null or DistanceBetweenPoints(d.CasterLoc,ArrowNewLoc)>=MaxArrowDist then
        call RemoveUnit(d.Arrow)
        call PauseTimer(t)
        call CleanAttachedVars(t)
        call DestroyTimer(t)
        call data.destroy(d)
    endif
    call DestroyGroup(g)
    call RemoveLocation(ArrowLoc)
    call RemoveLocation(ArrowNewLoc)
    set Target=null
    set ArrowLoc=null
    set ArrowNewLoc=null
    set g=null
    set t=null
endfunction

private function ChargedArrowActions takes nothing returns nothing
    local timer t=CreateTimer()
    local data d=data.create()
    set d.Caster=GetTriggerUnit()
    set d.CasterLoc=GetUnitLoc(d.Caster)
    set d.TargetLoc=GetSpellTargetLoc()
    set d.FacingAngle=AngleBetweenPoints(d.CasterLoc,d.TargetLoc)
    set d.Arrow=CreateUnitAtLoc(GetOwningPlayer(d.Caster),ArrowUnitId,d.CasterLoc,d.FacingAngle)
    call AttachInt(t,"d",d)
    call TimerStart(t, TimerInterval,true,function MoveArrow)
    set t=null
endfunction

function ChargedArrowConditions takes nothing returns boolean
    return GetSpellAbilityId()==SpellAbilityId
endfunction

function InitTrig_Charged_Arrow takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CAST)
    call TriggerAddCondition( t, Condition( function ChargedArrowConditions))
    call TriggerAddAction(t, function ChargedArrowActions)
endfunction

endscope

 

[sephiroth482]

O i see, okay i get it now lol. Thanks alot works perfectly now.

 

[Builder Bob]

oh, one more leak fix:

null t in your PickedUnitConditions function

or

private function PickedUnitConditions takes nothing returns boolean
    local data d=data(GetAttachedInt(GetExpiredTimer(),"d"))
    local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    return b1 and b2 and b3 and b4
endfunction

 

[sephiroth482]

Oh okay thanks, never knew i had to nulll that one too. K thanks for all your help appreciate it.

 

[Xorifelse]

private function PickedUnitConditions takes nothing returns boolean
    local data     d = data(GetAttachedInt(GetExpiredTimer(),"d"))
    local unit     u = GetFilterUnit()
    local boolean b1 = GetWidgetLife( u ) > 0.405
    local boolean b2 = IsUnitEnemy( u, GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType( u, UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType( u, UNIT_TYPE_STRUCTURE) == false
    
    set u = null

    return b1 and b2 and b3 and b4
endfunction

This is just a little faster then that you have in your code.
It saves about 3 native calls, and I removed a BJ...

Throughout your code, you use locations, avoid using them and use natives instead.
Locations leak and they are slow and can cause lagg.
Use the x, y real variables, much faster..

The only thing you should use locations for is
- Getting the x and y from "GetSpellTargetLoc()"
- Getting the z from the terrain ground using "GetLocationZ()"

 

[saw792]

Locations cause lag? Interesting, when it is perfectly simple to remove the leaks.

While I agree that x- and y-coordinates are better, I believe that you are incorrect saying locations are bad. The only real advantage is in the method of manipulating them, and in the lack of leaks to remove.

To say 'Locations are slow' is misleading.

 

[Vassilev]

   local boolean b1 = IsUnitAliveBJ(GetFilterUnit()) == true
    local boolean b2 = IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(d.Caster)) == true
    local boolean b3 = IsUnitType(GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE) == false
    local boolean b4 = IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false
    set t=null
    return b1 and b2 and b3 and b4

Just curious, why do you need four seperate booleans? why not put them all in one line and one boolean? Or.. that only works for a boolexpr? whats the difference anyway

 

[saw792]

It works fine in one line. It just looks ugly, and is harder to read.

A boolexpr is a variable type that is generated from a function that returns a boolean.

 

[Vassilev]

oic, thanks =)

 

[Xorifelse]

Locations cause lag? Interesting, when it is perfectly simple to remove the leaks.

While I agree that x- and y-coordinates are better, I believe that you are incorrect saying locations are bad. The only real advantage is in the method of manipulating them, and in the lack of leaks to remove.

To say 'Locations are slow' is misleading.

It is just as I say it is, while you can remove the leak it sill causes more lag then using the x and y method.
Not to mention because you have to remove the location you call yet an extra unnecessary native.
And if you perhaps use a local location you still need to null it, thus an extra line of coding.

It is still causing more lag then using the "real" variables.
And in real is mean the types of: "integer", "real", "boolean".
While a "string" and a "code" type should in this case be on the list I do not name them because a "code" does not really exists in programming.
And a "string" type behaves more like an object in Jass then a real string.

Everything else including locations are objects.

Objects take more space in the memory then a "real, integer, boolean" variables types. They also take longer to define and create.
See objects like vJass structs.

struct location
    real x
    real y
    real z
endstruct

While you only use the x and the y of the location ( polar projection ) the z is really unnecessary and just fills your computers memory with crap that you don't use.

Locations are made just for GUI, to make it more easier to create triggers.
But my guess is what natives like "GetUnitLoc" really does is:

native GetUnitLoc takes unit u returns location
    return Location( GetUnitX(u), GetUnitY(u) )
endnative

Though, it doesn't really work like that but it should give you some insight.
To sum it all up, locations are bad and stop using them.
Only use them for the 2 things what I said before, and nothing else.

 

[saw792]

while you can remove the leak it sill causes more lag...
It is still causing more lag...
And if you perhaps use a local location you still need to null it, thus an extra line of coding...

You still missed the entire point of my post. I understand how objects work. You don't understand what 'lag' is. Lag occurs when one computer fails to have the same information as the host computer, and thus the host computer waits for it to catch up. In single player, it occurs when the computer is undergoing too many operations in too short a time space, thus the game must wait for the processor to catch up (or read from memory, or whatever). Lag occurs when a threshold is reached, not as a gradually increasing function.

Using locations does not create lag. Just because a map uses locations does not mean that a computer using that map will not be able to handle the commands it is given. Locations may be slower (by a few nanoseconds) and use up more memory temporarily, (as the information is removed when correctly dealt with, and if there is not enough space for you to store that tiny bit of extra information you have bigger problems than using reals will help to solve), but they do not directly create lag. That is why I say your previous point was 'misleading', not 'wrong'.

As far as taking up an extra line goes, using two reals will require two lines for declaration, and two lines for every operation performed on them. So that last argument won't hold water.