Thanks for the review :)

I also grabbed the oldest map that I could find off the Widelands homepage and 
it loaded without problems.

@bunnybot merge

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

You assume correctly :)

> +                             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     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to