Merge lp:~widelands-dev/widelands/bug580923 into lp:widelands

[Jens Beyer]

Freely creating workers in special situations (cheating ^^) is gone.

For widelands game itself it should not make a big difference, as the carrier doesn't cost anything. The game still creates carriers once a second or so, even if not needed.

But when you apply a buildcost to the carrier, the automatic creation stops, and they are created only if necessary.

If there is no carrier, and no resources to create one, the economy stalls...

Please test thoroughly, I don't know if I have found all cases and circumstances to test.

[TiborB (tiborb95)]

It looks good to me. But what is the purpose of this change? Can you imagine that carriers will have any kind of buildcost in the future? If not, then such change is not needed.

[Jens Beyer (qcumber-some)]

Hi TiborB, thanks for looking into it.

For the reason of the change, please have a look at the related bug ( https://bugs.launchpad.net/widelands/+bug/580923 ), SirVer states it quite clearly, it is not a bug for Widelands (Game), but for Widelands (Engine), and there have been people (including myself) trying to alter the game by applying buildcost to the Carrier, which was undermining by this bug.

[GunChleoc (gunchleoc)]

I have tested it and LTGM :)

review: Approve

=== modified file 'src/logic/warehouse.cc'
--- src/logic/warehouse.cc	2015-07-24 18:21:58 +0000
+++ src/logic/warehouse.cc	2015-10-02 18:24:14 +0000
@@ -788,11 +788,17 @@
 {
 	WareIndex const carrierid = descr().tribe().safe_worker_index("carrier");
 
-	if (!m_supply->stock_workers(carrierid)) // XXX yep, let's cheat
-		insert_workers(carrierid, 1);
-
-	launch_worker(game, carrierid, Requirements()).start_task_fetchfromflag
-		(game);
+	if (!m_supply->stock_workers(carrierid))
+	{
+		if (can_create_worker(game, carrierid))
+		{
+			create_worker(game, carrierid);
+		}
+	}
+	if (m_supply->stock_workers(carrierid))
+	{
+		launch_worker(game, carrierid, Requirements()).start_task_fetchfromflag(game);
+	}
 
 	return true;
 }
@@ -930,29 +936,37 @@
 	// Create the ware
 	WareInstance & ware = *new WareInstance(ware_index, descr().tribe().get_ware_descr(ware_index));
 	ware.init(game);
-	do_launch_ware(game, ware);
-
-	m_supply->remove_wares(ware_index, 1);
-
+	if (do_launch_ware(game, ware))
+	{
+		m_supply->remove_wares(ware_index, 1);
+	}
 	return ware;
 }
 
 
 /// Get a carrier to actually move this ware out of the warehouse.
-void Warehouse::do_launch_ware(Game & game, WareInstance & ware)
+bool Warehouse::do_launch_ware(Game & game, WareInstance & ware)
 {
 	// Create a carrier
 	WareIndex const carrierid = descr().tribe().worker_index("carrier");
-	const WorkerDescr & workerdescr = *descr().tribe().get_worker_descr(carrierid);
-
-	Worker & worker = workerdescr.create(game, owner(), this, m_position);
-
-	// Yup, this is cheating.
+
+	if (!m_supply->stock_workers(carrierid))
+	{
+		if (can_create_worker(game, carrierid))
+		{
+			create_worker(game, carrierid);
+		}
+	}
 	if (m_supply->stock_workers(carrierid))
-		m_supply->remove_workers(carrierid, 1);
+	{
+		Widelands::Worker & worker = launch_worker(game, carrierid, Requirements());
+		// Setup the carrier
+		worker.start_task_dropoff(game, ware);
+		return true;
+	}
 
-	// Setup the carrier
-	worker.start_task_dropoff(game, ware);
+	//we did not launch the ware...
+	return false;
 }
 
 

=== modified file 'src/logic/warehouse.h'
--- src/logic/warehouse.h	2015-02-12 21:49:05 +0000
+++ src/logic/warehouse.h	2015-10-02 18:24:14 +0000
@@ -183,7 +183,7 @@
 	void incorporate_worker(EditorGameBase&, Worker* worker);
 
 	WareInstance & launch_ware(Game &, WareIndex);
-	void do_launch_ware(Game &, WareInstance &);
+	bool do_launch_ware(Game &, WareInstance &);
 
 	// Adds the ware to our inventory. Takes ownership and might delete 'ware'.
 	void incorporate_ware(EditorGameBase&, WareInstance* ware);