From 150cef9399a57e14bece8736ac3e12f38b543c75 Mon Sep 17 00:00:00 2001 From: luk3yx Date: Tue, 13 Aug 2024 10:20:51 +1200 Subject: [PATCH] Change prefix of auto-generated field names Fixes behaviour with https://github.com/minetest/minetest/pull/14878 I've also added a workaround to preserve the current behaviour of dropdowns that don't have index_event set. --- .luacheckrc | 9 +++++---- README.md | 4 ++++ embed.lua | 2 +- init.lua | 43 +++++++++++++++++++++++++++++++++---------- test.lua | 52 ++++++++++++++++++++++++++-------------------------- 5 files changed, 69 insertions(+), 41 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index 49f9b7e..78cb70e 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -1,14 +1,15 @@ max_line_length = 80 globals = { - 'formspec_ast', - 'minetest', - 'hud_fs', 'flow', - 'dump', } read_globals = { + 'dump', + 'formspec_ast', + 'fs51', + 'hud_fs', + 'minetest', string = {fields = {'split', 'trim'}}, table = {fields = {'copy', 'indexof'}} } diff --git a/README.md b/README.md index 749b918..f6ce064 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,8 @@ Vaguely inspired by Flutter and GTK. - This mod doesn't support all of the features that regular formspecs do. - [FS51](https://content.minetest.net/packages/luk3yx/fs51/) is required if you want to have full support for Minetest 5.3 and below. + - Make sure you're using the latest version of flow if you are on MT 5.10-dev + or later, older versions used a hack which no longer works. ## Basic example @@ -593,4 +595,6 @@ local parent_form = flow.make_gui(function(player, ctx) end) ``` +Special characters (excluding `-` and `_`) are not allowed in embed names. + diff --git a/embed.lua b/embed.lua index 151594c..c99b8af 100644 --- a/embed.lua +++ b/embed.lua @@ -78,7 +78,7 @@ return function(self, fields) return self._build(player, parent_ctx) end - local prefix = "\2" .. name .. "\2" + local prefix = "_#" .. name .. "#" local child_ctx = embed_create_ctx(parent_ctx, name, prefix) change_ctx(child_ctx) local root_node = self._build(player, child_ctx) diff --git a/init.lua b/init.lua index b36a144..d833e11 100644 --- a/init.lua +++ b/init.lua @@ -317,7 +317,7 @@ function align_types.fill(node, x, w, extra_space) node[1] = { type = "style", -- MT 5.1.0 only supports one style selector - selectors = {"\1"}, + selectors = {"_#"}, -- bgimg_pressed is included for 5.1.0 support -- bgimg_hovered is unnecessary as it was added in 5.2.0 (which @@ -329,7 +329,7 @@ function align_types.fill(node, x, w, extra_space) -- removed node[2] = { type = "style", - selectors = {"\1:hovered", "\1:pressed"}, + selectors = {"_#:hovered", "_#:pressed"}, props = {bgimg = ""}, } @@ -339,7 +339,7 @@ function align_types.fill(node, x, w, extra_space) drawborder = false, x = 0, y = 0, w = node.w + extra_space, h = node.h, - name = "\1", label = node.label, + name = "_#", label = node.label, style = node.style, } @@ -350,7 +350,7 @@ function align_types.fill(node, x, w, extra_space) drawborder = false, x = 0, y = 0, w = node.w + extra_space, h = node.h, - name = "\1", label = "", + name = "_#", label = "", } node.y = node.y - LABEL_OFFSET @@ -639,12 +639,32 @@ function field_value_transformers.tabheader(node) return range_check_transformer(node.captions and #node.captions or 0) end -function field_value_transformers.dropdown(node) +function field_value_transformers.dropdown(node, _, formspec_version) local items = node.items or {} - if node.index_event then + if node.index_event and not node._index_event_hack then return range_check_transformer(#items) end + -- MT will start sanitising formspec fields on its own at some point + -- (https://github.com/minetest/minetest/pull/14878), however it may strip + -- escape sequences from dropdowns as well. Since we know what the actual + -- value of the dropdown is anyway, we can just enable index_event for new + -- clients and keep the same behaviour + if (formspec_version and formspec_version >= 4) or + (minetest.global_exists("fs51") and fs51.monkey_patching_enabled) then + node.index_event = true + + -- Detect reuse of the same Dropdown element (this is unsupported and + -- will break in other ways) + node._index_event_hack = true + + return function(value) + return items[tonumber(value)] + end + elseif node._index_event_hack then + node.index_event = nil + end + -- Make sure that the value sent by the client is in the list of items return function(value) if table.indexof(items, value) > 0 then @@ -708,7 +728,7 @@ local button_types = { -- Removes on_event from a formspec_ast tree and returns a callbacks table local function parse_callbacks(tree, ctx_form, auto_name_id, - replace_backgrounds) + replace_backgrounds, formspec_version) local callbacks local btn_callbacks = {} local saved_fields = {} @@ -809,7 +829,8 @@ local function parse_callbacks(tree, ctx_form, auto_name_id, local get_transformer = field_value_transformers[node.type] saved_fields[node_name] = get_transformer and - get_transformer(node, tablecolumn_count) or + get_transformer(node, tablecolumn_count, + formspec_version) or default_field_value_transformer end end @@ -818,7 +839,9 @@ local function parse_callbacks(tree, ctx_form, auto_name_id, if node.on_event then local is_btn = button_types[node.type] if not node_name then - node_name = ("\1%x"):format(auto_name_id) + -- Flow internal field names start with "_#" to avoid + -- conflicts with user-provided fields. + node_name = ("_#%x"):format(auto_name_id) node.name = node_name auto_name_id = auto_name_id + 1 elseif btn_callbacks[node_name] or @@ -1043,7 +1066,7 @@ function Form:_render(player, ctx, formspec_version, id1, embedded, lang_code) local tree = render_ast(box, embedded) local callbacks, btn_callbacks, saved_fields, id2 = parse_callbacks( - tree, orig_form, id1, embedded + tree, orig_form, id1, embedded, formspec_version ) -- This should be after parse_callbacks so it can take advantage of diff --git a/test.lua b/test.lua index 059c89d..c26f86b 100644 --- a/test.lua +++ b/test.lua @@ -81,12 +81,12 @@ string.split = string.split or function(str, chr) end function minetest.explode_textlist_event(event) - local event_type, number = event:match("^([A-Z]+):(%d+)$") + local event_type, number = event:match("^([A-Z]+):(%d+)#") return {type = event_type, index = tonumber(number) or 0} end function minetest.explode_table_event(event) - local event_type, row, column = event:match("^([A-Z]+):(%d+):(%d+)$") + local event_type, row, column = event:match("^([A-Z]+):(%d+):(%d+)#") return {type = event_type, row = tonumber(row) or 0, column = tonumber(column) or 0} end @@ -352,7 +352,7 @@ describe("Flow", function() gui.List{inventory_location = "a", list_name = "b", w = 2, h = 2}, gui.Style{selectors = {"test"}, props = {prop = "value"}}, - }, ([[ + }, [[ size[3.6,3.6] button[0.3,0.3;3,1;;1] image[0.3,0.3;3,3;2] @@ -361,14 +361,14 @@ describe("Flow", function() field_close_on_enter[5;false] field[0.3,0.3;3,3;5;;] - style[\1;bgimg=;bgimg_pressed=] - style[\1:hovered,\1:pressed;bgimg=] - image_button[0.3,1.6;3,0.4;blank.png;\1;Test;;false] - image_button[0.3,1.6;3,0.4;blank.png;\1;;;false] + style[_#;bgimg=;bgimg_pressed=] + style[_#:hovered,_#:pressed;bgimg=] + image_button[0.3,1.6;3,0.4;blank.png;_#;Test;;false] + image_button[0.3,1.6;3,0.4;blank.png;_#;;;false] list[a;b;0.675,0.675;2,2] style[test;prop=value] - ]]):gsub("\\1", "\1")) + ]]) end) it("ignores gui.Nil", function() @@ -816,11 +816,11 @@ describe("Flow", function() test_render(gui.Button{ w = 1, h = 1, on_event = function() end, style = {hello = "world"} - }, ([[ + }, [[ size[1.6,1.6] - style[\10;hello=world] + style[_#0;hello=world] button[0.3,0.3;1,1;\10;] - ]]):gsub("\\1", "\1")) + ]]) end) it("supports advanced selectors", function() @@ -951,7 +951,7 @@ describe("Flow", function() gui.Label{label = "This is the embedded form!"}, -- The exact prefix is an implementation detail, you -- shouldn't rely on this in your own code - gui.Field{name = "\2theprefix\2test2"}, + gui.Field{name = "_#theprefix#test2"}, }, gui.Label{label = "ffaksksdf"} }) @@ -997,7 +997,7 @@ describe("Flow", function() gui.Label{label = "asdft"}, gui.VBox{ gui.Label{label = "This is the embedded form!"}, - gui.Field{name = "\2theprefix\2test2"}, + gui.Field{name = "_#theprefix#test2"}, gui.Label{label = "A is true! WOW!"} }, gui.Label{label = "ffaksksdf"} @@ -1005,7 +1005,7 @@ describe("Flow", function() end) it("flow form context table", function() test_render(function(p, x) - x.form["\2the_name\2jkl"] = 3 + x.form["_#the_name#jkl"] = 3 local child = flow.make_gui(function(_p, xc) xc.form.thingy = true xc.form.jkl = 9 @@ -1014,8 +1014,8 @@ describe("Flow", function() player = p, name = "the_name" } - assert.True(x.form["\2the_name\2thingy"]) - assert.equal(9, x.form["\2the_name\2jkl"]) + assert.True(x.form["_#the_name#thingy"]) + assert.equal(9, x.form["_#the_name#jkl"]) return child end, gui.Label{label = "asdf"}) end) @@ -1026,7 +1026,7 @@ describe("Flow", function() return e end, gui.VBox{ gui.Label{label = "This is the embedded form!"}, - gui.Field{name = "\2asdf\2test2"}, + gui.Field{name = "_#asdf#test2"}, gui.Box{w = 1, h = 3} }) end) @@ -1059,8 +1059,8 @@ describe("Flow", function() local player, ctx = wrapped_p, state.ctx state.callbacks.quit(player, ctx) - state.callbacks["\2thesubform\2field"](player, ctx) - state.btn_callbacks["\2thesubform\2btn"](player, ctx) + state.callbacks["_#thesubform#field"](player, ctx) + state.btn_callbacks["_#thesubform#btn"](player, ctx) assert.same(state.ctx.thesubform, wrapped_x) @@ -1073,8 +1073,8 @@ describe("Flow", function() -- Each of these are wrapped with another function to put the actual function in the correct environment assert.Not.same(func_quit, state.callbacks.quit) - assert.Not.same(func_field_event, state.callbacks["\2thesubform\2field"]) - assert.Not.same(func_btn_event, state.callbacks["\2thesubform\2btn"]) + assert.Not.same(func_field_event, state.callbacks["_#thesubform#field"]) + assert.Not.same(func_btn_event, state.callbacks["_#thesubform#btn"]) end) describe("metadata", function() it("style data is modified", function() @@ -1086,7 +1086,7 @@ describe("Flow", function() test_render(function(p, _x) return style_embedded_form:embed{player = p, name = "asdf"} end, gui.VBox{ - gui.Style{selectors = {"\2asdf\2test"}, props = {prop = "value"}}, + gui.Style{selectors = {"_#asdf#test"}, props = {prop = "value"}}, }) end) it("scroll_container data is modified", function() @@ -1098,7 +1098,7 @@ describe("Flow", function() test_render(function(p, _x) return scroll_embedded_form:embed{player = p, name = "asdf"} end, gui.VBox{ - gui.ScrollContainer{scrollbar_name = "\2asdf\2name"} + gui.ScrollContainer{scrollbar_name = "_#asdf#name"} }) end) it("tooltip data is modified", function() @@ -1110,7 +1110,7 @@ describe("Flow", function() test_render(function(p, _x) return tooltip_embedded_form:embed{player = p, name = "asdf"} end, gui.VBox{ - gui.Tooltip{gui_element_name = "\2asdf\2lololol"} + gui.Tooltip{gui_element_name = "_#asdf#lololol"} }) end) end) @@ -1130,7 +1130,7 @@ describe("Flow", function() assert.same("new value!", x.asdf.field, "values that it set set are here") return subform end, gui.VBox{ - gui.Field{name = "\2asdf\2field"} + gui.Field{name = "_#asdf#field"} }) end) it("supports fresh initial form values", function() @@ -1148,7 +1148,7 @@ describe("Flow", function() end return tooltip_embedded_form:embed{player = p, name = "asdf"} end, gui.VBox{ - gui.Field{name = "\2asdf\2field"} + gui.Field{name = "_#asdf#field"} }) end) it("updates flow.get_context", function()