Fix bug #13118 (OoS with hidden units, mainly with AI)

Now store and send the path used (using x=x1,x2,.. and y=y1,y2.. in [move])

This currently break compatibility with replay done with RC1 (if they worked)

I will try to restore backward compatibility in next commit.

PS: This commit is a small variation of the patch posted in the bugreport

src/actions.cpp

@@ -2340,7 +2340,7 @@ size_t move_unit(game_display* disp,
 	unit::clear_status_caches();
 
 	if(move_recorder != NULL) {
-		move_recorder->add_movement(steps.front(),steps.back());
+		move_recorder->add_movement(steps);
 	}
 
 	bool event_mutated = false;

src/ai.cpp

@@ -478,6 +478,8 @@ map_location ai_interface::move_unit_partial(location from, location to,
 
 	const std::map<location,paths>::iterator p_it = possible_moves.find(from);
 
+	std::vector<location> steps;
+	
 	if(p_it != possible_moves.end()) {
 		paths& p = p_it->second;
 		std::map<location,paths::route>::iterator rt = p.routes.begin();
@@ -493,7 +495,7 @@ map_location ai_interface::move_unit_partial(location from, location to,
 			}
 			u_it->second.set_movement(rt->second.move_left);
 
-			std::vector<location> steps = rt->second.steps;
+			steps = rt->second.steps;
 
 			while(steps.empty() == false && info_.units.find(to) != info_.units.end() && from != to){
 				LOG_AI << "AI attempting illegal move. Attempting to move onto existing unit\n";
@@ -597,7 +599,7 @@ map_location ai_interface::move_unit_partial(location from, location to,
 		info_.disp.draw();
 	}
 
-	recorder.add_movement(from,to);
+	recorder.add_movement(steps);
 
 	game_events::fire("moveto",to);
 

src/map_location.cpp

@@ -341,3 +341,23 @@ std::vector<map_location> parse_location_range(const std::string& x, const std::
 	}
 	return res;
 }
+
+void write_locations(const std::vector<map_location>& locs, config& cfg)
+{
+	std::stringstream x, y;
+
+	std::vector<map_location>::const_iterator i = locs.begin(),
+			end = locs.end();
+
+	for(; i != end; ++i) {
+		x << (i->x + 1);
+		y << (i->y + 1);
+		if(i+1 != end){
+			x << ",";
+			y << ",";
+		}
+	}
+
+	cfg["x"] = x.str();
+	cfg["y"] = y.str();
+}

src/map_location.hpp

@@ -110,6 +110,10 @@ struct map_location {
 std::vector<map_location> parse_location_range(const std::string& xvals,
 	const std::string& yvals, const gamemap *const map=NULL);
 
+/** Write a vector of locations into a config
+ *  adding keys x=x1,x2,..,xn and y=y1,y2,..,yn */
+void write_locations(const std::vector<map_location>& locs, config& cfg);
+
 /** Dumps a position on a stream, for debug purposes. */
 std::ostream &operator<<(std::ostream &s, map_location const &l);
 

src/menu_events.cpp

@@ -1338,7 +1338,7 @@ private:
 			gui_->invalidate(route.back());
 			gui_->draw();
 
-			recorder.add_movement(route.front(),route.back());
+			recorder.add_movement(action.route);
 		}
 		gui_->invalidate_unit();
 		gui_->invalidate_game_status();

src/replay.cpp

@@ -295,9 +295,14 @@ void replay::add_countdown_update(int value, int team)
 }
 
 
-void replay::add_movement(const map_location& a,const map_location& b)
+void replay::add_movement(const std::vector<map_location>& steps)
 {
-	add_pos("move",a,b);
+	config* const cmd = add_command();
+
+	config move;
+	write_locations(steps, move);
+
+	cmd->add_child("move",move);
 }
 
 void replay::add_attack(const map_location& a, const map_location& b, int att_weapon, int def_weapon)
@@ -557,10 +562,17 @@ void replay::undo()
 		{
 			// A unit's move is being undone.
 			// Repair unsynced cmds whose locations depend on that unit's location.
-			map_location dst(*(child->child("destination")),
-				game_events::get_state_of_game());
-			map_location src(*(child->child("source")),
-				game_events::get_state_of_game());
+			const std::vector<map_location> steps =
+					parse_location_range((*child)["x"],(*child)["y"]);
+
+			if(steps.empty()) {
+				ERR_REPLAY << "trying to undo a move using an empty path";
+				return; // nothing to do, I suppose.
+			}
+
+			const map_location src = steps.front();
+			const map_location dst = steps.back();
+
 			for (std::vector<config::child_iterator>::iterator async_cmd =
 				 async_cmds.begin(); async_cmd != async_cmds.end(); async_cmd++)
 			{
@@ -1028,15 +1040,23 @@ bool do_replay_handle(game_display& disp, const gamemap& map,
 			}
 		}
 		else if((child = cfg->child("move")) != NULL) {
-			const config* const destination = child->child("destination");
-			const config* const source = child->child("source");
 
-			if(destination == NULL || source == NULL) {
-				replay::throw_error("no destination/source found in movement\n");
+			const std::string& x = (*child)["x"];
+			const std::string& y = (*child)["y"];
+
+			if(x.empty() || y.empty()) {
+				// x,y keys will be missing in replay recorded before 1.5.13
+				replay::throw_error("missing path data in [move], mandatory since 1.5.13 - 1.6RC2 \n");
 			}
 
-			const map_location src(*source, game_events::get_state_of_game());
-			const map_location dst(*destination, game_events::get_state_of_game());
+			const std::vector<map_location> steps =	parse_location_range(x,y);
+
+			if(steps.empty()) {
+				replay::throw_error("incorrect path data found in [move]\n");
+			}
+
+			const map_location src = steps.front();
+			const map_location dst = steps.back();
 
 			if (src == dst) {
 				WRN_REPLAY << "Warning: Move with identical source and destination. Skipping...";
@@ -1058,27 +1078,7 @@ bool do_replay_handle(game_display& disp, const gamemap& map,
 				replay::throw_error(errbuf.str());
 			}
 
-			// search path assuming current_team as viewing team
-			const shortest_path_calculator calc(u->second, current_team, units, teams, map);
-			std::set<map_location> allowed_teleports;
-			if(u->second.get_ability_bool("teleport",src)) {
-				// search all known empty friendly villages
-				for(std::set<map_location>::const_iterator i = current_team.villages().begin();
-					i != current_team.villages().end(); ++i) {
-					if (current_team.is_enemy(u->second.side()) && current_team.fogged(*i))
-						continue;
-
-					unit_map::const_iterator occupant = find_visible_unit(units, *i, map,teams, current_team);
-					if (occupant != units.end() && occupant != u)
-						continue;
-
-					allowed_teleports.insert(*i);
-				}
-			}
-
-			paths::route route = a_star_search(src, dst, 10000.0, &calc, map.w(), map.h(), &allowed_teleports);
-
-			::move_unit(&disp, map, units, teams, route.steps, NULL, NULL, NULL, true, true, true);
+			::move_unit(&disp, map, units, teams, steps, NULL, NULL, NULL, true, true, true);
 		}
 
 		else if((child = cfg->child("attack")) != NULL) {

src/replay.hpp

@@ -57,7 +57,7 @@ public:
 	void add_recall(int unit_index, const map_location& loc);
 	void add_disband(int unit_index);
 	void add_countdown_update(int value,int team);
-	void add_movement(const map_location& a, const map_location& b);
+	void add_movement(const std::vector<map_location>& steps);
 	void add_attack(const map_location& a, const map_location& b,
 	                int att_weapon, int def_weapon);
 	void choose_option(int index);