Need help fixing these triggers!

[Proxym8]

I am looking for another pair of expert(er) eyes to help me identify mistakes and just plain wrong GUI coding in this trigger-aided spell. The spell Carrion Swarm releases a wave (much like the original), but now also teleports the caster to the targeted location. The caster is supposed to be immune/untargetable during this travel time, hence the "Hide unit" function.
I am sure my trigger, as it stands, is 90% there. But I am facing a weird bug where the first cast does everything as intended, except the casting unit remains stuck in place, unable to perform any actions (it is as if the game auto-cancels any action/button you press, near instantly).
I am also aware this spell leaks a ton, so I wouldn't mind help in finding those culprits as well.

Below is the initial trigger, with setups for various needed variables (MUI-compatible with the use of arrays):

• CarrionSwarm Cast
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Carrion Swarm (custom)
  • Actions
    • Set VariableSet CarrionSwarm_PlayerIndex = ((Player number of (Owner of (Casting unit))) - 2)
    • -------- CarrionSwarm_Index is calculated this way because the first two player numbers are reserved for non-player factions. Player 1 is actually Player 3 by game's definition. --------
    • Set VariableSet CarrionSwarm_Caster[CarrionSwarm_PlayerIndex] = (Casting unit)
    • Set VariableSet CarrionSwarm_CastPoint[CarrionSwarm_PlayerIndex] = (Position of CarrionSwarm_Caster[CarrionSwarm_PlayerIndex])
    • Set VariableSet CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex] = (Target point of ability being cast)
    • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_PlayerIndex] = (Integer((Distance between CarrionSwarm_CastPoint[CarrionSwarm_PlayerIndex] and CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex])))
    • Set VariableSet CarrionSwarm_Angle[CarrionSwarm_PlayerIndex] = (Integer((Angle from CarrionSwarm_CastPoint[CarrionSwarm_PlayerIndex] to CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex])))
    • Set VariableSet CarrionSwarm_InvAngle[CarrionSwarm_PlayerIndex] = (Integer((Angle from CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex] to CarrionSwarm_CastPoint[CarrionSwarm_PlayerIndex])))
    • Unit - Create 1 CarrionSwarmDummy for (Owner of CarrionSwarm_Caster[CarrionSwarm_PlayerIndex]) at CarrionSwarm_CastPoint[CarrionSwarm_PlayerIndex] facing (Real(CarrionSwarm_Angle[CarrionSwarm_PlayerIndex])) degrees
    • Set VariableSet CarrionSwarm_Dummy[CarrionSwarm_PlayerIndex] = (Last created unit)
    • Unit - Hide CarrionSwarm_Caster[CarrionSwarm_PlayerIndex]
    • Trigger - Turn on CarrionSwarm Travel <gen>

I have identified the issue to be present in the following trigger, as the spell "functions" normally until the last few pieces added (i.e. no visible bugs):

• CarrionSwarm Travel
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • For each (Integer A) from 1 to 8, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CarrionSwarm_Distance[(Integer A)] Greater than 0
          • Then - Actions
            • Set VariableSet CarrionSwarm_Distance[(Integer A)] = (CarrionSwarm_Distance[(Integer A)] - 30)
            • Unit - Move CarrionSwarm_Dummy[(Integer A)] instantly to (CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex] offset by (Real(CarrionSwarm_Distance[(Integer A)])) towards (Real(CarrionSwarm_InvAngle[(Integer A)])) degrees.)
          • Else - Actions
            • Unit - Move CarrionSwarm_Caster[(Integer A)] instantly to CarrionSwarm_TargetPoint[(Integer A)]
            • Unit - Unhide CarrionSwarm_Caster[(Integer A)]
            • Selection - Select CarrionSwarm_Caster[(Integer A)] for (Player(((Integer A) + 2)))
            • Unit - Remove CarrionSwarm_Dummy[(Integer A)] from the game

I believe it might be related to the fact that I can't quite figure out how to stop the loop in the 2nd trigger from running tirelessly in the background when no spell-casting is done. I would appreciate any kind fellows in assisting with fixing this mess. I also plan on adding a damage component to this spell, but that is second fiddle to getting the actual movement behaviour done 100%.

 

[ThompZon]

I'm one of those who prefer unit indexer and index the array using the units "custom value".
That way, you can have a unit-group for the casters and easily check if it is empty or not.
That way, you can even have multiple casters per player (if that is a thing on your map).

I'd use one of Bribe indexers, I.E. the stand alone one or the one built-into the GUI event.

https://www.hiveworkshop.com/members/bribe.180488/#resources

 

[Pyrogasm]

• I realize why you are doing -2 for the index but it is wholly unnecessary. You could simply change your loop from 1-8 to 3-10. That being said, you have basically created a MPI trigger instead of a MUI one... but with the added complication that you are trying to loop over all potential indices rather than only the ones you need to touch at any given time. I agree with ThompZon that a unit indexer would be a viable solution, but another solution similar to what you have done that I would recommend understanding is Dynamic Indexing. Understanding both methods will be valuable to you in the future.
  
  
• There is no reason to store the distance and angle as integers instead of reals. GUI makes it annoyingly difficult to use integers in computations that would involve reals because it makes you convert between them (when in reality you just shouldn't have to and the output would be a real unless every number in the computation was an integer).
  
  
• What's the purpose of the dummy unit? If it's just for SFX could you not simply accomplish that by actually basing the ability on carrion swarm and letting it play its SFX?
  
  
• Your periodic trigger never turns itself off. Because of the method you've used here it would be nontrivial for it to figure out if it should do that, but it would be possible. The other two structural solutions we have suggested would make this easy to determine (counting units in a group or checking if an integer is greater than 0).
  
  
• You never remove CastPoint or TargetPoint, and each time you move the dummy unit you create a new point with this line: (CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex] offset by (Real(CarrionSwarm_Distance[(Integer A)])) towards (Real(CarrionSwarm_InvAngle[(Integer A)])) degrees. These are all memory leaks you should clean up.

 

[Proxym8]

• I realize why you are doing -2 for the index but it is wholly unnecessary. You could simply change your loop from 1-8 to 3-10. That being said, you have basically created a MPI trigger instead of a MUI one... but with the added complication that you are trying to loop over all potential indices rather than only the ones you need to touch at any given time. I agree with ThompZon that a unit indexer would be a viable solution, but another solution similar to what you have done that I would recommend understanding is Dynamic Indexing. Understanding both methods will be valuable to you in the future.
  
  
• There is no reason to store the distance and angle as integers instead of reals. GUI makes it annoyingly difficult to use integers in computations that would involve reals because it makes you convert between them (when in reality you just shouldn't have to and the output would be a real unless every number in the computation was an integer).
  
  
• What's the purpose of the dummy unit? If it's just for SFX could you not simply accomplish that by actually basing the ability on carrion swarm and letting it play its SFX?
  
  
• Your periodic trigger never turns itself off. Because of the method you've used here it would be nontrivial for it to figure out if it should do that, but it would be possible. The other two structural solutions we have suggested would make this easy to determine (counting units in a group or checking if an integer is greater than 0).
  
  
• You never remove CastPoint or TargetPoint, and each time you move the dummy unit you create a new point with this line: (CarrionSwarm_TargetPoint[CarrionSwarm_PlayerIndex] offset by (Real(CarrionSwarm_Distance[(Integer A)])) towards (Real(CarrionSwarm_InvAngle[(Integer A)])) degrees. These are all memory leaks you should clean up.

Thanks for the advice, I will continue to look into it tomorrow, it is getting quite late for me now.
1. I will change that asap, thanks!
2. I have noticed that as well, a lot of unnecessary conversions.
3. I use a dummy unit to play the SFX because using the spell's SFX would result in the SFX 'overshooting' when choosing a target point that is shorter than the defined ability range (say cast spell range is 700, ability range is 500; the unit can choose any cast point between 1 - 700, but the SFX would always end at the 500 range).
4. That's probably the main culprit here. Will check into Dynamic Indexing to see if I can fix it that way.
5. I am working on converting this trigger to Lua, both to finally learn it and to make preventing leaks like this easier for the future.

Thanks for the tips!

 

[Proxym8]

Thanks for the advice, I will continue to look into it tomorrow, it is getting quite late for me now.
1. I will change that asap, thanks!
2. I have noticed that as well, a lot of unnecessary conversions.
3. I use a dummy unit to play the SFX because using the spell's SFX would result in the SFX 'overshooting' when choosing a target point that is shorter than the defined ability range (say cast spell range is 700, ability range is 500; the unit can choose any cast point between 1 - 700, but the SFX would always end at the 500 range).
4. That's probably the main culprit here. Will check into Dynamic Indexing to see if I can fix it that way.
5. I am working on converting this trigger to Lua, both to finally learn it and to make preventing leaks like this easier for the future.

Thanks for the tips!

• CarrionSwarm Travel
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • For each (Integer CarrionSwarm_LoopIndex) from 1 to CarrionSwarm_Index, do (Actions)
      • Loop - Actions
        • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = (CarrionSwarm_Distance[CarrionSwarm_LoopIndex] - 30.00)
        • Set VariableSet CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex] = (CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex] offset by CarrionSwarm_Distance[CarrionSwarm_LoopIndex] towards CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] degrees.)
        • Unit - Move CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex]
        • Custom script: RemoveLocation(CarrionSwarm_LoopPoint)
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CarrionSwarm_Distance[CarrionSwarm_LoopIndex] Less than or equal to 0.00
          • Then - Actions
            • Unit - Remove CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] from the game
            • Unit - Move CarrionSwarm_Caster[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex]
            • Unit - Unhide CarrionSwarm_Caster[CarrionSwarm_LoopIndex]
            • Selection - Select CarrionSwarm_Caster[CarrionSwarm_LoopIndex] for (Owner of CarrionSwarm_Caster[CarrionSwarm_LoopIndex])
            • Set VariableSet CarrionSwarm_Caster[CarrionSwarm_LoopIndex] = CarrionSwarm_Caster[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] = CarrionSwarm_Dummy[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Angle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Index = (CarrionSwarm_Index - 1)
            • Set VariableSet CarrionSwarm_LoopIndex = (CarrionSwarm_LoopIndex - 1)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • CarrionSwarm_Index Equal to 0
              • Then - Actions
                • Custom script: RemoveLocation(CarrionSwarm_CastPoint)
                • Custom script: RemoveLocation(CarrionSwarm_TargetPoint)
                • Trigger - Turn off (This trigger)
              • Else - Actions
          • Else - Actions

Alright, this is the updated, MUI-friendly version - and it works!
Now, since I use LUA as the scripting language, I am not 100% sure on how function/native names translate from JASS to LUA. I've seen people use 'RemoveLoc()' and the full-name version 'RemoveLocation()', so not sure if my stuff leaks still.

Also, if the prefix that JASS uses for variable reference is necessary in LUA version?
JASS example:

• Custom script: callRemoveLocation(udg_CarrionSwarm_CastPoint)

LUA example:

• Custom script: RemoveLocation(CarrionSwarm_CastPoint)

Anyways, hopefully this is it. I still have to add a damage component, so I will be using Unit Groups and am aware I need to clean those up as well! Thanks for the help guys!

 

[Pyrogasm]

Alright, this is the updated, MUI-friendly version - and it works!

Excellent, happy to help!

Now, since I use LUA as the scripting language, I am not 100% sure on how function/native names translate from JASS to LUA. I've seen people use 'RemoveLoc()' and the full-name version 'RemoveLocation()', so not sure if my stuff leaks still.

Generally it's identical naming conventions, you just drop the set and call keywords. There may be specific custom functions those people are referencing. I believe the udg_ prefix is still required because that's set by the variable editor when you make the variable, its just that Lua doesn't mind ignoring that the variable doesn't exist if you type it wrong and just won't error you. Not sure on that one.

 

[Proxym8]

Excellent, happy to help!

Generally it's identical naming conventions, you just drop the set and call keywords. There may be specific custom functions those people are referencing. I believe the udg_ prefix is still required because that's set by the variable editor when you make the variable, its just that Lua doesn't mind ignoring that the variable doesn't exist if you type it wrong and just won't error you. Not sure on that one.

It seems that callRemoveLocation(udg_CarrionSwarm_LoopPoint) is proper even for LUA as now the loop never ends. Not sure why, I don't see how cleaning the temporary point that CarrionSwarm_LoopPoint is referring to during the "travelling" itself damages the loop... especially since it is done after the Dummy unit is moved to it, so there should be no consequence to removing it immediately after - I think removing it at the end of the loop wouldn't remove the memory leak, right?

• CarrionSwarm Travel
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • For each (Integer CarrionSwarm_LoopIndex) from 1 to CarrionSwarm_Index, do (Actions)
      • Loop - Actions
        • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = (CarrionSwarm_Distance[CarrionSwarm_LoopIndex] - 30.00)
        • Set VariableSet CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex] = (CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex] offset by CarrionSwarm_Distance[CarrionSwarm_LoopIndex] towards CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] degrees.)
        • Unit - Move CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex]
        • Custom script: RemoveLocation(udg_CarrionSwarm_LoopPoint)
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CarrionSwarm_Distance[CarrionSwarm_LoopIndex] Less than or equal to 0.00
          • Then - Actions
            • Unit - Remove CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] from the game
            • Unit - Move CarrionSwarm_Caster[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex]
            • Unit - Unhide CarrionSwarm_Caster[CarrionSwarm_LoopIndex]
            • Selection - Select CarrionSwarm_Caster[CarrionSwarm_LoopIndex] for (Owner of CarrionSwarm_Caster[CarrionSwarm_LoopIndex])
            • Set VariableSet CarrionSwarm_Caster[CarrionSwarm_LoopIndex] = CarrionSwarm_Caster[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] = CarrionSwarm_Dummy[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Angle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Index = (CarrionSwarm_Index - 1)
            • Set VariableSet CarrionSwarm_LoopIndex = (CarrionSwarm_LoopIndex - 1)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • CarrionSwarm_Index Equal to 0
              • Then - Actions
                • Custom script: RemoveLocation(udg_CarrionSwarm_CastPoint)
                • Custom script: RemoveLocation(udg_CarrionSwarm_TargetPoint)
                • Trigger - Turn off (This trigger)
              • Else - Actions
          • Else - Actions

 

[Proxym8]

It seems that callRemoveLocation(udg_CarrionSwarm_LoopPoint) is proper even for LUA as now the loop never ends. Not sure why, I don't see how cleaning the temporary point that CarrionSwarm_LoopPoint is referring to during the "travelling" itself damages the loop... especially since it is done after the Dummy unit is moved to it, so there should be no consequence to removing it immediately after - I think removing it at the end of the loop wouldn't remove the memory leak, right?

• CarrionSwarm Travel
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • For each (Integer CarrionSwarm_LoopIndex) from 1 to CarrionSwarm_Index, do (Actions)
      • Loop - Actions
        • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = (CarrionSwarm_Distance[CarrionSwarm_LoopIndex] - 30.00)
        • Set VariableSet CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex] = (CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex] offset by CarrionSwarm_Distance[CarrionSwarm_LoopIndex] towards CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] degrees.)
        • Unit - Move CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex]
        • Custom script: RemoveLocation(udg_CarrionSwarm_LoopPoint)
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CarrionSwarm_Distance[CarrionSwarm_LoopIndex] Less than or equal to 0.00
          • Then - Actions
            • Unit - Remove CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] from the game
            • Unit - Move CarrionSwarm_Caster[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex]
            • Unit - Unhide CarrionSwarm_Caster[CarrionSwarm_LoopIndex]
            • Selection - Select CarrionSwarm_Caster[CarrionSwarm_LoopIndex] for (Owner of CarrionSwarm_Caster[CarrionSwarm_LoopIndex])
            • Set VariableSet CarrionSwarm_Caster[CarrionSwarm_LoopIndex] = CarrionSwarm_Caster[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] = CarrionSwarm_Dummy[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Angle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Index = (CarrionSwarm_Index - 1)
            • Set VariableSet CarrionSwarm_LoopIndex = (CarrionSwarm_LoopIndex - 1)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • CarrionSwarm_Index Equal to 0
              • Then - Actions
                • Custom script: RemoveLocation(udg_CarrionSwarm_CastPoint)
                • Custom script: RemoveLocation(udg_CarrionSwarm_TargetPoint)
                • Trigger - Turn off (This trigger)
              • Else - Actions
          • Else - Actions

Edit: I think I figured it out - I was trying to destroy a location assigned to a variable with an array, without also referencing the array index! Seems to proceed normally now!

• Custom script: RemoveLocation(udg_CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex])

(I've also gone and fixed the other instances of this error further down the code!)

 

[Pyrogasm]

You have an error there still: CarrionSwarm_LoopIndex missing the udg_ so I think either the thread will crash when it gets there or it will substitute 0 for the unknown variable.

 

[Proxym8]

You have an error there still: CarrionSwarm_LoopIndex missing the udg_ so I think either the thread will crash when it gets there or it will substitute 0 for the unknown variable.

Well the trigger proceeds normally, no visible bugs, though I will fix that right up. So, whenever I am referencing a variable in a custom script, I have to prefix it with udg_?

 

[Proxym8]

You have an error there still: CarrionSwarm_LoopIndex missing the udg_ so I think either the thread will crash when it gets there or it will substitute 0 for the unknown variable.

Well, now I've an issue with the damage component; it deals too much damage. Likely due to somehow not respecting Unit Group placements, and damaging "illegal targets" multiple times - though I can't figure out where I went wrong.

• CarrionSwarm Loop
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • For each (Integer CarrionSwarm_LoopIndex) from 1 to CarrionSwarm_Index, do (Actions)
      • Loop - Actions
        • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = (CarrionSwarm_Distance[CarrionSwarm_LoopIndex] - 30.00)
        • Set VariableSet CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex] = (CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex] offset by CarrionSwarm_Distance[CarrionSwarm_LoopIndex] towards CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] degrees.)
        • -------- This is the part that deals damage --------
        • Set VariableSet CarrionSwarm_AoE[CarrionSwarm_LoopIndex] = (Units within 250.00 of CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex].)
        • Unit Group - Pick every unit in CarrionSwarm_AoE[CarrionSwarm_LoopIndex] 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 CarrionSwarm_Caster[CarrionSwarm_LoopIndex]).) Equal to True
                • ((Picked unit) is hidden) Equal to False
                • ((Picked unit) is dead) Equal to False
                • ((Picked unit) is A structure) Equal to False
                • ((Picked unit) is in CarrionSwarm_IllegalTargets[CarrionSwarm_LoopIndex].) Equal to False
              • Then - Actions
                • -------- this part just checks if the target is behind/infront of the wave --------
                • Set VariableSet CarrionSwarm_VictimPoint = (Position of (Picked unit))
                • Set VariableSet CarrionSwarm_VictimAngle = (Angle from CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex] to CarrionSwarm_VictimPoint)
                • Set VariableSet CarrionSwarm_AngleLimitStart = (CarrionSwarm_Angle[CarrionSwarm_LoopIndex] + 90.00)
                • Set VariableSet CarrionSwarm_AngleLimitEnd = (CarrionSwarm_Angle[CarrionSwarm_LoopIndex] - 90.00)
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • And - All (Conditions) are true
                      • Conditions
                        • CarrionSwarm_VictimAngle Less than or equal to CarrionSwarm_AngleLimitStart
                        • CarrionSwarm_VictimAngle Greater than or equal to CarrionSwarm_AngleLimitEnd
                  • Then - Actions
                    • Unit Group - Add (Picked unit) to CarrionSwarm_LegalTargets[CarrionSwarm_LoopIndex]
                  • Else - Actions
                • Custom script: RemoveLocation(udg_CarrionSwarm_VictimPoint)
              • Else - Actions
        • Unit Group - Pick every unit in CarrionSwarm_LegalTargets[CarrionSwarm_LoopIndex] and do (Actions)
          • Loop - Actions
            • Unit - Cause CarrionSwarm_Caster[CarrionSwarm_LoopIndex] to damage (Picked unit), dealing (50.00 + (75.00 x (Real((Level of Carrion Swarm (custom) for CarrionSwarm_Caster[CarrionSwarm_LoopIndex]))))) damage of attack type Spells and damage type Normal
            • Unit Group - Add (Picked unit) to CarrionSwarm_IllegalTargets[CarrionSwarm_LoopIndex]
        • Unit Group - Remove all units from CarrionSwarm_AoE[CarrionSwarm_LoopIndex].
        • Custom script: DestroyGroup(udg_CarrionSwarm_AoE[udg_CarrionSwarm_LoopIndex])
        • -------- End of the damage part --------
        • Unit - Move CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_LoopPoint[CarrionSwarm_LoopIndex]
        • Custom script: RemoveLocation(udg_CarrionSwarm_LoopPoint[udg_CarrionSwarm_LoopIndex])
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CarrionSwarm_Distance[CarrionSwarm_LoopIndex] Less than or equal to 0.00
          • Then - Actions
            • Unit - Remove CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] from the game
            • Unit - Move CarrionSwarm_Caster[CarrionSwarm_LoopIndex] instantly to CarrionSwarm_TargetPoint[CarrionSwarm_LoopIndex]
            • Unit - Unhide CarrionSwarm_Caster[CarrionSwarm_LoopIndex]
            • Selection - Select CarrionSwarm_Caster[CarrionSwarm_LoopIndex] for (Owner of CarrionSwarm_Caster[CarrionSwarm_LoopIndex])
            • Unit Group - Remove all units from CarrionSwarm_IllegalTargets[CarrionSwarm_LoopIndex].
            • Custom script: DestroyGroup(udg_CarrionSwarm_IllegalTargets[udg_CarrionSwarm_LoopIndex])
            • Custom script: RemoveLocation(udg_CarrionSwarm_TargetPoint[udg_CarrionSwarm_LoopIndex])
            • Custom script: RemoveLocation(udg_CarrionSwarm_CastPoint[udg_CarrionSwarm_LoopIndex])
            • Set VariableSet CarrionSwarm_Caster[CarrionSwarm_LoopIndex] = CarrionSwarm_Caster[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Dummy[CarrionSwarm_LoopIndex] = CarrionSwarm_Dummy[CarrionSwarm_Index]
            • Set VariableSet CarrionSwarm_Distance[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Angle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_InvAngle[CarrionSwarm_LoopIndex] = 0.00
            • Set VariableSet CarrionSwarm_Index = (CarrionSwarm_Index - 1)
            • Set VariableSet CarrionSwarm_LoopIndex = (CarrionSwarm_LoopIndex - 1)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • CarrionSwarm_Index Equal to 0
              • Then - Actions
                • Trigger - Turn off (This trigger)
              • Else - Actions
          • Else - Actions

 

[Pyrogasm]

udg_ is a prefix automatically given to all variables made in the GUI variable editor. It stands for User Defined/Designated Global. Blizzard wanted to avoid potential name conflicts/problems so they made all their variables start with bj_ and made sure any user variable were given udg_ under the hood. If you extract war3map.j you should see a globals block where all your variable editor-defined variables are... defined, which will show their full names.

• What are you trying to accomplish with the VictimAngle stuff? If you already have an exclusion group for previously-hit targets (IllegalTargets) then this check will usually not do anything of value for you anyway since the first time units are found by the aoe search they will always be 'in front of' the search point anyway. This would only ever come into effect if a unit enters the aoe after it has already passed that unit but is still close enough to hit it 'behind' the search point; IMO this is an extreme edge case not worth considering.
  
  That being said, your angle comparison can fail when the wave is launched between 0..90 or 270..360 degrees (most noticeable near 0 degrees). Why? Angle between points returns in the range 0-359.99 but by adding/subtracting 90 from those endpoints you can end up with values outside that range that you then compare angles to. It's possible to then fail the angle check because of math even though the angle was 'correctly inside the intended arc'.
  
  There are solutions to this problem you can find on this site; they mostly show up for people wanting to do "backstab" detection. In this case, though, I think you should just forgo the angle detection altogether.
  
• CarrionSwarm_AoE doesn't need to be an array because it's never actually stored between firings of the periodic trigger. (you just do stuff with the untis found there and then delete the group right after)
  
  
• You are using more groups than you need to be (will refer to them by their suffixes from now on). _AoE is only temporarily used, but instead of adding 'legal' units from it into another group you could just remove illegal units from it and then loop over _AoE again to damage units afterward. Realistically you could just put the damage line inside that inner if where you add to _LegalTargets[] but I understand why it's structurally helpful to separate it for your own comprehension.
  
  
• The reason units are damaged multiple times is that you are destroying _IllegalTargets[] after the spell instance ends, and I expect you are not re-assigning it an empty unit group when the spell is cast. Because of this, units can never actually be added to it so this line always evaluates to false: ((Picked unit) is in CarrionSwarm_IllegalTargets[CarrionSwarm_LoopIndex].)
  
  • Set CarrionSwarm_IllegalTargets[CarrionSwarm_Index] = (Empty unit group)

 

[Uncle]

I attached an example of a custom Shockwave spell I made a little while ago. It's not perfect but it should be pretty similar to what you already have but with everything up and running the way you'd expect.

I personally wouldn't hide the unit and instead make it invulnerable, set it's transparency to 100%, and stun it like so:

• Unit - Make Caster Invulnerable
• Unit - Set vertex coloring of Caster to x/y/z with 100.00% transparency
• Custom script: call BlzPauseUnitEx(udg_Caster, true)

Then undo these effects at the end of the spell.

 

[Proxym8]

udg_ is a prefix automatically given to all variables made in the GUI variable editor. It stands for User Defined/Designated Global. Blizzard wanted to avoid potential name conflicts/problems so they made all their variables start with bj_ and made sure any user variable were given udg_ under the hood. If you extract war3map.j you should see a globals block where all your variable editor-defined variables are... defined, which will show their full names.

• What are you trying to accomplish with the VictimAngle stuff? If you already have an exclusion group for previously-hit targets (IllegalTargets) then this check will usually not do anything of value for you anyway since the first time units are found by the aoe search they will always be 'in front of' the search point anyway. This would only ever come into effect if a unit enters the aoe after it has already passed that unit but is still close enough to hit it 'behind' the search point; IMO this is an extreme edge case not worth considering.
  
  That being said, your angle comparison can fail when the wave is launched between 0..90 or 270..360 degrees (most noticeable near 0 degrees). Why? Angle between points returns in the range 0-359.99 but by adding/subtracting 90 from those endpoints you can end up with values outside that range that you then compare angles to. It's possible to then fail the angle check because of math even though the angle was 'correctly inside the intended arc'.

I checked by making a simple trigger to turn my unit in both negative and positive angle values; 0 - 360(359.99) angles worked as expected, as did the negative values (-50 value corresponded to turning 310 degrees).
I don't see the problem in my Victim_Angle triggers bearing this in mind?

 

[Pyrogasm]

Whatever you tested wasn't conclusive. I've illustrated an example of it failing here for you to see:

I have labeled each of the relevant angles here with the variables they correspond to in your most recent complete trigger post in this thread. I also gave them approximate numbers just in case that makes it easier to see where the issue comes from.

• The orange circle is the current location of search (_LoopPoint[] in your code) and the orange line is the angle of travel.
• Blue is the travel angle +90 degrees, and purple is travel angle -90 degrees. Note that these numbers can be outside the 0..360 range because you compute them by addition/subtraction without using a Modulo operator.
• The green circle is the location of a unit being tested for legality and the green line is the angle from the orange to the green circle. This number will always be in the 0..360 range because that's what AngleBetweenPoints returns.
• All angles are calculated relative to the black line at 0 degrees. (This is an implicit assumption/is baked into how angles work, I'm just highlighting it for clarity.)

The example I chose should find that _VictimAngle is within the desired range and is therefore a legal target, but it won't because one of the comparisons will always fail. In this case 325 is not less than 120. It's possible to construct a diagram like this to find points that will fail for any value of _Angle[] that causes either _AngleLimitEnd or _AngleLimitStart to overflow/underflow the 0..360 range.

There are a variety of ways to resolve this using the periodicity of sine/cosine functions. The easiest is generally going to compute a difference between the two relevant angles and constrain it to the range 0..180 by daisy-chaining cosine and arccosine:

• Conditions
  • (Acos((Cos((_VictimAngle - _Angle[]))))) Less than 90.00