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

[cghislai]

Two fixes for warehouses.
- Correctly reassign all incorporated workers to the dismantlingsite when dismantling. Previously, this was not done and workers started to become fugitive.
- Hide the carrier button in the option window. This will prevent infinite creation of carrier if one decide to empty them.

[SirVer (sirver)]

I am slightly confused. Why does it say this is merged? Trunk does definitely not contain the new code. Could it be that you branched and merged your different branches from each other instead from trunk?

however, the code looks fine. Just a nit

upcast(Warehouse, wh, building);
if (wh)

can be written as

if (upcast(Warehouse, wh, building))

review: Approve

[SirVer (sirver)]

Charly, could you comment on what went wrong here?

[cghislai (charlyghislain)]

Yes I think this is it. For small changes, like here, I once tried to push them in a branch, then revert locally to trunk, and commit locally other changes that would be pushed elsewhere. So I didn't have to recompile the whole codebase to test them.

Weird stuff happened, I will avoid working that way in the future.

[SirVer (sirver)]

i strongly suggest using ccache (http://ccache.samba.org/). I also use this (but without the repoalias plugin) which makes bzr better for me: http://www.taoeffect.com/blog/2009/06/using-bazaar-like-git-repoalias-plugin/.

 What about these changes here? Can you get them into trunk manually somehow?

[cghislai (charlyghislain)]

Thanks for pointing me to ccache, I will try that. Otherwise I already use bzr kindof like git, with a shared repo, but I should try the method of that guy.

Concerning this branch I will make a patch and apply to trunk

[SirVer (sirver)]

I did not see a commit message for this. Have you done this already?

[cghislai (charlyghislain)]

yes it is in r6711

=== modified file 'src/logic/player.cc'
--- src/logic/player.cc	2013-07-26 20:19:36 +0000
+++ src/logic/player.cc	2013-07-28 09:14:26 +0000
@@ -642,7 +642,13 @@
 		//  Get workers and soldiers
 		//  Make copies of the vectors, because the originals are destroyed with
 		//  the building.
-		const std::vector<Worker  *> workers  = building->get_workers();
+		std::vector<Worker  *> workers;
+		upcast(Warehouse, wh, building);
+		if (wh) {
+			workers = wh->get_incorporated_workers();
+		} else {
+			workers = building->get_workers();
+		}
 
 		building->remove(egbase()); //  no fire or stuff
 		//  Hereafter the old building does not exist and building is a dangling

=== modified file 'src/logic/warehouse.cc'
--- src/logic/warehouse.cc	2013-07-26 20:19:36 +0000
+++ src/logic/warehouse.cc	2013-07-28 09:14:26 +0000
@@ -542,14 +542,14 @@
 
 	const WareList & workers = get_workers();
 
+	// This will empty the stock and launch all workers
+	// including incorporated ones
 	for (Ware_Index id = Ware_Index::First(); id < workers.get_nrwareids(); ++id) {
 		const uint32_t stock = workers.stock(id);
 
-		if (stock > 0) {
-			for (uint32_t i = 0; i < stock; ++i) {
-				launch_worker(game, id, Requirements()).start_task_leavebuilding
-					(game, true);
-			}
+		for (uint32_t i = 0; i < stock; ++i) {
+			launch_worker(game, id, Requirements()).start_task_leavebuilding
+				(game, true);
 		}
 	}
 
@@ -747,6 +747,19 @@
 	return m_supply->get_workers();
 }
 
+PlayerImmovable::Workers Warehouse::get_incorporated_workers()
+{
+	PlayerImmovable::Workers all_workers;
+	container_iterate(IncorporatedWorkers, m_incorporated_workers, cpair) {
+		WorkerList & clist = cpair->second;
+		container_iterate(WorkerList, clist, w) {
+			all_workers.push_back(*w.current);
+		}
+	}
+	return all_workers;
+}
+
+
 /// Magically create wares in this warehouse. Updates the economy accordingly.
 void Warehouse::insert_wares(Ware_Index const id, uint32_t const count)
 {
@@ -902,6 +915,9 @@
 		//  FIXME And even such workers should be removed and only a small record
 		//  FIXME with the experience (and possibly other data that must survive)
 		//  FIXME may be kept.
+		//  FIXME When this is done, the get_incorporated_workers method above must
+		//  FIXME be reworked so that workers are recreated, and rescheduled for
+		//  FIXME incorporation.
 		if (dynamic_cast<Carrier const *>(&w)) {
 			w.remove(egbase);
 			return;

=== modified file 'src/logic/warehouse.h'
--- src/logic/warehouse.h	2013-07-26 20:19:36 +0000
+++ src/logic/warehouse.h	2013-07-28 09:14:26 +0000
@@ -140,6 +140,12 @@
 	const WareList & get_wares() const;
 	const WareList & get_workers() const;
 
+	/**
+	 * Returns a vector of all incorporated workers. These are the workers
+	 * that are still present in the game, not just a stock figure.
+	 */
+	Workers get_incorporated_workers();
+
 	void insert_wares  (Ware_Index, uint32_t count);
 	void remove_wares  (Ware_Index, uint32_t count);
 	void insert_workers(Ware_Index, uint32_t count);

=== modified file 'src/wui/warehousewindow.cc'
--- src/wui/warehousewindow.cc	2013-07-26 20:19:36 +0000
+++ src/wui/warehousewindow.cc	2013-07-28 09:14:26 +0000
@@ -64,6 +64,11 @@
 {
 	set_inner_size(width, 0);
 	add_warelist(type == Widelands::wwWORKER ? m_warehouse.get_workers() : m_warehouse.get_wares());
+	if (type == Widelands::wwWORKER) {
+		Widelands::Ware_Index carrier_index =
+			m_warehouse.descr().tribe().worker_index("carrier");
+		hide_ware(carrier_index);
+	}
 }
 
 void WarehouseWaresDisplay::draw_ware(RenderTarget & dst, Widelands::Ware_Index ware)