Elune's Arrow v1.3

Moderator · Oct 4, 2011

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 · Oct 4, 2011

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

Ruke · Oct 4, 2011

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

OH THX man, your comment is so useful :ogre_hurrhurr:

.VTZ. · Oct 4, 2011

lol what about the omnislash?

Bribe · Oct 5, 2011

JASS:

        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 · Oct 5, 2011

Bribe said:
JASS:

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 · Oct 5, 2011

Yeah Timer32 would be perfect for that period.

The code looks a lot better than most!

Ruke · Oct 5, 2011

Spell's updated

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

Ruke · Oct 5, 2011

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

JASS:

// 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 · Oct 5, 2011

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 · Oct 5, 2011

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 · Oct 5, 2011

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 · Oct 5, 2011

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 · Oct 5, 2011

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 · Oct 5, 2011

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 · Oct 9, 2011

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 · Oct 10, 2011

What did you change to make it work?

Ruke · Oct 10, 2011

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 · Nov 10, 2011

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 · Nov 10, 2011

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 · Nov 10, 2011

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 · Nov 11, 2011

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 · Mar 10, 2019

@Ruke

Hey man seems there's a pretty serious bug.

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

Code:

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.

Code:

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

Pyrogasm · Mar 10, 2019

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 · Mar 15, 2019

Pyrogasm said:
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 · Mar 18, 2019

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:

JASS:

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 · Aug 27, 2022

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.

Pyrogasm said:
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 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

Elune's Arrow v1.3

Elune's Arrow without Handle Vars (Map)

Moderator

Moderator

Similar threads