Edit:
I've updated the testmap and added a second spell.
There is a lot of improvements in version 0.1.7 and I plan to make it even faster and even more modular. I also plan to create an easy to understand API.

Changed name to Missile.
Completely remade in modules, supports now a lot of optional stuff.
Added SpellHelper, fixed a lot of problems, made the Miranas Arrow example WAY more powerful to show what this system is able to be capable of.

BPower:
11:02, 25th Feb 2016
Reason for re-review:
Nowadays the spell section is packed full of missile systems from different authors, therefore a more qualified
moderator comment than "this is neat stuff" is required. There is keen...

Reason for re-review:
Nowadays the spell section is packed full of missile systems from different authors, therefore a more qualified
moderator comment than "this is neat stuff" is required. There is keen competition, hence your new
review may not be as good as it was before.

Criticism:

A system released for public usage needs a proper documentation, at least about its API.
You didn't write a single line of comments for other users. Compare it to the JassHelper
without Jass Helper Manual or the WorldEditor with neither blizzard.j nor common.j.
---

It's very unusual to use a timer interval, which can't be multiplied with an integer to a result of 1.
The most common timer interval in JASS is 1/32 which equals 0.03125.
You use

publicstaticreal period =0.03175

which is ~1/31.49
---

TRUE

and

FALSE

are constants for

true

and

false

.
It's not required to use them. I guess you know that.
---

GetUnitState(theUnit,UNIT_STATE_LIFE)>0.405

could be optimized to

GetWidgetLife(theUnit)>0.405

---

I guess it's your very own style of writting code, but your functions are arranged in a way, that the
Jass Helper must generate a lot of pseudo-code to obtain a valid function order.
Just look into MissileUnitHit. Functions are placed precise in the wrong order. Jass Helper fixes that, but for which price?
---

The unit enumeration radius is inaccurate, but that's the case in most missile systems.
You must add the maximum collision size used in the map to get all units in collision range.
---

You hide units which are below terrain level, very neat, but where do you restore the full range
of the locust effect after showing the unit? Btw In that function you have a unit leak.
---

set .destHitGroup[GetHandleId(theDest)]=1

is good, not perfect. Handle ids get recycled once there
is not reference to their object and the object is removed from the map. Therefore it must be

set .destHitGroup.destructable[GetHandleId(theDest)]= theDest

.
One of many reasons why Vexorians Table is compared to Bribes not perfect in API.
---

publicmethod checkMissileHit takesnothingreturnsnothing

does nothing, but takes space.
---

The math used in missile movement is cool. It's unfair to only criticise your work.
---

Array handle variables should be nulled once no longer needed.
---

Missile cast helper should be a seperate library named CastHelper. We have that better executed namely SpellEffectEvent.
---

Overall a bit unread-able code and even harder to use for someone who didn't code the system by him/her-self.

I don't know why a DC was taken into consideration in the past, obviously not out of objective reasons.
I depreciate the rating form 5/5 to 2,5/5. As we give out integer ratings you'll get a 3/5.
The entire system is good for own usage. For public submission
I would like to see a proper API documentation.

12:32, 8th Apr 2010

TriggerHappy's review

TriggerHappy:

Review 1

I have not yet tested the system.

You might as well comment out ParabolaZ2 as it serves the same purpose as the first one. The only reason is should even be in the library is to give a better explanation as to how the first one works. so yeah.. just comment it out (/* */).

Your globals are private, don't give them prefixes (CM_NAME).

You could store

(2. *bj_PI* CM_PERIOD)

in a constant instead of doing the operation each time.

Is there really a need for the local in enumDestsInRange?

You are using a group per instance, either A) recycle them or B) use a global group.

Is there a reason why you're creating the timer in onInit instead of just initializing it?

It would be more efficient if you placed the contents of all your update functions directly in the move method. Yes it's going to be harder to read but just make some comments explaining them. Because currently you are doing a bunch of unnecessary function calls.

The system name is horrible. Just by using a system not made by blizzard already indicates that it's custom. You might was well call it Missile.

I have tested the system and the demo map is really lacking.
If you want to show people the power of your system include some really awesome eyecandy examples.

Still need to recycle groups.

Your comments explaining the interface use the wrong grammar. It's ran, not runned.

You still need to remove the prefixes on your globals.

Why don't you just make locRect static? As of now you are creating and destroying rects when you could just be re-using the same one.

You realize you are destroying enumGroup twice, right?

The_Reborn_Devil:
Due to Trigger's demotion I am now the only spell mod and thus I can finally review your system.

Hmm, it would seem my review is gone, but the system is great.

Status: Approved
Rating: Highly Recommended [Director's Cut]

hvo-busterkomo: After evaluating this system I do not think it meets the standards to be a Director's Cut. However, in respect to The_Reborn_Devil, I have left it as Highly Recommended.

I made it that way, I think that is most realistic.
(That would be cheating instead)
However, you can still enable terrain hitting (but then missiles that hit terrain die, however, you can overwrite this behavior as well)