My loop only runs once :S

[shadowcon]

function Trig_Crush_PushEmBack takes nothing returns nothing
//Local variable declarations
//Units
local unit PickedUnit = GetEnumUnit()
local unit TriggeringUnit = GetTriggerUnit()
//Reals
local real Angle
//Locations
local location PickedUnitLoc = GetUnitLoc(PickedUnit)
local location TargetLoc = GetSpellTargetLoc()
local location pushbackedMinus200
//Loop Debug stuff
local boolean loccheck = false
local integer loopdebug = 0
//Declarations done
//Set the declared but not defined variables
set Angle = AngleBetweenPoints(TargetLoc, PickedUnitLoc)
set pushbackedMinus200= PolarProjectionBJ(TargetLoc, 200.00, Angle)
//Variables done
 //   call UnitDamageTargetBJ( TriggeringUnit, PickedUnit, 200.00, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_ENHANCED )
    call SetUnitPositionLocFacingLocBJ( PickedUnit, GetUnitLoc(PickedUnit), TargetLoc)
    loop
        exitwhen loccheck == true
        if (pushbackedMinus200 == GetUnitLoc(PickedUnit)) then
            set loccheck = true
            set loopdebug = loopdebug-1
        else
            set loopdebug = loopdebug+1
            call MoveUnitBack(loopdebug, PickedUnit, TargetLoc, Angle)
            call TriggerSleepAction(0.020)
            endif
    endloop
    
endfunction

here's the other functions that it calls;

function PolarMinus5 takes real angle returns location
    local real x = GetLocationX(GetUnitLoc(GetEnumUnit())) + 5.00 * Cos(angle * bj_DEGTORAD)
    local real y = GetLocationY(GetUnitLoc(GetEnumUnit())) + 5.00 * Sin(angle * bj_DEGTORAD)
    return Location(x, y)

endfunction

function MoveUnitBack takes integer loopdebug, unit PickedUnit, location TargetLoc, real Angle returns nothing
//Moves the unit back 5 distance and causes it to face the spell center
local location face = TargetLoc
local location loc = PolarMinus5(Angle)
    call SetUnitPositionLoc(PickedUnit, loc)
    call SetUnitFacing(PickedUnit, AngleBetweenPoints(loc, face))
    call DisplayTextToForce(GetPlayersAll(),I2S(loopdebug))
endfunction

okay, the trigger only runs MoveUnitBack once, I want it to run at least 20 times, call MoveUnitBack is in a loop that SHOULD run like 20 times, and not 1 time, what have I done wrong?

 

[Renendaru]

Well, you kind of used TriggerSleepAction in the loop, which can malfunction in loops, and do crazy things.

 

[Gtam]

yea you should use a timer. Loops and sleepaction never works i have expirinced that many times before.

 

[Azlier]

[lJASS]TriggerSleepAction[/lJASS] works just fine in loops. Its inaccuracy and the way it has a minimum period makes it unusable for sliding and smooth effect, though.

 

[shadowcon]

if I use a timer I wouldn't be able to store local variables from one function to the timer expires trigger, then I would have to use globals and then it wouldnt be MUI right? or is there anyway to use hashtables to be able to avoid that?

 

[Gwypaas]

[lJASS]GetEnumUnit()[/lJASS] only works on ForGroups, make sure you are calling your function with a ForGroup instead of a normal function call or something like that.

 

[Gtam]

there at polarminus5 use GetUnitX/Y because those locations leak and the UnitX/Y wont use locations and it will save a function call.

 

[Lyerae]

Am I the only one who saw this? [ljass]set loccheck = true [/ljass].
It's not looping because the [ljass]exitwhen[/ljass] has been met.

 

[Gtam]

am i the only on seeing the that is in an if block thast only met after like 20 times running the loop.

 

[Lyerae]

It looks to me that the [ljass]if[/ljass] statement is initially true, but I can't get near a computer to test the code.

 

[Gtam]

me neither but i think thats not the reason. But i could be wrong my head is not good at running code.

 

[Lyerae]

It has to be the reason.
Nothing else sets it's [ljass]exitwhen[/ljass] to true.

 

[Gwypaas]

Have you used some debug messages and checked if this if becomes true on first iteration or something like that?

        if (pushbackedMinus200 == GetUnitLoc(PickedUnit)) then
            set loccheck = true
            set loopdebug = loopdebug-1

Edit -Just realized, if you are using ForGroup to call the function then you can't use TriggerSleepActions in the function so that might be the cause of the problem.

 

[GetTriggerUnit-]

Just like that, you're leaking tons of locations in this code.

 

[Jesus4Lyf]

if you are using ForGroup to call the function then you can't use TriggerSleepActions in the function

Gwypaas is the winner. Everyone else is irrelevant.
The ForGroup callback function is like a trigger condition - you may not use waits, it will terminate.

 

[Gwypaas]

@J4L

It's actually pretty fun, the local needs ForGroup to not be null and the loop can't run using a ForGroup

Just decide what you want, if you still want to loop through all units in a group use a FirstOfGroup loop.

Here's how it looks (Hand coded so it might be some wrong arguments and function names):

local group g = CreateGroup()
local unit u
call GroupAddGroup(g, source) // So you dont change anything in the normal group
set u = FirstOfGroup(g)
loop
    exitwhen u = null
    ..... // Do stuff with u
    call GroupRemoveUnit(g, u)
    set u = FirstOfGroup(g)
endloop
call DestroyGroup(g)
set g = null
set u = null

 

[shadowcon]

@everyone who wanted to test the code, i didnt add the entire trigger, only the what I thought was relevant part

@the one who thought that exitwhen was met, no, it wasn't met because it was supposed to check distance from a location and the unit affected was nowhere near it.

@Gwypaas, Yeah, the function was called using ForGroup so that with no wait inside it must been it
btw, I'm jass nub so I don't get what your function does... plz pm me in swedish

 

[ZugZugZealot]

It's not the loop that's screwed up per se.
[ljass]if (pushbackedMinus200 == GetUnitLoc(PickedUnit)) then[/ljass]
Is the problem to your code. Warcraft III can't do location comparisons.

Since there's a set amount of times it'll loop, you should just evaluate how many times it has looped instead.

(example, untested code)

function Trig_Crush_PushEmBack takes nothing returns nothing
    local unit pickedUnit = GetEnumUnit()
    local unit trigUnit = GetTriggerUnit()
    
    local real x = GetUnitX(pickedUnit)
    local real y = GetUnitY(pickedUnit)
    local real angle = AngleBetweenXY( GetUnitX(trigUnit), GetUnitY(trigUnit), x, y, (GetUnitFacing(trigUnit)/180) * bj_PI, false)
    local integer i
    
    set i = 0
    loop
        exitwhen (i > 39)
        exitwhen (GetUnitState(pickedUnit, UNIT_STATE_LIFE) <= 0)
        set x = 5 * Cos(angle)
        set y = 5 * Sin(angle)
        exitwhen (IsTerrainPathable(x, y, PATHING_TYPE_WALKABILITY)
        call SetUnitPosition( pickedUnit, x, y)
        set i = i + 1
        call TriggerSleepAction(0.020)
    endloop
    
endfunction

Included function

function AngleBetweenXY takes real x1, real y1, real x2, real y2, real default, boolean zRandom returns real
    local x = x2 - x1
    local y = y2 - y1
    
    if (y == 0.00) then
        if (x > 0.00) then
            return (0.00)
        elseif (x < 0.00) then
            return (bj_PI)
        else
            if (zRandom) then
                return (GetRandomReal(0.00, 2*bj_PI))
            else
                return (default)
            endif
        endif
    elseif (x == 0.00) then
        if (y > 0.00) then
            return (bj_PI * 0.50)
        else
            return (bj_PI * 1.50)
        endif
    endif
    return (Atan2(y,x))
endfunction

 

[Gwypaas]

@ZugZugZealot

Sure that will cause bugs later but now that is NOT the problem, it will just return false. The only problem is that causing this functions thread to crash is the TriggerSleepAction. And your AngleBetweenXY is slow, bad coded and just contains useless stuff. You should use [ljass]call Atan2(y2-y, x2-x)[/ljass] directly instead of that shit.


@shadowcon

Ill try to send a working when I have enough time.

 

[ZugZugZealot]

I made it that way because for (y == 0) and x less than 0 because it would return 0. But somewhere down the line it looks like they fixed that.

Also, abrasive behavior is uncalled for in an assistance forum.

The main point of that was to get at was using a count for travel instead of position comparison.