[Trigger] Critique my Trigger

[Gnuoy]

Sometimes in this map hero altars are rushed and they get destroyed, these triggers are supposed to search the map for rebuild altars, and then use those altars in the auto-rez triggers as well as future rush attack targets by the NPC.

Spoiler: find altar

• find altar
  • Events
  • Conditions
  • Actions
    • Set VariableSet owned_units_temp = (Units owned by Player 1 (Red).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[1] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[1] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[1])
                • Set VariableSet flyer_altar[1] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 2 (Blue).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[2] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[2] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[2])
                • Set VariableSet flyer_altar[2] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 3 (Teal).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[3] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[3] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[3])
                • Set VariableSet flyer_altar[3] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 4 (Purple).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[4] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[4] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[4])
                • Set VariableSet flyer_altar[4] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 13 (Maroon).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • Unit Group - Add (Picked unit) to altar_units
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[13] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[13] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[13])
                • Set VariableSet flyer_altar[13] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 14 (Navy).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[14] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[14] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[14])
                • Set VariableSet flyer_altar[14] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 15 (Turquoise).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • Unit Group - Add (Picked unit) to altar_units
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[15] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[15] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[15])
                • Set VariableSet flyer_altar[15] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.
    • Set VariableSet owned_units_temp = (Units owned by Player 16 (Violet).)
    • Unit Group - Pick every unit in owned_units_temp and do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Or - Any (Conditions) are true
              • Conditions
                • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
                • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
                • (Unit-type of (Picked unit)) Equal to Altar of Storms
                • (Unit-type of (Picked unit)) Equal to Altar of the Depths
                • (Unit-type of (Picked unit)) Equal to Pandaren Shrine
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (altar_unit_var[16] is dead) Equal to True
              • Then - Actions
                • Set VariableSet altar_unit_var[16] = (Picked unit)
                • Custom script: call RemoveLocation(udg_flyer_altar[16])
                • Set VariableSet flyer_altar[16] = (Position of (Picked unit))
              • Else - Actions
          • Else - Actions
        • Unit Group - Remove (Picked unit) from owned_units_temp.

Spoiler: auto rez - player 1 example

• hero auto rez 0
  • Events
    • Unit - A unit owned by Player 1 (Red) Becomes revivable
    • Unit - A unit owned by Player 2 (Blue) Becomes revivable
    • Unit - A unit owned by Player 3 (Teal) Becomes revivable
    • Unit - A unit owned by Player 4 (Purple) Becomes revivable
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • hc_bool Equal to True
      • Then - Actions
        • Unit - Remove (Revivable Hero) from the game
        • Countdown Timer - Start hardcore_cosmetic_timer[(Player number of (Owner of (Revivable Hero)))] as a One-shot timer that will expire in 0.10 seconds
      • Else - Actions
        • Countdown Timer - Start hero_rez_timer[(Player number of (Owner of (Revivable Hero)))] as a One-shot timer that will expire in 2.50 seconds

• hero auto rez 1
  • Events
    • Time - hero_rez_timer[1] expires
  • Conditions
  • Actions
    • Unit - Order altar_unit_var[1] to Revive Hero hero_array[1]

Spoiler: rush attack trigger

• spawn dragons
  • Events
  • Conditions
  • Actions
    • -------- this trigger makes flyers rush ancinets below wave 20 and altars above wave 20 -- it is ran in the main spawn trigger --------
    • Set VariableSet legion_index = (legion_index + 1)
    • If (legion_index Greater than 9) then do (Set VariableSet legion_index = 1) else do (Do nothing)
    • Unit - Create 1 Nether Dragon for legion_array[legion_index] at spawn_pt facing 90.00 degrees
    • Unit Group - Add (Last created unit) to demon_flyer
    • Set VariableSet sfx_pt = (Position of (Last created unit))
    • Special Effect - Create a special effect at sfx_pt using Abilities\Spells\Human\MassTeleport\MassTeleportTarget.mdl
    • Special Effect - Destroy (Last created special effect)
    • Set VariableSet legion_index = (legion_index + 1)
    • If (legion_index Greater than 9) then do (Set VariableSet legion_index = 1) else do (Do nothing)
    • Unit - Create 1 Nether Dragon Hatchling for legion_array[legion_index] at spawn_pt facing 90.00 degrees
    • Unit Group - Add (Last created unit) to demon_flyer
    • Set VariableSet sfx_pt = (Position of (Last created unit))
    • Special Effect - Create a special effect at sfx_pt using Abilities\Spells\Human\MassTeleport\MassTeleportTarget.mdl
    • Special Effect - Destroy (Last created special effect)
    • Custom script: call RemoveLocation(udg_flyer_target_pt)
    • Set VariableSet flyer_target_pt = (Position of ran_target)
    • Set VariableSet ran_num_fly_tar = (Random integer number between 1 and 4)
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • difficulty_setting Equal to 6
      • Then - Actions
        • Set VariableSet play_teleporter_sfx = True
        • For each (Integer A) from 1 to 12, do (Actions)
          • Loop - Actions
            • Set VariableSet legion_index = (legion_index + 1)
            • If (legion_index Greater than 9) then do (Set VariableSet legion_index = 1) else do (Do nothing)
            • Unit - Create 1 Nether Dragon Hatchling for legion_array[legion_index] at spawn_pt facing 90.00 degrees
            • Unit Group - Add (Last created unit) to demon_flyer
            • Set VariableSet sfx_pt = (Position of (Last created unit))
            • Special Effect - Create a special effect at sfx_pt using Abilities\Spells\Human\MassTeleport\MassTeleportTarget.mdl
            • Special Effect - Destroy (Last created special effect)
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • spawn_counter Less than or equal to 19
      • Then - Actions
        • Unit Group - Pick every unit in demon_flyer and do (Actions)
          • Loop - Actions
            • Unit - Order (Picked unit) to Attack-Move To flyer_target_pt
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • spawn_counter Greater than 19
      • Then - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (altar_unit_var[ran_num_fly_tar] is alive) Equal to True
          • Then - Actions
            • Unit Group - Pick every unit in demon_flyer and do (Actions)
              • Loop - Actions
                • Unit - Order (Picked unit) to Move To flyer_altar[ran_num_fly_tar]
          • Else - Actions
            • Unit Group - Pick every unit in demon_flyer and do (Actions)
              • Loop - Actions
                • Unit - Order (Picked unit) to Attack-Move To flyer_target_pt
      • Else - Actions

 

[Uncle]

find altar is very inefficient.

1) Use a Player Group variable to easily loop over all of those players. You should have multiple Player Group variables setup beforehand to keep track of different combinations of Players which can be used throughout all of your triggers. These should also be managed when a Player leaves the games, if necessary. For example you could create these Variables and Set them at the 0.01 second mark:

PG_Team_1_All
PG_Team_1_Playing
PG_Team_1_Leavers
PG_Team_2_All
PG_Team_2_Playing
PG_Team_2_Leavers
PG_Teams_All
PG_Teams_Playing
PG_Teams_Leavers

You can use Arrays to simplify this down to only three variables where the [index] represents the Team Number.

PG_Team_All[]
PG_Team_Playing[]
PG_Team_Leavers[]

[0] = Team 1, [1] = Team 2, [2] = Both Teams.

Edit: Remember that Team Numbers in GUI, unlike Player Numbers, start at 0. This is why I made [0] = Team 1.

The names should be self-explanatory. You're keeping track of all of the Players in a Team regardless of their leaver status. You're keeping track of Players in a Team that are actively playing the game. Lastly, you're keeping track of Players in a Team that have left the game.


2) Check if the picked player's altar_unit is actually dead before setting owned_units_temp. You don't need to do anything if it's still alive.


3) Delete this action since it doesn't actually do anything beneficial:

• Unit Group - Remove (Picked unit) from owned_units_temp.

4) You're leaking owned_units_temp multiple times throughout. Remember the rule for dealing with memory leaks:

Set variable, Use variable, Destroy variable.

So if you ever set a Unit Group variable before Destroying the previous one -> You've created a memory leak. This logic applies to Points and Player Groups as well.


5) Instead of doing this inefficient check:

• Or - Any (Conditions) are true
  • Conditions
    • (Unit-type of (Picked unit)) Equal to Altar of Elders (custom merc)
    • (Unit-type of (Picked unit)) Equal to Corrupted Altar of Elders (corrupt player)
    • (Unit-type of (Picked unit)) Equal to Altar of Storms
    • (Unit-type of (Picked unit)) Equal to Altar of the Depths
    • (Unit-type of (Picked unit)) Equal to Pandaren Shrine

Check for a single thing that all of these Altars have in common:

• Conditions
  • (Level of Altar Class for (Picked unit)) Equal to 1

^ A shared ability is a good choice.

 

[Gnuoy]

How critical is #1?

Also thought you could effectively destroy a unitgroup by removing every unit from it.

 

[Uncle]

How critical is #1?

Also thought you could effectively destroy a unitgroup by removing every unit from it.

I don't know how your map works exactly, but I assume that you'll often have inactive Player Slots. Is it intended that you run your game logic for every Player, even if they don't exist? Do you rely on inactive players in some way?

Also, I don't see why you wouldn't want to start using what I suggested in #1, it will basically speed up your trigger making process by X, where X is the number of Players. There's no need for all of that repetition.

Look how simple find altar becomes when taking advantage of Player Groups:

• find altar
  • Events
  • Conditions
  • Actions
    • -------- PG_Team indexes: [0] = Team 1, [1] = Team 2, [2] = Both Teams combined. --------
    • -------- --------
    • Player Group - Pick every player in PG_Team_Playing[2] and do (Actions)
      • Loop - Actions
        • Set VariableSet PN = (Player number of (Picked player))
        • -------- --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (altar_unit[PN] is alive) Equal to False
          • Then - Actions
            • Set VariableSet temp_unit_group = (Units owned by (Picked player).)
            • Unit Group - Pick every unit in temp_unit_group and do (Actions)
              • Loop - Actions
                • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Level of Altar Class for (Picked unit)) Equal to 1
                  • Then - Actions
                    • Set VariableSet altar_unit[PN] = (Picked unit)
                    • Custom script: call RemoveLocation( udg_flyer_altar[udg_PN] )
                    • Set VariableSet flyer_altar[PN] = (Position of (Picked unit))
                  • Else - Actions
            • Custom script: call DestroyGroup( udg_temp_unit_group )
          • Else - Actions

Plus the trigger is now dynamic. When a Player leaves the game it'll automatically stop running Actions for them since they'll no longer be in PG_Team_Playing[2].

Also, you don't NEED to rely on a Player Group Array here. I like this design since it's dynamic and keeps the variable usage low. But maybe a single variable like this would make more sense to you:

• Player Group - Pick every player in All_Active_Players and do (Actions)

And to answer your last question:

No, the game isn't going to destroy your empty Unit Groups. You can add/remove units to and from a Unit Group whenever you want. So the game is always going to assume that your Unit Group is permanent, otherwise, it could end up destroying a Unit Group that you wanted to keep around.

 

[Pyrogasm]

Also thought you could effectively destroy a unitgroup by removing every unit from it.

Absolutely not. An empty unit group is still an object.