Merge lp:~widelands-dev/widelands/bug-1734199 into lp:widelands

[TiborB]

Buildings in remaining_basic_buildings are now stored as strings.

[GunChleoc (gunchleoc)]

Code LGTM, not tested. It would be even better though to reinitialize the remaining buildings when the old packet version is used, because we can't trust the numbers. I leave it up to you if you wish to change that though.

review: Approve

[TiborB (tiborb95)]

It is a big question. Usually buildings by index are fine. And alternative is to make the set empty, and this would indicate that basic economy was achieved and it might not be yet, and it would confuse AI.

So I personally prefer just to trust the indexes as it is done now...

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2876. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/308392427.
Appveyor build 2685. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1734199-2685.

[GunChleoc (gunchleoc)]

After a game has been started, using the indexes is good, because it's more efficient. During game startup, those indexes are automatically generated from names. Configuration looks like this:

    tribes:new_militarysite_type {
       name = "atlanteans_guardhall",
       descname = pgettext("atlanteans_building", "Guardhall"),

       ....
    }

This is loaded before the savegame is loaded.

So, the configurations doesn't know anything about indexes at all. Configuring it like this instead:

    tribes:new_militarysite_type {
       id = 42,
       descname = pgettext("atlanteans_building", "Guardhall"),

       ....
    }

Would not be manageable for humans.

So, whenever we add a new building type, or change the order in which we load them, using indexes during saveloading will break.

@bunnybot merge

=== modified file 'src/game_io/game_player_ai_persistent_packet.cc'
--- src/game_io/game_player_ai_persistent_packet.cc	2017-11-20 07:54:19 +0000
+++ src/game_io/game_player_ai_persistent_packet.cc	2017-11-28 11:00:28 +0000
@@ -29,7 +29,9 @@
 namespace Widelands {
 
 // Introduction of genetic algorithm with all structures that are needed for it
-constexpr uint16_t kCurrentPacketVersion = 3;
+constexpr uint16_t kCurrentPacketVersion = 4;
+// First version with genetics
+constexpr uint16_t kPacketVersion3 = 3;
 // Old Version before using genetics
 constexpr uint16_t kPacketVersion2 = 2;
 
@@ -66,7 +68,7 @@
 					// Make the AI initialize itself
 					player->ai_data.initialized = 0;
 				} else {
-					// kCurrentPacketVersion
+					// Contains Genetic algorithm data
 					player->ai_data.initialized = (fr.unsigned_8() == 1) ? true : false;
 					player->ai_data.colony_scan_area = fr.unsigned_32();
 					player->ai_data.trees_around_cutters = fr.unsigned_32();
@@ -131,8 +133,16 @@
 
 					size_t remaining_basic_buildings_size = fr.unsigned_32();
 					for (uint16_t i = 0; i < remaining_basic_buildings_size; ++i) {
-						player->ai_data.remaining_basic_buildings.emplace(
-						   static_cast<Widelands::DescriptionIndex>(fr.unsigned_32()), fr.unsigned_32());
+						if (packet_version == kPacketVersion3) {  // Old genetics (buildings saved as idx)
+							player->ai_data.remaining_basic_buildings.emplace(
+							   static_cast<Widelands::DescriptionIndex>(fr.unsigned_32()),
+							   fr.unsigned_32());
+						} else {  // New genetics (buildings saved as strigs)
+							const std::string building_string = fr.string();
+							const Widelands::DescriptionIndex bld_idx =
+							   player->tribe().building_index(building_string);
+							player->ai_data.remaining_basic_buildings.emplace(bld_idx, fr.unsigned_32());
+						}
 					}
 					// Basic sanity check for remaining basic buildings
 					assert(player->ai_data.remaining_basic_buildings.size() <
@@ -203,11 +213,12 @@
 		// Remaining buildings for basic economy
 		fw.unsigned_32(player->ai_data.remaining_basic_buildings.size());
 		for (auto bb : player->ai_data.remaining_basic_buildings) {
-			fw.unsigned_32(bb.first);
+			const std::string bld_name = game.tribes().get_building_descr(bb.first)->name().c_str();
+			fw.string(bld_name);
 			fw.unsigned_32(bb.second);
 		}
 	}
 
 	fw.write(fs, "binary/player_ai");
 }
-}
+}  // namespace Widelands