[Trigger] Help with triggers please..

[scourge069]

I'm working on this spell and I call it "Body-link Magic".
This is what the spell does:

The caster invokes a powerful spell to a target hero which results to the target feeling the senses of the caster, each time the caster takes damage, the targeted hero also takes a portion of the total damage taken by the caster as damage to itself. Lasts 20 seconds.
Level 1 - 35% of the damage taken.
Level 2 - 55% of the damage taken.
Level 3 - 75% of the damage taken.

*here are the triggers:

triggers

Body-link Magic Cast

• Events
  • Unit - A unit Starts the effect of an ability
• Conditions
  • (Ability being cast) Equal to Body-link Magic
• Actions
  • Set BMagic_Target = (Target unit of ability being cast)
  • Set BMagic_Timer = 20.00
  • Set BMagic_ReturnPercentage = (0.15 + (0.20 x (Real((Level of (Ability being cast) for (Triggering unit))))))
  • Hashtable - Save Handle OfBMagic_Target as 0 of (Key (Triggering unit)) in BMagic_Hash
  • Hashtable - Save BMagic_Timer as 1 of (Key (Triggering unit)) in BMagic_Hash
  • Hashtable - Save BMagic_ReturnPercentage as 2 of (Key (Triggering unit)) in BMagic_Hash
  • Unit Group - Add (Triggering unit) to BMagic_CasterGroup
  • Trigger - Turn on Bodylink Magic Interval <gen>
  • Player - Disable Body-link Magic for (Owner of (Triggering unit))
  • Player - Disable Body-link Magic Book for (Owner of BMagic_Target)
  • Unit - Add Body-link Magic (Caster) to (Triggering unit)
  • Unit - Set level of Body-link Magic (Caster) for (Triggering unit) to (Level of (Ability being cast) for (Triggering unit))
  • Unit - Add Body-link Magic Book to BMagic_Target
  • Unit - Set level of Body-link Magic (Target) for BMagic_Target to (Level of (Ability being cast) for (Triggering unit))

Body-link Magic Interval

• Events
  • Time - Every 1.00 seconds of game time
• Conditions
• Actions
  • Unit Group - Pick every unit in BMagic_CasterGroup and do (Actions)
    • Loop - Actions
      • Set BMagic_Target = (Load 0 of (Key (Picked unit)) in BMagic_Hash)
      • Set BMagic_Timer = (Load 1 of (Key (Picked unit)) from BMagic_Hash)
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • BMagic_Timer Greater than 0.00
        • Then - Actions
          • Hashtable - Save (BMagic_Timer - 1.00) as 1 of (Key (Picked unit)) in BMagic_Hash
        • Else - Actions
          • Unit - Remove Body-link Magic (Caster) from (Picked unit)
          • Unit - Remove Body-link Magic (Caster) buff from (Picked unit)
          • Unit - Remove Body-link Magic Book from BMagic_Target
          • Unit - Remove Body-link Magic (Target) buff from BMagic_Target
          • Player - Enable Body-link Magic for (Owner of (Picked unit))
          • Hashtable - Clear all child hashtables of child (Key (Picked unit)) in BMagic_Hash
          • Unit Group - Remove (Picked unit) from BMagic_CasterGroup
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Number of units in BMagic_CasterGroup) Equal to 0
            • Then - Actions
              • Trigger - Turn off (This trigger)
            • Else - Actions

Body-link Magic Damage Event

• Events
  • Game - GDD_Event becomes Equal to 0.00
• Conditions
  • (GDD_DamagedUnit is in BMagic_CasterGroup) Equal to True
  • (GDD_DamageSource belongs to an enemy of (Owner of GDD_DamagedUnit)) Equal to True
• Actions
  • Unit Group - Pick every unit in BMagic_CasterGroup and do (Actions)
    • Loop - Actions
      • Set BMagic_Target = (Load 0 of (Key (Picked unit)) in BMagic_Hash)
      • Set BMagic_ReturnPercentage = (Load 2 of (Key (Picked unit)) from BMagic_Hash)
      • Set BMagic_DamageDealt = (GDD_Damage x BMagic_ReturnPercentage)
      • Unit - Cause GDD_DamagedUnit to damage BMagic_Target, dealing BMagic_DamageDealt damage of attack type Spells and damage type Magic
      • Set BMagic_DamageDealt = 0.00

*I haven't found any errors with this triggers, but I would like to know the comments of those who a are a lot experienced than me specially that I have only recently learned about hashtables.
I'll give +REP to those who could tell me my mistakes and how I can improve this...THANKS !!

 

[Spartipilo]

1. Avoid REPEATING functions.
You use several Time (Owner of (Triggering Unit)) and Key(Handle(Triggering Unit)). You must know that Parentheses means slowlyness. You must avoid them as much as you can as long as you use them more than twice in the same trigger. How? Using variables. Set U = (Triggering Unit) and from there on, use U instead of (Triggering Unit). Do the same with all the other Values

• Set UnitVariable = (Triggering Unit)
• Set IntegerVariable = Key(Handle(UnitVariable))

2. When do you turn on the Loo?

3. Don't check for the Units in the group inside the Loop. Do the loop within the check

• If - Conditions
  • Number of Units in Group > 0 then
• Then - Actions
  • Unit Group - Do your actions
• Else - Actions
  • Trigger - Turn off Trigger

4. Same as 1: Replace (Picked Unit) with a Unit variable and use the Unit Variable from there on.

5. There's a trick using a custom script that allows you to store the Handle of a Unit without having to use (Key(Handle)) wich is

• Custom script: set udg_IntegerVariableName = GetHandleId(YourUnit)

It's faster so, better. It also works for anything: Destructibles, Trees, etc. Any object inside the game.

6. Do the changes, test, Re-Post the triggers so we can keep on optimizing. I would go further to easy JASS since Most of the actions you're using are BJ's wich basically means that these triggers are 2 or 3 times heavier/slower than the required JASS script to do the same.

 

[scourge069]

• If - Conditions
• Number of Units in Group > 0 then
• Then - Actions
• Unit Group - Do your actions
• Else - Actions
• Trigger - Turn off Trigger

*about this, is mine in any case not valid ? coz I only followed the steps I found in a tutorial here in hive..and that's exactly how they did it..(just asking sir)

I don't know if I got this right, is this what you are implying sir??

Revised Triggers

Body-link Magic Cast

• Events
  • Unit - A unit Starts the effect of an ability
• Conditions
  • (Ability being cast) Equal to Body-link Magic
• Actions
  • Set BMagic_Caster = (Triggering unit)
  • Set BMagic_Target = (Target unit of ability being cast)
  • Set BMagic_Timer = 20.00
  • Set BMagic_ReturnPercentage = (0.15 + (0.20 x (Real((Level of (Ability being cast) for BMagic_Caster)))))
  • Custom script: set udg_BMagic_Handle = GetHandleId(udg_BMagic_Caster)
  • Hashtable - Save Handle OfBMagic_Target as 0 of BMagic_Handle in BMagic_Hash
  • Hashtable - Save BMagic_Timer as 1 of BMagic_Handle in BMagic_Hash
  • Hashtable - Save BMagic_ReturnPercentage as 2 of BMagic_Handle in BMagic_Hash
  • Unit Group - Add BMagic_Caster to BMagic_CasterGroup
  • Trigger - Turn on Bodylink Magic Interval <gen>
  • Player - Disable Body-link Magic for (Owner of BMagic_Caster)
  • Player - Disable Body-link Magic Book for (Owner of BMagic_Target)
  • Unit - Add Body-link Magic (Caster) to BMagic_Caster
  • Unit - Set level of Body-link Magic (Caster) for BMagic_Caster to (Level of (Ability being cast) for BMagic_Caster)
  • Unit - Add Body-link Magic Book to BMagic_Target
  • Unit - Set level of Body-link Magic (Target) for BMagic_Target to (Level of (Ability being cast) for BMagic_Caster)

Body-link Magic Interval

• Events
  • Time - Every 1.00 seconds of game time
• Conditions
• Actions
  • Unit Group - Pick every unit in BMagic_CasterGroup and do (Actions)
    • Loop - Actions
      • Set BMagic_Caster = (Picked unit)
      • Custom script: set udg_BMagic_Handle = GetHandleId(udg_BMagic_Caster)
      • Set BMagic_Target = (Load 0 of BMagic_Handle in BMagic_Hash)
      • Set BMagic_Timer = (Load 1 of BMagic_Handle from BMagic_Hash)
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • BMagic_Timer Greater than 0.00
        • Then - Actions
          • Hashtable - Save (BMagic_Timer - 1.00) as 1 of BMagic_Handle in BMagic_Hash
        • Else - Actions
          • Unit - Remove Body-link Magic (Caster) from BMagic_Caster
          • Unit - Remove Body-link Magic (Caster) buff from BMagic_Caster
          • Unit - Remove Body-link Magic Book from BMagic_Target
          • Unit - Remove Body-link Magic (Target) buff from BMagic_Target
          • Player - Enable Body-link Magic for (Owner of BMagic_Caster)
          • Hashtable - Clear all child hashtables of child BMagic_Handle in BMagic_Hash
          • Unit Group - Remove BMagic_Caster from BMagic_CasterGroup
          • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
            • If - Conditions
              • (Number of units in BMagic_CasterGroup) Equal to 0
            • Then - Actions
              • Trigger - Turn off (This trigger)
            • Else - Actions

Body-link Magic Damage Event

• Events
  • Game - GDD_Event becomes Equal to 0.00
• Conditions
  • (GDD_DamagedUnit is in BMagic_CasterGroup) Equal to True
  • (GDD_DamageSource belongs to an enemy of (Owner of GDD_DamagedUnit)) Equal to True
• Actions
  • Unit Group - Pick every unit in BMagic_CasterGroup and do (Actions)
    • Loop - Actions
      • Set BMagic_Caster = (Picked unit)
      • Custom script: set udg_BMagic_Handle = GetHandleId(udg_BMagic_Caster)
      • Set BMagic_Target = (Load 0 of BMagic_Handle in BMagic_Hash)
      • Set BMagic_ReturnPercentage = (Load 2 of BMagic_Handle from BMagic_Hash)
      • Set BMagic_DamageDealt = (GDD_Damage x BMagic_ReturnPercentage)
      • Unit - Cause GDD_DamagedUnit to damage BMagic_Target, dealing BMagic_DamageDealt damage of attack type Spells and damage type Magic
      • Set BMagic_DamageDealt = 0.00

 

[Spartipilo]

Yours works, but you're checking that for every unit in the group... If The group isn't empty and you have 100 units in the group, you're making 99 unusefull checks wich requires memory. In my suggestion you only do once, and that's enough.

And Yes, you got it perfectly right You can use Temp Globas tough, or jass locals if you know how to. There's no need for an specific Unit varaible just for this ability.

 

[scourge069]

, oh right, haven't thought of that..revising again..

meaning to say, other than the things you mentioned above, the triggers or may I say the spell itself is acceptable??

 

[Spartipilo]

If you see it working, it's. You could improve it by using Bribe's Unit Indexer to replace using a Hashtable, but it's good enough, considering it's made in GUI.

Anyway, i reaaaaaally suggest you to turn it to easy jass to make it 3x better.

 

[scourge069]

, I would have if only I knew..I'm like 0 knowledge about JASS,

BTW thanks for replying on this thread..

 

[Spartipilo]

Create a copy of these 3 you have. Name the regular ones with GUI prefix, and the other 3 with JASS prefix. Then turn the 3 JASS's to Custom Text (Edit - Convert to Custom Text). Then compare each JASS with each GUI to see what functions do what...

You'll see that there's something called "GetTriggerUnit()" You don't have to be a genius to guess that represents the (Triggering Unit) in the GUI. You'll see that Actions and Conditions take part inside Functions (function JASS_Trig_Func_A9991DC takes nothing returns nothing) same goes with conditions, wich returns true or false.

Then you'll remove the FLESH of the GUI, and see the BONES in JASS, and see the code behind the "pretty" interface GUI provides, but it's really limited.

THEN you do

local unit caster = GetTriggerUnit()
local unit target = GetSpellTarget() // Or something like that
local id = GetHandleId(caster)
// and so on

and then replace all the variables you're using with the locals...

I could o it in a seg, but I want you to learn. This will improve GREATLY all your triggers in many many ways.

 

[scourge069]

, hmmm ...ok I'll try this ..thanks again for the help ..