From 9992a26e7d6d9504c0018346b7911bec2047be35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tam=C3=A1s=20B=C3=A1lint=20Misius?= Date: Wed, 19 Mar 2025 19:33:58 +0100 Subject: [PATCH] Fix elem.loadDefault handling element tools wrong Firstly, it would not remove custom element tools when appropriate, reproduce with elem.allocate("CUSTOM", "FAKE") elem.element(elem.CUSTOM_PT_FAKE, elem.element(elem.DEFAULT_PT_SLCN)) elem.loadDefault() elem.allocate("CUSTOM", "FAKE") elem.element(elem.CUSTOM_PT_FAKE, elem.element(elem.DEFAULT_PT_SLCN)) There will be two FAKE element tools, one of them in the Tools section, called NULL, and looking very default. Broken since ff4500620e56, where I assumed that builtinElements's size reflected the amount of enabled built-in elements. This has not been the case at all since dd875987b92b, where GetElements was made to return a PT_NUM-sized array instead of a vector with only built-in elements, and so its size increased to PT_NUM, including all possible element numbers. But, strictly speaking, even before that it had not been a correct assumption, because some entries (e.g. 146) of builtinElements could have been disabled, but were possible to allocate from Lua. Secondly, it would not update built-in element tools when appropriate, reproduce with elem.property(elem.DEFAULT_PT_SLCN, "Name", "FAKE") elem.loadDefault() SLCN's tool will still be called FAKE. Also broken since ff4500620e56, where I neglected to deal with element tools whose elements got updated under them. At the time this would have been done by calling AllocElementTool on them again, but today the appropriate way is UpdateElementTool. --- src/lua/LuaElements.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/lua/LuaElements.cpp b/src/lua/LuaElements.cpp index 960178849..2392b3054 100644 --- a/src/lua/LuaElements.cpp +++ b/src/lua/LuaElements.cpp @@ -787,13 +787,22 @@ static int loadDefault(lua_State *L) lua_settable(L, -3); manageElementIdentifier(L, id, false); - if (id < (int)builtinElements.size()) + auto oldEnabled = elements[id].Enabled; + if (id < (int)builtinElements.size() && builtinElements[id].Enabled) { elements[id] = builtinElements[id]; } else { elements[id] = Element(); + } + // TODO: somehow unify element and corresponding element tool management in a way that makes it hard to mess up + if (oldEnabled && elements[id].Enabled) + { + lsi->gameModel->UpdateElementTool(id); + } + else if (oldEnabled && !elements[id].Enabled) + { lsi->gameModel->FreeTool(lsi->gameModel->GetToolFromIdentifier(identifier)); } manageElementIdentifier(L, id, true);