GUI local variable & wait question

[Ekxan]

Hi, I made this trigger as GUI but it needs a local variable because it has a conditional wait and it needs the units ID from the indexer after the wait. I never used JASS before so i have 2 questions about what i did.
the unit indexer i am using is GUI Unit Indexer 1.0.1.1 by Bribe

Question 1:
do i need to remove the local variable on the end of the function?
if yes, how to do it?

Question 2:
the original wait call also calls RMaxBJ, i removed it and it seems to work. but i don't know if what i did should be done.
before(original WE):

call TriggerSleepAction(RMaxBJ(bj_WAIT_FOR_COND_MIN_INTERVAL, 0.10))

changed code:

call TriggerSleepAction(0.10)

im not sure RMaxBJ(bj_WAIT_FOR_COND_MIN_INTERVAL, 0.10) is needed in this case. correct me if im wrong.

here are the TriggerSleepAction and RMaxBJ functions, i do not know the value of bj_WAIT_FOR_COND_MIN_INTERVAL

native TriggerSleepAction   takes real timeout returns nothing
function RMaxBJ takes real a, real b returns real
    if (a < b) then
        return b
    else
        return a
    endif
endfunction

below is the original GUI version with no local variable(more than one unit enters the rect at once and Unit_ID_EL is overwritten):

• Left Rect
  • Events
    • Unit - A unit enters East Left <gen>
  • Conditions
  • Actions
    • Set Unit_ID_EL = (Custom value of (Triggering unit))
    • Wait until ((Distance between (Position of UDexUnits[Unit_ID_EL]) and Unit_Rally_Point[Unit_ID_EL]) Less than 200.00), checking every 0.10 seconds
    • Set Unit_Current_Waypoint[Unit_ID_EL] = (Unit_Current_Waypoint[Unit_ID_EL] + 8)
    • Set Unit_Rally_Point[Unit_ID_EL] = Waypoints[Unit_Current_Waypoint[Unit_ID_EL]]
    • Unit - Order UDexUnits[Unit_ID_EL] to Move To Unit_Rally_Point[Unit_ID_EL]

here is the changed GUI with the local variable so Unit_ID_EL does not get overwritten:
I think I have a location leak at "exitwhen ( DistanceBetweenPoints(GetUnitLoc(udg_UDexUnits[Unit_ID_EL]), udg_Unit_Rally_Point[Unit_ID_EL]) < 200 )" but im not sure how to fix it because it is inside "exitwhen"

• Left Rect
  • Events
    • Unit - A unit enters East Left <gen>
  • Conditions
  • Actions
    • Custom script: local integer Unit_ID_EL = GetUnitUserData(GetTriggerUnit())
    • Custom script: loop
    • Custom script: exitwhen ( DistanceBetweenPoints(GetUnitLoc(udg_UDexUnits[Unit_ID_EL]), udg_Unit_Rally_Point[Unit_ID_EL]) < 200 )
    • Custom script: call TriggerSleepAction(0.10)
    • Custom script: endloop
    • Custom script: set udg_Unit_Current_Waypoint[Unit_ID_EL] = ( udg_Unit_Current_Waypoint[Unit_ID_EL] + 8 )
    • Custom script: set udg_Unit_Rally_Point[Unit_ID_EL] = udg_Waypoints[udg_Unit_Current_Waypoint[Unit_ID_EL]]
    • Custom script: call IssuePointOrderLocBJ( udg_UDexUnits[Unit_ID_EL], "move", udg_Unit_Rally_Point[Unit_ID_EL] )

 

[Bannar]

Loca

Local variables have to be nulled/destroyed at the end of functions.
Integers/reals/booleans/strings do not need to be nulled, since they dont cause leaks.

I wont try writing it in Jass, since I've just learnt basic of vJass, and still teaching Jass - I do not want to write some shit here.

Anyways, it can be easily done in GUI, but reqiures two triggers:

• Left Rect
  • Events
    • Unit - A unit enters East Left <gen>
  • Conditions
  • Actions
    • Unit Group - Add (Triggering unit) to Unitgroup

I dont know what UDexUnit[] is here, and other variables, just wrote basing on what you have already posted.

• Loop
  • Events
    • Game - Every 0.1 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in UnitGroup and do actions
      • Loop - Actions
        • Set Unit_ID_EL = (Custom value of (Picked unit))
        • Set tempp1 = (Position of (UDexUnits[Unit_ID_EL]))
        • Set Unit_Rally_Point[Unit_ID_EL] = <dont know>
        • If (All conditions are true) then do (Then actions) else do (Else actions)
          • If - Conditions
            • (Distance between tempp1 and Unit_Rally_Point[Unit_ID_EL]) Less than 200.00
          • Then - Actions
            • Set Unit_Current_Waypoint[Unit_ID_EL] = (Unit_Current_Waypoint[Unit_ID_EL] + 8)
            • Set Unit_Rally_Point[Unit_ID_EL] = Waypoints[Unit_Current_Waypoint[Unit_ID_EL]]
            • Unit - Order UDexUnits[Unit_ID_EL] to Move To Unit_Rally_Point[Unit_ID_EL]
            • Unit Group - Remove (Picked unit) from UnitGroup
          • Else - Actions
        • Custom script: call RemoveLocation (udg_tempp1)
        • Custom script: call RemoveLocation (udg_Unit_Rally_Point[udg_Unit_ID_EL])

I'm not sure what you want to do, but this is how you can fix your wait problem using loop in GUI.

 

[Pharaoh_]

1) Depends; anything except for reals/integers/strings should be nulled. In my opinion, string should never be local at all, since they create a leak once.
e.g.

local unit u = GetTriggerUnit()
//actions here
set u = null

2) TriggerSleepAction under 0.27 is inaccurate. For greater accuracy and efficiency, you will have to use Timers.
(and no, that's not needed)

3) In Jass, prefer coordinates, instead of locations:

call IssuePointOrder (u, "move", x, y)

this way, you won't have to mess with leaks, since reals (the coordinates) do not leak.

4) Avoid using BJ, natives are faster and consume less memory.

5) Finally, use a timer; totally convert the trigger into custom text (from Edit menu), because keeping it GUI just for the sake of an event is clearly a no.
Use a local timer, use a hashtable to save the units in the timer and each time the periodic timer expires, check the distance between the units' coordinates.

 

[mckill2009]

in addition DistanceBetweenPoints calls 2 locations, locA and locB...

to fix this, use reals...

local real x = GetSpellTargetX() - GetUnitX(u)
local real y = GetSpellTargetY() - GetUnitY(u)
local real distance = SquareRoot(x * x + y * y)

this is just an idea on how to replace locations and get the distance between points...

 

[Ekxan]

thx for the replies, the gui version of spinnaker would work well for me, but now that im using some JASS i decided to keep the JASS version to learn more.

i got rid of the leaks by using only real of the x, y location
and decided to keep the wait and not use timer for the moment, I didn't understand very well the timers yet.

here is the script: it works fine thx all!

function Trig_Left_Rect_Actions takes nothing returns nothing
    local integer Unit_ID_EL = GetUnitUserData(GetTriggerUnit())
    local real Unit_Rally_Point_X = GetLocationX(udg_Unit_Rally_Point[Unit_ID_EL])
    local real Unit_Rally_Point_Y = GetLocationY(udg_Unit_Rally_Point[Unit_ID_EL])
    local real x = Unit_Rally_Point_X - GetUnitX(udg_UDexUnits[Unit_ID_EL])
    local real y = Unit_Rally_Point_Y - GetUnitY(udg_UDexUnits[Unit_ID_EL])
    local real distance = SquareRoot(x * x + y * y)
//    call DisplayTextToForce( GetPlayersAll(), "In Region" )
    loop
    exitwhen ( distance < 200 )
//    call DisplayTextToForce( GetPlayersAll(), R2S(distance))
    set x = Unit_Rally_Point_X - GetUnitX(udg_UDexUnits[Unit_ID_EL])
    set y = Unit_Rally_Point_Y - GetUnitY(udg_UDexUnits[Unit_ID_EL])
    set distance = SquareRoot(x * x + y * y)
    call TriggerSleepAction(0.10)
    endloop
//    call DisplayTextToForce( GetPlayersAll(), "In range" )
    set udg_Unit_Current_Waypoint[Unit_ID_EL] = ( udg_Unit_Current_Waypoint[Unit_ID_EL] + 8 )
    set udg_Unit_Rally_Point[Unit_ID_EL] = udg_Waypoints[udg_Unit_Current_Waypoint[Unit_ID_EL]]
    call IssuePointOrder (udg_UDexUnits[Unit_ID_EL], "move", GetLocationX(udg_Unit_Rally_Point[Unit_ID_EL]), GetLocationY(udg_Unit_Rally_Point[Unit_ID_EL]))
endfunction

//===========================================================================
function InitTrig_Left_Rect takes nothing returns nothing
    set gg_trg_Left_Rect = CreateTrigger(  )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_East_Left )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_East_Top )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_East_Right )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_East_Bottom )

    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_North_Left )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_North_Top )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_North_Right )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_North_Bottom )

    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_West_Left )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_West_Top )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_West_Right )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_West_Bottom )

    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_South_Left )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_South_Top )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_South_Right )
    call TriggerRegisterEnterRectSimple( gg_trg_Left_Rect, gg_rct_South_Bottom )

    call TriggerAddAction( gg_trg_Left_Rect, function Trig_Left_Rect_Actions )
endfunction

 

[Bannar]

At first I recommend get rid of Simple events, look on jass script:

function TriggerRegisterEnterRectSimple takes trigger trig, rect r returns event
    local region rectRegion = CreateRegion()
    call RegionAddRect(rectRegion, r)
    return TriggerRegisterEnterRegion(trig, rectRegion, null)
endfunction

It creates a lot of unneeded code, since it returns function TriggerRegisterEnterRegion which do the whole job. Instead of TriggerRegisterEnterRectSimple use native:

native TriggerRegisterEnterRegion takes trigger whichTrigger, region whichRegion, boolexpr filter returns event

Like:

function InitTrig_Left_Rect takes nothing returns nothing
    set gg_trg_Left_Rect = CreateTrigger(  )
    call TriggerRegisterEnterRegion( gg_trg_Left_Rect, gg_rct_South_Bottom, null )

    call TriggerAddAction( gg_trg_Left_Rect, function Trig_Left_Rect_Actions )
endfunction

 

[Dr Super Good]

Local variables are automatically destroyed at the end of the function. No extra code is needed.

A bug in the WC3 JASS interpreter means that locals of type handle declared in a function (not as parameters) do not have their content processed when the local is deallocated. This causes a handle index garbage collector leak so the handle index can never be recycled and thus if the object it is assigned to gets destroyed it can never be reused.

This leak is only a problem for handles that can and will get recycled. Players for example can never be completly destroyed and so their handle can never be recycled (thus a counter leak in the garbage collector is of no importance). On the other hand effects which are destroyed very often will need to have their handle index recycled (thus a counter leak in the garbage collector will cause the handle index to leak once the object is destroyed as it can never be recycled).

Again, this is only a bug for declared locals only. Local handles originating from the function's arguments do get their garbage collector reference counter decrimented automatically at the end of the function (as it should be but is not for locals you declare).

The garbage collector for handle indicies allocates a counter to each handle representing the number of refferences currently pointing to the handle. Every time you

set somevar = somehandle

, the counter for somehandle is incrimented by 1. When

set somevar = somedifferenthandle

is then run, the counter for somehandle is decrimented (as it no longer has that refference) and the one for somedifferenthandle is incrimented (like above). The bug is that declared locals (not those from arguments once again) do not decriment the counter for any handle they are assigned to at the end of the function. Thus the work around is to

set somevar = null

before the end of the function so that you force it to decriment the handle counter correctly (and null is a constant so does not need garbage collecting).

 

[Bannar]

This is an egzample of how it could be code using Jass script.
Thanks to DSG for helping me doing this properly.

Loop - variable of type trigger; UnitGroup - variable of type unit group
You can compare GUI script I've written above with this to see whats going on.
Again, I'm not sure what are you planing to do with this trigger so treat this as base, not as copy+paste trigger.

// Calculating
function Loop_Periodic takes nothing returns nothing
    
    local integer Unit_ID_EL = GetUnitUserData(GetEnumUnit())
    local real x = Unit_Rally_Point_X - GetUnitX(udg_UDexUnits[Unit_ID_EL])
    local real y = Unit_Rally_Point_Y - GetUnitY(udg_UDexUnits[Unit_ID_EL])
    local real distance = (x * x + y * y)
                                                           
    if distance < 40000 then
        set udg_Unit_Current_Waypoint[Unit_ID_EL] = ( udg_Unit_Current_Waypoint[Unit_ID_EL] + 8 )
        set udg_Unit_Rally_Point[Unit_ID_EL] = udg_Waypoints[udg_Unit_Current_Waypoint[Unit_ID_EL]]
        call IssuePointOrder (udg_UDexUnits[Unit_ID_EL], "move", GetLocationX(udg_Unit_Rally_Point[Unit_ID_EL]), GetLocationY(udg_Unit_Rally_Point[Unit_ID_EL]))
        call GroupRemoveUnit (udg_UnitGroup, GetEnumUnit())
    endif
endfunction

// Picks all units in UnitGroup
function Loop_Actions takes nothing returns nothing
    call ForGroup(udg_UnitGroup, function Loop_Periodic)
endfunction

// Add unit to UnitGroup
function GetUnit takes nothing returns nothing
    call GroupAddUnit (udg_UnitGroup, GetTriggerUnit() )
endfunction

// Executes Init actions
function Trig_Left_Rect_Actions takes nothing returns nothing
    set local trigger t = CreaterTrigger( )
    call TriggerRegisterEnterRegion( t, gg_rct_South_Bottom, null )
    call TriggerAddAction( t, function GetUnit )

// Use of trigger variable
    set udg_Loop = CreateTrigger()
    call TriggerRegisterTimerEvent(udg_Loop, 0.1, true)
    call TriggerAddAction(udg_Loop, function Loop_Actions) 
endfunction

//===========================================================================
function InitTrig_Left_Rect takes nothing returns nothing
    set gg_trg_Left_Rect = CreateTrigger(  )
    call TriggerRegisterTimerEvent(gg_trg_Left_Rect, 0.00,false)
    call TriggerAddAction(gg_trg_Left_Rect, function Trig_Left_Rect_Actions)
endfunction

 

[mckill2009]

As much as possible, NEVER use locations in JASS...

this...

local real distance = (x * x + y * y)

should be this...

local real distance = SquareRoot(x * x + y * y)

and, GetLocationX and Y should be reals as well...

local real angle = Atan2(Unit_Rally_Point_Y - GetUnitY(udg_UDexUnits[Unit_ID_EL]), Unit_Rally_Point_X - GetUnitX(udg_UDexUnits[Unit_ID_EL]))
local real x1 = SOURCE_X+OFFSET_REAL*Cos(angle)
local real y1 = SOURCE_Y+OFFSET_REAL*Sin(angle)

//===
call IssuePointOrder(udg_UDexUnits[Unit_ID_EL], "move", x1, y1)

 

[Dr Super Good]

this...
Jass:
local real distance = (x * x + y * y)should be this...
Jass:
local real distance = SquareRoot(x * x + y * y)

If you are in a cold country and need to use your computer as a heater, yes. Unless you are activly using the distance in part of a calculation, it is better to use distance squared as it saves a lot of time. Especially when comparing against a constant, you can just square the constant rather than root the distance each time. Constant squared is a constant. Square root of a variable is the square root of a variable. 1 has a compiler time expense, the other a run time expense.

As much as possible, NEVER use locations in JASS...

Except if a position is constant. Passing 1 location argument is faster than 2 reals. If you need to manipulate the location it looses almost all benifit so this is only the case for constants.

and, GetLocationX and Y should be reals as well...

Jass:
local real angle = Atan2(Unit_Rally_Point_Y - GetUnitY(udg_UDexUnits[Unit_ID_EL]), Unit_Rally_Point_X - GetUnitX(udg_UDexUnits[Unit_ID_EL]))
local real x1 = SOURCE_X+OFFSET_REAL*Cos(angle)
local real y1 = SOURCE_Y+OFFSET_REAL*Sin(angle)

//===
call IssuePointOrder(udg_UDexUnits[Unit_ID_EL], "move", x1, y1)

This has anything to do with his code?

 

[mckill2009]

I'm just explaining "this" to "that", I never said anything about running square root each time...if he wanted to get the distance between points, he should do it in the casting of the spell or entry of a unit, etc...then reference it as a real # in the loop...

This has anything to do with his code?

no, this is just a reference on how to get to A to B without using locations GetLocationX and Y...