[Trigger] What is problem with this?

omidh · Aug 29, 2011

Hi
Please someone review my Triggers and tell me what problems it have?
LIFESTEAL

LS0
- Events
  - Map initialization
- Conditions
- Actions
  - -------- Spirits Number --------
  - Set LS_Integer[1] = 5
  - -------- Sprits Expration Time --------
  - Set LS_Real[3] = 3.00
  - -------- Ghoul Number For Heroes --------
  - Set LS_Integer[2] = 4
  - -------- Ghoul Number For Non-Hero --------
  - Set LS_Integer[3] = 1
  - -------- Ghouls Expration Time --------
  - Set LS_Real[2] = 30.00
  - -------- Heal Return Per Spirit (100 x LifeSteal lvl ) --------
  - Set LS_Real[2] = 100.00
  - -------- Radius of Effect --------
  - Set LS_Real[1] = 600.00
  - -------- --------------- --------
  - Hashtable - Create a hashtable
  - Set LS_Hash = (Last created hashtable)

LS1
- Events
  - Unit - A unit Starts the effect of an ability
- Conditions
  - (Ability being cast) Equal to LifeSteal
- Actions
  - Set LS_Unit[1] = (Triggering unit)
  - Set LS_Points[1] = (Position of LS_Unit[1])
  - Set LS_G1 = (Units within LS_Real[1] of (Position of LS_Unit[1]) matching (((((Matching unit) is A structure) Equal to False) and (((Matching unit) has buff Shadow Strike LS) Equal to False)) and ((((Matching unit) belongs to an ally of (Owner of LS_Unit[1])) Equal to Fa
  - For each (Integer A) from 1 to LS_Integer[1], do (Actions)
    - Loop - Actions
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        ((LS_G1 is empty) Equal to True) and ((All units of LS_G1 are dead) Equal to True)
        
        Then - Actions
        
        Skip remaining actions
        
        Else - Actions
      - Unit - Create 1 LS_Spirit for (Owner of LS_Unit[1]) at LS_Points[1] facing Default building facing degrees
      - Set LS_Unit[2] = (Last created unit)
      - Set LS_HandleID = (Key (Last created unit))
      - Hashtable - Save Handle OfLS_Unit[1] as (Key Caster) of LS_HandleID in LS_Hash
      - Set LS_Unit[3] = (Random unit from LS_G1)
      - Unit - Add a LS_Real[3] second Generic expiration timer to LS_Unit[2]
      - Unit - Add LS_Strike to LS_Unit[2]
      - Unit - Set level of LS_Strike for LS_Unit[2] to (Level of LifeSteal for LS_Unit[1])
      - Unit - Order LS_Unit[2] to Night Elf Warden - Shadow Strike LS_Unit[3]
      - Unit Group - Remove LS_Unit[3] from LS_G1
  - Custom script: call RemoveLocation(udg_LS_Points[1])
  - Custom script: call DestroyGroup (udg_LS_G1)

LS2
- Events
  - Unit - A unit Starts the effect of an ability
- Conditions
  - (Ability being cast) Equal to LS_Strike
- Actions
  - Set LS_Unit[1] = (Triggering unit)
  - Set LS_Unit[5] = (Target unit of ability being cast)
  - Unit Group - Add LS_Unit[5] to LS_G2
  - Set LS_HandleID = (Key (Triggering unit))
  - Set LS_Unit[4] = (Load (Key Caster) of LS_HandleID in LS_Hash)
  - Set LS_Unit[3] = (Target unit of ability being cast)
  - Set LS_Points[1] = (Position of LS_Unit[1])
  - Unit - Create 1 LS_Dummy for (Owner of LS_Unit[1]) at LS_Points[1] facing Default building facing degrees
  - Set LS_Unit[2] = (Last created unit)
  - Unit - Add a 3.00 second Generic expiration timer to LS_Unit[2]
  - Unit - Add LS_Miss to LS_Unit[2]
  - Unit - Set level of LS_Miss for LS_Unit[2] to (Level of LS_Strike for LS_Unit[1])
  - Unit - Order LS_Unit[2] to Neutral Pandaren Brewmaster - Drunken Haze LS_Unit[3]
  - Hashtable - Clear all child hashtables of child LS_HandleID in LS_Hash
  - Set LS_HandleID = (Key (Target unit of ability being cast))
  - Hashtable - Save Handle OfLS_Unit[4] as (Key Owner) of LS_HandleID in LS_Hash
  - Unit - Add a 0.20 second Generic expiration timer to LS_Unit[1]
  - Custom script: call RemoveLocation(udg_LS_Points[1])

LS3
- Events
  - Unit - A unit Dies
- Conditions
  - ((Triggering unit) is in LS_G2) Equal to True
- Actions
  - Set LS_Unit[2] = (Triggering unit)
  - Set LS_HandleID = (Key (Triggering unit))
  - Set LS_Points[1] = (Position of LS_Unit[2])
  - Set LS_Unit[1] = (Load (Key Owner) of LS_HandleID in LS_Hash)
  - Unit Group - Remove LS_Unit[2] from LS_G2
  - Hashtable - Clear all child hashtables of child LS_HandleID in LS_Hash
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - (LS_Unit[1] is dead) Equal to True
    - Then - Actions
      - Skip remaining actions
    - Else - Actions
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
    - Then - Actions
      - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        
        If - Conditions
        
        (LS_Unit[2] is A Hero) Equal to True
        
        Then - Actions
        
        For each (Integer A) from 1 to LS_Integer[2], do (Actions)
        
        Loop - Actions
        
        Unit - Create 1 Ghoul LS for (Owner of LS_Unit[1]) at LS_Points[1] facing Default building facing degrees
        
        Unit - Add a LS_Real[2] second Generic expiration timer to (Last created unit)
        
        Set LS_HandleID = (Key (Last created unit))
        
        Hashtable - Save Handle OfLS_Unit[1] as (Key Owner) of LS_HandleID in LS_Hash
        
        Else - Actions
        
        For each (Integer B) from 1 to LS_Integer[3], do (Actions)
        
        Loop - Actions
        
        Unit - Create 1 Ghoul LS for (Owner of LS_Unit[1]) at LS_Points[1] facing Default building facing degrees
        
        Unit - Add a LS_Real[2] second Generic expiration timer to (Last created unit)
        
        Set LS_HandleID = (Key (Last created unit))
        
        Hashtable - Save Handle OfLS_Unit[1] as (Key Owner) of LS_HandleID in LS_Hash
        
        Game - Display to (All players) the text: ddd
    - Else - Actions
  - Custom script: call RemoveLocation(udg_LS_Points[1])

LS4
- Events
  - Unit - A unit Dies
- Conditions
  - (Unit-type of (Triggering unit)) Equal to Ghoul LS
- Actions
  - Set LS_Unit[2] = (Triggering unit)
  - Set LS_HandleID = (Key (Triggering unit))
  - Set LS_Points[1] = (Position of LS_Unit[2])
  - Set LS_Unit[1] = (Load (Key Owner) of LS_HandleID in LS_Hash)
  - Hashtable - Clear all child hashtables of child LS_HandleID in LS_Hash
  - If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    - If - Conditions
      - (LS_Unit[1] is dead) Equal to True
    - Then - Actions
      - Skip remaining actions
    - Else - Actions
  - Unit - Create 1 LS_Spirit for (Owner of LS_Unit[1]) at LS_Points[1] facing Default building facing degrees
  - Set LS_Unit[3] = (Last created unit)
  - Unit - Add LS_Finger to LS_Unit[3]
  - Unit - Order LS_Unit[3] to Special Archimonde - Finger Of Death LS_Unit[1]
  - Custom script: call RemoveLocation(udg_LS_Points[1])

LS5
- Events
  - Unit - A unit Starts the effect of an ability
- Conditions
  - (Ability being cast) Equal to LS_Finger
- Actions
  - Set LS_Unit[1] = (Triggering unit)
  - Set LS_Unit[2] = (Target unit of ability being cast)
  - Unit - Set life of LS_Unit[2] to ((Life of LS_Unit[2]) + (LS_Real[2] x (Real((Level of LifeSteal for LS_Unit[2])))))
  - Unit - Remove LS_Unit[1] from the game
  - Special Effect - Create a special effect attached to the origin of LS_Unit[2] using Abilities\Spells\Undead\AnimateDead\AnimateDeadTarget.mdl
  - Special Effect - Destroy (Last created special effect)

Thanks

Bribe · Aug 29, 2011

Things like LS_Integer, LS_Real, LS_Unit, LS_Points should not be arrays as
arrays are a special feature of the game not a toy to reduce the creativity
for you in thinking of variable names. As well, arrays start at index 0 not
at 1.

((LS_G1 is empty) Equal to True) and ((All units of LS_G1 are dead) Equal to True)

The first checks that the group is empty which is fine but why do you need
the second check? I think these two should be seperated by an "or" if you
want to use them.

Rather than using "skip remaining actions" you could drag the rest of the
actions into its appropriate "else" actions.

Once you get rid of the arrays that will make this whole thing a lot more
readable.

omidh · Aug 29, 2011

Okay, I Make Arrays to reduce variables number

Bribe · Aug 29, 2011

It's not worth it, arrays take up way more RAM than normal variables do,
they are slower to work with in-game and like I said destroy readability.

Also, if you do it right your variables can be shared with other systems.
Things like TempUnit or TargetPoint, OffsetPoint and CenterPoint can all
be used by lots of different systems.

[Trigger] What is problem with this?

omidh

omidh

Resources

Bribe

Bribe

Resources

omidh

omidh

Resources

Bribe

Bribe

Resources

Similar threads