Merge lp:~widelands-dev/widelands/choose-attack-soldiers into lp:widelands

[Benedikt Straub]

Allow the player to choose the soldiers to send in the attack box

The attack box contains two lists of soldiers: One for the attacking soldiers and one for the rest. Click on a soldier to move him to the other list. Ctrl-Click to move all soldiers.

If you just want to attack quickly and don´t care about soldier choice, you can still use the slider or the more/less buttons.

Note that one soldier will always remain in every militarysite. Currently the engine decides which soldier this is, and he will not be shown in the attack box. Ideally, all available soldiers should be shown, grouped by their building, and the player can then choose which soldier(s) remain(s) behind, but this would clutter up the interface too much in my opinion if there are many own militarysites, especially small ones, nearby.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4908. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/529279191.
Appveyor build 4689. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_choose_attack_soldiers-4689.

[Toni Förster (stonerl)]

Did you remove the changes from this branch?

https://code.launchpad.net/~widelands-dev/widelands/soldier-capacity-ctrl

[Benedikt Straub (nordfriese)]

Formatted the tooltips and replied to diff comments.

> Did you remove the changes from this branch? […]

No, I didn´t even touch the file where that code is located?

[Toni Förster (stonerl)]

Sorry my mistake. But I do have a request. Could you add the CTRL-key behaviour to set the soldiers to max or minimum?

This is the only window left where this isn't possible.

[Benedikt Straub (nordfriese)]

OK, I´ll add this :)
By the way, you can also Ctrl-click one of the ListOfSoldiers to put all soldiers on the other list.

[Toni Förster (stonerl)]

Tested 9101 and it works perfect.

Just one more thing. There are only 8 Soldiers shown per row. Which means there is a huge space below the "Start Attack" button. Is it somehow possible to use this space as well and show 9 soldiers per row?

[GunChleoc (gunchleoc)]

How about removing the slider and replacing it with some buttons next to the lists? I hate fiddling with my mouse to position it over that tiny thing.

[GunChleoc (gunchleoc)]

Another idea: Add soldier levels to the tooltips. Good idea/bad idea?

[Benedikt Straub (nordfriese)]

I´m in favour of keeping the slider. When you have a massive army of similar-strength soldiers and you want to attack with about half of them, the slider is the most efficient way to do this. A spinner, even with big-step buttons, would be too slow IMHO if you have really huge armies.

Since the tooltip already contains the description what happens on click and Ctrl-click, the soldier level shouldn´t be shown there as well. But I can add it to a textbox below the list like in militarysites…

[Toni Förster (stonerl)]

> How about removing the slider and replacing it with some buttons next to the
> lists? I hate fiddling with my mouse to position it over that tiny thing.

Why not just make the slider bigger?

[GunChleoc (gunchleoc)]

How about using Shift-click for adding all soldiers from the start of the list to the one that you are clicking?

> But I can add it to a textbox below the list like in militarysites…

Sounds good :)

[Toni Förster (stonerl)]

Some changes I'd like to propose.

Do we need the word "soldiers" left to the slider? Shouldn't this be
obvious? We could get more space by removing text. Secondly make the
slider buttons as big as the attack button.

Here is how it could look like:

https://fosuta.org/pics/attack.png

I can attach a diff to the bug report if you want me to.

Is it possible to move the "Start Attack" Button into the attack_box?
We would then get rid of the space below.

[Toni Förster (stonerl)]

Here is another idea:

https://fosuta.org/pics/attack1.png

[Toni Förster (stonerl)]

Sorry for spamming, but I think this would be the ideal solution, IMHO.

https://fosuta.org/pics/attack2.png

[Benedikt Straub (nordfriese)]

Un nu de Weddervörhersaag: Een bannig dichter Hagel vun Vörslagens ;)
How do you like the one I implemented now?

[Toni Förster (stonerl)]

> Un nu de Weddervörhersaag: Een bannig dichter Hagel vun Vörslagens ;)

:D

> How do you like the one I implemented now?

Almost awesome. :)

Some nits though:

The tooltips for the lists hide the line were the soldier’s strength is shown.

If you could change the values to the ones in the diff-comments and add the
space it would look perfect.

See the screenshot:

https://fosuta.org/pics/attack3.png

And don't forget to remove or comment out this line in fieldaction.cc:

static const char* const pic_attack = "images/wui/buildings/menu_attack.png";

[Toni Förster (stonerl)]

Oh I forgot, If you could make the slider 17 pix high instead of 20 it would look more elegant, IMHO.

[Benedikt Straub (nordfriese)]

Implemented the changes as you suggested :)

> The tooltips for the lists hide the line were the soldier’s strength is shown.

I´m not sure how to solve this best: putting the textarea with the info on top wouldn´t look good IMHO, and the tooltip is long enough and shouldn´t contain this as well.
Perhaps I could swap the contents of the textarea and the tooltip, so the explanation what happens when clicking is displayed in the bottom line, and the info of the soldier under the mouse is shown as a tooltip that contains nothing else. What do you think?

[Toni Förster (stonerl)]

Sorry to bother you once more :(

Could you make the slider 210px wide, please? Then we would have a maximum of 10 soldiers per row, which looks even better. :)

Regarding the tooltip. Putting them in the text area where the soldier's info are shown is too intrusive, IMHO. Can text areas also hold tooltips?

On the other hand these tooltips are quite massive. Why don't we add them to the controls-list in the encyclopedia?

[Benedikt Straub (nordfriese)]

> Sorry to bother you once more :(
 No problem :)

> Could you make the slider 210px wide, please
 Done

> Can text areas also hold tooltips?
 Every UI element can have a tooltip, so it could be moved to the bottom line. Problem with this is that nobody has a reason to check out a textarea´s tooltip, and since the behaviour of Shift-click is not obvious, people would notice this feature only by chance.

> On the other hand these tooltips are quite massive.
 No more than those on the inputqueue buttons ;)

> Why don't we add them to the controls-list in the encyclopedia?
 How many players actually read that frequently? ;) Again there´s the danger that many people won´t notice if it isn´t explained in the attackbox. Besides, this would be inconsistent as Ctrl-/Shift modifiers are explained in a tooltip for all other controls.

[Toni Förster (stonerl)]

Hmmm, what if we added them to the "Attackers" and "Not attacking" fields? Players would definitely hover over these areas.

[Benedikt Straub (nordfriese)]

Good idea, implemented it like this then :)

[Toni Förster (stonerl)]

Looks good

review: Approve

[Toni Förster (stonerl)]

a/this

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 4932. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/530921209.
Appveyor build 4713. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_choose_attack_soldiers-4713.

[GunChleoc (gunchleoc)]

I have pushed a commit with i18n fixes. Please merge this branch if you agree with them.

I replaced "x / y soldiers" with "x soldiers", because the total is already on the button next to the slider, and we don't need the information twice.

review: Approve

[Benedikt Straub (nordfriese)]

Thanks for the fixes :)

@bunnybot merge

[Toni Förster (stonerl)]

src/wui/attack_box.cc:109:75: warning: unused parameter 'max_attackers' [-Wunused-parameter]
static inline std::string slider_heading(uint32_t num_attackers, uint32_t max_attackers)

[GunChleoc (gunchleoc)]

Oops. I have cleaned this up now.

=== modified file 'src/ai/defaultai_warfare.cc'
--- src/ai/defaultai_warfare.cc	2019-04-09 16:43:49 +0000
+++ src/ai/defaultai_warfare.cc	2019-05-11 10:49:50 +0000
@@ -481,7 +481,8 @@
 	}
 
 	// how many attack soldiers we can send?
-	int32_t attackers = player_->find_attack_soldiers(*flag);
+	std::vector<Soldier*> soldiers;
+	int32_t attackers = player_->find_attack_soldiers(*flag, &soldiers);
 	assert(attackers < 500);
 
 	if (attackers > 5) {
@@ -499,7 +500,12 @@
 	    player_number(), flag->get_position().x, flag->get_position().y, best_score, attackers,
 	    enemy_sites[best_target].attack_counter + 1,
 	    (gametime - enemy_sites[best_target].last_time_attacked) / 1000);
-	game().send_player_enemyflagaction(*flag, player_number(), static_cast<uint16_t>(attackers));
+	std::vector<Serial> attacking_soldiers;
+	for (int a = 0; a < attackers; ++a) {
+		// TODO(Nordfriese): We could now choose the soldiers we want to send
+		attacking_soldiers.push_back(soldiers[a]->serial());
+	}
+	game().send_player_enemyflagaction(*flag, player_number(), attacking_soldiers);
 	assert(1 <
 	       player_->vision(Map::get_index(flag->get_building()->get_position(), map.get_width())));
 	attackers_count_ += attackers;
 
=== modified file 'src/logic/game.cc'
--- src/logic/game.cc	2019-05-05 18:53:14 +0000
+++ src/logic/game.cc	2019-05-11 10:49:50 +0000
@@ -820,10 +820,11 @@
 
 void Game::send_player_enemyflagaction(const Flag& flag,
                                        PlayerNumber const who_attacks,
-                                       uint32_t const num_soldiers) {
+                                       const std::vector<Serial>& soldiers) {
 	if (1 < player(who_attacks)
-	           .vision(Map::get_index(flag.get_building()->get_position(), map().get_width())))
-		send_player_command(*new CmdEnemyFlagAction(get_gametime(), who_attacks, flag, num_soldiers));
+	           .vision(Map::get_index(flag.get_building()->get_position(), map().get_width()))) {
+		send_player_command(*new CmdEnemyFlagAction(get_gametime(), who_attacks, flag, soldiers));
+	}
 }
 
 void Game::send_player_ship_scouting_direction(Ship& ship, WalkingDir direction) {
 
=== modified file 'src/logic/game.h'
--- src/logic/game.h	2019-03-01 16:24:48 +0000
+++ src/logic/game.h	2019-05-11 10:49:50 +0000
@@ -273,7 +273,7 @@
 	void send_player_change_training_options(TrainingSite&, TrainingAttribute, int32_t);
 	void send_player_drop_soldier(Building&, int32_t);
 	void send_player_change_soldier_capacity(Building&, int32_t);
-	void send_player_enemyflagaction(const Flag&, PlayerNumber, uint32_t count);
+	void send_player_enemyflagaction(const Flag&, PlayerNumber, const std::vector<Serial>&);
 
 	void send_player_ship_scouting_direction(Ship&, WalkingDir);
 	void send_player_ship_construct_port(Ship&, Coords);
 
=== modified file 'src/logic/map_objects/tribes/militarysite.cc'
--- src/logic/map_objects/tribes/militarysite.cc	2019-03-01 04:19:53 +0000
+++ src/logic/map_objects/tribes/militarysite.cc	2019-05-11 10:49:50 +0000
@@ -374,6 +374,7 @@
 			      stationed % (capacity_ - stationed))
 			        .str();
 		} else {
+			/** TRANSLATORS: Number of soldiers stationed at a militarysite. */
 			*s = (boost::format(ngettext("%u soldier", "%u soldiers", stationed)) % stationed).str();
 		}
 	} else {
 
=== modified file 'src/logic/player.cc'
--- src/logic/player.cc	2019-05-04 10:47:44 +0000
+++ src/logic/player.cc	2019-05-11 10:49:50 +0000
@@ -950,20 +950,17 @@
 
 // TODO(unknown): Clean this mess up. The only action we really have right now is
 // to attack, so pretending we have more types is pointless.
-void Player::enemyflagaction(Flag& flag, PlayerNumber const attacker, uint32_t const count) {
-	if (attacker != player_number())
+void Player::enemyflagaction(Flag& flag, PlayerNumber const attacker,
+		const std::vector<Widelands::Soldier*>& soldiers) {
+	if (attacker != player_number()) {
 		log("Player (%d) is not the sender of an attack (%d)\n", attacker, player_number());
-	else if (count == 0)
-		log("enemyflagaction: count is 0\n");
-	else if (is_hostile(flag.owner())) {
+	} else if (soldiers.empty()) {
+		log("enemyflagaction: no soldiers given\n");
+	} else if (is_hostile(flag.owner())) {
 		if (Building* const building = flag.get_building()) {
 			if (const AttackTarget* attack_target = building->attack_target()) {
 				if (attack_target->can_be_attacked()) {
-					std::vector<Soldier*> attackers;
-					find_attack_soldiers(flag, &attackers, count);
-					assert(attackers.size() <= count);
-
-					for (Soldier* temp_attacker : attackers) {
+					for (Soldier* temp_attacker : soldiers) {
 						upcast(MilitarySite, ms, temp_attacker->get_location(egbase()));
 						ms->send_attacker(*temp_attacker, *building);
 					}
 
=== modified file 'src/logic/player.h'
--- src/logic/player.h	2019-04-26 16:52:39 +0000
+++ src/logic/player.h	2019-05-11 10:49:50 +0000
@@ -537,7 +537,7 @@
 	uint32_t find_attack_soldiers(Flag&,
 	                              std::vector<Soldier*>* soldiers = nullptr,
 	                              uint32_t max = std::numeric_limits<uint32_t>::max());
-	void enemyflagaction(Flag&, PlayerNumber attacker, uint32_t count);
+	void enemyflagaction(Flag&, PlayerNumber attacker, const std::vector<Widelands::Soldier*>&);
 
 	uint32_t casualties() const {
 		return casualties_;
 
=== modified file 'src/logic/playercommand.cc'
--- src/logic/playercommand.cc	2019-03-09 08:58:52 +0000
+++ src/logic/playercommand.cc	2019-05-11 10:49:50 +0000
@@ -1589,7 +1589,11 @@
 	des.unsigned_8();
 	serial = des.unsigned_32();
 	des.unsigned_8();
-	number = des.unsigned_8();
+	const uint32_t number = des.unsigned_32();
+	soldiers.clear();
+	for (uint32_t i = 0; i < number; ++i) {
+		soldiers.push_back(des.unsigned_32());
+	}
 }
 
 void CmdEnemyFlagAction::execute(Game& game) {
@@ -1597,16 +1601,21 @@
 
 	if (upcast(Flag, flag, game.objects().get_object(serial))) {
 		log("Cmd_EnemyFlagAction::execute player(%u): flag->owner(%d) "
-		    "number=%u\n",
-		    player->player_number(), flag->owner().player_number(), number);
+		    "number=%" PRIuS "\n",
+		    player->player_number(), flag->owner().player_number(), soldiers.size());
 
 		if (const Building* const building = flag->get_building()) {
 			if (player->is_hostile(flag->owner()) &&
-			    1 < player->vision(Map::get_index(building->get_position(), game.map().get_width())))
-				player->enemyflagaction(*flag, sender(), number);
-			else
+			    1 < player->vision(Map::get_index(building->get_position(), game.map().get_width()))) {
+				std::vector<Soldier*> result;
+				for (Serial s : soldiers) {
+					result.push_back(dynamic_cast<Soldier*>(game.objects().get_object(s)));
+				}
+				player->enemyflagaction(*flag, sender(), result);
+			} else {
 				log("Cmd_EnemyFlagAction::execute: ERROR: wrong player target not "
 				    "seen or not hostile.\n");
+			}
 		}
 	}
 }
@@ -1617,20 +1626,40 @@
 	ser.unsigned_8(1);
 	ser.unsigned_32(serial);
 	ser.unsigned_8(sender());
-	ser.unsigned_8(number);
+	ser.unsigned_32(soldiers.size());
+	for (Serial s : soldiers) {
+		ser.unsigned_32(s);
+	}
 }
 
-constexpr uint16_t kCurrentPacketVersionCmdEnemyFlagAction = 3;
+constexpr uint16_t kCurrentPacketVersionCmdEnemyFlagAction = 4;
 
 void CmdEnemyFlagAction::read(FileRead& fr, EditorGameBase& egbase, MapObjectLoader& mol) {
 	try {
 		const uint16_t packet_version = fr.unsigned_16();
-		if (packet_version == kCurrentPacketVersionCmdEnemyFlagAction) {
+		if (packet_version <= kCurrentPacketVersionCmdEnemyFlagAction && packet_version >= 3) {
 			PlayerCommand::read(fr, egbase, mol);
 			fr.unsigned_8();
 			serial = get_object_serial_or_zero<Flag>(fr.unsigned_32(), mol);
 			fr.unsigned_8();
-			number = fr.unsigned_8();
+
+			soldiers.clear();
+			if (packet_version == kCurrentPacketVersionCmdEnemyFlagAction) {
+				const uint32_t number = fr.unsigned_32();
+				for (uint32_t i = 0; i < number; ++i) {
+					soldiers.push_back(mol.get<Soldier>(fr.unsigned_32()).serial());
+				}
+			} else {
+				const uint8_t number = fr.unsigned_8();
+				upcast(Flag, flag, egbase.objects().get_object(serial));
+				assert(flag);
+				std::vector<Soldier*> result;
+				egbase.get_player(sender())->find_attack_soldiers(*flag, &result, number);
+				assert(result.size() == number);
+				for (const auto& s : result) {
+					soldiers.push_back(s->serial());
+				}
+			}
 		} else {
 			throw UnhandledVersionError(
 			   "CmdEnemyFlagAction", packet_version, kCurrentPacketVersionCmdEnemyFlagAction);
@@ -1653,7 +1682,10 @@
 
 	// Now param
 	fw.unsigned_8(sender());
-	fw.unsigned_8(number);
+	fw.unsigned_32(soldiers.size());
+	for (Serial s : soldiers) {
+		fw.unsigned_32(mos.get_object_file_index(*egbase.objects().get_object(s)));
+	}
 }
 
 /*** struct PlayerMessageCommand ***/
 
=== modified file 'src/logic/playercommand.h'
--- src/logic/playercommand.h	2019-02-23 11:00:49 +0000
+++ src/logic/playercommand.h	2019-05-11 10:49:50 +0000
@@ -733,10 +733,10 @@
 };
 
 struct CmdEnemyFlagAction : public PlayerCommand {
-	CmdEnemyFlagAction() : PlayerCommand(), serial(0), number(0) {
+	CmdEnemyFlagAction() : PlayerCommand(), serial(0) {
 	}  // For savegame loading
-	CmdEnemyFlagAction(uint32_t t, int32_t p, const Flag& f, uint32_t num)
-	   : PlayerCommand(t, p), serial(f.serial()), number(num) {
+	CmdEnemyFlagAction(uint32_t t, int32_t p, const Flag& f, const std::vector<Serial>& s)
+	   : PlayerCommand(t, p), serial(f.serial()), soldiers(s) {
 	}
 
 	// Write these commands to a file (for savegames)
@@ -754,7 +754,7 @@
 
 private:
 	Serial serial;
-	uint8_t number;
+	std::vector<Serial> soldiers;
 };
 
 /// Abstract base for commands about a message.
 
=== modified file 'src/wui/attack_box.cc'
--- src/wui/attack_box.cc	2019-02-23 11:00:49 +0000
+++ src/wui/attack_box.cc	2019-05-11 10:49:50 +0000
@@ -30,7 +30,7 @@
 #include "graphic/text_constants.h"
 #include "logic/map_objects/tribes/soldier.h"
 
-constexpr int32_t kUpdateTimeInGametimeMs = 1000;  //  1 second, gametime
+constexpr int32_t kUpdateTimeInGametimeMs = 500;  //  half a second, gametime
 
 AttackBox::AttackBox(UI::Panel* parent,
                      Widelands::Player* player,
@@ -45,19 +45,24 @@
 	init();
 }
 
-uint32_t AttackBox::get_max_attackers() {
+std::vector<Widelands::Soldier*> AttackBox::get_max_attackers() {
 	assert(player_);
 
 	if (upcast(Building, building, map_.get_immovable(*node_coordinates_))) {
-		if (player_->vision(map_.get_index(building->get_position(), map_.get_width())) <= 1) {
-			// Player can't see the buildings door, so it can't be attacked
-			// This is the same check as done later on in send_player_enemyflagaction()
-			return 0;
+		if (player_->vision(map_.get_index(building->get_position(), map_.get_width())) > 1) {
+			std::vector<Widelands::Soldier*> v;
+			// TODO(Nordfriese): This method decides by itself which soldier remains in the building.
+			// This soldier will not show up in the result vector. Perhaps we should show all
+			// available soldiers, grouped by building, so the player can choose between all soldiers
+			// knowing that at least one of each group will have to stay at home. However, this
+			// could clutter up the screen a lot. Especially if you have many small buildings.
+			player_->find_attack_soldiers(building->base_flag(), &v);
+			return v;
 		}
-
-		return player_->find_attack_soldiers(building->base_flag());
+		// Player can't see the buildings door, so it can't be attacked
+		// This is the same check as done later on in send_player_enemyflagaction()
 	}
-	return 0;
+	return std::vector<Widelands::Soldier*>();
 }
 
 std::unique_ptr<UI::HorizontalSlider> AttackBox::add_slider(UI::Box& parent,
@@ -86,7 +91,7 @@
                                                   void (AttackBox::*fn)(),
                                                   const std::string& tooltip_text) {
 	std::unique_ptr<UI::Button> button(new UI::Button(
-	   &parent, text, 8, 8, 26, 26, UI::ButtonStyle::kWuiPrimary, text, tooltip_text));
+	   &parent, text, 8, 8, 34, 34, UI::ButtonStyle::kWuiPrimary, text, tooltip_text));
 	button->sigclicked.connect(boost::bind(fn, boost::ref(*this)));
 	parent.add(button.get());
 	return button;
@@ -96,31 +101,78 @@
  * Update available soldiers
  */
 void AttackBox::think() {
-	const int32_t gametime = player_->egbase().get_gametime();
-	if ((gametime - lastupdate_) > kUpdateTimeInGametimeMs) {
-		update_attack();
-		lastupdate_ = gametime;
+	if ((player_->egbase().get_gametime() - lastupdate_) > kUpdateTimeInGametimeMs) {
+		update_attack(false);
 	}
 }
 
-void AttackBox::update_attack() {
+static inline std::string slider_heading(uint32_t num_attackers) {
+	/** TRANSLATORS: Number of soldiers that should attack. Used in Attack box. */
+	return (boost::format(ngettext("%u soldier", "%u soldiers", num_attackers)) % num_attackers).str();
+}
+
+void AttackBox::update_attack(bool action_on_panel) {
+	lastupdate_ = player_->egbase().get_gametime();
+
 	assert(soldiers_slider_.get());
 	assert(soldiers_text_.get());
 	assert(less_soldiers_.get());
 	assert(more_soldiers_.get());
-
-	int32_t max_attackers = get_max_attackers();
-
+	assert(attacking_soldiers_.get());
+	assert(remaining_soldiers_.get());
+
+	std::vector<Widelands::Soldier*> all_attackers = get_max_attackers();
+	const int max_attackers = all_attackers.size();
+
+	// Update number of available soldiers
 	if (soldiers_slider_->get_max_value() != max_attackers) {
 		soldiers_slider_->set_max_value(max_attackers);
 	}
 
+	// Add new soldiers and remove missing soldiers to/from the list
+	for (const auto& s : all_attackers) {
+		if (!attacking_soldiers_->contains(s) && !remaining_soldiers_->contains(s)) {
+			remaining_soldiers_->add(s);
+		}
+	}
+	for (const auto& s : remaining_soldiers_->get_soldiers()) {
+		if (std::find(all_attackers.begin(), all_attackers.end(), s) == all_attackers.end()) {
+			remaining_soldiers_->remove(s);
+		}
+	}
+	for (const auto& s : attacking_soldiers_->get_soldiers()) {
+		if (std::find(all_attackers.begin(), all_attackers.end(), s) == all_attackers.end()) {
+			attacking_soldiers_->remove(s);
+		}
+	}
+
+	if (action_on_panel) {
+		// The player clicked on soldiers in the list – update slider
+		soldiers_slider_->set_value(attacking_soldiers_->count_soldiers());
+	} else {
+		// The slider was moved or we were called from think() – shift lacking/extra soldiers between the lists
+		const int32_t lacking = soldiers_slider_->get_value() - attacking_soldiers_->count_soldiers();
+		if (lacking > 0) {
+			for (int32_t i = 0; i < lacking; ++i) {
+				const Widelands::Soldier* s = remaining_soldiers_->get_soldier();
+				remaining_soldiers_->remove(s);
+				attacking_soldiers_->add(s);
+			}
+		} else if (lacking < 0) {
+			for (int32_t i = 0; i > lacking; --i) {
+				const Widelands::Soldier* s = attacking_soldiers_->get_soldier();
+				attacking_soldiers_->remove(s);
+				remaining_soldiers_->add(s);
+			}
+		}
+	}
+
+	// Update slider, buttons and texts
 	soldiers_slider_->set_enabled(max_attackers > 0);
 	more_soldiers_->set_enabled(max_attackers > soldiers_slider_->get_value());
 	less_soldiers_->set_enabled(soldiers_slider_->get_value() > 0);
-	soldiers_text_->set_text(
-	   /** TRANSLATORS: %1% of %2% soldiers. Used in Attack box. */
-	   (boost::format(_("%1% / %2%")) % soldiers_slider_->get_value() % max_attackers).str());
+
+	soldiers_text_->set_text(slider_heading(soldiers_slider_->get_value()));
 
 	more_soldiers_->set_title(std::to_string(max_attackers));
 }
@@ -128,48 +180,232 @@
 void AttackBox::init() {
 	assert(node_coordinates_);
 
-	uint32_t max_attackers = get_max_attackers();
-
-	UI::Box& linebox = *new UI::Box(this, 0, 0, UI::Box::Horizontal);
-	add(&linebox);
-	add_text(linebox, _("Soldiers:"));
-	linebox.add_space(8);
-
-	less_soldiers_ =
-	   add_button(linebox, "0", &AttackBox::send_less_soldiers, _("Send less soldiers"));
-
-	//  Spliter of soldiers
+	std::vector<Widelands::Soldier*> all_attackers = get_max_attackers();
+	const size_t max_attackers = all_attackers.size();
+
+	UI::Box& mainbox = *new UI::Box(this, 0, 0, UI::Box::Vertical);
+	add(&mainbox);
+
+	UI::Box& linebox = *new UI::Box(&mainbox, 0, 0, UI::Box::Horizontal);
+	mainbox.add(&linebox);
+
+	less_soldiers_ = add_button(linebox, "0", &AttackBox::send_less_soldiers,
+			_("Send less soldiers. Hold down Ctrl to send no soldiers"));
+
 	UI::Box& columnbox = *new UI::Box(&linebox, 0, 0, UI::Box::Vertical);
 	linebox.add(&columnbox);
 
-	const std::string attack_string =
-	   (boost::format(_("%1% / %2%")) % (max_attackers > 0 ? 1 : 0) % max_attackers).str();
-
-	soldiers_text_.reset(
-	   &add_text(columnbox, attack_string, UI::Align::kCenter, UI_FONT_SIZE_ULTRASMALL));
+	soldiers_text_.reset(&add_text(columnbox, slider_heading(max_attackers > 0 ? 1 : 0),
+			UI::Align::kCenter, UI_FONT_SIZE_ULTRASMALL));
 
 	soldiers_slider_ = add_slider(
-	   columnbox, 100, 10, 0, max_attackers, max_attackers > 0 ? 1 : 0, _("Number of soldiers"));
-
-	soldiers_slider_->changed.connect(boost::bind(&AttackBox::update_attack, this));
-	more_soldiers_ = add_button(linebox, std::to_string(max_attackers),
-	                            &AttackBox::send_more_soldiers, _("Send more soldiers"));
+	   columnbox, 210, 17, 0, max_attackers, max_attackers > 0 ? 1 : 0, _("Number of soldiers"));
+	soldiers_slider_->changed.connect([this]() { update_attack(false); });
+
+	more_soldiers_ = add_button(linebox, std::to_string(max_attackers), &AttackBox::send_more_soldiers,
+			_("Send more soldiers. Hold down Ctrl to send as many soldiers as possible"));
+	linebox.add_space(8);
+
+	attack_button_.reset(new UI::Button(&linebox, "attack", 8, 8, 34, 34, UI::ButtonStyle::kWuiPrimary,
+			g_gr->images().get("images/wui/buildings/menu_attack.png"), _("Start attack")));
+	linebox.add(attack_button_.get());
+
+	attacking_soldiers_.reset(new ListOfSoldiers(&mainbox, this, 0, 0, 30, 30));
+	remaining_soldiers_.reset(new ListOfSoldiers(&mainbox, this, 0, 0, 30, 30));
+	attacking_soldiers_->set_complement(remaining_soldiers_.get());
+	remaining_soldiers_->set_complement(attacking_soldiers_.get());
+	for (const auto& s : all_attackers) {
+		remaining_soldiers_->add(s);
+	}
+
+	boost::format tooltip_format("%s<br><p><font size=%d bold=0>%s<br>%s</font></p>");
+	{
+		UI::Textarea& txt = add_text(mainbox, _("Attackers:"));
+		// Needed so we can get tooltips
+		txt.set_handle_mouse(true);
+		txt.set_tooltip((tooltip_format
+				% _("Click on a soldier to remove him from the list of attackers")
+				% UI_FONT_SIZE_MESSAGE
+				% _("Hold down Ctrl to remove all soldiers from the list")
+				% _("Hold down Shift to remove all soldiers up to the one you’re pointing at"))
+				.str());
+		mainbox.add(attacking_soldiers_.get(), UI::Box::Resizing::kFullSize);
+	}
+
+	{
+		UI::Textarea& txt = add_text(mainbox, _("Not attacking:"));
+		txt.set_handle_mouse(true);
+		txt.set_tooltip((tooltip_format
+				% _("Click on a soldier to add him to the list of attackers")
+				% UI_FONT_SIZE_MESSAGE
+				% _("Hold down Ctrl to add all soldiers to the list")
+				% _("Hold down Shift to add all soldiers up to the one you’re pointing at"))
+				.str());
+		mainbox.add(remaining_soldiers_.get(), UI::Box::Resizing::kFullSize);
+	}
+
+	current_soldier_stats_.reset(new UI::Textarea(&mainbox, "", UI::Align::kCenter));
+	mainbox.add(current_soldier_stats_.get(), UI::Box::Resizing::kFullSize, UI::Align::kCenter);
 
 	soldiers_slider_->set_enabled(max_attackers > 0);
 	more_soldiers_->set_enabled(max_attackers > 0);
-	less_soldiers_->set_enabled(max_attackers > 0);
 }
 
 void AttackBox::send_less_soldiers() {
 	assert(soldiers_slider_.get());
-	soldiers_slider_->set_value(soldiers_slider_->get_value() - 1);
+	soldiers_slider_->set_value((SDL_GetModState() & KMOD_CTRL) ? 0 : soldiers_slider_->get_value() - 1);
 }
 
 void AttackBox::send_more_soldiers() {
-	soldiers_slider_->set_value(soldiers_slider_->get_value() + 1);
-}
-
-uint32_t AttackBox::soldiers() const {
-	assert(soldiers_slider_.get());
-	return soldiers_slider_->get_value();
-}
+	soldiers_slider_->set_value((SDL_GetModState() & KMOD_CTRL) ? soldiers_slider_->get_max_value() :
+			soldiers_slider_->get_value() + 1);
+}
+
+size_t AttackBox::count_soldiers() const {
+	return attacking_soldiers_->count_soldiers();
+}
+
+std::vector<Widelands::Serial> AttackBox::soldiers() const {
+	std::vector<Widelands::Serial> result;
+	for (const auto& s : attacking_soldiers_->get_soldiers()) {
+		result.push_back(s->serial());
+	}
+	return result;
+}
+
+constexpr int kSoldierIconWidth = 32;
+constexpr int kSoldierIconHeight = 30;
+
+AttackBox::ListOfSoldiers::ListOfSoldiers(UI::Panel* const parent,
+	           AttackBox* parent_box,
+	           int32_t const x,
+	           int32_t const y,
+	           int const w,
+	           int const h,
+               bool restrict_rows)
+   : UI::Panel(parent, x, y, w, h),
+     restricted_row_number_(restrict_rows),
+     attack_box_(parent_box) {
+	update_desired_size();
+}
+
+bool AttackBox::ListOfSoldiers::handle_mousepress(uint8_t btn, int32_t x, int32_t y) {
+	if (btn != SDL_BUTTON_LEFT || !other_) {
+		return UI::Panel::handle_mousepress(btn, x, y);
+	}
+	if (SDL_GetModState() & KMOD_CTRL) {
+		for (const auto& s : get_soldiers()) {
+			remove(s);
+			other_->add(s);
+		}
+	} else {
+		const Widelands::Soldier* soldier = soldier_at(x, y);
+		if (!soldier) {
+			return UI::Panel::handle_mousepress(btn, x, y);
+		}
+		if (SDL_GetModState() & KMOD_SHIFT) {
+			for (const auto& s : get_soldiers()) {
+				remove(s);
+				other_->add(s);
+				if (s == soldier) {
+					break;
+				}
+			}
+		} else {
+			remove(soldier);
+			other_->add(soldier);
+		}
+	}
+	attack_box_->update_attack(true);
+	return true;
+}
+
+void AttackBox::ListOfSoldiers::handle_mousein(bool) {
+	attack_box_->set_soldier_info_text();
+}
+
+bool AttackBox::ListOfSoldiers::handle_mousemove(uint8_t, int32_t x, int32_t y, int32_t, int32_t) {
+	if (const Widelands::Soldier* soldier = soldier_at(x, y)) {
+		attack_box_->set_soldier_info_text(
+				(boost::format(_("HP: %1$u/%2$u  AT: %3$u/%4$u  DE: %5$u/%6$u  EV: %7$u/%8$u")) %
+				soldier->get_health_level() % soldier->descr().get_max_health_level() %
+				soldier->get_attack_level() % soldier->descr().get_max_attack_level() %
+				soldier->get_defense_level() % soldier->descr().get_max_defense_level() %
+				soldier->get_evade_level() % soldier->descr().get_max_evade_level())
+				.str());
+	} else {
+		attack_box_->set_soldier_info_text();
+	}
+	return true;
+}
+
+Widelands::Extent AttackBox::ListOfSoldiers::size() const {
+	const size_t nr_soldiers = count_soldiers();
+	uint32_t rows = nr_soldiers / current_size_;
+	if (nr_soldiers == 0 || rows * current_size_ < nr_soldiers) {
+		++rows;
+	}
+	if (restricted_row_number_) {
+		return Widelands::Extent(rows, current_size_);
+	} else {
+		return Widelands::Extent(current_size_, rows);
+	}
+}
+
+void AttackBox::ListOfSoldiers::update_desired_size() {
+	current_size_ = std::max(1, restricted_row_number_ ? get_h() / kSoldierIconHeight : get_w() / kSoldierIconWidth);
+	const Widelands::Extent e = size();
+	set_desired_size(e.w * kSoldierIconWidth, e.h * kSoldierIconHeight);
+}
+
+const Widelands::Soldier* AttackBox::ListOfSoldiers::soldier_at(int32_t x, int32_t y) const {
+	if (x < 0 || y < 0 || soldiers_.empty()) {
+		return nullptr;
+	}
+	const int32_t col = x / kSoldierIconWidth;
+	const int32_t row = y / kSoldierIconHeight;
+	assert(col >= 0);
+	assert(row >= 0);
+	if ((restricted_row_number_ ? row : col) >= current_size_) {
+		return nullptr;
+	}
+	const int index = restricted_row_number_ ? current_size_ * col + row : current_size_ * row + col;
+	assert(index >= 0);
+	return static_cast<unsigned int>(index) < soldiers_.size() ? soldiers_[index] : nullptr;
+}
+
+void AttackBox::ListOfSoldiers::add(const Widelands::Soldier* s) {
+	soldiers_.push_back(s);
+	update_desired_size();
+}
+
+void AttackBox::ListOfSoldiers::remove(const Widelands::Soldier* s) {
+	const auto it = std::find(soldiers_.begin(), soldiers_.end(), s);
+	assert(it != soldiers_.end());
+	soldiers_.erase(it);
+	update_desired_size();
+}
+
+void AttackBox::ListOfSoldiers::draw(RenderTarget& dst) {
+	const size_t nr_soldiers = soldiers_.size();
+	int32_t column = 0;
+	int32_t row = 0;
+	for (uint32_t i = 0; i < nr_soldiers; ++i) {
+		Vector2i location(column * kSoldierIconWidth, row * kSoldierIconHeight);
+		soldiers_[i]->draw_info_icon(location, 1.0f, false, &dst);
+		if (restricted_row_number_) {
+			++row;
+			if (row >= current_size_) {
+				row = 0;
+				++column;
+			}
+		} else {
+			++column;
+			if (column >= current_size_) {
+				column = 0;
+				++row;
+			}
+		}
+	}
+}
+
 
=== modified file 'src/wui/attack_box.h'
--- src/wui/attack_box.h	2019-02-23 11:00:49 +0000
+++ src/wui/attack_box.h	2019-05-11 10:49:50 +0000
@@ -22,6 +22,8 @@
 
 #include <list>
 #include <memory>
+#include <set>
+#include <vector>
 
 #include "graphic/font_handler.h"
 #include "graphic/text/font_set.h"
@@ -51,10 +53,18 @@
 
 	void init();
 
-	uint32_t soldiers() const;
+	size_t count_soldiers() const;
+	std::vector<Widelands::Serial> soldiers() const;
+	void set_soldier_info_text(std::string text = "") {
+		current_soldier_stats_->set_text(text);
+	}
+
+	UI::Button* get_attack_button() const {
+		return attack_button_.get();
+	}
 
 private:
-	uint32_t get_max_attackers();
+	std::vector<Widelands::Soldier*> get_max_attackers();
 	std::unique_ptr<UI::HorizontalSlider> add_slider(UI::Box& parent,
 	                                                 uint32_t width,
 	                                                 uint32_t height,
@@ -73,7 +83,7 @@
 	                                       const std::string& tooltip_text);
 
 	void think() override;
-	void update_attack();
+	void update_attack(bool);
 	void send_less_soldiers();
 	void send_more_soldiers();
 
@@ -88,6 +98,72 @@
 	std::unique_ptr<UI::Button> less_soldiers_;
 	std::unique_ptr<UI::Button> more_soldiers_;
 
+	// A SoldierPanel is not applicable here as it's keyed to a building and thinks too much
+	struct ListOfSoldiers : public UI::Panel {
+		ListOfSoldiers(UI::Panel* const parent,
+		               AttackBox* parent_box,
+		               int32_t const x,
+		               int32_t const y,
+		               int const w,
+		               int const h,
+		               bool restrict_rows = false);
+
+		bool handle_mousepress(uint8_t btn, int32_t x, int32_t y) override;
+		void handle_mousein(bool) override;
+		bool handle_mousemove(uint8_t, int32_t, int32_t, int32_t, int32_t) override;
+
+		const Widelands::Soldier* soldier_at(int32_t x, int32_t y) const;
+		void add(const Widelands::Soldier*);
+		void remove(const Widelands::Soldier*);
+		bool contains(const Widelands::Soldier* soldier) const {
+			for (const auto& s : soldiers_) {
+				if (s == soldier) {
+					return true;
+				}
+			}
+			return false;
+		}
+
+		std::vector<const Widelands::Soldier*> get_soldiers() const {
+			return soldiers_;
+		}
+		const Widelands::Soldier* get_soldier() const {
+			return soldiers_.back();
+		}
+
+		size_t count_soldiers() const {
+			return soldiers_.size();
+		}
+		Widelands::Extent size() const;
+		bool row_number_restricted() const {
+			return restricted_row_number_;
+		}
+		void set_row_number_restricted(bool r) {
+			restricted_row_number_ = r;
+		}
+
+		void draw(RenderTarget& dst) override;
+
+		void set_complement(ListOfSoldiers* o) {
+			other_ = o;
+		}
+
+	private:
+		bool restricted_row_number_;
+		uint16_t current_size_; // Current number of rows or columns
+		std::vector<const Widelands::Soldier*> soldiers_;
+
+		ListOfSoldiers* other_;
+		AttackBox* attack_box_;
+
+		void update_desired_size() override;
+	};
+
+	std::unique_ptr<ListOfSoldiers> attacking_soldiers_;
+	std::unique_ptr<ListOfSoldiers> remaining_soldiers_;
+	std::unique_ptr<UI::Textarea> current_soldier_stats_;
+	std::unique_ptr<UI::Button> attack_button_;
+
 	/// The last time the information in this Panel got updated
 	uint32_t lastupdate_;
 };
 
=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc	2019-02-23 11:00:49 +0000
+++ src/wui/fieldaction.cc	2019-05-11 10:49:50 +0000
@@ -232,7 +232,6 @@
 static const char* const pic_geologist = "images/wui/fieldaction/menu_geologist.png";
 
 static const char* const pic_tab_attack = "images/wui/fieldaction/menu_tab_attack.png";
-static const char* const pic_attack = "images/wui/buildings/menu_attack.png";
 
 /*
 ===============
@@ -382,8 +381,9 @@
 				attack_box_ = new AttackBox(&a_box, player_, &node_, 0, 0);
 				a_box.add(attack_box_);
 
-				set_fastclick_panel(&add_button(
-				   &a_box, "attack", pic_attack, &FieldActionWindow::act_attack, _("Start attack")));
+				UI::Button* attack_button = attack_box_->get_attack_button();
+				attack_button->sigclicked.connect(boost::bind(&FieldActionWindow::act_attack, this));
+				set_fastclick_panel(attack_button);
 			}
 		}
 	}
@@ -720,10 +720,9 @@
 	assert(attack_box_);
 	upcast(Game, game, &ibase().egbase());
 	if (upcast(Building, building, game->map().get_immovable(node_)))
-		if (attack_box_->soldiers() > 0) {
+		if (attack_box_->count_soldiers() > 0) {
 			upcast(InteractivePlayer const, iaplayer, &ibase());
-			game->send_player_enemyflagaction(building->base_flag(), iaplayer->player_number(),
-			                                  attack_box_->soldiers() /*  number of soldiers */);
+			game->send_player_enemyflagaction(building->base_flag(), iaplayer->player_number(), attack_box_->soldiers());
 		}
 	reset_mouse_and_die();
 }