Need guidance with my triggers.

[Cookie!]

I started making maps for Warcraft sometime ago, but I've never had anyone take a look at my triggers to tell me if they're any good (or absolute rubbish ), so, well, here goes.

These triggers are supposed to detect if a Hero dies, wait for a period of time, and then revive the Hero back at his/her base; it's the typical revive system you see in most AoS style maps.

I'd like to know...
1) Is it any good at all?
2) Is it leak-free?
3) Is it MUI?
4) Any improvements?

(It does work, if that's any help )



Variables used:
Dead_Heroes - Unit[16]
Hero_Is_Dead - Boolean[16]
Seconds_To_Revive - Integer[16]
Num_Dead_Heroes - Integer
Hero_Revive_Timer - Timer[16]
Hero_Revive_Timer_Window - Timer Window[16]

temp_int, temp_unit_group etc are all exactly what they say.

Trigger # 1, Always ON.

• Hero Dies
  • Events
    • Unit - A unit Dies
  • Conditions
    • ((Dying unit) is A Hero) Equal to True
  • Actions
    • Set Dead_Heroes[((Player number of (Owner of (Dying unit))) - 1)] = (Dying unit)
    • Set Hero_Is_Dead[((Player number of (Owner of (Dying unit))) - 1)] = True
    • Set Seconds_To_Revive[((Player number of (Owner of (Dying unit))) - 1)] = (((Hero level of (Dying unit)) / 3) + 10)
    • Set Num_Dead_Heroes = (Num_Dead_Heroes + 1)
    • Countdown Timer - Start Hero_Revive_Timer[((Player number of (Owner of (Dying unit))) - 1)] as a One-shot timer that will expire in (Real(Seconds_To_Revive[((Player number of (Owner of (Dying unit))) - 1)])) seconds
    • Countdown Timer - Create a timer window for (Last started timer) with title Hero Respawns In
    • Countdown Timer - Show (Last created timer window) for (Owner of (Dying unit))
    • Set Hero_Revive_Timer_Window[((Player number of (Owner of (Dying unit))) - 1)] = (Last created timer window)
    • Trigger - Turn on Revive Hero <gen>

Trigger # 2, initially OFF.

• Revive Hero
  • Events
    • Time - Every 1.00 seconds of game time
  • Conditions
  • Actions
    • For each (Integer temp_int) from 0 to 15, do (Actions)
      • Loop - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • Hero_Is_Dead[temp_int] Equal to True
          • Then - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • Seconds_To_Revive[temp_int] Greater than 0
              • Then - Actions
                • Set Seconds_To_Revive[temp_int] = (Seconds_To_Revive[temp_int] - 1)
              • Else - Actions
                • Countdown Timer - Hide Hero_Revive_Timer_Window[temp_int] for (Player((temp_int + 1)))
                • Countdown Timer - Destroy Hero_Revive_Timer_Window[temp_int]
                • Set temp_unit_group = (Units owned by (Player((temp_int + 1))) matching (((Matching unit) is A structure) Equal to True))
                • Set temp_point = (Position of (First unit of group temp_unit_group))
                • Hero - Instantly revive Dead_Heroes[temp_int] at temp_point, Show revival graphics
                • Set Hero_Is_Dead[temp_int] = False
                • Set Num_Dead_Heroes = (Num_Dead_Heroes - 1)
                • Selection - Select Dead_Heroes[temp_int] for (Player((temp_int + 1)))
                • Camera - Pan camera for (Player((temp_int + 1))) to temp_point over 0.50 seconds
          • Else - Actions
            • Do nothing
    • Unit Group - Destroy unit group temp_unit_group
    • Point - Remove temp_point
    • If (Num_Dead_Heroes Less than 1) then do (Trigger - Turn off (This trigger)) else do (Do nothing)

Every player has only one Hero, and only one building, by the way.

So, have a look, and give me your opinion .

 

[M0RBiD]

you would better to use somthing like this

Event
------- Unit owning by player 1 Dies
Action
------- Wait x Seconds
------- Revive at (Any Location)


Event
------- Unit owning by player 2 Dies
Action
------- Wait x Seconds
------- Revive at (Any Location)

and for each player use one triger . . .
it was my idea and used it in my own map . . .
sry if is not good . . .

 

[vypur85]

Revive Hero
    Events
        Time - Every 1.00 seconds of game time
    Conditions
    Actions
        For each (Integer temp_int) from 0 to 15, do (Actions)
            Loop - Actions
                If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                    If - Conditions
                        Hero_Is_Dead[temp_int] Equal to True
                    Then - Actions
                        If (All Conditions are True) then do (Then Actions) else do (Else Actions)
                            If - Conditions
                                Seconds_To_Revive[temp_int] Greater than 0
                            Then - Actions
                                Set Seconds_To_Revive[temp_int] = (Seconds_To_Revive[temp_int] - 1)
                            Else - Actions
                                Countdown Timer - Hide Hero_Revive_Timer_Window[temp_int] for (Player((temp_int + 1)))
                                Countdown Timer - Destroy Hero_Revive_Timer_Window[temp_int]
                                Set temp_unit_group = (Units owned by (Player((temp_int + 1))) matching (((Matching unit) is A structure) Equal to True))
                                Set temp_point = (Position of (First unit of group temp_unit_group))
                                Hero - Instantly revive Dead_Heroes[temp_int] at temp_point, Show revival graphics
                                Set Hero_Is_Dead[temp_int] = False
                                Set Num_Dead_Heroes = (Num_Dead_Heroes - 1)
                                Selection - Select Dead_Heroes[temp_int] for (Player((temp_int + 1)))
                                Camera - Pan camera for (Player((temp_int + 1))) to temp_point over 0.50 seconds
                                [B]Unit Group - Destroy unit group temp_unit_group
                                Point - Remove temp_point[/B]
                    Else - Actions
                        Do nothing
        If (Num_Dead_Heroes Less than 1) then do (Trigger - Turn off (This trigger)) else do (Do nothing)

Change to the bolded ones above. Haven't checked thoroughly though.

 

[Accname]

besides the stuff vypur85 already mentioned (moving the "remove location" action inside the loop) there are no major flaws i can see with these triggers.
but, in the first trigger i would definitely declare a temp_int variable for ((Player number of (Owner of (Dying unit))) - 1), it would make things alot easier to read and to understand, and of course the most important, to change.
another thing is that you ask us if your trigger is MUI, it is not, but as you already said it isnt meant to be in the first place.
MUI would mean it will work for any unit on the whole map (as long as it reaches certain conditions), your trigger is designed to work only for a single unit of each player, this is called MPI.

 

[Cookie!]

MUI would mean it will work for any unit on the whole map (as long as it reaches certain conditions), your trigger is designed to work only for a single unit of each player, this is called MPI.

Ah, I see. Had those two confused with each other .