Implement a new Lua API to the undo system

data/lua/diversion.lua

@@ -1,7 +1,6 @@
 
 local _ = wesnoth.textdomain 'wesnoth-help'
 local T = wml.tag
-local on_event = wesnoth.require("on_event")
 
 local u_pos_filter = function(u_id)
 
@@ -37,8 +36,7 @@ local u_pos_filter = function(u_id)
         end
 end
 
-
-local status_anim_update = function(is_undo)
+local function status_anim_update(is_undo)
 
         local ec = wesnoth.current.event_context
         local changed_something  = false
@@ -88,20 +86,16 @@ local status_anim_update = function(is_undo)
                     }
                 end
         end
-        if changed_something and not is_undo then
-                wesnoth.wml_actions.on_undo {
-                        wml.tag.on_undo_diversion {
-                        }
-                }
+        if not is_undo then
+                wesnoth.game_events.set_undoable(true)
+                if changed_something then
+                        wesnoth.game_events.add_undo_actions(function(_)
+                                status_anim_update(true)
+                        end)
+                end
         end
 end
 
-function wesnoth.wml_actions.on_undo_diversion(cfg)
-        status_anim_update(true)
-end
-
-on_event("moveto, die, recruit, recall", function()
-        status_anim_update()
-
+wesnoth.game_events.add_repeating("moveto, die, recruit, recall", function(_)
+        status_anim_update(false)
 end)
-

data/lua/on_event.lua

@@ -51,5 +51,5 @@ local function on_event(eventname, priority, fcn)
 	end
 end
 
-core_on_event = on_event
+core_on_event = wesnoth.deprecate_api('on_event', 'wesnoth.game_events.add_repeating', 1, nil, on_event)
 return on_event

data/lua/wml-tags.lua

@@ -645,7 +645,11 @@ function wml_actions.put_to_recall_list(cfg)
 end
 
 function wml_actions.allow_undo(cfg)
-	wesnoth.allow_undo()
+	wesnoth.game_events.set_undoable(true)
+end
+
+function wml_actions.disallow_undo(cfg)
+	wesnoth.game_events.set_undoable(false)
 end
 
 function wml_actions.allow_end_turn(cfg)
@@ -1025,3 +1029,11 @@ function wml_actions.progress_achievement(cfg)
 
 	wesnoth.achievements.progress(cfg.content_for, cfg.id, cfg.amount, tonumber(cfg.limit) or 999999999)
 end
+
+function wml_actions.on_undo(cfg)
+	if cfg.delayed_variable_substitution then
+		wesnoth.game_events.add_undo_actions(wml.literal(cfg));
+	else
+		wesnoth.game_events.add_undo_actions(wml.parsed(cfg));
+	end
+end

src/actions/undo_action.cpp

@@ -29,6 +29,27 @@
 namespace actions
 {
 
+undo_event::undo_event(int fcn_idx, const config& args, const game_events::queued_event& ctx)
+	: lua_idx(fcn_idx)
+	, commands(args)
+	, data(ctx.data)
+	, loc1(ctx.loc1)
+	, loc2(ctx.loc2)
+	, filter_loc1(ctx.loc1.filter_loc())
+	, filter_loc2(ctx.loc2.filter_loc())
+	, uid1(), uid2()
+{
+	unit_const_ptr u1 = ctx.loc1.get_unit(), u2 = ctx.loc2.get_unit();
+	if(u1) {
+		id1 = u1->id();
+		uid1 = u1->underlying_id();
+	}
+	if(u2) {
+		id2 = u2->id();
+		uid2 = u2->underlying_id();
+	}
+}
+
 undo_event::undo_event(const config& cmds, const game_events::queued_event& ctx)
 	: commands(cmds)
 	, data(ctx.data)
@@ -68,8 +89,12 @@ undo_action::undo_action()
 	, unit_id_diff(synced_context::get_unit_id_diff())
 {
 	auto& undo = synced_context::get_undo_commands();
-	auto command_transformer = [](const std::pair<config, game_events::queued_event>& p) {
-		return undo_event(p.first, p.second);
+	auto command_transformer = [](const synced_context::event_info& p) {
+		if(p.lua_.has_value()) {
+			return undo_event(*p.lua_, p.cmds_, p.evt_);
+		} else {
+			return undo_event(p.cmds_, p.evt_);
+		}
 	};
 	std::transform(undo.begin(), undo.end(), std::back_inserter(umc_commands_undo), command_transformer);
 	undo.clear();
@@ -115,7 +140,11 @@ namespace {
 		scoped_weapon_info w2("second_weapon", e.data.optional_child("second"));
 
 		game_events::queued_event q(tag, "", map_location(x1, y1, wml_loc()), map_location(x2, y2, wml_loc()), e.data);
-		resources::lua_kernel->run_wml_action("command", vconfig(e.commands), q);
+		if(e.lua_idx.has_value()) {
+			resources::lua_kernel->run_wml_event(*e.lua_idx, vconfig(e.commands), q);
+		} else {
+			resources::lua_kernel->run_wml_action("command", vconfig(e.commands), q);
+		}
 		sound::commit_music_changes();
 
 		x1 = oldx1; y1 = oldy1;
@@ -142,7 +171,7 @@ void undo_action::write(config & cfg) const
 void undo_action::read_event_vector(event_vector& vec, const config& cfg, const std::string& tag)
 {
 	for(auto c : cfg.child_range(tag)) {
-		vec.emplace_back(c.child_or_empty("filter"), c.child_or_empty("filter_second"), c.child_or_empty("filter_weapons"), c.child_or_empty("command"));
+		vec.emplace_back(c.child_or_empty("filter"), c.child_or_empty("filter_second"), c.child_or_empty("data"), c.child_or_empty("command"));
 	}
 }
 
@@ -150,10 +179,14 @@ void undo_action::write_event_vector(const event_vector& vec, config& cfg, const
 {
 	for(const auto& evt : vec)
 	{
+		if(evt.lua_idx.has_value()) {
+			// TODO: Log warning that this cannot be serialized
+			continue;
+		}
 		config& entry = cfg.add_child(tag);
 		config& first = entry.add_child("filter");
 		config& second = entry.add_child("filter_second");
-		entry.add_child("filter_weapons", evt.data);
+		entry.add_child("data", evt.data);
 		entry.add_child("command", evt.commands);
 		// First location
 		first["filter_x"] = evt.filter_loc1.wml_x();

src/actions/undo_action.hpp

@@ -15,6 +15,7 @@
 
 #pragma once
 
+#include <optional>
 #include "vision.hpp"
 #include "map/location.hpp"
 #include "units/ptr.hpp"
@@ -26,10 +27,12 @@ namespace actions {
 	class undo_list;
 
 	struct undo_event {
+		std::optional<int> lua_idx;
 		config commands, data;
 		map_location loc1, loc2, filter_loc1, filter_loc2;
 		std::size_t uid1, uid2;
 		std::string id1, id2;
+		undo_event(int fcn_idx, const config& args, const game_events::queued_event& ctx);
 		undo_event(const config& cmds, const game_events::queued_event& ctx);
 		undo_event(const config& first, const config& second, const config& weapons, const config& cmds);
 	};

src/game_events/action_wml.cpp

@@ -910,13 +910,4 @@ WML_HANDLER_FUNCTION(unit,, cfg)
 
 }
 
-WML_HANDLER_FUNCTION(on_undo, event_info, cfg)
-{
-	if(cfg["delayed_variable_substitution"].to_bool(false)) {
-		synced_context::add_undo_commands(cfg.get_config(), event_info);
-	} else {
-		synced_context::add_undo_commands(cfg.get_parsed_config(), event_info);
-	}
-}
-
 } // end namespace game_events

src/scripting/game_lua_kernel.cpp

@@ -3985,6 +3985,23 @@ static std::string read_event_name(lua_State* L, int idx)
 	}
 }
 
+/**
+ * Add undo actions for the current active event
+ * Arg 1: Either a table of ActionWML or a function to call
+ * Arg 2: (optional) If Arg 1 is a function, this is a WML table that will be passed to it
+ */
+int game_lua_kernel::intf_add_undo_actions(lua_State *L)
+{
+	config cfg;
+	if(luaW_toconfig(L, 1, cfg)) {
+		synced_context::add_undo_commands(cfg, get_event_info());
+	} else {
+		luaW_toconfig(L, 2, cfg);
+		synced_context::add_undo_commands(save_wml_event(1), cfg, get_event_info());
+	}
+	return 0;
+}
+
 /** Add a new event handler
  * Arg 1: Table of options.
  * name: Event to handle, as a string or list of strings
@@ -4073,9 +4090,25 @@ int game_lua_kernel::intf_add_event(lua_State *L)
 	return 0;
 }
 
+/**
+ * Upvalue 1: The event function
+ * Upvalue 2: The undo function
+ * Arg 1: The event content
+ */
+int game_lua_kernel::cfun_undoable_event(lua_State* L)
+{
+	lua_pushvalue(L, lua_upvalueindex(1));
+	lua_push(L, 1);
+	luaW_pcall(L, 1, 0);
+	synced_context::add_undo_commands(lua_upvalueindex(2), get_event_info());
+	return 0;
+}
+
 /** Add a new event handler
  * Arg 1: Event to handle, as a string or list of strings; or menu item ID if this is a menu item
  * Arg 2: The function to call when the event triggers
+ * Arg 3: (optional) Event priority
+ * Arg 4: (optional, non-menu-items only) The function to call when the event is undone
  *
  * Lua API:
  * - wesnoth.game_events.add_repeating
@@ -4094,6 +4127,10 @@ int game_lua_kernel::intf_add_event_simple(lua_State *L)
 	if(is_menu_item) {
 		id = name;
 		name = "menu item " + name;
+	} else if(lua_absindex(L, -1) > 2 && lua_isfunction(L, -1)) {
+		// If undo is provided as a separate function, link them together into a single function
+		// The function can be either the 3rd or 4th argument.
+		lua_pushcclosure(L, &dispatch<&game_lua_kernel::cfun_undoable_event>, 2);
 	}
 	auto new_handler = man.add_event_handler_from_lua(name, id, repeat, priority, is_menu_item);
 	if(new_handler.valid()) {
@@ -4913,7 +4950,6 @@ game_lua_kernel::game_lua_kernel(game_state & gs, play_controller & pc, reports
 		{ "get_era",                  &intf_get_era                  },
 		{ "get_resource",             &intf_get_resource             },
 		{ "modify_ai",                &intf_modify_ai_old            },
-		{ "allow_undo",                &dispatch<&game_lua_kernel::intf_allow_undo                 >        },
 		{ "cancel_action",             &dispatch<&game_lua_kernel::intf_cancel_action              >        },
 		{ "log_replay",                &dispatch<&game_lua_kernel::intf_log_replay                 >        },
 		{ "log",                       &dispatch<&game_lua_kernel::intf_log                        >        },
@@ -5281,6 +5317,8 @@ game_lua_kernel::game_lua_kernel(game_state & gs, play_controller & pc, reports
 		{ "remove", &dispatch<&game_lua_kernel::intf_remove_event> },
 		{ "fire", &dispatch2<&game_lua_kernel::intf_fire_event, false> },
 		{ "fire_by_id", &dispatch2<&game_lua_kernel::intf_fire_event, true> },
+		{ "add_undo_actions", &dispatch<&game_lua_kernel::intf_add_undo_actions> },
+		{ "set_undoable", &dispatch<&game_lua_kernel::intf_allow_undo >        },
 		{ nullptr, nullptr }
 	};
 	lua_getglobal(L, "wesnoth");

src/scripting/game_lua_kernel.hpp

@@ -161,6 +161,8 @@ class game_lua_kernel : public lua_kernel_base
 	int intf_add_event_simple(lua_State* L);
 	int intf_add_event_wml(lua_State* L);
 	int intf_add_event(lua_State *L);
+	int intf_add_undo_actions(lua_State *L);
+	int cfun_undoable_event(lua_State *L);
 	int intf_remove_event(lua_State *L);
 	int intf_color_adjust(lua_State *L);
 	int intf_get_color_adjust(lua_State *L);

src/synced_context.cpp

@@ -358,6 +358,16 @@ void synced_context::add_undo_commands(const config& commands, const game_events
 	undo_commands_.emplace_front(commands, ctx);
 }
 
+void synced_context::add_undo_commands(int idx, const game_events::queued_event& ctx)
+{
+	undo_commands_.emplace_front(idx, ctx);
+}
+
+void synced_context::add_undo_commands(int idx, const config& args, const game_events::queued_event& ctx)
+{
+	undo_commands_.emplace_front(idx, args, ctx);
+}
+
 set_scontext_synced_base::set_scontext_synced_base()
 	: new_rng_(synced_context::get_rng_for_action())
 	, old_rng_(randomness::generator)

src/synced_context.hpp

@@ -190,13 +190,24 @@ public:
 	/** If we are in a mp game, ask the server, otherwise generate the answer ourselves. */
 	static config ask_server_choice(const server_choice&);
 
-	typedef std::deque<std::pair<config, game_events::queued_event>> event_list;
+	struct event_info {
+		config cmds_;
+		std::optional<int> lua_;
+		game_events::queued_event evt_;
+		event_info(const config& cmds, game_events::queued_event evt) : cmds_(cmds), evt_(evt) {}
+		event_info(int lua, game_events::queued_event evt) : lua_(lua), evt_(evt) {}
+		event_info(int lua, const config& args, game_events::queued_event evt) : cmds_(args), lua_(lua), evt_(evt) {}
+	};
+
+	typedef std::deque<event_info> event_list;
 	static event_list& get_undo_commands()
 	{
 		return undo_commands_;
 	}
 
 	static void add_undo_commands(const config& commands, const game_events::queued_event& ctx);
+	static void add_undo_commands(int fcn_idx, const game_events::queued_event& ctx);
+	static void add_undo_commands(int fcn_idx, const config& args, const game_events::queued_event& ctx);
 
 	static void reset_undo_commands()
 	{
@@ -221,7 +232,7 @@ private:
 	/** Used to restore the unit id manager when undoing. */
 	static inline int last_unit_id_ = 0;
 
-	/** Actions wml to be executed when the current action is undone. */
+	/** Actions to be executed when the current action is undone. */
 	static inline event_list undo_commands_ {};
 };
 

utils/emmylua/wesnoth/game_events.lua

@@ -53,7 +53,8 @@ function wesnoth.game_events.add(opts) end
 ---@param name string|string[] The event or events to handle
 ---@param action fun(WML) The function called when the event triggers
 ---@param priority? number Events execute in order of decreasing priority, and secondarily in order of addition
-function wesnoth.game_events.add_repeating(name, action, priority) end
+---@param undo_action? fun(WML) The function called if undoing after the event triggers.
+function wesnoth.game_events.add_repeating(name, action, priority, undo_action) end
 
 ---Add a game event handler triggered from a menu item, bound directly to a Lua function
 ---@param id string
@@ -83,3 +84,11 @@ function wesnoth.game_events.fire_by_id(id, first, second, data) end
 ---Remove an event handler by ID
 ---@param id string The event to remove
 function wesnoth.game_events.remove(id) end
+
+---Set whether the current event is undoable.
+---@param can_undo boolean Whether the event is undoable.
+function wesnoth.game_events.set_undoable(can_undo) end
+
+---Add undo actions for the current event
+---@param actions WML|fun(ctx):boolean The undo actions, either as ActionWML or a Lua function.
+function wesnoth.game_events.add_undo_actions(actions) end