Implosion
What the spell does: 17/30
- I don't remember seeing this before, it is like Fan of knives that isn't homing, but the required stuff to do this were already possible.
Code : 5/20
- The way it was implemented is not the best, since it is in JASS, it could have just used an array and loops instead of repeating the same code 8 times.
- Over uses BJ functions that could have been replaced with natives, for example instead of counting the units in the group and checking if it was 0, it could just use FirstOfGroup and check if it returns null. The same with the order functions that could have been replaced with order natives.
- It also has things like ==true, comparing a boolean with true does nothing, and just using the boolean directly works.
- Could have used MoveLocation instead of removing the location and creating a new one every time.
- Major mistake: uses units for the damage, and gives them to neutral passive, should give them to the owner of the hero that is casting the ability, OR much better use the Damage functions from patch 1.17 (Although it uses their attacks for missiles too), Because of this mistake the spell gives no bounty/experience to the hero.
Efficiency: 23/30
- Fixes the memory leaks, but should also set the handle derived local variables to null to fix the local variables leak.
- As I said ( CountUnitsInGroup(g) != 0 ) is the same as ( FirsOfGroup(g) != null ) but FirstOfGroup is much faster.
- Replacing BJ functions with natives would increase efficiency
Multi instancibility 10/10
- Completelly multi instanceable
Eye Candy: 7/10
- Looks nice to see those fire balls exploding, don't get the Implosion name. But the effects are enough, without causing too much
Total 62/100 (3/5) : I do see potential in you, must recomend to practice better JASS methods for doing stuff and using Handle Variables and all that stuff, try checking the tutorials and scripts at
http://www.wc3jass.com if you want to improve.
