Moderator
M
Moderator
12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.
20:58, 2nd Nov 2012
Magtheridon96: First of all, I would like to commend you on the fact that you've improved. I'm going to attempt to improve myself by trying out a new format for my Moderator reviews. (Update: I have removed the solved points and updated this review accordingly)
Indexing
Leaks
Efficiency
IcemanBo: Too long as NeedsFix. Rejected.
20:58, 2nd Nov 2012
Magtheridon96: First of all, I would like to commend you on the fact that you've improved. I'm going to attempt to improve myself by trying out a new format for my Moderator reviews. (Update: I have removed the solved points and updated this review accordingly)
Indexing
Since your indexing method isn't functional, you should check out Tank-Commander's indexing method, which happens to be /much/ simpler.
Leaks
You're leaking the following things:
- All the special effects that are being created in your spell are not being removed. They should be stored into arrays so you can destroy them upon deallocating a spell instance (when the spell ends)
- In your looping trigger, there is a loop in which you do this:
Custom script: call RemoveLocation(udg_FPB_TargetPoint[udg_FPB_Current_Index])
This line should be outside the loop because it runs more than once when you're destroying the spell instance. This means that you're removing the location more than once.
Efficiency
- It would be more efficient to store the owner of the caster into a spell array variable so you don't have to constantly repeat the calls over and over again. Create a player array for storing the owner of the caster.
- FPB_DummyPoint doesn't need to be an array.