[on_undo] and [on_redo] now get a snapshot of the event context

This means they can access auto-stored variables.
However, using [unstore_unit] for $unit or $second_unit is not recommended.
Also, $unit.x and $unit.y may not be the same as they were during the original event.
(The same with $second_unit)

changelog

@@ -16,6 +16,11 @@ Version 1.13.4+dev:
    * New ~SCALE_INTO_SHARP(w,h) IPF which preserves aspect ratio, using
      nearest neighbor scaling.
    * Support delayed_variable_substitution= in [on_undo], [on_redo]
+     Note that this means $unit.x and $unit.y may not reflect the unit's
+     true location, so using [unstore_unit] on $unit may have unexpected effects.
+     This applies to $second_unit too. The $x1, $y1, $x2, $y2 variables work fine
+     though, so in most cases they can be used instead. Anything else in $unit
+     or $second_unit is also fine.
    * formula= in SUF can now reference $other_unit via the formula variable "other"
    * formula= now supported in location, side, and weapon filters
    * Weapon filters now support number, parry, accuracy, and movement_used

src/actions/undo_action.cpp

@@ -2,40 +2,188 @@
 #include "scripting/game_lua_kernel.hpp"
 #include "resources.hpp"
 #include "variable.hpp" // vconfig
-#include "game_events/pump.hpp" //game_events::queued_event
+#include "game_data.hpp"
+#include "units/unit.hpp"
 
 #include <cassert>
+#include <iterator>
+#include <algorithm>
 
 namespace actions
 {
+
+undo_event::undo_event(const config& cmds, const game_events::queued_event& ctx)
+	: commands(cmds)
+	, data(ctx.data)
+	, loc1(ctx.loc1)
+	, loc2(ctx.loc2)
+	, filter_loc1(ctx.loc1.filter_x(), ctx.loc1.filter_y())
+	, filter_loc2(ctx.loc2.filter_x(), ctx.loc2.filter_y())
+	, 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& first, const config& second, const config& weapons, const config& cmds)
+	: commands(cmds)
+	, data(weapons)
+	, loc1(first["x"], first["y"])
+	, loc2(second["x"], second["y"])
+	, filter_loc1(first["filter_x"], first["filter_y"])
+	, filter_loc2(second["filter_x"], second["filter_y"])
+	, uid1(first["underlying_id"])
+	, uid2(second["underlying_id"])
+	, id1(first["id"])
+	, id2(second["id"])
+{
+}
+
+undo_action::undo_action()
+	: undo_action_base()
+	, replay_data()
+	, unit_id_diff(synced_context::get_unit_id_diff())
+{
+	auto& undo = synced_context::get_undo_commands();
+	auto& redo = synced_context::get_redo_commands();
+	auto command_transformer = [](const std::pair<config, game_events::queued_event>& p) {
+		return undo_event(p.first, p.second);
+	};
+	std::transform(undo.begin(), undo.end(), std::back_inserter(umc_commands_undo), command_transformer);
+	std::transform(redo.begin(), redo.end(), std::back_inserter(umc_commands_redo), command_transformer);
+	undo.clear();
+	redo.clear();
+}
+
+undo_action::undo_action(const config& cfg)
+	: undo_action_base()
+	, replay_data(cfg.child_or_empty("replay_data"))
+	, unit_id_diff(cfg["unit_id_diff"])
+{
+	read_event_vector(umc_commands_undo, cfg, "undo_actions");
+	read_event_vector(umc_commands_redo, cfg, "redo_actions");
+}
+
+namespace {
+	unit_ptr get_unit(size_t uid, const std::string& id) {
+		assert(resources::units);
+		auto iter = resources::units->find(uid);
+		if(!iter.valid() || iter->id() != id) {
+			return nullptr;
+		}
+		return iter.get_shared_ptr();
+	}
+	void execute_event(const undo_event& e, std::string tag) {
+		assert(resources::lua_kernel);
+		assert(resources::gamedata);
+
+		config::attribute_value& x1 = resources::gamedata->get_variable("x1");
+		config::attribute_value& y1 = resources::gamedata->get_variable("y1");
+		config::attribute_value& x2 = resources::gamedata->get_variable("x2");
+		config::attribute_value& y2 = resources::gamedata->get_variable("y2");
+		int oldx1 = x1, oldy1 = y1, oldx2 = x2, oldy2 = y2;
+		x1 = e.filter_loc1.x + 1; y1 = e.filter_loc1.y + 1;
+		x2 = e.filter_loc2.x + 1; y2 = e.filter_loc2.y + 1;
+
+		int realx1, realy1, realx2, realy2;
+		boost::scoped_ptr<scoped_xy_unit> u1, u2;
+		if(unit_ptr who = get_unit(e.uid1, e.id1)) {
+			realx1 = who->get_location().x;
+			realy1 = who->get_location().y;
+			who->set_location(e.loc1);
+			u1.reset(new scoped_xy_unit("unit", realx1, realy1, *resources::units));
+		}
+		if(unit_ptr who = get_unit(e.uid2, e.id2)) {
+			realx2 = who->get_location().x;
+			realy2 = who->get_location().y;
+			who->set_location(e.loc2);
+			u2.reset(new scoped_xy_unit("unit", realx2, realy2, *resources::units));
+		}
+
+		scoped_weapon_info w1("weapon", e.data.child("first"));
+		scoped_weapon_info w2("second_weapon", e.data.child("second"));
+
+		game_events::queued_event q(tag, map_location(x1, y1), map_location(x2, y2), e.data);
+		resources::lua_kernel->run_wml_action("command", vconfig(e.commands), q);
+
+		if(u1) {
+			unit_ptr who = get_unit(e.uid1, e.id1);
+			who->set_location(map_location(realx1, realy1));
+		}
+		if(u2) {
+			unit_ptr who = get_unit(e.uid2, e.id2);
+			who->set_location(map_location(realx2, realy2));
+		}
+
+		x1 = oldx1; y1 = oldy1;
+		x2 = oldx2; y2 = oldy2;
+	}
+}
+
 void undo_action::execute_undo_umc_wml()
 {
-	assert(resources::lua_kernel);
-	for(const config& c : umc_commands_undo)
+	for(const undo_event& e : umc_commands_undo)
 	{
-		resources::lua_kernel->run_wml_action("command", vconfig(c), game_events::queued_event("undo", map_location(), map_location(), config()));
+		execute_event(e, "undo");
 	}
 }
 
 void undo_action::execute_redo_umc_wml()
 {
 	assert(resources::lua_kernel);
-	for(const config& c : umc_commands_redo)
+	assert(resources::gamedata);
+	for(const undo_event& e : umc_commands_redo)
 	{
-		resources::lua_kernel->run_wml_action("command", vconfig(c), game_events::queued_event("redo", map_location(), map_location(), config()));
+		execute_event(e, "redo");
 	}
 }
 
-void undo_action::read_tconfig_vector(tconfig_vector& vec, const config& cfg, const std::string& tag)
+void undo_action::write(config & cfg) const
 {
-	config::const_child_itors r = cfg.child_range(tag);
-	vec.insert(vec.end(), r.first, r.second);
+	cfg.add_child("replay_data", replay_data);
+	cfg["unit_id_diff"] = unit_id_diff;
+	write_event_vector(umc_commands_undo, cfg, "undo_actions");
+	write_event_vector(umc_commands_redo, cfg, "redo_actions");
+	undo_action_base::write(cfg);
 }
-void undo_action::write_tconfig_vector(const tconfig_vector& vec, config& cfg, const std::string& tag)
+
+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("filter"), c.child("filter_second"), c.child("filter_weapons"), c.child("commands"));
+	}
+}
+
+void undo_action::write_event_vector(const event_vector& vec, config& cfg, const std::string& tag)
 {
-	for(const config& c : vec)
+	for(const auto& evt : vec)
 	{
-		cfg.add_child(tag, c);
+		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("command", evt.commands);
+		// First location
+		first["filter_x"] = evt.filter_loc1.x;
+		first["filter_y"] = evt.filter_loc1.y;
+		first["underlying_id"] = evt.uid1;
+		first["id"] = evt.id1;
+		first["x"] = evt.loc1.x;
+		first["y"] = evt.loc1.y;
+		// Second location
+		second["filter_x"] = evt.filter_loc2.x;
+		second["filter_y"] = evt.filter_loc2.y;
+		second["underlying_id"] = evt.uid2;
+		second["id"] = evt.id2;
+		second["x"] = evt.loc2.x;
+		second["y"] = evt.loc2.y;
 	}
 }
 

src/actions/undo_action.hpp

@@ -4,14 +4,23 @@
 #include "map/location.hpp"
 #include "units/ptr.hpp"
 #include "synced_context.hpp"
+#include "game_events/pump.hpp" // for queued_event
+#include "config.hpp"
 
 #include <boost/noncopyable.hpp>
-#include <boost/ptr_container/ptr_vector.hpp>
 #include <boost/optional.hpp>
+
 namespace actions {
 	class undo_list;
-}
-namespace actions {
+
+	struct undo_event {
+		config commands, data;
+		map_location loc1, loc2, filter_loc1, filter_loc2;
+		size_t uid1, uid2;
+		std::string id1, id2;
+		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);
+	};
 
 	/// Records information to be able to undo an action.
 	/// Each type of action gets its own derived type.
@@ -40,38 +49,13 @@ namespace actions {
 		/// Default constructor.
 		/// It is assumed that undo actions are contructed after the action is performed
 		/// so that the unit id diff does not change after this contructor.
-		undo_action()
-			: undo_action_base()
-			, replay_data()
-			, unit_id_diff(synced_context::get_unit_id_diff())
-			, umc_commands_undo()
-			, umc_commands_redo()
-		{
-			umc_commands_undo.swap(synced_context::get_undo_commands());
-			umc_commands_redo.swap(synced_context::get_redo_commands());
-		}
-		undo_action(const config& cfg)
-			: undo_action_base()
-			, replay_data(cfg.child_or_empty("replay_data"))
-			, unit_id_diff(cfg["unit_id_diff"])
-			, umc_commands_undo()
-			, umc_commands_redo()
-		{
-			read_tconfig_vector(umc_commands_undo, cfg, "undo_actions");
-			read_tconfig_vector(umc_commands_redo, cfg, "redo_actions");
-		}
+		undo_action();
+		undo_action(const config& cfg);
 		// Virtual destructor to support derived classes.
 		virtual ~undo_action() {}
 
 		/// Writes this into the provided config.
-		virtual void write(config & cfg) const
-		{
-			cfg.add_child("replay_data", replay_data);
-			cfg["unit_id_diff"] = unit_id_diff;
-			write_tconfig_vector(umc_commands_undo, cfg, "undo_actions");
-			write_tconfig_vector(umc_commands_redo, cfg, "redo_actions");
-			undo_action_base::write(cfg);
-		}
+		virtual void write(config & cfg) const;
 
 		/// Undoes this action.
 		/// @return true on success; false on an error.
@@ -87,14 +71,14 @@ namespace actions {
 		/// TODO: does it really make sense to allow undoing if the unit id counter has changed?
 		int unit_id_diff;
 		/// actions wml (specified by wml) that should be executed when undoing this command.
-		typedef boost::ptr_vector<config> tconfig_vector;
-		tconfig_vector umc_commands_undo;
-		tconfig_vector umc_commands_redo;
+		typedef std::vector<undo_event> event_vector;
+		event_vector umc_commands_undo;
+		event_vector umc_commands_redo;
 		void execute_undo_umc_wml();
 		void execute_redo_umc_wml();
 
-		static void read_tconfig_vector(tconfig_vector& vec, const config& cfg, const std::string& tag);
-		static void write_tconfig_vector(const tconfig_vector& vec, config& cfg, const std::string& tag);
+		static void read_event_vector(event_vector& vec, const config& cfg, const std::string& tag);
+		static void write_event_vector(const event_vector& vec, config& cfg, const std::string& tag);
 	};
 
 	/// entry for player actions that do not need any special code to be performed when undoing such as right-click menu items.

src/game_events/action_wml.cpp

@@ -1180,21 +1180,21 @@ WML_HANDLER_FUNCTION(volume,, cfg)
 
 }
 
-WML_HANDLER_FUNCTION(on_undo,, 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());
+		synced_context::add_undo_commands(cfg.get_config(), event_info);
 	} else {
-		synced_context::add_undo_commands(cfg.get_parsed_config());
+		synced_context::add_undo_commands(cfg.get_parsed_config(), event_info);
 	}
 }
 
-WML_HANDLER_FUNCTION(on_redo,, cfg)
+WML_HANDLER_FUNCTION(on_redo, event_info, cfg)
 {
 	if(cfg["delayed_variable_substitution"].to_bool(false)) {
-		synced_context::add_redo_commands(cfg.get_config());
+		synced_context::add_redo_commands(cfg.get_config(), event_info);
 	} else {
-		synced_context::add_redo_commands(cfg.get_parsed_config());
+		synced_context::add_redo_commands(cfg.get_parsed_config(), event_info);
 	}
 }
 

src/game_events/entity_location.cpp

@@ -104,5 +104,20 @@ bool entity_location::matches_unit_filter(const unit_map::const_iterator & un_it
 	       matches_unit(un_it);
 }
 
+unit_const_ptr entity_location::get_unit() const
+{
+	if(resources::units == nullptr) {
+		return nullptr;
+	}
+	if(id_ == 0) {
+		auto un_it = resources::units->find(*this);
+		if(un_it.valid()) {
+			return un_it.get_shared_ptr();
+		}
+		return nullptr;
+	}
+	return resources::units->find(id_).get_shared_ptr();
+}
+
 } // end namespace game_events
 

src/game_events/entity_location.hpp

@@ -41,6 +41,7 @@ namespace game_events
 		bool matches_unit(const unit_map::const_iterator & un_it) const;
 		bool matches_unit_filter(const unit_map::const_iterator & un_it,
 		                         const vconfig & filter) const;
+		unit_const_ptr get_unit() const;
 
 		static const entity_location null_entity;
 

src/synced_context.cpp

@@ -53,8 +53,8 @@ static lg::log_domain log_replay("replay");
 
 synced_context::synced_state synced_context::state_ = synced_context::UNSYNCED;
 int synced_context::last_unit_id_ = 0;
-synced_context::tconfig_vector synced_context::undo_commands_ = synced_context::tconfig_vector();
-synced_context::tconfig_vector synced_context::redo_commands_ = synced_context::tconfig_vector();
+synced_context::event_list synced_context::undo_commands_;
+synced_context::event_list synced_context::redo_commands_;
 bool synced_context::is_simultaneously_ = false;
 
 bool synced_context::run(const std::string& commandname, const config& data, bool use_undo, bool show, synced_command::error_handler_function error_handler)
@@ -383,14 +383,24 @@ config synced_context::ask_server_choice(const server_choice& sch)
 	}
 }
 
-void synced_context::add_undo_commands(const config& commands)
+void synced_context::add_undo_commands(const config& commands, const game_events::queued_event& ctx)
 {
-	undo_commands_.insert(undo_commands_.begin(), new config(commands));
+	undo_commands_.emplace_front(commands, ctx);
 }
 
-void synced_context::add_redo_commands(const config& commands)
+void synced_context::add_redo_commands(const config& commands, const game_events::queued_event& ctx)
 {
-	redo_commands_.insert(redo_commands_.begin(), new config(commands));
+	redo_commands_.emplace_front(commands, ctx);
+}
+
+void synced_context::reset_undo_commands()
+{
+	undo_commands_.clear();
+}
+
+void synced_context::reset_redo_commands()
+{
+	redo_commands_.clear();
 }
 
 set_scontext_synced_base::set_scontext_synced_base()