beyond-all-reason / beyond-all-reason/RecoilEngine

Handlers of Script.DelayByFrames can't be unregistered

Open
#2,568 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
679
Forks
290
Avg merge
3d 2h
Merged PRs (30d)
40

Description

## Why

You are not able to cancel a delayed callback currently, this forces gamedevs to implement their own cancel logic when their feature requires, making it more bug prone and unnecessarily increasing overhead.

In some cases they can do their own cancelling logic, which is bug-prone, if they don't have a wrapper for safe operation cancelling, e.g.:

```lua
-- cancellable_delay.lua

local currHandleId = 1 -- ideally some uuid, but assuming we can recycle safely on MAX_HANDLES
local handleIds = {}

local function newHandleId()
local foundEmptyHandle

for _=0, MAX_HANDLES do
currHandleId = (currHandle % MAX_HANDLES) + 1 -- note handleIds is 1-indexed

if not handleIds[currHandle]
foundEmptyHandle = true
handleIds[currHandleId] = true
break
end
end

if not foundEmptyHandle then
return nil
end

return currHandle
end

local function delayedByFrames(handleId, callback)
-- note handleId can get recycled if the handle intended for this handle id was cancelled early
-- we rely on big enough MAX_POOL or we should find a true uuid index.
-- alternatively we use ttl versioning of handleids but this is just for illustrative purposes
if not handleIds[handleId] then
return
end

callback()
handleIds[handleId] = nil
end

function cancelDelay(handleId)
handleIds[handleId] = nil
end

--- @return int? id `nil` when the handle pool is full
function cancellableDelay(delay, callback)
local handleId = newHandleId()

if not handleId then
return
end

Script.DelayByFrames(DELAY, function() delayedByFrames(handleId, callback) end)

return handleId
end
```

```lua
-- some_widget.lua

local unitDelays = {}

local function something(unitId)
-- do something

local delayHandle = newHandle()
unitDelays[unitId] = delayHandle

local delayId = cancellableDelay(UNIT_DELAY, function()
somethingElse(unitId)

unitDelays[unitId] = nil
end)

if not delayId then
error "Cancellable operations pool full, consider increasing MAX_HANDLES or check if you're leaking handles"
end

unitDelays[unitId] = delayId
end

function widget:UnitDestroyed(unitId)
-- we shouldnt perform op on unit that doesnt exist anymore.
-- and unitId can get recycled so Spring.IsValidUnit(unitId) is not reliable
cancelHandle(unitDelayHandles[unitId])
end
```

## Proposal

`Spring.DelayByFrames` should return a `cancelDelay` func or `delayId` so games have a much easier time. Advantage of returning id is space economy, but requires an additional callin and vice-versa for returning func, e.g.:

```lua
local unitDelays = {}

local function doSomething(unitId)
unitDelays[unitId] = Script.DelayByFrames(DELAY, function()
doSomethingElse(unitId)
unitDelays[unitId] = nil
end)
end

function widget:UnitDestroyed(unitId)
unitDelays[unitId] = nil

Script.DelayedCancel(unitDelays[unitId]) -- DelayedCancel is noop on invalid delayId
end
```

Contributor guide

Open the contributing guide

Research direction

Start by locating the implementation and Lua binding for Script.DelayByFrames, then inspect how delayed callbacks are stored and dispatched. Resolve whether cancellation should use a returned function or delay ID, implement the chosen API, and verify that a cancelled callback no longer runs, including behavior for invalid handles.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, lua
Domain
game-dev
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.