widelands-dev team mailing list archive

[Miroslav Remák <miroslavr256@xxxxxxxxx>]

Review: Needs Fixing code, testing

See diff comments.

Diff comments:

> === modified file 'src/game_io/game_player_info_packet.cc'
> --- src/game_io/game_player_info_packet.cc	2016-02-16 10:27:23 +0000
> +++ src/game_io/game_player_info_packet.cc	2016-05-02 11:22:37 +0000
> @@ -72,6 +73,19 @@
>  					player.civil_blds_defeated_ = fr.unsigned_32();
>  				}
>  			}
> +
> +			// Result screen
> +			PlayersManager* manager = game.player_manager();
> +			const uint8_t no_endstatus = fr.unsigned_8();

This code should not be executed for packet version 19.

> +			for (uint8_t i = 0; i < no_endstatus; ++i) {
> +				PlayerEndStatus status;
> +				status.player = fr.unsigned_8();
> +				status.result = static_cast<PlayerEndResult>(fr.unsigned_8());
> +				status.time = fr.unsigned_32();
> +				status.info = fr.c_string();
> +				manager->set_player_end_status(status);
> +			}
> +
>  			game.read_statistics(fr);
>  		} else {
>  			throw UnhandledVersionError("GamePlayerInfoPacket", packet_version, kCurrentPacketVersion);
> 
> === modified file 'src/logic/playersmanager.cc'
> --- src/logic/playersmanager.cc	2016-02-16 10:27:23 +0000
> +++ src/logic/playersmanager.cc	2016-05-02 11:22:37 +0000
> @@ -120,5 +120,21 @@
>  	}
>  }
>  
> +void PlayersManager::set_player_end_status(const PlayerEndStatus& status)
> +{
> +	bool found = false;

This variable is not necessary. You can 'return' when a matching status is found.

> +	for (auto it = players_end_status_.begin(); it != players_end_status_.end(); ++it) {

It would be neater to use the range-based for loop syntax here.

> +		PlayerEndStatus pes = *it;
> +		if (pes.player == status.player) {
> +			pes = status;

This line is redundant.

> +			found = true;
> +			break;
> +		}
> +	}
> +	if (!found) {
> +		players_end_status_.push_back(status);
> +	}
> +}
> +
>  
>  }  // namespace Widelands


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1302593-result-screen/+merge/293521
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1302593-result-screen.