Merge lp:~widelands-dev/widelands/reed-compatibility into lp:widelands

[GunChleoc]

Fix savegame compatibility for reed, buildings, players view and economy requests.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5058. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/537337141.
Appveyor build 4838. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_reed_compatibility-4838.

[Notabilis (notabilis27)]

So ... you created a diff of 800 lines by passing objects around so you can call a method in only a handful of places?? I like it! :)
The changes are mostly looking good to me. Replacing the GOTO might be broken, though, see my diff comment below.

I haven't tested the changes, mostly because I have no sensible idea how to do so. But based on the code I guess it shouldn't break anything.

[GunChleoc (gunchleoc)]

> So ... you created a diff of 800 lines by passing objects around so you can call a method in only a handful of places?? I like it! :)

Lol yep. It will save a bit of memory though if the objects are created only once.

For triggering the bug, you need a savegame created with bzr9118 or older that has reed or wheat fields on it.

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 5154. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/541619525.
Appveyor build 4936. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_reed_compatibility-4936.

[GunChleoc (gunchleoc)]

I have added a second if statement for the test variable to simulate the effect of the goto and break out of the outer loop. Thanks for the review!

@bunybot merge

[Notabilis (notabilis27)]

Sorry, I am having second thoughts about the goto-part. Actually, I am not only unsure about the goto-replacement but over the whole code part independent of your changes. :-/
If I understand it right, the code iterates over the possible jobs in the production site. If there is a job the loaded worker can work on, we iterate over all working positions of the site and check if *any* of these is empty, so we can assign the worker. Shouldn't we only check the working positions that are for the required job? Otherwise, the code could break with production sites which have multiple types of worker positions (Mines probably? These need a miner and a master miner).

Maybe I am just confused and the code is fine, but it would be good if someone could check it.

[GunChleoc (gunchleoc)]

This code is pretty old, so I'm sure we would have had bug reports for it in the past if it was broken. It should be fine if iterating starts with the most advanced working position, because then we won't put a master miner i a miner's slot and end up stumped when we have to put a miner in a master miner's slot, which is not allowed.

I'll reinstate the goto to make sure we can't possibly have any accidental semantic changes, so that we can get this bugfix in.

[GunChleoc (gunchleoc)]

Done. Thanks for the review!

@bunnybot merge

[GunChleoc (gunchleoc)]

Inputqueues again

@bunnybot merge force

[bunnybot (widelandsofficial)]

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 5164. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/542121335.

[hessenfarmer (stephan-lutz)]

@bunnybot merge force

[hessenfarmer (stephan-lutz)]

review: Approve

=== modified file 'src/economy/expedition_bootstrap.cc'
--- src/economy/expedition_bootstrap.cc	2019-02-23 11:00:49 +0000
+++ src/economy/expedition_bootstrap.cc	2019-06-08 16:55:17 +0000
@@ -203,7 +203,7 @@
 }
 
 void ExpeditionBootstrap::load(
-   Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, uint16_t packet_version) {
+   Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table, uint16_t packet_version) {
 
 	static const uint16_t kCurrentPacketVersion = 7;
 	assert(queues_.empty());
@@ -214,7 +214,7 @@
 			uint8_t num_queues = fr.unsigned_8();
 			for (uint8_t i = 0; i < num_queues; ++i) {
 				WorkersQueue* wq = new WorkersQueue(warehouse, INVALID_INDEX, 0);
-				wq->read(fr, game, mol);
+				wq->read(fr, game, mol, tribes_lookup_table);
 				wq->set_callback(input_callback, this);
 
 				if (wq->get_index() == INVALID_INDEX) {
@@ -232,7 +232,7 @@
 		uint8_t num_queues = fr.unsigned_8();
 		for (uint8_t i = 0; i < num_queues; ++i) {
 			WaresQueue* wq = new WaresQueue(warehouse, INVALID_INDEX, 0);
-			wq->read(fr, game, mol);
+			wq->read(fr, game, mol, tribes_lookup_table);
 			wq->set_callback(input_callback, this);
 
 			if (wq->get_index() == INVALID_INDEX) {
 
=== modified file 'src/economy/expedition_bootstrap.h'
--- src/economy/expedition_bootstrap.h	2019-02-23 11:00:49 +0000
+++ src/economy/expedition_bootstrap.h	2019-06-08 16:55:17 +0000
@@ -88,7 +88,7 @@
 	 * packet, and there in the warehouse data packet.
 	 */
 	void
-	load(Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, uint16_t version);
+	load(Warehouse& warehouse, FileRead& fr, Game& game, MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table, uint16_t version);
 
 	/** Save this into a file.
 	 *
 
=== modified file 'src/economy/input_queue.cc'
--- src/economy/input_queue.cc	2019-02-23 11:00:49 +0000
+++ src/economy/input_queue.cc	2019-06-08 16:55:17 +0000
@@ -130,7 +130,7 @@
 
 constexpr uint16_t kCurrentPacketVersion = 3;
 
-void InputQueue::read(FileRead& fr, Game& game, MapObjectLoader& mol) {
+void InputQueue::read(FileRead& fr, Game& game, MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 
 	uint16_t const packet_version = fr.unsigned_16();
 	try {
@@ -140,25 +140,26 @@
 		if (packet_version == 1 || packet_version == kCurrentPacketVersion) {
 			if (fr.unsigned_8() == 0) {
 				assert(type_ == wwWARE);
-				index_ = owner().tribe().ware_index(fr.c_string());
+				index_ = owner().tribe().ware_index(tribes_lookup_table.lookup_ware(fr.c_string()));
 			} else {
 				assert(type_ == wwWORKER);
-				index_ = owner().tribe().worker_index(fr.c_string());
+				index_ = owner().tribe().worker_index(tribes_lookup_table.lookup_worker(fr.c_string()));
 			}
 			max_size_ = fr.unsigned_32();
 			max_fill_ = fr.signed_32();
 			consume_interval_ = fr.unsigned_32();
 			if (fr.unsigned_8()) {
 				request_.reset(new Request(owner_, 0, InputQueue::request_callback, type_));
-				request_->read(fr, game, mol);
+				request_->read(fr, game, mol, tribes_lookup_table);
 			} else {
 				request_.reset();
 			}
 
 			read_child(fr, game, mol);
 		} else if (packet_version == 2) {
+			// TODO(GunChleoc): Savegame compatibility, get rid after Build 21
 			assert(type_ == wwWARE);
-			index_ = owner().tribe().ware_index(fr.c_string());
+			index_ = owner().tribe().ware_index(tribes_lookup_table.lookup_ware(fr.c_string()));
 			max_size_ = fr.unsigned_32();
 			max_fill_ = fr.signed_32();
 			// No read_child() call here, doing it manually since there is no child-version number
@@ -166,7 +167,7 @@
 			consume_interval_ = fr.unsigned_32();
 			if (fr.unsigned_8()) {
 				request_.reset(new Request(owner_, 0, InputQueue::request_callback, type_));
-				request_->read(fr, game, mol);
+				request_->read(fr, game, mol, tribes_lookup_table);
 			} else {
 				request_.reset();
 			}
 
=== modified file 'src/economy/input_queue.h'
--- src/economy/input_queue.h	2019-02-27 19:00:36 +0000
+++ src/economy/input_queue.h	2019-06-08 16:55:17 +0000
@@ -184,7 +184,7 @@
 	 * @param game The game this queue will be part of.
 	 * @param mol The game/map loader that handles the lading. Required to pass to Request::read().
 	 */
-	void read(FileRead& f, Game& g, MapObjectLoader& mol);
+	void read(FileRead& f, Game& g, MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table);
 
 	/**
 	 * Writes the state of this class.
 
=== modified file 'src/economy/request.cc'
--- src/economy/request.cc	2019-02-23 11:00:49 +0000
+++ src/economy/request.cc	2019-06-08 16:55:17 +0000
@@ -98,18 +98,18 @@
  * might have been initialized. We have to kill them and replace
  * them through the data in the file
  */
-void Request::read(FileRead& fr, Game& game, MapObjectLoader& mol) {
+void Request::read(FileRead& fr, Game& game, MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version == kCurrentPacketVersion) {
 			const TribeDescr& tribe = target_.owner().tribe();
 			char const* const type_name = fr.c_string();
-			DescriptionIndex const wai = tribe.ware_index(type_name);
+			DescriptionIndex const wai = tribe.ware_index(tribes_lookup_table.lookup_ware(type_name));
 			if (tribe.has_ware(wai)) {
 				type_ = wwWARE;
 				index_ = wai;
 			} else {
-				DescriptionIndex const woi = tribe.worker_index(type_name);
+				DescriptionIndex const woi = tribe.worker_index(tribes_lookup_table.lookup_worker(type_name));
 				if (tribe.has_worker(woi)) {
 					type_ = wwWORKER;
 					index_ = woi;
 
=== modified file 'src/economy/request.h'
--- src/economy/request.h	2019-02-23 11:00:49 +0000
+++ src/economy/request.h	2019-06-08 16:55:17 +0000
@@ -24,6 +24,7 @@
 #include "logic/map_objects/tribes/requirements.h"
 #include "logic/map_objects/tribes/wareworker.h"
 #include "logic/widelands.h"
+#include "map_io/tribes_legacy_lookup_table.h"
 
 class FileRead;
 class FileWrite;
@@ -116,7 +117,7 @@
 
 	void start_transfer(Game&, Supply&);
 
-	void read(FileRead&, Game&, MapObjectLoader&);
+	void read(FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
 	void write(FileWrite&, Game&, MapObjectSaver&) const;
 	Worker* get_transfer_worker();
 
 
=== modified file 'src/game_io/game_player_info_packet.cc'
--- src/game_io/game_player_info_packet.cc	2019-05-19 12:25:24 +0000
+++ src/game_io/game_player_info_packet.cc	2019-06-08 16:55:17 +0000
@@ -37,7 +37,7 @@
 
 void GamePlayerInfoPacket::read(FileSystem& fs, Game& game, MapObjectLoader*) {
 	try {
-		std::unique_ptr<TribesLegacyLookupTable> tribe_lookup_table(new TribesLegacyLookupTable());
+		std::unique_ptr<TribesLegacyLookupTable> tribes_lookup_table(new TribesLegacyLookupTable());
 		FileRead fr;
 		fr.open(fs, "binary/player_info");
 		uint16_t const packet_version = fr.unsigned_16();
@@ -72,7 +72,7 @@
 						}
 					}
 
-					player->read_statistics(fr, packet_version, *tribe_lookup_table.get());
+					player->read_statistics(fr, packet_version, *tribes_lookup_table.get());
 					player->read_remaining_shipnames(fr);
 
 					player->casualties_ = fr.unsigned_32();
 
=== modified file 'src/map_io/map_buildingdata_packet.cc'
--- src/map_io/map_buildingdata_packet.cc	2019-05-28 21:04:36 +0000
+++ src/map_io/map_buildingdata_packet.cc	2019-06-08 16:55:17 +0000
@@ -71,7 +71,7 @@
 void MapBuildingdataPacket::read(FileSystem& fs,
                                  EditorGameBase& egbase,
                                  bool const skip,
-                                 MapObjectLoader& mol) {
+                                 MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	if (skip)
 		return;
 
@@ -180,18 +180,18 @@
 					Game& game = dynamic_cast<Game&>(egbase);
 
 					if (upcast(ConstructionSite, constructionsite, &building)) {
-						read_constructionsite(*constructionsite, fr, game, mol);
+						read_constructionsite(*constructionsite, fr, game, mol, tribes_lookup_table);
 					} else if (upcast(DismantleSite, dms, &building)) {
-						read_dismantlesite(*dms, fr, game, mol);
+						read_dismantlesite(*dms, fr, game, mol, tribes_lookup_table);
 					} else if (upcast(MilitarySite, militarysite, &building)) {
-						read_militarysite(*militarysite, fr, game, mol);
+						read_militarysite(*militarysite, fr, game, mol, tribes_lookup_table);
 					} else if (upcast(Warehouse, warehouse, &building)) {
-						read_warehouse(*warehouse, fr, game, mol);
+						read_warehouse(*warehouse, fr, game, mol, tribes_lookup_table);
 					} else if (upcast(ProductionSite, productionsite, &building)) {
 						if (upcast(TrainingSite, trainingsite, productionsite)) {
-							read_trainingsite(*trainingsite, fr, game, mol);
+							read_trainingsite(*trainingsite, fr, game, mol, tribes_lookup_table);
 						} else {
-							read_productionsite(*productionsite, fr, game, mol);
+							read_productionsite(*productionsite, fr, game, mol, tribes_lookup_table);
 						}
 					} else {
 						//  type of building is not one of (or derived from)
@@ -215,7 +215,7 @@
 void MapBuildingdataPacket::read_partially_finished_building(PartiallyFinishedBuilding& pfb,
                                                              FileRead& fr,
                                                              Game& game,
-                                                             MapObjectLoader& mol) {
+                                                             MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version == kCurrentPacketPFBuilding) {
@@ -226,7 +226,7 @@
 			if (fr.unsigned_8()) {
 				pfb.builder_request_ =
 				   new Request(pfb, 0, PartiallyFinishedBuilding::request_builder_callback, wwWORKER);
-				pfb.builder_request_->read(fr, game, mol);
+				pfb.builder_request_->read(fr, game, mol, tribes_lookup_table);
 			} else
 				pfb.builder_request_ = nullptr;
 
@@ -244,7 +244,7 @@
 				pfb.wares_.resize(size);
 				for (uint16_t i = 0; i < pfb.wares_.size(); ++i) {
 					pfb.wares_[i] = new WaresQueue(pfb, INVALID_INDEX, 0);
-					pfb.wares_[i]->read(fr, game, mol);
+					pfb.wares_[i]->read(fr, game, mol, tribes_lookup_table);
 				}
 			} catch (const WException& e) {
 				throw GameDataError("wares: %s", e.what());
@@ -266,11 +266,11 @@
 void MapBuildingdataPacket::read_constructionsite(ConstructionSite& constructionsite,
                                                   FileRead& fr,
                                                   Game& game,
-                                                  MapObjectLoader& mol) {
+                                                  MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version >= kCurrentPacketVersionConstructionsite) {
-			read_partially_finished_building(constructionsite, fr, game, mol);
+			read_partially_finished_building(constructionsite, fr, game, mol, tribes_lookup_table);
 
 			for (ConstructionSite::Wares::iterator wares_iter = constructionsite.wares_.begin();
 			     wares_iter != constructionsite.wares_.end(); ++wares_iter) {
@@ -291,11 +291,11 @@
 void MapBuildingdataPacket::read_dismantlesite(DismantleSite& dms,
                                                FileRead& fr,
                                                Game& game,
-                                               MapObjectLoader& mol) {
+                                               MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version == kCurrentPacketVersionDismantlesite) {
-			read_partially_finished_building(dms, fr, game, mol);
+			read_partially_finished_building(dms, fr, game, mol, tribes_lookup_table);
 
 			// Nothing to do
 		} else {
@@ -310,7 +310,7 @@
 void MapBuildingdataPacket::read_warehouse(Warehouse& warehouse,
                                            FileRead& fr,
                                            Game& game,
-                                           MapObjectLoader& mol) {
+                                           MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version >= 6) {
@@ -319,7 +319,7 @@
 			const TribeDescr& tribe = player->tribe();
 
 			while (fr.unsigned_8()) {
-				const DescriptionIndex& id = tribe.ware_index(fr.c_string());
+				const DescriptionIndex& id = tribe.ware_index(tribes_lookup_table.lookup_ware(fr.c_string()));
 				Quantity amount = fr.unsigned_32();
 				Warehouse::StockPolicy policy = static_cast<Warehouse::StockPolicy>(fr.unsigned_8());
 
@@ -329,7 +329,7 @@
 				}
 			}
 			while (fr.unsigned_8()) {
-				const DescriptionIndex& id = tribe.worker_index(fr.c_string());
+				const DescriptionIndex& id = tribe.worker_index(tribes_lookup_table.lookup_worker(fr.c_string()));
 				uint32_t amount = fr.unsigned_32();
 				Warehouse::StockPolicy policy = static_cast<Warehouse::StockPolicy>(fr.unsigned_8());
 
@@ -364,15 +364,16 @@
 			   tribe.worker_types_without_cost();
 
 			for (;;) {
-				char const* const worker_typename = fr.c_string();
-				if (!*worker_typename)  //  encountered the terminator ("")
+				const std::string worker_typename = tribes_lookup_table.lookup_worker(fr.c_string());
+				if (worker_typename.empty()) { //  No more workers found
 					break;
+				}
 				uint32_t const next_spawn = fr.unsigned_32();
 				DescriptionIndex const worker_index = tribe.safe_worker_index(worker_typename);
 				if (!game.tribes().worker_exists(worker_index)) {
 					log("WARNING: %s %u has a next_spawn time for nonexistent "
 					    "worker type \"%s\" set to %u, ignoring\n",
-					    warehouse.descr().name().c_str(), warehouse.serial(), worker_typename,
+					    warehouse.descr().name().c_str(), warehouse.serial(), worker_typename.c_str(),
 					    next_spawn);
 					continue;
 				}
@@ -380,7 +381,7 @@
 					log("WARNING: %s %u has a next_spawn time for worker type "
 					    "\"%s\", that costs something to build, set to %u, "
 					    "ignoring\n",
-					    warehouse.descr().name().c_str(), warehouse.serial(), worker_typename,
+					    warehouse.descr().name().c_str(), warehouse.serial(), worker_typename.c_str(),
 					    next_spawn);
 					continue;
 				}
@@ -392,7 +393,7 @@
 							                    "\"%s\" set to %u, but it was previously set "
 							                    "to %u\n",
 							                    warehouse.descr().name().c_str(), warehouse.serial(),
-							                    worker_typename, next_spawn,
+							                    worker_typename.c_str(), next_spawn,
 							                    warehouse.next_worker_without_cost_spawn_[i]);
 						warehouse.next_worker_without_cost_spawn_[i] = next_spawn;
 						break;
@@ -409,13 +410,13 @@
 			while (nr_planned_workers--) {
 				warehouse.planned_workers_.push_back(Warehouse::PlannedWorkers());
 				Warehouse::PlannedWorkers& pw = warehouse.planned_workers_.back();
-				pw.index = tribe.worker_index(fr.c_string());
+				pw.index = tribe.worker_index(tribes_lookup_table.lookup_worker(fr.c_string()));
 				pw.amount = fr.unsigned_32();
 
 				uint32_t nr_requests = fr.unsigned_32();
 				while (nr_requests--) {
 					pw.requests.push_back(new Request(warehouse, 0, &Warehouse::request_cb, wwWORKER));
-					pw.requests.back()->read(fr, game, mol);
+					pw.requests.back()->read(fr, game, mol, tribes_lookup_table);
 				}
 			}
 
@@ -430,7 +431,7 @@
 					// doesn't lend itself to request and other stuff.
 					if (warehouse.portdock_->expedition_started()) {
 						warehouse.portdock_->expedition_bootstrap()->load(
-						   warehouse, fr, game, mol, packet_version);
+						   warehouse, fr, game, mol, tribes_lookup_table, packet_version);
 					}
 				}
 			}
@@ -463,7 +464,7 @@
 void MapBuildingdataPacket::read_militarysite(MilitarySite& militarysite,
                                               FileRead& fr,
                                               Game& game,
-                                              MapObjectLoader& mol) {
+                                              MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version >= 5 && packet_version <= kCurrentPacketVersionMilitarysite) {
@@ -472,18 +473,20 @@
 			if (fr.unsigned_8()) {
 				militarysite.normal_soldier_request_.reset(
 				   new Request(militarysite, 0, MilitarySite::request_soldier_callback, wwWORKER));
-				militarysite.normal_soldier_request_->read(fr, game, mol);
-			} else
+				militarysite.normal_soldier_request_->read(fr, game, mol, tribes_lookup_table);
+			} else {
 				militarysite.normal_soldier_request_.reset();
+			}
 
 			if (fr.unsigned_8()) {
 				militarysite.upgrade_soldier_request_.reset(new Request(
 				   militarysite,
 				   (!militarysite.normal_soldier_request_) ? 0 : militarysite.owner().tribe().soldier(),
 				   MilitarySite::request_soldier_callback, wwWORKER));
-				militarysite.upgrade_soldier_request_->read(fr, game, mol);
-			} else
+				militarysite.upgrade_soldier_request_->read(fr, game, mol, tribes_lookup_table);
+			} else {
 				militarysite.upgrade_soldier_request_.reset();
+			}
 
 			if ((militarysite.didconquer_ = fr.unsigned_8())) {
 				//  Add to map of military influence.
@@ -549,7 +552,7 @@
 void MapBuildingdataPacket::read_productionsite(ProductionSite& productionsite,
                                                 FileRead& fr,
                                                 Game& game,
-                                                MapObjectLoader& mol) {
+                                                MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		// TODO(GunChleoc): Savegame compatibility, remove after Build 21.
@@ -562,7 +565,7 @@
 			for (uint16_t i = nr_worker_requests; i; --i) {
 				Request& req =
 				   *new Request(productionsite, 0, ProductionSite::request_worker_callback, wwWORKER);
-				req.read(fr, game, mol);
+				req.read(fr, game, mol, tribes_lookup_table);
 				const DescriptionIndex& worker_index = req.get_index();
 
 				//  Find a working position that matches this request.
@@ -587,8 +590,9 @@
 						}
 						found_working_position = true;
 						break;
-					} else
+					} else {
 						wp += count;
+					}
 				}
 
 				if (!found_working_position)
@@ -682,7 +686,7 @@
 			assert(!productionsite.input_queues_.size());
 			for (uint16_t i = 0; i < nr_queues; ++i) {
 				WaresQueue* wq = new WaresQueue(productionsite, INVALID_INDEX, 0);
-				wq->read(fr, game, mol);
+				wq->read(fr, game, mol, tribes_lookup_table);
 
 				if (!game.tribes().ware_exists(wq->get_index())) {
 					delete wq;
@@ -695,7 +699,7 @@
 				nr_queues = fr.unsigned_16();
 				for (uint16_t i = 0; i < nr_queues; ++i) {
 					WorkersQueue* wq = new WorkersQueue(productionsite, INVALID_INDEX, 0);
-					wq->read(fr, game, mol);
+					wq->read(fr, game, mol, tribes_lookup_table);
 
 					if (!game.tribes().worker_exists(wq->get_index())) {
 						delete wq;
@@ -732,18 +736,18 @@
 void MapBuildingdataPacket::read_trainingsite(TrainingSite& trainingsite,
                                               FileRead& fr,
                                               Game& game,
-                                              MapObjectLoader& mol) {
+                                              MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		uint16_t const packet_version = fr.unsigned_16();
 		if (packet_version == kCurrentPacketVersionTrainingsite) {
-			read_productionsite(trainingsite, fr, game, mol);
+			read_productionsite(trainingsite, fr, game, mol, tribes_lookup_table);
 
 			delete trainingsite.soldier_request_;
 			trainingsite.soldier_request_ = nullptr;
 			if (fr.unsigned_8()) {
 				trainingsite.soldier_request_ =
 				   new Request(trainingsite, 0, TrainingSite::request_soldier_callback, wwWORKER);
-				trainingsite.soldier_request_->read(fr, game, mol);
+				trainingsite.soldier_request_->read(fr, game, mol, tribes_lookup_table);
 			}
 
 			trainingsite.capacity_ = fr.unsigned_8();
 
=== modified file 'src/map_io/map_buildingdata_packet.h'
--- src/map_io/map_buildingdata_packet.h	2019-02-23 11:00:49 +0000
+++ src/map_io/map_buildingdata_packet.h	2019-06-08 16:55:17 +0000
@@ -21,6 +21,7 @@
 #define WL_MAP_IO_MAP_BUILDINGDATA_PACKET_H
 
 #include "map_io/map_data_packet.h"
+#include "map_io/tribes_legacy_lookup_table.h"
 
 class FileRead;
 class FileWrite;
@@ -42,18 +43,18 @@
  */
 class MapBuildingdataPacket {
 public:
-	void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&);
+	void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
 	void write(FileSystem&, EditorGameBase&, MapObjectSaver&);
 
 private:
-	void read_constructionsite(ConstructionSite&, FileRead&, Game&, MapObjectLoader&);
-	void read_dismantlesite(DismantleSite&, FileRead&, Game&, MapObjectLoader&);
+	void read_constructionsite(ConstructionSite&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void read_dismantlesite(DismantleSite&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
 	void
-	read_partially_finished_building(PartiallyFinishedBuilding&, FileRead&, Game&, MapObjectLoader&);
-	void read_warehouse(Warehouse&, FileRead&, Game&, MapObjectLoader&);
-	void read_militarysite(MilitarySite&, FileRead&, Game&, MapObjectLoader&);
-	void read_trainingsite(TrainingSite&, FileRead&, Game&, MapObjectLoader&);
-	void read_productionsite(ProductionSite&, FileRead&, Game&, MapObjectLoader&);
+	read_partially_finished_building(PartiallyFinishedBuilding&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void read_warehouse(Warehouse&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void read_militarysite(MilitarySite&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void read_trainingsite(TrainingSite&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void read_productionsite(ProductionSite&, FileRead&, Game&, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
 
 	void write_constructionsite(const ConstructionSite&, FileWrite&, Game&, MapObjectSaver&);
 	void write_dismantlesite(const DismantleSite&, FileWrite&, Game&, MapObjectSaver&);
 
=== modified file 'src/map_io/map_flagdata_packet.cc'
--- src/map_io/map_flagdata_packet.cc	2019-02-23 11:00:49 +0000
+++ src/map_io/map_flagdata_packet.cc	2019-06-08 16:55:17 +0000
@@ -43,7 +43,7 @@
 void MapFlagdataPacket::read(FileSystem& fs,
                              EditorGameBase& egbase,
                              bool const skip,
-                             MapObjectLoader& mol) {
+                             MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	if (skip)
 		return;
 
@@ -131,7 +131,7 @@
 							Flag::FlagJob f;
 							if (fr.unsigned_8()) {
 								f.request = new Request(flag, 0, Flag::flag_job_request_callback, wwWORKER);
-								f.request->read(fr, dynamic_cast<Game&>(egbase), mol);
+								f.request->read(fr, dynamic_cast<Game&>(egbase), mol, tribes_lookup_table);
 							} else {
 								f.request = nullptr;
 							}
 
=== modified file 'src/map_io/map_flagdata_packet.h'
--- src/map_io/map_flagdata_packet.h	2019-02-23 11:00:49 +0000
+++ src/map_io/map_flagdata_packet.h	2019-06-08 16:55:17 +0000
@@ -21,7 +21,14 @@
 #define WL_MAP_IO_MAP_FLAGDATA_PACKET_H
 
 #include "map_io/map_data_packet.h"
+#include "map_io/tribes_legacy_lookup_table.h"
 
-MAP_DATA_PACKET(MapFlagdataPacket)
+namespace Widelands {
+class MapFlagdataPacket {
+public:
+	void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void write(FileSystem&, EditorGameBase&, MapObjectSaver&);
+};
+}
 
 #endif  // end of include guard: WL_MAP_IO_MAP_FLAGDATA_PACKET_H
 
=== modified file 'src/map_io/map_object_packet.cc'
--- src/map_io/map_object_packet.cc	2019-02-23 11:00:49 +0000
+++ src/map_io/map_object_packet.cc	2019-06-08 16:55:17 +0000
@@ -49,7 +49,7 @@
                            EditorGameBase& egbase,
                            MapObjectLoader& mol,
                            const WorldLegacyLookupTable& world_lookup_table,
-                           const TribesLegacyLookupTable& tribe_lookup_table) {
+                           const TribesLegacyLookupTable& tribes_lookup_table) {
 	try {
 		FileRead fr;
 		fr.open(fs, "binary/mapobjects");
@@ -66,7 +66,7 @@
 					return;
 				case MapObject::HeaderImmovable:
 					loaders.insert(
-					   Immovable::load(egbase, mol, fr, world_lookup_table, tribe_lookup_table));
+					   Immovable::load(egbase, mol, fr, world_lookup_table, tribes_lookup_table));
 					break;
 
 				case MapObject::HeaderBattle:
@@ -81,11 +81,11 @@
 					// We can't use the worker's savegame version, because some stuff is loaded before
 					// that
 					// packet version, and we removed the tribe name.
-					loaders.insert(Worker::load(egbase, mol, fr, tribe_lookup_table, packet_version));
+					loaders.insert(Worker::load(egbase, mol, fr, tribes_lookup_table, packet_version));
 					break;
 
 				case MapObject::HeaderWareInstance:
-					loaders.insert(WareInstance::load(egbase, mol, fr, tribe_lookup_table));
+					loaders.insert(WareInstance::load(egbase, mol, fr, tribes_lookup_table));
 					break;
 
 				case MapObject::HeaderShip:
 
=== modified file 'src/map_io/map_object_packet.h'
--- src/map_io/map_object_packet.h	2019-02-23 11:00:49 +0000
+++ src/map_io/map_object_packet.h	2019-06-08 16:55:17 +0000
@@ -57,7 +57,7 @@
 	          EditorGameBase&,
 	          MapObjectLoader&,
 	          const WorldLegacyLookupTable& world_lookup_table,
-	          const TribesLegacyLookupTable& tribe_lookup_table);
+	          const TribesLegacyLookupTable& tribes_lookup_table);
 
 	void load_finish();
 
 
=== modified file 'src/map_io/map_players_view_packet.cc'
--- src/map_io/map_players_view_packet.cc	2019-04-09 16:43:49 +0000
+++ src/map_io/map_players_view_packet.cc	2019-06-08 16:55:17 +0000
@@ -192,17 +192,17 @@
 };
 
 // reads an immovable depending on whether it is a tribe or world immovable
-const ImmovableDescr& read_immovable_type(StreamRead* fr, const EditorGameBase& egbase) {
+const ImmovableDescr& read_immovable_type(StreamRead* fr, const EditorGameBase& egbase, const TribesLegacyLookupTable& tribes_lookup_table, const WorldLegacyLookupTable& world_lookup_table) {
 	uint8_t owner = fr->unsigned_8();
 	char const* const name = fr->c_string();
 	if (owner == static_cast<uint8_t>(MapObjectDescr::OwnerType::kWorld)) {
-		DescriptionIndex const index = egbase.world().get_immovable_index(name);
+		DescriptionIndex const index = egbase.world().get_immovable_index(world_lookup_table.lookup_immovable(name));
 		if (index == Widelands::INVALID_INDEX)
 			throw WorldImmovableNonexistent(name);
 		return *egbase.world().get_immovable_descr(index);
 	} else {
 		assert(owner == static_cast<uint8_t>(MapObjectDescr::OwnerType::kTribe));
-		DescriptionIndex const index = egbase.tribes().immovable_index(name);
+		DescriptionIndex const index = egbase.tribes().immovable_index(tribes_lookup_table.lookup_immovable(name));
 		if (index == Widelands::INVALID_INDEX)
 			throw TribeImmovableNonexistent(name);
 		return *egbase.tribes().get_immovable_descr(index);
@@ -238,7 +238,7 @@
 
 inline static MapObjectData read_unseen_immovable(const EditorGameBase& egbase,
                                                   uint8_t& immovable_kind,
-                                                  FileRead& immovables_file,
+                                                  FileRead& immovables_file, const TribesLegacyLookupTable& tribes_lookup_table, const WorldLegacyLookupTable& world_lookup_table,
                                                   uint8_t& version) {
 	MapObjectData m;
 	try {
@@ -247,7 +247,7 @@
 			m.map_object_descr = nullptr;
 			break;
 		case UNSEEN_TRIBEORWORLD:  //  The player sees a tribe or world immovable.
-			m.map_object_descr = &read_immovable_type(&immovables_file, egbase);
+			m.map_object_descr = &read_immovable_type(&immovables_file, egbase, tribes_lookup_table, world_lookup_table);
 			break;
 		case UNSEEN_FLAG:  //  The player sees a flag.
 			m.map_object_descr = &g_flag_descr;
@@ -284,7 +284,7 @@
 void MapPlayersViewPacket::read(FileSystem& fs,
                                 EditorGameBase& egbase,
                                 bool const skip,
-                                MapObjectLoader&)
+                                MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table, const WorldLegacyLookupTable& world_lookup_table)
 
 {
 	if (skip)
@@ -559,7 +559,7 @@
 						                            kCurrentPacketVersionImmovableKinds);
 					}
 					MapObjectData mod = read_unseen_immovable(
-					   egbase, imm_kind, node_immovables_file, node_immovables_file_version);
+					   egbase, imm_kind, node_immovables_file, tribes_lookup_table, world_lookup_table, node_immovables_file_version);
 					f_player_field.map_object_descr = mod.map_object_descr;
 					f_player_field.constructionsite = mod.csi;
 
@@ -633,7 +633,7 @@
 					// TODO(sirver): Remove this logic the next time we break
 					// savegame compatibility.
 					read_unseen_immovable(
-					   egbase, im_kind, triangle_immovables_file, triangle_immovables_file_version);
+					   egbase, im_kind, triangle_immovables_file, tribes_lookup_table, world_lookup_table, triangle_immovables_file_version);
 				}
 				if (f_seen | br_seen | r_seen) {
 					//  The player currently sees the R triangle. Therefore his
@@ -662,7 +662,7 @@
 					// suporting immovables on the triangles instead as on the
 					// nodes.
 					read_unseen_immovable(
-					   egbase, im_kind, triangle_immovables_file, triangle_immovables_file_version);
+					   egbase, im_kind, triangle_immovables_file, tribes_lookup_table, world_lookup_table, triangle_immovables_file_version);
 				}
 
 				{  //  edges
 
=== modified file 'src/map_io/map_players_view_packet.h'
--- src/map_io/map_players_view_packet.h	2019-02-23 11:00:49 +0000
+++ src/map_io/map_players_view_packet.h	2019-06-08 16:55:17 +0000
@@ -21,6 +21,10 @@
 #define WL_MAP_IO_MAP_PLAYERS_VIEW_PACKET_H
 
 #include "map_io/map_data_packet.h"
+#include "map_io/tribes_legacy_lookup_table.h"
+#include "map_io/world_legacy_lookup_table.h"
+
+namespace Widelands {
 
 /// For each player, its view of each node, edge and triangle that he has seen
 /// but does not see currently. Information that he currently sees is not
@@ -31,6 +35,11 @@
 /// This information can not be loaded before the terrains, roads, immovables
 /// and players' vision maps are loaded. The vision maps are completely loaded
 /// after MapBobdataPacket has been loaded.
-MAP_DATA_PACKET(MapPlayersViewPacket)
+class MapPlayersViewPacket {
+public:
+	void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table, const WorldLegacyLookupTable& world_lookup_table);
+	void write(FileSystem&, EditorGameBase&, MapObjectSaver&);
+};
+}
 
 #endif  // end of include guard: WL_MAP_IO_MAP_PLAYERS_VIEW_PACKET_H
 
=== modified file 'src/map_io/map_roaddata_packet.cc'
--- src/map_io/map_roaddata_packet.cc	2019-02-23 11:00:49 +0000
+++ src/map_io/map_roaddata_packet.cc	2019-06-08 16:55:17 +0000
@@ -44,7 +44,7 @@
 void MapRoaddataPacket::read(FileSystem& fs,
                              EditorGameBase& egbase,
                              bool const skip,
-                             MapObjectLoader& mol) {
+                             MapObjectLoader& mol, const TribesLegacyLookupTable& tribes_lookup_table) {
 	if (skip)
 		return;
 
@@ -136,7 +136,7 @@
 						if (fr.unsigned_8()) {
 							(carrier_request =
 							    new Request(road, 0, Road::request_carrier_callback, wwWORKER))
-							   ->read(fr, game, mol);
+							   ->read(fr, game, mol, tribes_lookup_table);
 						} else {
 							carrier_request = nullptr;
 						}
 
=== modified file 'src/map_io/map_roaddata_packet.h'
--- src/map_io/map_roaddata_packet.h	2019-02-23 11:00:49 +0000
+++ src/map_io/map_roaddata_packet.h	2019-06-08 16:55:17 +0000
@@ -21,11 +21,18 @@
 #define WL_MAP_IO_MAP_ROADDATA_PACKET_H
 
 #include "map_io/map_data_packet.h"
+#include "map_io/tribes_legacy_lookup_table.h"
 
 /*
  * This parses the roads data (where it is, where it belongs to
  * and so on)
  */
-MAP_DATA_PACKET(MapRoaddataPacket)
+namespace Widelands {
+class MapRoaddataPacket {
+public:
+	void read(FileSystem&, EditorGameBase&, bool, MapObjectLoader&, const TribesLegacyLookupTable& tribes_lookup_table);
+	void write(FileSystem&, EditorGameBase&, MapObjectSaver&);
+};
+}
 
 #endif  // end of include guard: WL_MAP_IO_MAP_ROADDATA_PACKET_H
 
=== modified file 'src/map_io/tribes_legacy_lookup_table.cc'
--- src/map_io/tribes_legacy_lookup_table.cc	2019-05-25 10:47:18 +0000
+++ src/map_io/tribes_legacy_lookup_table.cc	2019-06-08 16:55:17 +0000
@@ -18,6 +18,7 @@
  */
 
 #include "map_io/tribes_legacy_lookup_table.h"
+
 // Whenever we break savegame compatibility, we can empty these maps
 TribesLegacyLookupTable::TribesLegacyLookupTable()
    :  // Workers
@@ -36,7 +37,7 @@
         {"reed_medium", "reedfield_medium"},
         {"reed_small", "reedfield_small"},
         {"reed_tiny", "reedfield_tiny"},
-        {"reed_tiny", "reedfield_ripe"},
+        {"reed_ripe", "reedfield_ripe"},
      } {
 }
 
 
=== modified file 'src/map_io/widelands_map_loader.cc'
--- src/map_io/widelands_map_loader.cc	2019-03-09 10:01:09 +0000
+++ src/map_io/widelands_map_loader.cc	2019-06-08 16:55:17 +0000
@@ -152,7 +152,7 @@
 
 	std::unique_ptr<WorldLegacyLookupTable> world_lookup_table(
 	   create_world_legacy_lookup_table(old_world_name_));
-	std::unique_ptr<TribesLegacyLookupTable> tribe_lookup_table(new TribesLegacyLookupTable());
+	std::unique_ptr<TribesLegacyLookupTable> tribes_lookup_table(new TribesLegacyLookupTable());
 	log("Reading Terrain Data ... ");
 	{
 		MapTerrainPacket p;
@@ -163,7 +163,7 @@
 	MapObjectPacket mapobjects;
 
 	log("Reading Map Objects ... ");
-	mapobjects.read(*fs_, egbase, *mol_, *world_lookup_table, *tribe_lookup_table);
+	mapobjects.read(*fs_, egbase, *mol_, *world_lookup_table, *tribes_lookup_table);
 	log("took %ums\n ", timer.ms_since_last_query());
 
 	log("Reading Player Start Position Data ... ");
@@ -259,21 +259,21 @@
 		log("Reading Flagdata Data ... ");
 		{
 			MapFlagdataPacket p;
-			p.read(*fs_, egbase, is_game, *mol_);
+			p.read(*fs_, egbase, is_game, *mol_, *tribes_lookup_table);
 		}
 		log("took %ums\n ", timer.ms_since_last_query());
 
 		log("Reading Roaddata Data ... ");
 		{
 			MapRoaddataPacket p;
-			p.read(*fs_, egbase, is_game, *mol_);
+			p.read(*fs_, egbase, is_game, *mol_, *tribes_lookup_table);
 		}
 		log("took %ums\n ", timer.ms_since_last_query());
 
 		log("Reading Buildingdata Data ... ");
 		{
 			MapBuildingdataPacket p;
-			p.read(*fs_, egbase, is_game, *mol_);
+			p.read(*fs_, egbase, is_game, *mol_, *tribes_lookup_table);
 		}
 		log("took %ums\n ", timer.ms_since_last_query());
 
@@ -297,7 +297,7 @@
 		log("Reading Players View Data ... ");
 		{
 			MapPlayersViewPacket p;
-			p.read(*fs_, egbase, is_game, *mol_);
+			p.read(*fs_, egbase, is_game, *mol_, *tribes_lookup_table, *world_lookup_table);
 		}
 		log("took %ums\n ", timer.ms_since_last_query());