Looping and Lists glitch

[Weyrling]

This is what I call the following functions with:

Special Effect - Create a special effect attached to the AttachPoint of TempUnit using SFXString[TempInteger]
Set SFX = (Last created special effect)
Custom script:   call SFX(udg_TempUnit,udg_SFX)

^^In GUI, this part works perfectly^^
In a unit group, the following trigger and subsequent function does not remove the sfx:

Set TempUnit = (Picked unit)
Custom script:   call RemoveSFX(udg_TempUnit)

These are the called functions:

function SFX takes unit u, effect e returns nothing 
local integer num = 1 
local boolean b = false 
    loop 
        exitwhen b == true 
        if ( udg_SFX_Unit[num] == null) then 
            set udg_SFX_FX[num] = e 
            set udg_SFX_Unit[num] = u 
            set b = true 
	 set udg_SFXInt = udg_SFXInt + 1
        endif 
        set num = num + 1 
    endloop 
endfunction 
function RemoveSFX takes unit u returns nothing 
local integer num = udg_SFXInt 
local boolean b = false 
    loop 
        exitwhen b == true 
        if ( udg_SFX_Unit[num] == u) then 
            call DestroyEffect(udg_SFX_FX[num]) 
            set udg_SFX_Unit[num] = null 
            set udg_SFX_FX[num] = null 
            if ( udg_SFXInt == num ) then
                set udg_SFXInt = udg_SFXInt - 1
            endif
            set b = true 
        endif 
        set num = num - 1
    endloop 
endfunction

Some assistance would be most appreciated (The +rep kind of appreciation).

 

[Weyrling]

This forum never helps me >.<
*Bump*

 

[Romek]

You hit the OP limit in that loop.
You can't just loop like that waiting for a boolean.

Use waits inside the loop.

 

[Flare]

You hit the OP limit in that loop.
You can't just loop like that waiting for a boolean.

Use waits inside the loop.

He would only hit the op limit (why are you capitalizing op, just out of curiosity ) if the unit passed to RemoveSFX was never passed to the SFX function as well.

Not really sure why it wouldn't work (unless you didn't call SFX on that unit, not really sure since you haven't posted full code where you are calling the function), but you could do something like

function SFX takes ...
//locals
... //whatever else you're doing, adding sfx, incrementing counter, etc.
call GroupAddUnit (whichUnit, udg_SFX_Units)
endfunction

function RemoveSFX takes ...
//locals
if IsUnitInGroup (whichUnit, udg_SFX_Units) then
//do your loop
//when unit is found
...
call GroupRemoveUnit (whichUnit, udg_SFX_Units)
endif
endfunction

That way, you would only do the loop if an SFX was known to have been applied to the unit, but it would limit you to 1 SFX per unit without causing bugs (if you have NewGen WE, you could probably work around it with Cohadar's PUI, assuming custom value wasn't in use)

I've probably got the parameter order wrong there for those GroupAdd/Remove calls

 

[Weyrling]

I forgot to make an endloop at 0, thanks for pointing that out.
I've actually capped the max number at around 500, and that's never lagged on me even with 500 aiming at the 1, so I'm just going to add an "if num == 0 then set b = true" to it.

 

[Xorifelse]

I see you're using a boolean to exit the loop on the next round.
However, I'll give you a better way to do such things.

function SFX takes unit u, effect e returns nothing 
local integer num = 1 
    loop 
        if ( udg_SFX_Unit[num] == null) then 
            set udg_SFX_FX[num] = e 
            set udg_SFX_Unit[num] = u 
	    set udg_SFXInt = udg_SFXInt + 1

            exitwhen true
        endif 
        set num = num + 1 
    endloop 
endfunction 

function RemoveSFX takes unit u returns nothing 
local integer num = udg_SFXInt 
    loop 
        if ( udg_SFX_Unit[num] == u) then 
            call DestroyEffect(udg_SFX_FX[num]) 
            set udg_SFX_Unit[num] = null 
            set udg_SFX_FX[num] = null 
            if ( udg_SFXInt == num ) then
                set udg_SFXInt = udg_SFXInt - 1
            endif
            exitwhen true
        endif 
        set num = num - 1
    endloop 
endfunction

A long time ago, I did the same thing as you did.
But seriously, abusing the "exitwhen true" really is a good thing to do.
Keep in mind that it exists the loop directly, and you need to adjust some of the code to work with it.