[Trigger] Destroying Group to clean leaks breaks my spell? Help!

[Caedrus]

So... this custom triggered spell works (though is obviously not MUI/optimized etc yet, and I need to get rid of that wait in the middle somehow), except when I added in the "Call Destroy Group" script at the end (Specifically, Custom script: call DestroyGroup(udg_ImpaleTargetGroup[0])). If I disable that line, it goes back to working fine, but when enabled, the first usage of the spell works, then all future uses cause it to instantly kill an enemy. I can't quite put my finger on what's wrong...

Can anyone help? +Rep for helpers, of course.

PS: Would also like advice on what should be used instead of waits in situations like this.

• Impale
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Impale
  • Actions
    • Set ImpaleCaster[0] = (Casting unit)
    • Set ImpaleOwner[0] = (Owner of ImpaleCaster[0])
    • Set ImpalePoint1[0] = (Position of (Casting unit))
    • Set ImpalePoint2[0] = (Target point of ability being cast)
    • Set ImpaleAngle[0] = (Angle from ImpalePoint1[0] to ImpalePoint2[0])
    • Animation - Change ImpaleCaster[0]'s animation speed to 0.00% of its original speed
    • For each (Integer IMP[0]) from 1 to 32, do (Actions)
      • Loop - Actions
        • Set ImpalePoint3[0] = (ImpalePoint1[0] offset by (10.00 x (Real(IMP[0]))) towards ImpaleAngle[0] degrees)
        • Set ImpaleTempGroup[0] = (Units within 100.00 of ImpalePoint3[0])
        • Unit Group - Pick every unit in ImpaleTempGroup[0] 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 ImpaleOwner[0]) Equal to True
                • ((Picked unit) is Magic Immune) Equal to False
                • ((Picked unit) is alive) Equal to True
                • ((Picked unit) is in ImpaleTargetGroup[0]) Equal to False
              • Then - Actions
                • Unit Group - Add (Picked unit) to ImpaleTargetGroup[0]
                • Unit - Pause (Picked unit)
                • Animation - Change (Picked unit)'s animation speed to 0.00% of its original speed
                • Special Effect - Create a special effect attached to the origin of (Picked unit) using Objects\Spawnmodels\Human\HumanLargeDeathExplode\HumanLargeDeathExplode.mdl
                • Unit - Cause ImpaleCaster[0] to damage (Picked unit), dealing 200.00 damage of attack type Spells and damage type Normal
              • Else - Actions
        • Custom script: call RemoveLocation(udg_ImpalePoint3[0])
        • Custom script: call DestroyGroup(udg_ImpaleTempGroup[0])
    • Unit - Create 1 Dummy(ImpaleSpawn) for ImpaleOwner[0] at ImpalePoint1[0] facing ImpaleAngle[0] degrees
    • Animation - Change (Last created unit)'s animation speed to 200.00% of its original speed
    • Unit - Add a 1.50 second Generic expiration timer to (Last created unit)
    • Set ImpalePoint3[0] = (ImpalePoint1[0] offset by 150.00 towards ImpaleAngle[0] degrees)
    • Unit - Create 1 Dummy(ImpaleRed) for ImpaleOwner[0] at ImpalePoint3[0] facing (ImpaleAngle[0] - 180.00) degrees
    • Unit - Add a 1.50 second Generic expiration timer to (Last created unit)
    • Wait 1.50 seconds
    • Animation - Change ImpaleCaster[0]'s animation speed to 100.00% of its original speed
    • Unit Group - Pick every unit in ImpaleTargetGroup[0] and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
          • Then - Actions
            • Unit Group - Remove (Picked unit) from ImpaleTargetGroup[0]
            • Unit - Unpause (Picked unit)
            • Animation - Change (Picked unit)'s animation speed to 100.00% of its original speed
            • Special Effect - Create a special effect attached to the origin of (Picked unit) using Objects\Spawnmodels\Human\HumanLargeDeathExplode\HumanLargeDeathExplode.mdl
            • Unit - Cause ImpaleCaster[0] to damage (Picked unit), dealing 200.00 damage of attack type Spells and damage type Normal
          • Else - Actions
    • Custom script: call DestroyGroup(udg_ImpaleTargetGroup[0])
    • Custom script: call RemoveLocation(udg_ImpalePoint1[0])
    • Custom script: call RemoveLocation(udg_ImpalePoint2[0])
    • Custom script: call RemoveLocation(udg_ImpalePoint3[0])

 

[D4RK_G4ND4LF]

if you destroy the group the group is destroyed
-> no units to pick cause there is no group

you should use indexing or hashtables
https://www.hiveworkshop.com/forums/trigger-gui-editor-tutorials-279/hashtables-mui-133407/

indexing is faster but harder to understand and not that flexible

you could also create a local group and destroy it at the end of the spell but that would require JASS/more custom script

edit: example for local group

Trigger

• Events
  • Unit - A unit Is issued an order targeting a point
• Conditions
• Actions
  • -------- Must be the first line of the trigger --------
  • -------- Create local group --------
  • Custom script: local group g = CreateGroup()
  • -------- Set target point --------
  • Set loc = (Target point of issued order)
  • -------- Set global group (this also automatically creates the group) --------
  • Set g = (Units within 300.00 of loc)
  • -------- Remove point leak --------
  • Custom script: call RemoveLocation(udg_loc)
  • -------- Add units of global group to local group --------
  • Custom script: call GroupAddGroup(udg_g, g)
  • -------- Destroy the global group --------
  • Custom script: call DestroyGroup(udg_g)
  • -------- Wait --------
  • Wait 1.00 game-time seconds
  • -------- Create global group --------
  • Custom script: set udg_g = CreateGroup()
  • -------- Add units of local group to global group --------
  • Custom script: call GroupAddGroup(g, udg_g)
  • -------- Kill units in global group --------
  • Unit Group - Pick every unit in g and do (Actions)
    • Loop - Actions
      • Unit - Kill (Picked unit)
  • -------- Destroy the local group --------
  • Custom script: call DestroyGroup(g)
  • -------- Destroy the global group --------
  • Custom script: call DestroyGroup(udg_g)
  • -------- Set local group = null (local variables except reals, integers and strings should be nulled to reduce leak) --------
  • Custom script: set g = null

as you can see it is quite complicated so you should switch to JASS instead of using GUI since many things can be done easier with it
vJASS is even easier but requires JNGP (JassNewGenPack, a special editor, can be found at wc3c.net and in the tools section)

 

[Caedrus]

if you destroy the group the group is destroyed
-> no units to pick cause there is no group

How come it works for *other* groups then? It has never created a problem for me in the past to destroy a group to remove leaks (and then reuse the trigger). Only this spell, and only the TargetGroup removal (not the TempGroup removal) causes problems.

as you can see it is quite complicated so you should switch to JASS instead of using GUI since many things can be done easier with it
vJASS is even easier but requires JNGP (JassNewGenPack, a special editor, can be found at wc3c.net and in the tools section)

JNGP does not appear to work for me for some reason, and no one seems terribly willing to help on that front, so I'm stuck without vJASS

you should use indexing

That's what I usually do. I just haven't taken the extra time to make the spell MUI yet.

 

[D4RK_G4ND4LF]

How come it works for *other* groups then? It has never created a problem for me in the past to destroy a group to remove leaks (and then reuse the trigger). Only this spell, and only the TargetGroup removal (not the TempGroup removal) causes problems..

unitgroups creates with the variable editor are created but if you destroy them they will stay destroyed and not be created again
the GUI "set group = units in range or whatever" function also creates a group (as you could read in the comments to the trigger)

set yourGroupName = CreateGroup()

creates a group so you can use it again

JNGP does not appear to work for me for some reason, and no one seems terribly willing to help on that front, so I'm stuck without vJASS .

JASS is still good enough just takes a little more time (less than GUI though)

That's what I usually do. I just haven't taken the extra time to make the spell MUI yet.

PS: Would also like advice on what should be used instead of waits in situations like this.

use indexing

and if I'm at it I'll explain how unitgroups in GUI work
this way you can answer the questions yourself

this his how "Pick all units in range of loc matching..." looks like in JASS:

function GetUnitsInRangeOfLocMatching takes real radius, location whichLocation, boolexpr filter returns group
    local group g = CreateGroup()//Create a group
    call GroupEnumUnitsInRangeOfLoc(g, whichLocation, radius, filter)//Add units within range to group
    call DestroyBoolExpr(filter)
    return g//return group
endfunction

but if you destroy a group it will leak cause blizzard sux
it is much better to add units to an already existing group
of cause you have to clear the group of all units first
unfortunately you can't add units to a unitgroup in GUI without creating a new group which would leak so you will have to use JASS
it is also MUCH! faster to add units to a group instead of creating a new group every time

 

[YourNameHere]

Write

    set udg_ImpaleTargetGroup[0] = CreateGroup()

At the beginning of the trigger.