• 🏆 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!

Leak or not?

Status
Not open for further replies.
Level 14
Joined
May 22, 2010
Messages
362
Does this trigger leak and how do I fix the leaks?
  • MultiShot1
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Multi Shot
    • Actions
      • Set CastingUnit = (Casting unit)
      • Set Location1 = (Position of CastingUnit)
      • Unit - Create 1 MultiShot (Dummy) for (Owner of CastingUnit) at Location1 facing ((Facing of CastingUnit) + (Random real number between 10.00 and 25.00)) degrees
      • Unit - Add a 0.60 second Generic expiration timer to (Last created unit)
      • Unit - Set level of Multi Shot SUP for (Last created unit) to (Level of Multi Shot for CastingUnit)
      • Custom script: call RemoveLocation(udg_Location1)
      • Set Location1 = (Position of (Last created unit))
      • Set Location2 = (Location1 offset by 256.00 towards (Facing of (Last created unit)) degrees)
      • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm Location2
      • Custom script: call RemoveLocation(udg_Location1)
      • Custom script: call RemoveLocation(udg_Location2)
 
Level 8
Joined
Sep 7, 2008
Messages
320
Does this trigger leak and how do I fix the leaks?
  • MultiShot1
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Multi Shot
    • Actions
      • Set CastingUnit = (Casting unit)
      • Set Location1 = (Position of CastingUnit)
      • Unit - Create 1 MultiShot (Dummy) for (Owner of CastingUnit) at Location1 facing ((Facing of CastingUnit) + (Random real number between 10.00 and 25.00)) degrees
      • Unit - Add a 0.60 second Generic expiration timer to (Last created unit)
      • Unit - Set level of Multi Shot SUP for (Last created unit) to (Level of Multi Shot for CastingUnit)
      • Custom script: call RemoveLocation(udg_Location1)
      • Set Location1 = (Position of (Last created unit))
      • Set Location2 = (Location1 offset by 256.00 towards (Facing of (Last created unit)) degrees)
      • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm Location2
      • Custom script: call RemoveLocation(udg_Location1)
      • Custom script: call RemoveLocation(udg_Location2)

Remove Leak

  • Set CastingUnit = (No unit)
 
Level 20
Joined
Jul 14, 2011
Messages
3,213
No. Warcraft 3 will never create two units at the same time They'll be created with 0.00001 seconds difference. Also there's some kind of "in-function" thing that makes the (last created unit) the last created unit within the same trigger, as long as there isn't any kind of "wait" action all the functions you do inside the trigger will work almost simultaneously (this means you can create the unit, do other 100 actions, and then set do something with the -last created unit- and it will take the unit created 100 actions before in the same function, even if you have several functions working the same way).
 
Level 14
Joined
May 22, 2010
Messages
362
1.So I dont have to create a global variable(Last Created Unit) for storing the last created unit?
2.Another thing, I want to use array variables for the locations, but how do I remove array variables?
Lets say Location[0] Location[1] Location[2]
Is this how it is done?
  • Custom script: call RemoveLocation(udg_Location[1])
And will this clear only location[1] or all of the trigger arrays?
 
Level 33
Joined
Mar 27, 2008
Messages
8,035
Replace (Casting unit) to (Triggering unit), works more fast.

Since your dummy location and your position of your triggering unit is at the same place, just re-use it, you don't need to create and destroy and create it again, basically you are repeating the same thing.

Therefore, you should remove the Custom script lines in the first above code and just let it destroys once.

And set it once too (Position of (Triggering unit)).

Instead of creating a location again, basically you are creating a dummy at your own location, therefore it's the same location.

  • Unit - Add a 0.60 second Generic expiration timer to (Last created unit)
You could set it to minimum of 0.20 ~ 0.30 second of unit expiration time.
The time difference is quite small but when you play a spell that spams a lot, you know the difference.
You can also use a dummy recycler, but that requires more advance, no need for that for now.

Your overall trigger should look like this:
  • MultiShot1
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Multi Shot
    • Actions
      • Set CastingUnit = (Triggering unit)
      • Set Location1 = (Position of CastingUnit)
      • Unit - Create 1 MultiShot (Dummy) for (Owner of CastingUnit) at Location1 facing ((Facing of CastingUnit) + (Random real number between 10.00 and 25.00)) degrees
      • Unit - Add a 0.20 second Generic expiration timer to (Last created unit)
      • Unit - Set level of Multi Shot SUP for (Last created unit) to (Level of Multi Shot for CastingUnit)
      • Set Location2 = (Location1 offset by 256.00 towards (Facing of (Last created unit)) degrees)
      • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm Location2
      • Custom script: call RemoveLocation(udg_Location1)
      • Custom script: call RemoveLocation(udg_Location2)
 
Level 20
Joined
Jul 14, 2011
Messages
3,213
· Whenever you use a Generic Unit Event (A unit xxxx) you can use "Triggering Player" to replace "Owner of Triggering Unit" and it's better/faster/easier this way.

· To improve triggers you have to reduce parentheses as much as you can. That's why you use a Unit Variable (CastingUnit) to work with the (Triggering Unit). Since you're going to use (Triggering Unit) several times, it's better to save it into a variable so the game doesn't have to look for it over and over. Same happens with (Last Created Unit) I would create another variable for it, to avoid repeating (Last Created Unit) several times.

Whenever you're going to use something more than 2 times, it's better to use a variable.

  • MultiShot1
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
      • (Ability being cast) Equal to Multi Shot
    • Actions
      • Set CastingUnit = (Triggering unit)
      • Set Location1 = (Position of CastingUnit)
      • Unit - Create 1 MultiShot (Dummy) for (Triggering Player) at Location1 facing ((Facing of CastingUnit) + (Random real number between 10.00 and 25.00)) degrees
      • Set DummyUnit = (Last Created Unit)
      • Unit - Add a 0.20 second Generic expiration timer to DummyUnit
      • Unit - Set level of Multi Shot SUP for DummyUnit to (Level of Multi Shot for CastingUnit)
      • Set Location2 = (Location1 offset by 256.00 towards (Facing of DummyUnit ) degrees)
      • Unit - Order DummyUnit to Undead Dreadlord - Carrion Swarm Location2
      • Custom script: call RemoveLocation(udg_Location1)
      • Custom script: call RemoveLocation(udg_Location2)

<< EDIT >>

This is how it looks in JASS

JASS:
//As you can see, WC3 creates a function for the condition --> Trig_JASS_Trigger_Conditions <--
// In this case, we don't even need a function for this. We can do it with an "If/Then/Else" inside the main action function 
function Trig_JASS_Trigger_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'AUan' ) ) then
        return false
    endif
    return true
endfunction

// This is the main Action function "Trig_JASS_Trigger_Actions"
function Trig_JASS_Trigger_Actions takes nothing returns nothing
    set udg_CastingUnit = GetTriggerUnit() // You can see that Global Variables have an "udg_" prefix 
    set udg_Location1 = GetUnitLoc(udg_CastingUnit) // Same happens here
    call CreateNUnitsAtLoc( 1, 'hfoo', GetTriggerPlayer(), udg_Location1, ( GetUnitFacing(udg_CastingUnit) + GetRandomReal(10.00, 25.00) ) )
    set udg_DummyUnit = GetLastCreatedUnit()
    call UnitApplyTimedLifeBJ( 0.20, 'BTLF', udg_DummyUnit )
    call SetUnitAbilityLevelSwapped( 'AOvd', udg_DummyUnit, GetUnitAbilityLevelSwapped('ANab', udg_CastingUnit) )
    set udg_Location2 = PolarProjectionBJ(udg_Location1, 256, GetUnitFacing(udg_DummyUnit))
    call IssuePointOrderLocBJ( udg_DummyUnit, "carrionswarm", udg_Location2 )
    call RemoveLocation(udg_Location1)
    call RemoveLocation(udg_Location2)
endfunction

// All the RED actions can (and should) be improved, since these call another actions that we can call directly, reducing
// greatly the amount of actions, and therefore, the trigger speed and efficiency.

//===========================================================================
function InitTrig_JASS_Trigger takes nothing returns nothing
    set gg_trg_JASS_Trigger = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_JASS_Trigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_JASS_Trigger, Condition( function Trig_JASS_Trigger_Conditions ) )
    call TriggerAddAction( gg_trg_JASS_Trigger, function Trig_JASS_Trigger_Actions )
endfunction

We can improve it by removing BJ's and actions that call more actions. It will seem harder, but it's easier and better.
It's better to work with X/Y coordenates than using Locations, since they're faster, and don't need to be removed.

JASS:
function Trig_JASS_Trigger_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'AUan'
endfunction

function Trig_JASS_Trigger_Actions takes nothing returns nothing
    //locals have to be created/delcared before anything else in the function.
    local unit CastingUnit = GetTriggerUnit() //The triggering unit
    local unit DummyUnit //We'll give a value to it later
    local real CUx = GetUnitX(CastingUnit) // Casting Unit X
    local real CUy = GetUnitY(CastingUnit) // Casting Unit Y
    local real FA // Facing angle of the Dummy Unit. We'll set it later
    local real PolarX // X of Polar Projection of Location2
    local real PolarY // Y of Polar Projection of Location2
    
    // We set DummyUnit equal to Creating the Unit. Set DummyUnit equal to "CreateUnit" is better than "CreateNUnitsAtLoc" and then setting udg_DummyUnit = LastCreatedUnit
    set DummyUnit = CreateUnit(GetTriggerPlayer, 'ahoo', CUx, CUy, (GetUnitFacing(CastingUnit) + GetRandomReal(10.00, 25.00)))
    call UnitApplyTimedLife(DummyUnit, 'BTLF', 0.20) // Expiration Timer
    call SetUnitAbilityLevel(DummyUnit, 'AOvd', GetUnitAbilityLevel(CastingUnit, 'ANab')) // Ability Level
    set FA = GetUnitFacing(DummyUnit) * bj_DEGTORAD) // Now we set the Facing Angle * bj_DEGTORAD to reduce repetitive calling (Check FA repeats twice in the next actions)
    set PolarX = CUx + 256 * Cos(FA) // X of the Polar Projection
    set PolarY = CUy + 256 * Sin(FA) // Y of the Polar Projection
    call IssuePointOrder(DummyUnit, "carriongswarm", PolarX, PolarY) // The Carrion Swarm order with coords

// Now we null the Unit variables. Reals doesn't need to be nulled    
    set CastingUnit = null
    set DummyUnit = null
endfunction

//===========================================================================
function InitTrig_JASS_Trigger takes nothing returns nothing
    set gg_trg_JASS_Trigger = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_JASS_Trigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_JASS_Trigger, Condition( function Trig_JASS_Trigger_Conditions ) )
    call TriggerAddAction( gg_trg_JASS_Trigger, function Trig_JASS_Trigger_Actions )
endfunction


JASS:
function Trig_JASS_Trigger_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'AUan'
endfunction

function Trig_JASS_Trigger_Actions takes nothing returns nothing
    local unit CastingUnit = GetTriggerUnit()
    local unit DummyUnit
    local real CUx = GetUnitX(CastingUnit)
    local real CUy = GetUnitY(CastingUnit)
    local real FA
    local real PolarX
    local real PolarY
    
    set DummyUnit = CreateUnit(GetTriggerPlayer, 'ahoo', CUx, CUy, (GetUnitFacing(CastingUnit) + GetRandomReal(10.00, 25.00)))
    call UnitApplyTimedLife(DummyUnit, 'BTLF', 0.20)
    call SetUnitAbilityLevel(DummyUnit, 'AOvd', GetUnitAbilityLevel(CastingUnit, 'ANab'))
    set FA = GetUnitFacing(DummyUnit) * bj_DEGTORAD)
    set PolarX = CUx + 256 * Cos(FA)
    set PolarY = CUy + 256 * Sin(FA)
    call IssuePointOrder(DummyUnit, "carriongswarm", PolarX, PolarY)
    
    set CastingUnit = null
    set DummyUnit = null
endfunction

//===========================================================================
function InitTrig_JASS_Trigger takes nothing returns nothing
    set gg_trg_JASS_Trigger = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_JASS_Trigger, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_JASS_Trigger, Condition( function Trig_JASS_Trigger_Conditions ) )
    call TriggerAddAction( gg_trg_JASS_Trigger, function Trig_JASS_Trigger_Actions )
endfunction

I hope it helps you to learn easily and understand how some things are done. Of course, you have to replace the ability raw code with the one you have in your map. You also have to replace the Dummy unit raw code with the one your Dummy has in your map. To see the raw code of things in the Object Editor press CTRL+D
 
Last edited:
Level 33
Joined
Mar 27, 2008
Messages
8,035
Unit does not leaks, get your fact straight.
However, localized variable of unit will leak if you don't null it.

There are 2 types of variables which is; Global | Local
Global is the variable which we can use from Trigger A to Trigger B, it can be related to save data whereas Local variable can only be used within the trigger itself.

So, in case you're using a global variable now, it won't leak.
You don't need to set it to No unit because you're using a Global variable instead Local.

Global variable is the variable created via Trigger Editor (CTRL + B).

You can make a global variable turns to local too (this needs nulling too to avoid leaks)
 
Level 33
Joined
Mar 27, 2008
Messages
8,035
Example Trigger:
  • Null
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
    • Actions
      • Custom script: local unit u = GetSpellTargetUnit()
      • Wait 5.00 seconds
      • Custom script: set u = null
If you're using a pure local variable, you must know all functions by writing it out, each of it.
If you're using a global variable turned into locals, you can use GUI functions and does not have to remember those syntax function

To make global variable a local, simply:

  • Global Null
    • Events
      • Unit - A unit Starts the effect of an ability
    • Conditions
    • Actions
      • Custom script: local unit udg_Target = GetSpellTargetUnit()
      • Wait 5.00 seconds
      • Custom script: set udg_Target = null
1. Create a global variable (CTRL + B) in Trigger Editor and name it Target
2. Set it as udg_Target = WhatYouWant
3. Use it.
 
Level 20
Joined
Jul 14, 2011
Messages
3,213
JASS is GUI without clothes on... Just as removing points, destroying groups, and else, you have to know some JASS cripts (nude gui functions, some unaccesible through the graphic interface) to clean stuff.
 
Level 33
Joined
Mar 27, 2008
Messages
8,035
It's set udg_Target = GetTriggerUnit() in GUI it means Set Target = (Triggering unit).

So in that case, it depends on what variable did you use, global -> local, or simply local.

Of it's from global, you have to add the "udg_" in front of it.

  • Custom script: set udg_Target = null
  • OR
  • Custom script: set Target = null
There is a reason to use local, mostly to make it MUI/MPI a data over certain things.
At your standard, it should be basic set of (Triggering unit) via normal Trigger Editor function Set Variable
 
Level 14
Joined
May 22, 2010
Messages
362
What I meant was
  • Trig
    • Events
      • Unit - A unit Begins casting an ability
    • Conditions
    • Actions
      • Unit - Set life of (Triggering unit) to 100.00%
      • Animation - Play (Triggering unit)'s spell slam animation
      • Unit - Reset ability cooldowns for (Triggering unit)
form what I understand this trigger will leak because of the multiple uses of the triggering unit.

  • Trig
    • Events
      • Unit - A unit Begins casting an ability
    • Conditions
    • Actions
      • Unit - Set life of (Triggering unit) to 100.00%
      • Animation - Play (Triggering unit)'s spell slam animation
      • Unit - Reset ability cooldowns for (Triggering unit)
      • Custom script: set udg_(Triggering Unit) = null
Will this make the trigger not leak?
 
Level 20
Joined
Jul 14, 2011
Messages
3,213
No.

Leak is something that remains there, and is unusefull, takes space, like trash. It happens when you don't destroy a texttag, or a special effect, or groups, and stuff, also when you don't null locals.

Besides leaks, we try to IMPROVE triggering avoiding REPETITIVE data search. Whenever you see (Triggering Unit) the game has to look for it. If you call 100 times (Triggering Unit) in a trigger, you're telling the game to look 100 times for the same unit. If you declare the Unit into a variable, and use it 100 times, the game just looks for the unit once, and from there on, it already knows which unit has to handle, since you already saved it into the variable.

I mean, it's not the same going 100 times to the marked to buy rice, if you can go once and buy 100 rice packages...

We also avoid ACTIONS that call ANOTHER ACTIONS (commonly Bj's) because there's no need to tell Action A to tell Action B what we want, if we can access inmediatly to action B without having to use Action A. This way it's faster, and lighter for the system to work with.

There are several lists around of what leaks in WC3...

· Locations
· Some locals
· TextTags (Floating texts)
· Generic Player Groups
· Generic Unit Groups
· Uncleared data from hashtables and variables
· Some camera things
· Some other stuff

You can null a unit in the game :p (Triggering Unit) is a unit, not a variable.

If you set MyUnit = (Triggering Unit) and then you set MyUnit = null, it's ok because you're nullying the variable, not the unit.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
a leak is when an instance is creating or pointing TO something like locations/timers/units etc...then not destroyed...
TriggeringUnit is not a leak coz you didnt create it, instead it's just pointing FROM that instance, but the unit itself leaks...
on the other hand if you create a UNIT local/global variable and points it TO TriggerUnit, then it will cause leak...

  • Custom script: set udg_(Triggering Unit) = null
that will cause error and you dont need to null global variables...
 
Level 17
Joined
Apr 27, 2008
Messages
2,455
Again.

...
and you dont need to null global variables...



Plus :


The problem with structs is that you can't always be sure that all instances will be re-used soon or late, or if handle members will be destroyed or not during the game.

As an example you could reach a max instances of 100 at some point during the game, but then use no more than 10 of them.
If you have 90 differents handles stored in these struct instances and they are likely to be destroyed later during the game, you will have 90 handle id leaks.
Now, that's probably an unrealistic scenario, but this + this + this ...
That's why i always null struct members on instance destroy.

Yeah talking about structs is overkill here since he doesn't care about vJass, but it's an other common error, i just felt to say it.
 
Last edited:
Status
Not open for further replies.
Top