• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

[JASS] My Spell is not MUI :(

Status
Not open for further replies.

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
When I make 2+ units use this spell, the timer gets destroyed before all of them have finished the spell. The condition is being met before it should.

JASS:
scope leapOfFaith initializer i
    struct dLeap
        integer steps
        location last
        unit caster
    endstruct
    
    globals
        private constant integer ciLeap='A058' //Triggering abilitycode
        private constant integer iSteps=80     //Total number of repeated timer steps
        private constant integer offset=22     //Distance in game units a unit is offset by
        private constant integer reducer=2     //Ammount steps is reduced by each time
        private constant real zMultiplier=.5   //Multiplier for caster z height; larger is a taller arc
        private dLeap array leapDB
        private integer dbIndex=0
        private timer time
    endglobals
    
    private function c takes nothing returns boolean
        return GetSpellAbilityId()==ciLeap
    endfunction
    
    private function p takes nothing returns nothing
        local integer index=0
        local dLeap tempDat
        local location lOffset
        local real z1
        local real z2
        local effect fx
        local integer iGrabbed=dbIndex
        loop
            exitwhen index>=iGrabbed
            set tempDat=leapDB[index]
            set lOffset=PolarProjectionBJ(GetUnitLoc(tempDat.caster),offset,GetUnitFacing(tempDat.caster))
            set z1=I2R(tempDat.steps/5)
            set z2=I2R(tempDat.steps)/5
            if z1==z2 then
                set fx=AddSpecialEffectLoc("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl",lOffset)
                call DestroyEffect(fx)
            endif
            set tempDat.steps=tempDat.steps-reducer
            call SetUnitFlyHeight(tempDat.caster,GetUnitFlyHeight(tempDat.caster)+zMultiplier*tempDat.steps,0)
            call SetUnitPositionLoc(tempDat.caster,lOffset)
            if IsTerrainPathable(GetLocationX(lOffset),GetLocationY(lOffset),PATHING_TYPE_FLOATABILITY)==false then
                set tempDat.last=lOffset
            endif
            if tempDat.steps<-1*iSteps then
                call SetUnitPathing(tempDat.caster,true)
                set leapDB[dbIndex]=leapDB[index]
                set dbIndex=dbIndex-1
                call SetUnitFlyHeight(tempDat.caster,0,0)
                if IsTerrainPathable(GetLocationX(lOffset),GetLocationY(lOffset),PATHING_TYPE_FLOATABILITY) then //WHY IS THIS BACKWARDS!?
                    call SetUnitPositionLoc(tempDat.caster,tempDat.last)
                    call SetUnitState(tempDat.caster,UNIT_STATE_LIFE,GetUnitState(tempDat.caster,UNIT_STATE_LIFE)/2+1)
                endif
            endif
            set index=index+1
        endloop
        if dbIndex==0 then
            call DestroyTimer(time)
            call BJDebugMsg("timer destroyed")
        endif
        call RemoveLocation(lOffset)
        //call tempDat.destroy() //? Do I need this?
    endfunction
    
    private function a takes nothing returns nothing
        local dLeap tempDat=dLeap.create()
        local unit caster=GetSpellAbilityUnit()
        local real rCasterX=GetUnitX(caster)
        local real rCasterY=GetUnitY(caster)
        call SetSoundPosition(gg_snd_jumpSoundDefendCasterWav,rCasterX,rCasterY,100)
        call StartSound(gg_snd_jumpSoundDefendCasterWav)
        call UnitAddAbility(caster,'Amrf')
        call UnitRemoveAbility(caster,'Amrf')
        call SetUnitPathing(caster,false)
        set tempDat.steps=iSteps
        set tempDat.caster=caster
        set leapDB[dbIndex]=tempDat
        set dbIndex=dbIndex+1
        if dbIndex==1 then
            set time=CreateTimer()
            call TimerStart(time,.03,true,function p)
        endif
    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))
        call TriggerAddAction(t,function a)
    endfunction
endscope
 
Oh, sorry, it was really stupid xD

Hmm... The only thing I find wrong is the line set leapDB[dbIndex]=leapDB[index]

Shouldn't it be the opposite: set leapDB[Index]=leapDB[dbindex]?

Because leapDB[index] is in the middle of the array; leapDB[dbindex] is the last used instance of the array.
Therefore, when an instance is done, you should replace it with the last one.

The way it is now, you will be nulling the last instance too - therefore, dbindex will decrease twice everytime an instance finishes moving... Or something like that xD
 
Level 3
Joined
Aug 9, 2008
Messages
60
Here is your code fixed:
JASS:
scope leapOfFaith initializer i
    globals
        private constant integer leap='A058' //Triggering abilitycode
        private constant integer steps=80 //Total number of repeated timer steps
        private constant integer offset=22 //Distance in game units a unit is offset by
        private constant real zMultiplier=.5 //Multiplier for caster z height; larger is a taller arc
        private timer time
    endglobals
    
    private struct spellinfo
        integer  steps  = 0
        real         x  = 0
        real         y  = 0
        unit     caster = null
        
        static method create takes unit u, real x, real y returns spellinfo
            local spellinfo this = spellinfo.allocate()
            set this.steps  = steps
            set this.caster = u
            set this.x = GetUnitX(u)
            set this.y = GetUnitY(u)
            return this
        endmethod
        
        private method onDestroy takes nothing returns nothing
            set .caster = null
        endmethod
    endstruct

    private function c takes nothing returns boolean
        return GetSpellAbilityId() == leap
    endfunction

    private function p takes nothing returns nothing
        local spellinfo data = GetTimerData(GetExpiredTimer())
        set data.steps = data.steps - 1
        if data.steps < 0 then
            call SetUnitPathing(data.caster,true)
            call SetUnitFlyHeight(data.caster,0,0)
            if IsTerrainPathable(data.x,data.y,PATHING_TYPE_FLOATABILITY) then
                set data.x = data.x - offset*Cos(GetUnitFacing(data.caster)*bj_DEGTORAD)
                set data.y = data.y - offset*Sin(GetUnitFacing(data.caster)*bj_DEGTORAD)
                call SetUnitX(data.caster, data.x)
                call SetUnitY(data.caster, data.y)
                call SetUnitState(data.caster,UNIT_STATE_LIFE,GetUnitState(data.caster,UNIT_STATE_LIFE)/2+1)
            endif
            call ReleaseTimer(GetExpiredTimer())
            call data.destroy()
        endif
        set data.x = data.x + offset*Cos(GetUnitFacing(data.caster)*bj_DEGTORAD)
        set data.y = data.y + offset*Sin(GetUnitFacing(data.caster)*bj_DEGTORAD)
        if I2R(data.steps/5)==I2R(data.steps)/5 then
            call DestroyEffect(AddSpecialEffect("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl", data.x, data.y))
        endif
        call SetUnitFlyHeight(data.caster,GetUnitFlyHeight(data.caster)+zMultiplier*data.steps,0)
        if IsTerrainPathable(data.x,data.y,PATHING_TYPE_FLOATABILITY)==true then
            call SetUnitX(data.caster, data.x)
            call SetUnitY(data.caster, data.y)
        else
            set data.x = data.x - offset*Cos(GetUnitFacing(data.caster)*bj_DEGTORAD)
            set data.y = data.y - offset*Sin(GetUnitFacing(data.caster)*bj_DEGTORAD)
        endif
    endfunction

    private function a takes nothing returns nothing
        local unit caster=GetSpellAbilityUnit()
        local spellinfo data = spellinfo.create(caster,GetUnitX(caster),GetUnitY(caster))
        local timer t = NewTimer()
        call SetTimerData(t, data)
        call SetSoundPosition(gg_snd_jumpSoundDefendCasterWav,GetUnitX(caster),GetUnitY(caster),100)
        call StartSound(gg_snd_jumpSoundDefendCasterWav)
        call UnitAddAbility(caster,'Amrf')
        call UnitRemoveAbility(caster,'Amrf')
        call SetUnitPathing(caster,false)
        call TimerStart( t, 0.03, true, function p)
    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))
        call TriggerAddAction(t,function a)
    endfunction
endscope

To use it, you will need a new library, place this in an empty trigger in your map.
JASS:
library TimerAttach initializer Init
globals
    private integer array data
    private timer array t
    private integer n = 0
endglobals

function H2I takes handle h returns integer
    return h
    return 0
endfunction


function SetTimerData takes timer tt, integer d returns nothing
    local integer offset = H2I(t[0])
    set data[H2I(tt) - offset] = d
endfunction

function GetTimerData takes timer tt returns integer
    local integer offset = H2I(t[0])
    return data[H2I(tt) - offset]
endfunction

function NewTimer takes nothing returns timer
    if n == 0 then
        debug call BJDebugMsg("You're using too many timers, create more timers on initialization!")
        return null
    endif
    set n = n - 1
    call SetTimerData(t[n], 0)
    return t[n]
endfunction

function ReleaseTimer takes timer ti returns nothing
    if GetTimerData(ti) == 0x69696969 then
        debug call BJDebugMsg("Trying to release an empty timer!")
        return
    endif
    call PauseTimer(ti)
    call SetTimerData(ti, 0x69696969)
    set t[n] = ti
    set n = n + 1
endfunction


// If you feel like not wasting any memory, use powers of 2 for amount
// of timers, so your i should be something like 63, 127, 255, 511...
private function Init takes nothing returns nothing
    local integer i = 0
    loop
        exitwhen i == 127
        set t[i] = CreateTimer()
        set i=i+1
    endloop
    set n = i
endfunction

endlibrary

Just remember to released unused timers, instead of destroying them! And only use this timer system when you need to attach structs to timers, because it's slower than just creating a normal timer if you don't need to attach anything to it.
 
Level 3
Joined
Aug 9, 2008
Messages
60
I have updated my system to the latest version as well. By the way, I don't get why you're being so rude to me, I completely changed your trigger to improve it dramatically, and coordinates are far better than locations btw.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
Sorry if I came off as seeming rude, it wasn't intended. I felt like you spent a lot of time to make a working trigger that's slower and using outdated libs and functions.

I'm using locations so that players can still send turn commands without letting the unit move.

Your script will not work because return h; return 0 is being fixed when the next 1.23 patch is released.

I'm also pretty frustrated that I can't figure it out myself.
 
Level 3
Joined
Aug 9, 2008
Messages
60
Sorry if I came off as seeming rude, it wasn't intended. I felt like you spent a lot of time to make a working trigger that's slower and using outdated libs and functions.

I'm using locations so that players can still send turn commands without letting the unit move.

Your script will not work because return h; return 0 is being fixed when the next 1.23 patch is released.

I'm also pretty frustrated that I can't figure it out myself.

It's faster than yours, you use a lot of useless calls and variables. And blizzard placed a new function that replaces H2I on the version, as I stated, my system is already updated to 1.23, I didn't post it because it's not usable yet, only on the new patch. You can also set the units movement speed to 0, which will disable him running, he'll be able to turn much better than the other method as well.
 
Level 3
Joined
Aug 9, 2008
Messages
60
If there's a native for that, I'm pretty sure there is. And you're using arrays as well, if you want to you can convert the spell I made into one timer, but attaching structs to timers is one of the best ways of retaining data inside of a spell that uses a timer.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
This is what I made before I converted it to vJass. If I didn't want to use a struct stack I would have literally done nothing.

JASS:
globals
    real test1
    real test2
endglobals

function interceptorLeapOfFaithC takes nothing returns boolean
    return GetSpellAbilityId()=='A058'
endfunction

function interceptorLeapOfFaithP takes nothing returns nothing
    local timer time=GetExpiredTimer()
    local integer steps=GetHandleInt(time,"steps")
    local unit caster=GetHandleUnit(time,"unit")
    local location off=PolarProjectionBJ(GetUnitLoc(caster),26,GetUnitFacing(caster))
    local real zCas=GetLocationZ(GetUnitLoc(caster))
    local real zOff=GetLocationZ(off)
    set test1=I2R(steps/5)
    set test2=I2R(steps)/5
    if test1==test2 then
        set bj_lastCreatedEffect=AddSpecialEffectLoc("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl",off)
        call DestroyEffect(bj_lastCreatedEffect)
    endif
    call SetHandleInt(time,"steps",steps-2)
    call SetUnitFlyHeight(caster,GetUnitFlyHeight(caster)+.5*steps,0)
    if zOff-zCas<10 then
        call SetUnitPositionLoc(caster,off)
    endif
    if steps<-40 then
        call FlushHandleLocals(time)
        call DestroyTimer(time)
    endif
endfunction

function interceptorLeapOfFaithA takes nothing returns nothing
    local integer steps=40
    local timer time=CreateTimer()
    local unit caster=GetSpellAbilityUnit()
    call StartSound(gg_snd_jumpSoundDefendCasterWav)
    call UnitAddAbility(caster,'Amrf')
    call SetHandleInt(time,"steps",steps)
    call SetHandleHandle(time,"unit",caster)
    call TimerStart(time,.03,true,function interceptorLeapOfFaithP)
    call TriggerSleepAction(1.1)
    call SetUnitFlyHeight(caster,0,0)
    call UnitRemoveAbility(caster,'Amrf')
endfunction

function InitTrig_interceptorLeapOfFaith takes nothing returns nothing
    set gg_trg_interceptorLeapOfFaith=CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(gg_trg_interceptorLeapOfFaith,EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(gg_trg_interceptorLeapOfFaith,Condition(function interceptorLeapOfFaithC))
    call TriggerAddAction(gg_trg_interceptorLeapOfFaith,function interceptorLeapOfFaithA)
endfunction
 
Level 11
Joined
Feb 22, 2006
Messages
752
First off, I'd just like to say I like how I have to copy the code into WORD b4 pasting it into here...or else it all comes in one line....seriously wtf.

JASS:
scope leapOfFaith initializer i
    struct dLeap
        integer steps
        location last
        unit caster
        boolean toDestroy = false
        private method onDestroy takes nothing returns nothing
            call SetUnitPathing(.caster,true)
            call SetUnitFlyHeight(.caster,0,0)
            if IsTerrainPathable(GetLocationX(.last),GetLocationY(.last),PATHING_TYPE_FLOATABILITY) then //WHY IS THIS BACKWARDS!?
                call SetUnitPositionLoc(.caster,.last)
                call SetUnitState(.caster,UNIT_STATE_LIFE,GetUnitState(.caster,UNIT_STATE_LIFE)/2+1)
            endif
            call RemoveLocation(.last)
        endmethod
    endstruct

    globals
        private constant integer ciLeap='A058' //Triggering abilitycode
        private constant integer iSteps=80     //Total number of repeated timer steps
        private constant integer offset=22     //Distance in game units a unit is offset by
        private constant integer reducer=2     //Ammount steps is reduced by each time
        private constant real zMultiplier=.5   //Multiplier for caster z height; larger is a taller arc
        private dLeap array leapDB
        private integer dbIndex=0
        private timer time
    endglobals

    private function c takes nothing returns boolean
        return GetSpellAbilityId()==ciLeap
    endfunction

    private function p takes nothing returns nothing
        local integer index=0
        local dLeap tempDat
        local location lOffset
        local real z1
        local real z2
        
        loop
            exitwhen index>=iGrabbed
            set tempDat=leapDB[index]
            if (tempDat.toDestroy) then
                set leapDB[index] = leapDB[dbIndex - 1]
                call tempDat.destroy()
                set dbIndex = dbIndex - 1
                set index = index - 1
                if (dbIndex == 0) then
                    call PauseTimer(time)
                endif
            else
                set lOffset=PolarProjectionBJ(GetUnitLoc(tempDat.caster),offset,GetUnitFacing(tempDat.caster))
                set z1=I2R(tempDat.steps/5)
                set z2=I2R(tempDat.steps)/5
                if z1==z2 then
                    call DestroyEffect(AddSpecialEffectLoc("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl",lOffset))
                endif
                set tempDat.steps=tempDat.steps-reducer
                call SetUnitFlyHeight(tempDat.caster,GetUnitFlyHeight(tempDat.caster)+zMultiplier*tempDat.steps,0)
                call SetUnitPositionLoc(tempDat.caster,lOffset)
                if IsTerrainPathable(GetLocationX(lOffset),GetLocationY(lOffset),PATHING_TYPE_FLOATABILITY)==false then
                    set tempDat.last=lOffset
                endif
                if tempDat.steps<-1*iSteps then
                    set tempDat.toDestroy = true
                endif
            endif
            set index=index+1
        endloop
        call RemoveLocation(lOffset)
        //call tempDat.destroy() //? Do I need this?
    endfunction

    private function a takes nothing returns nothing
        local dLeap tempDat=dLeap.create()
        local unit caster=GetSpellAbilityUnit()
        local real rCasterX=GetUnitX(caster)
        local real rCasterY=GetUnitY(caster)
        call SetSoundPosition(gg_snd_jumpSoundDefendCasterWav,rCasterX,rCasterY,100)
        call StartSound(gg_snd_jumpSoundDefendCasterWav)
        call UnitAddAbility(caster,'Amrf')
        call UnitRemoveAbility(caster,'Amrf')
        call SetUnitPathing(caster,false)
        set tempDat.steps=iSteps
        set tempDat.caster=caster
        set leapDB[dbIndex]=tempDat
        set dbIndex=dbIndex+1
        if dbIndex==1 then
            call TimerStart(time,.03,true,function p)
        endif
    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))
        call TriggerAddAction(t,function a)
        set time = CreateTimer()
    endfunction
endscope

This should fix your problem. Still don't know why you're using locations, personally I would use reals but I didn't change that.

By the way, constant variable identifiers are in all caps separated by underscores by convention (not just in JASS, but in pretty much every other programming/scripting language).
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
Thanks ricepuff.

Unfortunately I fixed it right before you posted that. Here it is:

JASS:
scope leapOfFaith initializer i
    private struct dLeap
        integer steps
        location last
        unit caster
    endstruct
    
    globals
        private constant integer ciLeap='A058' //Triggering abilitycode
        private constant integer iSteps=80     //Total number of repeated timer steps
        private constant integer offset=22     //Distance in game units a unit is offset by
        private constant integer reducer=2     //Ammount steps is reduced by each time
        private constant real zMultiplier=.5   //Multiplier for caster z height; larger is a taller arc
        private dLeap array leapDB
        private integer dbIndex=-1
        private timer time
    endglobals
    
    private function c takes nothing returns boolean
        return GetSpellAbilityId()==ciLeap
    endfunction
    
    private function p takes nothing returns nothing
        local integer index=0
        local dLeap tempDat
        local location lOffset
        local real z1
        local real z2
        local effect fx
        local integer iGrabbed=dbIndex
        loop
            exitwhen index>iGrabbed
            set tempDat=leapDB[index]
            set lOffset=PolarProjectionBJ(GetUnitLoc(tempDat.caster),offset,GetUnitFacing(tempDat.caster))
            set z1=I2R(tempDat.steps/5) //z heights being compared
            set z2=I2R(tempDat.steps)/5 //Should just use mod...
            if z1==z2 then              //This works though.
                set fx=AddSpecialEffectLoc("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl",lOffset)
                call DestroyEffect(fx)
            endif
            set tempDat.steps=tempDat.steps-reducer
            call SetUnitFlyHeight(tempDat.caster,GetUnitFlyHeight(tempDat.caster)+zMultiplier*tempDat.steps,0)
            call SetUnitPositionLoc(tempDat.caster,lOffset)
            if IsTerrainPathable(GetLocationX(lOffset),GetLocationY(lOffset),PATHING_TYPE_FLOATABILITY)==false then
                set tempDat.last=lOffset
            endif
            if tempDat.steps<-1*iSteps then
                call SetUnitPathing(tempDat.caster,true)
                call tempDat.destroy()
                set leapDB[index]=leapDB[dbIndex]
                set dbIndex=dbIndex-1
                call SetUnitFlyHeight(tempDat.caster,0,0)
                if dbIndex==-1 then
                    call DestroyTimer(time)
                endif
                if IsTerrainPathable(GetLocationX(lOffset),GetLocationY(lOffset),PATHING_TYPE_FLOATABILITY) then //WHY IS THIS BACKWARDS!?
                    call SetUnitPositionLoc(tempDat.caster,tempDat.last)
                    call SetUnitState(tempDat.caster,UNIT_STATE_LIFE,GetUnitState(tempDat.caster,UNIT_STATE_LIFE)/2+1)
                endif
            endif
            set index=index+1
        endloop
        call RemoveLocation(lOffset)
    endfunction
    
    private function a takes nothing returns nothing
        local dLeap tempDat=dLeap.create()
        local unit caster=GetSpellAbilityUnit()
        local real rCasterX=GetUnitX(caster)
        local real rCasterY=GetUnitY(caster)
        call SetSoundPosition(gg_snd_jumpSoundDefendCasterWav,rCasterX,rCasterY,100)
        call StartSound(gg_snd_jumpSoundDefendCasterWav)
        call UnitAddAbility(caster,'Amrf')
        call UnitRemoveAbility(caster,'Amrf')
        call SetUnitPathing(caster,false)
        set tempDat.steps=iSteps
        set tempDat.caster=caster
        if dbIndex==-1 then //While negative 1, the stack is empty. An empty stack must be jumpstarted!
            set time=CreateTimer()
            call TimerStart(time,.03,true,function p)
        endif
        set dbIndex=dbIndex+1
        set leapDB[dbIndex]=tempDat
    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))
        call TriggerAddAction(t,function a)
    endfunction
endscope

And I took Ferocity's suggestion and it turns out the turn rate and ms natives work fine, here's the code I created using reals instead of locations. Unfortunately it doesn't work. >.<

JASS:
scope leapOfFaith initializer i
    private struct dLeap
        integer steps
        real xLast
        real yLast
        unit caster
    endstruct
    
    globals
        private constant integer ciLeap='A058' //Triggering abilitycode
        private constant integer iSteps=80     //Total number of repeated timer steps
        private constant integer offset=22     //Distance in game units a unit is offset by
        private constant integer reducer=2     //Ammount steps is reduced by each time
        private constant real zMultiplier=.5   //Multiplier for caster z height; larger is a taller arc
        private dLeap array leapDB
        private integer dbIndex=-1
        private timer time
    endglobals
    
    private function c takes nothing returns boolean
        return GetSpellAbilityId()==ciLeap
    endfunction
    
    private function p takes nothing returns nothing
        local integer index=0
        local dLeap tempDat
        local real xLoc
        local real yLoc
        local real z1
        local real z2
        local effect fx
        loop
            exitwhen index>dbIndex
            set tempDat=leapDB[index]
            set z1=I2R(tempDat.steps/5) //z heights being compared
            set z2=I2R(tempDat.steps)/5 //Should just use mod...
            if z1==z2 then              //This works though.
                set fx=AddSpecialEffect("Abilities\\Weapons\\GlaiveMissile\\GlaiveMissileTarget.mdl",xLoc,yLoc)
                call DestroyEffect(fx)
            endif
            set tempDat.steps=tempDat.steps-reducer
            call BJDebugMsg(I2S(tempDat.steps)+" steps")
            call SetUnitFlyHeight(tempDat.caster,GetUnitFlyHeight(tempDat.caster)+zMultiplier*tempDat.steps,0)
            set xLoc=GetUnitX(tempDat.caster)+offset*Cos(GetUnitFacing(tempDat.caster)*bj_DEGTORAD)
            set yLoc=GetUnitY(tempDat.caster)+offset*Sin(GetUnitFacing(tempDat.caster)*bj_DEGTORAD)
            call SetUnitX(tempDat.caster,xLoc)
            call SetUnitY(tempDat.caster,yLoc)
            if IsTerrainPathable(xLoc,yLoc,PATHING_TYPE_FLOATABILITY)==false then
                set tempDat.xLast=xLoc
                set tempDat.yLast=yLoc
            endif
            if tempDat.steps<-1*iSteps then
                call SetUnitPathing(tempDat.caster,true)
                call SetUnitTurnSpeed(tempDat.caster,GetUnitDefaultTurnSpeed(tempDat.caster))
                call SetUnitMoveSpeed(tempDat.caster,GetUnitDefaultMoveSpeed(tempDat.caster))
                call tempDat.destroy()
                set leapDB[index]=leapDB[dbIndex]
                set dbIndex=dbIndex-1
                call SetUnitFlyHeight(tempDat.caster,0,0)
                if dbIndex==-1 then
                    call DestroyTimer(time)
                endif
                if IsTerrainPathable(xLoc,yLoc,PATHING_TYPE_FLOATABILITY) then //WHY IS THIS BACKWARDS!?
                    call SetUnitX(tempDat.caster,tempDat.xLast)
                    call SetUnitY(tempDat.caster,tempDat.yLast)
                    call SetUnitState(tempDat.caster,UNIT_STATE_LIFE,(GetUnitState(tempDat.caster,UNIT_STATE_LIFE)/2)+1)
                endif
            endif
            set index=index+1
        endloop
    endfunction
    
    private function a takes nothing returns nothing
        local dLeap tempDat=dLeap.create()
        local unit caster=GetSpellAbilityUnit()
        local real rCasterX=GetUnitX(caster)
        local real rCasterY=GetUnitY(caster)
        local real tSpeed=GetUnitDefaultTurnSpeed(caster)/8
        call SetSoundPosition(gg_snd_jumpSoundDefendCasterWav,rCasterX,rCasterY,100)
        call StartSound(gg_snd_jumpSoundDefendCasterWav)
        call UnitAddAbility(caster,'Amrf')
        call UnitRemoveAbility(caster,'Amrf')
        call SetUnitTurnSpeed(caster,tSpeed)
        call SetUnitMoveSpeed(caster,0)
        call SetUnitPathing(caster,false)
        set tempDat.steps=iSteps
        set tempDat.caster=caster
        if dbIndex==-1 then //While negative 1, the stack is empty. An empty stack must be jumpstarted!
            set time=CreateTimer()
            call TimerStart(time,.03,true,function p)
        endif
        set dbIndex=dbIndex+1
        set leapDB[dbIndex]=tempDat
    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))
        call TriggerAddAction(t,function a)
    endfunction
endscope
 
Level 11
Joined
Feb 22, 2006
Messages
752
Well, I wasn't talking about your system. And even timer recycling is not needed here. Think about it. Why would you go through the trouble of using a recycling/attaching system here when on every timeout, you have to loop through every struct in the array anyway? If it's going to take O(n) time no matter wut, then you might as well do it the most efficient way: global array loop.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
Lol, I don't care if you use my code or not. I'm just saying using a single timer is btr than destroying/creating timers. And btw, if you don't pause a timer before you destroy it, you will get bugs.

And can you be more specific about what exactly it is that doesnt work.

hmm, so you're saying that even though I declare my timer as global, destroying and creating new ones is just as bad? I thought the advantage was to have only 1 timer at a time (so if you have 10 units using abil, it's still only 1 timer)

And good to know on the pause thing.

What doesn't work is that the unit isn't moved periodically. I hear the sound and then it gets reduced turn rate and such, but none of the periodic actions seem to occur.

Edit: Are you saying it's better to let an empty timer run constantly than to just turn it off while the stack is empty and turn it on again later? That doesn't seem logical to me >.<
 
Level 11
Joined
Feb 22, 2006
Messages
752
No, you pause the timer when there are no structs left and start it when at least one exists.

JASS:
set dbIndex=dbIndex+1
set leapDB[dbIndex]=tempDat

Move those lines above where you start the timer. It probably DOESN'T matter but it's still not good habit to execute something before you've properly set it up.

And use GetTriggerUnit(), not GetSpellAbilityUnit().

And is there any indication that ANY of the actions in the timer callback are being executed? Does the unit eventually get its movement speed/turn rate restored?

EDIT: You need to add the line

JASS:
set index = index - 1

after you've destroyed a struct. Otherwise it will skip the struct that was just moved into the destroyed struct's index in the array (the one that was at the end of the array).

ANOTHER EDIT: You can't use a struct after you destroy it. So all those references to tempDat in function p after you call tempDat.destroy() won't work.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
No, you pause the timer when there are no structs left and start it when at least one exists.

JASS:
set dbIndex=dbIndex+1
set leapDB[dbIndex]=tempDat

Move those lines above where you start the timer. It probably DOESN'T matter but it's still not good habit to execute something before you've properly set it up.

I did that for readability sake since I'm trying to get in the mindset now that negative 1 is an empty stack (I had a professional programmer over that was talking to me about this stuff a little). I don't think changing that will make a difference.
And use GetTriggerUnit(), not GetSpellAbilityUnit().
I'd rather waste that .000001 seconds of difference so I can recognize the triggering unit as one casting the spell
And is there any indication that ANY of the actions in the timer callback are being executed? Does the unit eventually get its movement speed/turn rate restored?

It does not.
EDIT: You need to add the line

JASS:
set index = index - 1

after you've destroyed a struct. Otherwise it will skip the struct that was just moved into the destroyed struct's index in the array (the one that was at the end of the array).

Wouldn't matter since that would only change the outcome of that 1 particular instance of the loop (1 step, .03 seconds)
ANOTHER EDIT: You can't use a struct after you destroy it. So all those references to tempDat in function p after you call tempDat.destroy() won't work.

Nice catch, thanks for that.

None of these have actually fixed the problem though.

Again, thanks for looking through it for me. I'd rather understand why mine is bad than why yours is good.
 
Level 11
Joined
Feb 22, 2006
Messages
752
Wouldn't matter since that would only change the outcome of that 1 particular instance of the loop (1 step, .03 seconds)

Yea it does matter. The point of coding is that you will be able to safely predict what is going to happen when your code executes. Without that line, you cannot predict what will happen because behavior will be different depending on which struct is at that last index position. It's silly to have your code be indeterministic just because you don't want to add an integer decrement.

I did that for readability sake since I'm trying to get in the mindset now that negative 1 is an empty stack (I had a professional programmer over that was talking to me about this stuff a little). I don't think changing that will make a difference.

Um, in pretty much all languages, you use array sizes, i.e. how many indices exist, to determine how "large" your array is, not the largest index number that points to a non-null (non-zero) element in the array.

Yea, I can't figure out why none of the code in your timer is executing. Try putting debug messages in there at multiple locations to see what runs and what doesnt.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,522
Yea it does matter. The point of coding is that you will be able to safely predict what is going to happen when your code executes. Without that line, you cannot predict what will happen because behavior will be different depending on which struct is at that last index position. It's silly to have your code be indeterministic just because you don't want to add an integer decrement.



Um, in pretty much all languages, you use array sizes, i.e. how many indices exist, to determine how "large" your array is, not the largest index number that points to a non-null (non-zero) element in the array.

Yea, I can't figure out why none of the code in your timer is executing. Try putting debug messages in there at multiple locations to see what runs and what doesnt.

Will do, thanks again for your time. +reps ofc. (Edit: aznricepuff, I need to spread the reps around)
 
Status
Not open for further replies.
Top