Do I have memory leaks? (Custom chain lightning)

[Robsta]

So I was making a chain lightning spell that improves its damage based on the caster's int score, and it works, however repeated usage of the spell causes the game to eventually crash. I suspect memory leaks are the culprit!

Testing leads me to suspect these lines cause the leak

Triggers

• Chain Lightning
  • Events
  • Conditions
  • Actions
    • Lightning - Create a Chain Lightning - Primary Lightning effect from source (Position of casting unit) to target (Position of ChainLightningTarget[0])
    • Set ChainLightningEffect[0] = (Last created lightning effect)
    • for each (Integer A) from 1 to 4 do (Actions)
      • Loop - Actions
        • If (All conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
          • Then - Actions
            • Lightning - create a Chain Lightning - Primary lightning effect from source (Position of ChainLightningTarget[((Integer A) - 1)]) to target (Position of ChainLightningTarget[(Integer A)])
            • Set ChainLightningEffect[(Integer A)] = (Last created lightning effect)
    • For each (Integer A) from 0 to 4, do (Actions)
      • Loop - Actions
        • Lightning - Destroy ChainLightningEffect[(Integer A)]

Here is my full trigger

Triggers

• Chain Lightning
  • Events
  • Conditions
    • (Ability being cast) equal to Chain Lightning (Warmage)
  • Actions
    • for each (Integer A) from 0 to 4 do (Actions)
      • Loop - Actions
        • Set ChainLightningTarget[(Integer A)] = No unit
    • Set ChainLightningTarget[0] = (Target unit of ability being cast)
    • Set ChainLightningDamage = Damage // took out the calculation here cause it's long and irrelivant
    • Unit - Cause (Casting unit) to damage ChainLightningTarget[0] dealing ChainLightningDamage damage of attack type Spells and damage type Lightning
    • Lightning - Create a Chain Lightning - Primary Lightning effect from source (Position of casting unit) to target (Position of ChainLightningTarget[0])
    • Set ChainLightningEffect[0] = (Last created lightning effect)
    • wait 0.06 seconds
    • for each (Integer A) from 1 to 4 do (Actions)
      • Loop - Actions
        • Set ChainLightningTarget[(Integer A)] = next target //Another long and irrelivant calculation - just gets another viable target within range of the last target
        • If (All conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ChainLightningTarget[(Integer A)] Not equal to No unit
          • Then - Actions
            • Set ChainLightningDamage = ChainLightningDamage * 0.75
            • Unit - Cause (casting unit) to damage ChainLightningTarget[(Integer A)] dealing ChainLightningDamage of attack type Spells and damage type Lightning
            • Lightning - create a Chain Lightning - Primary lightning effect from source (Position of ChainLightningTarget[((Integer A) - 1)]) to target (Position of ChainLightningTarget[(Integer A)])
            • Set ChainLightningEffect[(Integer A)] = (Last created lightning effect)
            • Wait 0.10 seconds
          • Else - Actions
    • For each (Integer A) from 0 to 4, do (Actions)
      • Loop - Actions
        • Lightning - Destroy ChainLightningEffect[(Integer A)]

So far I've built the map completely with GUI triggers so I'm hoping to keep it that way. Any thoughts?

 

[maddeem]

This leaks, waits will do that to you.

 

[Robsta]

Aren't waits required so that you see the lightning before it's destroyed?

I guess what I'm asking is more, how can I make it so it doesn't leak and do what I want it to.

 

[mckill2009]

- position of *something* if not removed always leaks.
- your lightning will leak also.
- waits are really bad for this situation as your trigger leaks, use timers instead.

 

[CoLd Bon3]

Are my eyes f*cked up or you simply don't have events in your triggers?! O_O

 

[Robsta]

To improve efficiancy I keep the trigger turned off untill someone picks the hero with this skill, when they do it turns on the trigger and adds the event for that unit to cast an ability. -> but that's not the point of the post.

I've finally figured out how to switch the trigger over to using timers, and yes I realise that the lightning leaks, but I haven't yet figured out how to STOP that leak.

I tried adding the code

• Custom script: call DestroyLightning( udg_ChainLightningTarget[bj_forLoopAIndex] )

however this causes an error when I try to compile and I have to turn off the trigger.

 

[Robsta]

Here are my new triggers (using timers forced me to use 2 triggers) that I used to try and remove memory leaks. I still don't know how to stop the lightning from leaking and that's the largest problem.

Triggers

• Chain Lightning
• Events
• Conditions
  • (Ability being cast) Equal to Chain Lightning (Warmage)
• Actions
  • For each (Integer A) from 0 to 4, do (Actions)
    • Loop - Actions
      • Set ChainLightningTarget[(Integer A)] = No unit
  • Set ChainLightningTarget[0] = (Target unit of ability being cast)
  • Set ChainLightningDamage = Calculate Damage
  • Unit - Cause (Casting unit) to damage ChainLightningTarget[0], dealing ChainLightningDamage damage of attack type Spells and damage type Lightning
  • Set TempLocation = (Position of Warmage)
  • Set TempLocation2 = (Position of ChainLightningTarget[0])
  • Lightning - Create a Chain Lightning - Primary lightning effect from source TempLocation to target TempLocation2
  • Set ChainLightningEffect[0] = (Last created lightning effect)
  • Custom script: call RemoveLocation(udg_TempLocation)
  • Custom script: call RemoveLocation(udg_TempLocation2)
  • Set ChainLightningTemp = 0
  • Trigger - Turn on Chain Lightning Finish <gen>

• Chain Lightning Finish
• Events
  • Time - Every 0.15 seconds of game time
• Conditions
• Actions
  • Set ChainLightningTemp = (ChainLightningTemp + 1)
  • Set TempLocation = (Postion of ChainLightningTarget[(ChainLightningTemp -1)])
  • Set ChainLightningTarget[ChainLightningTemp] = (Random unit from (Units within 450.00 of TempLocation matching (((((Matching unit) belongs to an enemy of (Owner of Warmage)) Equal to True) and (((Matching unit) is A structure) Equal to False)) and (((Matching unit) Not equal to ChainLightningTarget[(ChainLightningTemp -1)]) and (((Matching unit) is alive) Equal to True))))
  • Custom script: call RemoveLocation(udg_TempLocation)
  • if (All Conditions are True) then do (The Actions) else do (Else Actions)
    • If - Conditions
      • Or - Any (Conditions) are true
        • Conditions
          • ChainLightningTarget[ChainLightningTemp] Equal to No unit
          • ChainLightningTemp Greater than 4
    • Then - Actions
      • For each (Integer A) from 0 to 4, do (Actions)
        • Loop - Actions
          • Lightning - Destroy ChainLightningEffect[(Integer A)]
          • Custom script: call DestroyLightning( udg_ChainLightningTarget[bj_forLoopAIndex]) // Currently disabled because it breaks the trigger if enabled
      • Trigger - Turn off (This trigger)
    • Else - Actions
      • Set ChainLightingDamage = (ChainLightningDamage x 0.75)
      • Unit - Cause Warmage to damage ChainLightningTarget[ChainLightningTemp], dealing ChainLightningDamage damage of attack type Spells and damage type Lightning
      • Set TempLocation = (Position of ChainLightningTarget[(ChainLightningTemp -1)])
      • Set TempLocation2 = (Position of ChainLightningTarget[ChainLightningTemp])
      • Lightning - Create a Chain Lightning - Primary lightning effect from source TempLocation to target TempLocation2
      • Set ChainLightningEffect[ChainLightningTemp] = (Last created lightning effect)
      • Custom script: call RemoveLocation(udg_TempLocation)
      • Custom script: call RemoveLocation(udg_TempLocation2)

 

[Magtheridon96]

That's because udg_ChainLightningTarget is not a lightning effect, but a unit.

Also, you have group leaks.

The first trigger is fine, there are no leaks. (Although, it would be better if you were to use (Triggering unit) instead of (Casting unit))

Trig

• Set ChainLightningTarget[ChainLightningTemp] = (Random unit from (Units within 450.00 of TempLocation matching (((((Matching unit) belongs to an enemy of (Owner of Warmage)) Equal to True) and (((Matching unit) is A structure) Equal to False)) and (((Matching unit) Not equal to ChainLightningTarget[(ChainLightningTemp -1)]) and (((Matching unit) is alive) Equal to True))))

This is where you're leaking a group .. 2 groups actually.
That function (Random unit) is horrible :/

Instead, you should have some boolean variable equal to false, then pick every unit in _____ matching bla bla and add the condition: (Boolean == false), inside that "pick every unit", you would set a ChainLightningTarget to the picked unit.

This will only leak 1 group.
To destroy this leak, put this custom script above the "Pick every units" line:

Trig

• Custom script: set bj_wantDestroyGroup = true

Simple as that.

You can remove this completely: Custom script: call DestroyLightning( udg_ChainLightningTarget[bj_forLoopAIndex]) // Currently disabled because it breaks the trigger if enabled

 

[CoLd Bon3]

@Robsta - you should learn how to use the 'hidden' BB Code tag! That will show your triggers only when you click a button. Example :

TextShownNextToButton

Actual_content(triggers in this case)

 

[Robsta]

That's because udg_ChainLightningTarget is not a lightning effect, but a unit.

Wow duh, yeah thanks for pointing that out, fixed it to destroy ChainLightningEffect instead.

The first trigger is fine, there are no leaks. (Although, it would be better if you were to use (Triggering unit) instead of (Casting unit))

Good idea, but even better I now use Warmage instead of Casting unit. (Since Warmage is the only unit with this skill).

This is where you're leaking a group .. 2 groups actually.
That function (Random unit) is horrible :/

Instead, you should have some boolean variable equal to false, then pick every unit in _____ matching bla bla and add the condition: (Boolean == false), inside that "pick every unit", you would set a ChainLightningTarget to the picked unit.

This will only leak 1 group.
To destroy this leak, put this custom script above the "Pick every units" line:

• Custom script: set bj_wantDestroyGroup = true

Umm so are you saying I should:

Triggers

• Set TempGroup = (Units within 450.00 of (Position of ChianLightningTarget[(ChainLightningTemp -1)]))
• Unit Group - Pick every unit in TempGroup and do (Actions)
  • Loop - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • ((Matching unit) belongs to an enemy of (Owner of Warmage)) Equal to True
        • // Other conditions here
      • Then - Actions
      • Else - Actions
        • Unit Group - Remove (Matching unit) from TempGroup
• Custom script: set bj_wantDestroyGroup = true
• Set ChainLightningTarget[ChainLightningTemp] = (Random unit from TempGroup)
• Custom script: call DestroyForce(udg_TempGroup)

?

I'm not sure how to pick a random unit from a unit group (without giving preference to one unit just cause it got picked first/last) otherwise.

BTW thank you for the post: by switching that custom script to destroy the lightning, I can now play the map for over 30 min spamming chain lightning without it crashing (the test game ended faster then I expected, or expect to happen on a constant basis).

 

[Magtheridon96]

Actually, I meant something like this:

Trig

• Set TempLoc = (Position of ChainLightningTarget[(ChainLightningTemp - 1)])
• Set TempBoolean = False
• Custom script: set bj_wantDestroyGroup
• Unit Group - Pick every unit in (Units within 450.00 of TempLoc) and do (Actions)
  • Loop - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • ((Picked unit) belongs to an enemy of (Owner of Warmage)) Equal to True
        • // Other conditions
        • TempBoolean Equal to False
      • Then - Actions
        • Set ChainLightningTarget[(ChainLightningTemp - 1)] = (Picked unit)
        • Set TempBoolean = True
      • Else - Actions
• // Other actions here
• Custom script: call DestroyGroup(udg_TempGroup)

This will select a single random unit from the group

However, you know what would be even faster, shorter and better?
This:

Trig

• Set TempGroup = (Units within 450.00 of (Position of ChianLightningTarget[(ChainLightningTemp -1)]))
• Custom script: set udg_ChainLightningTarget[udg_ChainLightningTemp - 1] = FirstOfGroup(udg_TempGroup)
• Custom script: call DestroyGroup(udg_TempGroup)

This is really fast and short
What it does is simply get a unit from the group after setting it, but it does it without creating any extra groups like (Random unit ...) calls do

 

[Robsta]

ah I see what you're doing.
Your second trigger is short and sweet, but you forgot to add the conditions to it so it didn't work (notable the condition that chain lightning has to jump to a different target didn't work, so it was just jumping to itself every time).

The first trigger almost works, but when I was testing it I noticed the chain lightning always jumped to the first unit created. This could be a problem because heroes will be created before almost all of the units, so chain lightning would kind of magnitise towards heroes. (not an effect I want) also this creates a high chance of bouncing back and forth between 2 targets (which will happen unless the bounce to the new target finds an even older target to bounce to for the next bounce.) I also don't want this effect, I want the bounces to be random.

The trigger I proposed does what I want it to, but does it successfully get rid of the random unit leak?

Thanks

 

[Magtheridon96]

Your second trigger is short and sweet, but you forgot to add the conditions to it so it didn't work (notable the condition that chain lightning has to jump to a different target didn't work, so it was just jumping to itself every time).

So add them ;p

To get a better random effect, you can make a group array that adds unit that were already hit. Then, in the group filter, add a condition to check if the unit is in the group or not.

Note: Group arrays are always going to have null groups at first, so make sure you do something like this in the cast trigger:

Trig

• Custom script: set udg_GROUP[udg_INDEX] = CreateGroup()

When the spell finishes casting, destroy the group.
Or better yet, how about you do this:

Trig

• Custom script: if udg_GROUP[udg_INDEX] == null then
• Custom script: set udg_GROUP[udg_INDEX] = CreateGroup()
• Custom script: endif

And when the spell finishes casting, you can simply clear the group:

Trig

• Custom script: call GroupClear(udg_GROUP[udg_INDEX])

edit
By the way, you only need one of these lines:

Trig

• Lightning - Destroy ChainLightningEffect[(Integer A)]
• Custom script: call DestroyLightning( udg_ChainLightningTarget[bj_forLoopAIndex]) // Currently disabled because it breaks the trigger if enabled

Not both. Because that would destroy the lightning effect twice ;O

edit
And can you edit all your posts to put all the triggers in hidden-tags? They're extending the width of the page

 

[Robsta]

Yes but I don't see how that would solve the problem that is Warcraft creates its groups by a set method.

I would just be making the chain lightning so that it can't bounce to a target multiple times, so if you had a group of units {unit1, unit2, unit3 ... unit x} that it could possibly hit with the chain lightning. If you set the chain lightning target to the first one in the group, it will always bounce to unit1 first, then unit2 then unit3 until it runs out of bounces. This is not something I want.

The only way I can think of getting a random unit without the "get random unit from unit group" trigger is to count the number of units in the unit group then get a random integer between 1 and the number of units -> then set that unit from the unit group to the target.

That being said, I haven't had problems with my trigger yet in testing, although I recently found out why my test games have been shorter then expected so I plan to run some tests with longer test sessions soon.

In response to destroying the lightning effect, I only use the custom script now because it appears to do a better job at cleaning up the lightning (As I discovered by trying tests with one, the other and both).

I've edited my posts to use hidden tags.