• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

Things That Leak

Level 20
Joined
Feb 23, 2014
Messages
1,264
Will this leak every 2 second? Or just keep one consistent group?
It won't leak - you're reusing a unit group that already exists and is assigned to a variable.

Leaks happen when you create a unit group that you have no reference to or do something to lose an existing reference (e.g. assigning a variable that held a unit group to another unit group, without storing it under another variable first). None of that happens here, so you're good.
 
Last edited:

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
View attachment 387961
Will this leak every 2 second? Or just keep one consistent group?

PS: This is just a trigger I made up. I'm not actually using it. I'm just wondering if a new group is created if I add a new unit to a cleared group.
No leaks, as there is no group being created. You're just referencing the existing UG_ActiveWave group.
 
Level 13
Joined
Jun 23, 2009
Messages
294
are there things that leak but cannot be destroyed? other than the ones mentioned in OP (all of which can be destroyed)
All leaks can be fixed by either destroying the leaking variable or nulling it except strings iirc. (correct me if I'm wrong, people out there!)
EDIT: According to various sources around the site units created always leak around 0.04 kb no matter how you handle them

Anyway, I need a refresher on this whole deal, I think I was told once that if you use unit variables to point at units with expiration timers you don't actually need to null the unit variable afterwards because when the unit expires it's basically null anyway, is that correct?

In other words does code like this:
JASS:
private function Flame takes nothing returns nothing
local unit dummy
local flame f = GetTimerData(GetExpiredTimer())
set dummy = CreateUnit(f.p, TWKDummy, f.x, f.y, bj_UNIT_FACING)
    call UnitAddAbility(dummy, DUMMY_SPELL)
    call UnitApplyTimedLife(dummy, 'BTLF', 1.00)
    call IssuePointOrder(dummy, "flamestrike", f.x, f.y)
    call ReleaseTimer(GetExpiredTimer())
    call f.destroy()
endfunction
Leak when it comes to the unit variable?

Bonus question: Timers are to be destroyed only when they're no longer needed, does that logic also apply to vJass structs? Like, if I pass the same struct through multiple functions and put them into local variables each time (like in this case local flame f, that was made together with the timer that's bringing it over) do I have to destroy those only in the last function they're used?
 
Last edited:
Level 9
Joined
Mar 26, 2017
Messages
376
Leaks that are concerned are mostly game engine objects and/or their attached handle.

The function above will cause a handle leak. Although the unit will be removed from memory after decay, the unit handle will remain. This is due to a bug in JASS where a local variable after function completion is not credited for its loss of reference to the referenced object. This is only achieved through explicitly setting to null. Else the unit handle will never be removed from memory by game engine cleanup process.

In Lua there is no such issue, and it is not needed to null the local.


Other question: It depends on what data is obtained by function GetTimerData.
The function GetExpiredTimer returns a reference to the Timer handle. If this handle will be stored in another variable indefinitely, it will leak. As long as least one variable references a particular handle, that handle will remain in memory.
 
Level 13
Joined
Jun 23, 2009
Messages
294
Thank you for the explanation, it's mostly clear to me now, aside from this:
Other question: It depends on what data is obtained by function GetTimerData.
GetTimerData is from TimerUtils 2.0, which is the last version Vexorian made iirc: function GetTimerData takes timer t returns integer
Based on this and the fact vJass (and by extension JassHelper) accepts structs as integers I would assume that I'm fine with not nulling/zeroing the structs at the end of every single function I use them in and that destroying them after having been used is enough, my structs however often contain handles so that's where I'm getting confused.
Granted, I could probably look at what the compiled code looks like and I'll have my answer but still... asking doesn't hurt anybody so, using the simplest snippet of code I can find atm:
vJASS:
scope SneakAttack initializer Init

globals
    private constant integer SPELL = 'Bek1'
    private constant integer BaseDamage = 75
    private constant integer IncDamage = 75
endglobals

private struct caster
    unit u

    static method create takes unit casting returns caster
        local caster Cas = caster.allocate()
    set Cas.u = casting
    return Cas
    endmethod

endstruct

private function Unpause takes nothing returns nothing
local caster u = GetTimerData(GetExpiredTimer())
call PauseUnit(u.u, false)
call ReleaseTimer(GetExpiredTimer())
call u.destroy()
endfunction

private function Main takes nothing returns boolean
local caster u
if GetSpellAbilityId() == SPELL then
    set u = caster.create(GetTriggerUnit())
    call PauseUnit(GetTriggerUnit(), true)
    call DestroyEffect(AddSpecialEffectTarget("Abilities\\Spells\\NightElf\\Blink\\BlinkCaster.mdl", GetTriggerUnit(), "origin"))
    call SetUnitPosition(GetTriggerUnit(), GetUnitX(GetSpellTargetUnit()), GetUnitY(GetSpellTargetUnit()))
    call SetUnitFacing(GetTriggerUnit(), bj_RADTODEG * Atan2(GetUnitY(GetSpellTargetUnit()) - GetUnitY(GetTriggerUnit()), GetUnitX(GetSpellTargetUnit()) - GetUnitX(GetTriggerUnit())))
    call SetUnitAnimation(GetTriggerUnit(), "attack")
    call UnitDamageTarget(GetTriggerUnit(), GetSpellTargetUnit(), BaseDamage + (IncDamage * GetUnitAbilityLevel(GetTriggerUnit(), SPELL)), true, false, ATTACK_TYPE_MELEE, DAMAGE_TYPE_ENHANCED, WEAPON_TYPE_METAL_LIGHT_SLICE)
    call FadingText(null, I2S(BaseDamage + (IncDamage * GetUnitAbilityLevel(GetTriggerUnit(), SPELL))) + "!", 255, 20, 50, GetUnitX(GetSpellTargetUnit()), GetUnitY(GetSpellTargetUnit()), 0.04, 2, 5, 10)
    if GetWidgetLife(GetSpellTargetUnit()) > 0.405 then
        call IssueTargetOrder(GetTriggerUnit(), "attack", GetSpellTargetUnit())
    endif
    call TimerStart(NewTimerEx(u), .9, false, function Unpause)
endif
return false
endfunction

private function Init takes nothing returns nothing
local trigger t = CreateTrigger()

    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function Main))

set t = null
endfunction

endscope
Is it fine like this? Can local caster u in function Main cause any sort of leaks?
 
Last edited:
@Michael Peppers calling destroy() on struct will internally make the struct instance (integer) be available again for new struct creations. It does this by calling this.deallocate(). Extra setting to 0 or something is not needed, and calling destroy() only once is enough.

If the struct has agent members, like units, they should be nulled together when an instance is destroyed. One usually defines own destroy() function, and puts the clean up logics there:
JASS:
method destroy takes nothing returns nothing
    call this.deallocate()          
    set this.target = null
endmethod

GetExpiredTimer() does not really leak, as the timer being recycled in used library. Same timer can/will be used again. But sure, one can say, in a scenario where a recyled timer won't be ever used again, then it results same as reference leak. But not too dangerous imo.

 
Level 13
Joined
Jun 23, 2009
Messages
294
Perfect, everything's clear now, thanks to you both :grin:

GetExpiredTimer() does not really leak, as the timer being recycled in used library. Same timer can/will be used again. But sure, one can say, in a scenario where a recyled timer won't be ever used again, then it results same as reference leak.
I assume that a single reference leak is small potatoes compared to the memory saved by TimerUtils through recycling timers over and over again instead of creating them each time.
 
Last edited:
Level 2
Joined
Aug 6, 2022
Messages
9
Hi all,
Coming back around to map editing, it's been a while so I'm afraid I might be rusty... Could anyone check this trigger and tell me if it's "clean" enough, or if there are some leaks I missed? Thanks
1659800277271.png
 
Level 2
Joined
Aug 6, 2022
Messages
9
Your last "RemoveLocation(udg_Location(CasterMI))" should be before the "Unit Group - pick every unit" scope
True, I don't use that variable after I begin this loop. Does it make a difference in terms of leak, whether I remove it before or after the loop?
 
Level 38
Joined
Feb 27, 2007
Messages
4,951
There's no reason to save the dummies in an array, wait, and then remove them all directly. Instead of all that just give each dummy a 1.0 second expiration timer when they're created. Also note that a properly configured dummy unit (speed 0, movement None, 0.00 cast backswing, using a model without cast animations (or no model at all)) can cast instantaneously in any direction without turning, which means that you can do something like this where only one dummy is needed and it successfully casts invisibility on all affected units by itself.
 
Level 2
Joined
Aug 6, 2022
Messages
9
There's no reason to save the dummies in an array, wait, and then remove them all directly. Instead of all that just give each dummy a 1.0 second expiration timer when they're created. Also note that a properly configured dummy unit (speed 0, movement None, 0.00 cast backswing, using a model without cast animations (or no model at all)) can cast instantaneously in any direction without turning, which means that you can do something like this where only one dummy is needed and it successfully casts invisibility on all affected units by itself.
Thanks, I forgot about expiration timers; they remove units without leaks? If so, that would indeed be simpler.
As for the number of dummy units, I understand your point. Since I've done it with multiple units, I'll keep it this way for now, but I'll try it your way for the next one.
Thanks all for your help!
 
Level 6
Joined
Jul 22, 2015
Messages
65
  • unitgroup.gif
    Unit Group - Pick all units of type Footman and do...
  • cam.gif
    Camera - Pan Camera as needed...
i still didn't understand of this....
so if i using this trigger it will leak right?

  • HeroSpawn
    • Events
      • Unit - A unit Dies
    • Conditions
      • ((Triggering unit) is A Hero) Equal to True
      • (Owner of (Triggering unit)) Not equal to Player 23 (Emerald)
      • (Owner of (Triggering unit)) Not equal to Player 24 (Peanut)
    • Actions
      • Wait 5.00 seconds
      • Selection - Clear selection for (Owner of (Triggering unit))
      • Set PM_Loop = (Custom value of (Triggering unit))
      • Set Point[PM_Loop] = (Center of Region[CheckPoint[PM_Loop]])
      • Hero - Instantly revive (Triggering unit) at Point[PM_Loop], Show revival graphics
      • Unit - Set mana of (Triggering unit) to 100.00%
      • Selection - Select (Triggering unit) for (Owner of (Triggering unit))
      • Camera - Pan camera for (Owner of (Triggering unit)) to Point[PM_Loop] over 0.00 seconds
      • Custom script: call RemoveLocation(udg_Point[udg_PM_Loop])
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
i still didn't understand of this....
so if i using this trigger it will leak right?

  • HeroSpawn
    • Events
      • Unit - A unit Dies
    • Conditions
      • ((Triggering unit) is A Hero) Equal to True
      • (Owner of (Triggering unit)) Not equal to Player 23 (Emerald)
      • (Owner of (Triggering unit)) Not equal to Player 24 (Peanut)
    • Actions
      • Wait 5.00 seconds
      • Selection - Clear selection for (Owner of (Triggering unit))
      • Set PM_Loop = (Custom value of (Triggering unit))
      • Set Point[PM_Loop] = (Center of Region[CheckPoint[PM_Loop]])
      • Hero - Instantly revive (Triggering unit) at Point[PM_Loop], Show revival graphics
      • Unit - Set mana of (Triggering unit) to 100.00%
      • Selection - Select (Triggering unit) for (Owner of (Triggering unit))
      • Camera - Pan camera for (Owner of (Triggering unit)) to Point[PM_Loop] over 0.00 seconds
      • Custom script: call RemoveLocation(udg_Point[udg_PM_Loop])
No, that won't leak because you aren't using "Pan Camera as needed" and are just using the normal "Pan Camera" function.

However, if you are using a WarCraft 3 version >= 1.29, the problem with that function is resolved.
 
Level 4
Joined
Nov 1, 2015
Messages
11
1675689208743.png

Conditions can leak right? Here are the examples of the ones I use alot:
The 1st condition makes sense to leak since it spawns a new unassigned group.
But what about the 2nd condition? I searched around but don't see any info about this "Living units" condition.
As for 3rd condition am not sure if that one would leak if i keep setting different units to Temp_UnitType
 
View attachment 424171
Conditions can leak right? The 1st condition makes sense to leak since it spawns a new unassigned group, but what about the 2nd condition? I searched around but don't see any info about this "Living units" condition.
Short answer: Yes, they can leak, but "living units count" does not.

Long answer (Requires some amount of jass knowledge):

Let's find out.
1. create the "minimum" trigger with the thing we want to check:
  • Untitled Trigger 001
    • Events
    • Conditions
      • (Number of living Footman units owned by Player 1 (Red)) Equal to 0
    • Actions
2. Edit -> Convert to custom text and look for the function called
JASS:
function Trig_Untitled_Trigger_001_Conditions takes nothing returns boolean
    if ( not ( CountLivingPlayerUnitsOfTypeId('hfoo', Player(0)) == 0 ) ) then //<-- This seems to contain the words we are looking for (Living units). 
        return false
    endif
    return true
endfunction

function Trig_Untitled_Trigger_001_Actions takes nothing returns nothing
endfunction

//===========================================================================
function InitTrig_Untitled_Trigger_001 takes nothing returns nothing
    set gg_trg_Untitled_Trigger_001 = CreateTrigger(  )
    call TriggerAddCondition( gg_trg_Untitled_Trigger_001, Condition( function Trig_Untitled_Trigger_001_Conditions ) )
    call TriggerAddAction( gg_trg_Untitled_Trigger_001, function Trig_Untitled_Trigger_001_Actions )
endfunction
3. Open blizzard.j (found in My documents/JassHelper/) with a decent text editor and search for <thing> (CountLivingPlayerUnitsOfTypeId). (Note: unsure if any GUI-action is defined in Common.j)
4. See if it leaks by creating something that it doesn't clean up.
JASS:
function CountLivingPlayerUnitsOfTypeId takes integer unitId, player whichPlayer returns integer
    local group g
    local integer matchedCount
    set g = CreateGroup()  //<--- Creates a group
    set bj_livingPlayerUnitsTypeId = unitId
    call GroupEnumUnitsOfPlayer(g, whichPlayer, filterLivingPlayerUnitsOfTypeId)
    set matchedCount = CountUnitsInGroup(g)
    call DestroyGroup(g)  //<--- cleans it up
    return matchedCount
endfunction
5. In this case, it creates a group and cleans it up. Neat!
 
So the leak is avoided because the actual function itself had the command to clean it up, pretty useful thanks.
Yes, correct.
Apparently those functions exists. I have the feeling like most GUI-actions that interacts with groups and points leaked, so I'm surprised that this didn't to be honest.
 
Yep, only risk is the handle ID reference leak, which is something almost impossible to avoid in GUI anyway.
I considered mentioning it, but I thought I had enough details and complexity in my answer already, but good point.
I.E. In order to be "completely leak free" the jass that the GUI-condition is calling need to set group g null before returning the answer.
It's fairly minor compared to many other things, but in a 0.01 second timer, it could get bad after a while.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
I considered mentioning it, but I thought I had enough details and complexity in my answer already, but good point.
I.E. In order to be "completely leak free" the jass that the GUI-condition is calling need to set group g null before returning the answer.
It's fairly minor compared to many other things, but in a 0.01 second timer, it could get bad after a while.
Switching to Lua would fix all such problems without having to modify the blizzard.j file.
 
Level 17
Joined
Jun 2, 2009
Messages
1,112
Hello. I have to ask something about leaks. This trigger runs every seconds. Should i destroy DestekGrubu and PG_Income every seconds? I am totally confused at this point. Because these groups not for temporary actions. Every seconds i am giving extra gold to players within groups.

  • Her saniye gold
    • Events
      • Time - Every 1.00 seconds of game time
    • Conditions
    • Actions
      • Player Group - Pick every player in (All players) and do (Player - Add 1 to (Picked player) Current gold)
      • Player Group - Pick every player in DestekGrubu and do (Player - Add 1 to (Picked player) Current gold)
      • Player Group - Pick every player in PG_Income and do (Player - Add 2 to (Picked player) Current gold)
 
Level 4
Joined
Nov 1, 2015
Messages
11
Hello. I have to ask something about leaks. This trigger runs every seconds. Should i destroy DestekGrubu and PG_Income every seconds? I am totally confused at this point. Because these groups not for temporary actions. Every seconds i am giving extra gold to players within groups.
1676916425050.png

As an example, the first one should leak because it's a "temporary" group that was made in this trigger, but the second one should not because it's an already existing group in our variables, though somebody can correct me if am wrong. I've also heard that All Players does not leak, so everything in your trigger should be fine.
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
Hello. I have to ask something about leaks. This trigger runs every seconds. Should i destroy DestekGrubu and PG_Income every seconds? I am totally confused at this point. Because these groups not for temporary actions. Every seconds i am giving extra gold to players within groups.

  • Her saniye gold
    • Events
      • Time - Every 1.00 seconds of game time
    • Conditions
    • Actions
      • Player Group - Pick every player in (All players) and do (Player - Add 1 to (Picked player) Current gold)
      • Player Group - Pick every player in DestekGrubu and do (Player - Add 1 to (Picked player) Current gold)
      • Player Group - Pick every player in PG_Income and do (Player - Add 2 to (Picked player) Current gold)
None of these leak.
 
Level 38
Joined
Feb 27, 2007
Messages
4,951
Should i destroy DestekGrubu and PG_Income every seconds? I am totally confused at this point. Because these groups not for temporary actions.
The only differences between something being a "leak" and something being "permanent data" is whether or not you re-use it (you are) and whether or not the reference (sometimes called a pointer) to that object is 'lost' (these are not lost). Neither is true here, so neither of those player groups should be considered "leaks"; they are permanent data for you.
 
Level 3
Joined
Nov 26, 2009
Messages
35
Should i clear variables(caster, casterPoint, dummy?) in this function?
JASS:
function a takes unit caster, location casterPoint, real distance, unit dummy returns  nothing
   
endfunction
 

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
Should i clear variables(caster, casterPoint, dummy?) in this function?
JASS:
function a takes unit caster, location casterPoint, real distance, unit dummy returns  nothing
  
endfunction
You do not need to null any of those variables. Parameters don't cause handle reference leaks like local variables do.

As Wrda said, you do need to call RemoveLocation, provided it's not a persistent location that is needed for something like a projectile system or Spell System.
 
Level 6
Joined
Sep 10, 2022
Messages
65
  • Custom script: set bj_wantDestroyGroup = true
  • Unit Group - Pick every unit in (Playable Map Area) and do (Unit - Hide (Picked unit))
Do* I need to write "set bj_wantDestroyGroup = true" every time before every new "Unit Group - Pick every unit in (Playable Map Area) and do (Unit - Hide (Picked unit))" ?
 
Last edited:

Bribe

Code Moderator
Level 50
Joined
Sep 26, 2009
Messages
9,456
  • Custom script: set bj_wantDestroyGroup = true
  • Unit Group - Pick every unit in (Playable Map Area) and do (Unit - Hide (Picked unit))
Does I need to write "set bj_wantDestroyGroup = true" every time before every new "Unit Group - Pick every unit in (Playable Map Area) and do (Unit - Hide (Picked unit))" ?
Yes, before each one. Except in Lua when using Lua-infused GUI.
 
Level 38
Joined
Feb 27, 2007
Messages
4,951
Frankly, Herly, there's no reason for anyone to use any of those functions which is why it's not part of the list.
  • Entire Map is pointless because no units can go into the bounds that it includes that Playable Map Area does not include. There's a reason this isn't just a global rect variable and instead returns a new rect object each time it's called.
  • Using regions for enter/leave events is reasonable but you should just define them in the region editor rather than making them dynamically.
  • 99% of the time you want to do something in an area (search, move objects, etc.) you want to do it in a circle rather than a rectangle, so GroupEnum... functions are the better choice. That covers basically every other use case for creating a dynamic region.
There is this native to properly destroy rects: native RemoveRect takes rect whichRect returns nothing.
 
Level 23
Joined
Jun 26, 2020
Messages
1,838
Frankly, Herly, there's no reason for anyone to use any of those functions which is why it's not part of the list.
  • Entire Map is pointless because no units can go into the bounds that it includes that Playable Map Area does not include. There's a reason this isn't just a global rect variable and instead returns a new rect object each time it's called.
  • Using regions for enter/leave events is reasonable but you should just define them in the region editor rather than making them dynamically.
  • 99% of the time you want to do something in an area (search, move objects, etc.) you want to do it in a circle rather than a rectangle, so GroupEnum... functions are the better choice. That covers basically every other use case for creating a dynamic region.
There is this native to properly destroy rects: native RemoveRect takes rect whichRect returns nothing.
Yeah I know about that native.
I was talking for that 1% of the cases when someone want to use a rectangle to check.
 
Top