[JASS] structs loop issue

[dardas]

hey.. i made a struct, and i made a spell for a request
that uses a loop to go forward, then backwards..
it uses grouputils and timerutils..
but i still cant figure out why the loop wont run..
no matter what i do..

TheCode

scope Boomerang initializer OnInit
    globals
        private timer Tim  = null
    endglobals
    private struct BR
        unit u = null
        unit t = null
        static unit st = null
        static unit tttu = null
        real x = 0
        real y = 0
        real x2 = 0
        real y2 = 0
        real di = 0
        real md = 0
        real dc = 0
        real da = 0
        real aoe = 0
        boolean bool = false
        static group g = null
        
        static method OnCond takes nothing returns boolean
            return GetSpellAbilityId() == 'A000'
        endmethod
        
        static method GroupCond takes nothing returns boolean
            return GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) > 0.405 and IsUnitEnemy(GetFilterUnit(), GetOwningPlayer(tttu)) == true
        endmethod
        
        static method OnLoop takes nothing returns nothing
            local real dx
            local real dy
            local timer t = GetExpiredTimer()
            local thistype this
                loop
                if this == GetTimerData(t) then
                set this = 1
                else
                set this = this + 1
                endif
                
                    call DisplayTextToForce(GetPlayersAll(), I2S(this))
                        set tttu = .u
                            if .dc < .md then
                                set .dc = .dc + .di
                                call SetUnitX(.t, GetUnitX(.t) + .di * Cos (GetUnitFacing(.t) * bj_DEGTORAD))
                                call SetUnitY(.t, GetUnitY(.t) + .di * Sin (GetUnitFacing(.t) * bj_DEGTORAD))
                                call DisplayTextToForce(GetPlayersAll(), "3")
                            else
                                set .dc = .dc - .di
                                call SetUnitX(.t, GetUnitX(.t) + .di * Cos (GetUnitFacing(.t) * bj_DEGTORAD))
                                call SetUnitY(.t, GetUnitY(.t) + .di * Sin (GetUnitFacing(.t) * bj_DEGTORAD))
                                   call DisplayTextToForce(GetPlayersAll(), "4")
                                   if SquareRoot(dx * dx + dy * dy) < 25 then
                                        set .bool = false
                                        call DisplayTextToForce(GetPlayersAll(), "5")
                                    endif
                            endif
                    
                                if .bool == true then
                                    set g = NewGroup()
                                    call GroupRefresh(g)
                                    call ReleaseGroup(g)
                                    call GroupEnumUnitsInArea(ENUM_GROUP, GetUnitX(.t), GetUnitY(.t), .aoe, Condition(function BR.GroupCond))
                                        loop
                                            set st = FirstOfGroup(ENUM_GROUP)
                                            exitwhen st == null
                                            call UnitDamageTarget(.t, st, .da, true, true, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
                                            call GroupRemoveUnit(ENUM_GROUP, st)
                                            set st = null
                                        endloop
                                endif
                    
                                    if .bool == false then
                                        call RemoveUnit(.t)
                                        set .u = null
                                        set .t = null
                                        set .dc = 0
                                        set .md = 0
                                        set .di = 0
                                    endif
                    
                exitwhen this == GetTimerData(t)
                endloop
        endmethod
        
        static method OnCast takes nothing returns nothing
        local thistype this = BR.allocate()
            call SetTimerData(Tim,this)
            call DisplayTextToForce(GetPlayersAll(), I2S(this))
            set .u = GetTriggerUnit()
            set .x = GetUnitX(.u)
            set .y = GetUnitY(.u)
            set .t = CreateUnit(GetOwningPlayer(.u), 'h001', .x, .y, 0)
            set .x2 = GetLocationX(GetSpellTargetLoc())
            set .y2 = GetLocationY(GetSpellTargetLoc())
            set .di = 300
            set .md = 700
            set .aoe = 250
            set .da = 100
            set .dc = 0
            set .bool = true
        endmethod
        
    endstruct

    private function OnInit takes nothing returns nothing
        local trigger DM = CreateTrigger()
        set Tim = NewTimer()
        call TriggerRegisterAnyUnitEventBJ(DM, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(DM, Condition(function BR.OnCond))
        call TriggerAddAction(DM, function BR.OnCast)
    
        call TimerStart(Tim, 0.032, true, function BR.OnLoop)
        set DM = null
    endfunction
endscope

Thanks in advance.

 

[Axarion]

Your indexing is totally horrible :/ http://www.hiveworkshop.com/forums/...ls-280/vjass-indexer-tutorial-structs-114223/ Indexing tutorial

 

[Adiktuz]

each cast will overwrite the timer data since you only use 1 timer...

instead of a local thistype you can use a static thistype

plus you dont set thistype this into a value at the start, that can cause problems...

(some variables cause error if not initialized especially LOCALS (units, groups, integers etc), I'm not sure if thistype is one of those but if yes then it causes the trigger to stop in the line if this == GetTimerData())

Plus I dont see why this lines is present...

set g = NewGroup()
call GroupRefresh(g)
call ReleaseGroup(g)

you just make a newgroup, refresh it then remove it... has it something to do with ENUM_GROUP? if yes why dont just use g as the group? then release the group after you finish the actions for the group...

i'm not sure but I think you should initialize the static group to CreateGroup() rather than null...

use GetWidgetLife, its faster then GetUnitState()...

plus if GetTimerData > 1, the first if-then wont run since the loop will terminate before you can run the if-then, since you use exitwhen this == GetTimerData

and set GetTimerData(t) into a variable...

Also Tukki said that if you're using GroupEnumUnitsInArea you should destroy the boolexpr used...

+improve indentation and indexing...

 

[Axarion]

plus you dont set thistype this into a value at the start, that can cause problems...

(some variables cause error if not initialized especially LOCALS (units, groups, integers etc), I'm not sure if thistype is one of those but if yes then it causes the trigger to stop in the line if this == GetTimerData())

since when is this a problem huh?

i'm not sure but I think you should initialize the static group to CreateGroup() rather than null...

Or just use ENUM_GROUP

Also Tukki said that if you're using GroupEnumUnitsInArea you should destroy the boolexpr used...

Or just know that this bug was fixed years ago... It doesnt leak anymore

 

[jk2pach]

scope Boomerang initializer OnInit  //needs TimerUtils,GroupUtils, SpellEvent
    globals //just use explicit names, when you get 350 triggers, it's easier to retrieve something
        private constant integer SPELL = 'A000' //spell ID
        private constant real TIMEOUT = 0.03 //loop interval
        private constant integer DUMMY_ID = 'h001' //missile or dummy id
        private constant real AOE = 250. //AOE of the damage
        private constant real DAMAGE = 100. //damage amount
        private constant integer SPEED = 20 //speed of the missile
        private player Owner //for group usage
        private unit Caster  //for group usage
    endglobals

    //Note 1: use more constant for settings
    //Note 2: use ENUM_GROUP for instant group usages, it allows to avoid an other group creation
    //Note 3: use a local struct and a local timer which runs only on cast
    //Note 4: use SpellEvent, since it works better than spell natives and is sexier :p (less triggers etc.)
    //Note 5: when you make a group enum, make your code used in the filter, it is faster than a loop

    //**************
    //TIPS: I suppose it was for a test, but remember the best way to deal damage is not with a trigger, in your case. Just because u make ultra-fast group enum, it's a bit laggy for old computer and w3 engine.
    //You should more, since your damage is always the same, add to your dummy an ability based on permanent immolation and no visible effect: this way, whatever the speed of your dummy missile, the damage will be dealt.
    //**************

    //The filter function : the damage is done there, no need to use a loop in your trigger
    private function OnDamage takes nothing returns boolean
        local unit target = GetFilterUnit()
            // the boolean UNIT_TYPE_DEAD is no more bugged, so check life >0.405 is no more needed
            if not(IsUnitType(target,UNIT_TYPE_DEAD)) and IsUnitEnemy(target,Owner) then
                 call UnitDamageTarget(Caster,target,DAMAGE, true, true, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
            endif
            set target = null
            return true
    endfunction

    private struct str
        unit caster
        unit dummy
        integer i
        real angle
        integer distance
        player owner
    endstruct

    private function OnLoop takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local str dat = GetTimerData(t)
        local real x = GetUnitX(dat.dummy)
        local real y = GetUnitY(dat.dummy)
            if dat.i < dat.distance then
               set dat.i = dat.i + 1 //iteration of the loop
               if dat.i < 0.5*dat.distance then  //before half the distance: damage+move on
                  call SetUnitX(dat.dummy, x + SPEED * Cos(dat.angle) )
                  call SetUnitY(dat.dummy, y + SPEED * Sin(dat.angle) )
                  set Caster = dat.caster //for filter, you need something able to recognize your unit , so a global
                  set Owner = dat.owner //there you can use the filter with globals
                  call GroupEnumUnitsInRange(ENUM_GROUP,x,y, AOE, Filter(function OnDamage)) //Use a filter is faster and you can do your things directly into the filter function; note the ENUM_GROUP is a global group used by grouputils, so you do not need to create a group for instant actions!!
               else //after half the distance: move back
                  call SetUnitX(dat.dummy, x - SPEED * Cos(dat.angle) )
                  call SetUnitY(dat.dummy, y - SPEED * Sin(dat.angle) )
               endif
            else
                call RemoveUnit(dat.dummy)
                call ReleaseTimer(t) //you know this: timer will be released through timerutils, making it usable by an other function
                call dat.destroy() //no need to nullify the variables since you may use them later
            endif
    endfunction
        

    private function OnCast takes nothing returns nothing
        local str dat = str.create() //a local struct will allow you to make the spell MUI, of course
        local timer t = NewTimer()  //This way the timer start (and will stop through OnLoop) only at the spell cast
        local real casterX = GetUnitX(SpellEvent.CastingUnit) //explicit name is always easier to understand
        local real casterY = GetUnitY(SpellEvent.CastingUnit) //SpellEvent.CastingUnit is the caster of the spell returned by the library SpellEvent
        local real targetX = SpellEvent.TargetX  // due to the library SpellEvent
        local real targetY = SpellEvent.TargetY // due to the library SpellEvent
            set dat.caster = SpellEvent.CastingUnit // due to the library SpellEvent
            set dat.owner = GetOwningPlayer(dat.caster) //store ur player, we will need it later
            set dat.dummy = CreateUnit(dat.owner,DUMMY_ID,casterX,casterY,0.) //your missile (or dummy)
            call SetUnitPathing(dat.dummy,false) //turn collision off
            set dat.i = 0 //this will make the iteration
            set dat.distance = R2I(SquareRoot( (targetX-casterX)*(targetX-casterX) + (targetY-casterY)*(targetY-casterY))/SPEED) //the distance there is the amounf of enumerations needed into your loop
            set dat.angle = Atan2((targetY -casterY), (targetX - casterX)) //angle between caster and target
            call SetTimerData(t,dat)   //seems you know the timer can store an integer (a struct) through TimerUtils
            call TimerStart(t,TIMEOUT,true,function OnLoop) //launch the timer
    endfunction

    public function OnInit takes nothing returns nothing //Scopes init are public functions, privates are for libraries, since sometimes the private init on scope will bug (rare)
            call RegisterSpellFinishResponse(SPELL,OnCast) //sexier, uh? :) )
    endfunction

endscope

Edit: a mistake fixed (I wrote GroupEnumUnitsInRect instead of InRange)

 

[dardas]

"spell event is not a type that allows " . " syntax".
any idea?

edit, requires table my bad..

edit2: saves correctly, doesnt create dummy

 

[jk2pach]

Did you download the library SpellEvent?

 

[Adiktuz]

since when is this a problem huh?

well, I encounter it sometimes... trigger wont run then when I put an initial value to the locals it works...

 

[jk2pach]

It does not require table.

About the dummy creation, did you changed its ID?

Note: I work without editor during the day, I do not get my computer, so they are maybe so mistakes (such as one I edited in my first post).

 

[Adiktuz]

It does not require table.

About the dummy creation, did you changed its ID?

Note: I work without editor during the day, I do not get my computer, so they are maybe so mistakes (such as one I edited in my first post).

you can use a static thistype rather than the local though I'm not sure if it works faster... ^^

 

[jk2pach]

In the situation the spell is "normally" used, it is not faster. Methods are not faster than functions.

 

[Adiktuz]

In the situation the spell is "normally" used, it is not faster. Methods are not faster than functions.

ah... okay...

btw, since you said that using Filter() enables you to use the actions directly on the filter, I want to say that you can also do that even if you use Condition() rather than Filter()... ^^

 

[dardas]

okay look..
id of dummy = h001,
everything else if[edit=IS] fine..
heres another thing:

library SpellEvent initializer Init requires Table

it does require table..
i got all group utils, table, spellevent, and timerutils.. o.o
still, doesnt work..

you can use a static thistype rather than the local though I'm not sure if it works faster... ^^

what? ;p you mean making it a static makes it better then local?..
i donno, cuz in the jasshelper manual it says local..

 

[Adiktuz]

okay look..
id of dummy = h001,
everything else if[edit=IS] fine..
heres another thing:

library SpellEvent initializer Init requires Table

it does require table..
i got all group utils, table, spellevent, and timerutils.. o.o
still, doesnt work..



what? ;p you mean making it a static makes it better then local?..
i donno, cuz in the jasshelper manual it says local..

I mean remove the local str dat and instead make a static thistype dat as a struct member... but jk2pach said that it doesnt matter.. but maybe if you add more methods using the static thistype is better than creating locals for every method run... or you can also make a global str dat if you want...

I think the not there is misplaced... enclose the not and the first condition in parenthesis coz apparently the not keyword affects ALL conditions after it...

and maybe use a global unit target rather than local unit target... (I'm not sure if its better but I guess making a variable once and then setting it to a value is better than creating and setting everytime you use it... plus you wont need to null it)

btw, what part of the trigger wont work?

 

[jk2pach]

About static method: always remember than prefer the simpliest way is better. VJass is powerfull, but w3 engine is outdated. The more VJass you use, the more you use the w3 engine limits (Such as 255 struct init allowed on a map , which arrives quite fast).
For such a simple spell, a local struct is quite enough.

About using coundition instead of Filter: Filter is supposed to be faster than Condition() Of course, a human will not see the difference, and with current computers, it's probably a so small difference of speed that it can not be experimented.

About the thing the spell does not work. Ahem. Without editor, I can not test it :/. I found an error (I was using dat.casterX / dat.target X instead of casterX and targetX)

I edited my spell in my first post. (Damn, without editor, it's hard xD)

 

[Adiktuz]

About static method: always remember than prefer the simpliest way is better. VJass is powerfull, but w3 engine is outdated. The more VJass you use, the more you use the w3 engine limits (Such as 255 struct init allowed on a map , which arrives quite fast).
For such a simple spell, a local struct is quite enough.

About using coundition instead of Filter: Filter is supposed to be faster than Condition() Of course, a human will not see the difference, and with current computers, it's probably a so small difference of speed that it can not be experimented.

About the thing the spell does not work. Ahem. Without editor, I can not test it :/. I found an error (I was using dat.casterX / dat.target X instead of casterX and targetX)

I edited my spell in my first post. (Damn, without editor, it's hard xD)

how bout a global structtype?

just said that you could do the same using Condition(), I did not say that you were wrong in saying Filter() is faster...

and yeah, its pretty hard without the editor...

 

[jk2pach]

There is no probleme using a local struct for such a usage: Spell, then, destroyed. It's enough

The only reason the spell was not working for our helpneeder is I wrote the code with some stupid errors.

 

[dardas]

lol, i allready fixed the dat.casterX etc, it bugged and i saw it was a local and not static/private/constant.. so i edited it ;p
and what part wont work?
nothing..
it wont create the unit, wont send it forward, wont show any message.

 

[jk2pach]

Of course they are no messages, I removed them xD.

Post your edited code again please.

Is the spell ID the good one?

What spell do you get as base spell? Try with "RegisterSpellCastResponse" instead of RegisterSpellFinishResponse.

 

[Adiktuz]

Of course they are no messages, I removed them xD.

Post your edited code again please.

Is the spell ID the good one?

What spell do you get as base spell? Try with "RegisterSpellCastResponse" instead of RegisterSpellFinishResponse.

I suggest RegisterSpellEffectResponse...

 

[dardas]

kk, base spell is shockwave
i used your code.. just fixed w/e bugged..

The Code

scope Boomerang initializer OnInit  //needs TimerUtils,GroupUtils, SpellEvent
    globals //just use explicit names, when you get 350 triggers, it's easier to retrieve something
        private constant integer SPELL = 'A000' //spell ID
        private constant real TIMEOUT = 0.03 //loop interval
        private constant integer DUMMY_ID = 'h001' //missile or dummy id
        private constant real AOE = 250. //AOE of the damage
        private constant real DAMAGE = 100. //damage amount
        private constant integer SPEED = 20 //speed of the missile
        private player Owner //for group usage
        private unit Caster  //for group usage
    endglobals

    //Note 1: use more constant for settings
    //Note 2: use ENUM_GROUP for instant group usages, it allows to avoid an other group creation
    //Note 3: use a local struct and a local timer which runs only on cast
    //Note 4: use SpellEvent, since it works better than spell natives and is sexier :p (less triggers etc.)
    //Note 5: when you make a group enum, make your code used in the filter, it is faster than a loop

    //**************
    //TIPS: I suppose it was for a test, but remember the best way to deal damage is not with a trigger, in your case. Just because u make ultra-fast group enum, it's a bit laggy for old computer and w3 engine.
    //You should more, since your damage is always the same, add to your dummy an ability based on permanent immolation and no visible effect: this way, whatever the speed of your dummy missile, the damage will be dealt.
    //**************

    //The filter function : the damage is done there, no need to use a loop in your trigger
    private function OnDamage takes nothing returns boolean
        local unit target = GetFilterUnit()
            // the boolean UNIT_TYPE_DEAD is no more bugged, so check life >0.405 is no more needed
            if not(IsUnitType(target,UNIT_TYPE_DEAD)) and IsUnitEnemy(target,Owner) then
                 call UnitDamageTarget(Caster,target,DAMAGE, true, true, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
            endif
            set target = null
            return true
    endfunction

    private struct str
        unit caster
        unit dummy
        integer i
        real angle
        integer distance
        player owner
    endstruct

    private function OnLoop takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local str dat = GetTimerData(t)
        local real x = GetUnitX(dat.dummy)
        local real y = GetUnitY(dat.dummy)
            if dat.i < dat.distance then
               set dat.i = dat.i + 1 //iteration of the loop
               if dat.i < 0.5*dat.distance then  //before half the distance: damage+move on
                  call SetUnitX(dat.dummy, x + SPEED * Cos(dat.angle) )
                  call SetUnitY(dat.dummy, y + SPEED * Sin(dat.angle) )
                  set Caster = dat.caster //for filter, you need something able to recognize your unit , so a global
                  set Owner = dat.owner //there you can use the filter with globals
                  call GroupEnumUnitsInArea(ENUM_GROUP,x,y, AOE, Filter(function OnDamage)) //Use a filter is faster and you can do your things directly into the filter function; note the ENUM_GROUP is a global group used by grouputils, so you do not need to create a group for instant actions!!
               else //after half the distance: move back
                  call SetUnitX(dat.dummy, x - SPEED * Cos(dat.angle) )
                  call SetUnitY(dat.dummy, y - SPEED * Sin(dat.angle) )
               endif
            else
                call RemoveUnit(dat.dummy)
                call ReleaseTimer(t) //you know this: timer will be released through timerutils, making it usable by an other function
                call dat.destroy() //no need to nullify the variables since you may use them later
            endif
    endfunction
        

    private function OnCast takes nothing returns nothing
        local str dat = str.create() //a local struct will allow you to make the spell MUI, of course
        local timer t = NewTimer()  //This way the timer start (and will stop through OnLoop) only at the spell cast
        local real casterX = GetUnitX(SpellEvent.CastingUnit) //explicit name is always easier to understand
        local real casterY = GetUnitY(SpellEvent.CastingUnit) //SpellEvent.CastingUnit is the caster of the spell returned by the library SpellEvent
        local real targetX = SpellEvent.TargetX  // due to the library SpellEvent
        local real targetY = SpellEvent.TargetY // due to the library SpellEvent
            set dat.caster = SpellEvent.CastingUnit // due to the library SpellEvent
            set dat.owner = GetOwningPlayer(dat.caster) //store ur player, we will need it later
            set dat.dummy = CreateUnit(dat.owner,DUMMY_ID,casterX,casterY,0.) //your missile (or dummy)
            call SetUnitPathing(dat.dummy,false) //turn collision off
            set dat.i = 0 //this will make the iteration
            set dat.distance = R2I(SquareRoot( (targetX-casterX)*(targetX-casterX) + (targetY-casterY)*(targetY-casterY))/SPEED) //the distance there is the amounf of enumerations needed into your loop
            set dat.angle = Atan2((targetY -casterY), (targetX - casterX)) //angle between caster and target
            call SetTimerData(t,dat)   //seems you know the timer can store an integer (a struct) through TimerUtils
            call TimerStart(t,TIMEOUT,true,function OnLoop) //launch the timer
    endfunction

    public function OnInit takes nothing returns nothing //Scopes init are public functions, privates are for libraries, since sometimes the private init on scope will bug (rare)
            call RegisterSpellFinishResponse(SPELL,OnCast) //sexier, uh? :) )
    endfunction

endscope

yeah, the spell raw id = A000.
and ill try editing to both solutions of event thingy..

 

[Adiktuz]

enclose the not and the first condition in parentheses... since not affixes to all conditions after it if not enclosed (I read on some threads)... so it might damage friendly units right now instead of enemies...

or use IsUnitType(target,UNIT_TYPE_DEAD) != true

and WEAPON_TYPE_WHOKNOWS == null so you can just use null there... ^^

 

[dardas]

you 2 confuse me more and more every post o.o

 

[jk2pach]

Then, just remember one thing: VJass is enough flexible to allow different ways to reach something.

This is the reason some tried to introduce standards (JESP was one).

Since each coder get its way to display codes...Just create your own one.

I personally prefer limit the amount of methods and I prefer functions. It's a way of code. Others perfer methods. Just choose the thing you prefer!

 

[dardas]

well.. after all this time, no 1 actually answered my original question yet..
why didnt the loop start? o.o (in my first code)]

edit: is there a way to make a knockback to pull you ( allready did that, using opposite Atan2, GetUnitY(Caster), instead of target, and so on),
and do the duration so it will exactly land near the hero?

set kd = 0.75 - (DistanceBetweenPoints(Location(GetUnitX(u), GetUnitY(u)), Location(GetUnitX(u), GetUnitY(u))) / GetUnitMoveSpeed(t))

thats not very accurate.. cuz if there are 2 units
1 closer then the other to the caster,
the closer 1 will go more.. (even if they are same units..)
so im kind of stuck =]

 

[Adiktuz]

well.. after all this time, no 1 actually answered my original question yet..
why didnt the loop start? o.o (in my first code)]

edit: is there a way to make a knockback to pull you ( allready did that, using opposite Atan2, GetUnitY(Caster), instead of target, and so on),
and do the duration so it will exactly land near the hero?

set kd = 0.75 - (DistanceBetweenPoints(Location(GetUnitX(u), GetUnitY(u)), Location(GetUnitX(u), GetUnitY(u))) / GetUnitMoveSpeed(t))

thats not very accurate.. cuz if there are 2 units
1 closer then the other to the caster,
the closer 1 will go more.. (even if they are same units..)
so im kind of stuck =]

maybe changed that distance between points and Location() cause Location() creates a leak... just use Squareroot(dx^2 + dy^2)

as for the knockback, you use unit u for both locations...

anyway if you use a formula to get the distance of the two units (caster and target) then just make him use that distance for the knockback, the unit will stop at the hero's position... and if you need to have the same time for both, use another formula to set the knockback distance per timer loop...

example, if you want knockbacks to last only 2 seconds then maybe you can do

set distancepertimeloop = Total distance*TimerInterval/2.00

in general for fixed time knockback

knockbackdistancepertimeloop = Total distance*TimerInterval/TotalTime

 

[dardas]

well.. i hate math >.>
i did square root thing used in the function of distance between points,
and heres how i do knockback:

http://www.hiveworkshop.com/forums/jass-functions-413/knockback-unit-35545/