Need Assistance in Skill

[TTHero]

Greetings. I would like to ask for help. Below is the skill resembling DOTA's Venomancer first skill (maybe is called Venomous Gale, or something else), but there are some issues. First, the missile stops halfway and is not removed when casted quickly and continuously, and secondly, while the missile is stuck, next cast makes new missiles travel very fast. What is the problem in my scripting? Thank you and sorry for my inefficiency in scripting.

globals
    unit array sv_caster
    unit array sv_missile
    group array sv_group
    group array sv_tempgroup
    boolexpr sv_b
    trigger array sv_loop
    integer sv_i = 0
endglobals

function ShadowVenomSpellCheck takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A03A' ) ) then
        return false
    endif
    return true
endfunction

function ShadowVenomFilter takes nothing returns boolean
    return ( IsUnitType( GetFilterUnit(), UNIT_TYPE_STRUCTURE ) == false ) and ( IsUnitType( GetFilterUnit(), UNIT_TYPE_MAGIC_IMMUNE ) == false ) and ( IsUnitType( GetFilterUnit(), UNIT_TYPE_MECHANICAL ) == false ) and ( IsUnitEnemy( GetFilterUnit(), GetOwningPlayer(sv_caster[sv_i]) ) == true )
endfunction

function ShadowVenomLoop takes nothing returns nothing
    local location point1 = GetUnitLoc( sv_missile[sv_i] )
    local location point2
    local real x = GetLocationX(point1) + 33.00 * Cos( GetUnitFacing(sv_missile[sv_i]) * bj_DEGTORAD )
    local real y = GetLocationY(point1) + 33.00 * Sin( GetUnitFacing(sv_missile[sv_i]) * bj_DEGTORAD )
    local unit u
    local unit dummy
    set point2 = Location( x, y )
    call GroupEnumUnitsInRangeOfLoc( sv_group[sv_i], point1, 200.00, sv_b )
    loop
        set u = FirstOfGroup(sv_group[sv_i])
        exitwhen(u == null)
        call GroupRemoveUnit(sv_group[sv_i], u)
        if ( IsUnitInGroup(u, sv_tempgroup[sv_i]) == false ) then
            set dummy = CreateUnitAtLoc( GetOwningPlayer(sv_caster[sv_i]), 'n002', point1, bj_UNIT_FACING )
            call UnitAddAbility( dummy, 'A03X' )
            call SetUnitAbilityLevel( dummy, 'A03X', GetUnitAbilityLevel( sv_caster[sv_i], 'A03A') )
            call UnitApplyTimedLife( dummy, 'BTLF', 1.00 )
            call ShowUnit( dummy, false )
            call IssueTargetOrder( dummy, "shadowstrike", u )
            call GroupAddUnit(sv_tempgroup[sv_i], u)
        endif
    endloop
    call SetUnitPositionLoc( sv_missile[sv_i], point2 )
    call SetUnitUserData( sv_missile[sv_i], ( GetUnitUserData(sv_missile[sv_i]) - 1 ) )
    call RemoveLocation(point1)
    call RemoveLocation(point2)
    set point1 = null
    set point2 = null
    set u = null
    set dummy = null
    if ( GetUnitUserData(sv_missile[sv_i]) <= 0 ) then
        call KillUnit( sv_missile[sv_i] )
        call GroupClear( sv_group[sv_i] )
        call GroupClear( sv_tempgroup[sv_i] )
        call DestroyGroup( sv_group[sv_i] )
        call DestroyGroup( sv_tempgroup[sv_i] )
        call DestroyTrigger(sv_loop[sv_i])
        set sv_caster[sv_i] = null
        set sv_missile[sv_i] = null
        set sv_group[sv_i] = null
        set sv_tempgroup[sv_i] = null
        set sv_loop[sv_i] = null
    endif
endfunction

function ShadowVenomAction takes nothing returns nothing
    local location casloc
    set sv_i = sv_i + 1
    call StartSound(gg_snd_ShadowStrikeBirth1)
    call Skill_TextTag("Shadow Venom")
    set sv_caster[sv_i] = GetTriggerUnit()
    set sv_loop[sv_i] = CreateTrigger()
    call DisableTrigger(sv_loop[sv_i])
    set sv_group[sv_i] = CreateGroup()
    set sv_tempgroup[sv_i] = CreateGroup()
    set casloc = GetUnitLoc(sv_caster[sv_i])
    set sv_missile[sv_i] = CreateUnitAtLoc(GetOwningPlayer(sv_caster[sv_i]), 'n00M', casloc, GetUnitFacing(sv_caster[sv_i]))
    call UnitAddAbility(sv_missile[sv_i], 'Aloc')
    call SetUnitUserData(sv_missile[sv_i], R2I(750/33))
    call TriggerRegisterTimerEvent( sv_loop[sv_i], 0.03, true )
    call TriggerAddAction(sv_loop[sv_i], function ShadowVenomLoop)
    call EnableTrigger( sv_loop[sv_i] )
    call RemoveLocation(casloc)
    set casloc = null
endfunction

//===========================================================================
function InitTrig_Shadow_Venom takes nothing returns nothing
    local trigger ShadowVenom = CreateTrigger()
    
    call TriggerRegisterAnyUnitEventBJ( ShadowVenom, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( ShadowVenom, Condition(function ShadowVenomSpellCheck) )
    call TriggerAddAction( ShadowVenom, function ShadowVenomAction )
    
    set sv_b = Condition(function ShadowVenomFilter)
endfunction

 

[Cokemonkey11]

Your script is inherently broken because you're:

• Creating a trigger for every instance of the ability
• calling the loop function periodically for each of these triggers
• Referencing the newest trigger's pointer for all instances of trigger's action

What you should be doing is:

• Create a single timer for your whole system
• add an instance to a list to be executed by the loop after each timer expiration
• loop through all instance indices in the loop function

You have many other issues. You use locations, you null variables that don't need nulling, you use trigger actions when you don't use TSA or break the op-limit, your condition checks use 4+ lines when they need one, you're referencing static constant values in a non-recommendable manner, you use locations, you don't recycle array indices and thus you're using a circular array without knowing it = 5 arrays creating 12,104 allocations each

Edit:

My suggestion is to learn how to use structs in vJass and start over. You've done a lot of things right but too may things wrong as well.

 

[TTHero]

You mean I have to unlearn everything just to learn vJASS?

 

[Cokemonkey11]

No, you don't have to learn vJass at all. What you're trying to do is perfectly possible in JASS but vJass will help you turn this:

    unit array sv_caster
    unit array sv_missile
    group array sv_group
    group array sv_tempgroup
    boolexpr sv_b
    trigger array sv_loop
    integer sv_i = 0

into this:

struct instance
    unit caster
    unit missile
    group g
endstruct

globals
    instance array db
    integer dbIndex
    timer time
endglobals

take a look at this - http://www.wc3c.net/vexorian/jasshelpermanual.html#stru

 

[TTHero]

Your script is inherently broken because you're:

• Creating a trigger for every instance of the ability
• calling the loop function periodically for each of these triggers
• Referencing the newest trigger's pointer for all instances of trigger's action

What you should be doing is:

• Create a single timer for your whole system
• add an instance to a list to be executed by the loop after each timer expiration
• loop through all instance indices in the loop function

You have many other issues. You use locations, you null variables that don't need nulling, you use trigger actions when you don't use TSA or break the op-limit, your condition checks use 4+ lines when they need one, you're referencing static constant values in a non-recommendable manner, you use locations, you don't recycle array indices and thus you're using a circular array without knowing it = 5 arrays creating 12,104 allocations each

Edit:

My suggestion is to learn how to use structs in vJass and start over. You've done a lot of things right but too may things wrong as well.

Can you explain in simple terms?

 

[Cokemonkey11]

You're mixing up two design paradigms. By that I mean there are 2 sort of "ways" to do your spell in an MUI fashion.

You can either:

1. Use a separate trigger/timer for each "instance" of your spell
2. Use a single trigger/timer to handle all "instance" of your spell at once

In your action function you do this:

    local location casloc
    set sv_i = sv_i + 1
    set sv_caster[sv_i] = GetTriggerUnit()
    set sv_loop[sv_i] = CreateTrigger()
    set sv_group[sv_i] = CreateGroup()
    set sv_tempgroup[sv_i] = CreateGroup()
    set casloc = GetUnitLoc(sv_caster[sv_i])
    set sv_missile[sv_i] = CreateUnitAtLoc(GetOwningPlayer(sv_caster[sv_i]), 'n00M', casloc, GetUnitFacing(sv_caster[sv_i]))
    call UnitAddAbility(sv_missile[sv_i], 'Aloc')
    call SetUnitUserData(sv_missile[sv_i], R2I(750/33))
    call TriggerRegisterTimerEvent( sv_loop[sv_i], 0.03, true )
    call TriggerAddAction(sv_loop[sv_i], function ShadowVenomLoop)

So in theory what you've done is:

• Increment a stack pointer variable
• Create a trigger and register its contents with reference to the stack pointer
• Start the trigger, periodically running loop function

then in your loop function you've done this:

    call SetUnitPositionLoc( sv_missile[sv_i], point2 )
    call SetUnitUserData( sv_missile[sv_i], ( GetUnitUserData(sv_missile[sv_i]) - 1 ) )
    if ( GetUnitUserData(sv_missile[sv_i]) <= 0 ) then
        call KillUnit( sv_missile[sv_i] )
        call GroupClear( sv_group[sv_i] )
        call GroupClear( sv_tempgroup[sv_i] )
        call DestroyGroup( sv_group[sv_i] )
        call DestroyGroup( sv_tempgroup[sv_i] )
        call DestroyTrigger(sv_loop[sv_i])
    endif

So in theory what you've done is:

• Move the missile at the stack index to the next position
• Decrement the missile at the stack index's distance to target value
• Conditionally destroy the missile at the stack index

There are two ways you can fix this. You can either:

• Loop through all members of the stack (for each sv_missile from 0 to sv_i) and prevent the function from running with respect to multiple triggers OR:
• Reference a specific stack cell the trigger/timer so that each iteration of the looping function applies to a specific instance rather than the LAST instance (sv_i)

 

[TTHero]

I still do not get it. Sorry.

I cannot catch a single meaning of it. I want another simple explanation.

 

[Cokemonkey11]

I don't know what else to say other than you just did it wrong.

Maybe a working example of a similar ability will help you?

scope wildCards initializer i
    private struct cardDat
        unit caster
        unit array cards[3]
        real array gradX[3]
        real array gradY[3]
        real damage
        real traveled=0.
        group struckUnits
    endstruct
    
    private struct cdrDat
        unit fate
    endstruct
    
    globals
        private constant real OFFSET_ANGLE=30.*bj_DEGTORAD
        private constant real PROJECTILE_SPEED=700.
        private constant real ENUM_RADIUS=125./2.
        private constant real TOTAL_DISTANCE=1450.
        private constant real COOLDOWN=6.
        private cardDat array datDB
        private integer dbIndex=-1
        private timer time=CreateTimer()
        private group grp=CreateGroup()
        private HandleTable cdrTable
    endglobals
    
    private function setCardRandHue takes unit u returns nothing
        local integer i=GetRandomInt(0,2)
        if i==0 then
            call SetUnitVertexColor(u,255,55,55,255)
        elseif i==1 then
            call SetUnitVertexColor(u,255,245,55,255)
        else
            call SetUnitVertexColor(u,80,80,255,255)
        endif
    endfunction
    
    private function p takes nothing returns nothing
        local integer index=0
        local integer tripleLooper
        local cardDat tempDat
        local real uX
        local real uY
        local unit FoG
        loop
            exitwhen index>dbIndex
            set tempDat=datDB[index]
            set tripleLooper=0
            loop
                exitwhen tripleLooper>2
                set uX=GetUnitX(tempDat.cards[tripleLooper])+tempDat.gradX[tripleLooper]
                set uY=GetUnitY(tempDat.cards[tripleLooper])+tempDat.gradY[tripleLooper]
                call SetUnitX(tempDat.cards[tripleLooper],uX)
                call SetUnitY(tempDat.cards[tripleLooper],uY)
                call GroupEnumUnitsInRange(grp,uX,uY,ENUM_RADIUS,null)
                loop
                    set FoG=FirstOfGroup(grp)
                    exitwhen FoG==null
                    if IsUnitEnemy(FoG,GetOwningPlayer(tempDat.caster)) and IsUnitType(FoG,UNIT_TYPE_DEAD)==false and IsUnitType(FoG,UNIT_TYPE_STRUCTURE)==false and IsUnitInGroup(FoG,tempDat.struckUnits)==false then
                        call DamageType_dealCodeDamage(tempDat.caster,FoG,tempDat.damage,DAMAGE_TYPE_MAGIC)
                        call GroupAddUnit(tempDat.struckUnits,FoG)
                    endif
                    call GroupRemoveUnit(grp,FoG)
                endloop
                set tripleLooper=tripleLooper+1
            endloop
            set tempDat.traveled=tempDat.traveled+PROJECTILE_SPEED*tfConsts_FPS
            if tempDat.traveled>=TOTAL_DISTANCE then
                call RemoveUnit(tempDat.cards[0])
                call RemoveUnit(tempDat.cards[1])
                call RemoveUnit(tempDat.cards[2])
                call DestroyGroup(tempDat.struckUnits)
                call tempDat.destroy()
                set datDB[index]=datDB[dbIndex]
                set index=index-1
                set dbIndex=dbIndex-1
                if dbIndex==-1 then
                    call PauseTimer(time)
                endif
            endif
            set index=index+1
        endloop
    endfunction
    
    private function cdr takes nothing returns nothing
        local timer clock=GetExpiredTimer()
        local cdrDat tempDat=cdrTable[clock]
        call UnitRemoveAbility(tempDat.fate,tfConsts_WILD_CARDS_ID)
        call UnitAddAbility(tempDat.fate,tfConsts_WILD_CARDS_ID)
        call tempDat.destroy()
        call cdrTable.flush(clock)
        call DestroyTimer(clock)
        set clock=null
    endfunction
    
    private function c takes nothing returns boolean
        local cardDat tempDat
        local cdrDat ridiculousUseOfStruct
        local player owner
        local unit tU
        local real tX
        local real tY
        local real tUX
        local real tUY
        local real angle
        local timer cdrClock
        if GetSpellAbilityId()==tfConsts_WILD_CARDS_ID then
            set tU=GetTriggerUnit()
            set owner=GetOwningPlayer(tU)
            set tX=GetSpellTargetX()
            set tY=GetSpellTargetY()
            set tUX=GetUnitX(tU)
            set tUY=GetUnitY(tU)
            set angle=Atan2(tY-tUY,tX-tUX)
            set tempDat=cardDat.create()
            set tempDat.cards[0]=CreateUnit(owner,tfConsts_CARD_DUMMY_ID,0.,0.,(angle-OFFSET_ANGLE)*bj_RADTODEG)
            set tempDat.cards[1]=CreateUnit(owner,tfConsts_CARD_DUMMY_ID,0.,0.,angle*bj_RADTODEG)
            set tempDat.cards[2]=CreateUnit(owner,tfConsts_CARD_DUMMY_ID,0.,0.,(angle+OFFSET_ANGLE)*bj_RADTODEG)
            call setCardRandHue(tempDat.cards[0])
            call setCardRandHue(tempDat.cards[1])
            call setCardRandHue(tempDat.cards[2])
            call SetUnitX(tempDat.cards[0],tUX)
            call SetUnitX(tempDat.cards[1],tUX)
            call SetUnitX(tempDat.cards[2],tUX)
            call SetUnitY(tempDat.cards[0],tUY)
            call SetUnitY(tempDat.cards[1],tUY)
            call SetUnitY(tempDat.cards[2],tUY)
            set tempDat.gradX[0]=PROJECTILE_SPEED*tfConsts_FPS*Cos(angle-OFFSET_ANGLE)
            set tempDat.gradX[1]=PROJECTILE_SPEED*tfConsts_FPS*Cos(angle)
            set tempDat.gradX[2]=PROJECTILE_SPEED*tfConsts_FPS*Cos(angle+OFFSET_ANGLE)
            set tempDat.gradY[0]=PROJECTILE_SPEED*tfConsts_FPS*Sin(angle-OFFSET_ANGLE)
            set tempDat.gradY[1]=PROJECTILE_SPEED*tfConsts_FPS*Sin(angle)
            set tempDat.gradY[2]=PROJECTILE_SPEED*tfConsts_FPS*Sin(angle+OFFSET_ANGLE)
            set tempDat.struckUnits=CreateGroup()
            set tempDat.caster=tU
            set tempDat.damage=10.+50.*GetUnitAbilityLevel(tU,tfConsts_WILD_CARDS_ID)+.65*GetHeroInt(tU,true)
            set dbIndex=dbIndex+1
            set datDB[dbIndex]=tempDat
            if dbIndex==0 then
                call TimerStart(time,tfConsts_FPS,true,function p)
            endif
            if GetUnitAbilityLevel(tempDat.caster,tfConsts_STACKED_DECK_ID)>0 then
                set cdrClock=CreateTimer()
                set ridiculousUseOfStruct=cdrDat.create()
                set ridiculousUseOfStruct.fate=tempDat.caster
                set cdrTable[cdrClock]=ridiculousUseOfStruct
                call TimerStart(cdrClock,COOLDOWN-COOLDOWN*0.03*GetUnitAbilityLevel(tempDat.caster,tfConsts_STACKED_DECK_ID),false,function cdr)
                set cdrClock=null
            endif
            set tU=null
        endif
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t,Condition(function c))
        set cdrTable=HandleTable.create()
        set t=null
    endfunction
endscope