[JASS] Freezing Field v1.1d

[Moderator]

Freezing Field v1.1c - Reviewed by Maker - 15th Feb 2013

Approved

The spell works and causes no fps drop even with multiple stacked casts. This is a bit like Blizzard except it is not a channeling spell. GetWidgetLife(u) > 0.405 -> not IsUnitType(u, UNIT_TYPE_DEAD) DAMAGE_TYPE_FIRE - > DAMAGE_TYPE_MAGIC Fire and magic work just the same way, but magic is the base type, I recommend using that.

[td]
[/td]

Old reviews

Reviewed by Maker

Required changes

• The timer is never paused and the hastable is not flushed
• Add importing instructions

16:57, 15th Jan 2013
Magtheridon96:

Here's a brief review:

• if modifier == 1 then
      set angle = GetRandomReal (0, 90)
  elseif modifier == 2 then
      set angle = GetRandomReal (90, 180)
  elseif modifier == 3 then
      set angle = GetRandomReal (180, 270)
  else
      set angle = GetRandomReal (270, 360)
  endif

  
  ->
  
  set angle = GetRandomReal(-90, 0) + modifier * 90.
  
• You're forgetting to null freezefx in the periodic function.
• Instead of triggers, I'd recommend using timers. Doing things by evluation count and timed triggers is a really bad method in terms of speed because if you have trigger evaluations piling up, you will face freezes at times, limiting the features of any given map :/
  DotA does this and this is why knocking back 10 - 15 units at the same time causes the game to freeze terribly. This is also the reason why spell casting can cause freezes on slower computers. It causes hundreds of triggers to evaluate and some of them to execute.
• What you should do is start a timer and save its handle id in the hashtable, then use that handle id to store other data. When a timer expires, get its handle id, and use that to retrieve spell data from the hashtable. Execution counts would also be retrieved from the hashtable.
• Your indentation is messed up in the periodic function directly after the first else line. The "if modifier == 1 then" line is not in the same tab level as its corresponding "endif" line. It's in the same tab level as the "endif" of the first if statement.
• The END_CAST event would be handled by a different function inside your code when you're going for the timer approach.
• You can avoid setting the caster, x, and y locals in your Cast function outside the if block. They aren't needed at all if you're not going to branch into the if block's code.
• Store the handle Id of the the timers you're creating inside the periodic function into a local integer.
• I'd like to warn you that your periodic function runs the following code:
  

  set x1 = x + offset*Cos(angle)
  set y1 = y + offset*Sin(angle)
  set t = CreateTimer()
  call SaveUnitHandle( udg_FF_Hashtable, GetHandleId(t), 0, caster )
  call SaveReal (udg_FF_Hashtable, GetHandleId(t), 1, x1 )
  call SaveReal (udg_FF_Hashtable, GetHandleId(t), 2, y1 )
  call TimerStart (t, FF_ExpDelay(), false, function FF_DamageUnit)
  set caster = null
  call DestroyEffect (AddSpecialEffect (FF_ExpFx(), x1, y1))
  set t = null

  even during the END_CAST event :V

 

[Almia]

OMG!!! Another storm-related spell from YOU!!!!

oh,forgot the review.

Review #1
1)when damaging target,you must turn to 2 booleans to false
2) if(GetSpellAbilityId()=='A004')then
ahaha missed this one.You cached it but didnt use it.
3)you forgot to null unit u in the function FF_Periodic

that's all i can see

 

[Doomlord]

OMG!!! Another storm-related spell from YOU!!!!

oh,forgot the review.

Review #1
1)when damaging target,you must turn to 2 booleans to false
2) if(GetSpellAbilityId()=='A004')then
ahaha missed this one.You cached it but didnt use it.
3)you forgot to null unit u in the function FF_Periodic

that's all i can see

Well okay I will make sure to change the theme next time

Now about the review

1&2: Code updated.
3: According to Mag, when you use exitwhen u == null it means the variable is already nulled when the loop finished, thus another nulling is unnecessary.

 

[Almia]

oh sorry,i didnt saw the FirstOfGroup loop

 

[Doomlord]

How the f**k did you reply one minute after me? Is it some kind of dark magic?

 

[Almia]

I can read yoar mind!!!

 

[Almia]

btw,WEAPON_TYPE_WHOKNOWS = null

 

[Doomlord]

I can read yoar mind!!!

Damn....

btw,WEAPON_TYPE_WHOKNOWS = null

Yeah I know xD

 

[Almia]

Then put it before Mag catches you

 

[Doomlord]

Then put it before Mag catches you

Actually we talked. He won't mind it at all

 

[Lambdadelta]

The snow shard model is by Evading Self (Source: http://www.wc3c.net/showthread.php?t=87344). I'm not sure about the actual field effect though.

There is no need to assign the triggering unit and targeted coordinates on local variable declaration. That part of code will be fired for every spell that is cast in the map as it runs on the spell effect even. Therefore you should do the assignments within the if block checking whether the spell id was that of Freezing Field's. Alternatively you could just have a separate function with everything else done in it.

Well, there are many more things with your code that you could improve, but I'll let someone else point them out. Mainly it is how you do things.

 

[Doomlord]

The snow shard model is by Evading Self (Source: http://www.wc3c.net/showthread.php?t=87344). I'm not sure about the actual field effect though.

There is no need to assign the triggering unit and targeted coordinates on local variable declaration. That part of code will be fired for every spell that is cast in the map as it runs on the spell effect even. Therefore you should do the assignments within the if block checking whether the spell id was that of Freezing Field's. Alternatively you could just have a separate function with everything else done in it.

Well, there are many more things with your code that you could improve, but I'll let someone else point them out. Mainly it is how you do things.

Thanks very much for taking your time reviewing this spell of mine. And yes there are still many things that can be improved. It would be great if you could point them out for me also but it is entirely your choice.

 

[Doomlord]

Spell currently being updated as Mag reviewed. Thank you very much for the review.

EDIT: Spell & code revamped and updated

 

[YuC0]

Nice btw...
Errr, can somebody here make a nomad spell pack for me? i'm newbie, just want to learn..
Nomad is the hero from "Heroes of Newerth"(HON)
http://forums.heroesofnewerth.com/wiki/index.php/Nomad
The link above is about all his abilities

 

[Doomlord]

Nice btw...
Errr, can somebody here make a nomad spell pack for me? i'm newbie, just want to learn..
Nomad is the hero from "Heroes of Newerth"(HON)
http://forums.heroesofnewerth.com/wiki/index.php/Nomad
The link above is about all his abilities

You are supposed to post that in the Request forum here

 

[YuC0]

owh i see, i'm new here...thanks anyway

 

[Skypsi]

nice spell is it possible to get a non-dotaish? version that scales with stats for more damage? also the name kinda gave the impression that it would freeze on impact not explode.

 

[Doomlord]

nice spell is it possible to get a non-dotaish? version that scales with stats for more damage? also the name kinda gave the impression that it would freeze on impact not explode.

Yeh. You just need to modify it a little bit (if you know JASS that is). Well about the name it is a homage to the original ability this is based on, and let's not forget the fact that the spell in DotA only slows, not freeze

 

[Daffa]

This one is amazing, has nice effect and no flaws.
5/5