[Lua] Calling functions inside a timer

[Oli]

Hi,

I have been struggling with the following code:

...
    local shipsmoving = CreateGroup()
...
    function naval_movement_start()
        TimerStart(CreateTimer(), 0.1, true, ForGroupBJ(shipsmoving, function()
            local u = GetEnumUnit()
            naval_movement_accelerate(u)
            naval_movement(u)
            end))
    end

    function naval_movement_order()
        local u = GetTriggerUnit()
        GroupAddUnitSimple(u, shipsmoving)
        naval_movement_start()
    end

    function naval_movement(u)
        local cv = GetUnitUserData(u)
        SetUnitPositionLoc(u, PolarProjectionBJ(GetUnitLoc(u), RVM.actualspeed[cv], GetUnitFacing(u)))
        print("dog1")
    end

    function naval_movement_accelerate(u)
        local cv = GetUnitUserData(u)
        if RVM.actualspeed[cv] ~= RVM.maxspeed[cv] then
            RVM.actualspeed[cv] = RVM.actualspeed[cv] + RVM.acceleration[cv]
        end
    end
    
    OnMapInit(function ()
        local trig = CreateTrigger()
        TriggerRegisterAnyUnitEventBJ(trig, EVENT_PLAYER_UNIT_ISSUED_POINT_ORDER)
        TriggerAddAction(trig, naval_movement_order)
    end)
...

In the current state the timer runs only once each time a unit is moved. Only a part of the script is shown, I believe the cause is related with me coding the timer wrong.
May I ask for help?

 

[Insanity_AI]

You're attempting to pass the return value of ForGroupBJ call as a timer callback function. ForGroupBJ has no return value, so you're effectively doing nothing with the timer.
You're calling ForGroupBJ exactly once (when the unit order event occurs).

What you have to do is define a callback function for the timer (it must not expect any arguments) and within that callback function you can call ForGroupBJ.
Try the following:

function naval_movement_start()
    TimerStart(CreateTimer(), 0.1, true, function()
        ForGroupBJ(shipsmoving, function()
            local u = GetEnumUnit()
            naval_movement_accelerate(u)
            naval_movement(u)
        end)
    end)
end

 

[Uncle]

Adding onto what Insanity said, here's some cleaned up code which might serve you better:

    local shipsmoving = CreateGroup()
    local shipstimer = nil

    function naval_movement_order()

        -- Note: If the ordered unit is already in the group then it won't be added again
        -- But it's probably wise to add a more efficient check before calling this function
        GroupAddUnit(shipsmoving, GetTriggerUnit())

        -- Exit early if the timer is already running since we don't
        -- want to create a new timer or restart an existing timer
        if shipstimer ~= nil then
            return
        end

        shipstimer = CreateTimer()
        TimerStart(shipstimer, 0.1, true, function()
            ForGroup(shipsmoving, function()
                local u = GetEnumUnit()
                local cv = GetUnitUserData(u)
                RVM.actualspeed[cv] = RVM.actualspeed[cv] + RVM.acceleration[cv]
                if RVM.actualspeed[cv] > RVM.maxspeed[cv] then
                    RVM.actualspeed[cv] = RVM.maxspeed[cv]
                end
                set_unit_offset_position(u, RVM.actualspeed[cv], GetUnitFacing(u))
            end)
        end)
    end

    function naval_movement_stop()
        -- Note: This function will kill the timer and stop moving the ships
        if shipstimer == nil then
            return
        end
        PauseTimer(shipstimer)
        DestroyTimer(shipstimer)
        GroupClear(shipsmoving)
        shipstimer = nil
    end

    function set_unit_offset_position(u, offset, angle)
        local x = GetUnitX(u)
        local y = GetUnitY(u)
        local rad = angle * bj_PI / 180.0
        local newX = x + offset * Cos(rad)
        local newY = y + offset * Sin(rad)
        SetUnitPosition(u, newX, newY)
    end
 
    OnMapInit(function()
        local trig = CreateTrigger()
        TriggerRegisterAnyUnitEventBJ(trig, EVENT_PLAYER_UNIT_ISSUED_POINT_ORDER)
        TriggerAddAction(trig, naval_movement_order)
    end)

Of course I'm not 100% certain about what you're trying to achieve so there could be some mistakes.

Anyway, this code avoids the memory leak from using GetUnitLoc(u), it avoids all BJ methods, and fixes your flawed logic when calculating speed -> "RVM.actualspeed[cv] ~= RVM.maxspeed[cv]", you shouldn't be making exact comparisons like that when working with Reals (look up floating point error), and if these values are Integers then it still seems incorrect since acceleration could bypass maxspeed and continue on infinitely. Locations are really unnecessary and you're holding yourself back when using them, the beauty of Jass/Lua is that you can work with coordinates directly.