Merge lp:~widelands-dev/widelands/bug-1718745-allows-seafaring into lp:widelands

[GunChleoc]

Split new function cleanup_portspaces() from allows_seafaring(), and refactored port spaces checks. allows_seafaring() is now const and used more widely. It also had a use-after-free error that has been fixed now. Exposed 3 new functions to LuaMap:

- get_allows_seafaring
- get_number_of_port_spaces
- set_port_space

[TiborB (tiborb95)]

So If I understand correctly, get_allows_seafaring will be LUA-only, so not usable by AI?

[GunChleoc (gunchleoc)]

allows_seafaring is already usable by the AI. And now you don't even need a mutable_map - a const map& will do.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2766. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/297640330.
Appveyor build 2578. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2578.

[GunChleoc (gunchleoc)]

New version - I decided to get a bit more fancy with the checks.

It would be good to have a replacement for the Buildcaps check, because it fails once a tree gets in the way. This is why we need the "force" parameter. Maybe check the terrain height in a certain radius?

review: Needs Resubmitting

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2774. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/298027831.
Appveyor build 2586. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2586.

[GunChleoc (gunchleoc)]

Well, the solution was staring me right in the face in the S2 map loading code that I had deleted... So, we now have a port space check that ignores immovables.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2779. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/298472005.
Appveyor build 2591. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2591.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2914. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/310475159.
Appveyor build 2723. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2723.

[TiborB (tiborb95)]

See comment in the code

[GunChleoc (gunchleoc)]

I completely removed the probability for the check - we can afford to run the function every 5 seconds.

review: Needs Resubmitting

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2934. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/312284476.
Appveyor build 2743. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2743.

[TiborB (tiborb95)]

But this might be too much - if we have 16 players, this could mean 3x per second....

[GunChleoc (gunchleoc)]

I have now added a static variable to control the check frequency. So, it's now globally checked every 5 seconds max.

[TiborB (tiborb95)]

See comment in diff

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2936. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/312567600.
Appveyor build 2745. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1718745_allows_seafaring-2745.

[GunChleoc (gunchleoc)]

Duh - thanks for having my back!

[TiborB (tiborb95)]

Also I cannot find an initialization of the variable :)

[GunChleoc (gunchleoc)]

Line 8 in the diff: uint32_t DefaultAI::last_seafaring_check_ = 0;

This has to be initialized in the .cc file - at least that has been my experience with static variables that are not constexpr.

[TiborB (tiborb95)]

But why not few lines below inside the constructor of DefaultAI object?

[GunChleoc (gunchleoc)]

Because the compiler will not allow initializing it within the constructor.

[TiborB (tiborb95)]

Because it is static? Do you mean it is shared among all AIs?

[GunChleoc (gunchleoc)]

Exactly, that is what static means - there is only 1 instance of the variable for all objects. This is also the reason why it can't be initialized in the constructor, because there are no individual copies of the variable, and it can be initialized only once.

Since whether a map is seafaring does not depend on which AI is doing the check, having a global check triggered about ever 5 seconds is sufficient. So, it won't cost extra load when we have lots of AIs, because only one of them will check for everybody.

I'm now wondering if that actually works, since seafaring_economy is not static. It does some check for if there is a ship being built somewhere else in the AI code - is this check actually needed now?

[TiborB (tiborb95)]

OK, I expected something like this. And this looks like very useful type of variable.

As for seafaring economy, this one can be as well changed into static I think... and/or renamed a bit...

[TiborB (tiborb95)]

Maybe you could rename swim_coords and visited_positions to more obvious names. Swim coords are coords reachable from previous ports and visited_positions are the one reachable from current port.
It is not easy to come up with short names, but something like this: previous_ports_reachable and current_port_reachable? I know they are long and dont sound good, but...

+ 1 comment in diff

[GunChleoc (gunchleoc)]

Excellent suggestions, I have implemented them. I also added a simple check for number of port spaces up front, this will speed things up even further on non-seafaring maps.

And ASan found a bug, that's now fixed too :)

review: Needs Resubmitting

[TiborB (tiborb95)]

I would rename the variable to reachable_from_previous_ports (plural), if there are multiple distinct waters it might scan multiple ports before running into one that can be connected to one of previous ports

[GunChleoc (gunchleoc)]

Done. Thanks for the review, this branch really would have been a mess without it!

@bunnybot merge

[bunnybot (widelandsofficial)]

Error merging this proposal:

Output:
stdout:

stderr:
bzr: ERROR: Parameter 'Merged lp:~widelands-dev/widelands/bug-1718745-allows-seafaring:\nSplit new function cleanup_portspaces() from allows_seafaring(), and refactored port spaces checks. allows_seafaring() is now const and used more widely. It also had a use-after-free error that has been fixed now. Exposed 3 new functions to LuaMap:\n\n\xc2\xa0\xc2\xa0\xc2\xa0\xc2\xa0- get_allows_seafaring\n\xc2\xa0\xc2\xa0\xc2\xa0\xc2\xa0- get_number_of_port_spaces\n\xc2\xa0\xc2\xa0\xc2\xa0\xc2\xa0- set_port_space' encoding is unsupported by ascii application locale.

[GunChleoc (gunchleoc)]

@bunnybot merge

=== modified file 'src/ai/defaultai.cc'
--- src/ai/defaultai.cc	2017-12-03 08:23:27 +0000
+++ src/ai/defaultai.cc	2017-12-11 06:56:51 +0000
@@ -82,6 +82,9 @@
 DefaultAI::WeakImpl DefaultAI::weak_impl;
 DefaultAI::VeryWeakImpl DefaultAI::very_weak_impl;
 
+uint32_t DefaultAI::last_seafaring_check_ = 0;
+bool DefaultAI::map_allows_seafaring_ = false;
+
 /// Constructor of DefaultAI
 DefaultAI::DefaultAI(Game& ggame, PlayerNumber const pid, Widelands::AiType const t)
    : ComputerPlayer(ggame, pid),
@@ -109,7 +112,6 @@
      enemysites_check_delay_(30),
      resource_necessity_water_needed_(false),
      highest_nonmil_prio_(0),
-     seafaring_economy(false),
      expedition_ship_(kNoShip) {
 
 	// Subscribe to NoteFieldPossession.
@@ -1951,11 +1953,9 @@
 
 	const Map& map = game().map();
 
-	if (gametime % 5 == 0) {
-		// TODO(unknown): Counting port spaces is very primitive way for this
-		// there should be better alternative f.e. like map::allows_seafaring()
-		// function but simplier
-		seafaring_economy = map.get_port_spaces().size() >= 2;
+	if (gametime > last_seafaring_check_ + 20000U) {
+		map_allows_seafaring_ = map.allows_seafaring();
+		last_seafaring_check_ = gametime;
 	}
 
 	for (int32_t i = 0; i < 4; ++i)
@@ -2663,7 +2663,7 @@
 						assert(!bo.is(BuildingAttribute::kShipyard));
 					} else if (bo.is(BuildingAttribute::kShipyard)) {
 						assert(bo.new_building == BuildingNecessity::kAllowed);
-						if (!seafaring_economy) {
+						if (!map_allows_seafaring_) {
 							continue;
 						}
 					} else {
@@ -2702,12 +2702,10 @@
 						    bf->unowned_mines_spots_nearby) {  // not close to mountains
 							prio -= std::abs(management_data.get_military_number_at(104)) / 5;
 						}
-					}
-
-					else if (bo.is(BuildingAttribute::kShipyard)) {
+					} else if (bo.is(BuildingAttribute::kShipyard)) {
 						// for now AI builds only one shipyard
 						assert(bo.total_count() == 0);
-						if (bf->open_water_nearby > 3 && seafaring_economy) {
+						if (bf->open_water_nearby > 3 && map_allows_seafaring_) {
 							prio += productionsites.size() * 5 +
 							        bf->open_water_nearby *
 							           std::abs(management_data.get_military_number_at(109)) / 10;
@@ -4253,7 +4251,7 @@
 		return BuildingNecessity::kForbidden;
 	}
 
-	if (bo.is(BuildingAttribute::kPort) && !seafaring_economy) {
+	if (bo.is(BuildingAttribute::kPort) && !map_allows_seafaring_) {
 		bo.new_building_overdue = 0;
 		bo.primary_priority = 0;
 		return BuildingNecessity::kForbidden;
@@ -4331,7 +4329,7 @@
 	}
 
 	// Perhaps buildings are not allowed because the map is no seafaring
-	if (purpose == PerfEvaluation::kForConstruction && !seafaring_economy &&
+	if (purpose == PerfEvaluation::kForConstruction && !map_allows_seafaring_ &&
 	    bo.is(BuildingAttribute::kNeedsSeafaring)) {
 		return BuildingNecessity::kForbidden;
 	}

=== modified file 'src/ai/defaultai.h'
--- src/ai/defaultai.h	2017-11-27 21:21:06 +0000
+++ src/ai/defaultai.h	2017-12-11 06:56:51 +0000
@@ -385,7 +385,9 @@
 
 	// seafaring related
 	enum { kReprioritize, kStopShipyard, kStapShipyard };
-	bool seafaring_economy;  // false by default, until first port space is found
+	static uint32_t last_seafaring_check_;
+	// False by default, until Map::allows_seafaring() is true
+	static bool map_allows_seafaring_;
 	uint32_t expedition_ship_;
 	uint32_t expedition_max_duration;
 	std::vector<int16_t> marine_task_queue;

=== modified file 'src/ai/defaultai_seafaring.cc'
--- src/ai/defaultai_seafaring.cc	2017-11-20 07:54:19 +0000
+++ src/ai/defaultai_seafaring.cc	2017-12-11 06:56:51 +0000
@@ -104,8 +104,7 @@
 // - build a ship
 // - start preparation for expedition
 bool DefaultAI::marine_main_decisions() {
-
-	if (!seafaring_economy) {
+	if (!map_allows_seafaring_) {
 		set_taskpool_task_time(kNever, SchedulerTaskId::KMarineDecisions);
 		return false;
 	}
@@ -226,9 +225,8 @@
 
 // This identifies ships that are waiting for command
 bool DefaultAI::check_ships(uint32_t const gametime) {
-
-	if (!seafaring_economy) {
-		set_taskpool_task_time(std::numeric_limits<int32_t>::max(), SchedulerTaskId::kCheckShips);
+	if (!map_allows_seafaring_) {
+		set_taskpool_task_time(kNever, SchedulerTaskId::kCheckShips);
 		return false;
 	}
 
@@ -402,7 +400,6 @@
 	if (type == NewShip::kBuilt) {
 		marine_task_queue.push_back(kStopShipyard);
 	} else {
-		seafaring_economy = true;
 		if (ship.state_is_expedition()) {
 			if (expedition_ship_ == kNoShip) {
 				// OK, this ship is in expedition

=== modified file 'src/editor/tools/set_port_space_tool.cc'
--- src/editor/tools/set_port_space_tool.cc	2017-09-15 19:18:08 +0000
+++ src/editor/tools/set_port_space_tool.cc	2017-12-11 06:56:51 +0000
@@ -65,7 +65,7 @@
 	do {
 		//  check if field is valid
 		if (port_tool_nodecaps(mr.location(), *map) != NodeCaps::CAPS_NONE) {
-			map->set_port_space(mr.location(), true);
+			map->set_port_space(world, mr.location(), true);
 			Area<FCoords> a(mr.location(), 0);
 			map->recalc_for_field_area(world, a);
 			++nr;
@@ -106,7 +106,7 @@
 	do {
 		//  check if field is valid
 		if (port_tool_nodecaps(mr.location(), *map)) {
-			map->set_port_space(mr.location(), false);
+			map->set_port_space(world, mr.location(), false);
 			Area<FCoords> a(mr.location(), 0);
 			map->recalc_for_field_area(world, a);
 			++nr;

=== modified file 'src/editor/ui_menus/main_menu_save_map.cc'
--- src/editor/ui_menus/main_menu_save_map.cc	2017-11-24 21:34:17 +0000
+++ src/editor/ui_menus/main_menu_save_map.cc	2017-12-11 06:56:51 +0000
@@ -280,6 +280,7 @@
 		   g_fs->create_sub_file_system(tmp_name, binary ? FileSystem::ZIP : FileSystem::DIR));
 
 		// Recompute seafaring tag
+		map->cleanup_port_spaces(egbase.world());
 		if (map->allows_seafaring()) {
 			map->add_tag("seafaring");
 		} else {

=== modified file 'src/logic/map.cc'
--- src/logic/map.cc	2017-11-05 19:59:33 +0000
+++ src/logic/map.cc	2017-12-11 06:56:51 +0000
@@ -546,7 +546,7 @@
 	}
 }
 
-NodeCaps Map::get_max_nodecaps(const World& world, const FCoords& fc) {
+NodeCaps Map::get_max_nodecaps(const World& world, const FCoords& fc) const {
 	NodeCaps caps = calc_nodecaps_pass1(world, fc, false);
 	caps = calc_nodecaps_pass2(world, fc, false, caps);
 	return caps;
@@ -937,7 +937,7 @@
 	f.field->caps = calc_nodecaps_pass1(world, f, true);
 }
 
-NodeCaps Map::calc_nodecaps_pass1(const World& world, const FCoords& f, bool consider_mobs) {
+NodeCaps Map::calc_nodecaps_pass1(const World& world, const FCoords& f, bool consider_mobs) const {
 	uint8_t caps = CAPS_NONE;
 
 	// 1a) Get all the neighbours to make life easier
@@ -1058,7 +1058,7 @@
 NodeCaps Map::calc_nodecaps_pass2(const World& world,
                                   const FCoords& f,
                                   bool consider_mobs,
-                                  NodeCaps initcaps) {
+                                  NodeCaps initcaps) const {
 	uint8_t caps = consider_mobs ? f.field->caps : static_cast<uint8_t>(initcaps);
 
 	// NOTE  This dependency on the bottom-right neighbour is the reason
@@ -1166,7 +1166,7 @@
                         bool avoidnature,
                         bool* ismine,
                         bool consider_mobs,
-                        NodeCaps initcaps) {
+                        NodeCaps initcaps) const {
 	if (consider_mobs) {
 		if (!(f.field->get_caps() & MOVECAPS_WALK))
 			return BaseImmovable::NONE;
@@ -1240,7 +1240,7 @@
  * The array \p dirs must have length \p length, where \p length is
  * the length of the cycle.
  */
-bool Map::is_cycle_connected(const FCoords& start, uint32_t length, const WalkingDir* dirs) {
+bool Map::is_cycle_connected(const FCoords& start, uint32_t length, const WalkingDir* dirs) const {
 	FCoords f = start;
 	bool prev_walkable = start.field->get_caps() & MOVECAPS_WALK;
 	uint32_t alternations = 0;
@@ -1304,18 +1304,27 @@
 	return portdock;
 }
 
+bool Map::is_port_space_allowed(const World& world, const FCoords& fc) const {
+	return (get_max_nodecaps(world, fc) & BUILDCAPS_SIZEMASK) == BUILDCAPS_BIG && !find_portdock(fc).empty();
+}
+
 /// \returns true, if Coordinates are in port space list
 bool Map::is_port_space(const Coords& c) const {
 	return port_spaces_.count(c);
 }
 
-/// Set or unset a space as port space
-void Map::set_port_space(Coords c, bool allowed) {
-	if (allowed) {
-		port_spaces_.insert(c);
+bool Map::set_port_space(const World& world, const Coords& c, bool set, bool force) {
+	bool success = false;
+	if (set) {
+		success = force || is_port_space_allowed(world, get_fcoords(c));
+		if (success) {
+			port_spaces_.insert(c);
+		}
 	} else {
 		port_spaces_.erase(c);
+		success = true;
 	}
+	return success;
 }
 
 /**
@@ -1950,60 +1959,67 @@
 			check_neighbour_heights(n[i], area);
 }
 
-/*
-===========
-Map::allows_seafaring()
-
-This function checks if there are two ports that are reachable
-for each other - then the map is seafaring.
-=============
-*/
-bool Map::allows_seafaring() {
-	Map::PortSpacesSet port_spaces = get_port_spaces();
-	std::vector<Coords> portdocks;
-	std::set<Coords> swim_coords;
-
-	for (const Coords& c : port_spaces) {
-		std::queue<Coords> q_positions;
-		std::set<Coords> visited_positions;
+bool Map::allows_seafaring() const {
+
+	// There need to be at least 2 port spaces for seafaring to make sense
+	if (get_port_spaces().size() < 2) {
+		return false;
+	}
+
+	std::set<Coords> reachable_from_previous_ports;
+
+	for (const Coords& c : get_port_spaces()) {
+		std::queue<Coords> positions_to_check;
+		std::set<Coords> reachable_from_current_port;
 		FCoords fc = get_fcoords(c);
-		portdocks = find_portdock(fc);
-
-		/* remove the port space if it is not longer valid port space */
-		if ((fc.field->get_caps() & BUILDCAPS_SIZEMASK) != BUILDCAPS_BIG || portdocks.empty()) {
-			set_port_space(c, false);
-			continue;
-		}
-
-		for (const Coords& portdock : portdocks) {
-			visited_positions.insert(portdock);
-			q_positions.push(portdock);
-		}
-
-		while (!q_positions.empty()) {
-			const Coords& swim_coord = q_positions.front();
-			q_positions.pop();
+
+		// Get portdock slots for this port
+		for (const Coords& portdock : find_portdock(fc)) {
+			reachable_from_current_port.insert(portdock);
+			positions_to_check.push(portdock);
+		}
+
+		// Pick up all positions that can be reached from the current port
+		while (!positions_to_check.empty()) {
+			// Take a copy, because we'll pop it
+			const Coords current_position = positions_to_check.front();
+			positions_to_check.pop();
+
+			// Found one
+			if (reachable_from_previous_ports.count(current_position) > 0) {
+				return true;
+			}
+
+			// Adding the neighbors to the list
 			for (uint8_t i = 1; i <= 6; ++i) {
 				FCoords neighbour;
-				get_neighbour(get_fcoords(swim_coord), i, &neighbour);
+				get_neighbour(get_fcoords(current_position), i, &neighbour);
 				if ((neighbour.field->get_caps() & (MOVECAPS_SWIM | MOVECAPS_WALK)) == MOVECAPS_SWIM) {
-					if (visited_positions.count(neighbour) == 0) {
-						visited_positions.insert(neighbour);
-						q_positions.push(neighbour);
+					if (reachable_from_current_port.count(neighbour) == 0) {
+						reachable_from_current_port.insert(neighbour);
+						positions_to_check.push(neighbour);
 					}
 				}
 			}
 		}
 
-		for (const Coords& swim_coord : visited_positions)
-			if (swim_coords.count(swim_coord) == 0)
-				swim_coords.insert(swim_coord);
-			else
-				return true;
+		// Couldn't connect to another port, so we add our reachable nodes to the list
+		for (const Coords& reachable_coord : reachable_from_current_port) {
+			reachable_from_previous_ports.insert(reachable_coord);
+		}
 	}
 	return false;
 }
 
+void Map::cleanup_port_spaces(const World& world) {
+	for (const Coords& c : get_port_spaces()) {
+		if (!is_port_space_allowed(world, get_fcoords(c))) {
+			set_port_space(world, c, false);
+			continue;
+		}
+	}
+}
+
 bool Map::has_artifacts() {
 	for (MapIndex i = 0; i < max_index(); ++i) {
 		if (upcast(Immovable, immovable, fields_[i].get_immovable())) {

=== modified file 'src/logic/map.h'
--- src/logic/map.h	2017-11-26 14:44:23 +0000
+++ src/logic/map.h	2017-12-11 06:56:51 +0000
@@ -270,8 +270,8 @@
 	void set_scenario_player_ai(PlayerNumber, const std::string&);
 	void set_scenario_player_closeable(PlayerNumber, bool);
 
-	/// \returns the maximum theoretical possible nodecaps (no blocking bobs, etc.)
-	NodeCaps get_max_nodecaps(const World& world, const FCoords&);
+	/// \returns the maximum theoretical possible nodecaps (no blocking bobs, immovables etc.)
+	NodeCaps get_max_nodecaps(const World& world, const FCoords&) const;
 
 	BaseImmovable* get_immovable(const Coords&) const;
 	uint32_t find_bobs(const Area<FCoords>,
@@ -438,14 +438,26 @@
 	/// Translate the whole map so that the given point becomes the new origin.
 	void set_origin(const Coords&);
 
-	/// Port space specific functions
+	// Port space specific functions
+
+	/// Checks whether the maximum theoretical possible NodeCap of the field is big,
+	/// and there is room for a port space
+	bool is_port_space_allowed(const World& world, const FCoords& fc) const;
 	bool is_port_space(const Coords& c) const;
-	void set_port_space(Coords c, bool allowed);
+
+	/// If 'set', set the space at 'c' as port space, otherwise unset.
+	/// 'force' sets the port space even if it isn't viable, and is to be used for map loading only.
+	/// Returns whether the port space was set/unset successfully.
+	bool set_port_space(const World& world, const Widelands::Coords& c, bool set, bool force = false);
 	const PortSpacesSet& get_port_spaces() const {
 		return port_spaces_;
 	}
 	std::vector<Coords> find_portdock(const Widelands::Coords& c) const;
-	bool allows_seafaring();
+
+	/// Check whether there are at least 2 port spaces that can be reached from each other by water
+	bool allows_seafaring() const;
+	/// Remove all port spaces that are not valid (Buildcap < big or not enough space for a portdock).
+	void cleanup_port_spaces(const World& world);
 
 	/// Checks whether there are any artifacts on the map
 	bool has_artifacts();
@@ -458,19 +470,19 @@
 	void recalc_brightness(const FCoords&);
 	void recalc_nodecaps_pass1(const World& world, const FCoords&);
 	void recalc_nodecaps_pass2(const World& world, const FCoords& f);
-	NodeCaps calc_nodecaps_pass1(const World& world, const FCoords&, bool consider_mobs = true);
+	NodeCaps calc_nodecaps_pass1(const World& world, const FCoords&, bool consider_mobs = true) const;
 	NodeCaps calc_nodecaps_pass2(const World& world,
 	                             const FCoords&,
 	                             bool consider_mobs = true,
-	                             NodeCaps initcaps = CAPS_NONE);
+	                             NodeCaps initcaps = CAPS_NONE) const;
 	void check_neighbour_heights(FCoords, uint32_t& radius);
 	int calc_buildsize(const World& world,
 	                   const FCoords& f,
 	                   bool avoidnature,
 	                   bool* ismine = nullptr,
 	                   bool consider_mobs = true,
-	                   NodeCaps initcaps = CAPS_NONE);
-	bool is_cycle_connected(const FCoords& start, uint32_t length, const WalkingDir* dirs);
+	                   NodeCaps initcaps = CAPS_NONE) const;
+	bool is_cycle_connected(const FCoords& start, uint32_t length, const WalkingDir* dirs) const;
 	template <typename functorT>
 	void find_reachable(const Area<FCoords>&, const CheckStep&, functorT&) const;
 	template <typename functorT> void find(const Area<FCoords>&, functorT&) const;

=== modified file 'src/logic/map_objects/tribes/production_program.cc'
--- src/logic/map_objects/tribes/production_program.cc	2017-11-24 21:34:17 +0000
+++ src/logic/map_objects/tribes/production_program.cc	2017-12-11 06:56:51 +0000
@@ -742,9 +742,9 @@
 void ProductionProgram::ActCheckMap::execute(Game& game, ProductionSite& ps) const {
 	switch (feature_) {
 	case SEAFARING: {
-		if (game.map().get_port_spaces().size() > 1)
+		if (game.map().allows_seafaring()) {
 			return ps.program_step(game, 0);
-		else {
+		} else {
 			ps.set_production_result(_("No use for ships on this map!"));
 			return ps.program_end(game, Failed);
 		}

=== modified file 'src/map_io/map_port_spaces_packet.cc'
--- src/map_io/map_port_spaces_packet.cc	2017-08-19 22:22:20 +0000
+++ src/map_io/map_port_spaces_packet.cc	2017-12-11 06:56:51 +0000
@@ -51,8 +51,7 @@
 
 			Section& s2 = prof.get_safe_section("port_spaces");
 			for (uint16_t i = 0; i < num; ++i) {
-				map->set_port_space(
-				   get_safe_coords(std::to_string(static_cast<unsigned int>(i)), ext, &s2), true);
+				map->set_port_space(egbase.world(), get_safe_coords(std::to_string(static_cast<unsigned int>(i)), ext, &s2), true, true);
 			}
 		} else {
 			throw UnhandledVersionError("MapPortSpacesPacket", packet_version, kCurrentPacketVersion);
@@ -70,8 +69,7 @@
 	s1.set_int("packet_version", kCurrentPacketVersion);
 
 	const Map& map = egbase.map();
-	const uint16_t num = map.get_port_spaces().size();
-	s1.set_int("number_of_port_spaces", num);
+	s1.set_int("number_of_port_spaces", map.get_port_spaces().size());
 
 	Section& s2 = prof.create_section("port_spaces");
 	int i = 0;

=== modified file 'src/map_io/s2map.cc'
--- src/map_io/s2map.cc	2017-08-19 13:02:14 +0000
+++ src/map_io/s2map.cc	2017-12-11 06:56:51 +0000
@@ -392,7 +392,7 @@
 
 	map_.recalc_whole_map(egbase.world());
 
-	postload_fix_conversion(egbase);
+	postload_set_port_spaces(egbase.world());
 
 	set_state(STATE_LOADED);
 
@@ -473,8 +473,9 @@
 		for (int16_t x = 0; x < mapwidth; ++x, ++f, ++pc) {
 			uint8_t c = *pc;
 			// Harbour buildspace & textures - Information taken from:
-			if (c & 0x40)
-				map_.set_port_space(Widelands::Coords(x, y), true);
+			if (c & 0x40) {
+				port_spaces_to_set_.insert(Widelands::Coords(x, y));
+			}
 			f->set_terrain_d(terrain_converter.lookup(worldtype_, c & 0x1f));
 		}
 
@@ -490,8 +491,9 @@
 			uint8_t c = *pc;
 			// Harbour buildspace & textures - Information taken from:
 			// http://bazaar.launchpad.net/~xaser/s25rttr/s25edit/view/head:/WLD_reference.txt
-			if (c & 0x40)
-				map_.set_port_space(Widelands::Coords(x, y), true);
+			if (c & 0x40) {
+				port_spaces_to_set_.insert(Widelands::Coords(x, y));
+			}
 			f->set_terrain_r(terrain_converter.lookup(worldtype_, c & 0x1f));
 		}
 
@@ -1041,41 +1043,25 @@
 	}
 }
 
-/// Try to fix data, which is incompatible between S2 and Widelands
-void S2MapLoader::postload_fix_conversion(Widelands::EditorGameBase& egbase) {
-
-	/*
-	 * 1: Try to fix port spaces
-	 */
-	const Widelands::Map::PortSpacesSet ports(map_.get_port_spaces());
-	const Widelands::World& world = egbase.world();
-
-	// Check if port spaces are valid
-	for (const Widelands::Coords& c : ports) {
-		Widelands::FCoords fc = map_.get_fcoords(c);
-		Widelands::NodeCaps nc = map_.get_max_nodecaps(world, fc);
-		if ((nc & Widelands::BUILDCAPS_SIZEMASK) != Widelands::BUILDCAPS_BIG ||
-		    map_.find_portdock(fc).empty()) {
-			log("Invalid port build space: ");
-			map_.set_port_space(c, false);
-
-			bool fixed = false;
+/// Try to fix data which is incompatible between S2 and Widelands.
+/// This is only the port space locations.
+void S2MapLoader::postload_set_port_spaces(const Widelands::World& world) {
+	// Set port spaces near desired locations if possible
+	for (const Widelands::Coords& coords : port_spaces_to_set_) {
+		bool was_set = map_.set_port_space(world, coords, true);
+		const Widelands::FCoords fc = map_.get_fcoords(coords);
+		if (!was_set) {
+			// Try to set a port space at alternative location
 			Widelands::MapRegion<Widelands::Area<Widelands::FCoords>> mr(
-			   map_, Widelands::Area<Widelands::FCoords>(fc, 3));
+				map_, Widelands::Area<Widelands::FCoords>(fc, 3));
 			do {
-				// Check whether the maximum theoretical possible NodeCap of the field is big + port
-				Widelands::NodeCaps nc2 =
-				   map_.get_max_nodecaps(world, const_cast<Widelands::FCoords&>(mr.location()));
-				if ((nc2 & Widelands::BUILDCAPS_SIZEMASK) == Widelands::BUILDCAPS_BIG &&
-				    (!map_.find_portdock(mr.location()).empty())) {
-					map_.set_port_space(Widelands::Coords(mr.location().x, mr.location().y), true);
-					fixed = true;
-				}
-			} while (mr.advance(map_) && !fixed);
-			if (!fixed) {
-				log("FAILED! No alternative port buildspace for (%i, %i) found!\n", fc.x, fc.y);
-			} else
-				log("Fixed!\n");
+				was_set = map_.set_port_space(world, Widelands::Coords(mr.location().x, mr.location().y), true);
+			} while (!was_set && mr.advance(map_));
+		}
+		if (!was_set) {
+			log("FAILED! No port buildspace for (%i, %i) found!\n", fc.x, fc.y);
+		} else {
+			log("SUCCESS! Port buildspace set for (%i, %i) \n", fc.x, fc.y);
 		}
 	}
 }

=== modified file 'src/map_io/s2map.h'
--- src/map_io/s2map.h	2017-06-24 10:38:19 +0000
+++ src/map_io/s2map.h	2017-12-11 06:56:51 +0000
@@ -42,10 +42,11 @@
 private:
 	const std::string filename_;
 	WorldType worldtype_;
+	std::set<Widelands::Coords> port_spaces_to_set_;
 
 	void load_s2mf_header(FileRead&);
 	void load_s2mf(Widelands::EditorGameBase&);
-	void postload_fix_conversion(Widelands::EditorGameBase&);
+	void postload_set_port_spaces(const Widelands::World& world);
 };
 
 #endif  // end of include guard: WL_MAP_IO_S2MAP_H

=== modified file 'src/scripting/lua_map.cc'
--- src/scripting/lua_map.cc	2017-11-23 23:32:40 +0000
+++ src/scripting/lua_map.cc	2017-12-11 06:56:51 +0000
@@ -1138,7 +1138,7 @@
 
 .. class:: Map
 
-   Access to the map and it's objects. You cannot instantiate this directly,
+   Access to the map and its objects. You cannot instantiate this directly,
    instead access it via :attr:`wl.Game.map`.
 */
 const char LuaMap::className[] = "Map";
@@ -1146,9 +1146,12 @@
    METHOD(LuaMap, place_immovable),
    METHOD(LuaMap, get_field),
    METHOD(LuaMap, recalculate),
+   METHOD(LuaMap, set_port_space),
    {nullptr, nullptr},
 };
 const PropertyType<LuaMap> LuaMap::Properties[] = {
+   PROP_RO(LuaMap, allows_seafaring),
+   PROP_RO(LuaMap, number_of_port_spaces),
    PROP_RO(LuaMap, width),
    PROP_RO(LuaMap, height),
    PROP_RO(LuaMap, player_slots),
@@ -1167,6 +1170,31 @@
  ==========================================================
  */
 /* RST
+   .. attribute:: allows_seafaring
+
+      (RO) Whether the map currently allows seafaring. This will calculate a path between port spaces,
+		so it's more accurate but less efficient than :any:`number_of_port_spaces`.
+
+		:returns: True if there are at least two port spaces that can be reached from each other.
+*/
+int LuaMap::get_allows_seafaring(lua_State* L) {
+	lua_pushboolean(L, get_egbase(L).map().allows_seafaring());
+	return 1;
+}
+/* RST
+   .. attribute:: number_of_port_spaces
+
+      (RO) The amount of port spaces on the map. If this is >= 2, one can assume that the map
+      allows seafaring. This is checked very quickly and is more efficient than :any:`allows_seafaring`,
+      but it won't detect whether the port spaces can be reached from each other, so it's less accurate.
+
+      :returns: An integer with the number of port spaces.
+*/
+int LuaMap::get_number_of_port_spaces(lua_State* L) {
+	lua_pushuint32(L, get_egbase(L).map().get_port_spaces().size());
+	return 1;
+}
+/* RST
    .. attribute:: width
 
       (RO) The width of the map in fields.
@@ -1297,6 +1325,31 @@
 	return 0;
 }
 
+/* RST
+   .. method:: set_port_space(x, y, allowed)
+
+      Sets whether a port space is allowed at the coordinates (x, y).
+      Returns false if the port space couldn't be set.
+
+      :arg x: The x coordinate of the port space to set/unset.
+      :type x: :class:`int`
+      :arg y: The y coordinate of the port space to set/unset.
+      :type y: :class:`int`
+      :arg allowed: Whether building a port will be allowed here.
+      :type allowed: :class:`bool`
+
+      :returns: :const:`true` on success, or :const:`false` otherwise
+      :rtype: :class:`bool`
+*/
+int LuaMap::set_port_space(lua_State* L) {
+	const int x = luaL_checkint32(L, 2);
+	const int y = luaL_checkint32(L, 3);
+	const bool allowed = luaL_checkboolean(L, 4);
+	const bool success = get_egbase(L).mutable_map()->set_port_space(get_egbase(L).world(), Widelands::Coords(x, y), allowed);
+	lua_pushboolean(L, success);
+	return 1;
+}
+
 /*
  ==========================================================
  C METHODS

=== modified file 'src/scripting/lua_map.h'
--- src/scripting/lua_map.h	2017-11-28 08:57:52 +0000
+++ src/scripting/lua_map.h	2017-12-11 06:56:51 +0000
@@ -86,6 +86,8 @@
 	/*
 	 * Properties
 	 */
+	int get_allows_seafaring(lua_State*);
+	int get_number_of_port_spaces(lua_State*);
 	int get_width(lua_State*);
 	int get_height(lua_State*);
 	int get_player_slots(lua_State*);
@@ -96,6 +98,7 @@
 	int place_immovable(lua_State*);
 	int get_field(lua_State*);
 	int recalculate(lua_State*);
+	int set_port_space(lua_State*);
 
 	/*
 	 * C methods

=== modified file 'src/wui/building_statistics_menu.cc'
--- src/wui/building_statistics_menu.cc	2017-11-11 10:08:54 +0000
+++ src/wui/building_statistics_menu.cc	2017-12-11 06:56:51 +0000
@@ -138,8 +138,8 @@
 	               g_gr->images().get("images/wui/fieldaction/menu_tab_buildmine.png"),
 	               tabs_[BuildingTab::Mines], _("Mines"));
 
-	// Hide the ports tab for non-seafaring maps
-	if (iplayer().game().map().get_port_spaces().size() > 1) {
+	// Only show the ports tab for seafaring maps
+	if (iplayer().game().map().allows_seafaring()) {
 		tab_panel_.add("building_stats_ports",
 		               g_gr->images().get("images/wui/fieldaction/menu_tab_buildport.png"),
 		               tabs_[BuildingTab::Ports], _("Ports"));

=== modified file 'src/wui/fieldaction.cc'
--- src/wui/fieldaction.cc	2017-11-27 08:21:32 +0000
+++ src/wui/fieldaction.cc	2017-12-11 06:56:51 +0000
@@ -416,11 +416,13 @@
 		//  Some building types cannot be built (i.e. construction site) and not
 		//  allowed buildings.
 		if (dynamic_cast<const Game*>(&ibase().egbase())) {
-			if (!building_descr->is_buildable() || !player_->is_building_type_allowed(building_index))
+			if (!building_descr->is_buildable() || !player_->is_building_type_allowed(building_index)) {
 				continue;
+			}
 			if (building_descr->needs_seafaring() &&
-			    ibase().egbase().map().get_port_spaces().size() < 2)
+				 !ibase().egbase().map().allows_seafaring()) {
 				continue;
+			}
 		} else if (!building_descr->is_buildable() && !building_descr->is_enhanced())
 			continue;
 

=== modified file 'test/maps/expedition.wmf/scripting/init.lua'
--- test/maps/expedition.wmf/scripting/init.lua	2017-01-17 20:57:35 +0000
+++ test/maps/expedition.wmf/scripting/init.lua	2017-12-11 06:56:51 +0000
@@ -2,6 +2,7 @@
 include "scripting/coroutine.lua"
 include "scripting/infrastructure.lua"
 include "scripting/ui.lua"
+include "test/scripting/stable_save.lua"
 
 -- This is a test case for bug 1234058: there is constant demand for logs,
 -- so the expedition initially never got any.
@@ -53,18 +54,6 @@
 first_ship = nil
 second_ship = nil
 
--- Save the game so that reloading does not skip
-function stable_save(safename)
-   local old_speed = game.desired_speed
-   game.desired_speed = 1000
-   sleep(100)
-   game:save(safename)
-   game.desired_speed = 1000
-   sleep(2000)  -- Give the loaded game a chance to catch up
-   game.desired_speed = old_speed
-   sleep(1000)
-end
-
 function click_on_ship(which_ship)
    local mv = wl.ui.MapView()
    for x=0,map.width-1 do
@@ -211,7 +200,7 @@
    game.desired_speed = 10 * 1000
    sleep(10000)
 
-   stable_save("ready_to_sail")
+   stable_save(game, "ready_to_sail")
 
    sleep(10000)
    assert_equal(1, p1:get_workers("barbarians_builder"))
@@ -255,7 +244,7 @@
    assert_equal("ccw",expedition_ship.island_explore_direction)
    sleep(6000)
 
-   stable_save("sailing")
+   stable_save(game, "sailing")
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
    cancel_expedition_in_shipwindow(expedition_ship)
@@ -289,7 +278,7 @@
    sleep(500)
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
-   stable_save("reached_port_space")
+   stable_save(game, "reached_port_space")
    sleep(5000)
    ships = p1:get_ships()
    --ships table should contain 1-2 items (1-2 ships)
@@ -342,7 +331,7 @@
    sleep(500)
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
-   stable_save("port_done")
+   stable_save(game, "port_done")
    game.desired_speed = 25 * 1000
 
    -- build a lumberjack and see if the ship starts transporting stuff

=== modified file 'test/maps/expedition.wmf/scripting/test_cancel_when_port_space_was_reached_two_ships.lua'
--- test/maps/expedition.wmf/scripting/test_cancel_when_port_space_was_reached_two_ships.lua	2015-11-04 16:48:56 +0000
+++ test/maps/expedition.wmf/scripting/test_cancel_when_port_space_was_reached_two_ships.lua	2017-12-11 06:56:51 +0000
@@ -27,7 +27,7 @@
    sleep(500)
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
-   stable_save("reached_port_space")
+   stable_save(game, "reached_port_space")
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
    cancel_expedition_in_shipwindow(expedition_ship)

=== modified file 'test/maps/expedition.wmf/scripting/test_ship_movement_controls.lua'
--- test/maps/expedition.wmf/scripting/test_ship_movement_controls.lua	2015-12-03 21:12:12 +0000
+++ test/maps/expedition.wmf/scripting/test_ship_movement_controls.lua	2017-12-11 06:56:51 +0000
@@ -105,7 +105,7 @@
    sleep(500)
    assert_equal("exp_colonizing", ships[1].state)
    sleep(15000)
-   stable_save("port_in_constr")
+   stable_save(game, "port_in_constr")
 
    -- while unfinished yet, removing it
    new_port=map:get_field(16,2).immovable

=== modified file 'test/maps/expedition.wmf/scripting/test_starting_wait_a_while_cancel.lua'
--- test/maps/expedition.wmf/scripting/test_starting_wait_a_while_cancel.lua	2015-10-31 12:11:44 +0000
+++ test/maps/expedition.wmf/scripting/test_starting_wait_a_while_cancel.lua	2017-12-11 06:56:51 +0000
@@ -6,7 +6,7 @@
    -- gives the builder enough time to walk over.
    port:start_expedition()
    sleep(50000)
-   stable_save("cancel_in_port")
+   stable_save(game, "cancel_in_port")
    assert_equal(1, p1:get_workers("barbarians_builder"))
 
    port:cancel_expedition()

=== modified file 'test/maps/ship_transportation.wmf/scripting/init.lua'
--- test/maps/ship_transportation.wmf/scripting/init.lua	2016-01-28 05:24:34 +0000
+++ test/maps/ship_transportation.wmf/scripting/init.lua	2017-12-11 06:56:51 +0000
@@ -1,6 +1,7 @@
 include "scripting/lunit.lua"
 include "scripting/coroutine.lua"
 include "scripting/infrastructure.lua"
+include "test/scripting/stable_save.lua"
 
 game = wl.Game()
 map = game.map
@@ -49,15 +50,3 @@
 
 ship = p1:place_ship(map:get_field(10, 10))
 p1.see_all = true
-
--- Save the game so that reloading does not skip
-function stable_save(safename)
-   local old_speed = game.desired_speed
-   game.desired_speed = 1000
-   sleep(100)
-   game:save(safename)
-   game.desired_speed = 1000
-   sleep(2000)  -- Give the loaded game a chance to catch up
-   game.desired_speed = old_speed
-   sleep(1000)
-end

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_many_ships.lua'
--- test/maps/ship_transportation.wmf/scripting/test_many_ships.lua	2016-01-31 21:03:15 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_many_ships.lua	2017-12-11 06:56:51 +0000
@@ -22,7 +22,7 @@
         sleep(2000)
         end
 
-  stable_save("20_ships")
+  stable_save(game, "20_ships")
 
    i = 0
    while i < 10 do

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_ware_in_portdock.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_ware_in_portdock.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_ware_in_portdock.lua	2017-12-11 06:56:51 +0000
@@ -24,7 +24,7 @@
 
    sleep(100)
    assert_equal(p1:get_wares("blackwood"), 0)
-   stable_save("port1_just_removed")
+   stable_save(game, "port1_just_removed")
 
    sleep(5000)
 

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_worker_in_portdock.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_worker_in_portdock.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_first_port_with_worker_in_portdock.lua	2017-12-11 06:56:51 +0000
@@ -20,7 +20,7 @@
 
    southern_port():remove()
    sleep(100)
-   stable_save("port1_just_removed")
+   stable_save(game, "port1_just_removed")
 
    -- Wait till the worker runs to the warehouse.
    while wh:get_workers("barbarians_builder") == 0 do

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua	2015-11-21 11:47:05 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua	2017-12-11 06:56:51 +0000
@@ -29,7 +29,7 @@
 
    local flag_oversea = northern_port().flag
 
-   stable_save("restored_port")
+   stable_save(game, "restored_port")
 
    -- remove the portdock while the blackwood is in transit.
    north_port_portdock=northern_port().portdock

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_ware_in_transit.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_ware_in_transit.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_ware_in_transit.lua	2017-12-11 06:56:51 +0000
@@ -28,7 +28,7 @@
    southern_port():remove()
    sleep(1000)
 
-   stable_save("no_ports")
+   stable_save(game, "no_ports")
 
    -- There are no more ports, therefore also no fleet. The wares on the ship
    -- are not accessible and should therefore not show up in the stock anymore.

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua	2017-12-11 06:56:51 +0000
@@ -28,7 +28,7 @@
    southern_port():remove()
    sleep(1000)
 
-   stable_save("no_ports")
+   stable_save(game, "no_ports")
 
    -- There are no more ports, therefore also no fleet. The workers on the ship
    -- are not accessible and should therefore not show up in the stock anymore.

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_ware_in_portdock.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_ware_in_portdock.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_ware_in_portdock.lua	2017-12-11 06:56:51 +0000
@@ -29,7 +29,7 @@
    northern_port():remove()
    sleep(100)
 
-   stable_save("ware_in_portdock")
+   stable_save(game, "ware_in_portdock")
 
    --ship has to get to the place of former upper port and then return back to the bottom port
    sleep(30000)

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_worker_in_portdock.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_worker_in_portdock.lua	2015-11-21 11:47:05 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_second_port_with_worker_in_portdock.lua	2017-12-11 06:56:51 +0000
@@ -38,7 +38,7 @@
 
    sleep(100)
 
-   stable_save("worker_in_portdock")
+   stable_save(game, "worker_in_portdock")
 
    -- Wait till a ship unloads a worked at the souther port
    while (ship:get_workers("builder") == 1) do

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua	2017-12-11 06:56:51 +0000
@@ -16,10 +16,10 @@
    sleep(12000)
 
    -- remove the ship while the ware is in transit.
-   stable_save("0_before_removing_ship")
+   stable_save(game, "0_before_removing_ship")
    ship:remove()
    sleep(1000)
-   stable_save("1_no_more_ship")
+   stable_save(game, "1_no_more_ship")
 
    assert_equal(1, p1:get_wares("blackwood"))
    -- It is not in the port (still in the dock)

=== modified file 'test/maps/ship_transportation.wmf/scripting/test_rip_ship_while_transporting_ware.lua'
--- test/maps/ship_transportation.wmf/scripting/test_rip_ship_while_transporting_ware.lua	2015-11-03 18:52:00 +0000
+++ test/maps/ship_transportation.wmf/scripting/test_rip_ship_while_transporting_ware.lua	2017-12-11 06:56:51 +0000
@@ -18,10 +18,10 @@
    assert_equal(p1:get_wares("blackwood"), 1)
 
    -- remove the ship while the ware is in transit.
-   stable_save("0_before_removing_ship")
+   stable_save(game, "0_before_removing_ship")
    ship:remove()
    sleep(1000)
-   stable_save("1_no_more_ship")
+   stable_save(game, "1_no_more_ship")
 
    -- No more blackwood.
    assert_equal(p1:get_wares("blackwood"), 0)

=== added directory 'test/maps/two_ponds.wmf'
=== added directory 'test/maps/two_ponds.wmf/binary'
=== added file 'test/maps/two_ponds.wmf/binary/heights'
Binary files test/maps/two_ponds.wmf/binary/heights	1970-01-01 00:00:00 +0000 and test/maps/two_ponds.wmf/binary/heights	2017-12-11 06:56:51 +0000 differ
=== added file 'test/maps/two_ponds.wmf/binary/mapobjects'
Binary files test/maps/two_ponds.wmf/binary/mapobjects	1970-01-01 00:00:00 +0000 and test/maps/two_ponds.wmf/binary/mapobjects	2017-12-11 06:56:51 +0000 differ
=== added file 'test/maps/two_ponds.wmf/binary/resource'
Binary files test/maps/two_ponds.wmf/binary/resource	1970-01-01 00:00:00 +0000 and test/maps/two_ponds.wmf/binary/resource	2017-12-11 06:56:51 +0000 differ
=== added file 'test/maps/two_ponds.wmf/binary/terrain'
Binary files test/maps/two_ponds.wmf/binary/terrain	1970-01-01 00:00:00 +0000 and test/maps/two_ponds.wmf/binary/terrain	2017-12-11 06:56:51 +0000 differ
=== added file 'test/maps/two_ponds.wmf/elemental'
--- test/maps/two_ponds.wmf/elemental	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/elemental	2017-12-11 06:56:51 +0000
@@ -0,0 +1,12 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+packet_version="1"
+map_w="64"
+map_h="64"
+nr_players="1"
+name="Two Ponds"
+author="Unknown"
+descr="Test map with 2 bodies of water, with 1 port space each."
+hint=
+tags=

=== added file 'test/maps/two_ponds.wmf/minimap.png'
Binary files test/maps/two_ponds.wmf/minimap.png	1970-01-01 00:00:00 +0000 and test/maps/two_ponds.wmf/minimap.png	2017-12-11 06:56:51 +0000 differ
=== added file 'test/maps/two_ponds.wmf/objective'
--- test/maps/two_ponds.wmf/objective	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/objective	2017-12-11 06:56:51 +0000
@@ -0,0 +1,4 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+packet_version="2"

=== added file 'test/maps/two_ponds.wmf/player_names'
--- test/maps/two_ponds.wmf/player_names	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/player_names	2017-12-11 06:56:51 +0000
@@ -0,0 +1,10 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+packet_version="2"
+
+[player_1]
+name="Cluicheadair 1"
+tribe="barbarians"
+ai=
+closeable="false"

=== added file 'test/maps/two_ponds.wmf/player_position'
--- test/maps/two_ponds.wmf/player_position	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/player_position	2017-12-11 06:56:51 +0000
@@ -0,0 +1,5 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+packet_version="2"
+player_1="5 58"

=== added file 'test/maps/two_ponds.wmf/port_spaces'
--- test/maps/two_ponds.wmf/port_spaces	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/port_spaces	2017-12-11 06:56:51 +0000
@@ -0,0 +1,9 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+packet_version="1"
+number_of_port_spaces="2"
+
+[port_spaces]
+0="7 2"
+1="12 24"

=== added directory 'test/maps/two_ponds.wmf/scripting'
=== added file 'test/maps/two_ponds.wmf/scripting/init.lua'
--- test/maps/two_ponds.wmf/scripting/init.lua	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/scripting/init.lua	2017-12-11 06:56:51 +0000
@@ -0,0 +1,6 @@
+include "scripting/coroutine.lua"
+include "scripting/lunit.lua"
+include "test/scripting/stable_save.lua"
+
+game = wl.Game()
+map = game.map

=== added file 'test/maps/two_ponds.wmf/scripting/test_seafaring.lua'
--- test/maps/two_ponds.wmf/scripting/test_seafaring.lua	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/scripting/test_seafaring.lua	2017-12-11 06:56:51 +0000
@@ -0,0 +1,33 @@
+run(function()
+   -- The map in its initial state has 2 unconnected port spaces, so it should
+   -- not allow seafaring. One of the port spaces has trees on top of it.
+   assert_equal(2, map.number_of_port_spaces)
+   assert_equal(false, map.allows_seafaring)
+
+   -- Now try to add a port space on a medium buildcap, it should fail
+   assert_equal(false, map:set_port_space(11, 9, true))
+   assert_equal(2, map.number_of_port_spaces)
+   assert_equal(false, map.allows_seafaring)
+
+   -- Now try to add a port space away from water, it should fail
+   assert_equal(false, map:set_port_space(18, 9, true))
+   assert_equal(2, map.number_of_port_spaces)
+   assert_equal(false, map.allows_seafaring)
+
+   -- Now add a connecting port space - it should succeed and we should have seafaring then
+   assert_equal(true, map:set_port_space(0, 2, true))
+   assert_equal(3, map.number_of_port_spaces)
+   assert_equal(true, map.allows_seafaring)
+
+   stable_save(game, "port_spaces")
+   assert_equal(3, map.number_of_port_spaces)
+   assert_equal(true, map.allows_seafaring)
+
+  -- Remove the port space again
+   assert_equal(true, map:set_port_space(0, 2, false))
+   assert_equal(2, map.number_of_port_spaces)
+   assert_equal(false, map.allows_seafaring)
+
+   print("# All Tests passed.")
+   wl.ui.MapView():close()
+end)

=== added file 'test/maps/two_ponds.wmf/version'
--- test/maps/two_ponds.wmf/version	1970-01-01 00:00:00 +0000
+++ test/maps/two_ponds.wmf/version	2017-12-11 06:56:51 +0000
@@ -0,0 +1,11 @@
+# Automatically created by Widelands bzr8483[bug-1718745-allows-seafaring] (Release)
+
+[global]
+map_source_url=
+map_release=
+map_creator_version="bzr8483[bug-1718745-allows-seafaring]"
+map_version_major="0"
+map_version_minor="1"
+map_version_timestamp="1510038973"
+packet_version="1"
+packet_compatibility="1"

=== added directory 'test/scripting'
=== added file 'test/scripting/stable_save.lua'
--- test/scripting/stable_save.lua	1970-01-01 00:00:00 +0000
+++ test/scripting/stable_save.lua	2017-12-11 06:56:51 +0000
@@ -0,0 +1,13 @@
+include "scripting/coroutine.lua"
+
+-- Save the game so that reloading does not skip
+function stable_save(game, savename)
+   local old_speed = game.desired_speed
+   game.desired_speed = 1000
+   sleep(100)
+   game:save(savename)
+   game.desired_speed = 1000
+   sleep(2000)  -- Give the loaded game a chance to catch up
+   game.desired_speed = old_speed
+   sleep(1000)
+end