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

[TiborB]

This needs a discussion. Generally it works good enough, but I dont fully understand old logic. The code:
- returns portdock of size up to 2 (fields)
- makes sure all fields are valid
- but the problem (not invoked by this change) is when function returns 0 fields of portdock. The calling code is not ready for portdock of size 0. It crashes the game.

During my testing I had not run into such situation though...

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 727. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/109980041.
Appveyor build 574. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_find_portdock_reworked-574.

[bunnybot (widelandsofficial)]

Bunnybot encountered an error while working on this merge proposal:

Running 'bzr pull --overwrite' failed. Output:

Permission denied (publickey).
ConnectionReset reading response for 'BzrDir.open_2.1', retrying
Permission denied (publickey).
bzr: ERROR: Connection closed: Unexpected end of message. Please check connectivity and permissions, and report a bug if problems persist.
Using saved parent location: bzr+ssh://bazaar.launchpad.net/~widelands-dev/widelands/find_portdock_reworked/

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 727. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/109980041.
Appveyor build 574. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_find_portdock_reworked-574.

[GunChleoc (gunchleoc)]

LGTM :)

@bunnybot merge

review: Approve

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2016-02-18 08:42:46 +0000
+++ src/logic/map.cc	2016-02-20 12:53:00 +0000
@@ -1340,31 +1340,39 @@
 		WALK_E, WALK_E, WALK_E
 	};
 	const FCoords start = br_n(br_n(get_fcoords(c)));
-	FCoords f[16];
-	bool iswater[16];
-	int firstwater = -1;
-	int lastnonwater = -1;
-	f[0] = start;
+	const Widelands::PlayerNumber owner = start.field->get_owned_by();
+	bool is_good_water;
+	FCoords f = start;
+	std::vector<Coords> portdock;
 	for (uint32_t i = 0; i < 16; ++i) {
-		iswater[i] = (f[i].field->get_caps() & (MOVECAPS_SWIM|MOVECAPS_WALK)) == MOVECAPS_SWIM;
-		if (iswater[i]) {
-			if (firstwater < 0)
-				firstwater = i;
-		} else {
-			lastnonwater = i;
-		}
+		is_good_water = (f.field->get_caps() & (MOVECAPS_SWIM|MOVECAPS_WALK)) == MOVECAPS_SWIM;
+
+		// Any immovable here? (especially another portdock)
+		if (is_good_water && f.field->get_immovable()) {
+			is_good_water = false;
+		}
+
+		// If starting point is owned we make sure this field has the same owner
+		if (is_good_water && owner > 0 && f.field->get_owned_by() != owner) {
+			is_good_water = false;
+		}
+
+		// ... and is not on a border
+		if (is_good_water && owner > 0 && f.field->is_border()) {
+			is_good_water = false;
+		}
+
+		if (is_good_water) {
+			portdock.push_back(f);
+			// Occupy 2 fields maximum in order not to block space for other ports that
+			// might be built in the vicinity.
+			if (portdock.size() == 2) {
+				return portdock;
+			}
+		}
+
 		if (i < 15)
-			f[i + 1] = get_neighbour(f[i], cycledirs[i]);
-	}
-
-	std::vector<Coords> portdock;
-	if (firstwater >= 0) {
-		for (uint32_t i = firstwater; i < 16 && iswater[i]; ++i)
-			portdock.push_back(f[i]);
-		if (firstwater == 0 && lastnonwater >= 0) {
-			for (uint32_t i = lastnonwater + 1; i < 16; ++i)
-				portdock.push_back(f[i]);
-		}
+			f = get_neighbour(f, cycledirs[i]);
 	}
 
 	return portdock;