Review: Approve diff, test
Code changes look okay.
I am not quite sure how to test this. Saving and loading within the branch
worked without noticeable problems, loading an "old" savegame (current trunk)
failed.
Diff comments:
>
> === modified file 'src/game_io/game_player_ai_persistent_packet.cc'
> --- src/game_io/game_player_ai_persistent_packet.cc 2018-04-07 16:59:00
> +0000
> +++ src/game_io/game_player_ai_persistent_packet.cc 2018-07-13 10:36:23
> +0000
> @@ -44,129 +37,84 @@
> FileRead fr;
> fr.open(fs, "binary/player_ai");
> uint16_t const packet_version = fr.unsigned_16();
> - // TODO(GunChleoc): Savegame compatibility, remove after Build20
> - if (packet_version >= kPacketVersion2 && packet_version <=
> kCurrentPacketVersion) {
> + if (packet_version == kCurrentPacketVersion) {
> iterate_players_existing(p, nr_players, game, player)
> try {
> // Make sure that all containers are reset
> properly etc.
> player->ai_data.initialize();
> -
> - if (packet_version == kPacketVersion2) {
> - // Packet is not compatible. Consume
> without using the data.
> - fr.unsigned_8();
> - fr.unsigned_32();
> - fr.unsigned_32();
> - fr.unsigned_32();
> - fr.unsigned_16();
> - fr.unsigned_8();
> - fr.signed_16();
> - fr.unsigned_32();
> - fr.unsigned_32();
> - fr.signed_16();
> - fr.signed_32();
> - fr.unsigned_32();
> - fr.signed_32();
> - fr.unsigned_32();
> - fr.unsigned_32();
> - // Make the AI initialize itself
> - player->ai_data.initialized = 0;
> - } else {
> - // 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();
> - player->ai_data.expedition_start_time =
> fr.unsigned_32();
> - player->ai_data.ships_utilization =
> fr.unsigned_16();
> - player->ai_data.no_more_expeditions =
> (fr.unsigned_8() == 1) ? true : false;
> - player->ai_data.last_attacked_player =
> fr.signed_16();
> - player->ai_data.least_military_score =
> fr.unsigned_32();
> - player->ai_data.target_military_score =
> fr.unsigned_32();
> -
> player->ai_data.ai_productionsites_ratio = fr.unsigned_32();
> -
> player->ai_data.ai_personality_mil_upper_limit = fr.signed_32();
> -
> - // Magic numbers
> - size_t magic_numbers_size =
> fr.unsigned_32();
> -
> - // Here we deal with old savegames that
> contains 150 magic numbers only
> - if (packet_version <= kOldMagicNumbers)
> {
> - // The savegame contains less
> then expected number of magic numbers
> - assert(magic_numbers_size <
> -
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> -
> assert(player->ai_data.magic_numbers.size() ==
> -
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> - for (size_t i = 0; i <
> magic_numbers_size; ++i) {
> -
> player->ai_data.magic_numbers.at(i) = fr.signed_16();
> - }
> - // Adding '50' to missing
> possitions
> - for (size_t i =
> magic_numbers_size;
> - i <
> Widelands::Player::AiPersistentState::kMagicNumbersSize; ++i) {
> -
> player->ai_data.magic_numbers.at(i) = 50;
> - }
> - } else {
> - if (magic_numbers_size >
> -
> Widelands::Player::AiPersistentState::kMagicNumbersSize) {
> - throw
> GameDataError("Too many magic numbers: We have %" PRIuS
> - "
> but only %" PRIuS "are allowed",
> -
> magic_numbers_size,
> -
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> - }
> -
> assert(player->ai_data.magic_numbers.size() ==
> -
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> - for (size_t i = 0; i <
> magic_numbers_size; ++i) {
> -
> player->ai_data.magic_numbers.at(i) = fr.signed_16();
> - }
> - }
> -
> - // Neurons
> - const size_t neuron_pool_size =
> fr.unsigned_32();
> - if (neuron_pool_size >
> Widelands::Player::AiPersistentState::kNeuronPoolSize) {
> - throw GameDataError(
> - "Too many neurons: We have
> %" PRIuS " but only %" PRIuS "are allowed",
> - neuron_pool_size,
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> - }
> -
> assert(player->ai_data.neuron_weights.size() ==
> -
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> - for (size_t i = 0; i <
> neuron_pool_size; ++i) {
> -
> player->ai_data.neuron_weights.at(i) = fr.signed_8();
> - }
> -
> assert(player->ai_data.neuron_functs.size() ==
> -
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> - for (size_t i = 0; i <
> neuron_pool_size; ++i) {
> -
> player->ai_data.neuron_functs.at(i) = fr.signed_8();
> - }
> -
> - // F-neurons
> - const size_t f_neuron_pool_size =
> fr.unsigned_32();
> - if (f_neuron_pool_size >
> Widelands::Player::AiPersistentState::kFNeuronPoolSize) {
> - throw GameDataError(
> - "Too many f neurons: We have
> %" PRIuS " but only %" PRIuS "are allowed",
> - f_neuron_pool_size,
> Widelands::Player::AiPersistentState::kFNeuronPoolSize);
> - }
> - assert(player->ai_data.f_neurons.size()
> ==
> -
> Widelands::Player::AiPersistentState::kFNeuronPoolSize);
> - for (size_t i = 0; i <
> f_neuron_pool_size; ++i) {
> - player->ai_data.f_neurons.at(i)
> = fr.unsigned_32();
> - }
> -
> - // Remaining buildings for basic economy
> -
> assert(player->ai_data.remaining_basic_buildings.empty());
> -
> - size_t remaining_basic_buildings_size =
> fr.unsigned_32();
> - for (uint16_t i = 0; i <
> remaining_basic_buildings_size; ++i) {
> - 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() <
> -
> player->tribe().buildings().size());
> - }
> + // Contains Genetic algorithm data
I haven't read the next part, assuming only different indentation.
> + 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();
> + player->ai_data.expedition_start_time =
> fr.unsigned_32();
> + player->ai_data.ships_utilization =
> fr.unsigned_16();
> + player->ai_data.no_more_expeditions =
> (fr.unsigned_8() == 1) ? true : false;
> + player->ai_data.last_attacked_player =
> fr.signed_16();
> + player->ai_data.least_military_score =
> fr.unsigned_32();
> + player->ai_data.target_military_score =
> fr.unsigned_32();
> + player->ai_data.ai_productionsites_ratio =
> fr.unsigned_32();
> + player->ai_data.ai_personality_mil_upper_limit
> = fr.signed_32();
> +
> + // Magic numbers
> + const size_t magic_numbers_size =
> fr.unsigned_32();
> + if (magic_numbers_size >
> +
> Widelands::Player::AiPersistentState::kMagicNumbersSize) {
> + throw GameDataError("Too many magic
> numbers: We have %" PRIuS
> +
> " but only %" PRIuS "are allowed",
> +
> magic_numbers_size,
> +
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> + }
> + assert(player->ai_data.magic_numbers.size() ==
> +
> Widelands::Player::AiPersistentState::kMagicNumbersSize);
> + for (size_t i = 0; i < magic_numbers_size; ++i)
> {
> + player->ai_data.magic_numbers.at(i) =
> fr.signed_16();
> + }
> +
> + // Neurons
> + const size_t neuron_pool_size =
> fr.unsigned_32();
> + if (neuron_pool_size >
> Widelands::Player::AiPersistentState::kNeuronPoolSize) {
> + throw GameDataError(
> + "Too many neurons: We have %" PRIuS
> " but only %" PRIuS "are allowed",
> + neuron_pool_size,
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> + }
> + assert(player->ai_data.neuron_weights.size() ==
> +
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> + for (size_t i = 0; i < neuron_pool_size; ++i) {
> + player->ai_data.neuron_weights.at(i) =
> fr.signed_8();
> + }
> + assert(player->ai_data.neuron_functs.size() ==
> +
> Widelands::Player::AiPersistentState::kNeuronPoolSize);
> + for (size_t i = 0; i < neuron_pool_size; ++i) {
> + player->ai_data.neuron_functs.at(i) =
> fr.signed_8();
> + }
> +
> + // F-neurons
> + const size_t f_neuron_pool_size =
> fr.unsigned_32();
> + if (f_neuron_pool_size >
> Widelands::Player::AiPersistentState::kFNeuronPoolSize) {
> + throw GameDataError(
> + "Too many f neurons: We have %"
> PRIuS " but only %" PRIuS "are allowed",
> + f_neuron_pool_size,
> Widelands::Player::AiPersistentState::kFNeuronPoolSize);
> + }
> + assert(player->ai_data.f_neurons.size() ==
> +
> Widelands::Player::AiPersistentState::kFNeuronPoolSize);
> + for (size_t i = 0; i < f_neuron_pool_size; ++i)
> {
> + player->ai_data.f_neurons.at(i) =
> fr.unsigned_32();
> + }
> +
> + // Remaining buildings for basic economy
> +
> assert(player->ai_data.remaining_basic_buildings.empty());
> +
> + size_t remaining_basic_buildings_size =
> fr.unsigned_32();
> + for (uint16_t i = 0; i <
> remaining_basic_buildings_size; ++i) {
> + // Buildings saved as strings
> + 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() <
> + player->tribe().buildings().size());
> +
> } catch (const WException& e) {
> throw GameDataError("player %u: %s", p,
> e.what());
> }
--
https://code.launchpad.net/~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change/+merge/349390
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/remove-savegame-compatibility-after-economy-change.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
