[Solved] My trigger doesn't work, why?

[hugope]

It works like this:

Both units are created. I also tried with Attack-Move-To instead of Move To and it worked.



It doesn't work like this:

In this situation only one unit is created instead of two. This doesn't make any sense, why does this happen? I also tried with the spell Shockwave instead of Carrion Swarm, but the results were the same.

 

[Uncle]

Replace the Event with this:

• Events
• Unit - A unit Starts the effect of an ability

"Begins casting..." happens before the spell actually executes, and is interruptible by the user by canceling the order. The unit's orders can also get interrupted by a stun for example so you really shouldn't be relying on this Event for this type of effect. Instead, you want to use the "Starts the effect..." Event which happens when the ability has successfully executed (mana spent, cooldown started, effects of the ability occur).

And replace all instances of (Casting unit) with (Triggering unit).

Additionally, make sure that your Dummy unit can actually cast Carrion Swarm. This means that the Dummy has the Ability learned and the abilities Cast Range, Mana Cost, Requirements, etc are all setup properly.

You should also make sure that your Dummy unit is setup correctly. I recommend basing it off of the Locust unit and then modifying it with the following stats:
Movement Speed: 0
Movement Type: None
Attacks Enabled: None


I recommend looking into the basics of triggering (links in my signature). The Wait action is generally not a good option for custom spells since a lot of information can get lost/replaced after the Wait. Waits are generally reserved for triggers that can't have multiple instances running at the same time.

For example, an Event like this only happens once a game and doesn't set any Event Responses, so it's fairly safe to use a Wait in it without running into issues:

• Events
• Time - Elapsed game time is 10.00 seconds
• Actions
• Wait 3.00 seconds

But a trigger like yours with an Event that runs WHENEVER a unit casts an ability can happen multiple times and there can be multiple instances of it running at the same time. This can complicate things. For starters, this Event sets these Event Responses: Casting unit, Triggering unit (same as Casting unit), Target unit of ability being cast (if able), and Target point of ability being cast (if able).

Understand that out of these 4 Event Responses only Triggering unit will remain usable after a Wait action. Triggering unit is a special exception that works like a local variable that will remain the same throughout the trigger no matter what. The other Event Responses will either get unset (losing their data) or act as global variables which can have their data replaced with new data (potentially unwanted data) from the use of other triggers that share these same Event Responses.

There's many methods to combat these issues:

• Use Event Responses which don't lose their data like Triggering unit. This is sometimes not possible as Triggering unit is the only real exception.
• Use local variables which retain their data throughout the trigger (not intended for GUI users but still usable with Custom script).
• Don't use Waits at all and instead use some form of Indexing with Timers/Periodic Intervals in order to mimic the behavior of a Wait while retaining all of the necessary information. This is usually what custom spells require especially if you want some sort of time component to them (damage over time, delays, etc).
• Code your triggers in Lua/Jass for far greater control (more advanced).

Keep in mind that if you aren't using Waits then you generally don't have to worry about issues regarding Event Responses. That being said, it's possible to overwrite Event Responses and cause problems inside of a trigger even without any Waits. This can happen if the Actions of one trigger cause another trigger to run and both triggers use the same Event Responses.

After you've gotten a better understanding of how this stuff works you can move on to memory leaks, which there are many of in your current trigger. These aren't the end of the world and can often times be harmless/ignored but it's still something to be aware of. Removing memory leaks can help improve your map's performance since you'd be getting rid of information that is no longer needed but is still being stored in the game's memory. To simplify it you can think of it like emptying your Recycle Bin on Windows. Link in my signature.

 

[hugope]

It works! This is such a complete and informative answer. Uncle thank you again!

I didn't know the "A unit starts the effect of an ability" was so cool. Much better than what I used to do against stun, lol:

Is this the correct way to replace Wait?

If there's a more efficient way to replace wait please tell me, since I have a LOT of work to do.


I used to think that all of the Event Responses remained the same throughout the trigger no matter what. So... only Triggering unit does.




Sorry if I'm being a bother, but could you please help me with something else?
With this next trigger everything works as inteded EXCEPT that the lightning does not change color. It should be red. I tried with different colors but nothing happens. Also tried using using the variable CustomLightning instead of Last Created Lightning Effect but nothing happens either.

Finally, I found a tutorial about lightning effects and in the tutorial they change in the object editor "Art - Maximum Pitch Angle (degrees)" to make the unit change its position to horizontal. I did the same but nothing happens at all. I found some threads that say "Art - Maximum Pitch Angle (degrees)" got broken with the Reforged version but I'm not sure if its true.


Definitely will check your signature. Im particularly interested in "Things That Leak"

 

[Uncle]

Lightning is very buggy in Reforged. There's probably a way to get it working though, try searching around for more recent posts on lightning + reforged.

And you can still use Waits in your original trigger since you don't need any other Event Response besides Triggering unit. I assume you want something like this:

• Spell
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
  • Actions
    • Set VariableSet CS_Point[0] = (Position of (Triggering unit))
    • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Triggering unit)) degrees.)
    • Unit - Create 1 Dummy for (Owner of (Triggering unit)) at CS_Point[1] facing Default building facing degrees
    • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
    • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
    • Custom script: call RemoveLocation (udg_CS_Point[0])
    • Custom script: call RemoveLocation (udg_CS_Point[1])
    • Wait 3.00 seconds
    • Set VariableSet CS_Point[0] = (Position of (Triggering unit))
    • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Triggering unit)) degrees.)
    • Unit - Create 1 Dummy for (Owner of (Triggering unit)) at CS_Point[1] facing Default building facing degrees
    • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
    • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
    • Custom script: call RemoveLocation (udg_CS_Point[0])
    • Custom script: call RemoveLocation (udg_CS_Point[1])

I cleaned up the leaks by using the CS_Point variable. It's an Array which is why it has the [brackets]. You could use two different Point variables instead if you don't want to use a Point array.


Also, Trigger 2 doesn't really make much sense, I see what you were going for but there's no reason to use that method unless you want to make the ability MUI (Multi unit instanceable) -> A fancy word which means that you could give this ability to multiple units and have them all cast it at the same time without any issues. And if you did want it to be MUI then you're missing a large chunk of what's needed to get it to work properly. This could be in the form of Unit Indexing (link in signature) which would use a Unit Group to contain all of the casters and Arrays to track the amount of time that has passed. Or you can use Dynamic Indexing which doesn't require a Unit Indexer system but requires some extra steps/variables. Both have their use cases and it's important to understand both of them if you want to make MUI spells.

Also, you made some other mistakes. Here's what your Trigger 2 currently does:
Every 3 seconds increase an Integer by 1 then check if it's Equal to 3. If it ISN'T 3 then create a dummy unit.

This means that you're creating a Dummy after 3.00 seconds has passed, then another after 6.00 seconds has passed, but you're NOT creating a Dummy after 9.00 seconds has passed since your Integer would be Equal to 3. You then continue to create Dummys from that point on. Also, Converting the Integer to a Real serves no purpose.


Here's an example of your spell using the Unit Indexing method (link in signature) which takes advantage of the unit's custom value in order to save data to it. In this case using this method gives each unit their own version of the CS_Delay variable so that we can keep track of the amount of time that has passed on an individual basis:

• Spell
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
  • Actions
    • Set VariableSet CS_CV = (Custom value of (Triggering unit))
    • Set VariableSet CS_Delay[CS_CV] = 3.00
    • -------- --------
    • Set VariableSet CS_Point[0] = (Position of (Triggering unit))
    • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Triggering unit)) degrees.)
    • Unit - Create 1 Dummy for (Owner of (Triggering unit)) at CS_Point[1] facing Default building facing degrees
    • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
    • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
    • Custom script: call RemoveLocation (udg_CS_Point[0])
    • Custom script: call RemoveLocation (udg_CS_Point[1])
    • -------- --------
    • Unit Group - Add (Triggering unit) to CS_Group
    • Trigger - Turn on Periodic <gen>

• Periodic
  • Events
    • Time - Every 0.20 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in CS_Group and do (Actions)
      • Loop - Actions
        • Set VariableSet CS_CV = (Custom value of (Picked unit))
        • Set VariableSet CS_Delay[CS_CV] = (CS_Delay[CS_CV] - 0.20)
        • -------- --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • CS_Delay[CS_CV] Less than or equal to 0.01
          • Then - Actions
            • Unit Group - Remove (Picked unit) from CS_Group.
            • -------- --------
            • Set VariableSet CS_Point[0] = (Position of (Picked unit))
            • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Picked unit)) degrees.)
            • Unit - Create 1 Dummy for (Owner of (Picked unit)) at CS_Point[1] facing Default building facing degrees
            • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
            • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
            • Custom script: call RemoveLocation (udg_CS_Point[0])
            • Custom script: call RemoveLocation (udg_CS_Point[1])
          • Else - Actions
    • -------- --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Number of units in CS_Group) Equal to 0
      • Then - Actions
        • Trigger - Turn off (This trigger)
      • Else - Actions

Note that this isn't 100% precise since our Periodic Interval runs every 0.20 seconds. The smaller this number the more precise it becomes but that also comes at a cost of performance. Currently the delay could go anywhere between ~2.80 -> 3.20 seconds depending on when a unit casts an ability. Also, understand that Waits aren't 100% precise either, at least when playing online since they need to sync with other players and can vary due to ping.

 

[hugope]

I searched about lightning and found interesting information. According to some threads you can't change lightning color in reforged. This thread says that you can only change lightning color with the previous graphics. In another thread a user explains how to change the color of the preset lightnings of warcraft 3, which is useful but quite limited. Luckily someone created a bundle of lightnings that can be imported.
I guess "Art - Maximum Pitch Angle (degrees)" is also broken.

You are right, thats exactly what I wanted.

I do want MUI.

I think what Trigger 2 is lacking is a Turn Off in the Then actions. Otherwise the IntegerCount keeps growing by 1 every 3 seconds and additional dummys are created. Also, I'm not cleaning memory leaks apparently. And it isn't MUI, of course.

My map has a lot of units (1635) is it okay to use the Unit Indexing method?

What size should the CS_Delay array be?
"This method gives each unit their own version of the CS_Delay" I assume it also gives each unit their own version of CS_Point[0] and CS_Point[1] right? I'm having a hard time understanding this, I thought that Variables only were able to have one value at the same time.


What if I want to create more dummys? Is this the correct approach?

 

[Uncle]

It looks like you've got the right idea. Sorry if this is information overload or if I contradict myself anywhere. There's so much to say and it's hard to remember everything.

Anyway, you can use the Wait approach for this trigger since it only uses the Triggering unit Event Response. There's no other Event Responses that need to be remembered after the Waits so you don't have to worry about losing information. You also aren't keeping track of any global variables between the Waits so there's no fear of those getting overwritten. It's actually worse for performance to use the Periodic Interval approach in this case.

That being said, you had the right idea and I would NOT delete these triggers. They're very useful to keep around so that you can use them as a template for future spells that would actually need this method of indexing. And the Unit Indexer system is very lightweight so I don't think you'll notice a performance difference even with 1000's of units.

However, there's one last thing that will cause problems that I forgot to mention which is Floating point error. Unfortunately, Reals can and will lose a very small amount of precision after being used in Arithmetic. For example, you may end up with a number like 6.0000000001 instead of 6.00 after subtracting 0.20 from CS_Delay. This is an issue because in your Condition you're checking if CS_Delay is Equal to EXACTLY 6.00, which it wouldn't be, despite being extremely close to it.

The solution is to use Integers as they do not suffer from this issue. So what you want to do is replace CS_Delay with an Integer array and then you just need to do some math. If the Periodic Interval runs once every 0.20 seconds and increases by 1 each time then 5 is equal to 1 second passed (0.20*5 = 1). That means that 45 would be your starting value (5*9 = 45) and you would want to check in your Conditions if CS_Delay was equal to 30, 15, and 0. Additionally, you can use Math - Modulo to essentially check multiples of a number which saves you from more work:

• Spell
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
  • Actions
    • Set VariableSet CS_CV = (Custom value of (Triggering unit))
    • Set VariableSet CS_Delay[CS_CV] = 45
    • -------- --------
    • Set VariableSet CS_Point[0] = (Position of (Triggering unit))
    • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Triggering unit)) degrees.)
    • Unit - Create 1 Dummy for (Owner of (Triggering unit)) at CS_Point[1] facing Default building facing degrees
    • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
    • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
    • Custom script: call RemoveLocation (udg_CS_Point[0])
    • Custom script: call RemoveLocation (udg_CS_Point[1])
    • -------- --------
    • Unit Group - Add (Triggering unit) to CS_Group
    • Trigger - Turn on Periodic <gen>

• Periodic
  • Events
    • Time - Every 0.20 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in CS_Group and do (Actions)
      • Loop - Actions
        • Set VariableSet CS_CV = (Custom value of (Picked unit))
        • Set VariableSet CS_Delay[CS_CV] = (CS_Delay[CS_CV] - 1)
        • -------- --------
        • -------- Look into Mod (modulo), it's very useful for checking for multiples of a value. Note that it will also run when CS_Delay is 0: --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (CS_Delay[CS_CV] mod 15) Equal to 0
          • Then - Actions
            • Unit Group - Remove (Picked unit) from CS_Group.
            • -------- --------
            • Set VariableSet CS_Point[0] = (Position of (Picked unit))
            • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Picked unit)) degrees.)
            • Unit - Create 1 Dummy for (Owner of (Picked unit)) at CS_Point[1] facing Default building facing degrees
            • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
            • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
            • Custom script: call RemoveLocation (udg_CS_Point[0])
            • Custom script: call RemoveLocation (udg_CS_Point[1])
          • Else - Actions
    • -------- --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Number of units in CS_Group) Equal to 0
      • Then - Actions
        • Trigger - Turn off (This trigger)
      • Else - Actions

Again, this solution isn't necessary for your Carrion Swarm ability, I'm just showing you how to do it properly for when you do use it.

Edit:
Also, I forgot an important edge case. What happens if these casters die while inside of CS_Group? You'll definitely want to remove them from the CS_Group if this happens, it's not safe to run code for dead units especially with the Unit Indexer system which recycles unused custom values:

• Periodic
  • Events
    • Time - Every 0.20 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in CS_Group and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Picked unit) is alive) Equal to True
          • Then - Actions
            • Set VariableSet CS_CV = (Custom value of (Picked unit))
            • Set VariableSet CS_Delay[CS_CV] = (CS_Delay[CS_CV] - 1)
            • -------- --------
            • -------- Look into Mod (modulo), it's very useful for checking for multiples of a value. Note that it will also run when CS_Delay is 0: --------
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (CS_Delay[CS_CV] mod 15) Equal to 0
              • Then - Actions
                • Unit Group - Remove (Picked unit) from CS_Group.
                • -------- --------
                • Set VariableSet CS_Point[0] = (Position of (Picked unit))
                • Set VariableSet CS_Point[1] = (CS_Point[0] offset by -10.00 towards (Facing of (Picked unit)) degrees.)
                • Unit - Create 1 Dummy for (Owner of (Picked unit)) at CS_Point[1] facing Default building facing degrees
                • Unit - Add a 0.50 second Generic expiration timer to (Last created unit)
                • Unit - Order (Last created unit) to Undead Dreadlord - Carrion Swarm CS_Point[0]
                • Custom script: call RemoveLocation (udg_CS_Point[0])
                • Custom script: call RemoveLocation (udg_CS_Point[1])
              • Else - Actions
          • Else - Actions
            • Unit Group - Remove (Picked unit) from CS_Group.
    • -------- --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Number of units in CS_Group) Equal to 0
      • Then - Actions
        • Trigger - Turn off (This trigger)
      • Else - Actions

 

[hugope]

Sorry if this is information overload

Don't worry, I have a lot of free time for now, and also, I have the ability to process lots of information.

That being said, you had the right idea and I would NOT delete these triggers. They're very useful to keep around

Of course. I'm learning a lot, I'm not only replicating, it will definitely help me for creating future triggers.

Reals can and will lose a very small amount of precision after being used in Arithmetic.

This just solved me a problem with another trigger! The values I was getting weren't precise, I would get a -0 instead of a normal 0 and similar discrepancies.


What size should the arrays be? Should the size be the same number as the number of units that will cast the spell?

You are using the Event response Triggering Unit in the trigger Periodic, but the event is Time - Periodic Event, how does that work?

 

[Uncle]

Don't touch Array Size in 90% of cases. Most Array sizes are dynamic and will increase on their own to match your needs. Also note that an Array's size limit is 32,768. If you ever reach this limit then look into Hashtables as they can serve the exact same function with a much larger limit (they're also a bit more complex so stick with Arrays whenever possible). I believe the only variables that actually need defined Array Sizes are: Unit Groups, Player Groups, and Timers. I may be forgetting one or two.

Also, Triggering unit is a mistake that I missed. It should be Picked unit.