Skip to content

Commit

Permalink
fix: Combobox ConVar change callback deletion
Browse files Browse the repository at this point in the history
Fixes #1068

We need to remove the callback in a timer, because otherwise the ConVar change callback code
will throw an error while looping over the callbacks.
This happens, because the callback is removed from the same table that is iterated over.
Thus, the table size changes while iterating over it and leads to a nil callback as the last entry.

See https://github.com/Facepunch/garrysmod/blob/1b512930d1f8fb1acf6235e584c4ec1ff84e9362/garrysmod/lua/includes/modules/cvars.lua#L44
and
https://github.com/Facepunch/garrysmod/blob/1b512930d1f8fb1acf6235e584c4ec1ff84e9362/garrysmod/lua/includes/modules/cvars.lua#L97

here the code uses table.Remove, which causes the table to be reindexed and resized, this is the root cause for the problem.
We should thus avoid removing callbacks in the callback itself.
  • Loading branch information
saibotk committed Oct 16, 2023
1 parent 0270844 commit 499dbb8
Showing 1 changed file with 16 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -338,23 +338,29 @@ end

local convarTracker = 0
---
-- @param Panel menu to set the value of
-- @param Panel panel to set the value of
-- @param string conVar name of the convar
local function AddConVarChangeCallback(menu, conVar)
convarTracker = convarTracker % 1023 + 1
local myIdentifierString = "TTT2F1MenuConVarChangeCallback" .. tostring(convarTracker)

local function OnConVarChangeCallback(conVarName, oldValue, newValue)
if not IsValid(menu) then
cvars.RemoveChangeCallback(conVarName, myIdentifierString)
local function AddConVarChangeCallback(panel, conVar)
convarTracker = convarTracker + 1
local myIdentifierString = "TTT2F1MenuComboboxConVarChangeCallback" .. tostring(convarTracker)

local callback = function(conVarName, oldValue, newValue)
if not IsValid(panel) then
-- We need to remove the callback in a timer, because otherwise the ConVar change callback code
-- will throw an error while looping over the callbacks.
-- This happens, because the callback is removed from the same table that is iterated over.
-- Thus, the table size changes while iterating over it and leads to a nil callback as the last entry.
timer.Simple(0, function()
cvars.RemoveChangeCallback(conVarName, myIdentifierString)
end)

return
end

menu:SetValue(newValue, true)
panel:SetValue(newValue, true)
end

cvars.AddChangeCallback(conVar, OnConVarChangeCallback, myIdentifierString)
cvars.AddChangeCallback(conVar, callback, myIdentifierString)
end

---
Expand Down

0 comments on commit 499dbb8

Please sign in to comment.