MP Staging: fixed team setting issue when some sides were hidden (fixes #2436)

This was caused by the same bug I added a workaround for in a02c568b78.
Essentially, the differing size of player_teams_ and team_names_ could cause an index mismatch if
some sides had allow_player=no and debug mode wasn't on. I didn't notice this before since I always
work with debug mode on.

I've reworked the connect_engine handling of team data. Now, the team_name, user_team_name, and an
is-player-allowed-to-be-on-this-team flag are all handled by a data pod. I've completely removed the
player_teams stuff and instead generate an applicable list of teams for the team menu_button to
display at runtime.

To get around the index disparity issue, I save any displayed team option's index (relative to
connect_engine::team_data) in that option's config (the one then passed to the menu_button). This
allows me to get an appropriate value to pass to connect_engine::set_team without any fancy find-
and-adjust calculations.

src/game_initialization/connect_engine.cpp

@@ -80,9 +80,7 @@ connect_engine::connect_engine(saved_game& state, const bool first_scenario, mp_
 	, force_lock_settings_()
 	, side_engines_()
 	, era_factions_()
-	, team_names_()
-	, user_team_names_()
-	, player_teams_()
+	, team_data_()
 {
 	// Initial level config from the mp_game_settings.
 	level_ = mp::initial_level_config(state_);
@@ -123,10 +121,9 @@ connect_engine::connect_engine(saved_game& state, const bool first_scenario, mp_
 		bool add_team = true;
 		if(params_.use_map_settings) {
 			// Only add a team if it is not found.
-			bool found = std::find(team_names_.begin(), team_names_.end(),
-				team_name.str()) != team_names_.end();
-
-			if(found) {
+			if(std::any_of(team_data_.begin(), team_data_.end(), [&team_name](const team_data_pod& data){
+				return data.team_name == team_name.str();
+			})) {
 				add_team = false;
 			}
 		} else {
@@ -147,12 +144,12 @@ connect_engine::connect_engine(saved_game& state, const bool first_scenario, mp_
 		}
 
 		if(add_team) {
-			team_names_.push_back(params_.use_map_settings ? team_name : "Team " + side_str);
-			user_team_names_.push_back(user_team_name.t_str().to_serialized());
+			team_data_pod data;
+			data.team_name = params_.use_map_settings ? team_name : "Team " + side_str;
+			data.user_team_name = user_team_name.t_str().to_serialized();
+			data.is_player_team = side["allow_player"].to_bool(true);
 
-			if(side["allow_player"].to_bool(true) || game_config::debug) {
-				player_teams_.push_back(user_team_name.str());
-			}
+			team_data_.push_back(data);
 		}
 
 		++side_count;
@@ -928,15 +925,16 @@ side_engine::side_engine(const config& cfg, connect_engine& parent_engine, const
 
 	// Initialize team and color.
 	unsigned team_name_index = 0;
-	for(const std::string& name : parent_.team_names_) {
-		if(name == cfg["team_name"]) {
+	for(const connect_engine::team_data_pod& data : parent_.team_data_) {
+		if(data.team_name == cfg["team_name"]) {
 			break;
 		}
 
-		team_name_index++;
+		++team_name_index;
 	}
-	if(team_name_index >= parent_.team_names_.size()) {
-		assert(!parent_.team_names_.empty());
+
+	if(team_name_index >= parent_.team_data_.size()) {
+		assert(!parent_.team_data_.empty());
 		team_ = 0;
 		WRN_MP << "In side_engine constructor: Could not find my team_name " << cfg["team_name"] << " among the mp connect engine's list of team names. I am being assigned to the first team. This may indicate a bug!" << std::endl;
 	} else {
@@ -1103,7 +1101,7 @@ config side_engine::new_config() const
 			(*leader)["gender"] = "null";
 		}
 
-		res["team_name"] = parent_.team_names_[team_];
+		res["team_name"] = parent_.team_data_[team_].team_name;
 
 		// TODO: Fix this mess!
 		//
@@ -1140,7 +1138,7 @@ config side_engine::new_config() const
 		// I'm tired, and I'm cranky from wasting a over day on this, and so I'm exercising
 		// my prerogative as a grey-beard and leaving this for someone else to clean up.
 		if(res["user_team_name"].empty() || !parent_.params_.use_map_settings) {
-			res["user_team_name"] = parent_.user_team_names_[team_];
+			res["user_team_name"] = parent_.team_data_[team_].user_team_name;
 		}
 
 		res["allow_player"] = allow_player_;

src/game_initialization/connect_engine.hpp

@@ -89,8 +89,20 @@ public:
 			throw "No scenariodata found";
 	}
 	const std::set<std::string>& connected_users() const;
-	const std::vector<std::string>& user_team_names() const
-		{ return user_team_names_; }
+
+	struct team_data_pod
+	{
+		std::string team_name;
+		std::string user_team_name; // TODO: use t_string?
+
+		bool is_player_team;
+	};
+
+	const std::vector<team_data_pod>& team_data() const
+	{
+		return team_data_;
+	}
+
 	std::vector<side_engine_ptr>& side_engines() { return side_engines_; }
 	const mp_game_settings& params() const { return params_; }
 	bool first_scenario() const { return first_scenario_; }
@@ -129,9 +141,7 @@ private:
 
 	std::vector<side_engine_ptr> side_engines_;
 	std::vector<const config*> era_factions_;
-	std::vector<std::string> team_names_;
-	std::vector<std::string> user_team_names_;
-	std::vector<std::string> player_teams_;
+	std::vector<team_data_pod> team_data_;
 
 	std::set<std::string>& connected_users_rw();
 	void send_to_server(const config& cfg) const;
@@ -214,8 +224,6 @@ public:
 	bool waiting_to_choose_faction() const { return waiting_to_choose_faction_; }
 	void set_waiting_to_choose_status(bool status) { waiting_to_choose_faction_ = status;}
 	bool allow_shuffle() const { return !disallow_shuffle_;}
-	const std::vector<std::string>& player_teams() const
-		{ return parent_.player_teams_; }
 	flg_manager& flg() { return flg_; }
 
 	const std::string color_id() const { return color_id_; }
@@ -223,12 +231,12 @@ public:
 
 	const std::string team_name() const
 	{
-		return parent_.team_names_[team_];
+		return parent_.team_data_[team_].team_name;
 	}
 
 	const std::string user_team_name() const
 	{
-		return t_string::from_serialized(parent_.user_team_names_[team_]);
+		return t_string::from_serialized(parent_.team_data_[team_].user_team_name);
 	}
 
 private:

src/gui/dialogs/multiplayer/mp_staging.cpp

@@ -89,11 +89,9 @@ void mp_staging::pre_show(window& window)
 	// Set up sides list
 	//
 	for(const auto& side : connect_engine_.side_engines()) {
-		if(!side->allow_player() && !game_config::debug) {
-			continue;
+		if(side->allow_player() || game_config::debug) {
+			add_side_node(window, side);;
 		}
-
-		add_side_node(window, side);
 	}
 
 	//
@@ -198,7 +196,7 @@ void mp_staging::add_side_node(window& window, ng::side_engine_ptr side)
 
 	// We use an index-based loop in order to get the index of the selected option
 	std::vector<config> ai_options;
-	for(unsigned i = 0; i < ai_algorithms_.size(); i++) {
+	for(unsigned i = 0; i < ai_algorithms_.size(); ++i) {
 		ai_options.emplace_back(config {"label", ai_algorithms_[i]->text});
 
 		if(ai_algorithms_[i]->id == side->ai_algorithm()) {
@@ -243,17 +241,36 @@ void mp_staging::add_side_node(window& window, ng::side_engine_ptr side)
 	// Team
 	//
 	std::vector<config> team_names;
-	for(const auto& team : side->player_teams()) {
-		team_names.emplace_back(config {"label", team});
+	unsigned initial_team_selection = 0;
+
+	for(unsigned i = 0; i < connect_engine_.team_data().size(); ++i) {
+		const ng::connect_engine::team_data_pod& tdata = connect_engine_.team_data()[i];
+
+		if(!tdata.is_player_team && !game_config::debug) {
+			continue;
+		}
+
+		config entry;
+		entry["label"] = t_string::from_serialized(tdata.user_team_name);
+
+		// Since we're not necessarily displaying every every team, we need to store the
+		// index a displayed team has in the connect_engine's team_data vector. This is
+		// then utilized in the click callback.
+		entry["team_index"] = i;
+
+		team_names.push_back(std::move(entry));
+
+		// Since, again, every team might not be displayed, and side_engine::team() returns
+		// an index into the team_data vector, get an initial selection index for the menu
+		// adjusted for the displayed named.
+		if(side->team() == i) {
+			initial_team_selection = team_names.size() - 1;
+		}
 	}
 
 	menu_button& team_selection = find_widget<menu_button>(&row_grid, "side_team", false);
 
-	// HACK: side->team() does not get its index from side->player_teams(), but rather side->team_names().
-	// As such, the index is off if there is only 1 playable team. This is a hack to make sure the menu_button
-	// widget doesn't assert with the invalid initial selection. The connect_engine should be fixed once the GUI1
-	// dialog is dropped
-	team_selection.set_values(team_names, std::min<int>(team_names.size() - 1, side->team()));
+	team_selection.set_values(team_names, initial_team_selection);
 	team_selection.set_active(!saved_game);
 	team_selection.connect_click_handler(std::bind(&mp_staging::on_team_select, this, std::ref(window), side, std::ref(team_selection), _3, _4));
 
@@ -338,11 +355,17 @@ void mp_staging::on_color_select(ng::side_engine_ptr side, grid& row_grid)
 
 void mp_staging::on_team_select(window& window, ng::side_engine_ptr side, menu_button& team_menu, bool& handled, bool& halt)
 {
-	if(static_cast<int>(team_menu.get_value()) == side->team()) {
+	// Since we're not necessarily displaying every every team in the menu, we can't just
+	// use the selected index to set a side's team. Instead, we grab the index we stored
+	// in add_side_node from the selected config, which should correspond to the
+	// appropriate entry in the connect_engine's team name vector.
+	const unsigned team_index = team_menu.get_value_config()["team_index"].to_unsigned();
+
+	if(team_index == side->team()) {
 		return;
 	}
 
-	side->set_team(team_menu.get_value());
+	side->set_team(team_index);
 
 	// First, remove the node from the tree
 	find_widget<tree_view>(&window, "side_list", false).remove_node(side_tree_map_[side]);