<Jass> Leak code

[Ratman]

Hello every people. Please help me to find some leak. Sorry for my English. I hope you.

function Dec takes nothing returns nothing
    if (GetDestructableLife(GetEnumDestructable()) >0 ) then
    set udg_Decoration = true
    endif
endfunction

function GroupPick takes nothing returns boolean
    local unit f=GetFilterUnit()
    local real r=GetUnitState(f, UNIT_STATE_LIFE)
    local boolean b=false
    set b=GetBooleanAnd( (r>0), ( IsUnitEnemy(f, GetOwningPlayer(udg_MissileUnit[udg_loopIndex])) == true ) )
    set f=null
    set r=0
    return b
endfunction

function PickUnitDamage takes nothing returns nothing
    local unit p=GetEnumUnit()
    if ( IsUnitInGroup(p, udg_MissileGroupFire[udg_loopIndex]) == false) then
          call UnitDamageTarget( udg_MissileUnit[udg_loopIndex], p, udg_MissileDamage[udg_loopIndex], true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, WEAPON_TYPE_WHOKNOWS )
          call GroupAddUnit( udg_MissileGroupFire[udg_loopIndex],p )
    endif
    set p = null
endfunction

function DamageType1 takes nothing returns nothing
                local group G =CreateGroup() 
                local group G1=CreateGroup() 
                local location l=udg_MisslePoint
                set udg_MisslePoint = PolarLocation(l, udg_MissileSpeed[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
                call GroupEnumUnitsInRangeOfLoc(G,udg_MisslePoint,udg_MissileAOE[udg_loopIndex],Condition(function GroupPick) )
                call GroupEnumUnitsInRangeOfLoc(G1,udg_MisslePoint,udg_MissileAOED[udg_loopIndex],Condition(function GroupPick) )
                if ( FirstOfGroup(G)!= null ) then
                     call ForGroup( G1, function PickUnitDamage)
                            endif
                call DestroyGroup (G)
                set G=null
                call DestroyGroup (G1)
                set G1=null
                call RemoveLocation(l)
                set l=null
endfunction


function DamageType2 takes nothing returns nothing
                local group G =CreateGroup() 
                local location l=udg_MisslePoint
                call GroupEnumUnitsInRangeOfLoc(G, udg_MisslePoint, udg_MissileAOE[udg_loopIndex], Condition(function GroupPick))
                call DestroyBoolExpr(Condition(function GroupPick))
                set udg_MisslePoint = PolarLocation(l, udg_MissileSpeed[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
                if ( FirstOfGroup(G)!= null ) then
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif       
                set udg_Decoration = false
                call EnumDestructablesInRectAll( RectFromCenterSizeBJ(udg_MisslePoint, 200.00, 200.00), function Dec )
                if ( udg_Decoration== true ) then
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif  
                call DestroyGroup (G)
                set G=null
                call RemoveLocation(l)
                set l=null
endfunction

function DamageFinal takes nothing returns nothing
                local group G =CreateGroup()                 
                call GroupEnumUnitsInRangeOfLoc(G, udg_MisslePoint, udg_MissileAOED[udg_loopIndex], Condition(function GroupPick))
                if ( FirstOfGroup(G)!= null ) then
                     call ForGroup( G, function PickUnitDamage)
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif
                call DestroyGroup (G)
                set G=null
endfunction

function Trig_Shot2_Actions takes nothing returns nothing
  local real LifeTarget
  local real alfa
  local real Sinus
    set udg_loopIndex = 1
    loop
        exitwhen udg_loopIndex > udg_Index
        set LifeTarget=GetUnitState(udg_MissileUnit[udg_loopIndex], UNIT_STATE_LIFE)
        if (LifeTarget> 0) then
            set udg_MisslePoint = GetUnitLoc(udg_MissileUnit[udg_loopIndex])
            if (udg_MissileTypeDamage[udg_loopIndex]== 1) then
               call DamageType1()
            endif
           if (udg_MissileTypeDamage[udg_loopIndex]== 2) then
                call DamageType2()
            endif
            call SetUnitPositionLoc( udg_MissileUnit[udg_loopIndex], udg_MisslePoint)
            call SetUnitFacing(  udg_MissileUnit[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
            set udg_MissileDistanse[udg_loopIndex]= udg_MissileDistanse[udg_loopIndex]-udg_MissileSpeed[udg_loopIndex]
            
            if (udg_MissileH[udg_loopIndex]>0) then
                set alfa = ( ( 180.00 * udg_MissileDistanse[udg_loopIndex] ) / udg_MissileDistanseO[udg_loopIndex])
                set Sinus= ( Sin(alfa*(3.14159/180.00)) * udg_MissileH[udg_loopIndex] )
                call SetUnitFlyHeight( udg_MissileUnit[udg_loopIndex], Sinus, 0.00 )
            endif

            if (udg_MissileDistanse[udg_loopIndex]<=0 ) then
                call DamageFinal()
                call KillUnit( udg_MissileUnit[udg_loopIndex] )
                call DestroyGroup (udg_MissileGroupFire[udg_loopIndex])
                set udg_IndexZero = ( udg_IndexZero - 1 )
                if ( udg_IndexZero == 0 ) then
                    call DisableTrigger( gg_trg_Shot2)
                endif
             endif
        endif
        call RemoveLocation(udg_MisslePoint)
        set udg_loopIndex = udg_loopIndex + 1
    endloop
endfunction

//===========================================================================
function InitTrig_Shot2 takes nothing returns nothing
    set gg_trg_Shot2 = CreateTrigger(  )
    call TriggerRegisterTimerEventPeriodic( gg_trg_Shot2, 0.01 )
    call TriggerAddAction( gg_trg_Shot2, function Trig_Shot2_Actions )
endfunction

 

[BPower]

A location is conists of two points. A x coordinate and a y coordinate.
Replace all locations used with coordinates.
In order to get an units coordinates --> GetUnitX(myUnit) and GetUnitY(myUnit)

Your code is far away of beein good, but there are no leaks, if you get rid of all the locations.
In general don't create that many local groups. Create one global (udg_myGroup) and clear (not destroy) it when you're done with it.

To get a unit's life it is recommended to use GetWidgetLife(myUnit)
To check if a unit is dead use IsUnitType(myUnit, UNIT_TYPE_DEAD)

You don't have to define pi manually -- >bj_PI

function GroupPick takes nothing returns boolean
    local unit f=GetFilterUnit()
    local real r=GetUnitState(f, UNIT_STATE_LIFE)
    local boolean b=false
    set b=GetBooleanAnd( (r>0), ( IsUnitEnemy(f, GetOwningPlayer(udg_MissileUnit[udg_loopIndex])) == true ) )
    set f=null
    set r=0
    return b
endfunction

--->

function GroupPick takes nothing returns boolean
    return not IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD) and ( IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(udg_MissileUnit[udg_loopIndex])))
endfunction

Edit this is so messy. I can't answer offhand whether there are leaks or not. :/

 

[deathismyfriend]

Look at my tutorial Converting GUI to Efficient JASS.
It is in my sig below my post.

It will show you how to make efficient clean Jass code. Then after that if you believe you still have a leak post the new script here and we will see if we can find it.

 

[DysfunctionaI]

There's actually a lot of stuff there that needs fixing, we can work on it slowly. Post your code again after you've updated it with what people are saying in this thread, and it will be much cleaner and easier to read.

I'm just going to name a few:

1.)

local real LifeTarget
set LifeTarget=GetUnitState(udg_MissileUnit[udg_loopIndex], UNIT_STATE_LIFE)
    if (LifeTarget> 0) then

Don't create a local if you're only using it once. You have a few of these.

Just call the function directly since you're not using it anywhere else. Also, use GetWidgetLife instead of GetUnitState (as someone else said):

if ( GetWidgetLife( udg_MissileUnit[udg_loopIndex] > .405 ) then

GetWidgetLife is faster, and .405 is more accurate than zero.


2.)

function GroupPick takes nothing returns boolean
    local unit f=GetFilterUnit()
    local real r=GetUnitState(f, UNIT_STATE_LIFE)
    local boolean b=false
    set b=GetBooleanAnd( (r>0), ( IsUnitEnemy(f, GetOwningPlayer(udg_MissileUnit[udg_loopIndex])) == true ) )
    set f=null
    set r=0
    return b
endfunction

Don't need to set r=0 because it is a real.

Don't need the boolean, you would just return GetBooleanAnd()

Actually that whole function should look like this (someone correct me if I'm wrong):

function GroupPick takes nothing returns boolean
    if not ( IsUnitEnemy( GetFilterUnit(), GetOwningPlayer(udg_MissileUnit[udg_loopIndex]) ) ) then
         return false
    elseif GetWidgetLife( GetFilterUnit() ) < .405 then
         return false
    endif

    return true
endfunction

3.)

call SetUnitPositionLoc( udg_MissileUnit[udg_loopIndex], udg_MisslePoint)

Don't use SetUnitPositionLoc(), use SetUnitX and SetUnitY.

As someone else said, don't use locations at all, just reals.

So instead of GetUnitLoc() you would use GetUnitX() GetUnitY()

4.)

Are you positive you need a periodic timer of 0.01 ? In theory, that would run 100 times in 1 second. If that was 0.02, then it would only run 50 times (which is still excessive).

I'm not sure what you're trigger does, but if it still works correctly with a number like 0.03, then I would use that instead, as it would drastically reduce the number of calls.



After you fix up some stuff, repost the code and we can help some more. You'll be writing clean, efficient code in no time!

 

[Bribe]

call EnumDestructablesInRectAll( RectFromCenterSizeBJ(udg_MisslePoint, 200.00, 200.00), function Dec )

This leaks the rect you create.

 

[Ratman]

Thanks for answers. I think I understand that you mean.
Bribe, thanks. Сorrected the error, but leaks have not disappeared( Proposes smaller, but disappeared. I tested this trigger separately, so sure was he proposes.

This trigger implements motion shots. In other words, one trigger creates them, enters into an array of options. This trigger their moves and deals damage. Depending on the type of damage, shots can fly to the nearest enemy or beat through and fly through parabolle.
DysfunctionaI, 3. "Don't use SetUnitPositionLoc(), use SetUnitX and SetUnitY.
As someone else said, don't use locations at all, just reals."
Why location is bad than real?
question: if i want to do "call DamageFinal()" in "Trig_Shot2_Actions", I have to write each time in a loop "set G=CreateGroup()" / "call DestroyGroup (G)"?

attempt 2

function Dec takes nothing returns nothing
    if (GetDestructableLife(GetEnumDestructable()) >.405 ) then
    set udg_Decoration = true
    endif
endfunction

function GroupPick takes nothing returns boolean
    if not ( IsUnitEnemy( GetFilterUnit(), GetOwningPlayer(udg_MissileUnit[udg_loopIndex]) ) ) then
         return false
    elseif GetWidgetLife( GetFilterUnit() ) < .405 then
         return false
    endif
    return true
endfunction

function PickUnitDamage takes nothing returns nothing
    if ( IsUnitInGroup(GetEnumUnit(), udg_MissileGroupFire[udg_loopIndex]) == false) then
          call UnitDamageTarget( udg_MissileUnit[udg_loopIndex], GetEnumUnit(), udg_MissileDamage[udg_loopIndex], true, false, ATTACK_TYPE_CHAOS, DAMAGE_TYPE_UNIVERSAL, WEAPON_TYPE_WHOKNOWS )
          call GroupAddUnit( udg_MissileGroupFire[udg_loopIndex],GetEnumUnit() )
    endif
endfunction

function DamageType1 takes nothing returns nothing
                local group G =CreateGroup() 
                local group G1=CreateGroup() 
                local location l=udg_MisslePoint
                
                set udg_MisslePoint = PolarLocation(l, udg_MissileSpeed[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
                call GroupEnumUnitsInRangeOfLoc(G,udg_MisslePoint,udg_MissileAOE[udg_loopIndex],Condition(function GroupPick) )
                call GroupEnumUnitsInRangeOfLoc(G1,udg_MisslePoint,udg_MissileAOED[udg_loopIndex],Condition(function GroupPick) )
                if ( FirstOfGroup(G)!= null ) then
                     call ForGroup( G1, function PickUnitDamage)
                endif
                call DestroyGroup (G)
                set G=null
                call DestroyGroup (G1)
                set G1=null
                call RemoveLocation(l)
                set l=null
endfunction


function DamageType2 takes nothing returns nothing
                local group G =CreateGroup() 
                local location l=udg_MisslePoint
                local rect r

                call GroupEnumUnitsInRangeOfLoc(G, udg_MisslePoint, udg_MissileAOE[udg_loopIndex], Condition(function GroupPick))
                set udg_MisslePoint = PolarLocation(l, udg_MissileSpeed[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
                set r= Rect(GetLocationX(udg_MisslePoint) - 100, GetLocationY(udg_MisslePoint)  - 100, GetLocationX(udg_MisslePoint) + 100, GetLocationY(udg_MisslePoint)  + 100)
                if ( FirstOfGroup(G)!= null ) then
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif       
                set udg_Decoration = false
                call EnumDestructablesInRect(r, null, function Dec)
                if ( udg_Decoration== true ) then
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif  

                call DestroyGroup (G)
                set G=null
                call RemoveLocation(l)
                set l=null
                call RemoveRect(r)
                set r=null
endfunction

function DamageFinal takes nothing returns nothing
                local group G =CreateGroup()                 
                call GroupEnumUnitsInRangeOfLoc(G, udg_MisslePoint, udg_MissileAOED[udg_loopIndex], Condition(function GroupPick))
                if ( FirstOfGroup(G)!= null ) then
                     call ForGroup( G, function PickUnitDamage)
                     set udg_MissileDistanse[udg_loopIndex]=0
                endif
                call DestroyGroup (G)
                set G=null
endfunction

function Trig_Shot2_Actions takes nothing returns nothing
  local real alfa
  local real Sinus
    set udg_loopIndex = 1
    loop
        exitwhen udg_loopIndex > udg_Index
        if ( GetWidgetLife( udg_MissileUnit[udg_loopIndex] > .405 ) then 
            set udg_MisslePoint = GetUnitLoc(udg_MissileUnit[udg_loopIndex])
            if (udg_MissileTypeDamage[udg_loopIndex]== 1) then
               call DamageType1()
            endif
           if (udg_MissileTypeDamage[udg_loopIndex]== 2) then
                call DamageType2()
            endif
            call GetUnitX(udg_MissileUnit[udg_loopIndex], GetLocationX(udg_MisslePoint) )
            call GetUnitY(udg_MissileUnit[udg_loopIndex], GetLocationY(udg_MisslePoint) )
            call SetUnitFacing(  udg_MissileUnit[udg_loopIndex], udg_MissileAngle[udg_loopIndex])
            set udg_MissileDistanse[udg_loopIndex]= udg_MissileDistanse[udg_loopIndex]-udg_MissileSpeed[udg_loopIndex]
            
            if (udg_MissileH[udg_loopIndex]>.405) then
                set alfa = ( ( 180.00 * udg_MissileDistanse[udg_loopIndex] ) / udg_MissileDistanseO[udg_loopIndex])
                set Sinus= ( Sin(alfa*(bj_PI/180.00)) * udg_MissileH[udg_loopIndex] )
                call SetUnitFlyHeight( udg_MissileUnit[udg_loopIndex], Sinus, 0.00 )
            endif

            if (udg_MissileDistanse[udg_loopIndex]<.405 ) then
                call DamageFinal()
                call KillUnit( udg_MissileUnit[udg_loopIndex] )
                call DestroyGroup (udg_MissileGroupFire[udg_loopIndex])
                set udg_IndexZero = ( udg_IndexZero - 1 )
                if ( udg_IndexZero == 0 ) then
                    call DisableTrigger(  GetTriggeringTrigger() )
                endif
             endif
        endif
        call RemoveLocation(udg_MisslePoint)
        set udg_loopIndex = udg_loopIndex + 1
    endloop
endfunction

//===========================================================================
function InitTrig_Shot2 takes nothing returns nothing
    local trigger t = CreateTrigger() 
    call TriggerRegisterTimerEventPeriodic( t, 0.03 )
    call TriggerAddAction( t, function Trig_Shot2_Actions )
    set t = null
endfunction

 

[edo494]

the GroupPick function can be optimized

function GroupPick takes nothing returns boolean
    return IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(udg_MissileUnit[udg_loopIndex])) and IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD)
endfunction

 

[Ratman]

edo494, This changes things so much?

 

[deathismyfriend]

locations are bad because they leak. They are a lot slower than using reals.
Creating and destroying locations is very costly. Like creating and destroying groups.

There is still a lot of things in your trigger that should be done.
Like i said above you should look at my tutorial.

Also for your question above yes it changes a lot as it is more efficient and faster to run than the old way you had it with the ITE.

You should not use TriggerAddAction use TriggerAddCondition and run everything from the condition block.

 

[chobibo]

Or use a timer instead of using periodic events.

 

[deathismyfriend]

Yes what chobibo said should be changed.

This should also not be used unless dealing with global triggers or triggers you only want to run a certain amount of times.
call DisableTrigger( GetTriggeringTrigger() )

 

[SA Dashie]

You should get into using vJass.
It is recommended to use Jass Newgen, this was you can read and code alot more easier(you might already be using this?).

You can use libraries, with the use of being able to also create your global variables with your code instead of using the variable editor. This was you can not only not need to use the "udg_" prefix to every global variable but also have control with using them variables as private(/public/constant) variables and functions that can only be used within the library.

With the use of libraries you can decide to use whatever function names to have for the initialize function.

You don't have to learn the more complicated things that vJass provides, as using these things mentioned above arn't that hard to learn nor are the more complicated things required just to help yourself easier.

In simple it is just alot more convenient way of organizing your code.

edit: Here's an example of come of what I am talking about in what vJass can look like.
At initialize a timer is started to expire every second untill it is counted 30 times then the timer is destroyed.

library exampleCustomTimer initializer init

globals
    private integer Count = 0
endglobals

private function run take nothing returns nothing
    set Count = Count + 1
    call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, "Count = " + I2S(Count))
    
    if Count == 30 then
        call DestroyTimer(GetExpiredTimer())
    endif
endfunction

private function init takes nothing returns nothing
    call TimerStart(CreateTimer(), true, 1, function run)
endfunction

endlibrary

the "call TimerStart" thing could have it's values around the wrong was, as I'm doing this on a school computer.