Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-11-02 Thread GunChleoc
I tested with a game I created myself, by building an expedition port on my own 
island and quickly connecting a road to a nearby warehouse. Two Lagoons is a 
good map for that.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-11-01 Thread TiborB
BTW, can you tell where the first builder came from?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-11-01 Thread noreply
The proposal to merge lp:~widelands-dev/widelands/bug-1727673 into lp:widelands 
has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
-- 
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-11-01 Thread GunChleoc
I did some testing now. I don't get a crash in trunk, but the builder would 
disappear, while this branch will keep the extra builder on the Ship and return 
him to a port. So, definitely an improvement! I'll assume that the crash is 
fixed too and close the bug.

@bunnybot merge
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-10-28 Thread TiborB
This is exactly the same situation as when port constructionsite disappears 
during unloading of wares/worker. So yes..
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-10-28 Thread GunChleoc
Code LGTM, not tested.

What happens to the builder on the ship, will he be transported back and 
unloaded at a warehouse?
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-10-27 Thread bunnybot
Continuous integration builds have changed state:

Travis build 2716. State: passed. Details: 
https://travis-ci.org/widelands/widelands/builds/293854904.
Appveyor build 2529. State: failed. Details: 
https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1727673-2529.
-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1727673/+merge/332945
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/bug-1727673 into lp:widelands.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


[Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1727673 into lp:widelands

2017-10-27 Thread TiborB
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 +
+++ src/logic/map_objects/tribes/ship.cc	2017-10-27 19:33:48 +
@@ -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 %

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp