← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/bug-1732765-economy-refactoring into lp:widelands

 

There are 3 packets that have changed version - I have added comments to the diff so that you can find them. Savegames from other branches will be incompatible with this one.

Diff comments:

> 
> === modified file 'src/game_io/game_player_economies_packet.cc'
> --- src/game_io/game_player_economies_packet.cc	2018-04-07 16:59:00 +0000
> +++ src/game_io/game_player_economies_packet.cc	2018-06-19 09:26:25 +0000
> @@ -67,31 +67,21 @@
>  		FileRead fr;
>  		fr.open(fs, "binary/player_economies");
>  		uint16_t const packet_version = fr.unsigned_16();
> -		if (packet_version == 3 || packet_version == kCurrentPacketVersion) {
> +		if (packet_version == kCurrentPacketVersion) {

Version checking is here

>  			iterate_players_existing(p, nr_players, game, player) try {
> -				// In packet_version 4 we dump the number of economies a player had at
> -				// save time to debug
> -				// https://bugs.launchpad.net/widelands/+bug/1654897 which is likely
> -				// caused by players having more economies at load than they had at
> -				// save.
> -				Player::Economies& economies = player->economies_;
> -				if (packet_version > 3) {
> -					const size_t num_economies = fr.unsigned_16();
> -					if (num_economies != economies.size()) {
> -						throw GameDataError("Num economies on save (%" PRIuS
> -						                    ") != Num economies on load (%" PRIuS ")",
> -						                    num_economies, economies.size());
> -					}
> -				}
> -
> -				for (uint32_t i = 0; i < economies.size(); ++i) {
> +				const size_t num_economies = fr.unsigned_32();
> +				for (uint32_t i = 0; i < num_economies; ++i) {
>  					uint32_t value = fr.unsigned_32();
>  					if (value < 0xffffffff) {
>  						if (upcast(Flag const, flag, map[value].get_immovable())) {
> -							assert(flag->get_economy()->owner().player_number() ==
> -							       player->player_number());
> -							EconomyDataPacket d(flag->get_economy());
> -							d.read(fr);
> +							try {
> +								assert(flag->get_economy()->owner().player_number() ==
> +										 player->player_number());
> +								EconomyDataPacket d(flag->get_economy());
> +								d.read(fr);
> +							} catch (const GameDataError& e) {
> +								throw GameDataError("error reading economy data for flag at map index %d: %s", value, e.what());
> +							}
>  						} else {
>  							throw GameDataError("there is no flag at the specified location");
>  						}
> 
> === modified file 'src/logic/map_objects/tribes/ship.cc'
> --- src/logic/map_objects/tribes/ship.cc	2018-04-30 15:38:48 +0000
> +++ src/logic/map_objects/tribes/ship.cc	2018-06-19 09:26:25 +0000
> @@ -1201,28 +1217,16 @@
>  
>  MapObject::Loader* Ship::load(EditorGameBase& egbase, MapObjectLoader& mol, FileRead& fr) {
>  	std::unique_ptr<Loader> loader(new Loader);
> -
>  	try {
>  		// The header has been peeled away by the caller
>  		uint8_t const packet_version = fr.unsigned_8();
> -		if (1 <= packet_version && packet_version <= kCurrentPacketVersion) {
> +		if (packet_version == kCurrentPacketVersion) {

More version checking

>  			try {
>  				const ShipDescr* descr = nullptr;
>  				// Removing this will break the test suite
> -				if (packet_version < 5) {
> -					std::string tribe_name = fr.string();
> -					fr.c_string();  // This used to be the ship's name, which we don't need any more.
> -					if (!Widelands::tribe_exists(tribe_name)) {
> -						throw GameDataError("Tribe %s does not exist for ship", tribe_name.c_str());
> -					}
> -					const DescriptionIndex& tribe_index = egbase.tribes().tribe_index(tribe_name);
> -					const TribeDescr& tribe_descr = *egbase.tribes().get_tribe_descr(tribe_index);
> -					descr = egbase.tribes().get_ship_descr(tribe_descr.ship());
> -				} else {
> -					std::string name = fr.c_string();
> -					const DescriptionIndex& ship_index = egbase.tribes().safe_ship_index(name);
> -					descr = egbase.tribes().get_ship_descr(ship_index);
> -				}
> +				std::string name = fr.c_string();
> +				const DescriptionIndex& ship_index = egbase.tribes().safe_ship_index(name);
> +				descr = egbase.tribes().get_ship_descr(ship_index);
>  				loader->init(egbase, mol, descr->create_object());
>  				loader->load(fr);
>  			} catch (const WException& e) {
> 
> === modified file 'src/map_io/map_flag_packet.cc'
> --- src/map_io/map_flag_packet.cc	2018-04-07 16:59:00 +0000
> +++ src/map_io/map_flag_packet.cc	2018-06-19 09:26:25 +0000
> @@ -35,7 +35,7 @@
>  
>  namespace Widelands {
>  
> -constexpr uint16_t kCurrentPacketVersion = 1;
> +constexpr uint16_t kCurrentPacketVersion = 2;

We only bumped he version number for this one; version checking is unchanged, which is OK.

>  
>  void MapFlagPacket::read(FileSystem& fs,
>                           EditorGameBase& egbase,


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1732765-economy-refactoring/+merge/345277
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1732765-economy-refactoring.


References