[Solved] Fatal memory leak on terrain periodic check

[Swatosj]

Hey guys ! I'm working on a system which gives a buff to a unit according to the terrain path where the unit is.
I've tried many solutions out but it seems the trigger will go into an infinite loop after a few seconds of creeps spawning and walking on the "special" tiles.
The first slow buff is correctly applied but then the map starts to freeze and it's over.

Code is below :

• Creeps terrain effects
  • Events
    • Time - Every 0.61 seconds of game time
  • Conditions
  • Actions
    • -------- We are storing the units of AI players into an array --------
    • Set iZoneUnits = 0
    • For each (Integer B) from 21 to 22, do (Actions)
      • Loop - Actions
        • Unit Group - Pick every unit in (Units in P1Zone <gen> matching (((Owner of (Matching unit)) Equal to (Player((Integer B)))) and (((Matching unit) is A structure) Equal to False))) and do (Actions)
          • Loop - Actions
            • Set ZoneUnits[iZoneUnits] = (Picked unit)
            • Set iZoneUnits = (iZoneUnits + 1)
    • -------- We check the terrain under the unit --------
    • For each (Integer iUnitLoop) from 0 to iZoneUnits, do (Actions)
      • Loop - Actions
        • Set UnitLocations[iUnitLoop] = (Position of ZoneUnits[iUnitLoop])
        • -------- Icecrown - Dark Ice --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Terrain type at UnitLocations[iUnitLoop]) Equal to Icecrown Glacier - Dark Ice
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (ZoneUnits[iUnitLoop] has buff Slowed) Equal to False
              • Then - Actions
                • Unit - Create 1 dummy for (Owner of ZoneUnits[iUnitLoop]) at UnitLocations[iUnitLoop] facing Default building facing degrees
                • Unit - Add a 2.00 second Generic expiration timer to (Last created unit)
                • Unit - Add Slow Ice to (Last created unit)
                • Unit - Order (Last created unit) to Orc Shaman - Bloodlust ZoneUnits[iUnitLoop]
              • Else - Actions
          • Else - Actions
            • -------- Lava Cracks --------
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Terrain type at UnitLocations[iUnitLoop]) Equal to Dungeon - Lava Cracks
              • Then - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                  • Then - Actions
                    • Unit - Create 1 dummy for (Owner of ZoneUnits[iUnitLoop]) at UnitLocations[iUnitLoop] facing Default building facing degrees
                    • Unit - Add a 2.00 second Generic expiration timer to (Last created unit)
                    • Unit - Add Lava Speed to (Last created unit)
                    • Unit - Order (Last created unit) to Undead Death Knight - Death Coil ZoneUnits[iUnitLoop]
                  • Else - Actions
              • Else - Actions
                • -------- Normal Ice --------
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (ZoneUnits[iUnitLoop] has buff Slowed) Equal to True
                  • Then - Actions
                    • Unit - Remove Slowed buff from ZoneUnits[iUnitLoop]
                  • Else - Actions
        • Custom script: call RemoveLocation(udg_UnitLocations[udg_iUnitLoop])

The spawning (& a bit leaking) system is below :

• Spawn player 1
  • Events
    • Time - Every 2.20 seconds of game time
  • Conditions
    • (Number of units in (Units in P1Zone <gen> owned by Player 22 (Snow))) Less than 10
  • Actions
    • Unit - Create 1 Skeleton for Player 22 (Snow) at (Center of Player1Spawn <gen>) facing Default building facing degrees

Players 21 and 22 are the ones concerned by the system.

Thanks for reading and helping if you see some mistakes made.

 

[HelloFlighty]

Maybe a silly question but have you got a trigger that defines the variable

• iUnitLoop

You seem to use it quite a lot but I haven't seen a definition for the variable, perhaps post all of your triggers if there are more, in the meantime, I'll keep grazing over what you've posted.

EDIT:

It's asking if units in P1Zone owned by player 22 are less than 10 then proceeds to create a unit for player 22 in another region, so the number of units in P1Zone never changes and it will continue to create units every 2.2 seconds until there are 10 or more units in P1Zone, maybe change this from a periodic event to a timer that will only start if there are less than 10 units in that zone, example

• Untitled Trigger 001
  • Events
    • Time - Timer expires
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • NumberOfUnitsEqualTo10 Equal to False
      • Then - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Number of units in (Units in P1Zone owned by Player 22 (Snow))) Less than 10
          • Then - Actions
            • Unit - Create 1 Skeleton for Player 22 (Snow) at (Center of Player1Spawn) facing Default building facing degrees
            • Countdown Timer - Start Timer as a One-shot timer that will expire in 2.20 seconds
          • Else - Actions
            • Set NumberOfUnitsEqualTo10 = True
      • Else - Actions
        • Countdown Timer - Start Timer as a One-shot timer that will expire in 2.20 seconds

This is a Boolean variable

• NumberOfUnitsEqualTo10

Also to help with leaks, don't use "Player 22" instead use a variable that represents that particular player. If you want further help then PM me and I'll be glad to help [/TRIGGER]

 

[nedio95]

Maybe a silly question but have you got a trigger that defines the variable

• iUnitLoop

You seem to use it quite a lot but I haven't seen a definition for the variable, perhaps post all of your triggers if there are more, in the meantime, I'll keep grazing over what you've posted.

It is a loop integer.

• For each (Integer iUnitLoop) from 0 to iZoneUnits, do (Actions)

EDIT:

It's asking if units in P1Zone owned by player 22 are less than 10 then proceeds to create a unit for player 22 in another region, so the number of units in P1Zone never changes and it will continue to create units every 2.2 seconds until there are 10 or more units in P1Zone, maybe change this from a periodic event to a timer that will only start if there are less than 10 units in that zone, example

• Untitled Trigger 001
  • Events
    • Time - Timer expires
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • NumberOfUnitsEqualTo10 Equal to False
      • Then - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Number of units in (Units in P1Zone owned by Player 22 (Snow))) Less than 10
          • Then - Actions
            • Unit - Create 1 Skeleton for Player 22 (Snow) at (Center of Player1Spawn) facing Default building facing degrees
            • Countdown Timer - Start Timer as a One-shot timer that will expire in 2.20 seconds
          • Else - Actions
            • Set NumberOfUnitsEqualTo10 = True
      • Else - Actions
        • Countdown Timer - Start Timer as a One-shot timer that will expire in 2.20 seconds

This is a Boolean variable

• NumberOfUnitsEqualTo10

• Set NumberOfUnitsEqualTo10 = True

You forgot to start a timer here, which would terminate the whole spawn.

Also to help with leaks, don't use "Player 22" instead use a variable that represents that particular player. If you want further help then PM me and I'll be glad to help [/TRIGGER]

No.

That covered. To @Swatosj 's problem.

.
I don't see a reason to store and index the units in a variable array.
Use a unit group and loop through it's units.

You are leaking a lot of points, units and unit groups. Fix ALL of those:

• You could do with a single tempPoint instead of an array UnitLocations[]
• You have to destroy unit groups otherwise they would leak
• Use only a single dummy unit that is never destroyed. to cast all of your dummy spells. Each unit leaks 0.4kb of memory. (Heroes 0.44)
• You are leaking a point and a unit group in your spawning trigger, fix it.
• Read Things That Leak very useful post, a bit dated, but everything inside is actual.

regards
-Ned

 

[HelloFlighty]

It is a loop integer.

• For each (Integer iUnitLoop) from 0 to iZoneUnits, do (Actions)

• Set NumberOfUnitsEqualTo10 = True

You forgot to start a timer here, which would terminate the whole spawn.
No.

That covered. To @Swatosj 's problem.

.
I don't see a reason to store and index the units in a variable array.
Use a unit group and loop through it's units.

You are leaking a lot of points, units and unit groups. Fix ALL of those:

• You could do with a single tempPoint instead of an array UnitLocations[]
• You have to destroy unit groups otherwise they would leak
• Use only a single dummy unit that is never destroyed. to cast all of your dummy spells. Each unit leaks 0.4kb of memory. (Heroes 0.44)
• You are leaking a point and a unit group in your spawning trigger, fix it.
• Read Things That Leak very useful post, a bit dated, but everything inside is actual.

regards
-Ned

Sorry for the bad replies, I'm not that great with leaks, I was only trying to offer him some help since no one else had replied yet, thank you for the corrections though!!

 

[Swatosj]

@orcling3 I'm not sure the spawning system is causing the major freezes but your trigger is interesting, I never thought about timers for spawnings

@nedio95 Thanks for the complete answer, I've been trying so many things yesterday that my brain went crazy, many things were forgotten for the leaks.
I've been trying to iterate over a unit group and to use a single location global variable for the dummy position.. but it didn't solve the problem so I tried other things ^^

Anyway, the single dummy system is very interesting but what if there are many units walking on the special tiles at the same moment?
My dummy unit will learn/forget spells dynamically because I'm using many.

I'm fixing the leaks you mentioned above and I'll come back to tell you

 

[Swatosj]

I've completely rewritten my scripts, fixed the leaks and it's now working. However, I am now creating 2 different unit groups for my 2 periodic timers which doesn't suit me in term of solution.
I mean I could have only one group, init & hide the units inside, spawn them, iterate during periodic timers over it and then destroy it when it's no longer necessary.
What do you think of this possible solution and do you think the actual system can handle it with 20 players playing ?

Updated code & question

Here, I'm using static locations defined during a "Map Init" trigger. They represent my spawn points. This locations will never change so do I need to handle a memory leak here or is it just fine ?

• Start spawn
  • Events
    • Time - Every 2.00 seconds of game time
  • Conditions
  • Actions
    • For each (Integer iSpawnLoop) from 1 to 20, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (((Player(iSpawnLoop)) controller) Equal to User) and (((Player(iSpawnLoop)) slot status) Equal to Is playing)
          • Then - Actions
            • Set SpawnedUnits[iSpawnLoop] = (Units in PLAYER_BUILD_ZONE[iSpawnLoop] matching (((Owner of (Matching unit)) Equal to Player 21 (Coal)) or ((Owner of (Matching unit)) Equal to Player 22 (Snow))))
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Number of units in SpawnedUnits[iSpawnLoop]) Less than 10
              • Then - Actions
                • -------- PLAYER_SPAWN_POINT represents the center of every static spawning point --------
                • If (iSpawnLoop Greater than 10) then do (Unit - Create 1 Skeleton for Player 21 (Coal) at PLAYER_SPAWN_POINT[iSpawnLoop] facing Default building facing degrees) else do (Unit - Create 1 Skeleton for Player 22 (Snow) at PLAYER_SPAWN_POINT[iSpawnLoop] facing Default building facing degrees)
              • Else - Actions
            • Custom script: call DestroyGroup(udg_SpawnedUnits[udg_iSpawnLoop])
          • Else - Actions

• Creeps terrain effects
  • Events
    • Time - Every 0.23 seconds of game time
  • Conditions
  • Actions
    • For each (Integer B) from 1 to 20, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (((Player((Integer B))) controller) Equal to User) and (((Player((Integer B))) slot status) Equal to Is playing)
          • Then - Actions
            • -------- We are storing all the creeps units into a unit group --------
            • Set CurrentZoneUnits[(Integer B)] = (Units in PLAYER_BUILD_ZONE[(Integer B)] matching (((Owner of (Matching unit)) Equal to Player 21 (Coal)) or ((Owner of (Matching unit)) Equal to Player 22 (Snow))))
            • -------- We check the terrain under the unit --------
            • Unit Group - Pick every unit in CurrentZoneUnits[(Integer B)] and do (Actions)
              • Loop - Actions
                • Set tempTerrainLocation = (Position of (Picked unit))
                • -------- Icecrown - Dark Ice --------
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Terrain type at tempTerrainLocation) Equal to Icecrown Glacier - Dark Ice
                  • Then - Actions
                    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • ((Picked unit) has buff Slowed) Equal to False
                      • Then - Actions
                        • Unit - Create 1 dummy for (Player((Integer B))) at tempTerrainLocation facing Default building facing degrees
                        • Unit - Add Slow Ice to (Last created unit)
                        • Unit - Order (Last created unit) to Orc Shaman - Bloodlust (Picked unit)
                        • Unit - Add a 2.00 second Generic expiration timer to (Last created unit)
                      • Else - Actions
                  • Else - Actions
                    • -------- Dungeon - Lava Cracks --------
                    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • (Terrain type at tempTerrainLocation) Equal to Dungeon - Lava Cracks
                      • Then - Actions
                        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                          • If - Conditions
                          • Then - Actions
                            • Unit - Create 1 dummy for (Player((Integer B))) at tempTerrainLocation facing Default building facing degrees
                            • Unit - Add Lava Speed to (Last created unit)
                            • Unit - Order (Last created unit) to Orc Shaman - Bloodlust (Picked unit)
                            • Unit - Add a 2.00 second Generic expiration timer to (Last created unit)
                          • Else - Actions
                      • Else - Actions
                        • -------- Normal Ice --------
                        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                          • If - Conditions
                            • ((Picked unit) has buff Slowed) Equal to True
                          • Then - Actions
                            • Unit - Remove Slowed buff from (Picked unit)
                          • Else - Actions
                • Custom script: call RemoveLocation(udg_tempTerrainLocation)
            • Custom script: call DestroyGroup(udg_CurrentZoneUnits[GetForLoopIndexB()])
          • Else - Actions

 

[Light]

1. If locations are only set once and are never changed, you don't need to remove the leak.
2. You are creating a dummy unit for every creep, which is slow and leaks a tiny tiny leak every time, especially as it fires so often. I would recycle 1 dummy per player for this one.

 

[nedio95]

@Swatosj See what Light said ^^^

Also, you are still leaking some unit groups all around.
The idea with the dummy unit, is to have one in the corner of the map from the beginning of the game with all spells that it could possibly need and use only a single dummy. Without any dynamic learning involved.
Because you are using Bloodlust as base spell twice, you will have to have two different dummy units.
Which is still better than infinite dummy units.

regards
-Ned

 

[Wark]

I looked at this, and immediately thought: simpler is better.

Why not just like, make an invisible dummy units with auras for environmental effects and place them where needed?
I've done this myself, and noticed an aura area of 250 roughly covers 1 large square in the editor's grid.
In fact, I do this myself for a slowed movement speed effect when units walk through shallow water (using a negative value Endurance Aura)

Lava? Same effect can be done with a dummy unit and permanent immolation.
Hell, you could even make the unit model lava cracks or a fire doodad.

 

[nedio95]

I looked at this, and immediately thought: simpler is better.

Why not just like, make an invisible dummy units with auras for environmental effects and place them where needed?
I've done this myself, and noticed an aura area of 250 roughly covers 1 large square in the editor's grid.
In fact, I do this myself for a slowed movement speed effect when units walk through shallow water (using a negative value Endurance Aura)

Lava? Same effect can be done with a dummy unit and permanent immolation.
Hell, you could even make the unit model lava cracks or a fire doodad.

Could do. Would not stack with regular auras if you have any.
I like the lava crack idea

regards
-Ned

 

[Swatosj]

@nedio95 Could you point out what I missed ?
Also, thanks of what you said earlier, I've made one dummy for each spell in each zone, it doesn't represent that much dummy units overall.

Why not just like, make an invisible dummy units with auras for environmental effects and place them where needed?
Lava? Same effect can be done with a dummy unit and permanent immolation.
Hell, you could even make the unit model lava cracks or a fire doodad.

That is a good idea indeed, I am generating random terrain at the moment by changing it dynamically.
If I can handle well the auras & the models your idea is perfect for my map.

 

[nedio95]

@nedio95 Could you point out what I missed ?
Also, thanks of what you said earlier, I've made one dummy for each spell in each zone, it doesn't represent that much dummy units overall

I can't see any now :| No idea what I was looking at yesterday...

As you have quite a lot of iterations, you may also consider reducing the function calls to further reduce execution time.
Each () is a function call. For example:

• Unit - Create 1 dummy for (Player((Integer B))) at tempTerrainLocation facing Default building facing degree
• Unit - Add Lava Speed to (Last created unit)
• Unit - Order (Last created unit) to Orc Shaman - Bloodlust (Picked unit)
• Unit - Add a 2.00 second Generic expiration timer to (Last created unit)

(Last created unit) is a function call. What this does is, it asks the game who the last created unit is and the game gives it to you.
You could store the result of this in a variable and use that instead.

• Unit - Create 1 dummy for (Player((Integer B))) at tempTerrainLocation facing Default building facing degree
• Set tempUnit = (Last Created unit)
• Unit - Add Lava Speed to tempUnit)
• Unit - Order tempUnit to Orc Shaman - Bloodlust (Picked unit)
• Unit - Add a 2.00 second Generic expiration timer to tempUnit

Imagine you are looking for Karen in an office building.
You go to reception and ask "Where is Karen?"
The receptionist has to look for Karen for you.
Receptionist: "Give me a moment, I'll ask the system and will give you a room and desk number"
The receptionist takes half an hour to do that.

Next day, you have to look for Karen again, but you don't remember room or desk number.
So, you go to the receptionist again.
You: "Where's Karen?"
Receptionist: "Give me half an hour."

Pretty annoying to do that every day, eh ?
So, you write Karen's room & desk number down on day 1 onto a variable for later use.

regards
-Ned

 

[Swatosj]

@nedio95 Ahah is this example from you ? Yes I'm a lazy developer aswell so I tend not to optimize my scripts by 100%.

Is there a way to stick a thread as solved ?

 

[nedio95]

@nedio95 Ahah is this example from you ? Yes I'm a lazy developer aswell so I tend not to optimize my scripts by 100%.

Is there a way to stick a thread as solved ?

Edit the first post, change the tag ?

regards
-Ned