← Back to team overview

widelands-dev team mailing list archive

[Merge] lp:~widelands-dev/widelands/bug-1691339-editor-8-players into lp:widelands

 

GunChleoc has proposed merging lp:~widelands-dev/widelands/bug-1691339-editor-8-players into lp:widelands.

Commit message:
Refactored Editor Player Menu to use Box layout. Show dismissable warning if more than 8 players are selected.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1691339 in widelands: "Inform map editors not to use more than 8 players"
  https://bugs.launchpad.net/widelands/+bug/1691339

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1691339-editor-8-players/+merge/349092
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1691339-editor-8-players into lp:widelands.
=== modified file 'src/editor/CMakeLists.txt'
--- src/editor/CMakeLists.txt	2017-11-20 13:50:51 +0000
+++ src/editor/CMakeLists.txt	2018-07-08 07:14:33 +0000
@@ -103,6 +103,7 @@
     map_io
     map_io_map_loader
     notifications
+    profile
     random
     scripting_coroutine
     scripting_lua_interface

=== modified file 'src/editor/ui_menus/player_menu.cc'
--- src/editor/ui_menus/player_menu.cc	2018-05-22 11:29:41 +0000
+++ src/editor/ui_menus/player_menu.cc	2018-07-08 07:14:33 +0000
@@ -19,235 +19,283 @@
 
 #include "editor/ui_menus/player_menu.h"
 
+#include <memory>
+
+#include <boost/algorithm/string.hpp>
 #include <boost/format.hpp>
 
 #include "base/i18n.h"
-#include "base/wexception.h"
 #include "editor/editorinteractive.h"
 #include "editor/tools/set_starting_pos_tool.h"
 #include "graphic/graphic.h"
+#include "graphic/playercolor.h"
 #include "logic/map.h"
 #include "logic/map_objects/tribes/tribe_basic_info.h"
-#include "logic/player.h"
-#include "ui_basic/editbox.h"
-#include "ui_basic/messagebox.h"
-#include "ui_basic/textarea.h"
-
-#define UNDEFINED_TRIBE_NAME "<undefined>"
+#include "logic/widelands.h"
+#include "profile/profile.h"
+#include "ui_basic/checkbox.h"
+#include "ui_basic/multilinetextarea.h"
+
+namespace {
+constexpr int kMargin = 4;
+// If this ever gets changed, don't forget to change the strings in the warning box as well.
+constexpr Widelands::PlayerNumber max_recommended_players = 8;
+}  // namespace
+
+class EditorPlayerMenuWarningBox : public UI::Window {
+public:
+	explicit EditorPlayerMenuWarningBox(UI::Panel* parent)
+	   : Window(parent, "editor_player_menu_warning_box", 0, 0, 500, 220, _("Too Many Players")),
+	     box_(this, 0, 0, UI::Box::Vertical, 0, 0, 2 * kMargin),
+	     warning_label_(&box_,
+	                    0,
+	                    0,
+	                    300,
+	                    0,
+	                    UI::PanelStyle::kWui,
+	                    /** TRANSLATORS: Info text in editor player menu */
+	                    _("We do not recommend setting more than 8 players except for testing "
+	                      "purposes. Are you sure that you want more than 8 players?"),
+	                    UI::Align::kLeft,
+	                    UI::MultilineTextarea::ScrollMode::kNoScrolling),
+	     /** TRANSLATORS: Checkbox for: 'We do not recommend setting more than 8 players except for
+	        testing purposes. Are you sure that you want more than 8 players?' */
+	     reminder_choice_(&box_, Vector2i::zero(), _("Do not remind me again")),
+	     button_box_(&box_, kMargin, kMargin, UI::Box::Horizontal, 0, 0, 2 * kMargin),
+	     ok_(&button_box_, "ok", 0, 0, 120, 0, UI::ButtonStyle::kWuiPrimary, _("OK")),
+	     cancel_(&button_box_, "cancel", 0, 0, 120, 0, UI::ButtonStyle::kWuiSecondary, _("Abort")) {
+
+		set_center_panel(&box_);
+
+		box_.add(&warning_label_, UI::Box::Resizing::kFullSize);
+		box_.add(&reminder_choice_, UI::Box::Resizing::kFullSize);
+
+		button_box_.add_inf_space();
+		button_box_.add(&cancel_);
+		button_box_.add_inf_space();
+		button_box_.add(&ok_);
+		button_box_.add_inf_space();
+		box_.add_space(kMargin);
+		box_.add(&button_box_, UI::Box::Resizing::kFullSize);
+		box_.add_space(kMargin);
+
+		ok_.sigclicked.connect(boost::bind(&EditorPlayerMenuWarningBox::ok, boost::ref(*this)));
+		cancel_.sigclicked.connect(
+		   boost::bind(&EditorPlayerMenuWarningBox::cancel, boost::ref(*this)));
+	}
+
+	void ok() {
+		write_option();
+		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
+	}
+
+	void cancel() {
+		write_option();
+		end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
+	}
+
+	void write_option() {
+		if (reminder_choice_.get_state()) {
+			g_options.pull_section("global").set_bool(
+			   "editor_player_menu_warn_too_many_players", false);
+		}
+	}
+
+private:
+	UI::Box box_;
+	UI::MultilineTextarea warning_label_;
+	UI::Checkbox reminder_choice_;
+	UI::Box button_box_;
+	UI::Button ok_;
+	UI::Button cancel_;
+};
 
 inline EditorInteractive& EditorPlayerMenu::eia() {
 	return dynamic_cast<EditorInteractive&>(*get_parent());
 }
 
 EditorPlayerMenu::EditorPlayerMenu(EditorInteractive& parent, UI::UniqueWindow::Registry& registry)
-   : UI::UniqueWindow(&parent, "players_menu", &registry, 340, 400, _("Player Options")),
-     add_player_(this,
-                 "add_player",
-                 get_inner_w() - 5 - 20,
-                 5,
-                 20,
-                 20,
-                 UI::ButtonStyle::kWuiSecondary,
-                 g_gr->images().get("images/ui_basic/scrollbar_up.png"),
-                 _("Add player")),
-     remove_last_player_(this,
-                         "remove_last_player",
-                         5,
-                         5,
-                         20,
-                         20,
-                         UI::ButtonStyle::kWuiSecondary,
-                         g_gr->images().get("images/ui_basic/scrollbar_down.png"),
-                         _("Remove last player")),
-     tribenames_(Widelands::get_all_tribenames()) {
-	add_player_.set_enabled(parent.egbase().map().get_nrplayers() < kMaxPlayers);
-	add_player_.sigclicked.connect(
-	   boost::bind(&EditorPlayerMenu::clicked_add_player, boost::ref(*this)));
-	remove_last_player_.sigclicked.connect(
-	   boost::bind(&EditorPlayerMenu::clicked_remove_last_player, boost::ref(*this)));
-
-	int32_t const spacing = 5;
-	int32_t const width = 20;
-	int32_t posy = 0;
-
-	set_inner_size(375, 135);
-
-	UI::Textarea* ta = new UI::Textarea(this, 0, 0, _("Number of Players"));
-	ta->set_pos(Vector2i((get_inner_w() - ta->get_w()) / 2, posy + 5));
-	posy += spacing + width;
-
-	nr_of_players_ta_ = new UI::Textarea(this, 0, 0, "5");
-	nr_of_players_ta_->set_pos(Vector2i((get_inner_w() - nr_of_players_ta_->get_w()) / 2, posy + 5));
-
-	posy += width + spacing + spacing;
-
-	posy_ = posy;
-
-	for (Widelands::PlayerNumber i = 0; i < kMaxPlayers; ++i) {
-		plr_names_[i] = nullptr;
-		plr_set_pos_buts_[i] = nullptr;
-		plr_set_tribes_buts_[i] = nullptr;
-	}
-
-	if (parent.egbase().map().get_nrplayers() < 1) {
-		clicked_add_player();
-	}
-	update();
-
-	set_thinks(true);
-}
-
-/**
- * Think function. Some things may change while this window
- * is open
- */
-void EditorPlayerMenu::think() {
-	update();
-}
-
-/**
- * Update all
-*/
-void EditorPlayerMenu::update() {
-	if (is_minimal())
-		return;
-
-	Widelands::Map* map = eia().egbase().mutable_map();
-	Widelands::PlayerNumber const nr_players = map->get_nrplayers();
-	{
-		assert(nr_players <= 99);  //  2 decimal digits
-		char text[3];
-		if (char const nr_players_10 = nr_players / 10) {
-			text[0] = '0' + nr_players_10;
-			text[1] = '0' + nr_players % 10;
-			text[2] = '\0';
-		} else {
-			text[0] = '0' + nr_players;
-			text[1] = '\0';
-		}
-		nr_of_players_ta_->set_text(text);
-	}
-
-	//  Now remove all the unneeded stuff.
-	for (Widelands::PlayerNumber i = nr_players; i < kMaxPlayers; ++i) {
-		delete plr_names_[i];
-		plr_names_[i] = nullptr;
-		delete plr_set_pos_buts_[i];
-		plr_set_pos_buts_[i] = nullptr;
-		delete plr_set_tribes_buts_[i];
-		plr_set_tribes_buts_[i] = nullptr;
-	}
-	int32_t posy = posy_;
-	int32_t const spacing = 5;
-	int32_t const size = 20;
-
-	iterate_player_numbers(p, nr_players) {
-		int32_t posx = spacing;
-		if (!plr_names_[p - 1]) {
-			plr_names_[p - 1] = new UI::EditBox(this, posx, posy, 140, size, 2, UI::PanelStyle::kWui);
-			plr_names_[p - 1]->changed.connect(
-			   boost::bind(&EditorPlayerMenu::name_changed, this, p - 1));
-			posx += 140 + spacing;
-			plr_names_[p - 1]->set_text(map->get_scenario_player_name(p));
-		}
-
-		if (!plr_set_tribes_buts_[p - 1]) {
-			plr_set_tribes_buts_[p - 1] = new UI::Button(
-			   this, "tribe", posx, posy, 140, size, UI::ButtonStyle::kWuiSecondary, "");
-			plr_set_tribes_buts_[p - 1]->sigclicked.connect(
-			   boost::bind(&EditorPlayerMenu::player_tribe_clicked, boost::ref(*this), p - 1));
-			posx += 140 + spacing;
-		}
-
-		// Get/Set (localized) tribe names
-		if (map->get_scenario_player_tribe(p) != UNDEFINED_TRIBE_NAME) {
-			selected_tribes_[p - 1] = map->get_scenario_player_tribe(p);
-		} else {
-			selected_tribes_[p - 1] = tribenames_[0];
-			map->set_scenario_player_tribe(p, selected_tribes_[p - 1]);
-		}
-
-		plr_set_tribes_buts_[p - 1]->set_title(
-		   Widelands::get_tribeinfo(selected_tribes_[p - 1]).descname);
-
-		// Set default AI and closeable to false (always default - should be changed by hand)
-		map->set_scenario_player_ai(p, "");
-		map->set_scenario_player_closeable(p, false);
-
-		//  Set Starting pos button.
-		if (!plr_set_pos_buts_[p - 1]) {
-			plr_set_pos_buts_[p - 1] = new UI::Button(this, "starting_pos", posx, posy, size, size,
-			                                          UI::ButtonStyle::kWuiSecondary, nullptr, "");
-			plr_set_pos_buts_[p - 1]->sigclicked.connect(
-			   boost::bind(&EditorPlayerMenu::set_starting_pos_clicked, boost::ref(*this), p));
-		}
+   : UI::UniqueWindow(&parent, "players_menu", &registry, 100, 100, _("Player Options")),
+     box_(this, kMargin, kMargin, UI::Box::Vertical),
+     no_of_players_(&box_,
+                    0,
+                    0,
+                    50,
+                    100,
+                    24,
+                    _("Number of players"),
+                    UI::DropdownType::kTextual,
+                    UI::PanelStyle::kWui),
+     default_tribe_(Widelands::get_all_tribenames().front()) {
+	box_.set_size(100, 100);  // Prevent assert failures
+	box_.add(&no_of_players_, UI::Box::Resizing::kFullSize);
+	box_.add_space(2 * kMargin);
+
+	const Widelands::Map& map = eia().egbase().map();
+
+	// Ensure that there is at least 1 player
+	if (map.get_nrplayers() < 1) {
+		Widelands::Map* mutable_map = eia().egbase().mutable_map();
+		mutable_map->set_nrplayers(1);
+		// Init player 1
+		mutable_map->set_scenario_player_ai(1, "");
+		mutable_map->set_scenario_player_closeable(1, false);
+		/** TRANSLATORS: Default player name, e.g. Player 1 */
+		mutable_map->set_scenario_player_name(1, (boost::format(_("Player %u")) % 1).str());
+		mutable_map->set_scenario_player_tribe(1, default_tribe_);
+		eia().set_need_save(true);
+	}
+
+	const Widelands::PlayerNumber nr_players = map.get_nrplayers();
+	iterate_player_numbers(p, kMaxPlayers) {
+		const bool map_has_player = p <= nr_players;
+
+		no_of_players_.add(boost::lexical_cast<std::string>(static_cast<unsigned int>(p)), p);
+		no_of_players_.selected.connect(
+		   boost::bind(&EditorPlayerMenu::no_of_players_clicked, boost::ref(*this)));
+
+		UI::Box* row = new UI::Box(&box_, 0, 0, UI::Box::Horizontal);
+
+		// Name
+		UI::EditBox* plr_name = new UI::EditBox(row, 0, 0, 0, 0, kMargin, UI::PanelStyle::kWui);
+		if (map_has_player) {
+			plr_name->set_text(map.get_scenario_player_name(p));
+		}
+		plr_name->changed.connect(boost::bind(&EditorPlayerMenu::name_changed, this, p - 1));
+		row->add(plr_name, UI::Box::Resizing::kFillSpace);
+		row->add_space(kMargin);
+
+		// Tribe
+		UI::Dropdown<std::string>* plr_tribe =
+		   new UI::Dropdown<std::string>(row, 0, 0, 50, 400, plr_name->get_h(), _("Tribe"),
+		                                 UI::DropdownType::kPictorial, UI::PanelStyle::kWui);
+		{
+			i18n::Textdomain td("tribes");
+			for (const Widelands::TribeBasicInfo& tribeinfo : Widelands::get_all_tribeinfos()) {
+				plr_tribe->add(_(tribeinfo.descname), tribeinfo.name,
+				               g_gr->images().get(tribeinfo.icon), false, tribeinfo.tooltip);
+			}
+		}
+		const std::string player_scenario_tribe =
+		   map_has_player ? map.get_scenario_player_tribe(p) : default_tribe_;
+		plr_tribe->select(Widelands::tribe_exists(player_scenario_tribe) ? player_scenario_tribe :
+		                                                                   default_tribe_);
+		plr_tribe->selected.connect(
+		   boost::bind(&EditorPlayerMenu::player_tribe_clicked, boost::ref(*this), p - 1));
+		row->add(plr_tribe);
+		row->add_space(kMargin);
+
+		// Starting position
 		const Image* player_image =
 		   playercolor_image(p - 1, "images/players/player_position_menu.png");
 		assert(player_image);
 
-		plr_set_pos_buts_[p - 1]->set_pic(player_image);
-		posy += size + spacing;
-	}
-	add_player_.set_enabled(nr_players < kMaxPlayers);
-	remove_last_player_.set_enabled(1 < nr_players);
-	set_inner_size(get_inner_w(), posy + spacing);
-}
-
-void EditorPlayerMenu::clicked_add_player() {
-	Widelands::Map* map = eia().egbase().mutable_map();
-	Widelands::PlayerNumber const nr_players = map->get_nrplayers() + 1;
-	assert(nr_players <= kMaxPlayers);
-	map->set_nrplayers(nr_players);
-	{                             //  register new default name for this players
-		assert(nr_players <= 99);  //  2 decimal digits
-		const std::string name =
-		   /** TRANSLATORS: Default player name, e.g. Player 1 */
-		   (boost::format(_("Player %u")) % static_cast<unsigned int>(nr_players)).str();
-		map->set_scenario_player_name(nr_players, name);
-	}
-	map->set_scenario_player_tribe(nr_players, tribenames_[0]);
-	eia().set_need_save(true);
-	add_player_.set_enabled(nr_players < kMaxPlayers);
-	remove_last_player_.set_enabled(true);
-	update();
-}
-
-void EditorPlayerMenu::clicked_remove_last_player() {
+		UI::Button* plr_position =
+		   new UI::Button(row, "tribe", 0, 0, plr_tribe->get_h(), plr_tribe->get_h(),
+		                  UI::ButtonStyle::kWuiSecondary, player_image);
+		plr_position->sigclicked.connect(
+		   boost::bind(&EditorPlayerMenu::set_starting_pos_clicked, boost::ref(*this), p));
+		row->add(plr_position);
+
+		box_.add(row, UI::Box::Resizing::kFullSize);
+		box_.add_space(kMargin);
+		row->set_visible(map_has_player);
+
+		rows_.push_back(
+		   std::unique_ptr<PlayerEditRow>(new PlayerEditRow(row, plr_name, plr_position, plr_tribe)));
+	}
+
+	// Make room for 8 players
+	no_of_players_.set_max_items(max_recommended_players);
+	no_of_players_.select(nr_players);
+
+	// Init button states
+	set_starting_pos_clicked(1);
+	layout();
+}
+
+/**
+ * Update all
+ */
+void EditorPlayerMenu::layout() {
+	assert(!rows_.empty());
+	const Widelands::PlayerNumber nr_players = eia().egbase().map().get_nrplayers();
+	box_.set_size(310, no_of_players_.get_h() + kMargin +
+	                      nr_players * (rows_.front()->tribe->get_h() + kMargin));
+	set_inner_size(box_.get_w() + 2 * kMargin, box_.get_h() + 2 * kMargin);
+}
+
+void EditorPlayerMenu::no_of_players_clicked() {
 	EditorInteractive& menu = eia();
 	Widelands::Map* map = menu.egbase().mutable_map();
 	Widelands::PlayerNumber const old_nr_players = map->get_nrplayers();
-	Widelands::PlayerNumber const nr_players = old_nr_players - 1;
+	Widelands::PlayerNumber const nr_players = no_of_players_.get_selected();
 	assert(1 <= nr_players);
-
-	// if removed player was selected switch to the next highest player
-	if (old_nr_players == menu.tools()->set_starting_pos.get_current_player()) {
-		set_starting_pos_clicked(nr_players);
-	}
-
-	map->set_nrplayers(nr_players);
-	add_player_.set_enabled(nr_players < kMaxPlayers);
-	remove_last_player_.set_enabled(1 < nr_players);
-	update();
+	assert(nr_players <= kMaxPlayers);
+
+	if (old_nr_players == nr_players) {
+		return;
+	}
+
+	// Display a warning if there are too many players
+	if (nr_players > max_recommended_players) {
+		if (g_options.pull_section("global").get_bool(
+		       "editor_player_menu_warn_too_many_players", true)) {
+			EditorPlayerMenuWarningBox warning(get_parent());
+			if (warning.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kBack) {
+				// Abort setting of players
+				no_of_players_.select(std::min(old_nr_players, max_recommended_players));
+			}
+		}
+	}
+
+	if (old_nr_players < nr_players) {
+		// Add new players
+		map->set_nrplayers(nr_players);
+
+		for (Widelands::PlayerNumber pn = old_nr_players + 1; pn <= nr_players; ++pn) {
+			map->set_scenario_player_ai(pn, "");
+			map->set_scenario_player_closeable(pn, false);
+
+			// Register new default name and tribe for these players
+			const std::string name =
+			   /** TRANSLATORS: Default player name, e.g. Player 1 */
+			   (boost::format(_("Player %u")) % static_cast<unsigned int>(pn)).str();
+			map->set_scenario_player_name(pn, name);
+			rows_.at(pn - 1)->name->set_text(name);
+
+			const std::string& tribename = rows_.at(pn - 1)->tribe->get_selected();
+			assert(Widelands::tribe_exists(tribename));
+			map->set_scenario_player_tribe(pn, tribename);
+			rows_.at(pn - 1)->box->set_visible(true);
+		}
+		// Update button states
+		set_starting_pos_clicked(menu.tools()->set_starting_pos.get_current_player());
+	} else {
+		// If removed player was selected switch to the highest player
+		if (old_nr_players >= menu.tools()->set_starting_pos.get_current_player()) {
+			set_starting_pos_clicked(nr_players);
+		}
+
+		// Remove extra players
+		map->set_nrplayers(nr_players);
+		for (Widelands::PlayerNumber pn = nr_players; pn < old_nr_players; ++pn) {
+			rows_.at(pn)->box->set_visible(false);
+		}
+	}
+	menu.set_need_save(true);
+	layout();
 }
 
 /**
  * Player Tribe Button clicked
  */
 void EditorPlayerMenu::player_tribe_clicked(uint8_t n) {
+	const std::string& tribename = rows_.at(n)->tribe->get_selected();
+	assert(Widelands::tribe_exists(tribename));
 	EditorInteractive& menu = eia();
-	if (!Widelands::tribe_exists(selected_tribes_[n])) {
-		throw wexception("Map defines tribe %s, but it does not exist!", selected_tribes_[n].c_str());
-	}
-	uint32_t i;
-	for (i = 0; i < tribenames_.size(); ++i) {
-		if (tribenames_[i] == selected_tribes_[n]) {
-			break;
-		}
-	}
-	selected_tribes_[n] = i == tribenames_.size() - 1 ? tribenames_[0] : tribenames_[++i];
-	menu.egbase().mutable_map()->set_scenario_player_tribe(n + 1, selected_tribes_[n]);
+	menu.egbase().mutable_map()->set_scenario_player_tribe(n + 1, tribename);
 	menu.set_need_save(true);
-	update();
 }
 
 /**
@@ -267,18 +315,28 @@
 
 	//  reselect tool, so everything is in a defined state
 	menu.select_tool(menu.tools()->current(), EditorTool::First);
-	update();
+
+	// Signal player position states via button states
+	iterate_player_numbers(pn, map->get_nrplayers()) {
+		if (pn == n) {
+			rows_.at(pn - 1)->position->set_background_style(UI::ButtonStyle::kWuiPrimary);
+			rows_.at(pn - 1)->position->set_perm_pressed(true);
+		} else {
+			rows_.at(pn - 1)->position->set_background_style(UI::ButtonStyle::kWuiSecondary);
+			rows_.at(pn - 1)->position->set_perm_pressed(map->get_starting_pos(pn) !=
+			                                             Widelands::Coords::null());
+		}
+	}
 }
 
 /**
  * Player name has changed
  */
-void EditorPlayerMenu::name_changed(int32_t m) {
+void EditorPlayerMenu::name_changed(int32_t n) {
 	//  Player name has been changed.
-	std::string text = plr_names_[m]->text();
+	const std::string& text = rows_.at(n)->name->text();
 	EditorInteractive& menu = eia();
 	Widelands::Map* map = menu.egbase().mutable_map();
-	map->set_scenario_player_name(m + 1, text);
-	plr_names_[m]->set_text(map->get_scenario_player_name(m + 1));
+	map->set_scenario_player_name(n + 1, text);
 	menu.set_need_save(true);
 }

=== modified file 'src/editor/ui_menus/player_menu.h'
--- src/editor/ui_menus/player_menu.h	2018-04-07 16:59:00 +0000
+++ src/editor/ui_menus/player_menu.h	2018-07-08 07:14:33 +0000
@@ -20,22 +20,17 @@
 #ifndef WL_EDITOR_UI_MENUS_PLAYER_MENU_H
 #define WL_EDITOR_UI_MENUS_PLAYER_MENU_H
 
-#include <cstring>
-#include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
-#include "graphic/playercolor.h"
-#include "logic/widelands.h"
+#include "ui_basic/box.h"
 #include "ui_basic/button.h"
+#include "ui_basic/dropdown.h"
+#include "ui_basic/editbox.h"
 #include "ui_basic/unique_window.h"
 
 class EditorInteractive;
-namespace UI {
-struct Textarea;
-struct EditBox;
-struct Button;
-}
 
 class EditorPlayerMenu : public UI::UniqueWindow {
 public:
@@ -44,27 +39,33 @@
 	}
 
 private:
+	struct PlayerEditRow {
+		explicit PlayerEditRow(UI::Box* init_box,
+		                       UI::EditBox* init_name,
+		                       UI::Button* init_position,
+		                       UI::Dropdown<std::string>* init_tribe)
+		   : box(init_box), name(init_name), position(init_position), tribe(init_tribe) {
+		}
+		UI::Box* box;
+		UI::EditBox* name;
+		UI::Button* position;
+		UI::Dropdown<std::string>* tribe;
+	};
+
 	EditorInteractive& eia();
-	UI::UniqueWindow::Registry allow_buildings_menu_;
-	UI::Textarea* nr_of_players_ta_;
-	UI::EditBox* plr_names_[kMaxPlayers];
-	UI::Button add_player_, remove_last_player_;
-	UI::Button *plr_set_pos_buts_[kMaxPlayers], *plr_set_tribes_buts_[kMaxPlayers];
-
-	std::vector<std::string> tribenames_;
-
-	/// List of the tribes currently selected for all players
-	std::string selected_tribes_[kMaxPlayers];
-
-	int32_t posy_;
 
 	void name_changed(int32_t);
-	void clicked_add_player();
-	void clicked_remove_last_player();
+	void no_of_players_clicked();
 	void player_tribe_clicked(uint8_t);
 	void set_starting_pos_clicked(uint8_t);
-	void update();
-	void think() override;
+
+	void layout() override;
+
+	UI::Box box_;
+	UI::Dropdown<uintptr_t> no_of_players_;
+	std::vector<std::unique_ptr<PlayerEditRow>> rows_;
+
+	const std::string default_tribe_;
 };
 
 #endif  // end of include guard: WL_EDITOR_UI_MENUS_PLAYER_MENU_H

=== modified file 'src/ui_basic/button.cc'
--- src/ui_basic/button.cc	2018-04-27 06:11:05 +0000
+++ src/ui_basic/button.cc	2018-07-08 07:14:33 +0000
@@ -58,8 +58,7 @@
      time_nextact_(0),
      title_(title_text),
      title_image_(title_image),
-     background_style_(g_gr->styles().button_style(init_style)),
-     clr_down_(229, 161, 2) {
+     background_style_(g_gr->styles().button_style(init_style)) {
 	set_thinks(false);
 	set_can_focus(true);
 }
@@ -366,6 +365,10 @@
 	                           UI::Button::VisualState::kRaised);
 }
 
+void Button::set_background_style(UI::ButtonStyle style) {
+	background_style_ = g_gr->styles().button_style(style);
+}
+
 void Button::toggle() {
 	switch (visual_state_) {
 	case UI::Button::VisualState::kRaised:

=== modified file 'src/ui_basic/button.h'
--- src/ui_basic/button.h	2018-05-13 07:15:39 +0000
+++ src/ui_basic/button.h	2018-07-08 07:14:33 +0000
@@ -145,6 +145,9 @@
 	/// Convenience function. If 'pressed', sets the style to kPermpressed, otherwise to kRaised.
 	void set_perm_pressed(bool pressed);
 
+	/// Change the background style of the button.
+	void set_background_style(UI::ButtonStyle style);
+
 	/// Convenience function. Toggles between raised and permpressed style
 	void toggle();
 
@@ -170,7 +173,6 @@
 	const Image* title_image_;  //  custom icon on the button
 
 	const UI::PanelStyleInfo* background_style_;  // Background color and texture. Not owned.
-	RGBColor clr_down_;                           //  color of border while a flat button is "down"
 };
 
 }  // namespace UI

=== modified file 'src/ui_basic/dropdown.cc'
--- src/ui_basic/dropdown.cc	2018-04-27 06:11:05 +0000
+++ src/ui_basic/dropdown.cc	2018-07-08 07:14:33 +0000
@@ -129,8 +129,13 @@
 }
 
 BaseDropdown::~BaseDropdown() {
-	// Listselect is already taking care of the cleanup,
-	// so no call to clear() needed here.
+	// The list needs to be able to drop outside of windows, so it won't close with the window.
+	// Deleting here leads to conflict with who gets to delete it, so we hide it instead.
+	// TODO(GunChleoc): Investigate whether we can find a better solution for this
+	if (list_) {
+		list_->clear();
+		list_->set_visible(false);
+	}
 }
 
 void BaseDropdown::set_height(int height) {
@@ -138,6 +143,10 @@
 	layout();
 }
 
+void BaseDropdown::set_max_items(int items) {
+	set_height(list_->get_lineheight() * items + base_height(button_dimension_));
+}
+
 void BaseDropdown::layout() {
 	const int base_h = base_height(button_dimension_);
 	const int w = type_ == DropdownType::kPictorial ? button_dimension_ : get_w();

=== modified file 'src/ui_basic/dropdown.h'
--- src/ui_basic/dropdown.h	2018-04-27 06:11:05 +0000
+++ src/ui_basic/dropdown.h	2018-07-08 07:14:33 +0000
@@ -122,6 +122,9 @@
 
 	void set_height(int height);
 
+	/// Set the number of items to fit in the list
+	void set_max_items(int items);
+
 protected:
 	/// Add an element to the list
 	/// \param name         the display name of the entry

=== modified file 'src/wlapplication.cc'
--- src/wlapplication.cc	2018-06-11 18:21:26 +0000
+++ src/wlapplication.cc	2018-07-08 07:14:33 +0000
@@ -779,6 +779,7 @@
 	s.get_natural("metaserverport");
 	// Undocumented, checkbox appears on "Watch Replay" screen
 	s.get_bool("display_replay_filenames");
+	s.get_bool("editor_player_menu_warn_too_many_players");
 	// KLUDGE!
 
 	long int last_start = s.get_int("last_start", 0);


Follow ups