Optimizing Spell

[GIMLI_2]

Hi guys, i made a simple spell with the help of hashtables (A missle that flies to the enemy and dealing damage). what i want to know, can i do anything better or more efficient, are there any leaks, did i something wrong? What i can say: the spell works:

Triggers

• Hydro Pump 1
  • Events
    • Unit - A unit starts the effect of an ability
  • Conditions
    • (Ability being cast) equal to Hydro Pump
  • Actions
    • Set Hydropump_Caster[1] = (Casting unit)
    • Set Hydropump_Target[1] = (Target unit of ability being cast)
    • Set Hydropump_Point[1] = (Position of Hydropump_Caster[1])
    • Set Hydropump_Point[2] = (Hydropump_Point[1] offset by 25.00 towards (Facing of Hydropump_Caster[1]) degrees)
    • Unit - Create 1 Hydro Pump Dummy for (Owner of Hydropump_Caster[1]) at Hydropump_Point[2] facing (Facing of Egelsamen_Einheit) degrees
    • Set Hydropump_Dummy = (Last created unit)
    • Set Hydropump_Caster[2] = (Load 4 of (Key (Last created unit)) in Hashtabelle)
    • Set Hydropump_Caster[2] = Hydropump_Caster[1]
    • Hashtabelle - Save Handle OfHydropump_Caster[2] as 4 of (Key (Last created unit)) in Hashtabelle
    • Set Hydropump_Target[2] = (Load 9 of (Key (Last created unit)) in Hashtabelle)
    • Set Hydropump_Target[2] = Hydropump_Target[1]
    • Hashtabelle - Save Handle OfHydropump_Target[2] as 9 of (Key (Last created unit)) in Hashtabelle
    • Unit Group - Add Hydropump_Dummy to Hydropump_Group
    • Trigger - Turn on Hydro Pump 2 <gen>
    • Custom script: call RemoveLocation (udg_Hydropump_Point[1])
    • Custom script: call RemoveLocation (udg_Hydropump_Point[2])

• Hydro Pump 2
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in Hydropump_Group and do (Actions)
      • Loop - Action
        • Set Hydropump_Caster[2] = (Load 4 of (Key (Picked unit)) in Hashtabelle)
        • Set Hydropump_Target[2] = (Load 9 of (Key (Picked unit)) in Hashtabelle)
        • Set Hydropump_Point[3] = (Position of Hydropump_Target[2])
        • Set Hydropump_Point[4] = (Position of (Picked unit))
        • Unit - Move (Picked unit) instantly to (Hydropump_Point[4] offset by 25.00 towards (Angle from Hydropump_Point[4] to Hydropump_Point[3]) degrees)
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • 'IF'-Conditions
            • (Distance between Hydropump_Point[4] and Hydropump_Point[3]) smaller than 20.00
          • 'THEN'-Actions
            • Unit - Kill (Picked unit)
            • Unit - Cause Hydropump_Caster[2] to damage Hydropump_Target[2], dealing (1.25 x (Real((Strength of Hydropump_Caster[2] (Including bonuses))))) damage of attack type Spells and damage type Normal
            • Unit Group - Remove (Picked unit) from Hydropump_Group
          • 'ELSE'-Actions
        • Custom script: call RemoveLocation (udg_Hydropump_Point[3])
        • Custom script: call RemoveLocation (udg_Hydropump_Point[4])
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • 'IF'-Conditions
        • (Number of units in Hydropump_Group) equal to 0
      • 'THEN'-Actions
        • Trigger - Turn off (This trigger)
      • 'ELSE'-Actions

 

[GIMLI_2]

... ty for helping

 

[Spartipilo]

So, you wanna know if it's ok...

Improvement points.
1- You're using unneeded Global variables. Unless you're keeping track of something with a Global Variable, a simple "Temp" variable works for X amount of spells.
2- You're using unneeded global arrays. Be aware than arrays take much more space. Even if you only use some index, the size remains being 8192.
3- "Unit - Move (Picked unit) instantly to (Hydropump_Point[4] offset by 25.00 towards (Angle from Hydropump_Point[4] to Hydropump_Point[3]) degrees)" leaks. Points with offesets uses a base point, and creates the offset point, so, you have to declare both in the variable and remove both later.
4- Insert all the "Pick Every unit" actions in the periodic trigger in the "Else" of the If/Then/Else you have in the end.
5- Don't use "Casting Unit" if you can use "Triggering Unit". It's better and faster for the system.
6. Avoid repetitive callings like "Key(last created unit)". You already declared the last created unit into a variable, use that variable. If you're going to use several times the Key of that unit, use an integer variable and Custom script: set udg_*IntegerVariable* = GetHandleId(*YourUnit*)

If you wanna improve it even more...
1- Convert this to custom text and get rid of all the BJ's by manually doing all the functions.
2- Use locals, instead of global arrays
3- Use Reals, instead of points

Also, whats the use for this? You give a value to Hydropump_Caster[2], then you replace it with Hydropump_Caster[1]. And repeat the same thing with Hydropump_Target[2].

• Trigger
  • Set Hydropump_Caster[2] = (Load 4 of (Key (Last created unit)) in Hashtabelle)
  • Set Hydropump_Caster[2] = Hydropump_Caster[1]
  • Hashtabelle - Save Handle OfHydropump_Caster[2] as 4 of (Key (Last created unit)) in Hashtabelle
  • Set Hydropump_Target[2] = (Load 9 of (Key (Last created unit)) in Hashtabelle)
  • Set Hydropump_Target[2] = Hydropump_Target[1]
  • Hashtabelle - Save Handle OfHydropump_Target[2] as 9 of (Key (Last created unit)) in Hashtabelle

You also forgot to check in the periodic trigger if the target of the missile is alive.

And avoid Double Posting. There's no need to post twice in a row. You can use "Edit" button. Double Posting is against Hive Rules.

Feel free to ask anything you want if you have any doubt or want any further help.

 

[GIMLI_2]

first thing: TY
then i have a couple of questions, im a bit confused^^:
-I cant do local variables without jass right?
-What are the unneeded variables and arrays?

-

• Set Hydropump_Caster[2] = (Load 4 of (Key (Last created unit)) in Hashtabelle)
• Set Hydropump_Caster[2] = Hydropump_Caster[1]
• Hashtabelle - Save Handle OfHydropump_Caster[2] as 4 of (Key (Last created unit)) in Hashtabelle
• Set Hydropump_Target[2] = (Load 9 of (Key (Last created unit)) in Hashtabelle)
• Set Hydropump_Target[2] = Hydropump_Target[1]
• Hashtabelle - Save Handle OfHydropump_Target[2] as 9 of (Key (Last created unit)) in Hashtabelle

first thing about that: i wanted to change the "last created unit" to my variable, but i couldnt?? there was no selection for variables.
second thing, u said why i did Hydropump_Caster[1] to Hydropump_Caster[2]
because i dont know how to do it else. should i do it like that?

• Set Hydropump_Caster[1] = (Load 4 of (Key (Last created unit)) in Hashtabelle)
• Set Hydropump_Caster[1] = triggering unit
• Hashtabelle - Save Handle OfHydropump_Caster[2] as 4 of (Key (Last created unit)) in Hashtabelle
• Set Hydropump_Target[1] = (Load 9 of (Key (Last created unit)) in Hashtabelle)
• Set Hydropump_Target[1] = targeted unit
• Hashtabelle - Save Handle OfHydropump_Target[1] as 9 of (Key (Last created unit)) in Hashtabelle

it would be nice if you would show me the triggers improved.

TY AGAIN (btw, how can you give another person +rep? i dont know that )

 

[edo494]

And avoid Double Posting. There's no need to post twice in a row. You can use "Edit" button. Double Posting is against Hive Rules.

Isnt it within 48 hours? And if he wanted to get attention he had to bump the topic and edit doesnt bump threads so there is a need to double post sometimes.

GLIMPI you can do local variables "without" Jass.
you can also manipulate locals without using custom scripts, just create global variable which has the same name as local and them work with it

• Actions
  • Custom Script: local integer i=0
  • Set i = i + 1
  • Game - Display string(i) to Player 1 (Red) or w/e it is in gui

the i is global variable made in variable editor
another example:

• Events
  • Time - Every 2.00 seconds of game time
• Actions
  • Custom Script: local integer i
  • Set i = i+1
  • Game - Display string(i) to Player 1 (Red)...
  • Wait 3.00 seconds
  • Set i = i+1
  • Game - Display string(i) to Player 1 (Red)...

will display 1, 1, 2, 1, 2 and so on while if you used global without local one it would go 1, 2, 3, 4, 5...

the reputaion you can add, just check the post and under avatar are those "globes" and next to them(to the left) is √X.

 

[Spartipilo]

We save the damage and the triggering unit in te Dummy too.

• Hydro Pump 1
  • Events
    • Unit - A unit starts the effect of an ability
  • Conditions
    • (Ability being cast) equal to Hydro Pump
  • Actions
    • Set Unit1 = (Triggering Unit)
    • Set Unit2 = (Target unit of ability being cast)
    • Set Point1 = (Position of Unit1)
    • Set Point2 = (Point1 offset by 25 towards (Facing of Unit1) degrees)
    • Unit - Create 1 Hydro Pump Dummy for (Triggering Player) at Point2 facing (Facing of Engelsamen_Einheit) degrees
    • Set Unit3 = (Last created unit)
    • Custom script: set udg_i1 = GetHandleId(udg_Unit3)
    • Custom script: call SaveUnitHandle(udg_Hashtable, udg_i1, 4, udg_Unit1)
    • Custom script: call SaveUnitHandle(udg_Hashtable, udg_i1, 9, udg_Unit2)
    • Hashtable - Save (1.25 x (Real((Strenght of Unit1 (including bonuses))))) as 10 of i1 in Hashtable
    • Unit Group - Add Unit3 to Hydropump_Group
    • Trigger - Turn on Hydro Pump 2 <gen>
    • Custom script: call RemoveLocation (udg_Point1)
    • Custom script: call RemoveLocation (udg_Point2)

• Hydro Pump 2
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • 'IF'-Conditions
        • (Number of units in Hydropump_Group) Greater than 0
      • 'THEN'-Actions
        • Unit Group - Pick every unit in Hydropump_Group and do (Actions)
          • Loop - Action
            • Set Unit1 = (Picked Unit)
            • Custom script: set udg_i1 = GetHandleId(udg_Unit1)
            • Set Unit2 = (Load 9 i1 in Hash)
            • Set Point1 = (Position of Unit1)
            • Set Point2 = (Position of Unit2)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • 'IF'-Conditions
                • (Distance Point1 and Point2) smaller than 25.00
              • 'THEN'-Actions
                • Set Unit3 = (Load 4 of i1 in Hash)
                • Unit - Cause Unit3 to damage Unit2, dealing (Load 10 of i1 in Hash) damage of attack type Spells and damage type Normal
                • Hashtable - Clear all child of i1 in Hash
                • Unit - Kill Unit1
                • Unit Group - Remove Unit1 from Hydropump_Group
              • 'ELSE'-Actions
                • Set Point3 = (Point1 offset by 25 towards (Angle from Point1 to Point2) degrees)
                • Unit - Move Unit1 instantly to Point3
                • Custom script: call RemoveLocation (Point3)
              • Custom script: call RemoveLocation (Point2)
              • Custom script: call RemoveLocation (Point1)
      • 'ELSE'-Actions
        • Trigger - Turn off (This trigger)

You still have to add the "If unit is alive" condition to move the missile.

Try it, try to understand it too, understand how it works, and the logic between all these lines of triggering. Then, if you want, I can help you turning it to jass, use locals, remove bj's, etc.

 

[GIMLI_2]

ty rly, that helped me +rep

 

[Spartipilo]

Does it work? I made it here in Hive, didn't test xD

 

[GIMLI_2]

dont know ill test it tomorrow

 

[GIMLI_2]

doesnt work :/ the water globe always flies to center of playable map area.
found 1 mistake:

• Set Point3 = (Point2 offset by 25 towards (Angle from Point1 to Point2) degrees)

the Point2 is wrong, i setted there Point1

 

[Spartipilo]

You're right. [Updated my main trigger in the last post] Does it work as intented now?

 

[GIMLI_2]

u only changed what i said to you?
cannot see sth else. cause now it doesnt work
could it be that it doesnt work cause i used Point[1], Point[2], Point[3] instead of Point1 , Point2, Point3
I dont think so but maybe

 

[Spartipilo]

Nah, it should work (you should use the 3 variable points instead of a Point array tough)

It should work. Be sure to be typing everything ok, and be using all the correct variables. It's a bit messy, specially on the second trigger. Post the trigger you have so I can give a look at it.

 

[GIMLI_2]

i rly copied nearly everything now and it still doesnt work, the water globe still moves to center of playable map area and dies there

Trigger

• Hydro Pump 1
  • Events
    • Unit - A unit starts the effect of an ability
  • Conditions
    • (Ability being cast) equal to Hydro Pump
  • Actions
    • Set Unit1 = (Triggering unit)
    • Set Unit2 = (Target unit of ability being cast)
    • Set Point1 = (Position of Unit1)
    • Set Point2 = (Point1 offset by 25.00 towards (Facing of Unit1) degrees)
    • Unit - Create 1 Hydro Pump Dummy for (Triggering player) at Point2 facing (Facing of Unit1) degrees
    • Set Unit3 = (Last created unit)
    • Custom script: set udg_i1 = GetHandleId(udg_Unit1)
    • Custom script: set udg_i2 = GetHandleId(udg_Unit2)
    • Custom script: set udg_i3 = GetHandleId(udg_Unit3)
    • Hashtable - Save i1 as 4 of i3 in Hashtable
    • Hashtable - Save i2 as 9 of i3 in Hashtable
    • Hashtable - Save (1.25 x (Real((Strenght of Earthquake_Caster (including bonuses))))) as 10 of i3 in Hashtable
    • Unit Group - Add Unit3 to Hydropump_Group
    • Trigger - Turn on Hydro Pump 2 <gen>
    • Custom script: call RemoveLocation (udg_Point1)
    • Custom script: call RemoveLocation (udg_Point2)

• Hydro Pump 2
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • 'IF'-Conditions
        • (Number of units in Hydropump_Group) Greater than 0
      • 'THEN'-Actions
        • Unit Group - Pick every unit in Hydropump_Group and do (Actions)
          • Loop - Actions
            • Set Unit1 = (Picked unit)
            • Custom script: set udg_i1 = GetHandleId(udg_Unit1)
            • Set Unit2 = (Load 9 of i1 in Hashtable)
            • Set Point1 = (Position of Unit1)
            • Set Point2 = (Position of Unit2)
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • 'IF'-Conditions
                • (Distance between Point1 and Point2) smaller than 20.00
              • 'THEN'-Actions
                • Set Unit3 = (Load 4 of i1 in Hashtable)
                • Unit - Cause Unit3 to damage Unit2, dealing (Load 10 of i1 from Hashtable) damage of attack type Spells and damage type Normal
                • Hashtable - Clear all child hashtables of child i3 in Hashtable
                • Unit - Kill Unit1
                • Unit Group - Remove Unit1 from Hydropump_Group
              • 'ELSE'-Actions
                • Set Point3 = (Point1 offset by 25.00 towards (Angle from Point1 to Point2) degrees)
                • Unit - Move Unit1 instantly to Point3
                • Custom script: call RemoveLocation (udg_Point3)
            • Custom script: call RemoveLocation (udg_Point2)
            • Custom script: call RemoveLocation (udg_Point1)
      • 'ELSE'-Actions
        • Trigger - Turn off (This trigger)

 

[Spartipilo]

" Hashtable - Save (1.25 x (Real((Strenght of Earthquake_Caster (including bonuses))))) as 10 of i3 in Hashtable". There's no "Earthquake_Caster". That should be "Unit1", right?

When this happens (the dummy moves to the center of the map) it's because there's a point missing. Points are made by x/y coordenates, and when the point isn't there, x/y are equal to 0, which refers to the center of the map. So, the problem remains in a point. Now, the reason for the Point being null, it's probably because the Unit (which position we're looking for) is null too.

Sometimes (I don't know why) working with units ID's as integers doesn't work, but working with Unit Handles (the same integer) does work, so... lets try that.

Replace this:

• Custom script: set udg_i1 = GetHandleId(udg_Unit1)
• Custom script: set udg_i2 = GetHandleId(udg_Unit2)
• Custom script: set udg_i3 = GetHandleId(udg_Unit3)
• Hashtable - Save i1 as 4 of i3 in Hashtable
• Hashtable - Save i2 as 9 of i3 in Hashtable
• Hashtable - Save (1.25 x (Real((Strenght of Earthquake_Caster (including bonuses))))) as 10 of i3 in Hashtable

With this:

• Custom script: set udg_i1 = GetHandleId(udg_Unit3)
• Custom script: call SaveUnitHandle(udg_Hashtable, udg_i1, 4, udg_Unit1)
• Custom script: call SaveUnitHandle(udg_Hashtable, udg_i1, 9, udg_Unit2)
• Hashtable - Save (1.25 x (Real((Strenght of Unit1 (including bonuses))))) as 10 of i1 in Hashtable

The rest should remain the same. Seems like there's no need for i2 and i3 in this trigger.

Also replace this

• Hashtable - Clear all child hashtables of child i3 in Hashtable

With this:

• Hashtable - Clear all child hashtables of child i1 in Hashtable

This was a mistake i made in my trigger post.

>> Edited my main trigger post to add these changes <<

 

[GIMLI_2]

IT WORKS ! ! ! THANK YOU
i rly dont want to go on your nerves^^ but i have some last questions.
First thing with that arrays: instead of Unit1, Unit2 ---> Unit[1], Unit[2], is that bad?, and why are the 2 variables better?
Second thing is:

• Solar Beam 1
  • Events
    • Unit - A unit starts the effect of an ability
  • Conditions
    • (Ability being cast) equal to Solar Beam
  • Actions
    • Unit - Cause (Triggering unit) to damage (Target unit of ability being cast), dealing (1.50 x (Real((Strenght of (Triggering unit) (include bonuses))))) damage of attack type Spells and damage type Normal

• Solar Beam 1
  • Events
    • Unit - A unit starts the effect of an ability
  • Conditions
    • (Ability being cast) equal to Solar Beam
  • Actions
    • Set Caster = (Triggering Unit)
    • Unit - Cause Caster to damage (Target unit of ability being cast), dealing (1.50 x (Real((Strenght of Caster (include bonuses))))) damage of attack type Spells and damage type Normal

the second one is more efficient right?
cause you call the triggering unit 1 time instead of 2 times?

third thing is: the "if unit is dead" function, ill try it out myself, and if i get problems, i have to ask you again^^

ty again, i have needed that for many spells, you will find ur name in the credits of my map!

 

[Spartipilo]

You have to reduce parentheses and variables as much as you can using variables (as long as using them actually reduces the amount of data requirements)

Example:

• T
• Event
  • Unit - A unit dies
• Conditions
• Actions
  • Unit - Move (Triggering Unit) to (Center of (Playable Map Area))
  • Unit - Revive (Triggering Unit)
  • Unit - Make (Triggering Unit) Invulnerable
  • Unit - Pause (Triggering Unit)
  • Unit - Set Life of (Triggering unit) equal to ((Life of (Triggering Unit)) + (Life of (Triggering Unit) * 2))

As you can see, I'm using a lot of (Triggering Unit), and the system looks for it everytime. We've around 12 pairs of parentheses. When you use a variable, instead, the system already knows the unit it has to work with, since it's stored in the variable. It's like trying to remember a phone number, if you need one several times, you write it in your hand or paper, instead of look for it in your phone contact list over and over.

• T
• Event
  • Unit - A unit dies
• Conditions
• Actions
  • Set u = (Triggering Unit)
  • Set l = (Current life of u)
  • Unit - Move u to (Center of (Playable Map Area)
  • Unit - Revive u
  • Unit - Make u Invulnerable
  • Unit - Pause u
  • Unit - Set Life of u equal to (l + (l * 2))

Now we have 4 pair of parentheses, and two of them are just math.

Now, if you do as you said, use 1 variable when you're just going to use the data once or twice, is not worth it.

Arrays size are 8192, that's a lot higher than 1, 2, or 3. That's why arrays takes more space. If you're not really using the index for something, or using several of those (like unit indexer, or item lists, or unit types lists, etc. which uses hundreds of indexs) then it's better to use separated variables of the same type.

EDIT: Sure, ask as much as you want. I'll be glad to help. If you succeed. post your final trigger here so we can give a look at it and try to optimize it evern further (with locals and other stuff)

 

[GIMLI_2]

ty for this clear and useful description , i used array in nearly every trigger, i didnt know that it has more filesize, i have to fix that ^^

i think ill post the final trigger tomorrow