widelands-dev team mailing list archive

[TiborB <tiborb95@xxxxxxxxx>]

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

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1727673 in widelands: "Second builder on colonizing port constructionsite"
  https://bugs.launchpad.net/widelands/+bug/1727673

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945

This bug is situation when colonization ship is going to land a builder, but another builder is already on the port constructionsite (I dont know how it is possible) and request is thus nullptr. Sometimes it crashes.
The solution is to leave the builder on the ship.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/ship.cc'
--- src/logic/map_objects/tribes/ship.cc	2017-08-20 08:34:02 +0000
+++ src/logic/map_objects/tribes/ship.cc	2017-10-27 19:33:48 +0000
@@ -612,6 +612,10 @@
 	case ShipStates::kExpeditionColonizing: {
 		assert(!expedition_->seen_port_buildspaces.empty());
 		BaseImmovable* baim = map[expedition_->seen_port_buildspaces.front()].get_immovable();
+		// Following is a preparation for very rare situation, when colonizing port already have a
+		// worker (bug-1727673)
+		// We leave the worker on the ship then
+		bool leftover_builder = false;
 		if (baim) {
 			upcast(ConstructionSite, cs, baim);
 
@@ -644,6 +648,16 @@
 					break;
 				} else {
 					assert(worker);
+					// If constructionsite does not need worker anymore, we must leave it on the ship.
+					// Also we presume that he is on position 0
+					if (cs->get_builder_request() == nullptr) {
+						log("%2d: WARNING: Colonizing ship %s cannot unload the worker to new port at "
+						    "%3dx%3d because the request is no longer active\n",
+						    get_owner()->player_number(), shipname_.c_str(), cs->get_position().x,
+						    cs->get_position().y);
+						leftover_builder = true;
+						break;  // no more unloading (builder shoud be on position 0)
+					}
 					worker->set_economy(nullptr);
 					worker->set_location(cs);
 					worker->set_position(game, cs->get_position());
@@ -661,8 +675,8 @@
 			send_signal(game, "cancel_expedition");
 		}
 
-		if (items_.empty() || !baim) {            // we are done, either way
-			ship_state_ = ShipStates::kTransport;  // That's it, expedition finished
+		if (items_.empty() || !baim || leftover_builder) {  // we are done, either way
+			ship_state_ = ShipStates::kTransport;            // That's it, expedition finished
 
 			// Bring us back into a fleet and a economy.
 			init_fleet(game);
@@ -1002,13 +1016,14 @@
 	Bob::log_general_info(egbase);
 
 	molog("Ship belongs to fleet: %u\n destination: %s\n lastdock: %s\n",
-	      fleet_ ? fleet_->serial() : 0, (destination_.is_set()) ?
-	                                        (boost::format("%u (%d x %d)") % destination_.serial() %
-	                                         destination_.get(egbase)->get_positions(egbase)[0].x %
-	                                         destination_.get(egbase)->get_positions(egbase)[0].y)
-	                                           .str()
-	                                           .c_str() :
-	                                        "-",
+	      fleet_ ? fleet_->serial() : 0,
+	      (destination_.is_set()) ?
+	         (boost::format("%u (%d x %d)") % destination_.serial() %
+	          destination_.get(egbase)->get_positions(egbase)[0].x %
+	          destination_.get(egbase)->get_positions(egbase)[0].y)
+	            .str()
+	            .c_str() :
+	         "-",
 	      (lastdock_.is_set()) ?
 	         (boost::format("%u (%d x %d)") % lastdock_.serial() %
 	          lastdock_.get(egbase)->get_positions(egbase)[0].x %