widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

TiborB has proposed merging lp:~widelands-dev/widelands/find_portdock_reworked into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/find_portdock_reworked/+merge/286408

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...
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/find_portdock_reworked into lp:widelands.

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2016-02-14 14:09:29 +0000
+++ src/logic/map.cc	2016-02-17 21:24:46 +0000
@@ -1340,31 +1340,37 @@
 		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);
+			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;