Moderator
M
Moderator
19:21, 15th Mar 2016
BPower:
^These variables should be constants. Furthermore the variable naming could follow the JPAG.
The JPAG itself is guided by the naming convention of the common.j and blizzard.j.
I think you should split the globals into two seperate blocks.
One for the user setting and one below your set of customizable function for the internal variables.
The global group g is not required.
I would use a group array, one per spell instance. Also I would use ForGroup instead of swapping groups via FirstOfGroup.
I'm a big supporter of inlined FoG loops, but not of group swaps. FirstOfGroup has one small weakness, it fails
for invalid unit handles ( u == null ) even if the group is not empty.
As you hide units and / or change their invulnerability status you should provide the maximum safety possible in your code.
Null the local timer t and the array variables which require nulling.
I prefer a custom filter function instead of a hardcoded one if IsUnitEnemy(FoG, GetOwningPlayer(Caster[id])) and UnitAlive(FoG) then .
Something like if FilterUnit(u, casterOwner) then .
exitwhen index == NumberOfBeams(Level[id]) place this exit condition at the top of the loop.
It's my coding philosophy to put safety over performance. The thread could break for NumberOfBeams == 0.
Your code is MUI, but can only be active once per unit.
I want to draw your attention to SpellEffectEvent. A very short, yet powerful piece of code from our JASS database.
The function name "Event" seems not fitting to me. I would recommend "Init" or "OnInit".
Not that it matters much, but slightly improves the read-ability of your code.
Overall cool spell. Good job.
BPower:
JASS:
// The spell that triggers the Time Leap
private integer Spell = 'A000'
// Basically the speed of the wave, dont go too low on this one
private real Interval = 0.08
// Animation of the waves
private string Animation = "Abilities\\Spells\\NightElf\\Blink\\BlinkCaster.mdl"
// Animation when it captures a unit
private string AnimationDis = "Abilities\\Spells\\Human\\MassTeleport\\MassTeleportCaster.mdl"
// Animation when it releases a unit
private string AnimationRe = "Abilities\\Spells\\Human\\MassTeleport\\MassTeleportCaster.mdl"
The JPAG itself is guided by the naming convention of the common.j and blizzard.j.
I think you should split the globals into two seperate blocks.
One for the user setting and one below your set of customizable function for the internal variables.
The global group g is not required.
I would use a group array, one per spell instance. Also I would use ForGroup instead of swapping groups via FirstOfGroup.
I'm a big supporter of inlined FoG loops, but not of group swaps. FirstOfGroup has one small weakness, it fails
for invalid unit handles ( u == null ) even if the group is not empty.
As you hide units and / or change their invulnerability status you should provide the maximum safety possible in your code.
Null the local timer t and the array variables which require nulling.
I prefer a custom filter function instead of a hardcoded one if IsUnitEnemy(FoG, GetOwningPlayer(Caster[id])) and UnitAlive(FoG) then .
Something like if FilterUnit(u, casterOwner) then .
exitwhen index == NumberOfBeams(Level[id]) place this exit condition at the top of the loop.
It's my coding philosophy to put safety over performance. The thread could break for NumberOfBeams == 0.
Your code is MUI, but can only be active once per unit.
I want to draw your attention to SpellEffectEvent. A very short, yet powerful piece of code from our JASS database.
The function name "Event" seems not fitting to me. I would recommend "Init" or "OnInit".
Not that it matters much, but slightly improves the read-ability of your code.
Overall cool spell. Good job.