Moderator
M
Moderator
BPower:
16:26, 24th 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 competition, hence your new
review may not be as good as it was before.
Criticism:
Decreased rating from Highly recommended to Useful.
17:43, 31st Jul 2009
Eccho:
This is a neat system indeed, and is very handy for the one who could need it. It's efficient (Despite some GetFilterUnit() discussions I had with some others here, a local is probably faster when used more than 3 times), leakless and MUI. The documentation is fully understandable as well.
Even though you also tend to use CreateGroup() and DestroyGroup() I still will give you highly recommended. It's really smooth and my pc didn't notice anything else awkvard.
16:26, 24th 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 competition, hence your new
review may not be as good as it was before.
Criticism:
- Towards the filter function namely CondsFunc
GetUnitTypeId(GetFilterUnit()) != DUMMY_ID
is a case which can't happen as the dummy has locust abilities.GetWidgetLife(GetFilterUnit()) > 0.405
is not a valid "unit is dead" filter. On the other side why are dead units invalid targets by default.GetEventMissile.Caster != GetFilterUnit()
This makes sense in most cases, but remember that "self" is a target option of Warcraft III.- Finally the filter conditions could be inlined for performance issues. A critical point in projectile systems.
- Nulling array handle variables, once no longer needed, is a very appreciated.
--- - It's very strange that collision, one of many possible missile properties, is a argument for the creator function.
That's absolutly random and shouldn't be.
--- if not GetEventMissile.ma.onHit() and GetEventMissile.ma != 0 then
<< this order of arguments makes no sense.
---- You don't check for map boundaries. Not doing so can cause a fatal error, if a projectile leaves the map.
--- - You're code is arranged in a way that tons of pseudo code, trigger evaluations, ... are needed to
correct the function placement.
--- - You only support movement in 2D, missile systems of newer generations deal with z adjustment internally.
--- - It's hard to evaluate which units are actually hit. If a users uses the hu group with a inlined FirstOfGroup,
than they will not be marked as hit internally. You invoke a second group enumeration or ForGroup execution.
--- - The dummy must be moved before the interface triggers run. Otherwise GetUnitX/Y is simply incorrect when compared to your on collide event.
--- - Code may run for an instance which is already deallocated.
--- - Your iteration over the stack is incorrect. In case a missile is destroyed the index must not be increased by 1.
Decreased rating from Highly recommended to Useful.
17:43, 31st Jul 2009
Eccho:
This is a neat system indeed, and is very handy for the one who could need it. It's efficient (Despite some GetFilterUnit() discussions I had with some others here, a local is probably faster when used more than 3 times), leakless and MUI. The documentation is fully understandable as well.
Even though you also tend to use CreateGroup() and DestroyGroup() I still will give you highly recommended. It's really smooth and my pc didn't notice anything else awkvard.