[Spell] Will this leak and how to make it not do so?

[Krakenn99]

So the problem is that I know how to clear leaks but idk where to put the actions.

I believe this is the way to clear it:

• Custom script: call RemoveRect (udg_SpellRampageMap)
• Custom script: call DestroyGroup (udg_SpellRampageCaster)
• Custom script: call RemoveLocation (udg_SpellRampageCasterPoint)

I have no idea where to put it in. Everytime I place it in the code it stops working?

The Spell

• Rampage
  • Events
    • Time - Every 0.50 seconds of game time
  • Conditions
  • Actions
    • Set SpellRampageMap = (Playable map area)
    • Set SpellRampageCaster = (Units in SpellRampageMap matching (((Unit-type of (Matching unit)) Equal to Raging Warrior) and (((Owner of (Matching unit)) controller) Equal to User)))
    • Set SpellRampagePercentage[1] = 15
    • Set SpellRampagePercentage[2] = 30
    • Set SpellRampagePercentage[3] = 60
    • Set SpellRampagePercentage[4] = 61
    • Unit Group - Pick every unit in SpellRampageCaster and do (Actions)
      • Loop - Actions
        • Set SpellRampageCasterPoint = (Position of (Picked unit))
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Integer((Percentage life of (Picked unit)))) Less than or equal to SpellRampagePercentage[1]
          • Then - Actions
            • Animation - Change (Picked unit)'s vertex coloring to (100.00%, 10.00%, 10.00%) with 0.00% transparency
            • Unit - Create 1 Dummy for (Owner of (Picked unit)) at SpellRampageCasterPoint facing Default building facing degrees
            • Set SpellRampageDummy[1] = (Last created unit)
            • Unit - Add a 0.49 second Generic expiration timer to SpellRampageDummy[1]
            • Unit - Add Raging Warrior Rampage Dummy LVL3 to SpellRampageDummy[1]
            • Unit - Order SpellRampageDummy[1] to Orc Shaman - Bloodlust (Picked unit)
            • Skip remaining actions
          • Else - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Integer((Percentage life of (Picked unit)))) Less than or equal to SpellRampagePercentage[2]
              • Then - Actions
                • Animation - Change (Picked unit)'s vertex coloring to (100.00%, 30.00%, 30.00%) with 0.00% transparency
                • Unit - Create 1 Dummy for (Owner of (Picked unit)) at SpellRampageCasterPoint facing Default building facing degrees
                • Set SpellRampageDummy[2] = (Last created unit)
                • Unit - Add a 0.49 second Generic expiration timer to SpellRampageDummy[2]
                • Unit - Add Raging Warrior Rampage Dummy LVL2 to SpellRampageDummy[2]
                • Unit - Order SpellRampageDummy[2] to Orc Shaman - Bloodlust (Picked unit)
                • Skip remaining actions
              • Else - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Integer((Percentage life of (Picked unit)))) Less than or equal to SpellRampagePercentage[3]
                  • Then - Actions
                    • Animation - Change (Picked unit)'s vertex coloring to (100.00%, 70.00%, 70.00%) with 0.00% transparency
                    • Unit - Create 1 Dummy for (Owner of (Picked unit)) at SpellRampageCasterPoint facing Default building facing degrees
                    • Set SpellRampageDummy[3] = (Last created unit)
                    • Unit - Add a 0.49 second Generic expiration timer to SpellRampageDummy[3]
                    • Unit - Add Raging Warrior Rampage Dummy LVL1 to SpellRampageDummy[3]
                    • Unit - Order SpellRampageDummy[3] to Orc Shaman - Bloodlust (Picked unit)
                    • Skip remaining actions
                  • Else - Actions
                    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                      • If - Conditions
                        • (Integer((Percentage life of (Picked unit)))) Greater than or equal to SpellRampagePercentage[4]
                      • Then - Actions
                        • Animation - Change (Picked unit)'s vertex coloring to (100.00%, 100.00%, 100.00%) with 0.00% transparency
                      • Else - Actions

 

[Wietlol]

To understand where to place the cleanups, you have to understand the indenting of the "code" (yes, GUI triggers are also code... dont argue with me... just dont).

This image simply shows how the blocks work.

(Hidden parts are just hidden to make the image smaller.)
The black line is the block of the Trigger Actions which starts almost at the top of the trigger.
All actions until the loop actions of the unit group belong to that block of code.

The loop actions of the unit group creates a new block of code that runs (technically) independent from the first (black) block.
That block is green in the image.
The green block ends as soon as the loop of the unit group ends.

SpellRampageCasterPoint creates a new location object at the beginning of the green block.
Therefor, it should be cleaned at the end of that block or the next time it is used (but you shouldnt need the latter for now).
So after you are done with the location object, you remove it.
At the end of the green block, you have a very large chance that that block will run again, so you have to clean it inside that block.

Blocks can be loops, if statements (including their else blocks), but also functions, classes, etc.
For readability, you might even use the same styling to fill arrays or objects.

The rect and unit group objects are created in the black block and should therefor be cleaned in that one as soon as you dont need them any more.

In coding, (that means including GUI,) it is important to easily see where blocks of code start and where they end. That is why we indent the code in sets of spaces of 3, 4 or 5 (anything else is pretty much bullshit) so you can easily see how the code will behave, where loops start and end, what code may not run under certain circumstances, etc.
Ofcourse, (except in some languages,) this is merely an eye candy and can be ignored, but it does help you to read the code.
In some cases, it might be better to not follow the indenting rules to make it more readable.
Other rules like these are newlines, spaces between key characters and the order of consecutive if statements.

Two last thinga that you should know:
1, "Skip remaining actions" literally skips the remaining actions... in your case of the current iteration through the loop.
This means that you must clean the location leak just before each "Skip remaining actions" as well as at the end of the loop.
An easier way would be to remove the location just before you set it to a new value and remove the location once more at the end of the trigger (black block), then you dont have to remove it in the end of the unit group loop (green block) or at each "Skip remaining actions".
This is not a really preferred method but can in this case be easier.
2, "(Playable map area)" does not create a new rect, so it doesnt leak if you dont remove it. Better yet, (which is why yours doesnt work when you add it,) if you remove/destroy that rect, the next time you use "(Playable map area)" it will return that destroyed object... which doesnt really work well.

 

[Krakenn99]

To understand where to place the cleanups, you have to understand the indenting of the "code" (yes, GUI triggers are also code... dont argue with me... just dont).

This image simply shows how the blocks work.
...

So lot's of stuff I already knew but this cleared it up even more, so thanks a lot. Whan I got in the end is that I should set all the variables at the start of every IF action and clear the leaks at the end of every IF action before the ''skip remaining'' action. Is that it or am I wrong again?

 

[Wietlol]

The "Skip Remaining Actions" is supposed to just stop the trigger at all.
However, because of how the group loop is programmed, it will only stop that single iteration.
If/Then/Else statements do not make much of a difference except that conditions are also on a separate function, but you cant skip the remaining actions in a condition anyway.

"SpellRampageCasterPoint" must be removed each time at the end of the loop.
But if you skip the remaining actions, it will never do that, so in addition to one remove at the end, you also need one for each skip remaining actions.

 

[Krakenn99]

The "Skip Remaining Actions" is supposed to just stop the trigger at all.
However, because of how the group loop is programmed, it will only stop that single iteration.
If/Then/Else statements do not make much of a difference except that conditions are also on a separate function, but you cant skip the remaining actions in a condition anyway.

"SpellRampageCasterPoint" must be removed each time at the end of the loop.
But if you skip the remaining actions, it will never do that, so in addition to one remove at the end, you also need one for each skip remaining actions.

Thanks a lot sir.