[Manasurge] Whirlpool v1.0.5

Moderator · Sep 30, 2013

Whirlpool v1.0.3 | Reviewed by Maker | 11th Oct 2013

APPROVED

	Cool spell, very nice effects and the sounds match too
	Only the first parameter in setting a scale of a unit matters, the rest can be 0 Don't load everything in the looping trigger if the spell ends Use one item, don't create new ones Items get blocked by items, but units are not blocked by items. The pathing check can fail. There is a system in Spells You should use only one dummy to cast the ability on all targets

[tr]

Losam · Sep 30, 2013

Hmmm ... looks like a good job. Once you have time, I will test.
But, from what I can suggest is the following:

Actions
- Hashtable - Create a hashtable
- Set Hash = (Last created hashtable)

Instead of using two command lines to declare a hashtable, you can use:

Custom script: set udg_Hash = InitHashtable()

I use this line in my recent work and are approved.
That way, you save online, after this command I gave you is equivalent to two lines.If you have any questions, let me know. Do whatever possible to help.

:fp:

deathismyfriend · Oct 1, 2013

@Losam
That is not necessary as it is GUI although it is better to use the custom script as it is more efficient.

@TO
This does not accurately show if a unit is dead.

((Picked unit) is dead) Equal to True

If a unit is removed from game it shows as if it is alive. There is also some other cases.
You should use an or condition with
unit type of unit not equal to 0
((Picked unit) is dead) Equal to True

All of these variables should simply be temp variables.

Set WP_Key_PickedUnit = (Picked unit)
Set WP_Key = (Key WP_Key_PickedUnit)
Set WP_Caster = (Load 1 of WP_Key in WP_Hashtable)
Set WP_Damage_Total = (Load 2 of WP_Key from WP_Hashtable)
Set WP_Range_Total = (Load 3 of WP_Key from WP_Hashtable)
Set WP_Point_Whirlpool = (Load 4 of WP_Key in WP_Hashtable)
Set WP_Int_Counter = (Load 5 of WP_Key from WP_Hashtable)
Set WP_Int_Counter = (WP_Int_Counter + 1)
Hashtable - Save WP_Int_Counter as 5 of WP_Key in WP_Hashtable
Set WP_Pull_Intensity_Total = (Load 6 of WP_Key from WP_Hashtable)
Set WP_Int_Counter_2 = (Load 7 of WP_Key from WP_Hashtable)
Set WP_Int_Counter_2 = (WP_Int_Counter_2 + 1)

All variables that are only temporary should use temp variables. This way you don't create unnecessary variables.

axleion · Oct 1, 2013

Set WP_Damage_Base = 3.00
Set WP_Damage_Bonus = 2.00
Set WP_Damage_Scaling = 0.25

since you already add multilevel support to all parameter, add multilevel support to the WP_Damage_Scaling.

Special Effect - Create a special effect at WP_Point_Whirlpool using Abilities\Spells\Other\CrushingWave\CrushingWaveDamage.mdl

better store that sfx string in variable at init trigger so user can change that easily.

Unit - Order WP_Dummy_Loop to Human Sorceress - Slow WP_PickedUnit

store that orderstring into variable at init in case need to change dummy ability.

Rheiko · Oct 1, 2013

^{| Rheiko | 01/10/2013}

^{Rheiko's Review:}

Cons:

((Picked unit) is dead) Equal to True

Overall: [4/5]
No Comments

Suggestions:

Use WP_Key_PickedUnit instead of Picked unit
Store your special effect at your init trigger, so user can easily configure it
Store anything that you use twice or more to a variable for efficiency

Manasurge · Oct 1, 2013

deathismyfriend said:
@Losam
That is not necessary as it is GUI although it is better to use the custom script as it is more efficient.

@TO
This does not accurately show if a unit is dead.

((Picked unit) is dead) Equal to True

If a unit is removed from game it shows as if it is alive. There is also some other cases.
You should use an or condition with
unit type of unit not equal to 0
((Picked unit) is dead) Equal to True

All of these variables should simply be temp variables.

Set WP_Key_PickedUnit = (Picked unit)

Set WP_Key = (Key WP_Key_PickedUnit)

Set WP_Caster = (Load 1 of WP_Key in WP_Hashtable)

Set WP_Damage_Total = (Load 2 of WP_Key from WP_Hashtable)

Set WP_Range_Total = (Load 3 of WP_Key from WP_Hashtable)

Set WP_Point_Whirlpool = (Load 4 of WP_Key in WP_Hashtable)

Set WP_Int_Counter = (Load 5 of WP_Key from WP_Hashtable)

Set WP_Int_Counter = (WP_Int_Counter + 1)

Hashtable - Save WP_Int_Counter as 5 of WP_Key in WP_Hashtable

Set WP_Pull_Intensity_Total = (Load 6 of WP_Key from WP_Hashtable)

Set WP_Int_Counter_2 = (Load 7 of WP_Key from WP_Hashtable)

Set WP_Int_Counter_2 = (WP_Int_Counter_2 + 1)

All variables that are only temporary should use temp variables. This way you don't create unnecessary variables.

I had no idea about the checker picking units removed from the game xD
I'll fix next update.

Also, what do you mean by temp variables? I'm not sure I understand.

Dark_Axl said:
Set WP_Damage_Base = 3.00

Set WP_Damage_Bonus = 2.00

Set WP_Damage_Scaling = 0.25

since you already add multilevel support to all parameter, add multilevel support to the WP_Damage_Scaling.

Special Effect - Create a special effect at WP_Point_Whirlpool using Abilities\Spells\Other\CrushingWave\CrushingWaveDamage.mdl

better store that sfx string in variable at init trigger so user can change that easily.

Unit - Order WP_Dummy_Loop to Human Sorceress - Slow WP_PickedUnit

store that orderstring into variable at init in case need to change dummy ability.

Will do, the more customization the better. Well up to a certain point where it's just a blank trigger.

Rheiko said:
^{| Rheiko | 01/10/2013}

^{Rheiko's Review:}

Cons:

((Picked unit) is dead) Equal to True

Overall: [4/5]
No Comments

Suggestions:

Use WP_Key_PickedUnit instead of Picked unit

Store your special effect at your init trigger, so user can easily configure it

Store anything that you use twice or more to a variable for efficiency

I'll be fixing that condition for the next update.

- "Use WP_Key_PickedUnit instead of Picked unit". Not sure if that works if I'm thinking what I think you mean. Could try it.

- Will do.

- Will do.

Update v1.0.2 coming later today. :ogre_haosis:

Rheiko · Oct 1, 2013

temp means temporary

Manasurge · Oct 1, 2013

Rheiko said:
temp means temporary

I know that temp = temporary but I don't know what he means in this specific case.

deathismyfriend · Oct 1, 2013

Any variable only used for the length of a function with no waits is a temp variable.

Simply change WB_PickedUnit to tempPickedUnit or tempUnit. This way if someone has those variables which it is very likely that they do it will save them from creating extra unnecessary variables.

Manasurge · Oct 2, 2013

deathismyfriend said:
@Losam
That is not necessary as it is GUI although it is better to use the custom script as it is more efficient.

@TO
This does not accurately show if a unit is dead.

((Picked unit) is dead) Equal to True

If a unit is removed from game it shows as if it is alive. There is also some other cases.
You should use an or condition with
unit type of unit not equal to 0
((Picked unit) is dead) Equal to True

All of these variables should simply be temp variables.

Set WP_Key_PickedUnit = (Picked unit)

Set WP_Key = (Key WP_Key_PickedUnit)

Set WP_Caster = (Load 1 of WP_Key in WP_Hashtable)

Set WP_Damage_Total = (Load 2 of WP_Key from WP_Hashtable)

Set WP_Range_Total = (Load 3 of WP_Key from WP_Hashtable)

Set WP_Point_Whirlpool = (Load 4 of WP_Key in WP_Hashtable)

Set WP_Int_Counter = (Load 5 of WP_Key from WP_Hashtable)

Set WP_Int_Counter = (WP_Int_Counter + 1)

Hashtable - Save WP_Int_Counter as 5 of WP_Key in WP_Hashtable

Set WP_Pull_Intensity_Total = (Load 6 of WP_Key from WP_Hashtable)

Set WP_Int_Counter_2 = (Load 7 of WP_Key from WP_Hashtable)

Set WP_Int_Counter_2 = (WP_Int_Counter_2 + 1)

All variables that are only temporary should use temp variables. This way you don't create unnecessary variables.

deathismyfriend said:
Any variable only used for the length of a function with no waits is a temp variable.

Simply change WB_PickedUnit to tempPickedUnit or tempUnit. This way if someone has those variables which it is very likely that they do it will save them from creating extra unnecessary variables.

Fixed all of that, I think.
If what you mean by the unit checker is this:

Or - Any (Conditions) are true
- Conditions
  - ((Picked unit) is dead) Equal to True
  - (Unit-type of (Picked unit)) Equal to No unit-type

I found no option with 0. Although I saw it in someone elses jass code. Might be it doesn't exist in gui.

Rheiko · Oct 3, 2013

more suggestion
make the condition configurable at init trigger

Manasurge · Oct 3, 2013

Rheiko said:
more suggestion
make the condition configurable at init trigger

Nah. I've seen your spell and the condition config isn't working right. I couldn't figure out a easy way to do it and so it's not worth the trouble.

If you change magic immune condition to true it'll only hit magic immune units that are also matching the other conditions and not just include magic immune.

:goblin_boom:

Rheiko · Oct 3, 2013

well yeah, i didn't remove the magic immune condition just yet
but the other config is working right AFAIK

FUJI · Oct 5, 2013

Make a prefixes not temps xD!

Rheiko · Oct 5, 2013

i guess that's fine

Manasurge · Oct 5, 2013

jakeZinc said:
Make a prefixes not temps xD!

why? It's fine as it is when it's temporary variables. I'm naming them tempName so that if you're using the spell in your map you can also use those variables outside of the spell if you'd want.

Also, I'm leaking a unit group in the second part of the loop, I'm fixing that for the update I'm doing after I finish my damage calculations.

For some reason it deals a tiny tiny bit of damage too much and I'm trying to figure out why. It's less than 1 point of damage but it's not 100% right and that's bothering me.

FUJI · Oct 6, 2013

Downloading....

EDIT:

No, It will collide to others variables that user has.

EDIT2:

Nice! Heavy Documentation of the Spell.
Hmm,, seems you really effort this very hard XD.

+Rep for your effort and in your spell pack xD.

Manasurge · Oct 6, 2013

jakeZinc said:
Downloading....

EDIT:

No, It will collide to others variables that user has.

EDIT2:

Nice! Heavy Documentation of the Spell.
Hmm,, seems you really effort this very hard XD.

+Rep for your effort and in your spell pack xD.

Unless the user is using variables named tempName as globals there would be no conflicting variables.

I don't feel like changing it but I will if mods tell me to. :ogre_frown:

scy5115 · Oct 12, 2013

I don't know if this was intentional but this spell doesn't hurt ethreal units. I used banish on a unit in one of the groups and it was the only one that was unaffected by whirlpool.

Manasurge · Oct 12, 2013

scy5115 said:
I don't know if this was intentional but this spell doesn't hurt ethreal units. I used banish on a unit in one of the groups and it was the only one that was unaffected by whirlpool.

Indeed you are right! A small oversight on my part.
While the attack type was Spells the damage type was Normal and Ethereal units only take damage from Magic. Now that's been fixed they also take the extra 66% damage as ethereal units should from the Whirlpool.

Update v1.0.4
- Fixed an issue where the spell did not damage ethereal units. Attack type and damage type in now configurable.
- Moved hashtable loading to after the spell was confirmed to still be running.
- Now creates only 1 unit dummy per loop to cast slow.
- Now creates only 1 item for pathing checker.

Basically everything Maker said has been fixed. :goblin_good_job:

[Manasurge] Whirlpool v1.0.5

Whirlpool (Map)

Moderator

Moderator

Similar threads