From 499dbb88f7fd4314b42e96027dcc764cb8529830 Mon Sep 17 00:00:00 2001 From: saibotk Date: Mon, 16 Oct 2023 23:59:23 +0200 Subject: [PATCH 1/2] fix: Combobox ConVar change callback deletion 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. --- .../client/cl_vskin/vgui/dcombobox_ttt2.lua | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/gamemodes/terrortown/gamemode/client/cl_vskin/vgui/dcombobox_ttt2.lua b/gamemodes/terrortown/gamemode/client/cl_vskin/vgui/dcombobox_ttt2.lua index 5eed77dce..715f29a5e 100644 --- a/gamemodes/terrortown/gamemode/client/cl_vskin/vgui/dcombobox_ttt2.lua +++ b/gamemodes/terrortown/gamemode/client/cl_vskin/vgui/dcombobox_ttt2.lua @@ -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 --- From 2340526a6a776fae65c5732f5421c9e342efaa3e Mon Sep 17 00:00:00 2001 From: saibotk Date: Tue, 17 Oct 2023 00:02:19 +0200 Subject: [PATCH 2/2] chore: Add changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3addc7f9..6a814e5cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ All notable changes to TTT2 will be documented here. Inspired by [keep a changel - Scoreboard now sets preferred player volume and mute state in client's new `ttt2_voice` table (by @EntranceJew) - Keyed by steamid64, making it more reliable than UniqueID or the per-session mute and volume levels. +### Fixed + +- Fixed removing the convar change callback in `DComboboxTTT2` (by @saibotk) + ## [v0.11.7b](https://github.com/TTT-2/TTT2/tree/v0.11.7b) (2022-08-27) ### Added