Merge lp:~widelands-dev/widelands/bug-impregnable-castles into lp:widelands

[Notabilis]

Checking for visibility of military building before permitting attack.

With big military buildings, i.e., castles, it can happen that only the left side of the building is in the visibility range of another player.
If an own military building is near, that player can open the attack window and select and order some soldiers to attack the castle. Only after ordering the attack the game checks whether the door of the building is visible. Since it isn't, no attack happens.
For the player this has the effect that an attack is ordered but is never executed. This branch does an additional visibility check when calculating the number of available soldiers for displaying the UI, returning 0 when the door is invisible and the attack command would fail later on.

[GunChleoc (gunchleoc)]

Good catch!

Code LGTM, not tested

review: Approve

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 3509. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/378301859.
Appveyor build 3314. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_impregnable_castles-3314.

[GunChleoc (gunchleoc)]

Did a bit of testing.

I think we should pull out functions here in the future - I have created a bug: https://bugs.launchpad.net/widelands/+bug/1771058

@bunnybot merge

=== modified file 'src/wui/attack_box.cc'
--- src/wui/attack_box.cc	2018-05-13 07:15:39 +0000
+++ src/wui/attack_box.cc	2018-05-13 07:57:07 +0000
@@ -48,8 +48,16 @@
 uint32_t AttackBox::get_max_attackers() {
 	assert(player_);
 
-	if (upcast(Building, building, map_.get_immovable(*node_coordinates_)))
+	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;
+        }
+
 		return player_->find_attack_soldiers(building->base_flag());
+	}
 	return 0;
 }