Need Help with Triggers

[Arthur D]

Hi guys, I'm learning triggers to create custom spells so I really need some help. I was trying to make the Dark Archon's Confusion ability from SC2, targeting a group of enemy units and causing them to attack each other. Here are my triggers, and they don't seem to work:

• Confusion
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Confusion
  • Actions
    • Set Confusion_Caster = (Triggering unit)
    • Set Confusion_Owner = (Owner of Confusion_Caster)
    • Set Confusion_Ability_Level = (Level of Confusion for Confusion_Caster)
    • Set Confusion_Target_Location = (Target point of ability being cast)
    • Set Confusion_Target_Owner = (Random player from (All enemies of Confusion_Owner))
    • Set Confusion_Dummy_Ability = Confusion (Dummy)
    • -------- --------------------------------- --------
    • Set Confusion_Area_of_Effect = (200.00 + ((Real(Confusion_Ability_Level)) x 50.00))
    • Set Target_Group = (Units within Confusion_Area_of_Effect of Confusion_Target_Location matching ((((Matching unit) is A structure) Equal to False) and ((((Matching unit) is A Hero) Equal to False) and ((((Matching unit) is Mechanical) Equal to False) and ((((Matching unit) is a
    • -------- --------------------------------- --------
    • Unit Group - Pick every unit in (Random ((Number of units in Target_Group) / 2) units from Target_Group) and do (Actions)
      • Loop - Actions
        • Set Confusion_Picked_Location = (Position of (Picked unit))
        • -------- --------------------------------- --------
        • Unit - Create 1 Dummy for Confusion_Owner at Confusion_Picked_Location facing Default building facing degrees
        • Unit - Add a 1.50 second Generic expiration timer to (Last created unit)
        • Unit - Add Confusion_Dummy_Ability to (Last created unit)
        • Unit - Set level of Confusion_Dummy_Ability for (Last created unit) to Confusion_Ability_Level
        • Unit - Order (Last created unit) to Neutral Pit Lord - Doom (Picked unit)
        • -------- --------------------------------- --------
        • Wait until (((Picked unit) has buff Confusion (Dummy)) Equal to True), checking every 0.10 seconds
        • -------- --------------------------------- --------
        • Unit Group - Add (Picked unit) to Confused_Group
        • Unit - Set the custom value of (Picked unit) to (Player number of (Owner of (Picked unit)))
        • Unit - Change ownership of (Picked unit) to Neutral Extra and Retain color
        • -------- --------------------------------- --------
        • Player - Make Neutral Extra treat Confusion_Owner as an Ally with shared vision
        • Player - Make Confusion_Owner treat Neutral Extra as an Ally with shared vision
        • Player - Make Neutral Extra treat Confusion_Target_Owner as an Enemy with shared vision
        • Player - Make Confusion_Target_Owner treat Neutral Extra as an Enemy with shared vision
        • -------- --------------------------------- --------
        • Custom script: call RemoveLocation (udg_Confusion_Picked_Location)
    • Custom script: call RemoveLocation (udg_Confusion_Target_Location)
    • Custom script: call DestroyGroup (udg_Target_Group)

• Untitled Trigger 001
  • Events
    • Time - Every 0.10 seconds of game time
  • Conditions
    • (Confused_Group is empty) Equal to False
  • Actions
    • Unit Group - Pick every unit in Confused_Group and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Picked unit) has buff Confusion (Dummy)) Equal to False
          • Then - Actions
            • Unit Group - Remove (Picked unit) from Confused_Group
            • Unit - Change ownership of (Picked unit) to (Player((Custom value of (Picked unit)))) and Retain color
          • Else - Actions

Can anyone tell me what I did wrong? Any help will be much appreciated!

 

[KILLCIDE]

Set Confusion_Target_Owner = (Random player from (All enemies of Confusion_Owner))

^ There is a possibility that the selected player doesn't even have a unit inside the area of effect. Do you mind going into detail about what exactly isn't working?


Other nit-picky things:

• Set Confusion_Target_Owner = (Random player from (All enemies of Confusion_Owner))

  ^I believe this leaks a force

• Set Confusion_Dummy_Ability = Confusion (Dummy)

  ^There's no need to constantly cache this on cast
• It makes it really hard to read and or even see all the filters you use for your unit group loop. (Matching unit) is inefficient and can even be a pain to add/remove filters when you need to. I recommend just using an If/Then/Else inside the unit group loop to filter out what you need
• The wait timer inside your loop is pretty pointless. The unit should get the buff almost immediately

• Unit Group - Pick every unit in (Random ((Number of units in Target_Group) / 2) units from Target_Group)

  ^I believe this leaks as well
• I never see you turn on or turn off the loop trigger. Is it always on?

 

[Arthur D]

• Set Confusion_Target_Owner = (Random player from (All enemies of Confusion_Owner))

• Unit Group - Pick every unit in (Random ((Number of units in Target_Group) / 2) units from Target_Group)

Do you mind explaining how these leak? I'm a beginner and I still don't know many things.

I never see you turn on or turn off the loop trigger. Is it always on?

Yes, it is always on. In fact, it is the reason why the spell doesn't work. Without it, the spell works fine but there is just one thing if the spell is cast, the picked units will belong to neutral extra forever, which is not what I want. I think the loop trigger somehow ignores the condition that the picked units don't have the Confusion buff, so it just completely undoes the main trigger.
Do you have any suggestion for me to improve this spell? Thank you very much!

 

[KILLCIDE]

Do you mind explaining how these leak? I'm a beginner and I still don't know many things.

Memory Leaks by IcemanBo

Yes, it is always on. In fact, it is the reason why the spell doesn't work. Without it, the spell works fine but there is just one thing if the spell is cast, the picked units will belong to neutral extra forever, which is not what I want. I think the loop trigger somehow ignores the condition that the picked units don't have the Confusion buff, so it just completely undoes the main trigger.
Do you have any suggestion for me to improve this spell? Thank you very much!

There are a lot of improvements that can be done, but let's fix the original issue first. Can you remove the wait timer you have and let me know if it still doesn't work?

 

[Arthur D]

There are a lot of improvements that can be done, but let's fix the original issue first. Can you remove the wait timer you have and let me know if it still doesn't work?

Yeah, it still doesn't work. Like I said before, it did work without the loop trigger.

 

[KILLCIDE]

Yes, it is always on. In fact, it is the reason why the spell doesn't work. Without it, the spell works fine but there is just one thing if the spell is cast, the picked units will belong to neutral extra forever, which is not what I want.

Wait, do you mean when you cast it with more than one instance running?

You are checking for the right buff, correct? Also, does the "Doom" ability in your Object Editor actually apply the confusion buff?

 

[BloodDrunk]

Waits SHOULD NEVER! be used in unit group loops. It is clearly said when you select unit group action that wait should never be used there.

 

[IcemanBo]

True, it is because Waits will crash the thread when running inside a group enumeration.

The reason for this is that Waits only work inside TriggerActions.
The Group enumeration function is technicaly seperated from it and will be called for each unit inside the group.
So in GUI it looks like it is all inside the TriggerAction function, which is not really true.

This is also the reason why eariler declared local variables inside the TriggerAction function won't work anymore inside the group enumeration,
because it's technically an other function.