HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Help with periodic jass trigger -> leaks? crash?

03-28-2008, 11:59 AM#1
Bsycho
hey guys, im pretty new at jass, maybe some1 can help me out here.

my questions are:
1. are there any leaks? as it fires every 0.05s its quite essential, and im not sure about GetUnitX/Y...
do i have to null any variables?

2. why does it crash if a) i uncomment the condition IsUnitAliveBJ and b) the possessed unit dies?

Collapse JASS:
function Trig_Voodoo_Possession_Timer_Func003C takes nothing returns boolean
    if ( not ( GetRandomInt(1, 2) == 1 ) ) then
        return false
    endif
    return true
endfunction

function Trig_Voodoo_Possession_Timer_Actions takes nothing returns nothing

    local real x1 = GetUnitX(udg_unit_possessionowner)
    local real y1 = GetUnitY(udg_unit_possessionowner)
    local real z1 = GetLocationZ(GetUnitLoc(udg_unit_possessionowner))+75

    local real x2
    local real y2
    local real z2

    if ( not ( CountUnitsInGroup(udg_group_possessed) == 0 ) ) then

        set udg_integer_loop = 1
        loop
            exitwhen udg_integer_loop > 4

            set x2 = GetUnitX(udg_unit_possessed[udg_integer_loop])
            set y2 = GetUnitY(udg_unit_possessed[udg_integer_loop])
            set z2 = GetLocationZ(GetUnitLoc(udg_unit_possessed[udg_integer_loop]))+25

            call DestroyLightning( udg_lightning_spells[udg_integer_loop] )

            //if ( IsUnitAliveBJ(udg_unit_possessed[udg_integer_loop]) == true )  then

                if ( Trig_Voodoo_Possession_Timer_Func003C() ) then
                    set udg_lightning_spells[udg_integer_loop]=AddLightningEx("CLSB",true,x1,y1,z1,x2,y2,z2)
                else
                    set udg_lightning_spells[udg_integer_loop]=AddLightningEx("CLPB",true,x1,y1,z1,x2,y2,z2)
                endif
        
            //else
            //endif

        call SetLightningColorBJ( udg_lightning_spells[udg_integer_loop], 1.00, 1.00, 0.50, 0.50 )

        set udg_integer_loop = udg_integer_loop + 1
        endloop

        call StartTimerBJ( udg_timer_spells[1], false, 0.05 )

    else
    endif

endfunction

//===========================================================================
function InitTrig_Voodoo_Possession_Timer takes nothing returns nothing
    set gg_trg_Voodoo_Possession_Timer = CreateTrigger(  )
    call TriggerRegisterTimerExpireEventBJ( gg_trg_Voodoo_Possession_Timer, udg_timer_spells[1] )
    call TriggerAddAction( gg_trg_Voodoo_Possession_Timer, function Trig_Voodoo_Possession_Timer_Actions )
endfunction


edit: ok, if i remove the unit from the group if it dies, it doesnt crash. but it doesnt detroy the lightning either :(
03-28-2008, 12:07 PM#2
Captain Griffen
Yes, you leak locs. You're also going about it in a very bad way (a very GUI-ish way). Lots of BJs, and non-inlined conditions.
03-28-2008, 12:20 PM#3
Bsycho
well as im learning, its ok for me if it works and doesnt leak.
its only a very small project and im using a combination of gui and jass triggers.

whats wrong with BJs? can someone help me with that crash?

edit: i tried to fix the leaks. i also tried to use another condition except IsUnitAliveBJ, but if i try to save it will crash my WE.
units are nulled and removed from group if they die.

edit2: seems i fixed the bugs, can anyone tell me about the leaks please?

Collapse JASS:
function Trig_Voodoo_Possession_Timer_Actions takes nothing returns nothing

    local location p1 = GetUnitLoc(udg_unit_possessionowner)
    local real x1 = GetUnitX(udg_unit_possessionowner)
    local real y1 = GetUnitY(udg_unit_possessionowner)
    local real z1 = GetLocationZ(p1)+75

    local location p2
    local real x2
    local real y2
    local real z2

    local integer loop1
    local integer loop2

set loop1 = 1
        loop
            exitwhen loop1 > 4
            call DestroyLightning( udg_lightning_spells[loop1] )
set loop1 = loop1 + 1
        endloop

    if ( not ( CountUnitsInGroup(udg_group_possessed) == 0 ) ) then

        set loop2 = 1
        loop
            exitwhen loop2 > 4

            set p2 = GetUnitLoc(udg_unit_possessed[loop2])
            set x2 = GetUnitX(udg_unit_possessed[loop2])
            set y2 = GetUnitY(udg_unit_possessed[loop2])
            set z2 = GetLocationZ(p2)+25

            if ( udg_unit_possessed[loop2] != null ) then

                if ( not ( GetRandomInt(1, 2) == 1 ) ) then
                    set udg_lightning_spells[loop2]=AddLightningEx("CLSB",true,x1,y1,z1,x2,y2,z2)
                else
                    set udg_lightning_spells[loop2]=AddLightningEx("CLPB",true,x1,y1,z1,x2,y2,z2)
                endif

                set p2 = null 
       
            else
            endif

        call SetLightningColorBJ( udg_lightning_spells[loop2], 1.00, 1.00, 0.50, 0.50 )

        set loop2 = loop2 + 1
        endloop

        call StartTimerBJ( udg_timer_spells[1], false, 0.05 )

    else
    endif

    set p1 = null


endfunction

//===========================================================================
function InitTrig_Voodoo_Possession_Timer takes nothing returns nothing
    set gg_trg_Voodoo_Possession_Timer = CreateTrigger(  )
    call TriggerRegisterTimerExpireEventBJ( gg_trg_Voodoo_Possession_Timer, udg_timer_spells[1] )
    call TriggerAddAction( gg_trg_Voodoo_Possession_Timer, function Trig_Voodoo_Possession_Timer_Actions )
endfunction
03-28-2008, 01:48 PM#4
Fireeye
In the current trigger you still leak the locations.
About what a leak is ...
A leak happens when you create a handle with no reference to and due WC3 isn't intelligent it won't know that it is a handle which will be used only once.
So when this information stays in the memory, however you can not access it anymore, thus the memory usage increase with every leak, which will cause in most cases laggs.
You can clean up locations with 'call RemoveLocation(<which location>)' however it still creates a handle so i recommend using X and Y value, or if you really need a location, create 1 global location and move it.
03-28-2008, 04:06 PM#5
TheSecretArts
Quote:
Originally Posted by Bsycho
whats wrong with BJs?

They are horrible, pure and simple. There are some exceptions but for the most part, they use/are unneeded functions calls. They also tend to leak, so avoid them. Avoid functions with the name "swapped" in them.
They come from the blizzard.j file.
However the good functions (natives) come from the common.j files.

hey fireeye, where was that TDR quote taken from?
03-28-2008, 05:57 PM#6
Fireeye
Ontopic
Btw. you already have the location from which you want to take the Z Value, so you can use 1 location and move it with the command call MoveLocation(WhichLocation,New X, New Y).

Offtopic
Don't really know the thread anymore, were searching something and found this from TDR as Reason to not delet a post xD
03-29-2008, 02:36 AM#7
burningice95
Simply putting this bit of code right before
Collapse JASS:
 endfunction 
will prevent your leaks

Collapse JASS:
call RemoveLocation (p1)
call RemoveLocatoin(p2)

Also, why are you using all of those globals...?
03-30-2008, 08:29 AM#8
Bsycho
thx for the help so far.

im using some gui triggers in combination with jassed ones, therefore some globals. maybe i replace them.

i totally forgot the removelocation, its what i used in gui also (custom script).
put the removeloc p2 in the loop though.
03-30-2008, 01:51 PM#9
grupoapunte
As others said you have to avoid BJ functions, if you dont have a list of equivalent natives i suggest you use JassCraft (some people think this is old but it helps) to write your code, i learned a lot using it, simply copy paste the trigger in JassCraft and then double click on BJ functions and you will see in the bottom of the screen the internal code of that function, then is up to you to undestand how to reemplace it, it also has a list of functions in the right column.

Another thing i noticed in your code is that you use GUI conditions format for example:

Collapse JASS:
if ( not ( CountUnitsInGroup(udg_group_possessed) == 0 ) ) then
    //code
endif

Its like a double condition, its horrible, you are negating the condition, this would be the right way to do it:

Collapse JASS:
if (CountUnitsInGroup(udg_group_possessed) != 0) then
    //code
endif

I removed the "not" and changed the "==" (equal) for "!=" (distinct)

Good luck! you will find Jass very interesting so don't give up on it