[JASS] [Solved] Questions about leaks.

[hugope]

I have a problem with the next 2 triggers. In the trigger CreateUnits I use a hashtable to save a location. The problem is that when the custom script removes TempPoint[1] the hashtable (unwantedly) also gets updated and deletes its location.

• CreateUnits
  • Events
    • Unit - A unit Sells a unit
  • Conditions
  • Actions
    • Set VariableSet TempPoint[1] = ((Center of Region) offset by (0.00, (-1.00 x DistanceBetweenTheFourUnits)))
    • Hashtable - Save Handle OfTempPoint[1] as 200 of PNumberOfOwnerOfUnit in Hashtable.
    • Custom script: call RemoveLocation(udg_TempPoint[1])

• UnitPicksAnItem
  • Events
    • Unit - A unit Acquires an item
  • Conditions
  • Actions
    • Unit - Create 1 Circle of Power for (Owner of (Triggering unit)) at (Load 200 of PNumberOfOwnerOfUnit in Hashtable.) facing Default building facing degrees

How can I save a location from trigger CreateUnits to use it in trigger UnitPicksAnItem without it leaking?


Do my next trigger have leaks? I think there is a location leak in the custom script. If it doesn't leak, then here is the solution to my first question!

• Set VariableSet MoonwalkingTempPoint[10] = ((Center of Region) offset by (0.00, (-1.00 x DistanceBetweenTheFourUnits)))
• Set VariableSet PreventPointLeak[0] = (X of MoonwalkingTempPoint[10])
• Set VariableSet PreventPointLeak[1] = (Y of MoownalkingTempPoint[10])
• Custom script: call SaveLocationHandle(udg_Hashtable, udg_PNumberOfOwnerOfUnit, 200, Location (udg_PreventPointLeak[0], udg_PreventPointLeak[1]))
• Custom script: call RemoveLocation(udg_MoonwalkingTempPoint[10])

Do my next functions have leaks?

function TriggerToMove takes unit u1, unit u2, unit u3, unit u4 returns nothing
    local integer value = 100

    // WalkLeft
    call IssuePointOrder( u2, "move", GetUnitX(u2) - value, GetUnitY (u2))
    // WalkDown
    call IssuePointOrder( u3, "move", GetUnitX(u3), GetUnitY (u3) - value)
    // WalkRight
    call IssuePointOrder( u4, "move", GetUnitX(u4) + value, GetUnitY (u4))
    // WalkUp
    call IssuePointOrder( u1, "move", GetUnitX(u1), GetUnitY (u1) + value)
endfunction


function TriggerMovementRight takes unit source returns nothing
    local timer t = CreateTimer()
    local integer id = GetHandleId(t)
    local integer idunit = GetHandleId(source)
    local player owner = GetOwningPlayer(source)
    local integer idowner = GetPlayerId(owner)

    call SaveUnitHandle(udg_Hashtable, id, 0, source)
    call SaveUnitHandle(udg_Hashtable, idowner, 400, source)
    call SaveTimerHandle(udg_Hashtable, idunit, 1, t)

    call TimerStart(t, 0.01, true, function TriggerMovementRight_Expire)
 
    set t = null
endfunction


function TriggerMovementRight_Expire takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local real speed = 3.2
    local unit source = LoadUnitHandle(udg_Hashtable, id, 0)

    call SetUnitX(source, GetUnitX(source) + speed)

    set t = null
endfunction

 

[Uncle]

A leak is when you lose reference to something. In this case you're saving the Points (locations) into the Hashtable, and as long as you don't overwrite them you will have reference to them at all times and can Remove them when need be.

With that in mind, the solution is to delete this line:

• Custom script: call RemoveLocation(udg_TempPoint[1])

This line is destroying the Point (location) that you just saved to the Hashtable.

That's like saving a Unit to the Hashtable and then immediately Removing it from the game. This serves no purpose:

• Hashtable - Save Handle of TempUnit to Hashtable
• Unit - Remove TempUnit from the game

Understand that the global variable isn't the thing that would leak, it's the object that the variable created, which in this case is a Location:

set udg_TempPoint[1] = Location(x, y)
// each time this runs it creates an entirely new Location object
// TempPoint[1] is just a reference to that object, not the object itself

If you want to clean up that newly created Location you could do it after you're done Loading it:

• Set Variable TempPoint = Load 200 of PNumberOfOwnerOfUnit in Hashtable
• Unit - Create 1 Circle of Power for (Owner of (Triggering unit)) at TempPoint facing Default building facing degrees
• Custom script: call RemoveLocation(udg_TempPoint)

The MoonwalkTempPoint stuff doesn't make any sense to me. Not too sure what you were trying to do but it looks like this is what you want:

• Set Variable TempPoint = Center of Region
• Set Variable MoonwalkingTempPoint[10] = (TempPoint offset by (0.00, (-1.00 x DistanceBetweenTheFourUnits)))
• Hashtable - Save Handle Of MoonwalkingTempPoint[10] as 200 of PNumberOfOwnerOfUnit in Hashtable.
• Custom script: call RemoveLocation(udg_TempPoint)

Again, you don't need to clean up MoonwalkingTempPoint[10] right now because it's been saved in the Hashtable. Just make sure to Remove it before you save a new Point at that same Key in the Hashtable.


Regarding your Jass, you are forgetting to null players/units. You set t (timer) to null but you never null owner or source.
Every local variable needs to be nulled with the exception of: integers, reals, strings, booleans

set owner = null
set source = null

Anyway, if you're coding then how about taking advantage of X/Y coordinates? I hardly ever use Locations since you can reference the X/Y coordinates of just about anything. This way it doesn't matter if you overwrite the X/Y coordinates in the Hashtable since Reals don't leak.

 

[hugope]

Thank you for your reply!

If you want to clean up that newly created Location you could do it after you're done Loading it:

• Set Variable TempPoint = Load 200 of PNumberOfOwnerOfUnit in Hashtable
• Unit - Create 1 Circle of Power for (Owner of (Triggering unit)) at TempPoint facing Default building facing degrees
• Custom script: call RemoveLocation(udg_TempPoint)

The issue here is that I want to create that Circle of Power in another trigger. A trigger with the event "A unit Acquires an Item".

The MoonwalkTempPoint stuff doesn't make any sense to me. Not too sure what you were trying to do...

Please revalue that trigger. What was intended was to save a location Handle without setting a global variable as a location. This was done to prevent the memory leak created by that action. That's why I was asking (and still am) if that trigger has leaks.
The trigger uses a global variable, yes, but it gets removed after PreventPointLeak[0] and PreventPointLeak[1] gather its coordinates. It gets removed without affecting the hashtable, that's why it's different.

Understand that the global variable isn't the thing that would leak, it's the object that the variable created.

Again, you don't need to clean up MoonwalkingTempPoint[10] right now because it's been saved in the Hashtable. Just make sure to Remove it before you save a new Point at that same Key in the Hashtable.

Does that mean I can clean the memory leak created by setting a global variable to a point by removing the location in a hashtable, if I had previously saved it in such hashtable? Example:

• Trigger 1
  • Events
    • Map initialization
  • Conditions
  • Actions
    • Set VariableSet MoonwalkingTempPoint[10] = ((Center of Region) offset by (0.00, (-1.00 x DistanceBetweenTheFourUnits)))
    • Hashtable - Save Handle OfMoonwalkingTempPoint[10] as 200 of PNumberOfOwnerOfUnit in Hashtable.

• Trigger 2
  • Events
    • Time - Elapsed game time is 1.00 seconds
  • Conditions
  • Actions
    • Custom script: call RemoveLocation(LoadLocationHandle(udg_Hashtable, udg_PNumberOfOwnerOfUnit, 200))

Anyway, if you're coding then how about taking advantage of X/Y coordinates?

That was my intention with the MoonwalkingTempPoint stuff. Did I do it wrong?

 

[Uncle]

It sounds like you understand what I was saying. But to reiterate, the idea is that you're only leaking something when you can no longer get rid of it. Since you've saved the Point (location) in the Hashtable you have now created a reference to it. The Point variable is no longer needed as the Hashtable has basically taken over it's job of tracking the Location.


This trigger creates 1 Point (location) leak:

• Actions
• Set Variable TempPoint = Center of map
• Set Variable TempPoint = Center of map

A Location now exists in the game's memory containing the X/Y coordinates of the Center of the map. There's no way for you to EVER get rid of this Location because TempPoint has stopped tracking it.


This trigger doesn't leak:

• Actions
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 1 of 0 in Hashtable.
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 2 of 0 in Hashtable.
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 3 of 0 in Hashtable.

Why? Because these Points (locations) are being tracked by the Hashtable. We don't need TempPoint to track them anymore.
It's important to note that part of the reason it doesn't leak is because they have different positions in the Hashtable -> 1, 2, and 3.


The problem occurs when we run this trigger AGAIN before cleaning up the three previously saved Points:

• Actions
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 1 of 0 in Hashtable.
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 2 of 0 in Hashtable.
• Set Variable TempPoint = Center of map
• Hashtable - Save Handle Of TempPoint as 3 of 0 in Hashtable.

In this case we would have 3 leaks.

This is because we've now lost reference to our 3 original Points that were saved in the Hashtable. The original 3 Points have been pushed out of the Hashtable while the new 3 Points have taken their place, HOWEVER, the original 3 still exist in the game's memory. They're officially called memory leaks because we cannot get access to them anymore. And since we can't get access to them we can't use RemoveLocation() on them.

 

[hugope]

Every local variable needs to be nulled with the exception of: integers, reals, strings, booleans

What about global variables? For example a timer one:

• Set VariableSet MoonwalkingTimer = (Load 1 of (Key (Triggering unit).) in Hashtable.)
• Custom script: call PauseTimer(udg_MoonwalkingTimer)
• Custom script: call DestroyTimer(udg_MoonwalkingTimer)
• Custom script: set udg_MoonwalkingTimer = null

 

[Uncle]

What about global variables? For example a timer one:

• Set VariableSet MoonwalkingTimer = (Load 1 of (Key (Triggering unit).) in Hashtable.)
• Custom script: call PauseTimer(udg_MoonwalkingTimer)
• Custom script: call DestroyTimer(udg_MoonwalkingTimer)
• Custom script: set udg_MoonwalkingTimer = null

Global variables don't need to be nulled. You can simply replace whatever data they're currently tracking (if any) with something new, of course after cleaning up anything that would leak (Point, Group, etc). That being said, it isn't a bad thing to null a global variable, and could come in handy in certain situations.

The fundamental difference between locals and globals is that you can always access a global variable from anywhere, but a local variable can only be accessed inside of the function/trigger that it was created in.

Understand that brand new local variables are created each time a function/trigger runs that uses them. Once that function/trigger is finished, the local variables need to be cleaned up, and marking them as null is the final part of that process. Again, exceptions apply to integers, reals, strings, and booleans.

(Note that you don't need to null locals when using Lua, this is a Jass only thing. Lua's garbage collector will automatically collect and remove local variables for you, however, you still need to clean up Points, Groups, etc...)

A global variable is a static thing that is created once before the game starts and is reused throughout the rest of the game. You sometimes want to clean up (remove/destroy) the thing that the global variable is tracking, but the variable itself doesn't need to be nulled.

 

[hugope]

I wrote code for some timers. I pause and destroy those timers later in a GUI trigger. Should I use local variables to pause and delete those timers or is it okay to use a global variable to do it? Perhaps I'm being too fussy?

 

[Uncle]

I wrote code for some timers. I pause and destroy those timers later in a GUI trigger. Should I use local variables to pause and delete those timers or is it okay to use a global variable to do it? Perhaps I'm being too fussy?

I don't think you're fully understanding how this all works. Maybe my explanations have been a bit confusing, I struggled with the concept myself at first.
"Should I use local variables to pause and delete those timers or is it okay to use a global variable to do it?", this doesn't make any sense to me.

There's only one way to Pause and Destroy timers and that's with functions:

call PauseTimer(timer)
call DestroyTimer(timer)

You can put a local or global timer variable into those (parenthesis) and both will work properly as long as the variable actually contains a timer object.

A timer variable, local or global, simply points to the timer object that it's tracking. That timer object is what you're pausing/destroying in order to stop the timer and to avoid leaks. If you want to temporarily stop but reuse a timer then you can use the PauseTimer/ResumeTimer functions.

Now understand that in GUI the initialization process for global timer variables is out of your control. When you create a global timer variable in the Variable Editor it automatically creates a timer object to go along with it using the CreateTimer() function.

In Jass, you have control over the creation of the timer object:

timer TimerA
timer TimerB = CreateTimer()
// both of these are timer variables, it's just that TimerB is actually tracking a timer object while TimerA is tracking nothing

So hopefully now you understand the difference between a timer variable and a timer object. One tracks the timer object, one is the timer itself.

Continuing from there, we know that local variables have an extra step in that they need to be nulled when we're finished using them (excluding int/real/bool/string). This is because the variables themselves can leak due to Warcraft 3's poor design. By setting them to null you're telling the game that it's okay to get rid of the local variable for good. Note that this is no longer an issue in Lua mode as the garbage collector will automatically clean these up for you.

Global variables on the other hand always exist in the game's memory whether they're tracking something or not and cannot be gotten rid of. What you can do is remove the object that the global variable is tracking. You can also null the global variable which in this case simply tells it to stop tracking anything. Nulling a global does not get rid of the object it's tracking, it simply breaks the connection between the two. It's entirely unnecessary to null a global if you're trying to prevent memory leaks as these are unrelated. Nulling a global can be useful if you want to check the state of it in a condition, like -> If MyVariable == null then do something.

 

[hugope]

What should I do in the next situation?
1. I create a timers with code in JASS and start them. The timers are repeatable. I don't destroy the timer objects when they expire because I'm going to use them later.
2. Later, I use the time objects in a GUI trigger.
3. I no longer need the timer objects, so I'm going to assign variables to them to pause them and destroy them.

Should I use local variables or a global variable to destroy them?

My point of view:
Global variables start a timer object in the initialization, so that's a con.
Local variables have to be created each time the trigger is run, so that's a con.

 

[Uncle]

Since they're being used in a GUI trigger you don't have a choice, they need to be global. Why not just use Custom script to destroy them?

• Actions
• Custom script: call PauseTimer(YourTimer)
• Custom script: call DestroyTimer(YourTimer)

"3. I no longer need the timer objects, so I'm going to assign variables to them to pause them and destroy them."

They should already have variables assigned to them. If you created them using local variables then you have to destroy them inside of their Callback function using GetExpiredTimer() or inside of the same function that created them. Otherwise, you'll have no way of getting them again (leak).

If you show your triggers/code I can give a better answer.

 

[hugope]

"3. I no longer need the timer objects, so I'm going to assign variables to them to pause them and destroy them."

They should already have variables assigned to them. If you created them using local variables then you have to destroy them inside of their Callback function using GetExpiredTimer() or inside of the same function that created them. Otherwise, you'll have no way of getting them again (leak).

If you show your triggers/code I can give a better answer.

I assume the Callback is the code that runs when the timer expires, hope I'm right. This is the code and trigger I'm doing:

Code and GUI trigger

function TriggerMovementRight takes unit source returns nothing
    local timer t = CreateTimer()
    local integer id = GetHandleId(t)
    local integer idunit = GetHandleId(source)
    local player owner = GetOwningPlayer(source)
    local integer idowner = GetPlayerId(owner)

    call SaveUnitHandle(udg_Hashtable, id, 0, source)
    call SaveUnitHandle(udg_Hashtable, idowner, 100, source)
    call SaveTimerHandle(udg_Hashtable, idunit, 1, t)

    call TimerStart(t, 0.01, true, function TriggerMovementRight_Expire)
  
    set t = null
    set owner = null
endfunction


function TriggerMovementRight_Expire takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local integer id = GetHandleId(t)
    local real speed = 4
    local unit source = LoadUnitHandle(udg_Hashtable, id, 0)
    call SetUnitX(source, GetUnitX(source) + speed)

    set t = null
    set source = null
endfunction

• RemoveUnits Copy
  • Events
    • Player - Player 1 (Red) Selects a unit
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Triggering unit) Equal to MoonwalkUnit[1]
        • (Load MoonwalkIntegerOfRemovingUnits of MoonwalkPOwnerOfUnit from Hashtable.) Not equal to 1
      • Then - Actions
        • Unit - Kill MoonwalkUnit[1]
        • Unit - Remove MoonwalkUnit[1] from the game
        • Set VariableSet MoonwalkingTimer = (Load 1 of (Key (Triggering unit).) in Hashtable.)
        • Custom script: call PauseTimer(udg_MoonwalkingTimer)
        • Custom script: call DestroyTimer(udg_MoonwalkingTimer)
      • Else - Actions

After analyzing it more I just realized I don't even have to use variables. I can pause it and destroy it directly from the hashtable, though, I find it more ergonomic to use variables. Here's what I just realized I could do:

• Custom script: call PauseTimer(LoadTimerHandle(udg_Hashtable, GetHandleId(GetTriggerUnit()), 1))
• Custom script: call DestroyTimer(LoadTimerHandle(udg_Hashtable, GetHandleId(GetTriggerUnit()), 1))

 

[Uncle]

Ah, they're being stored in a Hashtable. That's what makes them global essentially.

It looks like what you're doing is fine as far as I know.

The only thing that could leak would be if you called TriggerMovementRight() multiple times in a row before destroying the timer object inside it's hashtable.