Merge lp:~widelands-dev/widelands/fix_asan_crash_nethost into lp:widelands

[SirVer]

Renamed some variables and fixed an ASAN reported crash.

The bug was that in MultiPlayerPlayerGroup, settings_->settings().players[id] was used. This vector always has the length of the players in the map. But id runs from 0 to MAX_PLAYERS - 1. The fix was simply to check that id < players.size().

[GunChleoc (gunchleoc)]

LGTM :)

Those index shifts were a major pain in the butt too when I implemented that.

@bunnybot merge

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2863. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/307774210.
Appveyor build 2672. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_fix_asan_crash_nethost-2672.

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2017-11-14 08:15:48 +0000
+++ src/network/gamehost.cc	2017-11-27 07:16:30 +0000
@@ -587,9 +587,9 @@
 			disconnect_client(i, "GAME_STARTED_AT_CONNECT");
 	}
 
-	SendPacket s;
-	s.unsigned_8(NETCMD_LAUNCH);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_LAUNCH);
+	broadcast(packet);
 
 	Widelands::Game game;
 	game.set_ai_training_mode(g_options.pull_section("global").get_bool("ai_training", false));
@@ -704,10 +704,10 @@
 			} else if (curtime - d->last_heartbeat >= SERVER_TIMESTAMP_INTERVAL) {
 				d->last_heartbeat = curtime;
 
-				SendPacket s;
-				s.unsigned_8(NETCMD_TIME);
-				s.signed_32(d->pseudo_networktime);
-				broadcast(s);
+				SendPacket packet;
+				packet.unsigned_8(NETCMD_TIME);
+				packet.signed_32(d->pseudo_networktime);
+				broadcast(packet);
 
 				committed_network_time(d->pseudo_networktime);
 
@@ -724,11 +724,11 @@
 void GameHost::send_player_command(Widelands::PlayerCommand& pc) {
 	pc.set_duetime(d->committed_networktime + 1);
 
-	SendPacket s;
-	s.unsigned_8(NETCMD_PLAYERCOMMAND);
-	s.signed_32(pc.duetime());
-	pc.serialize(s);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_PLAYERCOMMAND);
+	packet.signed_32(pc.duetime());
+	pc.serialize(packet);
+	broadcast(packet);
 	d->game->enqueue_command(&pc);
 
 	committed_network_time(d->committed_networktime + 1);
@@ -746,38 +746,38 @@
 		return;
 
 	if (msg.recipient.empty()) {
-		SendPacket s;
-		s.unsigned_8(NETCMD_CHAT);
-		s.signed_16(msg.playern);
-		s.string(msg.sender);
-		s.string(msg.msg);
-		s.unsigned_8(0);
-		broadcast(s);
+		SendPacket packet;
+		packet.unsigned_8(NETCMD_CHAT);
+		packet.signed_16(msg.playern);
+		packet.string(msg.sender);
+		packet.string(msg.msg);
+		packet.unsigned_8(0);
+		broadcast(packet);
 
 		d->chat.receive(msg);
 	} else {  //  personal messages
-		SendPacket s;
-		s.unsigned_8(NETCMD_CHAT);
+		SendPacket packet;
+		packet.unsigned_8(NETCMD_CHAT);
 
 		// Is this a pm for the host player?
 		if (d->localplayername == msg.recipient) {
 			d->chat.receive(msg);
 			// Write the SendPacket - will be used below to show that the message
 			// was received.
-			s.signed_16(msg.playern);
-			s.string(msg.sender);
-			s.string(msg.msg);
-			s.unsigned_8(1);
-			s.string(msg.recipient);
+			packet.signed_16(msg.playern);
+			packet.string(msg.sender);
+			packet.string(msg.msg);
+			packet.unsigned_8(1);
+			packet.string(msg.recipient);
 		} else {  // Find the recipient
 			int32_t clientnum = check_client(msg.recipient);
 			if (clientnum >= 0) {
-				s.signed_16(msg.playern);
-				s.string(msg.sender);
-				s.string(msg.msg);
-				s.unsigned_8(1);
-				s.string(msg.recipient);
-				d->net->send(d->clients.at(clientnum).sock_id, s);
+				packet.signed_16(msg.playern);
+				packet.string(msg.sender);
+				packet.string(msg.msg);
+				packet.unsigned_8(1);
+				packet.string(msg.recipient);
+				d->net->send(d->clients.at(clientnum).sock_id, packet);
 				log(
 				   "[Host]: personal chat: from %s to %s\n", msg.sender.c_str(), msg.recipient.c_str());
 			} else {
@@ -791,10 +791,10 @@
 					d->chat.receive(err);
 					return;  // nothing left to do!
 				}
-				s.signed_16(-2);  // System message
-				s.string("");
-				s.string(fail);
-				s.unsigned_8(0);
+				packet.signed_16(-2);  // System message
+				packet.string("");
+				packet.string(fail);
+				packet.unsigned_8(0);
 			}
 		}
 
@@ -819,7 +819,7 @@
 					if (d->clients.at(j).usernum == static_cast<int16_t>(i))
 						break;
 				if (j < d->clients.size())
-					d->net->send(d->clients.at(j).sock_id, s);
+					d->net->send(d->clients.at(j).sock_id, packet);
 				else
 					// Better no wexception it would break the whole game
 					log("WARNING: user was found but no client is connected to it!\n");
@@ -904,13 +904,13 @@
                                         const std::string& b,
                                         const std::string& c) {
 	// First send to all clients
-	SendPacket s;
-	s.unsigned_8(NETCMD_SYSTEM_MESSAGE_CODE);
-	s.string(code);
-	s.string(a);
-	s.string(b);
-	s.string(c);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SYSTEM_MESSAGE_CODE);
+	packet.string(code);
+	packet.string(a);
+	packet.string(b);
+	packet.string(c);
+	broadcast(packet);
 
 	// Now add to our own chatbox
 	ChatMessage msg(NetworkGamingMessages::get_message(code, a, b, c));
@@ -967,7 +967,7 @@
 
 	std::vector<PlayerSettings>::size_type oldplayers = d->settings.players.size();
 
-	SendPacket s;
+	SendPacket packet;
 
 	// Care about the host
 	if (static_cast<int32_t>(maxplayers) <= d->settings.playernum &&
@@ -989,11 +989,11 @@
 				d->clients.at(j).playernum = UserSettings::none();
 
 				// Broadcast change
-				s.reset();
-				s.unsigned_8(NETCMD_SETTING_USER);
-				s.unsigned_32(i);
-				write_setting_user(s, i);
-				broadcast(s);
+				packet.reset();
+				packet.unsigned_8(NETCMD_SETTING_USER);
+				packet.unsigned_32(i);
+				write_setting_user(packet, i);
+				broadcast(packet);
 			}
 	}
 
@@ -1016,16 +1016,16 @@
 	}
 
 	// Broadcast new map info
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_MAP);
-	write_setting_map(s);
-	broadcast(s);
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_MAP);
+	write_setting_map(packet);
+	broadcast(packet);
 
 	// Broadcast new player settings
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_ALLPLAYERS);
-	write_setting_all_players(s);
-	broadcast(s);
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_ALLPLAYERS);
+	write_setting_all_players(packet);
+	broadcast(packet);
 
 	// If possible, offer the map / saved game as transfer
 	// TODO(unknown): not yet able to handle directory type maps / savegames
@@ -1060,9 +1060,10 @@
 		}
 	}
 
-	s.reset();
-	if (write_map_transfer_info(s, mapfilename))
-		broadcast(s);
+	packet.reset();
+	if (write_map_transfer_info(packet, mapfilename)) {
+		broadcast(packet);
+	}
 }
 
 void GameHost::set_player_state(uint8_t const number,
@@ -1138,18 +1139,18 @@
 		player.name = get_computer_player_name(number);
 
 	// Broadcast change to player
-	SendPacket s;
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 
 	// Let clients know whether their slot has changed
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_ALLUSERS);
-	write_setting_all_users(s);
-	broadcast(s);
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_ALLUSERS);
+	write_setting_all_users(packet);
+	broadcast(packet);
 }
 
 void GameHost::set_player_tribe(uint8_t const number,
@@ -1179,11 +1180,11 @@
 				player.initialization_index = 0;
 
 			//  broadcast changes
-			SendPacket s;
-			s.unsigned_8(NETCMD_SETTING_PLAYER);
-			s.unsigned_8(number);
-			write_setting_player(s, number);
-			broadcast(s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_SETTING_PLAYER);
+			packet.unsigned_8(number);
+			write_setting_player(packet, number);
+			broadcast(packet);
 			return;
 		}
 	}
@@ -1205,11 +1206,11 @@
 				player.initialization_index = index;
 
 				//  broadcast changes
-				SendPacket s;
-				s.unsigned_8(NETCMD_SETTING_PLAYER);
-				s.unsigned_8(number);
-				write_setting_player(s, number);
-				broadcast(s);
+				SendPacket packet;
+				packet.unsigned_8(NETCMD_SETTING_PLAYER);
+				packet.unsigned_8(number);
+				write_setting_player(packet, number);
+				broadcast(packet);
 				return;
 			} else
 				log("Attempted to change to out-of-range initialization index %u "
@@ -1230,11 +1231,11 @@
 	player.random_ai = random_ai;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 }
 
 void GameHost::set_player_name(uint8_t const number, const std::string& name) {
@@ -1249,11 +1250,11 @@
 	player.name = name;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 }
 
 void GameHost::set_player_closeable(uint8_t const number, bool closeable) {
@@ -1288,11 +1289,11 @@
 	player.tribe = sharedplr.tribe;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 }
 
 void GameHost::set_player(uint8_t const number, const PlayerSettings& ps) {
@@ -1303,11 +1304,11 @@
 	player = ps;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 }
 
 void GameHost::set_player_number(uint8_t const number) {
@@ -1318,10 +1319,10 @@
 	d->settings.win_condition_script = wc;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_WIN_CONDITION);
-	s.string(wc);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_WIN_CONDITION);
+	packet.string(wc);
+	broadcast(packet);
 }
 
 void GameHost::switch_to_player(uint32_t user, uint8_t number) {
@@ -1367,11 +1368,11 @@
 	}
 
 	// Broadcast the user changes to everybody
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_USER);
-	s.unsigned_32(user);
-	write_setting_user(s, user);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_USER);
+	packet.unsigned_32(user);
+	write_setting_user(packet, user);
+	broadcast(packet);
 }
 
 void GameHost::set_player_team(uint8_t number, Widelands::TeamNumber team) {
@@ -1380,11 +1381,11 @@
 	d->settings.players.at(number).team = team;
 
 	// Broadcast changes
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETTING_PLAYER);
-	s.unsigned_8(number);
-	write_setting_player(s, number);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETTING_PLAYER);
+	packet.unsigned_8(number);
+	write_setting_player(packet, number);
+	broadcast(packet);
 }
 
 void GameHost::set_multiplayer_game_settings() {
@@ -1476,11 +1477,11 @@
 }
 
 /**
-* If possible, this function writes the MapTransferInfo to SendPacket & s
+* If possible, this function writes the MapTransferInfo to SendPacket & packet
 *
 * \returns true if the data was written, else false
 */
-bool GameHost::write_map_transfer_info(SendPacket& s, std::string mapfilename) {
+bool GameHost::write_map_transfer_info(SendPacket& packet, std::string mapfilename) {
 	// TODO(unknown): not yet able to handle directory type maps / savegames
 	if (g_fs->is_directory(mapfilename)) {
 		log("Map/Save is a directory! No way for making it available a.t.m.!\n");
@@ -1489,13 +1490,13 @@
 
 	// Write the new map/save file information, so client can decide whether it
 	// needs the file.
-	s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
-	s.string(mapfilename);
+	packet.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
+	packet.string(mapfilename);
 	// Scan-build reports that access to bytes here results in a dereference of null pointer.
 	// This is a false positive.
 	// See https://bugs.launchpad.net/widelands/+bug/1198919
-	s.unsigned_32(file_->bytes);  // NOLINT
-	s.string(file_->md5sum);
+	packet.unsigned_32(file_->bytes);  // NOLINT
+	packet.string(file_->md5sum);
 	return true;
 }
 
@@ -1579,62 +1580,63 @@
 
 	log("[Host]: Client %u: welcome to usernum %u\n", number, client.usernum);
 
-	SendPacket s;
-	s.unsigned_8(NETCMD_HELLO);
-	s.unsigned_8(NETWORK_PROTOCOL_VERSION);
-	s.unsigned_32(client.usernum);
-	d->net->send(client.sock_id, s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_HELLO);
+	packet.unsigned_8(NETWORK_PROTOCOL_VERSION);
+	packet.unsigned_32(client.usernum);
+	d->net->send(client.sock_id, packet);
 	// even if the network protocol is the same, the data might be different.
 	if (client.build_id != build_id())
 		send_system_message_code("DIFFERENT_WL_VERSION", effective_name, client.build_id, build_id());
 	// Send information about currently selected map / savegame
-	s.reset();
+	packet.reset();
 
-	s.unsigned_8(NETCMD_SETTING_MAP);
-	write_setting_map(s);
-	d->net->send(client.sock_id, s);
+	packet.unsigned_8(NETCMD_SETTING_MAP);
+	write_setting_map(packet);
+	d->net->send(client.sock_id, packet);
 
 	// If possible, offer the map / savegame as transfer
 	if (file_) {
-		s.reset();
-		if (write_map_transfer_info(s, file_->filename))
-			d->net->send(client.sock_id, s);
+		packet.reset();
+		if (write_map_transfer_info(packet, file_->filename)) {
+			d->net->send(client.sock_id, packet);
+		}
 	}
 
 	//  Send the tribe information to the new client.
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_TRIBES);
-	s.unsigned_8(d->settings.tribes.size());
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_TRIBES);
+	packet.unsigned_8(d->settings.tribes.size());
 	for (const TribeBasicInfo& tribe : d->settings.tribes) {
-		s.string(tribe.name);
+		packet.string(tribe.name);
 		size_t const nr_initializations = tribe.initializations.size();
-		s.unsigned_8(nr_initializations);
+		packet.unsigned_8(nr_initializations);
 		for (const TribeBasicInfo::Initialization& init : tribe.initializations)
-			s.string(init.script);
+			packet.string(init.script);
 	}
-	d->net->send(client.sock_id, s);
-
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_ALLPLAYERS);
-	write_setting_all_players(s);
-	d->net->send(client.sock_id, s);
-
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_ALLUSERS);
-	write_setting_all_users(s);
-	d->net->send(client.sock_id, s);
-
-	s.reset();
-	s.unsigned_8(NETCMD_WIN_CONDITION);
-	s.string(d->settings.win_condition_script);
-	d->net->send(client.sock_id, s);
+	d->net->send(client.sock_id, packet);
+
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_ALLPLAYERS);
+	write_setting_all_players(packet);
+	d->net->send(client.sock_id, packet);
+
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_ALLUSERS);
+	write_setting_all_users(packet);
+	d->net->send(client.sock_id, packet);
+
+	packet.reset();
+	packet.unsigned_8(NETCMD_WIN_CONDITION);
+	packet.string(d->settings.win_condition_script);
+	d->net->send(client.sock_id, packet);
 
 	// Broadcast new information about the player to everybody
-	s.reset();
-	s.unsigned_8(NETCMD_SETTING_USER);
-	s.unsigned_32(client.usernum);
-	write_setting_user(s, client.usernum);
-	broadcast(s);
+	packet.reset();
+	packet.unsigned_8(NETCMD_SETTING_USER);
+	packet.unsigned_32(client.usernum);
+	write_setting_user(packet, client.usernum);
+	broadcast(packet);
 
 	// Check if there is an unoccupied player left and if, assign.
 	for (uint8_t i = 0; i < d->settings.players.size(); ++i)
@@ -1731,9 +1733,9 @@
 			d->waiting = true;
 			broadcast_real_speed(0);
 
-			SendPacket s;
-			s.unsigned_8(NETCMD_WAIT);
-			broadcast(s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_WAIT);
+			broadcast(packet);
 		}
 	} else {
 		if (nrdelayed == 0) {
@@ -1748,10 +1750,10 @@
 void GameHost::broadcast_real_speed(uint32_t const speed) {
 	assert(speed <= std::numeric_limits<uint16_t>::max());
 
-	SendPacket s;
-	s.unsigned_8(NETCMD_SETSPEED);
-	s.unsigned_16(speed);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SETSPEED);
+	packet.unsigned_16(speed);
+	broadcast(packet);
 }
 
 /**
@@ -1829,10 +1831,10 @@
 
 	log("[Host]: Requesting sync reports for time %i\n", d->syncreport_time);
 
-	SendPacket s;
-	s.unsigned_8(NETCMD_SYNCREQUEST);
-	s.signed_32(d->syncreport_time);
-	broadcast(s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_SYNCREQUEST);
+	packet.signed_32(d->syncreport_time);
+	broadcast(packet);
 
 	d->game->enqueue_command(
 	   new CmdNetCheckSync(d->syncreport_time, [this] { sync_report_callback(); }));
@@ -1870,9 +1872,9 @@
 
 			d->game->save_syncstream(true);
 
-			SendPacket s;
-			s.unsigned_8(NETCMD_INFO_DESYNC);
-			broadcast(s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_INFO_DESYNC);
+			broadcast(packet);
 
 			disconnect_client(i, "CLIENT_DESYNCED");
 			// Pause the game, so that host and client have time to handle the
@@ -1948,9 +1950,9 @@
 	if ((forced_pause_ || real_speed() == 0) && (time(nullptr) > (d->lastpauseping + 20))) {
 		d->lastpauseping = time(nullptr);
 
-		SendPacket s;
-		s.unsigned_8(NETCMD_PING);
-		broadcast(s);
+		SendPacket send_packet;
+		send_packet.unsigned_8(NETCMD_PING);
+		broadcast(send_packet);
 	}
 
 	reaper();
@@ -1984,9 +1986,9 @@
 		if (cmd == NETCMD_METASERVER_PING) {
 			log("[Host]: Received ping from metaserver.\n");
 			// Send PING back
-			SendPacket s;
-			s.unsigned_8(NETCMD_METASERVER_PING);
-			d->net->send(client.sock_id, s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_METASERVER_PING);
+			d->net->send(client.sock_id, packet);
 
 			// Remove metaserver from list of clients
 			client.playernum = UserSettings::not_connected();
@@ -2155,11 +2157,11 @@
 		send_file_part(client.sock_id, 0);
 		// Remember client as "currently receiving file"
 		d->settings.users[client.usernum].ready = false;
-		SendPacket s;
-		s.unsigned_8(NETCMD_SETTING_USER);
-		s.unsigned_32(client.usernum);
-		write_setting_user(s, client.usernum);
-		broadcast(s);
+		SendPacket packet;
+		packet.unsigned_8(NETCMD_SETTING_USER);
+		packet.unsigned_32(client.usernum);
+		write_setting_user(packet, client.usernum);
+		broadcast(packet);
 		break;
 	}
 
@@ -2179,11 +2181,11 @@
 			send_system_message_code(
 			   "COMPLETED_FILE_TRANSFER", file_->filename, d->settings.users.at(client.usernum).name);
 			d->settings.users[client.usernum].ready = true;
-			SendPacket s;
-			s.unsigned_8(NETCMD_SETTING_USER);
-			s.unsigned_32(client.usernum);
-			write_setting_user(s, client.usernum);
-			broadcast(s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_SETTING_USER);
+			packet.unsigned_32(client.usernum);
+			write_setting_user(packet, client.usernum);
+			broadcast(packet);
 			return;
 		}
 		++part;
@@ -2207,12 +2209,12 @@
 	uint32_t size = (left > NETFILEPARTSIZE) ? NETFILEPARTSIZE : left;
 
 	// Send the part
-	SendPacket s;
-	s.unsigned_8(NETCMD_FILE_PART);
-	s.unsigned_32(part);
-	s.unsigned_32(size);
-	s.data(file_->parts[part].part, size);
-	d->net->send(csock_id, s);
+	SendPacket packet;
+	packet.unsigned_8(NETCMD_FILE_PART);
+	packet.unsigned_32(part);
+	packet.unsigned_32(size);
+	packet.data(file_->parts[part].part, size);
+	d->net->send(csock_id, packet);
 }
 
 void GameHost::disconnect_player_controller(uint8_t const number, const std::string& name) {
@@ -2265,11 +2267,11 @@
 		send_system_message_code(
 		   "CLIENT_X_LEFT_GAME", d->settings.users.at(client.usernum).name, reason, arg);
 
-		SendPacket s;
-		s.unsigned_8(NETCMD_SETTING_USER);
-		s.unsigned_32(client.usernum);
-		write_setting_user(s, client.usernum);
-		broadcast(s);
+		SendPacket packet;
+		packet.unsigned_8(NETCMD_SETTING_USER);
+		packet.unsigned_32(client.usernum);
+		write_setting_user(packet, client.usernum);
+		broadcast(packet);
 	} else
 		send_system_message_code("UNKNOWN_LEFT_GAME", reason, arg);
 
@@ -2277,13 +2279,14 @@
 
 	if (client.sock_id > 0) {
 		if (sendreason) {
-			SendPacket s;
-			s.unsigned_8(NETCMD_DISCONNECT);
-			s.unsigned_8(arg.empty() ? 1 : 2);
-			s.string(reason);
-			if (!arg.empty())
-				s.string(arg);
-			d->net->send(client.sock_id, s);
+			SendPacket packet;
+			packet.unsigned_8(NETCMD_DISCONNECT);
+			packet.unsigned_8(arg.empty() ? 1 : 2);
+			packet.string(reason);
+			if (!arg.empty()) {
+				packet.string(arg);
+			}
+			d->net->send(client.sock_id, packet);
 		}
 
 		d->net->close(client.sock_id);
 
=== modified file 'src/wui/multiplayersetupgroup.cc'
--- src/wui/multiplayersetupgroup.cc	2017-09-02 21:48:44 +0000
+++ src/wui/multiplayersetupgroup.cc	2017-11-27 07:16:30 +0000
@@ -61,7 +61,7 @@
 	     // Name needs to be initialized after the dropdown, otherwise the layout function will
 	     // crash.
 	     name(this, 0, 0, w - h - UI::Scrollbar::kSize * 11 / 5, h),
-	     s(settings),
+	     settings_(settings),
 	     id_(id),
 	     slot_selection_locked_(false) {
 		set_size(w, h);
@@ -106,7 +106,7 @@
 	/// This will update the client's player slot with the value currently selected in the slot
 	/// dropdown.
 	void set_slot() {
-		const GameSettings& settings = s->settings();
+		const GameSettings& settings = settings_->settings();
 		if (id_ != settings.usernum) {
 			return;
 		}
@@ -114,7 +114,7 @@
 		if (slot_dropdown_.has_selection()) {
 			const uint8_t new_slot = slot_dropdown_.get_selected();
 			if (new_slot != settings.users.at(id_).position) {
-				s->set_player_number(slot_dropdown_.get_selected());
+				settings_->set_player_number(slot_dropdown_.get_selected());
 			}
 		}
 		slot_selection_locked_ = false;
@@ -147,7 +147,7 @@
 
 	/// Take care of visibility and current values
 	void update() {
-		const GameSettings& settings = s->settings();
+		const GameSettings& settings = settings_->settings();
 		const UserSettings& user_setting = settings.users.at(id_);
 
 		if (user_setting.position == UserSettings::not_connected()) {
@@ -161,7 +161,7 @@
 
 	UI::Dropdown<uintptr_t> slot_dropdown_;  /// Select the player slot.
 	UI::Textarea name;                       /// Client nick name
-	GameSettingsProvider* const s;
+	GameSettingsProvider* const settings_;
 	uint8_t const id_;            /// User number
 	bool slot_selection_locked_;  // Ensure that dropdowns will close on selection.
 	std::unique_ptr<Notifications::Subscriber<NoteGameSettings>> subscriber_;
@@ -176,7 +176,7 @@
 	                       GameSettingsProvider* const settings,
 	                       NetworkPlayerSettingsBackend* const npsb)
 	   : UI::Box(parent, 0, 0, UI::Box::Horizontal, w, h, kPadding / 2),
-	     s(settings),
+	     settings_(settings),
 	     n(npsb),
 	     id_(id),
 	     player(this,
@@ -227,23 +227,26 @@
 		add_space(0);
 
 		subscriber_ =
-		   Notifications::subscribe<NoteGameSettings>([this](const NoteGameSettings& note) {
-			   switch (note.action) {
-			   case NoteGameSettings::Action::kMap:
-				   // We don't care about map updates, since we receive enough notifications for the
-				   // slots.
-				   break;
-			   default:
-				   if (s->settings().players.empty()) {
-					   // No map/savegame yet
-					   return;
-				   }
-				   if (id_ == note.position ||
-				       s->settings().players[id_].state == PlayerSettings::State::kShared) {
-					   update();
-				   }
-			   }
-			});
+		   Notifications::subscribe<NoteGameSettings>(
+		      [this](const NoteGameSettings& note) {
+			      const std::vector<PlayerSettings> & players = settings_->settings().players;
+			      switch (note.action) {
+			      case NoteGameSettings::Action::kMap:
+				      // We don't care about map updates, since we receive enough notifications for the
+				      // slots.
+				      break;
+			      default:
+				      if (players.empty()) {
+					      // No map/savegame yet
+					      return;
+				      }
+				      if (id_ == note.position ||
+				          (id_ < players.size() &&
+				           players.at(id_).state == PlayerSettings::State::kShared)) {
+					      update();
+				      }
+			      }
+			   });
 
 		// Init dropdowns
 		update();
@@ -263,7 +266,7 @@
 	/// This will update the game settings for the type with the value
 	/// currently selected in the type dropdown.
 	void set_type() {
-		if (!s->can_change_player_state(id_)) {
+		if (!settings_->can_change_player_state(id_)) {
 			return;
 		}
 		type_selection_locked_ = true;
@@ -328,7 +331,7 @@
 		type_dropdown_.add(
 		   _("Open"), "open", g_gr->images().get("images/ui_basic/continue.png"), false, _("Open"));
 
-		type_dropdown_.set_enabled(s->can_change_player_state(id_));
+		type_dropdown_.set_enabled(settings_->can_change_player_state(id_));
 
 		// Now select the entry according to server settings
 		const PlayerSettings& player_setting = settings.players[id_];
@@ -360,9 +363,9 @@
 
 	/// Whether the client who is running the UI is allowed to change the tribe for this player slot.
 	bool has_tribe_access() {
-		return s->settings().players[id_].state == PlayerSettings::State::kShared ?
-		          s->can_change_player_init(id_) :
-		          s->can_change_player_tribe(id_);
+		return settings_->settings().players[id_].state == PlayerSettings::State::kShared ?
+		          settings_->can_change_player_init(id_) :
+		          settings_->can_change_player_tribe(id_);
 	}
 
 	/// This will update the game settings for the tribe or shared_in with the value
@@ -371,7 +374,7 @@
 		if (!has_tribe_access()) {
 			return;
 		}
-		const PlayerSettings& player_settings = s->settings().players[id_];
+		const PlayerSettings& player_settings = settings_->settings().players[id_];
 		tribe_selection_locked_ = true;
 		tribes_dropdown_.set_disable_style(player_settings.state == PlayerSettings::State::kShared ?
 		                                      UI::ButtonDisableStyle::kPermpressed :
@@ -451,7 +454,7 @@
 	/// This will update the game settings for the initialization with the value
 	/// currently selected in the initialization dropdown.
 	void set_init() {
-		if (!s->can_change_player_init(id_)) {
+		if (!settings_->can_change_player_init(id_)) {
 			return;
 		}
 		init_selection_locked_ = true;
@@ -487,7 +490,7 @@
 		}
 
 		init_dropdown_.set_visible(true);
-		init_dropdown_.set_enabled(s->can_change_player_init(id_));
+		init_dropdown_.set_enabled(settings_->can_change_player_init(id_));
 	}
 
 	/// This will update the team settings with the value currently selected in the teams dropdown.
@@ -524,12 +527,12 @@
 		}
 		team_dropdown_.select(player_setting.team);
 		team_dropdown_.set_visible(true);
-		team_dropdown_.set_enabled(s->can_change_player_team(id_));
+		team_dropdown_.set_enabled(settings_->can_change_player_team(id_));
 	}
 
 	/// Refresh all user interfaces
 	void update() {
-		const GameSettings& settings = s->settings();
+		const GameSettings& settings = settings_->settings();
 		if (id_ >= settings.players.size()) {
 			set_visible(false);
 			return;
@@ -556,7 +559,7 @@
 		// Trigger update for the other players for shared_in mode when slots open and close
 		if (last_state_ != player_setting.state) {
 			last_state_ = player_setting.state;
-			for (PlayerSlot slot = 0; slot < s->settings().players.size(); ++slot) {
+			for (PlayerSlot slot = 0; slot < settings_->settings().players.size(); ++slot) {
 				if (slot != id_) {
 					n->set_player_state(slot, settings.players[slot].state);
 				}
@@ -564,7 +567,7 @@
 		}
 	}
 
-	GameSettingsProvider* const s;
+	GameSettingsProvider* const settings_;
 	NetworkPlayerSettingsBackend* const n;
 	PlayerSlot const id_;
 
@@ -594,8 +597,8 @@
                                              GameSettingsProvider* const settings,
                                              uint32_t buth)
    : UI::Box(parent, x, y, UI::Box::Horizontal, w, h, 8 * kPadding),
-     s(settings),
-     npsb(new NetworkPlayerSettingsBackend(s)),
+     settings_(settings),
+     npsb(new NetworkPlayerSettingsBackend(settings_)),
      clientbox(this, 0, 0, UI::Box::Vertical),
      playerbox(this, 0, 0, UI::Box::Vertical, w * 9 / 15, h, kPadding),
      buth_(buth) {
@@ -611,7 +614,7 @@
 	multi_player_player_groups.resize(kMaxPlayers);
 	for (PlayerSlot i = 0; i < multi_player_player_groups.size(); ++i) {
 		multi_player_player_groups.at(i) =
-		   new MultiPlayerPlayerGroup(&playerbox, playerbox.get_w(), buth_, i, s, npsb.get());
+		   new MultiPlayerPlayerGroup(&playerbox, playerbox.get_w(), buth_, i, settings, npsb.get());
 		playerbox.add(multi_player_player_groups.at(i));
 	}
 	playerbox.add_space(0);
@@ -627,7 +630,7 @@
 
 /// Update which slots are available based on current settings.
 void MultiPlayerSetupGroup::update() {
-	const GameSettings& settings = s->settings();
+	const GameSettings& settings = settings_->settings();
 
 	// Update / initialize client groups
 	if (multi_player_client_groups.size() < settings.users.size()) {
@@ -636,7 +639,7 @@
 	for (uint32_t i = 0; i < settings.users.size(); ++i) {
 		if (!multi_player_client_groups.at(i)) {
 			multi_player_client_groups.at(i) =
-			   new MultiPlayerClientGroup(&clientbox, clientbox.get_w(), buth_, i, s);
+			   new MultiPlayerClientGroup(&clientbox, clientbox.get_w(), buth_, i, settings_);
 			clientbox.add(multi_player_client_groups.at(i), UI::Box::Resizing::kFullSize);
 			multi_player_client_groups.at(i)->layout();
 		}
 
=== modified file 'src/wui/multiplayersetupgroup.h'
--- src/wui/multiplayersetupgroup.h	2017-06-26 11:32:11 +0000
+++ src/wui/multiplayersetupgroup.h	2017-11-27 07:16:30 +0000
@@ -57,7 +57,7 @@
 	void update();
 	void draw(RenderTarget& dst) override;
 
-	GameSettingsProvider* const s;
+	GameSettingsProvider* const settings_;
 	std::unique_ptr<NetworkPlayerSettingsBackend> npsb;
 	std::vector<MultiPlayerClientGroup*> multi_player_client_groups;  // not owned
 	std::vector<MultiPlayerPlayerGroup*> multi_player_player_groups;  // not owned