From bfcd2695f29752162d168f9a6759a49bdc5b767f Mon Sep 17 00:00:00 2001
From: gfgtdf <f492343@rmqkr.net>
Date: Mon, 2 Mar 2015 18:08:55 +0100
Subject: [PATCH] don't give OOS error on invalid unit rename

And fix some comments and error message strings.
---
 src/playturn.cpp |  2 +-
 src/replay.cpp   | 31 +++++++++++++++----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/playturn.cpp b/src/playturn.cpp
index ee5d631f0753..fe517d53ed52 100644
--- a/src/playturn.cpp
+++ b/src/playturn.cpp
@@ -98,7 +98,7 @@ turn_info::PROCESS_DATA_RESULT turn_info::handle_turn(const config& t)
 	assert(t.all_children_count() == 1);
 	/** @todo FIXME: Check what commands we execute when it's our turn! */
 
-	//note, that this function might call itself recursively: do_replay -> ... -> persist_var -> ... -> handle_generic_event -> sync_network -> handle_turn
+	//note, that this function might call itself recursively: do_replay -> ... -> get_user_choice -> ... -> playmp_controller::pull_remote_choice -> sync_network -> handle_turn
 	recorder.add_config(t, replay::MARK_AS_SENT);
 	PROCESS_DATA_RESULT retv = replay_to_process_data_result(do_replay());
 	return retv;
diff --git a/src/replay.cpp b/src/replay.cpp
index 13e3559d915b..b77595f87ada 100644
--- a/src/replay.cpp
+++ b/src/replay.cpp
@@ -705,7 +705,13 @@ REPLAY_RETURN do_replay(bool one_move)
 	update_locker lock_update(resources::screen->video(), resources::controller->is_skipping_replay());
 	return do_replay_handle(one_move);
 }
-
+/**
+	@returns:
+		if we expect a user choice and found something that prevents us from moving on we return REPLAY_FOUND_DEPENDENT (even if it is not a dependent command)
+		else if we found an [end_turn] we return REPLAY_FOUND_END_TURN
+		else if we found a player action and one_move=true we return REPLAY_FOUND_END_MOVE
+		else (<=> we reached the end of the replay) we return REPLAY_RETURN_AT_END
+*/
 REPLAY_RETURN do_replay_handle(bool one_move)
 {
 
@@ -774,21 +780,17 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 			const std::string &name = child["name"];
 
 			unit_map::iterator u = resources::units->find(loc);
-			if (u.valid()) {
-				if (u->unrenamable()) {
-					std::stringstream errbuf;
-					errbuf << "renaming unrenamable unit " << u->id() << '\n';
-					replay::process_error(errbuf.str());
-					continue;
-				}
+			if (u.valid() && !u->unrenamable()) {
 				u->rename(name);
 			} else {
-				// Users can rename units while it's being killed at another machine.
+				// Users can rename units while it's being killed or at another machine.
 				// This since the player can rename units when it's not his/her turn.
 				// There's not a simple way to prevent that so in that case ignore the
 				// rename instead of throwing an OOS.
+				// The same way it is possible that an unrenamable unit moves to a
+				// hex where previously a renamable unit was.
 				WRN_REPLAY << "attempt to rename unit at location: "
-				   << loc << ", where none exists (anymore).\n";
+				   << loc << (u.valid() ? ", which is unrenamable" : ", where none exists (anymore)") << "\n";
 			}
 		}
 
@@ -797,9 +799,8 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 
 			if(is_synced)
 			{
-				replay::process_error("found init_side in replay while is_synced=true\n" );
+				replay::process_error("found side initialization in replay expecting a user choice\n" );
 				get_replay_source().revert_action();
-				//fits better than the other options, and should have the desired effect.
 				return REPLAY_FOUND_DEPENDENT;
 			}
 			else
@@ -813,9 +814,8 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 		{
 			if(is_synced)
 			{
-				replay::process_error("found end_turn in replay while is_synced=true\n" );
+				replay::process_error("found turn end in replay while expecting a user choice\n" );
 				get_replay_source().revert_action();
-				//fits better than the other options, and should have the desired effect.
 				return REPLAY_FOUND_DEPENDENT;
 			}
 			else
@@ -868,9 +868,8 @@ REPLAY_RETURN do_replay_handle(bool one_move)
 
 			if(is_synced)
 			{
-				replay::process_error("found " + commandname + " command in replay while is_synced=true\n" );
+				replay::process_error("found [" + commandname + "] command in replay expecting a user choice\n" );
 				get_replay_source().revert_action();
-				//fits better than the other options, and should have the desired effect.
 				return REPLAY_FOUND_DEPENDENT;
 			}
 			else