Lautanen's Spell Pack

[Moderator]

Old Review

19:35, 15th Oct 2010
The_Reborn_Devil:

You need to use a proper in-game screenshot that shows your spells. You also have a lot of "Do Nothing" and you should use Triggering Unit instead of Casting Unit (more efficient). There are also some things you could optimize, for example the trigger "War Stomp". There's no point in running another loop after the first to remove the locations. Just remove them inside the first loop.


Status: Rejected until updated
Rating: N/A

PM me or another mod once you've updated this to get it reviewed again. Have a nice day!

Pharaoh_: Three of these spells are really easy to produce, while one is not even working as intended (since it is under progress). I will not approve of it in its current state, unless they become a bit more complex.

 

[Nightmare Moon]

Gonna check, if I'll use it, I'll give you credits.
Edit: make Bouncing Fire trigger ability, you can make targetable on allies but it will heal them.
You use a lot of

• Do nothing

actions, they aren't needed.
War Stomp ability looks interesting but too simple, i mean you can add some knockback, but to shokwaves.
And you never destroy SC_MotherPoint and SC_Target.
So i could use only Bouncing Fire if you'd change it (i need now only fire spells ).
Anyway spells are simple but usefull, i guess.

Please use Edit button instead of double posting. (i forget to use it sometimes too...)

 

[Lautanen]

Fixed description.

Okay, I saw 2 leaks in triggers and fixed them.
If you see any leaks, please say about that for me.
Thank you!

Gonna check, if I'll use it, I'll give you credits.
Edit: make Bouncing Fire trigger ability, you can make targetable on allies but it will heal them.
You use a lot of

• Do nothing

actions, they aren't needed.
War Stomp ability looks interesting but too simple, i mean you can add some knockback, but to shokwaves.
And you never destroy SC_MotherPoint and SC_Target.
So i could use only Bouncing Fire if you'd change it (i need now only fire spells ).
Anyway spells are simple but usefull, i guess.

Thank you, I´m on it!

Edit: I´m updated it now. Bouncing fire will heal when used on allies, but I like war stomp how it is now, so I will not change it, sorry.

Edit2: Updated the spell pack. Spells are now having better triggering... I think.

Edit3: Fixed the bug with map reviving dummies and moving camera.

 

[Nightmare Moon]

I like War Stomp too, but it's more usefull to add knockback. Try using this http://www.hiveworkshop.com/forums/spells-569/advanced-shockwave-paladon-documentation-110588/?prev=search%3Dknockback%26d%3Dlist%26r%3D20 system made by Paladon.
I think with knockback more users will use it, i mean just stomp and shokwaves to simple.

Edit: But if you don't want to do that, please let me edit it in my map (i'll use it), +rep.

 

[Lautanen]

I like War Stomp too, but it's more usefull to add knockback. Try using this http://www.hiveworkshop.com/forums/spells-569/advanced-shockwave-paladon-documentation-110588/?prev=search%3Dknockback%26d%3Dlist%26r%3D20 system made by Paladon.
I think with knockback more users will use it, i mean just stomp and shokwaves to simple.

Edit: But if you don't want to do that, please let me edit it in my map (i'll use it), +rep.

Knockback added, thank you for idea.

Edit: Shock Stomp can now have as many levels as you want on version 1.5a!

 

[Marcin013]

Not bad spells
Vote:3/5

 

[Noobsmoke]

Lautanen please don't double post, use EDIT button

 

[Laiev]

you should use ingame screenshot not icons

and you should avoid imports when submit a resource

 

[Lautanen]

Lautanen please don't double post, use EDIT button

I´m trying to remeber that.

you should use ingame screenshot not icons

Read the description.

Thank you for commenting!

 

[Titania]

Improve your description on Fade..

 

[Maker]

All the periodic triggers shouldbe turned on/off so they're not running all the time.
The spells are not easily configurable.
You could use the same variables for all spells, instead of having F_Point, BF_Point etc. you could just use one variable.
Create the hashtable in separate trigger (map initialization or some other), no sence to check if it's created every time you use the ability.

Sorm circle should use loop, it only supports 3 levels.

Trigger

• Storm Circle
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Storm Circle
  • Actions
    • -------- Initialize --------
    • Set SC_Target = (Target unit of ability being cast)
    • Set SC_MotherPoint = (Position of SC_Target)
    • Set SC_Caster = (Casting unit)
    • Set SC_Player = (Owner of SC_Caster)
    • Set i1 = (Level of Storm Circle for SC_Caster) * 3
    • For each (Integer loopA) from 1 to i1, do (Actions)
      • Loop - Actions
        • Set SC_Point[udg_loopA] = (SC_MotherPoint offset by 256.00 towards (((Real(loopA)) - 1.00) x (360.00 / (Real(i1)))) degrees)
    • -------- Run Do Trigger --------
    • Trigger - Run Storm Circle Do <gen> (ignoring conditions)

You have unneeded call RemoveLoactions there, some of them don't need to get called if the ability level is low.
Make Bouncing Fire support unlimited amount of levels, not just 3.
Use triggering unit, not casting unit.

 

[Lautanen]

You have unneeded call RemoveLoactions there, some of them don't need to get called if the ability level is low.

I will fix that!

Make Bouncing Fire support unlimited amount of levels, not just 3.

Well, I designed them to be only 3 leveled spells...

Use triggering unit, not casting unit.

Why?

I see that you are very advanced in triggers, thank you for commenting!

 

[Maker]

Well, I designed them to be only 3 leveled spells...

What if someone wants the spell to have 5 levels? He would have to edit the triggers. All spells should be designed for unlimited amount of levels.

Many event responses return triggering unit. Casting unit, dying unit and so on. Using triggering unit is slightly faster.

 

[Lautanen]

What if someone wants the spell to have 5 levels? He would have to edit the triggers. All spells should be designed for unlimited amount of levels.

Many event responses return triggering unit. Casting unit, dying unit and so on. Using triggering unit is slightly faster.

Okay, I didn´t know that. Thank you for commenting!

 

[Noobsmoke]

Originally Posted by Maker
Use triggering unit, not casting unit.

What's wrong using casting unit ?

 

[dardas]

nothing is wrong about it, its just slightly faster to use triggering unit,

actually.. i just looked in the jass code in newgen editor, this is how CastingUnit looks like.
(or, GetSpellAbilityUnit())

constant native GetSpellAbilityUnit takes nothing returns unit

its a constant.. i dont see anything wrong about it o.o

 

[Laiev]

You need to use a proper in-game screenshot that shows your spells.

you should use ingame screenshot not icons

and you should avoid imports when submit a resource

I said to you

 

[Golden-Drake]

yay i fine exaple of some one using my icons thx
+Rep

and btw Spell pack -> very good 4/5

 

[Magtheridon96]

its a constant.. i dont see anything wrong about it o.o

Absolutely no difference.

GetTriggerUnit and GetSpellAbilityUnit are exactly the same inside Warcraft
(educated guess ), but GetTriggerUnit is faster by about 1.28 times.
I got that number by taking into account name lengths and hashes per
4 characters ;D (According to Dr Super Good, who spent hours with an
assembler reading machine code)

• Set BF_Target = No unit
• Set BF_Dummy = No unit
• Set BF_Caster = No unit

You don't need to null global variables. They're not like local ones.
Global variables are static and can only be overwritten.
They are not automatic like local variables (They aren't created,
used and destroyed at the end of a function/trigger/whatever.)

 

[Lautanen]

• Set BF_Target = No unit
• Set BF_Dummy = No unit
• Set BF_Caster = No unit

You don't need to null global variables. They're not like local ones.
Global variables are static and can only be overwritten.
They are not automatic like local variables (They aren't created,
used and destroyed at the end of a function/trigger/whatever.)

Fixed

 

[Bribe]

Only count the number of units in the group after removing a unit from the group. It's a logical improvement.

When you reference variables by actual index Point[1] Point[2] just divide them into multiple variables. It's a readability improvement.

From the CPU's perspective both work out to be performance improvements.

 

[Lautanen]

Only count the number of units in the group after removing a unit from the group. It's a logical improvement.

Done

When you reference variables by actual index Point[1] Point[2] just divide them into multiple variables. It's a readability improvement.

What is the spell ur talking 'bout and do you mean to coordinates x and y?
GetLocationX()
GetLocationY()

 

[Bribe]

No no I mean instead of doing

Set Point[1] = (Position of (Casting unit))
Set Point[2] = (Position of (Target unit of ability being cast))

Do:

Set CenterPoint = (Target point of ability being cast)
Set TargetPoint = (Position of (Target unit of ability being cast))

 

[Lautanen]

No no I mean instead of doing

Set Point[1] = (Position of (Casting unit))
Set Point[2] = (Position of (Target unit of ability being cast))

Do:

Set CenterPoint = (Target point of ability being cast)
Set TargetPoint = (Position of (Target unit of ability being cast))

The dummies wont do anything now.
Could you explain that? The old trigger worked well.

Trigger

• Storm Circle
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Storm Circle
  • Actions
    • -------- Initialize --------
    • Set SC_Target = (Target unit of ability being cast)
    • Set SC_MotherPoint = (Position of SC_Target)
    • Set SC_Caster = (Triggering unit)
    • Set SC_Player = (Owner of SC_Caster)
    • Set SC_Points = ((Level of Storm Circle for SC_Caster) x 3)
    • Set SC_AngleIncrement = (360.00 / (Real(SC_Points)))
    • -------- Action --------
    • For each (Integer TempInt) from 1 to SC_Points, do (Actions)
      • Loop - Actions
        • Set TempReal = (SC_AngleIncrement x (Real(TempInt)))
        • Set SC_CastPoint = (SC_MotherPoint offset by 250.00 towards TempReal degrees)
        • Unit - Create 1 Dummy for SC_Player at WS_CastPoint facing Default building facing degrees
        • Set TempUnit = (Last created unit)
        • Unit - Order TempUnit to Human Mountain King - Storm Bolt SC_Target
        • Custom script: call RemoveLocation(udg_SC_CastPoint)
        • Unit - Add a 1.00 second Generic expiration timer to TempUnit
    • Custom script: call RemoveLocation(udg_SC_MotherPoint)

Edit: Used WS_CastPoint in accident, lol. Spellpack is updated.

 

[mateuspv]

Someone correct me if I'm wrong.
-Holy Beam

• Set HB_Dummy = (Last created unit)
• Unit Group - Add HB_Dummy to HB_Group
• Unit - Add a 2.00 second Generic expiration timer to HB_Dummy

It is better to use "Last created unit" ?:

• Unit Group - Add (Last created unit) to HB_Group
• Unit - Add a 2.00 second Generic expiration timer to (Last created unit)

I do not know if I'm right

 

[Pharaoh_]

When you refer to a certain event response or action response more than once, you are advised to use a variable, in order to save function calls.