[General] Is it ok this function? 2

[HerlySQR]

I create this function in Jass to the units keeping attacking to a point even if they stop in some moment, but it doesn't work, what is happening?

function UnitsCond takes nothing returns boolean
    local unit u=GetFilterUnit()
    if GetUnitTypeId(u)=='ndrl' then
        set u=null
        return false
    endif
    if GetOwningPlayer(u)!=Player(0) or GetOwningPlayer(u)!=Player(6) then
        set u=null
        return false
    endif
    if GetUnitCurrentOrder(u)==String2OrderIdBJ("attack") or IsUnitInGroup(u,udg_Se_van)==false then
        set u=null
        return false
    endif
    if IsUnitType(u,UNIT_TYPE_STRUCTURE)==true then
        set u=null
        return false
    endif
    if IsUnitType(u,UNIT_TYPE_HERO)==true then
        set u=null
        return false
    endif
    if IsUnitInGroup(u,udg_Unidades_del_verde)==true then
        set u=null
        return false
    endif
    set u=null
    return true
endfunction

function Trig_Ataquen_2_Actions takes nothing returns nothing
    local group attackers=CreateGroup()
    local unit u
    call GroupEnumUnitsInRect(attackers,GetPlayableMapRect(),Condition(function UnitsCond))
    loop
        set u=FirstOfGroup(attackers)
        exitwhen u==null
        call IssuePointOrderLoc(u,"attack",udg_Punto_de_ataque[GetUnitUserData(u)])
        call GroupRemoveUnit(udg_Se_van,u)
        call GroupRemoveUnit(attackers,u)
    endloop
    call DestroyGroup(attackers)
    set attackers=null
endfunction

//===========================================================================
function InitTrig_Ataquen_2 takes nothing returns nothing
    set gg_trg_Ataquen_2=CreateTrigger()
    call DisableTrigger(gg_trg_Ataquen_2)
    call TriggerRegisterTimerEventPeriodic(gg_trg_Ataquen_2,2.00)
    call TriggerAddAction(gg_trg_Ataquen_2,function Trig_Ataquen_2_Actions)
endfunction

And no, disabletrigger is not the reason.

 

[Chaosy]

You can simplify the condition function by a lot.

Use else ifs
instead of returning raw values in each if, set a local boolean to a value
null the unit variable at the bottom of the trigger
return the local boolean

This won't fix anything, probably. But easier code is easier to debug.

edit: some of the ifs should probably be merged too. Use OR instead of a seperate if.

 

[HerlySQR]

You can simplify the condition function by a lot.

Use else ifs
instead of returning raw values in each if, set a local boolean to a value
null the unit variable at the bottom of the trigger
return the local boolean

This won't fix anything, probably. But easier code is easier to debug.

edit: some of the ifs should probably be merged too. Use OR instead of a seperate if.

I think the function is ok in with that, because in some cases will stop without necessarly check all the conditions.

 

[GIMLI_2]

call GroupRemoveUnit(udg_Se_van,u)

You are removing units from a global group

IsUnitInGroup(u,udg_Se_van)==false

You check if unit is not in group, and if so, you return false. So unless you add the unit back to the group in another trigger, the unit will get the attack order only once (if the unit was in the group in the first place)

 

[Chaosy]

I think the function is ok in with that, because in some cases will stop without necessarly check all the conditions.

that's why I told you to change to elseif.

 

[HerlySQR]

call GroupRemoveUnit(udg_Se_van,u)

You are removing units from a global group

IsUnitInGroup(u,udg_Se_van)==false

You check if unit is not in group, and if so, you return false. So unless you add the unit back to the group in another trigger, the unit will get the attack order only once (if the unit was in the group in the first place)

It is a group that I created to the units that get spell damage from allies like flame strike and they go back.

But I think I see the real problem, where I used "or" I should use "and".

that's why I told you to change to elseif.

He, I didn't know JassHelper included that, or is from normal editor?

 

[Chaosy]

Standard jass.

Think of it this way, if GUI has it why wouldn't JASS?

 

[HerlySQR]

Standard jass.

Think of it this way, if GUI has it why wouldn't JASS?

GUI don't have elseif, just have else.

 

[Chaosy]

Not completely true.

You do:
if(x == true)

// standard if​

else

if(y == true=)

// basically an else if​

 

[HerlySQR]

Not completely true.

You do:
if(x == true)

// standard if​

else

if(y == true=)

// basically an else if​

But less elegants and with more endif