Merge lp:~widelands-dev/widelands/bug-1535065 into lp:widelands

[GunChleoc]

Unset player starting position in MapGenerator if coordinates are illegal.

This fixes a crash with the random map generator where coordinates of player positions would be outside of the map.

The player positions generated aren't necessarily viable, but that's a problem for another day.

[bunnybot (widelandsofficial)]

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/bug-1535065 mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_bug_1535065

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

[kaputtnik (franku)]

So a user make a random map, saves it and when trying to start a game with this map an error message is shown because no player position is set. Then he has to start the editor again, load the map again and probably figure out how to set player positions... And he may think he has to do this always when creating a random map.

Providing a feature which does not work properly is not a good solution in my mind. Also regarding the generated player positions, which may not be viable.

[GunChleoc (gunchleoc)]

I agree that this still needs more fixing - right now, I just want to get rid of the crash.

I tried fiddling wit it a bit, but as soon as I try to get the FCoords from the Coords, the player positions get messed up, and I have no idea why yet.

So, I am for merging this and leaving the bug open.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 507. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/106288371.
Appveyor build 388. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1535065-388.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 534. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/106464683.
Appveyor build 388. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1535065-388.

[SirVer (sirver)]

review: Approve

[GunChleoc (gunchleoc)]

@bunnybot merge

=== modified file 'src/editor/map_generator.cc'
--- src/editor/map_generator.cc	2016-01-28 05:24:34 +0000
+++ src/editor/map_generator.cc	2016-02-02 13:02:30 +0000
@@ -741,7 +741,7 @@
 	const std::string ai    = map_.get_scenario_player_ai(1);
 	map_.set_nrplayers(map_info_.numPlayers);
 	FindNodeSize functor(FindNodeSize::sizeBig);
-	Coords playerstart;
+	Coords playerstart(-1, -1);
 
 	// Build a basic structure how player start positions are placed
 	uint8_t line[3];
@@ -850,6 +850,21 @@
 			coords2 = playerstart;
 		}
 
+		// Remove coordinates if they are an illegal starting position.
+		if (coords2.x < 0 || coords2.x > map_.get_width() - 1 ||
+			 coords2.y < 0 || coords2.y > map_.get_height() - 1) {
+
+			// TODO(GunChleoc): If we check for buildcaps here, this always fails.
+			// we should bulldoze a bit of terrain to increase the chance that starting positions
+			// do not fail:
+			// map_.get_fcoords(coords2).field->nodecaps() & Widelands::BUILDCAPS_SIZEMASK
+			// != Widelands::BUILDCAPS_BIG)
+
+			log("WARNING: Player %u has no starting position - illegal coordinates (%d, %d).\n",
+				 n, coords2.x, coords2.y);
+			coords2 = Coords(-1, -1);
+		}
+
 		// Finally set the found starting position
 		map_.set_starting_pos(n, coords2);
 	}