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

[GunChleoc]

For multiplayer game setup, use dropdowns instead of buttons to select player states (open, closed, human, AIs, shared in). This uses 2 new notifications:
- NoteGameSettings to synchronize and update the game setup dropdowns
- NoteDropdown to ensure that only 1 dropdown is ever open at the same time

Also:

- Dropdown lists will now automatically expand upwards rather than downwards if they wouldn't fit on screen.

- Removed the tiny labels in the multiplayer game setup, since the function of each dropdown can be identified per its tooltip

- Pulled out some functions to avoid code duplications and shifted some calculations to GameHost.

This is the final branch in a 3-branch series to change all buttons in multiplayer setup to dropdown menus.

All BUGFIXING and TESTING is to be done ON THIS BRANCH.

I split this into 3 branches to try to make the code review diffs more manageable.

The interaction is complex, so this will need thorough testing. I tried to click on all possible combinations, but I might have missed some.

1. The dropdowns should dynamically change their contents to only contain usable entries
2. Server / client should always be in sync (apart from delays for processing everything)
3. No missing information
4. No crashes

[kaputtnik (franku)]

Does sometimes work, sometimes not (used the LAN/Direct IP setup):

Mostly the drop down box appear for a very short time after the mouse button is released.
Sometimes the drop down box appears when releasing the mouse button.
When the drop down box appears for one button, most of the other buttons shows the drop down box.
The described behave is unspecific to a particular button. E.g. sometimes the Tribe selection works, sometimes not.

Used the 'Nile' map for testing.

review: Needs Fixing

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2381. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/247251733.
Appveyor build 2209. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_multiplayer_dropdowns-2209.

[GunChleoc (gunchleoc)]

I have now tried this with setting up games between my Linux and Windows machines using the AppVeyor build, and I get none of these problems. Not a single one, all dropdowns work perfectly. So, I have no clue how I could possibly debug this :(

[kaputtnik (franku)]

I found what causing this:

Please try to click on the top area of a drop down button, so the tooltip is very near (or overlaps) the button. Then the drop down box disappear after mouse release.

If i click in the bottom area, all works fine.

[GunChleoc (gunchleoc)]

Now I get what you mean. Dropdowns will autoclose when you move the mouse away, and the tolerance on the top edge was too small. Should be fixed now.

[kaputtnik (franku)]

Yes, it's working now :-)

I am not familiar with the mutliplayer things, so i guess it's ok that the mutliplayer scenarios shows "Suggested teams" but one could not setup a team (e.g. Smugglers). Nevertheless disturbing.
This applies also to the map "Trident of fire" which is not a multiplayer scenario.

As said, i think this is ok, just wanted to mention here.

From what i can see all other things are fine:-)

review: Approve (testing)

[GunChleoc (gunchleoc)]

Great that it was an easy fix for once :)

Regarding the suggested teams, that's a feature that I've had in my head for years now. Once we have finished overhauling all the screens, I will get to work. Also, once the overhaul is finished, I will make some options for the win conditions configurable, like length of game and fog of war. This will all have to wait for Build 21 though.

[kaputtnik (franku)]

Thanks for implementing this. I think this is a great enhancement to the UI :-)

[SirVer (sirver)]

Code lgtm. That was a lot of work, thanks for doing this!

I tested it and have a few comments:
- The team UI is rather clever - maybe too clever, we'll see what the users say.
- I found one issue: If I change myself to be dual-control with another player, I can change their team and tribe (expected). If I now change away again from dual-control, I can still change their team and tribe in my UI (but others are not updated, so I think only the button needs to be disabled)

Please see the other code reviews for the base branches for some more comments.

Great feature! Thanks.

review: Approve (testing,compile,code-review)

[GunChleoc (gunchleoc)]

Thanks for the code review and the testing!

I have kicked out the new cast_(un)signed macros and have created 2 bug reports for the remaining issues.

@bunnybot merge

=== added file 'data/images/players/no_team.png'
Binary files data/images/players/no_team.png	1970-01-01 00:00:00 +0000 and data/images/players/no_team.png	2017-09-02 13:55:49 +0000 differ
=== added file 'data/images/players/team.png'
Binary files data/images/players/team.png	1970-01-01 00:00:00 +0000 and data/images/players/team.png	2017-09-02 13:55:49 +0000 differ
=== added file 'data/images/players/team_pc.png'
Binary files data/images/players/team_pc.png	1970-01-01 00:00:00 +0000 and data/images/players/team_pc.png	2017-09-02 13:55:49 +0000 differ
=== modified file 'src/ai/computer_player.cc'
--- src/ai/computer_player.cc	2017-01-25 18:55:59 +0000
+++ src/ai/computer_player.cc	2017-09-02 13:55:49 +0000
@@ -36,12 +36,13 @@
 	}
 
 	struct EmptyAIImpl : Implementation {
-		EmptyAIImpl() {
-			name = "empty";
-			/** TRANSLATORS: This is the name of an AI used in the game setup screens */
-			descname = _("No AI");
-			icon_filename = "images/ai/ai_empty.png";
-			type = Implementation::Type::kEmpty;
+		EmptyAIImpl()
+		   : Implementation(
+		        "empty",
+		        /** TRANSLATORS: This is the name of an AI used in the game setup screens */
+		        _("No AI"),
+		        "images/ai/ai_empty.png",
+		        Implementation::Type::kEmpty) {
 		}
 		ComputerPlayer* instantiate(Widelands::Game& g,
 		                            Widelands::PlayerNumber const pid) const override {

=== modified file 'src/ai/computer_player.h'
--- src/ai/computer_player.h	2017-04-22 08:02:21 +0000
+++ src/ai/computer_player.h	2017-09-02 13:55:49 +0000
@@ -23,9 +23,15 @@
 #include <string>
 #include <vector>
 
+#include <boost/algorithm/string/predicate.hpp>
+
 #include "base/macros.h"
 #include "logic/widelands.h"
 
+// We need to use a string prefix in the game setup screens to identify the AIs, so we make sure
+// that the AI names don't contain the separator that's used to parse the strings there.
+#define AI_NAME_SEPARATOR "|"
+
 namespace Widelands {
 class Game;
 }  // namespace Widelands
@@ -61,6 +67,17 @@
 		std::string descname;
 		std::string icon_filename;
 		Type type;
+		explicit Implementation(std::string init_name,
+		                        std::string init_descname,
+		                        std::string init_icon_filename,
+		                        Type init_type)
+		   : name(init_name),
+		     descname(init_descname),
+		     icon_filename(init_icon_filename),
+		     type(init_type) {
+			assert(!boost::contains(name, AI_NAME_SEPARATOR));
+		}
+
 		virtual ~Implementation() {
 		}
 		virtual ComputerPlayer* instantiate(Widelands::Game&, Widelands::PlayerNumber) const = 0;

=== modified file 'src/ai/defaultai.h'
--- src/ai/defaultai.h	2017-08-14 18:31:43 +0000
+++ src/ai/defaultai.h	2017-09-02 13:55:49 +0000
@@ -91,12 +91,13 @@
 
 	/// Implementation for Strong
 	struct NormalImpl : public ComputerPlayer::Implementation {
-		NormalImpl() {
-			name = "normal";
-			/** TRANSLATORS: This is the name of an AI used in the game setup screens */
-			descname = _("Normal AI");
-			icon_filename = "images/ai/ai_normal.png";
-			type = Implementation::Type::kDefault;
+		NormalImpl()
+		   : Implementation(
+		        "normal",
+		        /** TRANSLATORS: This is the name of an AI used in the game setup screens */
+		        _("Normal AI"),
+		        "images/ai/ai_normal.png",
+		        Implementation::Type::kDefault) {
 		}
 		ComputerPlayer* instantiate(Widelands::Game& game,
 		                            Widelands::PlayerNumber const p) const override {
@@ -105,12 +106,13 @@
 	};
 
 	struct WeakImpl : public ComputerPlayer::Implementation {
-		WeakImpl() {
-			name = "weak";
-			/** TRANSLATORS: This is the name of an AI used in the game setup screens */
-			descname = _("Weak AI");
-			icon_filename = "images/ai/ai_weak.png";
-			type = Implementation::Type::kDefault;
+		WeakImpl()
+		   : Implementation(
+		        "weak",
+		        /** TRANSLATORS: This is the name of an AI used in the game setup screens */
+		        _("Weak AI"),
+		        "images/ai/ai_weak.png",
+		        Implementation::Type::kDefault) {
 		}
 		ComputerPlayer* instantiate(Widelands::Game& game,
 		                            Widelands::PlayerNumber const p) const override {
@@ -119,12 +121,13 @@
 	};
 
 	struct VeryWeakImpl : public ComputerPlayer::Implementation {
-		VeryWeakImpl() {
-			name = "very_weak";
-			/** TRANSLATORS: This is the name of an AI used in the game setup screens */
-			descname = _("Very Weak AI");
-			icon_filename = "images/ai/ai_very_weak.png";
-			type = Implementation::Type::kDefault;
+		VeryWeakImpl()
+		   : Implementation(
+		        "very_weak",
+		        /** TRANSLATORS: This is the name of an AI used in the game setup screens */
+		        _("Very Weak AI"),
+		        "images/ai/ai_very_weak.png",
+		        Implementation::Type::kDefault) {
 		}
 		ComputerPlayer* instantiate(Widelands::Game& game,
 		                            Widelands::PlayerNumber const p) const override {

=== modified file 'src/graphic/playercolor.h'
--- src/graphic/playercolor.h	2017-04-28 06:47:01 +0000
+++ src/graphic/playercolor.h	2017-09-02 13:55:49 +0000
@@ -49,6 +49,19 @@
    RGBColor(144, 144, 144),  // light gray
 };
 
+// Hard coded team colors
+const RGBColor kTeamColors[kMaxPlayers / 2 + 1] = {
+	RGBColor(100, 100, 100),  // No team
+   RGBColor(2, 2, 198),      // blue
+   RGBColor(255, 41, 0),     // red
+   RGBColor(255, 232, 0),    // yellow
+   RGBColor(59, 223, 3),     // green
+   RGBColor(57, 57, 57),     // black/dark gray
+   RGBColor(255, 172, 0),    // orange
+   RGBColor(215, 0, 218),    // purple
+   RGBColor(255, 255, 255),  // white
+};
+
 /// Looks for a player color mask image, and if it finds one,
 /// returns the image with added playercolor. If no player color
 /// image file is found, gets the image from 'image_filename'

=== modified file 'src/logic/CMakeLists.txt'
--- src/logic/CMakeLists.txt	2017-07-11 19:38:04 +0000
+++ src/logic/CMakeLists.txt	2017-09-02 13:55:49 +0000
@@ -13,6 +13,7 @@
     io_filesystem
     logic
     logic_constants
+    notifications
     scripting_lua_interface
     scripting_lua_table
 )

=== modified file 'src/logic/game.cc'
--- src/logic/game.cc	2017-08-20 06:43:11 +0000
+++ src/logic/game.cc	2017-09-02 13:55:49 +0000
@@ -269,10 +269,10 @@
 	for (uint32_t i = 0; i < settings.players.size(); ++i) {
 		const PlayerSettings& playersettings = settings.players[i];
 
-		if (playersettings.state == PlayerSettings::stateClosed ||
-		    playersettings.state == PlayerSettings::stateOpen)
+		if (playersettings.state == PlayerSettings::State::kClosed ||
+		    playersettings.state == PlayerSettings::State::kOpen)
 			continue;
-		else if (playersettings.state == PlayerSettings::stateShared) {
+		else if (playersettings.state == PlayerSettings::State::kShared) {
 			shared.push_back(playersettings);
 			shared_num.push_back(i + 1);
 			continue;

=== modified file 'src/logic/game_settings.cc'
--- src/logic/game_settings.cc	2017-06-05 07:48:28 +0000
+++ src/logic/game_settings.cc	2017-09-02 13:55:49 +0000
@@ -1,1 +1,38 @@
-// Dummy to make CMake happy
+/*
+ * Copyright (C) 2017 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#include "logic/game_settings.h"
+
+Widelands::PlayerNumber GameSettings::find_shared(PlayerSlot slot) const {
+	Widelands::PlayerNumber result = 1;
+	for (; result <= players.size(); ++result) {
+		if (PlayerSettings::can_be_shared(players.at(result - 1).state) && (result - 1) != slot) {
+			break;
+		}
+	}
+	return result;
+}
+
+bool GameSettings::is_shared_usable(PlayerSlot slot, Widelands::PlayerNumber shared) const {
+	return shared <= players.size() && (shared - 1) != slot;
+}
+
+bool GameSettings::uncloseable(PlayerSlot slot) const {
+	return (scenario && !players.at(slot).closeable) || savegame;
+}

=== modified file 'src/logic/game_settings.h'
--- src/logic/game_settings.h	2017-08-16 04:31:56 +0000
+++ src/logic/game_settings.h	2017-09-02 13:55:49 +0000
@@ -28,11 +28,22 @@
 #include "logic/map_objects/tribes/tribe_basic_info.h"
 #include "logic/player_end_result.h"
 #include "logic/widelands.h"
+#include "notifications/note_ids.h"
+#include "notifications/notifications.h"
 #include "scripting/lua_interface.h"
 #include "scripting/lua_table.h"
 
+// PlayerSlot 0 will give us Widelands::PlayerNumber 1 etc., so we rename it to avoid confusion.
+// TODO(GunChleoc): Rename all uint8_t to PlayerSlot or Widelands::PlayerNumber
+using PlayerSlot = Widelands::PlayerNumber;
+
 struct PlayerSettings {
-	enum State { stateOpen, stateHuman, stateComputer, stateClosed, stateShared };
+	enum class State { kOpen, kHuman, kComputer, kClosed, kShared };
+
+	/// Returns whether the given state allows sharing a slot at all
+	static bool can_be_shared(PlayerSettings::State state) {
+		return state != PlayerSettings::State::kClosed && state != PlayerSettings::State::kShared;
+	}
 
 	State state;
 	uint8_t initialization_index;
@@ -71,6 +82,28 @@
 	             // not
 };
 
+/// The gamehost/gameclient are sending those to notify about status changes, which are then picked
+/// up by the UI.
+struct NoteGameSettings {
+	CAN_BE_SENT_AS_NOTE(NoteId::GameSettings)
+
+	enum class Action {
+		kUser,    // A client has picked a different player slot / become an observer
+		kPlayer,  // A player slot has changed its status (type, tribe etc.)
+		kMap      // A new map/savegame was selected
+	};
+
+	Action action;
+	PlayerSlot position;
+	uint8_t usernum;
+
+	explicit NoteGameSettings(Action init_action,
+	                          PlayerSlot init_position = std::numeric_limits<uint8_t>::max(),
+	                          uint8_t init_usernum = UserSettings::none())
+	   : action(init_action), position(init_position), usernum(init_usernum) {
+	}
+};
+
 /**
  * Holds all settings about a game that can be configured before the
  * game actually starts.
@@ -92,6 +125,14 @@
 		}
 	}
 
+	/// Find a player number that the slot could share in. Does not guarantee that a viable slot was
+	/// actually found.
+	Widelands::PlayerNumber find_shared(PlayerSlot slot) const;
+	/// Check if the player number returned by find_shared is usable
+	bool is_shared_usable(PlayerSlot slot, Widelands::PlayerNumber shared) const;
+	/// Savegame slots and certain scenario slots can't be closed
+	bool uncloseable(PlayerSlot slot) const;
+
 	/// Number of player position
 	int16_t playernum;
 	/// Number of users entry
@@ -154,7 +195,9 @@
 	                     bool savegame = false) = 0;
 	virtual void set_player_state(uint8_t number, PlayerSettings::State) = 0;
 	virtual void set_player_ai(uint8_t number, const std::string&, bool const random_ai = false) = 0;
-	virtual void next_player_state(uint8_t number) = 0;
+	// Multiplayer no longer toggles per button
+	virtual void next_player_state(uint8_t /* number */) {
+	}
 	virtual void
 	set_player_tribe(uint8_t number, const std::string&, bool const random_tribe = false) = 0;
 	virtual void set_player_init(uint8_t number, uint8_t index) = 0;
@@ -163,10 +206,11 @@
 	virtual void set_player_number(uint8_t number) = 0;
 	virtual void set_player_team(uint8_t number, Widelands::TeamNumber team) = 0;
 	virtual void set_player_closeable(uint8_t number, bool closeable) = 0;
-	virtual void set_player_shared(uint8_t number, uint8_t shared) = 0;
+	virtual void set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared) = 0;
 	virtual void set_win_condition_script(const std::string& wc) = 0;
 	virtual std::string get_win_condition_script() = 0;
 
+	// For retrieving tips texts
 	struct NoTribe {};
 	const std::string& get_players_tribe() {
 		if (UserSettings::highest_playernum() < settings().playernum)

=== modified file 'src/logic/single_player_game_settings_provider.cc'
--- src/logic/single_player_game_settings_provider.cc	2017-08-09 19:25:04 +0000
+++ src/logic/single_player_game_settings_provider.cc	2017-09-02 13:55:49 +0000
@@ -81,14 +81,15 @@
 
 	while (oldplayers < maxplayers) {
 		PlayerSettings& player = s.players[oldplayers];
-		player.state = (oldplayers == 0) ? PlayerSettings::stateHuman : PlayerSettings::stateComputer;
+		player.state =
+		   (oldplayers == 0) ? PlayerSettings::State::kHuman : PlayerSettings::State::kComputer;
 		player.tribe = s.tribes.at(0).name;
 		player.random_tribe = false;
 		player.initialization_index = 0;
 		player.name = (boost::format(_("Player %u")) % (oldplayers + 1)).str();
 		player.team = 0;
 		// Set default computerplayer ai type
-		if (player.state == PlayerSettings::stateComputer) {
+		if (player.state == PlayerSettings::State::kComputer) {
 			const ComputerPlayer::ImplementationVector& impls = ComputerPlayer::get_implementations();
 			if (impls.size() > 1) {
 				player.ai = impls.at(0)->name;
@@ -107,8 +108,8 @@
 	if (number == s.playernum || number >= s.players.size())
 		return;
 
-	if (state == PlayerSettings::stateOpen)
-		state = PlayerSettings::stateComputer;
+	if (state == PlayerSettings::State::kOpen)
+		state = PlayerSettings::State::kComputer;
 
 	s.players[number].state = state;
 }
@@ -147,7 +148,7 @@
 		s.players[number].ai = (*it)->name;
 	}
 
-	s.players[number].state = PlayerSettings::stateComputer;
+	s.players[number].state = PlayerSettings::State::kComputer;
 }
 
 void SinglePlayerGameSettingsProvider::set_player_tribe(uint8_t const number,
@@ -199,7 +200,7 @@
 	// nothing to do
 }
 
-void SinglePlayerGameSettingsProvider::set_player_shared(uint8_t, uint8_t) {
+void SinglePlayerGameSettingsProvider::set_player_shared(PlayerSlot, Widelands::PlayerNumber) {
 	// nothing to do
 }
 
@@ -219,8 +220,8 @@
 		return;
 	PlayerSettings const position = settings().players.at(number);
 	PlayerSettings const player = settings().players.at(settings().playernum);
-	if (number < settings().players.size() && (position.state == PlayerSettings::stateOpen ||
-	                                           position.state == PlayerSettings::stateComputer)) {
+	if (number < settings().players.size() && (position.state == PlayerSettings::State::kOpen ||
+	                                           position.state == PlayerSettings::State::kComputer)) {
 		set_player(number, player);
 		set_player(settings().playernum, position);
 		s.playernum = number;

=== modified file 'src/logic/single_player_game_settings_provider.h'
--- src/logic/single_player_game_settings_provider.h	2017-02-10 14:12:36 +0000
+++ src/logic/single_player_game_settings_provider.h	2017-09-02 13:55:49 +0000
@@ -54,7 +54,7 @@
 	void set_player_init(uint8_t const number, uint8_t const index) override;
 	void set_player_team(uint8_t number, Widelands::TeamNumber team) override;
 	void set_player_closeable(uint8_t, bool) override;
-	void set_player_shared(uint8_t, uint8_t) override;
+	void set_player_shared(PlayerSlot, Widelands::PlayerNumber) override;
 	void set_player_name(uint8_t const number, const std::string& name) override;
 	void set_player(uint8_t const number, const PlayerSettings& ps) override;
 	void set_player_number(uint8_t const number) override;

=== modified file 'src/network/CMakeLists.txt'
--- src/network/CMakeLists.txt	2017-05-14 21:06:23 +0000
+++ src/network/CMakeLists.txt	2017-09-02 13:55:49 +0000
@@ -33,7 +33,6 @@
     build_info
     chat
     game_io
-    graphic_playercolor
     helper
     io_fileread
     io_filesystem

=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc	2017-08-09 17:55:34 +0000
+++ src/network/gameclient.cc	2017-09-02 13:55:49 +0000
@@ -356,25 +356,25 @@
 	//  client is not allowed to do this
 }
 
-void GameClient::set_player_shared(uint8_t number, uint8_t player) {
+void GameClient::set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared) {
 	if ((number != d->settings.playernum))
 		return;
 
 	SendPacket s;
 	s.unsigned_8(NETCMD_SETTING_CHANGESHARED);
 	s.unsigned_8(number);
-	s.unsigned_8(player);
+	s.unsigned_8(shared);
 	d->net->send(s);
 }
 
-void GameClient::set_player_init(uint8_t number, uint8_t) {
+void GameClient::set_player_init(uint8_t number, uint8_t initialization_index) {
 	if ((number != d->settings.playernum))
 		return;
 
-	// Host will decide what to change, therefore the init is not send, just the request to change
 	SendPacket s;
 	s.unsigned_8(NETCMD_SETTING_CHANGEINIT);
 	s.unsigned_8(number);
+	s.unsigned_8(initialization_index);
 	d->net->send(s);
 }
 
@@ -404,8 +404,8 @@
 		return;
 	// Same if the player is not selectable
 	if (number < d->settings.players.size() &&
-	    (d->settings.players.at(number).state == PlayerSettings::stateClosed ||
-	     d->settings.players.at(number).state == PlayerSettings::stateComputer))
+	    (d->settings.players.at(number).state == PlayerSettings::State::kClosed ||
+	     d->settings.players.at(number).state == PlayerSettings::State::kComputer))
 		return;
 
 	// Send request
@@ -459,6 +459,7 @@
 	player.random_ai = packet.unsigned_8() == 1;
 	player.team = packet.unsigned_8();
 	player.shared_in = packet.unsigned_8();
+	Notifications::publish(NoteGameSettings(NoteGameSettings::Action::kPlayer, number));
 }
 
 void GameClient::receive_one_user(uint32_t const number, StreamRead& packet) {
@@ -473,10 +474,13 @@
 	d->settings.users.at(number).name = packet.string();
 	d->settings.users.at(number).position = packet.signed_32();
 	d->settings.users.at(number).ready = packet.unsigned_8() == 1;
+
 	if (static_cast<int32_t>(number) == d->settings.usernum) {
 		d->localplayername = d->settings.users.at(number).name;
 		d->settings.playernum = d->settings.users.at(number).position;
 	}
+	Notifications::publish(
+	   NoteGameSettings(NoteGameSettings::Action::kUser, d->settings.playernum, number));
 }
 
 void GameClient::send(const std::string& msg) {
@@ -567,6 +571,7 @@
 		// New map was set, so we clean up the buffer of a previously requested file
 		if (file_)
 			delete file_;
+		Notifications::publish(NoteGameSettings(NoteGameSettings::Action::kMap));
 		break;
 	}
 

=== modified file 'src/network/gameclient.h'
--- src/network/gameclient.h	2017-07-05 19:21:57 +0000
+++ src/network/gameclient.h	2017-09-02 13:55:49 +0000
@@ -84,13 +84,13 @@
 	virtual void set_player_tribe(uint8_t number,
 	                              const std::string& tribe,
 	                              bool const random_tribe = false) override;
-	void set_player_init(uint8_t number, uint8_t index) override;
+	void set_player_init(uint8_t number, uint8_t initialization_index) override;
 	void set_player_name(uint8_t number, const std::string& name) override;
 	void set_player(uint8_t number, const PlayerSettings& ps) override;
 	void set_player_number(uint8_t number) override;
 	void set_player_team(uint8_t number, Widelands::TeamNumber team) override;
 	void set_player_closeable(uint8_t number, bool closeable) override;
-	void set_player_shared(uint8_t number, uint8_t shared) override;
+	void set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared) override;
 	void set_win_condition_script(const std::string&) override;
 	std::string get_win_condition_script() override;
 

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2017-08-19 23:24:28 +0000
+++ src/network/gamehost.cc	2017-09-02 13:55:49 +0000
@@ -81,13 +81,16 @@
 		return true;
 	}
 	bool can_change_player_state(uint8_t const number) override {
+		if (number >= settings().players.size()) {
+			return false;
+		}
 		if (settings().savegame)
-			return settings().players.at(number).state != PlayerSettings::stateClosed;
+			return settings().players.at(number).state != PlayerSettings::State::kClosed;
 		else if (settings().scenario)
-			return ((settings().players.at(number).state == PlayerSettings::stateOpen ||
-			         settings().players.at(number).state == PlayerSettings::stateHuman) &&
+			return ((settings().players.at(number).state == PlayerSettings::State::kOpen ||
+			         settings().players.at(number).state == PlayerSettings::State::kHuman) &&
 			        settings().players.at(number).closeable) ||
-			       settings().players.at(number).state == PlayerSettings::stateClosed;
+			       settings().players.at(number).state == PlayerSettings::State::kClosed;
 		return true;
 	}
 	bool can_change_player_tribe(uint8_t const number) override {
@@ -105,7 +108,7 @@
 			return false;
 		if (number == settings().playernum)
 			return true;
-		return settings().players.at(number).state == PlayerSettings::stateComputer;
+		return settings().players.at(number).state == PlayerSettings::State::kComputer;
 	}
 
 	bool can_launch() override {
@@ -124,83 +127,6 @@
 
 		host_->set_player_state(number, state);
 	}
-	void next_player_state(uint8_t const number) override {
-		if (number > settings().players.size())
-			return;
-
-		PlayerSettings::State newstate = PlayerSettings::stateClosed;
-		switch (host_->settings().players.at(number).state) {
-		case PlayerSettings::stateClosed:
-			// In savegames : closed players can not be changed.
-			assert(!host_->settings().savegame);
-			newstate = PlayerSettings::stateOpen;
-			break;
-		case PlayerSettings::stateOpen:
-		case PlayerSettings::stateHuman:
-			if (host_->settings().scenario) {
-				assert(host_->settings().players.at(number).closeable);
-				newstate = PlayerSettings::stateClosed;
-				break;
-			}  // else fall through
-			FALLS_THROUGH;
-		case PlayerSettings::stateComputer: {
-			const ComputerPlayer::ImplementationVector& impls = ComputerPlayer::get_implementations();
-			ComputerPlayer::ImplementationVector::const_iterator it = impls.begin();
-			if (host_->settings().players.at(number).ai.empty()) {
-				set_player_ai(number, (*it)->name);
-				newstate = PlayerSettings::stateComputer;
-				break;
-			}
-			do {
-				++it;
-				if ((*(it - 1))->name == host_->settings().players.at(number).ai)
-					break;
-			} while (it != impls.end());
-			if (settings().players.at(number).random_ai) {
-				set_player_ai(number, std::string());
-				set_player_name(number, std::string());
-				// Do not share a player in savegames or scenarios
-				if (host_->settings().scenario || host_->settings().savegame)
-					newstate = PlayerSettings::stateOpen;
-				else {
-					uint8_t shared = 0;
-					for (; shared < settings().players.size(); ++shared) {
-						if (settings().players.at(shared).state != PlayerSettings::stateClosed &&
-						    settings().players.at(shared).state != PlayerSettings::stateShared)
-							break;
-					}
-					if (shared < settings().players.size()) {
-						newstate = PlayerSettings::stateShared;
-						set_player_shared(number, shared + 1);
-					} else
-						newstate = PlayerSettings::stateClosed;
-				}
-			} else if (it == impls.end()) {
-				do {
-					uint8_t random = (std::rand() % impls.size());  // Choose a random AI
-					it = impls.begin() + random;
-				} while ((*it)->type == ComputerPlayer::Implementation::Type::kEmpty);
-				set_player_ai(number, (*it)->name, true);
-				newstate = PlayerSettings::stateComputer;
-				break;
-			} else {
-				set_player_ai(number, (*it)->name);
-				newstate = PlayerSettings::stateComputer;
-			}
-			break;
-		}
-		case PlayerSettings::stateShared: {
-			// Do not close a player in savegames or scenarios
-			if (host_->settings().scenario || host_->settings().savegame)
-				newstate = PlayerSettings::stateOpen;
-			else
-				newstate = PlayerSettings::stateClosed;
-			break;
-		}
-		}
-
-		host_->set_player_state(number, newstate, true);
-	}
 
 	void
 	set_player_tribe(uint8_t number, const std::string& tribe, bool const random_tribe) override {
@@ -208,9 +134,10 @@
 			return;
 
 		if (number == settings().playernum ||
-		    settings().players.at(number).state == PlayerSettings::stateComputer ||
-		    settings().players.at(number).state == PlayerSettings::stateShared ||
-		    settings().players.at(number).state == PlayerSettings::stateOpen)  // For savegame loading
+		    settings().players.at(number).state == PlayerSettings::State::kComputer ||
+		    settings().players.at(number).state == PlayerSettings::State::kShared ||
+		    settings().players.at(number).state ==
+		       PlayerSettings::State::kOpen)  // For savegame loading
 			host_->set_player_tribe(number, tribe, random_tribe);
 	}
 
@@ -219,7 +146,7 @@
 			return;
 
 		if (number == settings().playernum ||
-		    settings().players.at(number).state == PlayerSettings::stateComputer)
+		    settings().players.at(number).state == PlayerSettings::State::kComputer)
 			host_->set_player_team(number, team);
 	}
 
@@ -229,7 +156,7 @@
 		host_->set_player_closeable(number, closeable);
 	}
 
-	void set_player_shared(uint8_t number, uint8_t shared) override {
+	void set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared) override {
 		if (number >= host_->settings().players.size())
 			return;
 		host_->set_player_shared(number, shared);
@@ -1019,9 +946,9 @@
 	// but not all should be closed!
 	bool one_not_closed = false;
 	for (PlayerSettings& setting : d->settings.players) {
-		if (setting.state != PlayerSettings::stateClosed)
+		if (setting.state != PlayerSettings::State::kClosed)
 			one_not_closed = true;
-		if (setting.state == PlayerSettings::stateOpen)
+		if (setting.state == PlayerSettings::State::kOpen)
 			return false;
 	}
 	return one_not_closed;
@@ -1071,7 +998,7 @@
 
 	while (oldplayers < maxplayers) {
 		PlayerSettings& player = d->settings.players.at(oldplayers);
-		player.state = PlayerSettings::stateOpen;
+		player.state = PlayerSettings::State::kOpen;
 		player.name = "";
 		player.tribe = d->settings.tribes.at(0).name;
 		player.random_tribe = false;
@@ -1146,9 +1073,7 @@
 	if (player.state == state)
 		return;
 
-	SendPacket s;
-
-	if (player.state == PlayerSettings::stateHuman) {
+	if (player.state == PlayerSettings::State::kHuman) {
 		//  0 is host and has no client
 		if (d->settings.users.at(0).position == number) {
 			d->settings.users.at(0).position = UserSettings::none();
@@ -1168,13 +1093,6 @@
 						break;
 					}
 				}
-
-				//  broadcast change
-				s.unsigned_8(NETCMD_SETTING_USER);
-				s.unsigned_32(i);
-				write_setting_user(s, i);
-				broadcast(s);
-
 				break;
 			}
 		}
@@ -1182,15 +1100,53 @@
 
 	player.state = state;
 
-	if (player.state == PlayerSettings::stateComputer)
+	// Make sure that shared slots have a player number to share in
+	if (player.state == PlayerSettings::State::kShared) {
+		const PlayerSlot shared = d->settings.find_shared(number);
+		if (d->settings.is_shared_usable(number, shared)) {
+			set_player_shared(number, shared);
+		} else {
+			player.state = PlayerSettings::State::kClosed;
+		}
+	}
+
+	// Update shared positions for other players
+	for (size_t i = 0; i < d->settings.players.size(); ++i) {
+		if (i == number) {
+			// Don't set own state
+			continue;
+		}
+		if (d->settings.players.at(i).state == PlayerSettings::State::kShared) {
+			const PlayerSlot shared = d->settings.find_shared(i);
+			if (d->settings.is_shared_usable(i, shared)) {
+				set_player_shared(i, shared);
+			} else {
+				set_player_state(i, PlayerSettings::State::kClosed, host);
+			}
+		}
+	}
+
+	// Make sure that slots that are not closeable stay open
+	if (player.state == PlayerSettings::State::kClosed && d->settings.uncloseable(number)) {
+		player.state = PlayerSettings::State::kOpen;
+	}
+
+	if (player.state == PlayerSettings::State::kComputer)
 		player.name = get_computer_player_name(number);
 
-	// Broadcast change
+	// 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);
+
+	// Let clients know whether their slot has changed
+	s.reset();
+	s.unsigned_8(NETCMD_SETTING_ALLUSERS);
+	write_setting_all_users(s);
+	broadcast(s);
 }
 
 void GameHost::set_player_tribe(uint8_t const number,
@@ -1312,7 +1268,7 @@
 	// uses it.
 }
 
-void GameHost::set_player_shared(uint8_t number, uint8_t shared) {
+void GameHost::set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared) {
 	if (number >= d->settings.players.size())
 		return;
 
@@ -1322,8 +1278,8 @@
 		return;
 
 	PlayerSettings& sharedplr = d->settings.players.at(shared - 1);
-	assert(sharedplr.state != PlayerSettings::stateClosed &&
-	       sharedplr.state != PlayerSettings::stateShared);
+	assert(PlayerSettings::can_be_shared(sharedplr.state));
+	assert(d->settings.is_shared_usable(number, shared));
 
 	player.shared_in = shared;
 	player.tribe = sharedplr.tribe;
@@ -1367,8 +1323,8 @@
 
 void GameHost::switch_to_player(uint32_t user, uint8_t number) {
 	if (number < d->settings.players.size() &&
-	    (d->settings.players.at(number).state != PlayerSettings::stateOpen &&
-	     d->settings.players.at(number).state != PlayerSettings::stateHuman))
+	    (d->settings.players.at(number).state != PlayerSettings::State::kOpen &&
+	     d->settings.players.at(number).state != PlayerSettings::State::kHuman))
 		return;
 
 	uint32_t old = d->settings.users.at(user).position;
@@ -1383,14 +1339,14 @@
 		temp2 = temp2.erase(op.name.find(temp), temp.size());
 		set_player_name(old, temp2);
 		if (temp2.empty())
-			set_player_state(old, PlayerSettings::stateOpen);
+			set_player_state(old, PlayerSettings::State::kOpen);
 	}
 
 	if (number < d->settings.players.size()) {
 		// Add clients name to new player slot
 		PlayerSettings& op = d->settings.players.at(number);
-		if (op.state == PlayerSettings::stateOpen) {
-			set_player_state(number, PlayerSettings::stateHuman);
+		if (op.state == PlayerSettings::State::kOpen) {
+			set_player_state(number, PlayerSettings::State::kHuman);
 			set_player_name(number, " " + name + " ");
 		} else
 			set_player_name(number, op.name + " " + name + " ");
@@ -1477,6 +1433,7 @@
 	packet.string(d->settings.mapfilename);
 	packet.unsigned_8(d->settings.savegame ? 1 : 0);
 	packet.unsigned_8(d->settings.scenario ? 1 : 0);
+	Notifications::publish(NoteGameSettings(NoteGameSettings::Action::kMap));
 }
 
 void GameHost::write_setting_player(SendPacket& packet, uint8_t const number) {
@@ -1490,6 +1447,7 @@
 	packet.unsigned_8(player.random_ai ? 1 : 0);
 	packet.unsigned_8(player.team);
 	packet.unsigned_8(player.shared_in);
+	Notifications::publish(NoteGameSettings(NoteGameSettings::Action::kPlayer, number));
 }
 
 void GameHost::write_setting_all_players(SendPacket& packet) {
@@ -1502,6 +1460,8 @@
 	packet.string(d->settings.users.at(number).name);
 	packet.signed_32(d->settings.users.at(number).position);
 	packet.unsigned_8(d->settings.users.at(number).ready ? 1 : 0);
+	Notifications::publish(NoteGameSettings(
+	   NoteGameSettings::Action::kUser, d->settings.users.at(number).position, number));
 }
 
 void GameHost::write_setting_all_users(SendPacket& packet) {
@@ -1673,7 +1633,7 @@
 
 	// Check if there is an unoccupied player left and if, assign.
 	for (uint8_t i = 0; i < d->settings.players.size(); ++i)
-		if (d->settings.players.at(i).state == PlayerSettings::stateOpen) {
+		if (d->settings.players.at(i).state == PlayerSettings::State::kOpen) {
 			switch_to_player(client.usernum, i);
 			break;
 		}
@@ -2096,10 +2056,13 @@
 
 	case NETCMD_SETTING_CHANGEINIT:
 		if (!d->game) {
+			// TODO(GunChleoc): For some nebulous reason, we don't receive the num that the client is
+			// sending when a player changes slot. So, keeping the access to the client off for now.
+			// Would be nice to have though.
 			uint8_t num = r.unsigned_8();
 			if (num != client.playernum)
 				throw DisconnectException("NO_ACCESS_TO_PLAYER");
-			d->npsb.toggle_init(num);
+			set_player_init(num, r.unsigned_8());
 		}
 		break;
 
@@ -2267,7 +2230,7 @@
 		}
 	}
 
-	set_player_state(number, PlayerSettings::stateOpen);
+	set_player_state(number, PlayerSettings::State::kOpen);
 	if (d->game)
 		init_computer_player(number + 1);
 }

=== modified file 'src/network/gamehost.h'
--- src/network/gamehost.h	2017-07-02 21:09:23 +0000
+++ src/network/gamehost.h	2017-09-02 13:55:49 +0000
@@ -76,7 +76,7 @@
 	void set_player_number(uint8_t number);
 	void set_player_team(uint8_t number, Widelands::TeamNumber team);
 	void set_player_closeable(uint8_t number, bool closeable);
-	void set_player_shared(uint8_t number, uint8_t shared);
+	void set_player_shared(PlayerSlot number, Widelands::PlayerNumber shared);
 	void switch_to_player(uint32_t user, uint8_t number);
 	void set_win_condition_script(const std::string& wc);
 

=== modified file 'src/network/network_player_settings_backend.cc'
--- src/network/network_player_settings_backend.cc	2017-02-14 10:18:15 +0000
+++ src/network/network_player_settings_backend.cc	2017-09-02 13:55:49 +0000
@@ -19,134 +19,69 @@
 
 #include "network/network_player_settings_backend.h"
 
-#include "base/i18n.h"
-#include "base/log.h"
-#include "base/wexception.h"
-#include "logic/game_settings.h"
-#include "logic/map_objects/tribes/tribe_descr.h"
-#include "logic/player.h"
-
-/// Toggle through the types
-void NetworkPlayerSettingsBackend::toggle_type(uint8_t id) {
-	if (id >= s->settings().players.size())
-		return;
-
-	s->next_player_state(id);
-}
-
-void NetworkPlayerSettingsBackend::set_tribe(uint8_t id, const std::string& tribename) {
+#include "ai/computer_player.h"
+
+void NetworkPlayerSettingsBackend::set_player_state(PlayerSlot id, PlayerSettings::State state) {
+	if (id >= s->settings().players.size()) {
+		return;
+	}
+	s->set_player_state(id, state);
+}
+
+void NetworkPlayerSettingsBackend::set_player_ai(PlayerSlot id,
+                                                 const std::string& name,
+                                                 bool random_ai) {
+	if (id >= s->settings().players.size()) {
+		return;
+	}
+	if (random_ai) {
+		const ComputerPlayer::ImplementationVector& impls = ComputerPlayer::get_implementations();
+		ComputerPlayer::ImplementationVector::const_iterator it = impls.begin();
+		if (impls.size() > 1) {
+			do {
+				size_t random = (std::rand() % impls.size());  // Choose a random AI
+				it = impls.begin() + random;
+			} while ((*it)->type == ComputerPlayer::Implementation::Type::kEmpty);
+		}
+		s->set_player_ai(id, (*it)->name, random_ai);
+	} else {
+		s->set_player_ai(id, name, random_ai);
+	}
+}
+
+void NetworkPlayerSettingsBackend::set_player_tribe(PlayerSlot id, const std::string& tribename) {
 	const GameSettings& settings = s->settings();
-
-	if (id >= settings.players.size() || tribename.empty())
+	if (id >= settings.players.size() || tribename.empty()) {
 		return;
-
-	if (settings.players.at(id).state != PlayerSettings::stateShared) {
+	}
+	if (settings.players.at(id).state != PlayerSettings::State::kShared) {
 		s->set_player_tribe(id, tribename, tribename == "random");
 	}
 }
 
 /// Set the shared in player for the given id
-void NetworkPlayerSettingsBackend::set_shared_in(uint8_t id, uint8_t shared_in) {
-	const GameSettings& settings = s->settings();
-	if (id > settings.players.size() || shared_in > settings.players.size())
-		return;
-	if (settings.players.at(id).state == PlayerSettings::stateShared) {
-		s->set_player_shared(id, shared_in);
-	}
-}
-
-/// Toggle through shared in players
-void NetworkPlayerSettingsBackend::toggle_shared_in(uint8_t id) {
-	const GameSettings& settings = s->settings();
-
-	if (id >= settings.players.size() ||
-	    settings.players.at(id).state != PlayerSettings::stateShared)
-		return;
-
-	uint8_t sharedplr = settings.players.at(id).shared_in;
-	for (; sharedplr < settings.players.size(); ++sharedplr) {
-		if (settings.players.at(sharedplr).state != PlayerSettings::stateClosed &&
-		    settings.players.at(sharedplr).state != PlayerSettings::stateShared)
-			break;
-	}
-	if (sharedplr < settings.players.size()) {
-		// We have already found the next player
-		set_shared_in(id, sharedplr + 1);
-		return;
-	}
-	sharedplr = 0;
-	for (; sharedplr < settings.players.at(id).shared_in; ++sharedplr) {
-		if (settings.players.at(sharedplr).state != PlayerSettings::stateClosed &&
-		    settings.players.at(sharedplr).state != PlayerSettings::stateShared)
-			break;
-	}
-	if (sharedplr < settings.players.at(id).shared_in) {
-		// We have found the next player
-		set_shared_in(id, sharedplr + 1);
-		return;
-	} else {
-		// No fitting player found
-		return toggle_type(id);
-	}
-}
-
-/// Toggle through the initializations
-void NetworkPlayerSettingsBackend::toggle_init(uint8_t id) {
-	const GameSettings& settings = s->settings();
-
-	if (id >= settings.players.size())
-		return;
-
-	const PlayerSettings& player = settings.players[id];
-	for (const TribeBasicInfo& temp_tribeinfo : settings.tribes) {
-		if (temp_tribeinfo.name == player.tribe) {
-			return s->set_player_init(
-			   id, (player.initialization_index + 1) % temp_tribeinfo.initializations.size());
-		}
-	}
-	NEVER_HERE();
-}
-
-/// Toggle through the teams
-void NetworkPlayerSettingsBackend::toggle_team(uint8_t id) {
-	const GameSettings& settings = s->settings();
-
-	if (id >= settings.players.size())
-		return;
-
-	Widelands::TeamNumber currentteam = settings.players.at(id).team;
-	Widelands::TeamNumber maxteam = settings.players.size() / 2;
-	Widelands::TeamNumber newteam;
-
-	if (currentteam >= maxteam)
-		newteam = 0;
-	else
-		newteam = currentteam + 1;
-
-	s->set_player_team(id, newteam);
-}
-
-/// Check if all settings for the player are still valid
-void NetworkPlayerSettingsBackend::refresh(uint8_t id) {
-	const GameSettings& settings = s->settings();
-
-	if (id >= settings.players.size())
-		return;
-
-	const PlayerSettings& player = settings.players[id];
-
-	if (player.state == PlayerSettings::stateShared) {
-		// ensure that the shared_in player is able to use this starting position
-		if (player.shared_in > settings.players.size())
-			toggle_shared_in(id);
-		if (settings.players.at(player.shared_in - 1).state == PlayerSettings::stateClosed ||
-		    settings.players.at(player.shared_in - 1).state == PlayerSettings::stateShared)
-			toggle_shared_in(id);
-
-		if (shared_in_tribe[id] != settings.players.at(player.shared_in - 1).tribe) {
-			s->set_player_tribe(id, settings.players.at(player.shared_in - 1).tribe,
-			                    settings.players.at(player.shared_in - 1).random_tribe);
-			shared_in_tribe[id] = settings.players.at(id).tribe;
-		}
-	}
+void NetworkPlayerSettingsBackend::set_player_shared(PlayerSlot id,
+                                                     Widelands::PlayerNumber shared) {
+	const GameSettings& settings = s->settings();
+	if (id >= settings.players.size() || shared > settings.players.size())
+		return;
+	if (settings.players.at(id).state == PlayerSettings::State::kShared) {
+		s->set_player_shared(id, shared);
+	}
+}
+
+/// Sets the initialization for the player slot (Headquarters, Fortified Village etc.)
+void NetworkPlayerSettingsBackend::set_player_init(PlayerSlot id, uint8_t initialization_index) {
+	if (id >= s->settings().players.size()) {
+		return;
+	}
+	s->set_player_init(id, initialization_index);
+}
+
+/// Sets the team for the player slot
+void NetworkPlayerSettingsBackend::set_player_team(PlayerSlot id, Widelands::TeamNumber team) {
+	if (id >= s->settings().players.size()) {
+		return;
+	}
+	s->set_player_team(id, team);
 }

=== modified file 'src/network/network_player_settings_backend.h'
--- src/network/network_player_settings_backend.h	2017-08-16 10:14:29 +0000
+++ src/network/network_player_settings_backend.h	2017-09-02 13:55:49 +0000
@@ -20,33 +20,22 @@
 #ifndef WL_NETWORK_NETWORK_PLAYER_SETTINGS_BACKEND_H
 #define WL_NETWORK_NETWORK_PLAYER_SETTINGS_BACKEND_H
 
-#include "graphic/playercolor.h"
 #include "logic/game_settings.h"
+#include "logic/widelands.h"
 
 struct NetworkPlayerSettingsBackend {
 
 	explicit NetworkPlayerSettingsBackend(GameSettingsProvider* const settings) : s(settings) {
-		for (uint8_t i = 0; i < kMaxPlayers; ++i)
-			shared_in_tribe[i] = std::string();
-	}
-
-	void toggle_type(uint8_t id);
-	void set_shared_in(uint8_t id, uint8_t shared_in);
-	void set_tribe(uint8_t id, const std::string& tribename);
-	void set_block_tribe_selection(bool blocked) {
-		tribe_selection_blocked = blocked;
-	}
-
-	void toggle_init(uint8_t id);
-	void toggle_team(uint8_t id);
-	void refresh(uint8_t id);
+	}
+
+	void set_player_state(PlayerSlot id, PlayerSettings::State state);
+	void set_player_ai(PlayerSlot id, const std::string& name, bool random_ai);
+	void set_player_shared(PlayerSlot id, Widelands::PlayerNumber shared);
+	void set_player_tribe(PlayerSlot id, const std::string& tribename);
+	void set_player_init(PlayerSlot id, uint8_t initialization_index);
+	void set_player_team(PlayerSlot id, Widelands::TeamNumber team);
 
 	GameSettingsProvider* const s;
-	std::string shared_in_tribe[kMaxPlayers];
-	bool tribe_selection_blocked = false;
-
-private:
-	void toggle_shared_in(uint8_t id);
 };
 
 #endif  // end of include guard: WL_NETWORK_NETWORK_PLAYER_SETTINGS_BACKEND_H

=== modified file 'src/network/network_protocol.h'
--- src/network/network_protocol.h	2017-05-06 11:00:19 +0000
+++ src/network/network_protocol.h	2017-09-02 13:55:49 +0000
@@ -395,9 +395,9 @@
     * \li unsigned_8: new shared player
     *
     * \note The client must not assume that the host will accept this
-    * request. Change of team number only becomes effective when/if the host
+    * request. Change of the initialization only becomes effective when/if the host
     * replies with a \ref NETCMD_SETTING_PLAYER or
-    *  \ref NETCMD_SETTING_ALLPLAYERS indicating the changed team.
+    *  \ref NETCMD_SETTING_ALLPLAYERS indicating the changed initialization.
     */
 	NETCMD_SETTING_CHANGESHARED = 27,
 
@@ -406,6 +406,7 @@
     * client wants to change a player's initialisation.
     *
     * \li unsigned_8: number of the player
+    * \li unsigned_8: index of the initialization
     *
     * \note The client must not assume that the host will accept this
     * request. Change of team number only becomes effective when/if the host

=== modified file 'src/notifications/note_ids.h'
--- src/notifications/note_ids.h	2017-08-30 07:45:52 +0000
+++ src/notifications/note_ids.h	2017-09-02 13:55:49 +0000
@@ -39,7 +39,9 @@
 	Economy,
 	GraphicResolutionChanged,
 	NoteExpeditionCanceled,
-	Sound
+	Sound,
+	Dropdown,
+	GameSettings
 };
 
 #endif  // end of include guard: WL_NOTIFICATIONS_NOTE_IDS_H

=== modified file 'src/ui_basic/dropdown.cc'
--- src/ui_basic/dropdown.cc	2017-06-02 08:14:40 +0000
+++ src/ui_basic/dropdown.cc	2017-09-02 13:55:49 +0000
@@ -43,6 +43,8 @@
 
 namespace UI {
 
+int BaseDropdown::next_id_ = 0;
+
 BaseDropdown::BaseDropdown(UI::Panel* parent,
                            int32_t x,
                            int32_t y,
@@ -56,11 +58,14 @@
    : UI::Panel(parent,
                x,
                y,
-               type == DropdownType::kTextual ? w : button_dimension,
+               type == DropdownType::kPictorial ? button_dimension : w,
                // Height only to fit the button, so we can use this in Box layout.
                base_height(button_dimension)),
+     id_(next_id_++),
      max_list_height_(h - 2 * get_h()),
      list_width_(w),
+     list_offset_x_(0),
+     list_offset_y_(0),
      button_dimension_(button_dimension),
      mouse_tolerance_(50),
      button_box_(this, 0, 0, UI::Box::Horizontal, w, h),
@@ -79,13 +84,23 @@
                      "dropdown_label",
                      0,
                      0,
-                     type == DropdownType::kTextual ? w - button_dimension : button_dimension,
+                     type == DropdownType::kTextual ?
+                        w - button_dimension :
+                        type == DropdownType::kTextualNarrow ? w : button_dimension,
                      get_h(),
                      background,
                      label),
      label_(label),
      type_(type),
      is_enabled_(true) {
+
+	// Close whenever another dropdown is opened
+	subscriber_ = Notifications::subscribe<NoteDropdown>([this](const NoteDropdown& note) {
+		if (id_ != note.id) {
+			close();
+		}
+	});
+
 	assert(max_list_height_ > 0);
 	// Hook into highest parent that we can get so that we can drop down outside the panel.
 	// Positioning breaks down with TabPanels, so we exclude them.
@@ -125,26 +140,42 @@
 
 void BaseDropdown::layout() {
 	const int base_h = base_height(button_dimension_);
-	const int w = type_ == DropdownType::kTextual ? get_w() : button_dimension_;
+	const int w = type_ == DropdownType::kPictorial ? button_dimension_ : get_w();
 	button_box_.set_size(w, base_h);
 	display_button_.set_desired_size(
 	   type_ == DropdownType::kTextual ? w - button_dimension_ : w, base_h);
 	int new_list_height =
 	   std::min(static_cast<int>(list_->size()) * list_->get_lineheight(), max_list_height_);
-	list_->set_size(type_ == DropdownType::kTextual ? w : list_width_, new_list_height);
+	list_->set_size(type_ != DropdownType::kPictorial ? w : list_width_, new_list_height);
 	set_desired_size(w, base_h);
 
 	// Update list position. The list is hooked into the highest parent that we can get so that we
 	// can drop down outside the panel. Positioning breaks down with TabPanels, so we exclude them.
 	UI::Panel* parent = get_parent();
-	int new_list_y = get_y() + get_h() + parent->get_y();
+	int new_list_y = get_y() + parent->get_y();
 	int new_list_x = get_x() + parent->get_x();
 	while (parent->get_parent() && !is_a(UI::TabPanel, parent->get_parent())) {
 		parent = parent->get_parent();
 		new_list_y += parent->get_y();
 		new_list_x += parent->get_x();
 	}
-	list_->set_pos(Vector2i(new_list_x, new_list_y));
+
+	// Drop up instead of down if it doesn't fit
+	if (new_list_y + list_->get_h() > g_gr->get_yres()) {
+		list_offset_y_ = -list_->get_h();
+	} else {
+		list_offset_y_ = display_button_.get_h();
+	}
+
+	// Right align instead of left align if it doesn't fit
+	if (new_list_x + list_->get_w() > g_gr->get_xres()) {
+		list_offset_x_ = display_button_.get_w() - list_->get_w();
+		if (push_button_ != nullptr) {
+			list_offset_x_ += push_button_->get_w();
+		}
+	}
+
+	list_->set_pos(Vector2i(new_list_x + list_offset_x_, new_list_y + list_offset_y_));
 }
 
 void BaseDropdown::add(const std::string& name,
@@ -178,16 +209,29 @@
 
 void BaseDropdown::set_label(const std::string& text) {
 	label_ = text;
-	if (type_ == DropdownType::kTextual) {
+	if (type_ != DropdownType::kPictorial) {
 		display_button_.set_title(label_);
 	}
 }
 
+void BaseDropdown::set_image(const Image* image) {
+	display_button_.set_pic(image);
+}
+
 void BaseDropdown::set_tooltip(const std::string& text) {
 	tooltip_ = text;
 	display_button_.set_tooltip(tooltip_);
 }
 
+void BaseDropdown::set_errored(const std::string& error_message) {
+	set_tooltip((boost::format(_("%1%: %2%")) % _("Error") % error_message).str());
+	if (type_ != DropdownType::kPictorial) {
+		set_label(_("Error"));
+	} else {
+		set_image(g_gr->images().get("images/ui_basic/different.png"));
+	}
+}
+
 void BaseDropdown::set_enabled(bool on) {
 	is_enabled_ = on;
 	set_can_focus(on);
@@ -213,6 +257,7 @@
 }
 
 void BaseDropdown::clear() {
+	close();
 	list_->clear();
 	current_selection_ = list_->selection_index();
 	list_->set_size(list_->get_w(), 0);
@@ -239,7 +284,7 @@
 	                            /** TRANSLATORS: Selection in Dropdown menus. */
 	                            pgettext("dropdown", "Not Selected");
 
-	if (type_ == DropdownType::kTextual) {
+	if (type_ != DropdownType::kPictorial) {
 		if (label_.empty()) {
 			display_button_.set_title(name);
 		} else {
@@ -268,19 +313,29 @@
 		return;
 	}
 	list_->set_visible(!list_->is_visible());
+	if (type_ != DropdownType::kTextual) {
+		display_button_.set_perm_pressed(list_->is_visible());
+	}
 	if (list_->is_visible()) {
 		list_->move_to_top();
 		focus();
+		Notifications::publish(NoteDropdown(id_));
 	}
 	// Make sure that the list covers and deactivates the elements below it
 	set_layout_toplevel(list_->is_visible());
 }
 
+void BaseDropdown::close() {
+	if (is_expanded()) {
+		toggle_list();
+	}
+}
+
 bool BaseDropdown::is_mouse_away() const {
-	return (get_mouse_position().x + mouse_tolerance_) < 0 ||
-	       get_mouse_position().x > (list_->get_w() + mouse_tolerance_) ||
-	       (get_mouse_position().y + mouse_tolerance_ / 2) < 0 ||
-	       get_mouse_position().y > (get_h() + list_->get_h() + mouse_tolerance_);
+	return (get_mouse_position().x + mouse_tolerance_) < list_offset_x_ ||
+	       get_mouse_position().x > (list_offset_x_ + list_->get_w() + mouse_tolerance_) ||
+	       (get_mouse_position().y + mouse_tolerance_) < list_offset_y_ ||
+	       get_mouse_position().y > (list_offset_y_ + get_h() + list_->get_h() + mouse_tolerance_);
 }
 
 bool BaseDropdown::handle_key(bool down, SDL_Keysym code) {

=== modified file 'src/ui_basic/dropdown.h'
--- src/ui_basic/dropdown.h	2017-04-21 09:56:42 +0000
+++ src/ui_basic/dropdown.h	2017-09-02 13:55:49 +0000
@@ -27,14 +27,26 @@
 
 #include "graphic/graphic.h"
 #include "graphic/image.h"
+#include "notifications/note_ids.h"
+#include "notifications/notifications.h"
 #include "ui_basic/box.h"
 #include "ui_basic/button.h"
 #include "ui_basic/listselect.h"
 #include "ui_basic/panel.h"
 
 namespace UI {
-
-enum class DropdownType { kTextual, kPictorial };
+// We use this to make sure that only 1 dropdown is open at the same time.
+struct NoteDropdown {
+	CAN_BE_SENT_AS_NOTE(NoteId::Dropdown)
+
+	int id;
+
+	explicit NoteDropdown(int init_id) : id(init_id) {
+	}
+};
+
+/// The narrow textual dropdown omits the extra push button
+enum class DropdownType { kTextual, kTextualNarrow, kPictorial };
 
 /// Implementation for a dropdown menu that lets the user select a value.
 class BaseDropdown : public Panel {
@@ -63,6 +75,7 @@
 	~BaseDropdown();
 
 public:
+	/// An entry was selected
 	boost::signals2::signal<void()> selected;
 
 	/// \return true if an element has been selected from the list
@@ -72,9 +85,15 @@
 	/// and displayed on the display button.
 	void set_label(const std::string& text);
 
+	/// Sets the image for the display button (for pictorial dropdowns).
+	void set_image(const Image* image);
+
 	/// Sets the tooltip for the display button.
 	void set_tooltip(const std::string& text);
 
+	/// Displays an error message on the button instead of the current selection.
+	void set_errored(const std::string& error_message);
+
 	/// Enables/disables the dropdown selection.
 	void set_enabled(bool on);
 
@@ -142,12 +161,22 @@
 	void set_value();
 	/// Toggles the dropdown list on and off.
 	void toggle_list();
+	/// Toggle the list closed if the dropdown is currently expanded.
+	void close();
 
 	/// Returns true if the mouse pointer left the vicinity of the dropdown.
 	bool is_mouse_away() const;
 
+	/// Give each dropdown a unique ID
+	static int next_id_;
+	const int id_;
+	std::unique_ptr<Notifications::Subscriber<NoteDropdown>> subscriber_;
+
+	// Dimensions
 	int max_list_height_;
 	int list_width_;
+	int list_offset_x_;
+	int list_offset_y_;
 	int button_dimension_;
 	const int mouse_tolerance_;  // Allow mouse outside the panel a bit before autocollapse
 	UI::Box button_box_;

=== modified file 'src/ui_basic/listselect.cc'
--- src/ui_basic/listselect.cc	2017-06-15 16:34:58 +0000
+++ src/ui_basic/listselect.cc	2017-09-02 13:55:49 +0000
@@ -359,9 +359,9 @@
 	if (selection_mode_ == ListselectLayout::kDropdown) {
 		RGBAColor black(0, 0, 0, 255);
 		//  top edge
-		dst.brighten_rect(Recti(0, 0, get_w(), 2), BUTTON_EDGE_BRIGHT_FACTOR / 4);
+		dst.brighten_rect(Recti(0, 0, get_w(), 2), BUTTON_EDGE_BRIGHT_FACTOR);
 		//  left edge
-		dst.brighten_rect(Recti(0, 0, 2, get_h()), BUTTON_EDGE_BRIGHT_FACTOR);
+		dst.brighten_rect(Recti(0, 2, 2, get_h()), BUTTON_EDGE_BRIGHT_FACTOR);
 		//  bottom edge
 		dst.fill_rect(Recti(2, get_h() - 2, get_eff_w() - 2, 1), black);
 		dst.fill_rect(Recti(1, get_h() - 1, get_eff_w() - 1, 1), black);

=== modified file 'src/ui_fsmenu/launch_game.cc'
--- src/ui_fsmenu/launch_game.cc	2017-05-18 06:52:04 +0000
+++ src/ui_fsmenu/launch_game.cc	2017-09-02 13:55:49 +0000
@@ -167,7 +167,7 @@
 					                            t->get_string("description"));
 				}
 			} catch (LuaTableKeyError& e) {
-				log("LaunchSPG: Error loading win condition: %s %s\n", win_condition_script.c_str(),
+				log("Launch Game: Error loading win condition: %s %s\n", win_condition_script.c_str(),
 				    e.what());
 			}
 		}
@@ -177,9 +177,8 @@
 		                    "could not be loaded.")) %
 		    settings_->settings().mapfilename)
 		      .str();
-		win_condition_dropdown_.set_label(_("Error"));
-		win_condition_dropdown_.set_tooltip(error_message);
-		log("LaunchSPG: Exception: %s %s\n", error_message.c_str(), e.what());
+		win_condition_dropdown_.set_errored(error_message);
+		log("Launch Game: Exception: %s %s\n", error_message.c_str(), e.what());
 	}
 }
 
@@ -202,8 +201,8 @@
 			}
 		}
 	} catch (LuaTableKeyError& e) {
-		log(
-		   "LaunchSPG: Error loading win condition: %s %s\n", win_condition_script.c_str(), e.what());
+		log("Launch Game: Error loading win condition: %s %s\n", win_condition_script.c_str(),
+		    e.what());
 	}
 	if (!is_usable) {
 		t.reset(nullptr);

=== modified file 'src/ui_fsmenu/launch_mpg.cc'
--- src/ui_fsmenu/launch_mpg.cc	2017-05-18 06:52:04 +0000
+++ src/ui_fsmenu/launch_mpg.cc	2017-09-02 13:55:49 +0000
@@ -181,7 +181,7 @@
 	map_info_.set_text(_("The host has not yet selected a map or saved game."));
 
 	mpsg_ = new MultiPlayerSetupGroup(
-	   this, get_w() / 50, get_h() / 8, get_w() * 57 / 80, get_h(), settings, butw_, buth_);
+	   this, get_w() / 50, change_map_or_save_.get_y(), get_w() * 57 / 80, get_h(), settings, buth_);
 
 	// If we are the host, open the map or save selection menu at startup
 	if (settings_->settings().usernum == 0 && settings_->settings().mapname.empty()) {
@@ -355,6 +355,8 @@
  * buttons and text.
  */
 void FullscreenMenuLaunchMPG::refresh() {
+	// TODO(GunChleoc): Investigate what we can handle with NoteGameSettings. Maybe we can get rid of
+	// refresh() and thus think().
 	const GameSettings& settings = settings_->settings();
 
 	if (settings.mapfilename != filename_proof_) {
@@ -425,8 +427,6 @@
 		}
 		win_condition_dropdown_.set_enabled(false);
 	}
-	// Update the multi player setup group
-	mpsg_->refresh();
 }
 
 /**
@@ -443,16 +443,21 @@
 	map.set_filename(settings.mapfilename);
 	ml->preload_map(true);
 	Widelands::PlayerNumber const nrplayers = map.get_nrplayers();
+	if (settings.players.size() != nrplayers) {
+		// Due to asynchronous notifications, the client can crash when an update is missing and the
+		// number of players is wrong.
+		return;
+	}
 	for (uint8_t i = 0; i < nrplayers; ++i) {
 		settings_->set_player_tribe(i, map.get_scenario_player_tribe(i + 1));
 		settings_->set_player_closeable(i, map.get_scenario_player_closeable(i + 1));
 		std::string ai(map.get_scenario_player_ai(i + 1));
 		if (!ai.empty()) {
-			settings_->set_player_state(i, PlayerSettings::stateComputer);
+			settings_->set_player_state(i, PlayerSettings::State::kComputer);
 			settings_->set_player_ai(i, ai);
-		} else if (settings.players.at(i).state != PlayerSettings::stateHuman &&
-		           settings.players.at(i).state != PlayerSettings::stateOpen) {
-			settings_->set_player_state(i, PlayerSettings::stateOpen);
+		} else if (settings.players.at(i).state != PlayerSettings::State::kHuman &&
+		           settings.players.at(i).state != PlayerSettings::State::kOpen) {
+			settings_->set_player_state(i, PlayerSettings::State::kOpen);
 		}
 	}
 }
@@ -470,22 +475,25 @@
 	std::string player_save_tribe[kMaxPlayers];
 	std::string player_save_ai[kMaxPlayers];
 
-	uint8_t i = 1;
-	for (; i <= nr_players_; ++i) {
+	for (uint8_t i = 1; i <= nr_players_; ++i) {
+		Section* s = prof.get_section((boost::format("player_%u") % static_cast<unsigned int>(i)).str());
+		if (s == nullptr) {
+			// Due to asynchronous notifications, the client can crash on savegame change when number
+			// of players goes down. So, we abort if the section does not exist to prevent crashes.
+			return;
+		}
+		player_save_name[i - 1] = s->get_string("name");
+		player_save_tribe[i - 1] = s->get_string("tribe");
+		player_save_ai[i - 1] = s->get_string("ai");
+
 		infotext += "\n* ";
-		Section& s =
-		   prof.get_safe_section((boost::format("player_%u") % static_cast<unsigned int>(i)).str());
-		player_save_name[i - 1] = s.get_string("name");
-		player_save_tribe[i - 1] = s.get_string("tribe");
-		player_save_ai[i - 1] = s.get_string("ai");
-
 		infotext += (boost::format(_("Player %u")) % static_cast<unsigned int>(i)).str();
 		if (player_save_tribe[i - 1].empty()) {
 			std::string closed_string = (boost::format("<%s>") % _("closed")).str();
 			infotext += ":\n    ";
 			infotext += closed_string;
 			// Close the player
-			settings_->set_player_state(i - 1, PlayerSettings::stateClosed);
+			settings_->set_player_state(i - 1, PlayerSettings::State::kClosed);
 			continue;  // if tribe is empty, the player does not exist
 		}
 
@@ -495,10 +503,10 @@
 
 		if (player_save_ai[i - 1].empty()) {
 			// Assure that player is open
-			if (settings_->settings().players.at(i - 1).state != PlayerSettings::stateHuman)
-				settings_->set_player_state(i - 1, PlayerSettings::stateOpen);
+			if (settings_->settings().players.at(i - 1).state != PlayerSettings::State::kHuman)
+				settings_->set_player_state(i - 1, PlayerSettings::State::kOpen);
 		} else {
-			settings_->set_player_state(i - 1, PlayerSettings::stateComputer);
+			settings_->set_player_state(i - 1, PlayerSettings::State::kComputer);
 			settings_->set_player_ai(i - 1, player_save_ai[i - 1]);
 		}
 
@@ -563,7 +571,9 @@
 	            "\n";
 	infotext +=
 	   std::string("• ") +
-	   (boost::format(ngettext("%u Player", "%u Players", nr_players_)) % nr_players_).str() + "\n";
+	   (boost::format(ngettext("%u Player", "%u Players", nr_players_)) % static_cast<unsigned int>(nr_players_))
+	      .str() +
+	   "\n";
 	if (settings_->settings().scenario)
 		infotext += std::string("• ") + (boost::format(_("Scenario mode selected"))).str() + "\n";
 	infotext += "\n";

=== modified file 'src/ui_fsmenu/launch_mpg.h'
--- src/ui_fsmenu/launch_mpg.h	2017-02-10 14:12:36 +0000
+++ src/ui_fsmenu/launch_mpg.h	2017-09-02 13:55:49 +0000
@@ -23,6 +23,7 @@
 #include <memory>
 #include <string>
 
+#include "logic/game_settings.h"
 #include "ui_basic/button.h"
 #include "ui_basic/dropdown.h"
 #include "ui_basic/multilinetextarea.h"

=== modified file 'src/ui_fsmenu/launch_spg.cc'
--- src/ui_fsmenu/launch_spg.cc	2017-05-04 05:08:14 +0000
+++ src/ui_fsmenu/launch_spg.cc	2017-09-02 13:55:49 +0000
@@ -210,8 +210,8 @@
 	for (uint8_t i = 0; i < nr_players_; ++i) {
 		pos_[i]->set_visible(true);
 		const PlayerSettings& player = settings.players[i];
-		pos_[i]->set_enabled(!is_scenario_ && (player.state == PlayerSettings::stateOpen ||
-		                                       player.state == PlayerSettings::stateComputer));
+		pos_[i]->set_enabled(!is_scenario_ && (player.state == PlayerSettings::State::kOpen ||
+		                                       player.state == PlayerSettings::State::kComputer));
 	}
 	for (uint32_t i = nr_players_; i < kMaxPlayers; ++i)
 		pos_[i]->set_visible(false);
@@ -291,14 +291,14 @@
 	// Check if a still valid place is open.
 	for (uint8_t i = 0; i < newplayernumber; ++i) {
 		PlayerSettings position = settings.players.at(i);
-		if (position.state == PlayerSettings::stateOpen) {
+		if (position.state == PlayerSettings::State::kOpen) {
 			switch_to_position(i);
 			return;
 		}
 	}
 
 	// Kick player 1 and take the position
-	settings_->set_player_state(0, PlayerSettings::stateClosed);
-	settings_->set_player_state(0, PlayerSettings::stateOpen);
+	settings_->set_player_state(0, PlayerSettings::State::kClosed);
+	settings_->set_player_state(0, PlayerSettings::State::kOpen);
 	switch_to_position(0);
 }

=== modified file 'src/wui/multiplayersetupgroup.cc'
--- src/wui/multiplayersetupgroup.cc	2017-05-03 07:24:06 +0000
+++ src/wui/multiplayersetupgroup.cc	2017-09-02 13:55:49 +0000
@@ -19,8 +19,10 @@
 
 #include "wui/multiplayersetupgroup.h"
 
+#include <memory>
 #include <string>
 
+#include <boost/algorithm/string.hpp>
 #include <boost/format.hpp>
 #include <boost/lexical_cast.hpp>
 
@@ -36,419 +38,550 @@
 #include "logic/map_objects/tribes/tribe_descr.h"
 #include "logic/map_objects/tribes/tribes.h"
 #include "logic/player.h"
+#include "logic/widelands.h"
 #include "ui_basic/button.h"
-#include "ui_basic/checkbox.h"
 #include "ui_basic/dropdown.h"
-#include "ui_basic/icon.h"
+#include "ui_basic/mouse_constants.h"
 #include "ui_basic/scrollbar.h"
 #include "ui_basic/textarea.h"
 
+#define AI_NAME_PREFIX "ai" AI_NAME_SEPARATOR
+
+constexpr int kPadding = 4;
+
+/// Holds the info and dropdown menu for a connected client
 struct MultiPlayerClientGroup : public UI::Box {
 	MultiPlayerClientGroup(UI::Panel* const parent,
-	                       uint8_t id,
-	                       int32_t const /* x */,
-	                       int32_t const /* y */,
 	                       int32_t const w,
 	                       int32_t const h,
+	                       PlayerSlot id,
 	                       GameSettingsProvider* const settings)
-	   : UI::Box(parent, 0, 0, UI::Box::Horizontal, w, h),
-	     type_icon(nullptr),
-	     type(nullptr),
+	   : UI::Box(parent, 0, 0, UI::Box::Horizontal, w, h, kPadding),
+	     slot_dropdown_(this, 0, 0, h, 200, h, _("Role"), UI::DropdownType::kPictorial),
+	     // 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),
 	     id_(id),
-	     save_(-2) {
+	     slot_selection_locked_(false) {
 		set_size(w, h);
-		name = new UI::Textarea(this, 0, 0, w - h - UI::Scrollbar::kSize * 11 / 5, h);
-		add(name);
-		// Either Button if changeable OR text if not
-		if (id == settings->settings().usernum) {  // Our Client
-			type = new UI::Button(
-			   this, "client_type", 0, 0, h, h, g_gr->images().get("images/ui_basic/but1.png"), "");
-			type->sigclicked.connect(
-			   boost::bind(&MultiPlayerClientGroup::toggle_type, boost::ref(*this)));
-			add(type);
-		} else {  // just a shown client
-			type_icon = new UI::Icon(
-			   this, 0, 0, h, h, g_gr->images().get("images/wui/fieldaction/menu_tab_watch.png"));
-			add(type_icon);
-		}
-	}
-
-	/// Switch human players and spectator
-	void toggle_type() {
-		UserSettings us = s->settings().users.at(id_);
-		int16_t p = us.position;
-		if (p == UserSettings::none())
-			p = -1;
-
-		for (++p; p < static_cast<int16_t>(s->settings().players.size()); ++p) {
-			if (s->settings().players.at(p).state == PlayerSettings::stateHuman ||
-			    s->settings().players.at(p).state == PlayerSettings::stateOpen) {
-				s->set_player_number(p);
-				return;
-			}
-		}
-		s->set_player_number(UserSettings::none());
-	}
-
-	/// Care about visibility and current values
-	void refresh() {
-		UserSettings us = s->settings().users.at(id_);
-		if (us.position == UserSettings::not_connected()) {
-			name->set_text((boost::format("<%s>") % _("free")).str());
-			if (type)
-				type->set_visible(false);
-			else
-				type_icon->set_visible(false);
-		} else {
-			name->set_text(us.name);
-			if (save_ != us.position) {
-				const Image* position_image;
-				std::string temp_tooltip;
-				if (us.position < UserSettings::highest_playernum()) {
-					position_image =
-					   playercolor_image(us.position, "images/players/genstats_player.png");
-					temp_tooltip =
-					   (boost::format(_("Player %u")) % static_cast<unsigned int>(us.position + 1))
-					      .str();
-				} else {
-					position_image = g_gr->images().get("images/wui/fieldaction/menu_tab_watch.png");
-					temp_tooltip = _("Spectator");
-				}
-
-				// Either Button if changeable OR text if not
-				if (id_ == s->settings().usernum) {
-					type->set_pic(position_image);
-					type->set_tooltip(temp_tooltip);
-					type->set_visible(true);
-				} else {
-					type_icon->set_icon(position_image);
-					type_icon->set_tooltip(temp_tooltip);
-					type_icon->set_visible(true);
-				}
-				save_ = us.position;
-			}
-		}
-	}
-
-	UI::Textarea* name;
-	UI::Icon* type_icon;
-	UI::Button* type;
+
+		add(&slot_dropdown_);
+		add(&name, UI::Box::Resizing::kAlign, UI::Align::kCenter);
+
+		slot_dropdown_.set_disable_style(UI::ButtonDisableStyle::kFlat);
+		slot_dropdown_.selected.connect(
+		   boost::bind(&MultiPlayerClientGroup::set_slot, boost::ref(*this)));
+
+		update();
+		layout();
+
+		subscriber_ =
+		   Notifications::subscribe<NoteGameSettings>([this](const NoteGameSettings& note) {
+			   switch (note.action) {
+			   case NoteGameSettings::Action::kMap:
+				   /// In case the client gets kicked off its slot due to number of player slots in the
+				   /// map
+				   update();
+				   break;
+			   case NoteGameSettings::Action::kUser:
+				   /// Player slot might have been closed, bumping the client to observer status. Also,
+				   /// take note if another player changed their position.
+				   if (id_ == note.usernum || note.usernum == UserSettings::none()) {
+					   update();
+				   }
+				   break;
+			   case NoteGameSettings::Action::kPlayer:
+				   break;
+			   }
+			});
+	}
+
+	/// Update dropdown sizes
+	void layout() override {
+		UI::Box::layout();
+		slot_dropdown_.set_height(g_gr->get_yres() * 3 / 4);
+	}
+
+	/// 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();
+		if (id_ != settings.usernum) {
+			return;
+		}
+		slot_selection_locked_ = true;
+		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());
+			}
+		}
+		slot_selection_locked_ = false;
+	}
+
+	/// Rebuild the slot dropdown from the server settings. This will keep the host and client UIs in
+	/// sync.
+	void rebuild_slot_dropdown(const GameSettings& settings) {
+		if (slot_selection_locked_) {
+			return;
+		}
+		const UserSettings& user_setting = settings.users.at(id_);
+
+		slot_dropdown_.clear();
+		for (PlayerSlot slot = 0; slot < settings.players.size(); ++slot) {
+			if (settings.players.at(slot).state == PlayerSettings::State::kHuman ||
+			    settings.players.at(slot).state == PlayerSettings::State::kOpen) {
+				slot_dropdown_.add((boost::format(_("Player %u")) % static_cast<unsigned int>(slot + 1)).str(),
+				                   slot, playercolor_image(slot, "images/players/genstats_player.png"),
+				                   slot == user_setting.position);
+			}
+		}
+		slot_dropdown_.add(_("Spectator"), UserSettings::none(),
+		                   g_gr->images().get("images/wui/fieldaction/menu_tab_watch.png"),
+		                   user_setting.position == UserSettings::none());
+		slot_dropdown_.set_visible(true);
+		slot_dropdown_.set_enabled(id_ == settings.usernum);
+	}
+
+	/// Take care of visibility and current values
+	void update() {
+		const GameSettings& settings = s->settings();
+		const UserSettings& user_setting = settings.users.at(id_);
+
+		if (user_setting.position == UserSettings::not_connected()) {
+			set_visible(false);
+			return;
+		}
+
+		name.set_text(user_setting.name);
+		rebuild_slot_dropdown(settings);
+	}
+
+	UI::Dropdown<uintptr_t> slot_dropdown_;  /// Select the player slot.
+	UI::Textarea name;                       /// Client nick name
 	GameSettingsProvider* const s;
-	uint8_t const id_;
-	int16_t save_;  // saved position to check rewrite need.
+	uint8_t const id_;            /// User number
+	bool slot_selection_locked_;  // Ensure that dropdowns will close on selection.
+	std::unique_ptr<Notifications::Subscriber<NoteGameSettings>> subscriber_;
 };
 
+/// Holds the dropdown menus for a player slot
 struct MultiPlayerPlayerGroup : public UI::Box {
 	MultiPlayerPlayerGroup(UI::Panel* const parent,
-	                       uint8_t id,
-	                       int32_t const /* x */,
-	                       int32_t const /* y */,
 	                       int32_t const w,
 	                       int32_t const h,
+	                       PlayerSlot id,
 	                       GameSettingsProvider* const settings,
 	                       NetworkPlayerSettingsBackend* const npsb)
-	   : UI::Box(parent, 0, 0, UI::Box::Horizontal, w, h),
-	     player(nullptr),
-	     type(nullptr),
-	     init(nullptr),
+	   : UI::Box(parent, 0, 0, UI::Box::Horizontal, w, h, kPadding / 2),
 	     s(settings),
 	     n(npsb),
 	     id_(id),
+	     player(this,
+	            "player",
+	            0,
+	            0,
+	            h,
+	            h,
+	            g_gr->images().get("images/ui_basic/but1.png"),
+	            playercolor_image(id, "images/players/player_position_menu.png"),
+	            (boost::format(_("Player %u")) % static_cast<unsigned int>(id_ + 1)).str(),
+	            UI::Button::Style::kFlat),
+	     type_dropdown_(this, 0, 0, 50, 200, h, _("Type"), UI::DropdownType::kPictorial),
 	     tribes_dropdown_(this, 0, 0, 50, 200, h, _("Tribe"), UI::DropdownType::kPictorial),
-	     last_state_(PlayerSettings::stateClosed),
-	     last_player_amount_(0) {
+	     init_dropdown_(
+	        this, 0, 0, w - 4 * h - 3 * kPadding, 200, h, "", UI::DropdownType::kTextualNarrow),
+	     team_dropdown_(this, 0, 0, h, 200, h, _("Team"), UI::DropdownType::kPictorial),
+	     last_state_(PlayerSettings::State::kClosed),
+	     type_selection_locked_(false),
+	     tribe_selection_locked_(false),
+	     init_selection_locked_(false),
+	     team_selection_locked_(false) {
 		set_size(w, h);
-		tribes_dropdown_.set_visible(false);
-		tribes_dropdown_.set_enabled(false);
+
+		player.set_disable_style(UI::ButtonDisableStyle::kFlat);
+		player.set_enabled(false);
+
+		type_dropdown_.set_disable_style(UI::ButtonDisableStyle::kFlat);
+		tribes_dropdown_.set_disable_style(UI::ButtonDisableStyle::kFlat);
+		init_dropdown_.set_disable_style(UI::ButtonDisableStyle::kFlat);
+		team_dropdown_.set_disable_style(UI::ButtonDisableStyle::kFlat);
+
+		type_dropdown_.selected.connect(
+		   boost::bind(&MultiPlayerPlayerGroup::set_type, boost::ref(*this)));
 		tribes_dropdown_.selected.connect(
 		   boost::bind(&MultiPlayerPlayerGroup::set_tribe_or_shared_in, boost::ref(*this)));
+		init_dropdown_.selected.connect(
+		   boost::bind(&MultiPlayerPlayerGroup::set_init, boost::ref(*this)));
+		team_dropdown_.selected.connect(
+		   boost::bind(&MultiPlayerPlayerGroup::set_team, boost::ref(*this)));
 
-		const Image* player_image = playercolor_image(id, "images/players/player_position_menu.png");
-		assert(player_image);
-		player = new UI::Icon(this, 0, 0, h, h, player_image);
-		add(player);
-		type = new UI::Button(
-		   this, "player_type", 0, 0, h, h, g_gr->images().get("images/ui_basic/but1.png"), "");
-		type->sigclicked.connect(
-		   boost::bind(&MultiPlayerPlayerGroup::toggle_type, boost::ref(*this)));
-		add(type);
+		add_space(0);
+		add(&player);
+		add(&type_dropdown_);
 		add(&tribes_dropdown_);
-		init = new UI::Button(this, "player_init", 0, 0, w - 4 * h, h,
-		                      g_gr->images().get("images/ui_basic/but1.png"), "");
-		init->sigclicked.connect(
-		   boost::bind(&MultiPlayerPlayerGroup::toggle_init, boost::ref(*this)));
-		add(init);
-		team = new UI::Button(
-		   this, "player_team", 0, 0, h, h, g_gr->images().get("images/ui_basic/but1.png"), "");
-		team->sigclicked.connect(
-		   boost::bind(&MultiPlayerPlayerGroup::toggle_team, boost::ref(*this)));
-		add(team);
-	}
-
-	/// Toggle through the types
-	void toggle_type() {
-		n->toggle_type(id_);
+		add(&init_dropdown_);
+		add(&team_dropdown_);
+		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();
+				   }
+			   }
+			});
+
+		// Init dropdowns
+		update();
+		layout();
+	}
+
+	/// Update dropdown sizes
+	void layout() override {
+		const int max_height = g_gr->get_yres() * 3 / 4;
+		type_dropdown_.set_height(max_height);
+		tribes_dropdown_.set_height(max_height);
+		init_dropdown_.set_height(max_height);
+		team_dropdown_.set_height(max_height);
+		UI::Box::layout();
+	}
+
+	/// 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_)) {
+			return;
+		}
+		type_selection_locked_ = true;
+		if (type_dropdown_.has_selection()) {
+			const std::string& selected = type_dropdown_.get_selected();
+			PlayerSettings::State state = PlayerSettings::State::kComputer;
+			if (selected == "closed") {
+				state = PlayerSettings::State::kClosed;
+			} else if (selected == "open") {
+				state = PlayerSettings::State::kOpen;
+			} else if (selected == "shared_in") {
+				state = PlayerSettings::State::kShared;
+			} else {
+				if (selected == AI_NAME_PREFIX "random") {
+					n->set_player_ai(id_, "", true);
+				} else {
+					if (boost::starts_with(selected, AI_NAME_PREFIX)) {
+						std::vector<std::string> parts;
+						boost::split(parts, selected, boost::is_any_of(AI_NAME_SEPARATOR));
+						assert(parts.size() == 2);
+						n->set_player_ai(id_, parts[1], false);
+					} else {
+						throw wexception("Unknown player state: %s\n", selected.c_str());
+					}
+				}
+			}
+			n->set_player_state(id_, state);
+		}
+		type_selection_locked_ = false;
+	}
+
+	/// Rebuild the type dropdown from the server settings. This will keep the host and client UIs in
+	/// sync.
+	void rebuild_type_dropdown(const GameSettings& settings) {
+		if (type_selection_locked_) {
+			return;
+		}
+		type_dropdown_.clear();
+		// AIs
+		for (const auto* impl : ComputerPlayer::get_implementations()) {
+			type_dropdown_.add(_(impl->descname),
+			                   (boost::format(AI_NAME_PREFIX "%s") % impl->name).str(),
+			                   g_gr->images().get(impl->icon_filename), false, _(impl->descname));
+		}
+		/** TRANSLATORS: This is the name of an AI used in the game setup screens */
+		type_dropdown_.add(_("Random AI"), AI_NAME_PREFIX "random",
+		                   g_gr->images().get("images/ai/ai_random.png"), false, _("Random AI"));
+
+		// Slot state. Only add shared_in if there are viable slots
+		if (settings.is_shared_usable(id_, settings.find_shared(id_))) {
+			type_dropdown_.add(_("Shared in"), "shared_in",
+			                   g_gr->images().get("images/ui_fsmenu/shared_in.png"), false,
+			                   _("Shared in"));
+		}
+
+		// Do not close a player in savegames or scenarios
+		if (!settings.uncloseable(id_)) {
+			type_dropdown_.add(_("Closed"), "closed", g_gr->images().get("images/ui_basic/stop.png"),
+			                   false, _("Closed"));
+		}
+
+		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_));
+
+		// Now select the entry according to server settings
+		const PlayerSettings& player_setting = settings.players[id_];
+		if (player_setting.state == PlayerSettings::State::kHuman) {
+			type_dropdown_.set_image(g_gr->images().get("images/wui/stats/genstats_nrworkers.png"));
+			type_dropdown_.set_tooltip((boost::format(_("%1%: %2%")) % _("Type") % _("Human")).str());
+		} else if (player_setting.state == PlayerSettings::State::kClosed) {
+			type_dropdown_.select("closed");
+		} else if (player_setting.state == PlayerSettings::State::kOpen) {
+			type_dropdown_.select("open");
+		} else if (player_setting.state == PlayerSettings::State::kShared) {
+			type_dropdown_.select("shared_in");
+		} else {
+			if (player_setting.state == PlayerSettings::State::kComputer) {
+				if (player_setting.ai.empty()) {
+					type_dropdown_.set_errored(_("No AI"));
+				} else {
+					if (player_setting.random_ai) {
+						type_dropdown_.select(AI_NAME_PREFIX "random");
+					} else {
+						const ComputerPlayer::Implementation* impl =
+						   ComputerPlayer::get_implementation(player_setting.ai);
+						type_dropdown_.select((boost::format(AI_NAME_PREFIX "%s") % impl->name).str());
+					}
+				}
+			}
+		}
+	}
+
+	/// 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_);
 	}
 
 	/// This will update the game settings for the tribe or shared_in with the value
 	/// currently selected in the tribes dropdown.
 	void set_tribe_or_shared_in() {
-		n->set_block_tribe_selection(true);
-		tribes_dropdown_.set_disable_style(s->settings().players[id_].state ==
-		                                         PlayerSettings::stateShared ?
+		if (!has_tribe_access()) {
+			return;
+		}
+		const PlayerSettings& player_settings = s->settings().players[id_];
+		tribe_selection_locked_ = true;
+		tribes_dropdown_.set_disable_style(player_settings.state == PlayerSettings::State::kShared ?
 		                                      UI::ButtonDisableStyle::kPermpressed :
-		                                      UI::ButtonDisableStyle::kMonochrome);
+		                                      UI::ButtonDisableStyle::kFlat);
 		if (tribes_dropdown_.has_selection()) {
-			if (s->settings().players[id_].state == PlayerSettings::stateShared) {
-				n->set_shared_in(
+			if (player_settings.state == PlayerSettings::State::kShared) {
+				n->set_player_shared(
 				   id_, boost::lexical_cast<unsigned int>(tribes_dropdown_.get_selected()));
 			} else {
-				n->set_tribe(id_, tribes_dropdown_.get_selected());
-			}
-		}
-		n->set_block_tribe_selection(false);
-	}
-
-	/// Toggle through the initializations
-	void toggle_init() {
-		n->toggle_init(id_);
-	}
-
-	/// Toggle through the teams
-	void toggle_team() {
-		n->toggle_team(id_);
-	}
-
-	/// Helper function to cast shared_in for use in the dropdown.
-	const std::string shared_in_as_string(uint8_t shared_in) {
-		return boost::lexical_cast<std::string>(static_cast<unsigned int>(shared_in));
-	}
-
-	/// Update the tribes dropdown from the server settings if the server setting changed.
-	/// This will keep the host and client UIs in sync.
-	void update_tribes_dropdown(const PlayerSettings& player_setting) {
-		if (player_setting.state == PlayerSettings::stateClosed ||
-		    player_setting.state == PlayerSettings::stateOpen) {
+				n->set_player_tribe(id_, tribes_dropdown_.get_selected());
+			}
+		}
+		tribe_selection_locked_ = false;
+	}
+
+	/// Rebuild the tribes dropdown from the server settings. This will keep the host and client UIs
+	/// in sync.
+	void rebuild_tribes_dropdown(const GameSettings& settings) {
+		if (tribe_selection_locked_) {
+			return;
+		}
+		const PlayerSettings& player_setting = settings.players[id_];
+		tribes_dropdown_.clear();
+		if (player_setting.state == PlayerSettings::State::kShared) {
+			for (size_t i = 0; i < settings.players.size(); ++i) {
+				if (i != id_) {
+					// Do not add players that are also shared_in or closed.
+					const PlayerSettings& other_setting = settings.players[i];
+					if (!PlayerSettings::can_be_shared(other_setting.state)) {
+						continue;
+					}
+
+					const Image* player_image =
+					   playercolor_image(i, "images/players/player_position_menu.png");
+					assert(player_image);
+					const std::string player_name =
+					   /** TRANSLATORS: This is an option in multiplayer setup for sharing
+					      another player's starting position. */
+					   (boost::format(_("Shared in Player %u")) % static_cast<unsigned int>(i + 1)).str();
+					tribes_dropdown_.add(player_name,
+					                     boost::lexical_cast<std::string>(static_cast<unsigned int>(i + 1)),
+					                     player_image, (i + 1) == player_setting.shared_in, player_name);
+				}
+			}
+			tribes_dropdown_.set_enabled(tribes_dropdown_.size() > 1);
+		} else {
+			{
+				i18n::Textdomain td("tribes");
+				for (const TribeBasicInfo& tribeinfo : Widelands::get_all_tribeinfos()) {
+					tribes_dropdown_.add(_(tribeinfo.descname), tribeinfo.name,
+					                     g_gr->images().get(tribeinfo.icon), false, tribeinfo.tooltip);
+				}
+			}
+			tribes_dropdown_.add(pgettext("tribe", "Random"), "random",
+			                     g_gr->images().get("images/ui_fsmenu/random.png"), false,
+			                     _("The tribe will be selected at random"));
+			if (player_setting.random_tribe) {
+				tribes_dropdown_.select("random");
+			} else {
+				tribes_dropdown_.select(player_setting.tribe);
+			}
+		}
+		const bool has_access = has_tribe_access();
+		if (tribes_dropdown_.is_enabled() != has_access) {
+			tribes_dropdown_.set_enabled(has_access && tribes_dropdown_.size() > 1);
+		}
+		if (player_setting.state == PlayerSettings::State::kClosed ||
+		    player_setting.state == PlayerSettings::State::kOpen) {
 			return;
 		}
 		if (!tribes_dropdown_.is_visible()) {
 			tribes_dropdown_.set_visible(true);
 		}
-		if (!tribes_dropdown_.is_expanded() && !n->tribe_selection_blocked &&
-		    tribes_dropdown_.has_selection()) {
-			const std::string selected_tribe = tribes_dropdown_.get_selected();
-			if (player_setting.state == PlayerSettings::stateShared) {
-				const std::string shared_in = shared_in_as_string(player_setting.shared_in);
-				if (shared_in != selected_tribe) {
-					tribes_dropdown_.select(shared_in);
-				}
-			} else {
-				if (player_setting.random_tribe) {
-					if (selected_tribe != "random") {
-						tribes_dropdown_.select("random");
-					}
-				} else if (selected_tribe != player_setting.tribe) {
-					tribes_dropdown_.select(player_setting.tribe);
-				}
-			}
-		}
-	}
-
-	/// If the map was changed or the selection mode changed between shared_in and tribe, rebuild the
-	/// dropdown.
-	void rebuild_tribes_dropdown(const GameSettings& settings) {
-		const PlayerSettings& player_setting = settings.players[id_];
-
-		if (player_setting.state == PlayerSettings::stateClosed ||
-		    player_setting.state == PlayerSettings::stateOpen) {
-			return;
-		}
-
-		if (tribes_dropdown_.empty() || last_player_amount_ != settings.players.size() ||
-		    ((player_setting.state == PlayerSettings::stateShared ||
-		      last_state_ == PlayerSettings::stateShared) &&
-		     player_setting.state != last_state_)) {
-			tribes_dropdown_.clear();
-
-			// We need to see the playercolor if setting shared_in is disabled
-			tribes_dropdown_.set_disable_style(player_setting.state == PlayerSettings::stateShared ?
-			                                      UI::ButtonDisableStyle::kPermpressed :
-			                                      UI::ButtonDisableStyle::kMonochrome);
-
-			if (player_setting.state == PlayerSettings::stateShared) {
-				for (size_t i = 0; i < settings.players.size(); ++i) {
-					if (i != id_) {
-						// TODO(GunChleoc): Do not add players that are also shared_in.
-						const Image* player_image =
-						   playercolor_image(i, "images/players/player_position_menu.png");
-						assert(player_image);
-						const std::string player_name =
-						   /** TRANSLATORS: This is an option in multiplayer setup for sharing
-						      another player's starting position. */
-						   (boost::format(_("Shared in Player %u")) % static_cast<unsigned int>(i + 1))
-						      .str();
-						tribes_dropdown_.add(
-						   player_name, shared_in_as_string(i + 1), player_image, false, player_name);
-					}
-				}
-				int shared_in = 0;
-				while (shared_in == id_) {
-					++shared_in;
-				}
-				tribes_dropdown_.select(shared_in_as_string(shared_in + 1));
-				tribes_dropdown_.set_enabled(tribes_dropdown_.size() > 1);
-			} else {
-				{
-					i18n::Textdomain td("tribes");
-					for (const TribeBasicInfo& tribeinfo : Widelands::get_all_tribeinfos()) {
-						tribes_dropdown_.add(_(tribeinfo.descname), tribeinfo.name,
-						                     g_gr->images().get(tribeinfo.icon), false,
-						                     tribeinfo.tooltip);
-					}
-				}
-				tribes_dropdown_.add(pgettext("tribe", "Random"), "random",
-				                     g_gr->images().get("images/ui_fsmenu/random.png"), false,
-				                     _("The tribe will be selected at random"));
-				if (player_setting.random_tribe) {
-					tribes_dropdown_.select("random");
-				} else {
-					tribes_dropdown_.select(player_setting.tribe);
-				}
-			}
-		}
-		last_player_amount_ = settings.players.size();
+	}
+
+	/// 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_)) {
+			return;
+		}
+		init_selection_locked_ = true;
+		if (init_dropdown_.has_selection()) {
+			n->set_player_init(id_, init_dropdown_.get_selected());
+		}
+		init_selection_locked_ = false;
+	}
+
+	/// Rebuild the init dropdown from the server settings. This will keep the host and client UIs in
+	/// sync.
+	void rebuild_init_dropdown(const GameSettings& settings) {
+		if (init_selection_locked_) {
+			return;
+		}
+
+		init_dropdown_.clear();
+		const PlayerSettings& player_setting = settings.players[id_];
+		if (settings.scenario) {
+			init_dropdown_.set_label(_("Scenario"));
+		} else if (settings.savegame) {
+			/** Translators: This is a game type */
+			init_dropdown_.set_label(_("Saved Game"));
+		} else {
+			init_dropdown_.set_label("");
+			i18n::Textdomain td("tribes");  // for translated initialisation
+			const TribeBasicInfo tribeinfo = Widelands::get_tribeinfo(player_setting.tribe);
+			for (size_t i = 0; i < tribeinfo.initializations.size(); ++i) {
+				const TribeBasicInfo::Initialization& addme = tribeinfo.initializations[i];
+				init_dropdown_.add(_(addme.descname), i, nullptr,
+				                   i == player_setting.initialization_index, _(addme.tooltip));
+			}
+		}
+
+		init_dropdown_.set_visible(true);
+		init_dropdown_.set_enabled(s->can_change_player_init(id_));
+	}
+
+	/// This will update the team settings with the value currently selected in the teams dropdown.
+	void set_team() {
+		team_selection_locked_ = true;
+		if (team_dropdown_.has_selection()) {
+			n->set_player_team(id_, team_dropdown_.get_selected());
+		}
+		team_selection_locked_ = false;
+	}
+
+	/// Rebuild the team dropdown from the server settings. This will keep the host and client UIs in
+	/// sync.
+	void rebuild_team_dropdown(const GameSettings& settings) {
+		if (team_selection_locked_) {
+			return;
+		}
+		const PlayerSettings& player_setting = settings.players[id_];
+		if (player_setting.state == PlayerSettings::State::kShared) {
+			team_dropdown_.set_visible(false);
+			team_dropdown_.set_enabled(false);
+			return;
+		}
+
+		team_dropdown_.clear();
+		team_dropdown_.add(_("No Team"), 0, g_gr->images().get("images/players/no_team.png"));
+#ifndef NDEBUG
+		const size_t no_of_team_colors = sizeof(kTeamColors) / sizeof(kTeamColors[0]);
+#endif
+		for (Widelands::TeamNumber t = 1; t <= settings.players.size() / 2; ++t) {
+			assert(t < no_of_team_colors);
+			team_dropdown_.add((boost::format(_("Team %d")) % static_cast<unsigned int>(t)).str(), t,
+			                   playercolor_image(kTeamColors[t], "images/players/team.png"));
+		}
+		team_dropdown_.select(player_setting.team);
+		team_dropdown_.set_visible(true);
+		team_dropdown_.set_enabled(s->can_change_player_team(id_));
 	}
 
 	/// Refresh all user interfaces
-	void refresh() {
+	void update() {
 		const GameSettings& settings = s->settings();
-
 		if (id_ >= settings.players.size()) {
 			set_visible(false);
 			return;
 		}
 
-		n->refresh(id_);
-
+		const PlayerSettings& player_setting = settings.players[id_];
+		rebuild_type_dropdown(settings);
 		set_visible(true);
 
-		const PlayerSettings& player_setting = settings.players[id_];
-		bool typeaccess = s->can_change_player_state(id_);
-		bool tribeaccess = s->can_change_player_tribe(id_);
-		bool const initaccess = s->can_change_player_init(id_);
-		bool teamaccess = s->can_change_player_team(id_);
-		type->set_enabled(typeaccess);
-
-		rebuild_tribes_dropdown(settings);
-
-		if (player_setting.state == PlayerSettings::stateClosed) {
-			type->set_tooltip(_("Closed"));
-			type->set_pic(g_gr->images().get("images/ui_basic/stop.png"));
-			team->set_visible(false);
-			team->set_enabled(false);
-			tribes_dropdown_.set_visible(false);
-			tribes_dropdown_.set_enabled(false);
-			init->set_visible(false);
-			init->set_enabled(false);
-			return;
-		} else if (player_setting.state == PlayerSettings::stateOpen) {
-			type->set_tooltip(_("Open"));
-			type->set_pic(g_gr->images().get("images/ui_basic/continue.png"));
-			team->set_visible(false);
-			team->set_enabled(false);
-			tribes_dropdown_.set_visible(false);
-			tribes_dropdown_.set_enabled(false);
-			init->set_visible(false);
-			init->set_enabled(false);
-			return;
-		} else if (player_setting.state == PlayerSettings::stateShared) {
-			type->set_tooltip(_("Shared in"));
-			type->set_pic(g_gr->images().get("images/ui_fsmenu/shared_in.png"));
-
-			update_tribes_dropdown(player_setting);
-
-			if (tribes_dropdown_.is_enabled() != initaccess) {
-				tribes_dropdown_.set_enabled(initaccess && !n->tribe_selection_blocked &&
-				                             tribes_dropdown_.size() > 1);
-			}
-
-			team->set_visible(false);
-			team->set_enabled(false);
-
+		if (player_setting.state == PlayerSettings::State::kClosed ||
+		    player_setting.state == PlayerSettings::State::kOpen) {
+			team_dropdown_.set_visible(false);
+			team_dropdown_.set_enabled(false);
+			tribes_dropdown_.set_visible(false);
+			tribes_dropdown_.set_enabled(false);
+			init_dropdown_.set_visible(false);
+			init_dropdown_.set_enabled(false);
 		} else {
-			std::string title;
-			std::string pic = "images/";
-			if (player_setting.state == PlayerSettings::stateComputer) {
-				if (player_setting.ai.empty()) {
-					title = _("Computer");
-					pic += "novalue.png";
-				} else {
-					if (player_setting.random_ai) {
-						/** TRANSLATORS: This is the name of an AI used in the game setup screens */
-						title = _("Random AI");
-						pic += "ai/ai_random.png";
-					} else {
-						const ComputerPlayer::Implementation* impl =
-						   ComputerPlayer::get_implementation(player_setting.ai);
-						title = _(impl->descname);
-						pic = impl->icon_filename;
-					}
-				}
-			} else {  // PlayerSettings::stateHuman
-				title = _("Human");
-				pic += "wui/stats/genstats_nrworkers.png";
-			}
-			type->set_tooltip(title.c_str());
-			type->set_pic(g_gr->images().get(pic));
-
-			update_tribes_dropdown(player_setting);
-
-			if (tribes_dropdown_.is_enabled() != tribeaccess) {
-				tribes_dropdown_.set_enabled(tribeaccess && !n->tribe_selection_blocked);
-			}
-
-			if (player_setting.team) {
-				team->set_title(std::to_string(static_cast<unsigned int>(player_setting.team)));
-			} else {
-				team->set_title("--");
-			}
-			team->set_visible(true);
-			team->set_enabled(teamaccess);
-		}
-		init->set_enabled(initaccess);
-		init->set_visible(true);
-
-		if (settings.scenario)
-			init->set_title(_("Scenario"));
-		else if (settings.savegame)
-			/** Translators: This is a game type */
-			init->set_title(_("Saved Game"));
-		else {
-			i18n::Textdomain td("tribes");  // for translated initialisation
-			for (const TribeBasicInfo& tribeinfo : settings.tribes) {
-				if (tribeinfo.name == player_setting.tribe) {
-					init->set_title(
-					   _(tribeinfo.initializations.at(player_setting.initialization_index).descname));
-					init->set_tooltip(
-					   _(tribeinfo.initializations.at(player_setting.initialization_index).tooltip));
-					break;
-				}
-			}
-		}
-		last_state_ = player_setting.state;
+			rebuild_tribes_dropdown(settings);
+			rebuild_init_dropdown(settings);
+			rebuild_team_dropdown(settings);
+		}
+
+		// 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) {
+				if (slot != id_) {
+					n->set_player_state(slot, settings.players[slot].state);
+				}
+			}
+		}
 	}
 
-	UI::Icon* player;
-	UI::Button* type;
-	UI::Button* init;
-	UI::Button* team;
 	GameSettingsProvider* const s;
 	NetworkPlayerSettingsBackend* const n;
-	uint8_t const id_;
+	PlayerSlot const id_;
+
+	UI::Button player;
+	UI::Dropdown<std::string>
+	   type_dropdown_;  /// Select who owns the slot (human, AI, open, closed, shared-in).
 	UI::Dropdown<std::string> tribes_dropdown_;  /// Select the tribe or shared_in player.
-	PlayerSettings::State last_state_;           /// The dropdown needs updating if this changes
-	size_t last_player_amount_;                  /// The dropdown needs rebuilding if this changes
+	UI::Dropdown<uintptr_t>
+	   init_dropdown_;  /// Select the initialization (Headquarters, Fortified Village etc.)
+	UI::Dropdown<uintptr_t> team_dropdown_;  /// Select the team number
+	PlayerSettings::State
+	   last_state_;  /// The dropdowns for the other slots need updating if this changes
+	/// Lock rebuilding dropdowns so that they can close on selection
+	bool type_selection_locked_;
+	bool tribe_selection_locked_;
+	bool init_selection_locked_;
+	bool team_selection_locked_;
+
+	std::unique_ptr<Notifications::Subscriber<NoteGameSettings>> subscriber_;
 };
 
 MultiPlayerSetupGroup::MultiPlayerSetupGroup(UI::Panel* const parent,
@@ -457,69 +590,41 @@
                                              int32_t const w,
                                              int32_t const h,
                                              GameSettingsProvider* const settings,
-                                             uint32_t /* butw */,
                                              uint32_t buth)
-   : UI::Panel(parent, x, y, w, h),
+   : UI::Box(parent, x, y, UI::Box::Horizontal, w, h, 8 * kPadding),
      s(settings),
      npsb(new NetworkPlayerSettingsBackend(s)),
-     clientbox(this, 0, buth, UI::Box::Vertical, w / 3, h - buth),
-     playerbox(this, w * 6 / 15, buth, UI::Box::Vertical, w * 9 / 15, h - buth),
+     clientbox(this, 0, 0, UI::Box::Vertical),
+     playerbox(this, 0, 0, UI::Box::Vertical, w * 9 / 15, h, kPadding),
      buth_(buth) {
-	int small_font = UI_FONT_SIZE_SMALL * 3 / 4;
-
-	// Clientbox and labels
-	labels.push_back(new UI::Textarea(
-	   this, UI::Scrollbar::kSize * 6 / 5, buth / 3, w / 3 - buth - UI::Scrollbar::kSize * 2, buth));
-	labels.back()->set_text(_("Client name"));
-	labels.back()->set_fontsize(small_font);
-
-	labels.push_back(new UI::Textarea(
-	   this, w / 3 - buth - UI::Scrollbar::kSize * 6 / 5, buth / 3, buth * 2, buth));
-	labels.back()->set_text(_("Role"));
-	labels.back()->set_fontsize(small_font);
-
-	clientbox.set_size(w / 3, h - buth);
+	clientbox.set_size(w / 3, h);
 	clientbox.set_scrolling(true);
 
-	// Playerbox and labels
-	labels.push_back(new UI::Textarea(this, w * 6 / 15, buth / 3, buth, buth));
-	labels.back()->set_text(_("Start"));
-	labels.back()->set_fontsize(small_font);
-
-	labels.push_back(new UI::Textarea(this, w * 6 / 15 + buth, buth / 3 - 10, buth, buth));
-	labels.back()->set_text(_("Type"));
-	labels.back()->set_fontsize(small_font);
-
-	labels.push_back(new UI::Textarea(this, w * 6 / 15 + buth * 2, buth / 3, buth, buth));
-	labels.back()->set_text(_("Tribe"));
-	labels.back()->set_fontsize(small_font);
-
-	labels.push_back(new UI::Textarea(
-	   this, w * 6 / 15 + buth * 3, buth / 3, w * 9 / 15 - 4 * buth, buth, UI::Align::kCenter));
-	labels.back()->set_text(_("Initialization"));
-	labels.back()->set_fontsize(small_font);
-
-	labels.push_back(new UI::Textarea(this, w - buth, buth / 3, buth, buth, UI::Align::kRight));
-	labels.back()->set_text(_("Team"));
-	labels.back()->set_fontsize(small_font);
-
-	playerbox.set_size(w * 9 / 15, h - buth);
+	add(&clientbox, UI::Box::Resizing::kExpandBoth);
+	add(&playerbox);
+
+	// Playerbox
+	playerbox.set_size(w * 9 / 15, h);
+	playerbox.add_space(0);
 	multi_player_player_groups.resize(kMaxPlayers);
-	for (uint8_t i = 0; i < multi_player_player_groups.size(); ++i) {
+	for (PlayerSlot i = 0; i < multi_player_player_groups.size(); ++i) {
 		multi_player_player_groups.at(i) =
-		   new MultiPlayerPlayerGroup(&playerbox, i, 0, 0, playerbox.get_w(), buth, s, npsb.get());
+		   new MultiPlayerPlayerGroup(&playerbox, playerbox.get_w(), buth_, i, s, npsb.get());
 		playerbox.add(multi_player_player_groups.at(i));
 	}
-	refresh();
+	playerbox.add_space(0);
+
+	subscriber_ =
+	   Notifications::subscribe<NoteGameSettings>([this](const NoteGameSettings&) { update(); });
+	set_size(w, h);
+	update();
 }
 
 MultiPlayerSetupGroup::~MultiPlayerSetupGroup() {
 }
 
-/**
- * Update display and enabled buttons based on current settings.
- */
-void MultiPlayerSetupGroup::refresh() {
+/// Update which slots are available based on current settings.
+void MultiPlayerSetupGroup::update() {
 	const GameSettings& settings = s->settings();
 
 	// Update / initialize client groups
@@ -529,15 +634,29 @@
 	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, i, 0, 0, clientbox.get_w(), buth_, s);
-			clientbox.add(
-			   &*multi_player_client_groups.at(i), UI::Box::Resizing::kAlign, UI::Align::kCenter);
-		}
-		multi_player_client_groups.at(i)->refresh();
-	}
-
-	// Update player groups
-	for (uint32_t i = 0; i < kMaxPlayers; ++i) {
-		multi_player_player_groups.at(i)->refresh();
+			   new MultiPlayerClientGroup(&clientbox, clientbox.get_w(), buth_, i, s);
+			clientbox.add(multi_player_client_groups.at(i), UI::Box::Resizing::kFullSize);
+			multi_player_client_groups.at(i)->layout();
+		}
+		multi_player_client_groups.at(i)->set_visible(true);
+	}
+
+	// Keep track of which player slots are visible
+	for (PlayerSlot i = 0; i < multi_player_player_groups.size(); ++i) {
+		const bool should_be_visible = i < settings.players.size();
+		if (should_be_visible != multi_player_player_groups.at(i)->is_visible()) {
+			multi_player_player_groups.at(i)->set_visible(should_be_visible);
+		}
+	}
+}
+
+void MultiPlayerSetupGroup::draw(RenderTarget& dst) {
+	for (MultiPlayerPlayerGroup* player_group : multi_player_player_groups) {
+		if (player_group->is_visible()) {
+			dst.brighten_rect(
+			   Recti(playerbox.get_x(), playerbox.get_y() + player_group->get_y() - kPadding / 2,
+			         playerbox.get_w() + kPadding, player_group->get_h() + kPadding),
+			   -MOUSE_OVER_BRIGHT_FACTOR);
+		}
 	}
 }

=== modified file 'src/wui/multiplayersetupgroup.h'
--- src/wui/multiplayersetupgroup.h	2017-01-25 18:55:59 +0000
+++ src/wui/multiplayersetupgroup.h	2017-09-02 13:55:49 +0000
@@ -33,7 +33,6 @@
 #include "ui_basic/textarea.h"
 
 struct GameSettingsProvider;
-struct MultiPlayerSetupGroupOptions;
 struct MultiPlayerClientGroup;
 struct MultiPlayerPlayerGroup;
 
@@ -44,26 +43,27 @@
  * clients, computers and closed players.
  *
  */
-struct MultiPlayerSetupGroup : public UI::Panel {
+struct MultiPlayerSetupGroup : public UI::Box {
 	MultiPlayerSetupGroup(UI::Panel* parent,
 	                      int32_t x,
 	                      int32_t y,
 	                      int32_t w,
 	                      int32_t h,
 	                      GameSettingsProvider* settings,
-	                      uint32_t butw,
 	                      uint32_t buth);
 	~MultiPlayerSetupGroup();
 
-	void refresh();
-
 private:
+	void update();
+	void draw(RenderTarget& dst) override;
+
 	GameSettingsProvider* const s;
 	std::unique_ptr<NetworkPlayerSettingsBackend> npsb;
 	std::vector<MultiPlayerClientGroup*> multi_player_client_groups;  // not owned
 	std::vector<MultiPlayerPlayerGroup*> multi_player_player_groups;  // not owned
+	std::unique_ptr<Notifications::Subscriber<NoteGameSettings>> subscriber_;
+
 	UI::Box clientbox, playerbox;
-	std::vector<UI::Textarea*> labels;
 
 	uint32_t buth_;
 

=== modified file 'src/wui/playerdescrgroup.cc'
--- src/wui/playerdescrgroup.cc	2017-01-30 14:40:12 +0000
+++ src/wui/playerdescrgroup.cc	2017-09-02 13:55:49 +0000
@@ -116,7 +116,7 @@
 
 	d->btnEnablePlayer->set_enabled(stateaccess);
 
-	if (player.state == PlayerSettings::stateClosed) {
+	if (player.state == PlayerSettings::State::kClosed) {
 		d->btnEnablePlayer->set_state(false);
 		d->btnPlayerTeam->set_visible(false);
 		d->btnPlayerTeam->set_enabled(false);
@@ -132,7 +132,7 @@
 		d->btnPlayerType->set_visible(true);
 		d->btnPlayerType->set_enabled(stateaccess);
 
-		if (player.state == PlayerSettings::stateOpen) {
+		if (player.state == PlayerSettings::State::kOpen) {
 			d->btnPlayerType->set_title(_("Open"));
 			d->btnPlayerTeam->set_visible(false);
 			d->btnPlayerTeam->set_visible(false);
@@ -144,7 +144,7 @@
 		} else {
 			std::string title;
 
-			if (player.state == PlayerSettings::stateComputer) {
+			if (player.state == PlayerSettings::State::kComputer) {
 				if (player.ai.empty())
 					title = _("Computer");
 				else {
@@ -156,7 +156,7 @@
 						title = _(impl->descname);
 					}
 				}
-			} else {  // PlayerSettings::stateHuman
+			} else {  // PlayerSettings::State::stateHuman
 				title = _("Human");
 			}
 			d->btnPlayerType->set_title(title);
@@ -215,11 +215,11 @@
 		return;
 
 	if (on) {
-		if (settings.players[d->plnum].state == PlayerSettings::stateClosed)
+		if (settings.players[d->plnum].state == PlayerSettings::State::kClosed)
 			d->settings->next_player_state(d->plnum);
 	} else {
-		if (settings.players[d->plnum].state != PlayerSettings::stateClosed)
-			d->settings->set_player_state(d->plnum, PlayerSettings::stateClosed);
+		if (settings.players[d->plnum].state != PlayerSettings::State::kClosed)
+			d->settings->set_player_state(d->plnum, PlayerSettings::State::kClosed);
 	}
 }