[Trigger] Cleanup

[Pharos]

SOLVED: Cleanup

I'm currently in the progress of removing memory leaks from a map. I would like to know if this trigger is free of memory leaks?

Trigger Warning

• Spawns Human
  • Events
    • Time - Every 55.00 seconds of game time
  • Conditions
    • (Player 12 (Brown) Food used) Less than or equal to 70
  • Actions
    • Set DefendersSpawnPoint = (Center of Spawn First <gen>)
    • Set DefendersGroup = (Units owned by Player 12 (Brown))
    • Set DefendersAttackPoint = (Center of Attack First Elfs <gen>)
    • Unit Group - Pick every unit in DefendersGroup and do (Unit - Order (Picked unit) to Attack-Move To DefendersAttackPoint)
    • Custom script: call DestroyGroup(udg_DefendersGroup)
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (BrollBearmantle is dead) Equal to True
      • Then - Actions
        • Hero - Instantly revive BrollBearmantle at DefendersSpawnPoint, Hide revival graphics
        • Unit - Order BrollBearmantle to Attack-Move To DefendersAttackPoint
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (ValeeraSanguinar is dead) Equal to True
      • Then - Actions
        • Hero - Instantly revive ValeeraSanguinar at DefendersSpawnPoint, Hide revival graphics
        • Unit - Order ValeeraSanguinar to Attack-Move To DefendersAttackPoint
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
        • (OnlyOnePaladin is dead) Equal to True
      • Then - Actions
        • Unit - Create 1 Paladin for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
        • Set OnlyOnePaladin = (Last created unit)
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 3 Footman for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 3 Rifleman for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Unit - Create 1 Dalaran Wizard for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Unit - Create 1 Dragonhawk Rider for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Unit - Create 1 Gryphon Rider for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 2 Knight for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 2 Cleric for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 2 Sorceress for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Player 12 (Brown) Food used) Less than or equal to 70
      • Then - Actions
        • Custom script: set bj_wantDestroyGroup = true
        • Unit - Create 2 Spell Breaker for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
      • Else - Actions
    • Custom script: call RemoveLocation(udg_DefendersSpawnPoint)
    • Set DefendersGroup = (Units owned by Player 12 (Brown))
    • Unit Group - Pick every unit in DefendersGroup and do (Unit - Order (Picked unit) to Attack-Move To DefendersAttackPoint)
    • Custom script: call DestroyGroup(udg_DefendersGroup)
    • Unit - Order BrollBearmantle to Attack-Move To DefendersAttackPoint
    • Unit - Order ValeeraSanguinar to Attack-Move To DefendersAttackPoint
    • Custom script: call RemoveLocation(udg_DefendersAttackPoint)

Unfortunately, I hadn't created the triggers, so I'm in the midst of cleaning many triggers similar to this. Suggestions?

 

[Nichilus]

seems to be leak-free.
Although the last two orders serve no purpose there (the one which orders Broll and Valeera to attack DefendersAttackPoint), because they're ordered to do that when they are revived.

 

[deathismyfriend]

Since you always use these.

• Set DefendersSpawnPoint = (Center of Spawn First <gen>)
• Set DefendersAttackPoint = (Center of Attack First Elfs <gen>)

Never destroy these and create them / set them only once in a map init trigger.

These should all be removed since you destroy the group already.

• Custom script: set bj_wantDestroyGroup = true

 

[Pharos]

The reason I haven't done that was because the points need to change as the map progresses. Though, that certainly does apply to other triggers. Thank you.

Edit: The custom script was put in because multiple units were created, so I assumed not using the script would cause leaks. I'll remove the scripts.

 

[Nichilus]

Actually those custom scripts seem to be used correctly there.
When you create more than 1 unit (e.g. Create 2/more Footman for player X at...), you can order all those created units to do something by giving an order to last created unit group.
If I do this:

• Unit - Create 3 Footman for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
• Unit - Create 3 Rifleman for Player 12 (Brown) at DefendersSpawnPoint facing Default building facing degrees
• Unit Group - Order (Last Created unit group) to *do something*

It will order those 3 Rifleman to *do something*.
Seeing that, it is safe to assume that a unit group is actually created, hence the custom script

• Custom script: set bj_wantDestroyGroup = true

should correctly destroy it, unless the action itself takes care of any leaks.

 

[deathismyfriend]

Seeing that, it is safe to assume that a unit group is actually created, hence the custom script

• Custom script: set bj_wantDestroyGroup = true

should correctly destroy it, unless the action itself takes care of any leaks.

They are not as you should look at the whole trigger.

@TO
You should combine all ITEs with the same condition.

 

[Nichilus]

When you create a unit, you can refer to that unit as to last created unit group. It's even written in the grayed out text explaining the "Unit - Create Unit" action in GUI.
It has nothing to do with the trigger OP posted, unless the unit group created by the "Unit - Create unit" action works differently than standard unit group.
The tip reads:

Use 'Last Created Unit Group' to refer to these created units. Use 'Last Created Unit' to refer to exactly one of these units.

---------
I also believe, that the trigger posted attempts to replenish soldiers for player 12.
Something akin to:
If food used <= 70, create 3 Footman
If food used is still <= 70, create 2 Rifleman
If food used is still <= 70, create ...
and so on. This results in the food used being a bit above 70, but not by much.

If he did merge all ITEs with same condition, then it would check if food used is <= 70 only once and if yes, he would create a whole new army.

For example if he had 68 food used (and assuming Broll and Valeera are alive), the current trigger would create 1 Paladin. Let's say he uses 5 food. So after being created, Player 12 would have 73 food used, which is more than 70, so no more units would be created.
However merging those ITEs would result in creating 18 new units if he had less than 70 food (e.g. 68 food used). That may be around 40 food used if not more, so he would end up with more than 108 food used.

OP should clarify what is the purpose of the trigger. There may be ways to optimize the trigger if we understand the purpose.

 

[Pharos]

There's NPC players in the map that spawn units to help attack/defend a certain area at a given time. Player 12 is one of those. Players train their own units, but the NPC's will always spawn to attack/defend. There are similar triggers for the attacking faction.

 

[Nichilus]

then the trigger is imo good as it is.

 

[Dr Super Good]

"set bj_wantDestroyGroup = true" has no effect on "CreateNUnitsAtLoc" so you are running a lot of times unnecessarily.