Approved
Moderator
M
Moderator
12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.
18:14, November 17 2012
Magtheridon96:
- Don't repeat things like (Attacking unit) and (Attacked unit). Store them into variables and use those.
- Don't use an "Attack" event in your spell, use a Damage event. Here's a resource that should be useful: Click here.
- You have leaks everywhere. Locations, groups and special effects all leak, so you need to remove them.
This thread should show you how to remove leaks properly.
Remember, store them, and then call either RemoveLocation or DestroyGroup. (for locations and groups respectively)
- Dying unit -> Triggering unit
- You should have configuration triggers that run on map initialization. These triggers allow the user to configure the spell and change things like the special effect path, damage per level, base damage or pretty much anything concerning the spell. Some spells even allow you to configure whether you want them to destroy trees or not.
- Casting unit -> Triggering unit
-
Seriously though, you totally do not need regions here. Just store the position of the caster into a variable, then choose a random distance (store it in a real variable), and a random angle (store it in a real variable), and then use the distance and the angle to have a new location based on the position of the caster with an offset using the distance you got and the angle.
-
-
- Cache (Last created unit) into a variable and use that variable instead of repeating (Last created unit) more than once to increase performance and cleanliness of the code.
- In the OS Illuslash trigger, since you have this line of code: "Custom script: call DestroyGroup( udg_CL_Group2[1] )", in the Else and the Then actions, you might as well keep one of them and move it to the end of the trigger. This way, you don't have to repeat this line twice.
- In the OS Illuslash trigger, when you set the group variable, you're leaking a location. You're leaking a location when you move the unit too.
- I don't understand why you're using CL_Group[1] followed by an evaluation of CL_Group[Temp] in your OS Illuslash trigger. It's as if these spells aren't MUI. In order to make it MUI, merge both of those triggers that run every 0.4 seconds. They are going to execute in sync. Currently, the effects by the OS Illuslash trigger are not MUI, and only work for 1 instance of the spell casting.
- In the Dummy killing trigger, you have location leaks everywhere. You also have group leaks such as: ((Units of type WTF) is empty) Equal to true. Seriously forget that line. Keep the trigger on at all times because (Units of type something) will create a permanent leak.
This spell needs a lot of work in order to be approved.
IcemanBo: Too long as NeedsFix. Rejected.
18:14, November 17 2012
Magtheridon96:
- Don't repeat things like (Attacking unit) and (Attacked unit). Store them into variables and use those.
- Don't use an "Attack" event in your spell, use a Damage event. Here's a resource that should be useful: Click here.
- You have leaks everywhere. Locations, groups and special effects all leak, so you need to remove them.
This thread should show you how to remove leaks properly.
Remember, store them, and then call either RemoveLocation or DestroyGroup. (for locations and groups respectively)
- Dying unit -> Triggering unit
- You should have configuration triggers that run on map initialization. These triggers allow the user to configure the spell and change things like the special effect path, damage per level, base damage or pretty much anything concerning the spell. Some spells even allow you to configure whether you want them to destroy trees or not.
- Casting unit -> Triggering unit
-
-
Set CL_RdLoc[Temp] = (Random point in (Region centered at (Position of CL_Caster[Temp]) with size (CL_AOE[Temp] CL_AOE[Temp])))
Seriously though, you totally do not need regions here. Just store the position of the caster into a variable, then choose a random distance (store it in a real variable), and a random angle (store it in a real variable), and then use the distance and the angle to have a new location based on the position of the caster with an offset using the distance you got and the angle.
-
There are plenty of leaks in all your spells. Some examples:
-
Unit - Create 1 WTF for CL_Player[Temp] at (Position of CL_Caster[Temp]) facing (Facing of CL_Caster[Temp]) degrees
-
Special Effect - Create a special effect attached to the weapon of CL_illusion[Temp] using Abilities\Spells\NightElf\SpiritOfVengeance\SpiritOfVengeanceBirthMissile.mdl
-
Special Effect - Create a special effect attached to the hand, left of CL_illusion[Temp] using Abilities\Spells\NightElf\SpiritOfVengeance\SpiritOfVengeanceBirthMissile.mdl
-
-
Set CL_Group[1] = (Units within CL_AOE[Temp] of CL_Loc[1] matching ((((Matching unit) is A structure) Equal to false) and ((((Matching unit) is alive) Equal to true) and (((Matching unit) belongs to an enemy of CL_Player[Temp]) Equal to true))))
- Cache (Last created unit) into a variable and use that variable instead of repeating (Last created unit) more than once to increase performance and cleanliness of the code.
- In the OS Illuslash trigger, since you have this line of code: "Custom script: call DestroyGroup( udg_CL_Group2[1] )", in the Else and the Then actions, you might as well keep one of them and move it to the end of the trigger. This way, you don't have to repeat this line twice.
- In the OS Illuslash trigger, when you set the group variable, you're leaking a location. You're leaking a location when you move the unit too.
- I don't understand why you're using CL_Group[1] followed by an evaluation of CL_Group[Temp] in your OS Illuslash trigger. It's as if these spells aren't MUI. In order to make it MUI, merge both of those triggers that run every 0.4 seconds. They are going to execute in sync. Currently, the effects by the OS Illuslash trigger are not MUI, and only work for 1 instance of the spell casting.
- In the Dummy killing trigger, you have location leaks everywhere. You also have group leaks such as: ((Units of type WTF) is empty) Equal to true. Seriously forget that line. Keep the trigger on at all times because (Units of type something) will create a permanent leak.
This spell needs a lot of work in order to be approved.
