widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Notabilis has proposed merging lp:~widelands-dev/widelands/bug-free-workers into lp:widelands.

Requested reviews:
  Widelands Developers (widelands-dev)
Related bugs:
  Bug #1643209 in widelands: "No-cost workers are not removed correctly"
  https://bugs.launchpad.net/widelands/+bug/1643209

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-free-workers/+merge/311661

When more than 100 workers without buildcost enter a warehouse and the warehouse is destroyed after some time, the workers are neither removed nor send away. In debug builds an assert will fail which crashes the game, in release builds it is probably only a memory leak.
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-free-workers into lp:widelands.

=== modified file 'src/logic/map_objects/tribes/warehouse.cc'
--- src/logic/map_objects/tribes/warehouse.cc	2016-11-17 06:08:27 +0000
+++ src/logic/map_objects/tribes/warehouse.cc	2016-11-23 20:53:07 +0000
@@ -826,7 +826,7 @@
 
 	supply_->add_workers(worker_index, 1);
 
-	//  We remove carriers, but we keep other workers around.
+	//  We remove free workers, but we keep other workers around.
 	//  TODO(unknown): Remove all workers that do not have properties such as experience.
 	//  And even such workers should be removed and only a small record
 	//  with the experience (and possibly other data that must survive)
@@ -834,8 +834,8 @@
 	//  When this is done, the get_incorporated_workers method above must
 	//  be reworked so that workers are recreated, and rescheduled for
 	//  incorporation.
-	if (upcast(Carrier, carrier, w)) {
-		carrier->remove(egbase);
+	if (w->descr().is_buildable() && w->descr().buildcost().empty()) {
+		w->remove(egbase);
 		return;
 	}