remove code duplication of unit advancements

the advancemnts after [unstore_unit] and after attacks now use the same
function.

src/actions/attack.cpp

@@ -56,6 +56,7 @@
 static lg::log_domain log_engine("engine");
 #define DBG_NG LOG_STREAM(debug, log_engine)
 #define LOG_NG LOG_STREAM(info, log_engine)
+#define WRN_NG LOG_STREAM(err, log_engine)
 #define ERR_NG LOG_STREAM(err, log_engine)
 
 static lg::log_domain log_config("config");
@@ -1369,7 +1370,7 @@ namespace
 	class unit_advancement_choice : public mp_sync::user_choice
 	{
 	public:
-		unit_advancement_choice(const map_location& loc, int total_opt, int side_num, const ai::unit_advancements_aspect& ai_advancement, bool force_dialog)
+		unit_advancement_choice(const map_location& loc, int total_opt, int side_num, const ai::unit_advancements_aspect* ai_advancement, bool force_dialog)
 			: loc_ (loc), nb_options_(total_opt), side_num_(side_num), ai_advancement_(ai_advancement), force_dialog_(force_dialog)
 		{
 		}
@@ -1400,14 +1401,17 @@ namespace
 
 				//if ai_advancement_ is the default advancement the following code will
 				//have no effect because get_advancements returns an empty list.
-				unit_map::iterator u = resources::units->find(loc_);
-				const std::vector<std::string>& options = u->advances_to();
-				const std::vector<std::string>& allowed = ai_advancement_.get_advancements(u);
-
-				for(std::vector<std::string>::const_iterator a = options.begin(); a != options.end(); ++a) {
-					if (std::find(allowed.begin(), allowed.end(), *a) != allowed.end()){
-						res = a - options.begin();
-						break;
+				if(ai_advancement_ != NULL)
+				{
+					unit_map::iterator u = resources::units->find(loc_);
+					const std::vector<std::string>& options = u->advances_to();
+					const std::vector<std::string>& allowed = ai_advancement_->get_advancements(u);
+
+					for(std::vector<std::string>::const_iterator a = options.begin(); a != options.end(); ++a) {
+						if (std::find(allowed.begin(), allowed.end(), *a) != allowed.end()){
+							res = a - options.begin();
+							break;
+						}
 					}
 				}
 
@@ -1435,21 +1439,21 @@ namespace
 		const map_location loc_;
 		int nb_options_;
 		int side_num_;
-		const ai::unit_advancements_aspect& ai_advancement_;
+		const ai::unit_advancements_aspect* ai_advancement_;
 		bool force_dialog_;
 	};
 }
 
 /*
 advances the unit and stores data in the replay (or reads data from replay).
 */
-void advance_unit_at(const map_location& loc, const ai::unit_advancements_aspect ai_advancement, bool force_dialog)
+void advance_unit_at(const advance_unit_params& params)
 {
 	//i just don't want infinite loops...
 	// the 20 is picked rather randomly.
 	for(int advacment_number = 0; advacment_number < 20; advacment_number++)
 	{
-		unit_map::iterator u = resources::units->find(loc);
+		unit_map::iterator u = resources::units->find(params.loc_);
 		//this implies u.valid()
 		if(!unit_helper::will_certainly_advance(u)) {
 			return;
@@ -1458,22 +1462,25 @@ void advance_unit_at(const map_location& loc, const ai::unit_advancements_aspect
 		//we don't want to let side 1 decide it during start/prestart.
 		int side_for = resources::gamedata->phase() == game_data::PLAY ? 0: u->side();
 		config selected = mp_sync::get_user_choice("choose",
-			unit_advancement_choice(loc, unit_helper::number_of_possible_advances(*u),u->side(), ai_advancement, force_dialog), side_for);
+			unit_advancement_choice(params.loc_, unit_helper::number_of_possible_advances(*u), u->side(), params.ai_advancements_, params.force_dialog_), side_for);
 		//calls actions::advance_unit.
-		bool result = dialogs::animate_unit_advancement(loc, selected["value"], true, true);
+		bool result = dialogs::animate_unit_advancement(params.loc_, selected["value"], params.fire_events_, params.animate_);
 
 		DBG_NG << "animate_unit_advancement result = " << result << std::endl;
-		u = resources::units->find(loc);
+		u = resources::units->find(params.loc_);
 		// level 10 unit gives 80 XP and the highest mainline is level 5
 		if (u.valid() && u->experience() > 80)
 		{
-			ERR_NG << "Unit has too many (" << u->experience() << ") XP left; cascade leveling goes on still." << std::endl;
+			WRN_NG << "Unit has too many (" << u->experience() << ") XP left; cascade leveling goes on still." << std::endl;
 		}
 	}
-	ERR_NG << "unit at " << loc << "tried to adcance more than 20 times" << std::endl;
+	ERR_NG << "unit at " << params.loc_ << "tried to advance more than 20 times. Advancing was aborted" << std::endl;
 }
 
 
+void advance_unit_at(const map_location& loc, const ai::unit_advancements_aspect ai_advancement = ai::unit_advancements_aspect())
+{ advance_unit_at(advance_unit_params(loc).ai_advancements(ai_advancement)); }
+
 void attack_unit_and_advance(const map_location &attacker, const map_location &defender,
                  int attack_with, int defend_with, bool update_display,
 				 const ai::unit_advancements_aspect& ai_advancement)

src/actions/attack.hpp

@@ -211,9 +211,22 @@ void attack_unit_and_advance(const map_location &attacker, const map_location &d
 
 	this method is currently not used by unstore_unit, if we want to do that we'd need to allow more arguments (animate, fire_events).
 */
-void advance_unit_at(const map_location& loc, const ai::unit_advancements_aspect ai_advancement = ai::unit_advancements_aspect(), bool force_dialog = false);
-inline void advance_unit_at(const map_location& loc, bool force_dialog = false)
-{ advance_unit_at(loc, ai::unit_advancements_aspect(), force_dialog); }
+struct advance_unit_params 
+{
+	advance_unit_params(const map_location& loc) : loc_(loc), ai_advancements_(NULL), force_dialog_(false), fire_events_(true), animate_(true) {}
+	advance_unit_params& ai_advancements(const ai::unit_advancements_aspect& value) {ai_advancements_ = &value; return *this;}
+	advance_unit_params& force_dialog(bool value) {force_dialog_ = value; return *this;}
+	advance_unit_params& fire_events(bool value) {fire_events_ = value; return *this;}
+	advance_unit_params& animate(bool value) {animate_ = value; return *this;}
+	friend void advance_unit_at(const advance_unit_params&); 
+private:
+	const map_location& loc_;
+	const ai::unit_advancements_aspect* ai_advancements_;
+	bool force_dialog_;
+	bool fire_events_;
+	bool animate_;
+};
+void advance_unit_at(const advance_unit_params& params);
 /**
  * Returns the advanced version of a unit (with traits and items retained).
  */

src/game_events/action_wml.cpp

@@ -205,39 +205,6 @@ namespace { // Types
 			return config();
 		}
 	};
-
-	struct unstore_unit_advance_choice: mp_sync::user_choice
-	{
-		int nb_options;
-		map_location loc;
-		bool use_dialog;
-
-		unstore_unit_advance_choice(int o, const map_location &l, bool d)
-			: nb_options(o), loc(l), use_dialog(d)
-		{}
-
-		virtual config query_user(int /*side*/) const
-		{
-			int selected;
-			if (use_dialog) {
-				DBG_NG << "dialog requested\n";
-				selected = dialogs::advance_unit_dialog(loc);
-			} else {
-				// VITAL this is NOT done using the synced RNG
-				selected = rand() % nb_options;
-			}
-			config cfg;
-			cfg["value"] = selected;
-			return cfg;
-		}
-
-		virtual config random_choice(int /*side*/) const
-		{
-			config cfg;
-			cfg["value"] = random_new::generator->next_random() % nb_options;
-			return cfg;
-		}
-	};
 } // end anonymous namespace (types)
 
 namespace { // Variables
@@ -2544,20 +2511,10 @@ WML_HANDLER_FUNCTION(unstore_unit, /*event_info*/, cfg)
 				// Print floating label
 				resources::screen->float_label(loc, text, cfg["red"], cfg["green"], cfg["blue"]);
 			}
-
-			const int side = controller->current_side();
-			unsigned loop_limit = 5;
-			while ( advance
-					&& unit_helper::will_certainly_advance(resources::units->find(loc))
-			        && (loop_limit > 0) )
-			{
-				loop_limit--;
-				int total_opt = unit_helper::number_of_possible_advances(*u);
-				bool use_dialog = side == u->side() &&
-					(*resources::teams)[side - 1].is_human();
-				config selected = mp_sync::get_user_choice("choose",
-					unstore_unit_advance_choice(total_opt, loc, use_dialog));
-				dialogs::animate_unit_advancement(loc, selected["value"], cfg["fire_event"].to_bool(false), cfg["animate"].to_bool(true));
+			if(advance) {
+				advance_unit_at(advance_unit_params(loc)
+					.fire_events(cfg["fire_event"].to_bool(false))
+					.animate(cfg["animate"].to_bool(true)));
 			}
 		} else {
 			if(advance && u->advances()) {

src/menu_events.cpp

@@ -3162,7 +3162,7 @@ void console_handler::do_unit() {
 		for (int levels=0; levels<int_value; levels++) {
 			i->set_experience(i->max_experience());
 
-			advance_unit_at(loc,true);
+			advance_unit_at(advance_unit_params(loc).force_dialog(true));
 			i = menu_handler_.units_.find(loc);
 			if (!i.valid()) {
 				break;