diff --git a/init.lua b/init.lua index aefcebf..bfc8919 100644 --- a/init.lua +++ b/init.lua @@ -701,15 +701,20 @@ local default_value_fields = { tabheader = "current_tab", } - local sensible_defaults = { default = "", selected = false, selected_idx = 1, value = 0, } +local button_types = { + button = true, image_button = true, item_image_button = true, + button_exit = true, image_button_exit = true +} + -- 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) - local callbacks = {} + local callbacks + local btn_callbacks = {} local saved_fields = {} local tablecolumn_count = 1 for node in formspec_ast.walk(tree) do @@ -810,13 +815,34 @@ local function parse_callbacks(tree, ctx_form, auto_name_id, -- Add the on_event callback (if any) to the callbacks table 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) node.name = node_name auto_name_id = auto_name_id + 1 + elseif btn_callbacks[node_name] or + (is_btn and saved_fields[node_name]) or + (callbacks and callbacks[node_name]) then + minetest.log("warning", ("[flow] Multiple callbacks have " .. + "been registered for elements with the same name (%q), " .. + "this will not work properly."):format(node_name)) + + -- Preserve previous behaviour + btn_callbacks[node_name] = nil + if callbacks then + callbacks[node_name] = nil + end + is_btn = is_btn and not saved_fields[node_name] end - callbacks[node_name] = node.on_event + -- Put buttons into a separate callback table so that malicious + -- clients can't send multiple button presses in one submission + if is_btn then + btn_callbacks[node_name] = node.on_event + else + callbacks = callbacks or {} + callbacks[node_name] = node.on_event + end node.on_event = nil end @@ -826,7 +852,7 @@ local function parse_callbacks(tree, ctx_form, auto_name_id, node._after_positioned = nil end end - return callbacks, saved_fields, auto_name_id + return callbacks, btn_callbacks, saved_fields, auto_name_id end local gui_mt = { @@ -903,14 +929,15 @@ function Form:_render(player, ctx, formspec_version, id1, embedded, lang_code) if not id1 or id1 > 1e6 then id1 = 0 end local tree = render_ast(box, embedded) - local callbacks, saved_fields, id2 = parse_callbacks(tree, orig_form, id1, - embedded) + local callbacks, btn_callbacks, saved_fields, id2 = parse_callbacks( + tree, orig_form, id1, embedded + ) local redraw_if_changed = {} for var in pairs(used_ctx_vars) do -- Only add it if there is no callback and the name exists in the -- formspec. - if saved_fields[var] and not callbacks[var] then + if saved_fields[var] and (not callbacks or not callbacks[var]) then redraw_if_changed[var] = true end end @@ -920,6 +947,7 @@ function Form:_render(player, ctx, formspec_version, id1, embedded, lang_code) return tree, { self = self, callbacks = callbacks, + btn_callbacks = btn_callbacks, saved_fields = saved_fields, redraw_if_changed = redraw_if_changed, ctx = ctx, @@ -1093,6 +1121,7 @@ end -- Declared locally above to be accessible to render_to_formspec_string function fs_process_events(player, form_info, fields) local callbacks = form_info.callbacks + local btn_callbacks = form_info.btn_callbacks local ctx = form_info.ctx local redraw_if_changed = form_info.redraw_if_changed local ctx_form = ctx.form @@ -1128,9 +1157,26 @@ function fs_process_events(player, form_info, fields) end -- Run on_event callbacks + -- The callbacks table may be nil as adding callbacks to non-buttons is + -- likely uncommon (so allocating an empty table would be useless) + if callbacks then + for field in pairs(fields) do + if callbacks[field] and callbacks[field](player, ctx) then + redraw_fs = true + end + end + end + + -- Run button callbacks after all other callbacks as that seems to be the + -- most intuitive thing to do + -- Note: Try not to rely on the order of on_event callbacks, I may change + -- it in the future. for field in pairs(fields) do - if callbacks[field] and callbacks[field](player, ctx) then - redraw_fs = true + if btn_callbacks[field] then + redraw_fs = btn_callbacks[field](player, ctx) or redraw_fs + + -- Only run a single button callback + break end end diff --git a/test.lua b/test.lua index b7e25f4..8710b62 100644 --- a/test.lua +++ b/test.lua @@ -251,15 +251,18 @@ describe("Flow", function() it("registers callbacks", function() local function func() end + local function func2() return true end local tree, state = render(function(player, ctx) return gui.VBox{ gui.Label{label = "Callback demo:"}, gui.Button{label = "Click me!", name = "btn", on_event = func}, + gui.Field{name = "field", on_event = func2} } end) - assert.same(state.callbacks, {btn = func}) + assert.same(state.callbacks, {field = func2}) + assert.same(state.btn_callbacks, {btn = func}) end) it("handles visible = false", function() @@ -697,11 +700,38 @@ describe("Flow", function() end) end) - it("does not save form input for Button", function() - local ctx, event = render_to_string(gui.Button{name = "a"}) - assert.equals(ctx.form.a, nil) - event({a = "test"}) - assert.equals(ctx.form.a, nil) + describe("Button", function() + it("does not save form input", function() + local ctx, event = render_to_string(gui.Button{name = "a"}) + assert.equals(ctx.form.a, nil) + event({a = "test"}) + assert.equals(ctx.form.a, nil) + end) + + it("only calls a single callback", function() + local f, b = 0, 0 + + local ctx, event = render_to_string(gui.VBox{ + gui.Field{name = "a", on_event = function() f = f + 1 end}, + gui.Button{name = "b", on_event = function() b = b + 1 end}, + gui.Button{name = "c", on_event = function() b = b + 1 end} + }) + event({}) + assert.equals(f, 0) + assert.equals(b, 0) + + event({a = "test", b = "test", c = "test"}) + assert.equals(f, 1) + assert.equals(b, 1) + + event({b = "test", c = "test"}) + assert.equals(f, 1) + assert.equals(b, 2) + + event({c = "test"}) + assert.equals(f, 1) + assert.equals(b, 3) + end) end) end) end)