Delay() to return cancellable object

User Story:

As a Roblox developer, it is currently too messy to implement a cancellable delay.

Use Case:

Take this case study as an example. We needed to interrupt the user’s sprint if they were sprinting for too long (exploiting or high latency), and we needed to start to regenerate their stamina one second after they had stopped sprinting.

The implementation of that functionality required us to keep track of the times events fired to prevent delayed code from running when its process had completed. With the ability to cancel delay(), we no longer need to do that.

Current Implementation:
local delays = {}
sprintRequestStart.OnServerEvent:Connect(function(invoker)
    local maxSprintTime = getSprintTimeAllowed(invoker)
    local start = tick()
    markAllowedForSprint(invoker, start)

    delays[invoker] = start
    delay(maxSprintTime, function()
        if delays[invoker] == start then
            markDisallowedForSprint(invoker)
        end
    end) 
end)

sprintRequestStop.OnServerEvent:Connect(function(invoker)
    local start = tick()
    markDisallowedForSprint(invoker)
    markWaitingForRegen(invoker, start)

    delays[invoker] = start
    delay(regenCooldown, function()
        if delays[invoker] == start then
            markForRegen(invoker)
        end
    end)
end)
Ideal Implementation:
local delays = {}
sprintRequestStart.OnServerEvent:Connect(function(invoker)
    if delays[invoker] then delays[invoker].cancel() end

    local maxSprintTime = getSprintTimeAllowed(invoker)
    markAllowedForSprint(invoker)

    delays[invoker] = delay(maxSprintTime, function()
        markDisallowedForSprint(invoker)
    end) 
end)

sprintRequestStop.OnServerEvent:Connect(function(invoker)
    delays[invoker].cancel()

    markDisallowedForSprint(invoker)

    delays[invoker] = delay(regenCooldown, function()
        markForRegen(invoker)
    end)
end)

Benefits of Resolving:

If Roblox is able to address this issue, it would improve my development experience by allowing me to avoid the unnecessary mental overhead introduced by this awkward workaround. As seen in the second example, the ability to cancel a delay() results in a clear, logical flow that doesn’t obscure the true intention of the source.

Suggestions:

  1. Cancelling a delay that already finished should not error, much like disconnecting an already-disconnected event doesn’t error
  2. Using a generic cancel function i.e. local d = delay(...) cancel(d) would simplify the above code by not requiring me to check if there was an existing delay in the first event. It could also be used for a JS setInverval equivalent if one is ever implemented
13 Likes

If possible, I’d like to make a suggestion as to implement other arguments to delay, such as delay(10, BindableEvent, ...) to Fire the bindable after 10 seconds with vararg arguments.

Or just delay(time, function, ...) (and if possible spawn(function, ...)) so we may be able to pass arguments to the function and have better control without needing to use upvalues and instantiate a new proto every time.

3 Likes

Seems like this need could be met by exposing a lightweight signal scheduler like a timer.

I was looking over that feature request earlier, but with the overhead it introduces, it wouldn’t be much better than the current behavior. Since the timer is just a timer, I have to manually schedule relevant code to run when the timer expires. Compare:

Current Approach:

local d = nil

do
    local start = tick()
    d = start
    delay(3, function()
        if d == start then
            ...
        end
    end)
end

d = nil

Timer Object:

local timer = nil

do
    timer = Timer.new()

    spawn(function()
        timer.Fired:wait() --This sticks around forever if timer never fires
        ...
    end)

    t:Start(3)
end

timer:Stop()

Cancellable Delay

local d = nil

do
    d = delay(3, function()
        ...
    end)
end

cancel(d)
1 Like

Is there some reason you can’t just add a simple check once the delayed function runs? :no_mouth:

See Second paragraph onwards of Use Case and Benefits of Resolving

1 Like

I have since decided that the timer wasn’t so great, and opted for a cancellable delay. I even wrote an implementation:

The only difference is that the cancelling trigger is just a function. I agree that it shouldn’t error if the delay is already finished. My gut feeling is that having any knowledge of the delay’s state would end up being an antipattern.

3 Likes

Just a quick note in case you never noticed:
local cancel should probably be local cancelTimeout in SignalTimeout.

Still really want a method to remove the current/given thread from the taskscheduler(queue). Removing a thread from the queue should prevent wait() from ever returning, while it’ll get GCed as it isn’t referenced anywhere anymore.

On the note of GC, given this code:

local BE = Instance.new("BindableEvent")
delay(1,function() BE:Destroy() end)
BE.Event:wait()

Would there still be a memory leak? The :Destroy() disconnects all connections and cancels the :wait(), so is there a memory leak? the :wait() is basically coroutine.yield() with the coroutine never being resumed.

Time for Testing

local BE = Instance.new("BindableEvent")

local co = setmetatable({},{__mode="v"})
co[1] = coroutine.create(function()
	print("A")
	BE.Event:Wait()
	print("B")
	BE.Event:Wait()
	print("C")
end)
coroutine.resume(co[1])

print(1,#co==0 and "GCed" or "") wait(1) -- should never be GCed
BE:Fire() wait(1)
print(2,#co==0 and "GCed" or "") wait(1) -- should never be GCed
BE:Destroy() wait(1)
for i=3,20 do
	print(i,#co==0 and "GCed" or "")
	if #co==0 then break end wait(1)
end

A
1 
B
2 
3 
4 
5 
6 GCed

Here’s a list of all the numbers from when GCed appeared after running the test 10x one after another:
6, 16, 3, 18, 4, 3, 3, 4, 3, 5 (either GC in an empty place is slower now, or it takes longer than 1 cycle)

So the timers using :Wait() apparently don’t have a thread leak. This might’ve been different before, but I doubt it. I’m not sure anyone actually tested it before anyway.

1 Like

That is fantastic! I was hoping it was like that, but I wasn’t sure.

I just tried something similar but changed BE:Destroy() to BE = nil and it worked just as well: coroutines created by events are also GCed when the event’s instance is GCed.

I have used Cord for iterators, but wasn’t sure if it was actually safe too.

1 Like

Oh, I didn’t really expect BE = nil to make BE GCable. I’ve even added a (non-GCed) connection now:


local BE = Instance.new("BindableEvent")

local function gc(o)
	return setmetatable({o},{__mode="v",__unm=function(s) return #s==0 end})
end

local co = gc(coroutine.create(function()
	print("A")
	BE.Event:Wait()
	print("B")
	BE.Event:Wait()
	print("C")
end))
coroutine.resume(co[1])

local con = BE.Event:Connect(warn)
local be = gc(BE)

print(1,-co and "GCed" or "") wait(1)
BE:Fire() wait(1)
print(2,-co and "GCed" or "") wait(1)
BE = nil wait(1)
for i=3,20 do
	print(i,-co and "GCed" or "",con.connected,-be and "BE_GCed" or "")
	if -co and -be then break end wait(1)
end

and it disconnects the connections/waits when BE is GCed:

A
1 
13:02:37.373 - 
B
2 
3  true 
4  true 
5  true 
6  false BE_GCed
7  false BE_GCed
8  false BE_GCed
9  false BE_GCed
10  false BE_GCed
11  false BE_GCed
12  false BE_GCed
13  false BE_GCed
14 GCed false BE_GCed

Which is weird, as I thought connections/waits prevented the instance from being GCed. on the other hand, since the BE variable is gone, there’s no way for the connection handler to access BE, unless it’s still in the DataModel, so kind of makes sense

Basically Lua’s GC combined with roblox’ events is more impressive than I thought.

1 Like

This would simplify some of my UI code relating to timers and countdown stuff without having to do a ton of variable messiness