Watery Heal v1.0

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

20:09, 10th Apr 2013
Magtheridon96:

Comments

• You can get rid of the SquareRoot call in your distance calculation in the Trig_WateryHeal_ProjTimer function. Instead of comparing curDistance to WateryHeal_CollideDistance(), you would compare it to WateryHeal_CollideDistance()*WateryHeal_CollideDistance().
• You should probably move these:
  
  Code

  set healingProjectile = null
  set healProjTimer = null
  set healGroup = null

  To the inside of the codeblock corresponding to the "Then" actions of the first if block because those variables are not changed unless that block of code runs.
• If you want, you can implement a dummy recycler to allow for more efficient execution (Way better than creating a ton of units. CreateUnit is a very slow function call as we all know)
• You should be able to use a global group variable created once instead of a local group inside the Trig_WateryHeal_ProjTimer function.
• Since you're repeating GetOwningPlayer(abilityCaster), GetUnitX(waterElemental), and GetUnitY(waterElemental) inside the loop, you should cache them into variables. It's a waste to call them again and again and again for all created projectiles. You should probably only cache them inside the "Then" code block though. You don't need those cached variables unless that block of code actually runs.
• In the function that runs upon casting the spell, you should only cache the triggering unit and the spell target coordinates inside the if block because you don't need them outside of it. You would also null them inside of it after caching them inside.
• The Trig_WateryHeal_Stacks functions runs every second even if the spell was never learned. Moreover, it iterates over all units in the map. This is terribly inefficient. You can speed this up greatly by having a static global group variable. Register an Ability-Learn event. Whenever a unit learns the right ability, add him to the group. The code may still run every second, but at least it wouldn't be as intensive as it was before. In fact, you don't even need a group for this. You can use a unit array and a count. Whenever a unit learns the right ability, add him to the array. Iterate over the units in the array every second and do what you have to do. This is WAY better in performance than doing a group enumeration over every single unit in the map every second.
• GetHandleId(GetExpiredTimer()) will return a constant value in the Stacks function because you're creating the timer once on map initialization. This means that you can totally get rid of this call and just use any constant value like $ffffff (Yes, I believe that's valid JASS)

 

[Almia]

I think using timers per instance is bad.You might wanna use ForGroup if you have too or just a dynamic array or linked list.

You can also use a dynamic group(one group for all instances) so that you will only run one timer(you can see examples from mine and Tank's resources)

Your math expressions are not done like that.
+,- should have spaces before/after them
*,/ dont.

Loop should be 0.03125,though that's not necessary.

RGB should be configurable.

GetRandomReal(0,360) must be 0 to 359 or 1 to 360,because in degrees,360 = 0

GetUnitAbilityLevel(tempGroupUnit,WateryHeal_AbilityId())
Cache this.

 

[Maker]

GetRandomReal(0,360) must be 0 to 359 or 1 to 360,because in degrees,360 = 0

Then you would miss angles between 359 and 360 or 0 and 1. (0,360) is better.

 

[Almia]

Yeah you are right.