TNT Cactus v1.5

[Moderator]

Approved.

You could increase follow through time a bit, and make the ability not disable other abilities.

Old review

10:27, 18th Dec 2012
Magtheridon96:

Everything looks fine now.
Just one last thing:

call KillUnit(whichProjectile)
This isn't enough to remove the dummy completely.
You should call RemoveUnit. Yes, this will ruin the effect, but the solution is to create and destroy a special effect using the model of the projectiles at the position of whichProjectile so it seems like the death animation played.
Then you would call RemoveUnit(whichProjectile).

 

[Almia]

Review:
1.I think removing the TNT_CactusAbilityLevel function is much better so it wont slow.
2.Mag can make this review too:Add an integer arguement on both real functions
3.Special effect should be configurable
4.Merge the condition ability being cast w/ the actions like:
[jass=]
function MyCond takes nothing returns boolean
if GetSpellAbilityId() == MyAbility then
//Actions here
endif
return false
endfunction[/code]
This way,it will be faster
5.Because you are just using the group in a one function,just make it a local.
6.Hashtable should have prefix(to prevent collision)
7.[jass=]exitwhen i > 9 + abilityLevel * 3[/code]
>
[jass=]
function GetMaxInteger takes integer level returns real
return 9 + level * 3
endfunction

//Usage
loop
exitwhen i > GetMaxInteger(abilityLevel)
[/code]
8.EnumUnit should be nulled.
That's all

 

[Ender]

5.Because you are just using the group in a one function,just make it a local. why should he do that? the purpose of using a global group is so that you don't have to create and destroy which is slower then accessing a global group.

 

[Arrr]

Shouldnt it be that the higher the level is, the less it needs to explode? that would make more sense in my opinion

 

[bowser499]

Updated.

 

[bowser499]

Updated to 1.5