IMO the way the stun functions is nonoptimal: it manually removes the stun buff after the appropriate duration. This means hitting a stunned target with a second arrow will not actually increase the stun duration (it adds actually 0 duration to the stun) since the buff is immediately removed when the first instance expires.
There is a better way to do this, now that ability fields can be set with triggers before the cast. You can just set the stun duration to be correct after it's given to the dummy, order the storm bolt, and then never have to worry about removing the buff or it overlapping and getting weird.
Honestly the way this spell is coded to count cycles instead of the actual distance the arrow traveled is just silly. Its like they forgot RMinBJ() and RMaxBJ() existed entirely (they did in 2011) and they never checked their math to see what the output stun duration was. Consider what happens with this relationship:
JASS:
if (((this.cycles * 30) / 150) * 0.5) > GetStunMaxDuration(this.level) then //that math just becomes this.cycles/10 > max stun, btw
set this.cycles = GetCycles() / 2
endif
//later in the function
call TimerStart(t, ((this.cycles * 30) / 150) * 0.5, false, function thistype.stun) //also just this.cycles/10 here
What it's
supposed to do is cap the stun duration at GetStunMaxDuration, but it doesn't actually do that. What it actually does is turn any stun that would have been longer than the max stun duration into whatever duration would have been applied at the halfway mark. This works fine for actually stunning something if the halfway point is where it should be maxmized, but that might not be how you want it to work.
That relationship straight up does not output a stun duration of 0.5s for a minimum-cycle hit, either. It outputs a stun duration of... 1/10 of a second. So what happens is that the dummy hasn't even cast the storm bolt yet when the 'removal' timer goes off and attempts to remove the buff which doesn't exist. So the stun is simply never removed. If your dummy can instantly cast in any direction you probably wouldn't have this issue but the stun would be super short. There's a potential issue when stunning invisible units is not enabled that leaves an if/then/else with an empty
then block, which used to be something the WE really did not play nicely with and you had to avoid; also the invisibility check uses the wrong player to compare to so it literally does nothing and is always false. And it checks for invis but not spell immune or resistant skin or straight up invulnerability... HOW DID THIS SPELL GET APPROVED EVEN IN 2011?!
There's also a few things like the arrow duration, speed, and aoe radius as well as the scaling for the stun that are hardcoded into the spell functionality rather than using functions or constants for those to make them easily modifiable. I would change those but at that point it's overhauling the whole spell and this thing is from 2011 so that
can't be the last thing I'd change (it checks UNIT_TYPE_DEAD instead of using UnitAlive, for instance, and I need to stop looking for other things to change now or I'll never stop).
Huge potential issue: the spell uses bj_lastCreatedGroup as its enum group. That, uh... only works as long as the 'last created group' hasn't been destroyed, because then that becomes a null group (until another is created). It could also just randomly grab some other group and fuck with it that could cause huge headaches for debugging down the line. I've changed the global declaration of G as well since that's super important to the spell actually functioning and is an insane oversight.
With all of that that in mind it's best simply to rewrite the function to alter the distance-checking logic and eliminate the need to remove the buff manually. Replace the following method and globals block and you should be good to go:
JASS:
//Replace what's already in the spell code with these altered versions.
//You can fully delete the "stun" method, it's no longer needed
globals
private constant group G = CreateGroup()
endglobals
private method periodic takes nothing returns nothing
local unit j
local unit dummy
local timer t
local real d
// Moving the arrow
call SetUnitPosition(this.dummy, GetUnitX(this.dummy) + 30. * Cos(this.a), GetUnitY(this.dummy) + 30. * Sin(this.a))
set this.cycles = this.cycles + 1
// Checking if the arrow has already covered the max distance
if this.cycles < GetCycles() then
call GroupEnumUnitsInRange(G, GetUnitX(this.dummy), GetUnitY(this.dummy), GetArrowDetect(), null)
loop
set j = FirstOfGroup(G)
exitwhen j == null
call GroupRemoveUnit(G, j)
// If we've a target
if GetAffectedUnits(this.caster, j) then
// Invis check...
// If the unit is invisible we will only put
// the stun if AFFECT_INVIS is true
if (AFFECT_INVIS) or (not AFFECT_INVIS and not IsUnitInvisible(j, GetOwningPlayer(this.caster))) then
set dummy = CreateUnit(GetOwningPlayer(this.caster), DUMMY, GetUnitX(j), GetUnitY(j), 0)
call UnitApplyTimedLife(dummy, 'BTLF', 1.)
call UnitAddAbility(dummy, STUN_RAWCODE)
// Timer of stun...
// the 0.5* affects where the stun is maximized; it will occur at whatever fraction of the travel that coefficeint is, in this case 50%
// if set to 1.0* the stun is maximized at 100% of the distance traveled
// if set to 0.2* the stun is maximized after 20% of the distance
// etc.
// Never remove this constant (just replace it with a 1.0) for fear of turning real division into integer division!
set d = 0.5 //this is the minimum stun duration
set d = d + this.cycles/(0.5*GetCycles())*(GetStunMaxDuration(this.level) - d)
call BlzSetAbilityRealLevelField(BlzGetUnitAbility(dummy, STUN_RAWCODE), ABILITY_RLF_DURATION_NORMAL, 0, d)
call BlzSetAbilityRealLevelField(BlzGetUnitAbility(dummy, STUN_RAWCODE), ABILITY_RLF_DURATION_HERO, 0, d)
call IssueTargetOrder(dummy, "thunderbolt", j)
else
endif
call KillUnit(this.dummy)
set this.dummy = null
set this.target = j
// Making damage...
call UnitDamageTarget(this.caster, j, GetDamage(this.level), false, true, ATK_TYPE, DMG_TYPE, null)
call this.stopPeriodic()
call this.destroy()
call GroupClear(G)
exitwhen true
endif
endloop
else
call KillUnit(this.dummy)
set this.dummy = null
call this.stopPeriodic()
call this.destroy()
endif
set dummy = null
set j = null
endmethod
