Elune's Arrow v1.3

[Moderator]

9th Nov 2011
Bribe: You don't need to initialize values to null/0 from the "create" method, they are already those values. Only set to null/0 from the "destroy" method.

You do not require a "t" member to remember the timer. It's a waste of an array. You might use "local timer t" or just one scalar global timer variable, to remember the timer, because you only need it for temporary reference not dynamic reference as you have done here.

SpellEffectEvent would be better here for handling the spell event so the trigger doesn't evaluate for every spell that's cast.

You might know me as the guy who says "no project is ever finished" so these are just room for improvement, but not to keep it from being approved here. Nice work.

 

[Noobsmoke]

First !
Your spell it's so original ^^
Edit : waiting for a hook

 

[Ruke]

First !
Your spell it's so original ^^
Edit : waiting for a hook

OH THX man, your comment is so useful

 

[.VTZ.]

lol what about the omnislash?

 

[Bribe]

        static method stun takes nothing returns nothing
            local thistype this = GetTimerData(GetExpiredTimer())
            call UnitRemoveAbility(this.target, STUN_BUFF)
            set this.target = null
            call this.destroy()
        endmethod

Put this below the "destroy" method.

TimerUtils is too much for such a small timeout like 0.025, use a different timer system for that, such as a stack loop.

Obviously for the other timer it should use TimerUtils.

if AFFECT_INVIS then you should learn about static if's.

You should also use proper convention: JPAG - Jass Proper Application Guide.

 

[Ruke]

        static method stun takes nothing returns nothing
            local thistype this = GetTimerData(GetExpiredTimer())
            call UnitRemoveAbility(this.target, STUN_BUFF)
            set this.target = null
            call this.destroy()
        endmethod

Put this below the "destroy" method.

TimerUtils is too much for such a small timeout like 0.025, use a different timer system for that, such as a stack loop.

Obviously for the other timer it should use TimerUtils.

if AFFECT_INVIS then you should learn about static if's.

You should also use proper convention: JPAG - Jass Proper Application Guide.

Ok, thx for the tips ^^.
And yes, when I was making the spell, I was thinking about use another timer system (like t32, maybe?) to make the projectile's movement...

And yes, I know a little about the static ifs, but yes, I will learn more ^^.

JPAG, ok, I will read it .

Anything else is right? xD

Greetings and thx again (sry for my english :S)

 

[Bribe]

Yeah Timer32 would be perfect for that period.

The code looks a lot better than most!

 

[Ruke]

Spell's updated

Thx Bribe for the tips ^^.
Now, I will try add the T32 library.

 

[Ruke]

Well, I made the spell with T32 library. Tell me guys, what do you think?. If it's fine, I will update it:

// Elune's arrow by Ruke
// ---------------------
//
// REQUIRES: TimerUtils -> [url]http://www.hiveworkshop.com/forums/submissions-414/system-timerutils-204500/[/url]
// REQUIRES: Timer32 -> [url]http://www.thehelper.net/forums/showthread.php/132538-Timer32[/url]
//
// The spell does what?
// --------------------
// Fires an arrow to a location with deadly precision, dealing large damage and stunning 
// the first unit it strikes. Stun duration increases based on how far the target 
// is, ranging from 0.5 to 5 seconds. Has 3000 range.
// More information: [url]http://www.playdota.com/heroes/priestess-of-the-moon#skill178[/url]
//
// How to import?
// --------------
// 1 ) Copy this trigger
// 2 ) Copy the dummy's unit (Object Editor -> Unit)
// 3 ) Copy Stun Elune (Object Editor -> Spell)
// 4 ) Copy Elune's Arrow (Object Editor -> Spell)

library EluneIsArrow initializer Init requires TimerUtils
    //==========================================================================================//
    //                                   CONFIGURABLE                                           //
    //==========================================================================================//
    globals
        private constant integer    DUMMY           = 'h003'                // Stun's dummy
        private constant integer    DUMMY_ARROW     = 'h002'                // Arrow's dummy
        private constant integer    STUN_RAWCODE    = 'A002'                // Stun's spell
        private constant integer    STUN_BUFF       = 'BPSE'                // Stun's buff
        private constant integer    SPELL_RAWCODE   = 'A001'                // Spell's rawcode
        private constant damagetype DMG_TYPE        = DAMAGE_TYPE_MAGIC     // Damage type
        private constant attacktype ATK_TYPE        = ATTACK_TYPE_NORMAL    // Attack type
        private constant boolean    AFFECT_INVIS    = false                 // Affects the stun to invisible units?
    endglobals
    
    // Returns the spell's damage
    private constant function GetDamage takes integer level returns real
        return 90. * level
    endfunction
    
    // Returns the max distance that the arrow can cover
    // cycles * 100 to get distance (as you can see, 100 cycles = 3000 max distance)
    private constant function GetCycles takes nothing returns integer
        return 100
    endfunction
    
    // Arrow's detection range
    private constant function GetArrowDetect takes nothing returns real
        return 115.
    endfunction
    
    // Returns the stun's max duration
    private constant function GetStunMaxDuration takes integer level returns integer
        return 5 * level
    endfunction
    
    // Returns the units that can be affected by the spell
    private function GetAffectedUnits takes unit caster, unit target returns boolean
        return IsUnitEnemy(target, GetOwningPlayer(caster)) and not(IsUnitType(target, UNIT_TYPE_DEAD)) and not(IsUnitType(target, UNIT_TYPE_STRUCTURE))
    endfunction
    
    //==========================================================================================//
    //                             CONFIGURABLE's END                                           //
    //==========================================================================================//
    
    globals
        private constant group G = CreateGroup()
    endglobals
    
    private struct Spell
        timer t                // Stun's timer
    
        unit caster            // Caster
        unit target            // Stun's target
        unit dummy             // Dummy's arrow
        
        integer level          // Spell's level
        
        real a                 // Dummy's angle
        
        integer cycles         // Arrow's distance -> To get the distance: cycles * 30 (30 is how much the arrow's dummy moves in every period)
        
        static thistype data   // For filter
        
        method destroy takes nothing returns nothing
            call this.stopPeriodic()
            
            // Killing the arrow's dummy
            call KillUnit(this.dummy)
            set this.dummy = null
            
            // If this.target is not null, means that we need
            // start another timer with the stun's duration.
            // So, when thistype.stun' run, we will remove
            // the stun to the target's unit
            if this.target != null then
                set this.t = NewTimer()
                call SetTimerData(this.t, this)
                call TimerStart(this.t, ((this.cycles * 30) / 150) * 0.5, false, function thistype.stun)
            else
                call PauseTimer(this.t)
                call ReleaseTimer(this.t)
                set this.t = null
                set this.caster = null
                set this.target = null
                
                call this.deallocate()
            endif
        endmethod
        
        static method stun takes nothing returns nothing
            local thistype this = GetTimerData(GetExpiredTimer())
            call UnitRemoveAbility(this.target, STUN_BUFF)
            set this.target = null
            call this.destroy()
        endmethod
        
        static method filter takes nothing returns boolean
            local unit u = GetFilterUnit()
            local unit dummy
            
            if GetAffectedUnits(data.caster, u) then
                // The stun can only stay for STUN_M_DURATION seconds
                if (((data.cycles * 30) / 150) * 0.5) > GetStunMaxDuration(data.level) then
                    set data.cycles = GetCycles() / 2
                endif
                
                set data.target = u
                
                // Applying stun to target's unit
                if IsUnitInvisible(u, GetOwningPlayer(u)) then
                    static if AFFECT_INVIS then
                        set dummy = CreateUnit(GetOwningPlayer(data.caster), DUMMY, GetUnitX(u), GetUnitY(u), 0)
                        
                        call UnitApplyTimedLife(dummy, 'BTLF', 1.)
                        call UnitAddAbility(dummy, STUN_RAWCODE)
                        call IssueTargetOrder(dummy, "thunderbolt", u)
                    endif
                else
                    set dummy = CreateUnit(GetOwningPlayer(data.caster), DUMMY, GetUnitX(u), GetUnitY(u), 0)
                        
                    call UnitApplyTimedLife(dummy, 'BTLF', 1.)
                    call UnitAddAbility(dummy, STUN_RAWCODE)
                    call IssueTargetOrder(dummy, "thunderbolt", u)
                endif
                
                // Making damage...
                call UnitDamageTarget(data.caster, u, GetDamage(data.level), false, true, ATK_TYPE, DMG_TYPE, null)
            endif
            
            set u = null
            set dummy = null
            
            return false
        endmethod
        
        private method periodic takes nothing returns nothing
            //local thistype this = GetTimerData(GetExpiredTimer())
            
            // Moving the dummy's 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                
                
                // If a unit is in the ARROW_DETECT range...
                if IsUnitInRangeXY(this.dummy, GetUnitX(this.dummy), GetUnitY(this.dummy), GetArrowDetect()) then
                    set data = this
                    
                    call GroupEnumUnitsInRange(G, GetUnitX(this.dummy), GetUnitY(this.dummy), GetArrowDetect(), Condition(function thistype.filter))
                    
                    set this.target = data.target
                    set this.cycles = data.cycles
                    
                    // If we've a target then...
                    if this.target != null then
                        // Invisible's check...
                        if AFFECT_INVIS then
                            set this.target = null
                        endif
                        
                        call this.destroy()
                    endif
                endif
            else
                call this.destroy()
            endif
        endmethod
        
        implement T32x
        
        static method create takes unit caster, real a, integer level returns thistype
            local thistype this = thistype.allocate()
            
            set this.caster = caster
            set this.dummy = CreateUnit(GetOwningPlayer(caster), DUMMY_ARROW, GetUnitX(caster), GetUnitY(caster), a *  bj_RADTODEG)
            set this.level = level
            set this.a = a
            set this.cycles = 0
            
            return this
        endmethod
    endstruct
    
    private function Conditions takes nothing returns boolean
        local unit caster
        local real x
        local real y
        local real a
        local integer level
        
        if (GetSpellAbilityId() == SPELL_RAWCODE) then
            set caster = GetTriggerUnit()
            set x = GetSpellTargetX()
            set y = GetSpellTargetY()
            
            // Angle for the dummy's arrow
            set a = Atan2(y - GetUnitY(caster), x - GetUnitX(caster))
            
            set level = GetUnitAbilityLevel(caster, SPELL_RAWCODE)
            call Spell.create(caster, a, level).startPeriodic()
        endif
        
        set caster = null
        
        return false
    endfunction

    private function Init takes nothing returns nothing
        local trigger t = CreateTrigger()
        local unit u
        
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, Condition(function Conditions))
        
        // Preloading ability and dummy unit
        set u = CreateUnit(Player(0), DUMMY, 0, 0, 0)
        call UnitAddAbility(u, STUN_RAWCODE)
        call RemoveUnit(u)
        
        set t = null
        set u = null
    endfunction
endlibrary

Greetings

------------

Edit: Uh, seems like I have a problem with MUI (only when I'm using T32) :/ (with TimerUtils works fine)

 

[Bribe]

what MUI problem is that?

Btw, you should move all the "conditions" actions into the "create" method of the struct, that way you don't create the same variables for 2 different functions.

 

[Ruke]

Mmm, when I cast multiple arrows (4) in some occasions, one stops.

Btw, you should move all the "conditions" actions into the "create" method of the struct, that way you don't create the same variables for 2 different functions.

Ok

 

[Bribe]

Hmm... looking at your code further, I am not sure what this is doing:

IsUnitInRangeXY(this.dummy, GetUnitX(this.dummy), GetUnitY(this.dummy)

Checking if the unit is within range of itself?

Also SetUnitX/Y instead of SetUnitPosition - the arrow does not need collision checks of course

 

[Ruke]

Hmm... looking at your code further, I am not sure what this is doing:

IsUnitInRangeXY(this.dummy, GetUnitX(this.dummy), GetUnitY(this.dummy)

Checking if the unit is within range of itself?

If a unit is in the range of the arrow

Also SetUnitX/Y instead of SetUnitPosition - the arrow does not need collision checks of course

Mmm, ok (but, for the map's limit?)

But, what will be the problem with T32? :S

 

[Bribe]

You are checking if the dummy is in range of the dummy. I am not sure what you were trying to do there but you could just be using IsUnitInRange, and put the dummy in the first two parameters, which makes more sense. But typically you'd just calculate the distance the moment the spell is cast and then subtract from the distance each iteration, until it reaches 0.

I have no idea what might be causing the MUI problem as I have not had such a problem myself using T32.

 

[Ruke]

Ah, you right, my mistake.

What I'm trying to do with that, was check if a unit is in the "GetArrowRange" range.
I will fix that (maybe is that the problem).

Thx again

-------------------

EDIT : Mmm, seems like I have problems with the filter method. How can I make it?

 

[Ruke]

Spell's updated. Now works properly with T32 ^^.

Greetings.

P.D : If you find a bug, please tell me and I will try fix it

 

[Bribe]

What did you change to make it work?

 

[Ruke]

Mmm, I just saw another spell (to know how to make the filter), and seems like the error was this:

this.target = data.target
this.cycles = data.cycles

weird no?

Greetings

 

[Ruke]

I have redesigned the spell, thx bribe .
-----------------

EDIT: Yes, I saw the SpellEffectEvent library, but it use another library, and two libraries more ... :\

Ok, I will change the timer for a local.
-----------------

Done

 

[Bribe]

SpellEffectEvent requires only RegisterPlayerUnitEvent, the "Table" library is optional (doesn't have to be included).

RegisterPlayerUnitEvent should be found in any vJass script that uses TriggerRegisterAnyUnitEventBJ, because of all the event/trigger handles it saves. SpellEffectEvent makes it so your function only runs if it's the right spell, that way you don't have to perform an if/else check yourself which saves development time and dynamic game run-time.

 

[Ruke]

Umm, yes I know that part, but, I need to add two more libraries to the spell (already has two), who makes 4 (5 if I add tables) libraries only for 1 spell.

If you want, I can add the library...
Greetings

 

[Bribe]

It really doesn't depend on approval to me as I already approved it, it's just that map makers should definitely be using those systems because it saves tons of handles, performance and map file size when there are many spells imported. Having them already required might be a deterrent if they don't have them imported, however it makes it easier for them if they do have them imported already. I recommend just doing whatever you think looks more appropriate for you.

 

[coolyoshi1]

@Ruke

Hey man seems there's a pretty serious bug.

Units can get permastunned if the ability is cast right next to the target.

call TimerStart(t, ((this.cycles * 30) / 150) * 0.5, false, function thistype.stun)

If cycles if less than 5 then the stun duration parameter will be set to 0 which means perma stun.

Also why did you multiply stun max duration by 5 * level? At level 4 this implies that the stun can last for 20 seconds.

private constant function GetStunMaxDuration takes integer level returns integer
        return 5 * level
endfunction

 

[Pyrogasm]

Actually it's < 4 because 120/150 = 1 in integer division. You can fix this error by changing 30 to 30.00 or 150 to 150.00. And honestly anything duration-related that's an integer... should be a real instead. Like the return value of that stun function you posted.

Sounds like this should be moved to substandard if not altered/updated.

 

[coolyoshi1]

Actually it's < 4 because 120/150 = 1 in integer division. You can fix this error by changing 30 to 30.00 or 150 to 150.00. And honestly anything duration-related that's an integer... should be a real instead. Like the return value of that stun function you posted.

Sounds like this should be moved to substandard if not altered/updated.

120/150 = 1? Interesting... anyways yeah this should probably be moved to substandard. Permastun is no joke

 

[Pyrogasm]

No I was totally wrong, and you're correct that it bugs with duration < 5. I think I thought I typed 150/120, which actually does = 1 in integer division. Int division truncates and never rounds:

1/10 = 0
9/10 = 0
11/10 = 1
199/100 = 1

//can fix it with periods so that each division involves at least one real:
1./10 = 0.10
9/10. = 0.90
11.00/10 = 1.10
199/100.00 = 1.99

 

[Pyrogasm]

Ironically I was directed back to this resource permastunning again 3 years later, and was assed to make a fix for it this time. Be warned that this resource has other issues and is generally suboptimally written... so I did not bother to fix anything besides the duration stuff and a really important thing about the enum group.

Here's kind of a pseudo-review and my explanation of my changes.

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:

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:

//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 group G = CreateGroup() //removed Constant keyword to avoid inline fuckery
    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