Help with fixing leaks pls

03-11-2007, 10:51 AM

Well, here is anX again, asking whether someone could help me with fixing leaks because i dont find any anymore. So, my code teleports the hero and damages the targets, any optimizing might be cool :>. Well, here is my code, where i do know from that it leaks? I ve tested it in my "spelltestmap" and after casting it ~ 20 times the performance was very - erm - mad. So, pls help me fixing that,

best wishes

anX

Code:

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


function Time_Void_DmgTargets takes nothing returns nothing
local player p
local real dmg

                                                                          
set p = GetOwningPlayer( udg_TimeVoid_Caster )
set dmg = 50.00
call UnitDamageTargetBJ( udg_TimeVoid_Caster, GetEnumUnit(), dmg, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL )
call GroupAddUnit	(udg_TimeVoid_Targets, GetEnumUnit())
call AddSpecialEffectTarget (GetAbilityEffectById('A01P', EFFECT_TYPE_AREA_EFFECT, 2), GetEnumUnit(),"chest")
call DestroyEffectBJ( GetLastCreatedEffectBJ() )
set p = null
endfunction


function Trig_Time_Void_conds takes nothing returns boolean
local player p
set p = GetOwningPlayer( udg_TimeVoid_Caster )

if IsPlayerAlly( p, GetOwningPlayer(GetFilterUnit())) then
   return false
elseif IsUnitType( GetFilterUnit(), UNIT_TYPE_STRUCTURE) then
   return false
elseif IsUnitDeadBJ(GetFilterUnit()) then
   return false
elseif IsUnitInGroup (GetFilterUnit(), udg_TimeVoid_Targets)then
   return false
endif

return true
endfunction

function Trig_Time_Void_expi takes nothing returns nothing
        local rect rs
        local group g
        local boolexpr filter = Condition(function Trig_Time_Void_conds) 
        local location p

        set g = CreateGroup ()
        set udg_r = ( udg_TimeVoid_dist * udg_TimeVoid_laufinteger )
        set p = PolarProjectionBJ (udg_TimeVoid_Casterpos, udg_r, udg_TimeVoid_deg)
        set rs = RectFromCenterSizeBJ(p, 150.00, 150.00)        
        call SetUnitPositionLoc	 (udg_TimeVoid_Caster,  p)     
        
        if ( udg_r >= udg_TimeVoid_Enddist ) then
        call PauseTimerBJ( true, udg_TimeVoid_Timer )       
    else 
    endif
    set udg_TimeVoid_laufinteger = (udg_TimeVoid_laufinteger+1)  
    call RemoveLocation (p)
    call GroupEnumUnitsInRect( g, rs, filter )
    call ForGroup( g, function Time_Void_DmgTargets )
    call RemoveRect (rs)
    call DestroyGroup (g)
    call DestroyBoolExpr(filter)    
    set g = null
    set filter = null
    set rs = null   
    set p = null

    endfunction

function Trig_Time_Void_Actions takes nothing returns nothing
     
    
    call TimerStart(udg_TimeVoid_Timer,0.01,true,function Trig_Time_Void_expi)  
                                                       
    set udg_TimeVoid_Caster    = GetSpellAbilityUnit()  
    set udg_TimeVoid_Casterpos = GetUnitLoc(udg_TimeVoid_Caster)    
    set udg_TimeVoid_TargetPoint = GetSpellTargetLoc ()
    set udg_TimeVoid_Enddist = DistanceBetweenPoints ( udg_TimeVoid_TargetPoint,  udg_TimeVoid_Casterpos)
    set udg_TimeVoid_deg  = AngleBetweenPoints ( udg_TimeVoid_Casterpos ,  udg_TimeVoid_TargetPoint   )
    set udg_TimeVoid_laufinteger  = 0
    set udg_TimeVoid_dist = 45
    call GroupClear (udg_TimeVoid_Targets)  
    call RemoveLocation (udg_TimeVoid_TargetPoint)
     endfunction
    
    
    
    
       

//===========================================================================
function InitTrig_Time_Void takes nothing returns nothing
    set gg_trg_Time_Void = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Time_Void, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddCondition( gg_trg_Time_Void, Condition( function Trig_Time_Void_Conditions ) )
    call TriggerAddAction( gg_trg_Time_Void, function Trig_Time_Void_Actions )
endfunction

03-11-2007, 11:00 AM	#2
Pyrogasm	Mmm hmm... I'm on it. Expect another long post in an hour

03-11-2007, 02:27 PM

Pyrogasm

First off, you've written:

JASS:

//===========================================================================
function InitTrig_Time_Void takes nothing returns nothing
    set gg_trg_Time_Void = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Time_Void, EVENT_PLAYER_UNIT_SPELL_CAST )
    call TriggerAddCondition( gg_trg_Time_Void, Condition( function Trig_Time_Void_Conditions ) )
    call TriggerAddAction( gg_trg_Time_Void, function Trig_Time_Void_Actions )
endfunction

You should know to use EVENT_PLAYER_UNIT_SPELL_EFFECT, since that fires at the correct time.

JASS:

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

The first thing we notice is that it's using an If statement in a condition that returns a boolean... Now, if we know that return <statement> returns either "true" or "false", wouldn't it just be a lot simpler to write:

JASS:

function Trig_Time_Void_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A01P'
endfunction

Next, starting from the bottom of your trigger, you've written this:

JASS:

function Trig_Time_Void_Actions takes nothing returns nothing
     
    
    call TimerStart(udg_TimeVoid_Timer,0.01,true,function Trig_Time_Void_expi)
                                                       
    set udg_TimeVoid_Caster    = GetSpellAbilityUnit()
    set udg_TimeVoid_Casterpos = GetUnitLoc(udg_TimeVoid_Caster) 
    set udg_TimeVoid_TargetPoint = GetSpellTargetLoc ()
    set udg_TimeVoid_Enddist = DistanceBetweenPoints ( udg_TimeVoid_TargetPoint,  udg_TimeVoid_Casterpos)
    set udg_TimeVoid_deg  = AngleBetweenPoints ( udg_TimeVoid_Casterpos ,  udg_TimeVoid_TargetPoint   )
    set udg_TimeVoid_laufinteger  = 0
    set udg_TimeVoid_dist = 45
    call GroupClear (udg_TimeVoid_Targets)
    call RemoveLocation (udg_TimeVoid_TargetPoint)
     endfunction

Things in red are things that need to be changed. Firstly, set udg_TimeVoid_Caster = GetSpellAbilityUnit() should be set udg_TimeVoid_Caster = GetTriggerUnit(), since GetTriggerUnit() is always preferable. Next, you're using locations, which are basically a ton slower and more restricting than X/Y values, so lets change all relevant location lines to X/Y lines, add in some local variables for convenience, and fix the formatting:

JASS:

function Time_Void_AngleBetweenPointsXY takes real X1, real Y1, real X2, real Y2 returns real
    return Atan2((Y2-Y1),(X2-X1))*bj_RADTODEG
endfunction

function Time_Void_Distance takes real X1, real Y1, real X2, real Y2 returns real
    return SquareRoot((X1-X2)*(X1-X2)+(Y1-Y2)*(Y1-Y2))
endfunction

//Place the above functions at the top of the trigger; they will be used in other functions

function Trig_Time_Void_Actions takes nothing returns nothing
    local unit U = GetTriggerUnit()
    local location L = GetSpellTargetLoc() //So that the code doesn't leak
    local real X1 = GetUnitX(U)
    local real Y1 = GetUnitY(U)
    local real X2 = GetLocationX(L)
    local real Y2 = GetLocationY(L)
    local timer T = CreateTimer()

    call RemoveLocation(L) //More leak fixing
    
    call TimerStart(T, 0.01, true, function Trig_Time_Void_expi)
    set udg_TimeVoid_Caster = GetTriggerUnit()
    set udg_TimeVoid_Enddist = Time_Void_Distance(X1, Y1, X2, Y2)
    set udg_TimeVoid_deg = Time_Void_AngleBetweenPointsXY(X1, Y1, X2, Y2)
    set udg_TimeVoid_laufinteger  = 0
    endfunction

You should notice that I removed the GroupClear() lines, and removed your TimeVoid_dist variable outright. Neither of them are needed, as will be explained later. I also added in two functions that are very useful in calculating trig as you're doing; the first one replaces "DistanceBetweenPoints", and the second one replaces "AngleBetweenPointsBJ". You're using a lot of global variables like the one for the timer and caster's position; you can just use local variables for that, which is one of the main advantages of JASS. I replaced your global timer variable with a local timer because of the way timers work; you can destroy them using GetExpiredTimer(), which I will explain later. Lastly, you should have noticed that I am both declaring and setting variables on the same line. This allows you to save a line of code that would normally be used for setting the variable in this situation:

JASS:

local unit U
set U = GetTriggerUnit()

To this: local unit U = GetTriggerUnit()
Next, lets look at your timer callback function and its accompanying filter:

JASS:

function Trig_Time_Void_conds takes nothing returns boolean
local player p
set p = GetOwningPlayer( udg_TimeVoid_Caster )

if IsPlayerAlly( p, GetOwningPlayer(GetFilterUnit())) then
   return false
elseif IsUnitType( GetFilterUnit(), UNIT_TYPE_STRUCTURE) then
   return false
elseif IsUnitDeadBJ(GetFilterUnit()) then
   return false
elseif IsUnitInGroup (GetFilterUnit(), udg_TimeVoid_Targets)then
   return false
endif

return true
endfunction

function Trig_Time_Void_expi takes nothing returns nothing
        local rect rs
        local group g
        local boolexpr filter = Condition(function Trig_Time_Void_conds) 
        local location p

        set g = CreateGroup ()
        set udg_r = ( udg_TimeVoid_dist * udg_TimeVoid_laufinteger )
        set p = PolarProjectionBJ (udg_TimeVoid_Casterpos, udg_r, udg_TimeVoid_deg)
        set rs = RectFromCenterSizeBJ(p, 150.00, 150.00)
        call SetUnitPositionLoc     (udg_TimeVoid_Caster,  p)
        
        if ( udg_r >= udg_TimeVoid_Enddist ) then
        call PauseTimerBJ( true, udg_TimeVoid_Timer )
    else
    endif
    set udg_TimeVoid_laufinteger = (udg_TimeVoid_laufinteger+1)  
    call RemoveLocation (p)
    call GroupEnumUnitsInRect( g, rs, filter )
    call ForGroup( g, function Time_Void_DmgTargets )
    call RemoveRect (rs)
    call DestroyGroup (g)
    call DestroyBoolExpr(filter)    
    set g = null
    set filter = null
    set rs = null
    set p = null

    endfunction

Once again, red = wrong. As stated above, you can just use return <statement> instead of the way GUI does it; we will, however, have to use the "and" operator to make it look something to this effect:

JASS:

function Trig_Time_Void_conds takes nothing returns boolean
    return (IsPlayerEnemy(GetOwningPlayer(udg_TimeVoid_Caster), GetOwningPlayer(GetFilterUnit()))) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false) and (GetWidgetLife(GetFilterUnit()) < 0.405)and (IsUnitInGroup(GetFilterUnit(), udg_TimeVoid_Targets))
endfunction

Additionally, I have replaced the "IsUnitDead" with "GetWidgetLife"; a widget is a unit, destructable, or structure. One a widget's life reaches 0.405, the widget is officially 'dead', and it is much faster to check that than with "IsUnitDeadBJ".

As for the rest of the callback, rects are inefficient and should be used sparingly; so we will instead set a group equal to units in range of the unit to be more efficient and faster. To apply this, we'll first need to get our group. We'll use the GroupEnumUnitsInRange() native for this, which picks units that match a condition in a radius of R from X and Y values. Our X and Y values will be those of udg_TimeVoid_Caster, our radius will be 150.00, and our condition (otherwise known as a boolexpr filter; here is an explaination of one) is already present. But before using GroupEnumUnitsInRange(), you must create a group. Thus, we will now have to add the following line to a variable declaration: local group g = CreateGroup().

Put together, it should look like this:

JASS:

function Trig_Time_Void_conds takes nothing returns boolean
    return (IsPlayerEnemy(GetOwningPlayer(udg_TimeVoid_Caster), GetOwningPlayer(GetFilterUnit()))) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false) and (GetWidgetLife(GetFilterUnit()) < 0.405)and (IsUnitInGroup(GetFilterUnit(), udg_TimeVoid_Targets))
endfunction

function Trig_Time_Void_expi takes nothing returns nothing
    local real X1 = GetUnitX(udg_TimeVoid_Caster)
    local real Y1 = GetUnitY(udg_TimeVoid_Caster)
    local group g = CreateGroup()
    local boolexpr filter = Condition(function Trig_Time_Void_conds)
    local real X2 = (X1 + (45.00*(Cos(udg_TimeVoid_deg*bj_DEGTORAD))))
    local real Y2 = (Y1 + (45.00*(Sin(udg_TimeVoid_deg*bj_DEGTORAD))))
    local timer T

    call SetUnitX(udg_TimeVoid_Caster, X2)
    call SetUnitY(udg_TimeVoid_Caster, Y2)
    if (45.00*udg_TimeVoid_laufinteger >= udg_TimeVoid_Enddist) then
        set T = GetExpiredTimer()
        call PauseTimer(T)
        call DestroyTimer(T)
    endif
    set udg_TimeVoid_laufinteger = (udg_TimeVoid_laufinteger+1)  
    call GroupEnumUnitsInRange(g, X2, Y2, 150.00, filter)
    call ForGroup(g, function Time_Void_DmgTargets)

    call DestroyGroup(g)
    call DestroyBoolExpr(filter)    
    set g = null
    set filter = null
    set T = null
    endfunction

We would appear to be done, but there is, however, one more thing that can be optimized about the code. Instead of a ForGroup() function call we will replace it with a new one that creates a loop work-around which works just fine. Basically, we initiate a loop in which we set a variable to the first one of the group, do some actions with it, remove it from the group, and then loop over again until there are no more units in the group. Here's what that basic format looks like:

JASS:

loop
    set U = FirstOfGroup(G) //U being the unit variable and G being our group
    exitwhen U == null //So we can exit when the group is empty
    //Do some stuff with U
    GroupRemoveUnit(G, U) //Removing it from the group
endloop

To apply it, we'll add a section that looks like this instead of the ForGroup() call:

JASS:

    local unit U //That line goes up with the declaration of locals

    loop
        set U = FirstOfGroup(g)
        exitwhen U == null
        call UnitDamageTarget(udg_TimeVoid_Caster, U, 50.00, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL, null)
        call GroupAddUnit(udg_TimeVoid_Targets, U)
        call DestroyEffect(AddSpecialEffectTarget (GetAbilityEffectById('A01P', EFFECT_TYPE_AREA_EFFECT, 2), U,"chest"))
        call GroupRemoveUnit(g, U)
    endloop

    set U == null //Down at the bottom somewhere.
endfunction

You'll notice that I used the function DestroyEffect() in a rather odd way. The native can be exploited if you the effect is able to play its animation and still be destroyed instantaneously; most effects are, but some (like "Ancestral Spirit (Caster)") cannot be. Using this method saves you a line of code, a BJ call, a SFX variable, and a slight chance of a mix-up.

Everything slapped together with that GroupClear() put back in in the correct place, the code should look something like this:

JASS:

function Trig_Time_Void_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A01P'
endfunction

function Time_Void_AngleBetweenPointsXY takes real X1, real Y1, real X2, real Y2 returns real
    return Atan2((Y2-Y1),(X2-X1))*bj_RADTODEG
endfunction

function Time_Void_Distance takes real X1, real Y1, real X2, real Y2 returns real
    return SquareRoot((X1-X2)*(X1-X2)+(Y1-Y2)*(Y1-Y2))
endfunction

function Trig_Time_Void_conds takes nothing returns boolean
    return (IsPlayerEnemy(GetOwningPlayer(udg_TimeVoid_Caster), GetOwningPlayer(GetFilterUnit()))) and (IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE) == false) and (GetWidgetLife(GetFilterUnit()) < 0.405)and (IsUnitInGroup(GetFilterUnit(), udg_TimeVoid_Targets))
endfunction

function Trig_Time_Void_expi takes nothing returns nothing
    local real X1 = GetUnitX(udg_TimeVoid_Caster)
    local real Y1 = GetUnitY(udg_TimeVoid_Caster)
    local group g = CreateGroup()
    local boolexpr filter = Condition(function Trig_Time_Void_conds)
    local real X2 = (X1 + (45.00*(Cos(udg_TimeVoid_deg*bj_DEGTORAD))))
    local real Y2 = (Y1 + (45.00*(Sin(udg_TimeVoid_deg*bj_DEGTORAD))))
    local unit U
    local timer T

    call SetUnitX(udg_TimeVoid_Caster, X2)
    call SetUnitY(udg_TimeVoid_Caster, Y2)
    if (45.00*udg_TimeVoid_laufinteger >= udg_TimeVoid_Enddist) then
        set T = GetExpiredTimer()
        call PauseTimer(T)
        call DestroyTimer(T)
    endif
    set udg_TimeVoid_laufinteger = (udg_TimeVoid_laufinteger+1)  
    call GroupEnumUnitsInRange(g, X2, Y2, 150.00, filter)
    loop
        set U = FirstOfGroup(g)
        exitwhen U == null
        call UnitDamageTarget(udg_TimeVoid_Caster, U, 50.00, false, false, ATTACK_TYPE_MAGIC, DAMAGE_TYPE_NORMAL, null)
        call GroupAddUnit(udg_TimeVoid_Targets, U)
        call DestroyEffect(AddSpecialEffectTarget(GetAbilityEffectById('A01P', EFFECT_TYPE_AREA_EFFECT, 2), U, "chest"))
        call GroupRemoveUnit(g, U)
    endloop

    call GroupClear(udg_TimeVoid_Targets)
    call DestroyGroup(g)
    call DestroyBoolExpr(filter)    
    set g = null
    set filter = null
    set T = null
    set U = null
endfunction

function Trig_Time_Void_Actions takes nothing returns nothing
    local unit U = GetTriggerUnit()
    local location L = GetSpellTargetLoc() //So that the code doesn't leak
    local real X1 = GetUnitX(U)
    local real Y1 = GetUnitY(U)
    local real X2 = GetLocationX(L)
    local real Y2 = GetLocationY(L)
    local timer T = CreateTimer()

    call RemoveLocation(L) //More leak fixing
    
    call TimerStart(T, 0.01, true, function Trig_Time_Void_expi)
    set udg_TimeVoid_Caster = GetTriggerUnit()
    set udg_TimeVoid_Enddist = Time_Void_Distance(X1, Y1, X2, Y2)
    set udg_TimeVoid_deg = Time_Void_AngleBetweenPointsXY(X1, Y1, X2, Y2)
    set udg_TimeVoid_laufinteger  = 0
    endfunction



//===========================================================================
function InitTrig_Time_Void takes nothing returns nothing
    set gg_trg_Time_Void = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Time_Void, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Time_Void, Condition( function Trig_Time_Void_Conditions ) )
    call TriggerAddAction( gg_trg_Time_Void, function Trig_Time_Void_Actions )
endfunction

I hope I've helped.

Please, next time, use the [jass] tags, and try to format your code in a non-counter-intuitive way.

03-11-2007, 03:18 PM

anXieTy

and what exactly was the objekt/function causing the leaks?

I would really like to thank you very much for improving my spell and explaing your steps, too. I made a "multiiinstance" spell of this sort with Handle Vars, it worked well but i didnt use it for my spell.

THx, best wishes

anX

Edit: The only problem in your trigger is, that the caster moves ?farer? (or more more far -_-) than the target point, which might be caused by the multiplication with 45 in the expi function.

Edit2: My theory was wrong, the caster moves exactly the double distance which he normally should move.

Edit 3: Ok, it works well, was my mistake.

Edit 4: And how can i implement a shadow, which makes it look like a fast time-spell, well something like a demonhunter casts this spell and a copy of him with another fade color runs behind him. And how can i implement the caster moving directly to the target location? Questions, Questions everywhere questions...

ty for being patient and maybe helping me again

Edit 5: Everything solved, thx

03-11-2007, 10:06 PM

Pyrogasm

Quote:

and what exactly was the objekt/function causing the leaks?

You had some point leaks, but general inefficiency was getting the spell bogged down.

Quote:

Edit 5: Everything solved, thx

So, you got the shadow thing working?

EDIT: We gentlemanly people tend to rep others who spend 4 hours fixing a screwed up code, just so's you know...

03-11-2007, 11:04 PM

Ammorth

Quote:

Originally Posted by Pyrogasm

EDIT: We gentlemanly people tend to rep others who spend 4 hours fixing a screwed up code, just so's you know...

Don't worry Pyrogasm, I gotchya covered. + rep!

03-12-2007, 12:44 AM	#7
Pyrogasm	Well, you who are not involved are not always expected to, but he, whose problem I solved, should be greateful.