widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

Added some comments with nits. Not tested yet.

Diff comments:

> === modified file 'src/network/gamehost.cc'
> --- src/network/gamehost.cc	2019-05-26 11:39:41 +0000
> +++ src/network/gamehost.cc	2019-07-18 15:23:31 +0000
> @@ -980,18 +981,22 @@
>  		return false;
>  
>  	// if there is one client that is currently receiving a file, we can not launch.
> +
> +	std::vector<UserSettings> &users = d->settings.users;

You can make this const

> +
>  	for (std::vector<Client>::iterator j = d->clients.begin(); j != d->clients.end(); ++j) {
> -		if (j->usernum == -1) {
> +		int usernum = j->usernum;

You can make this const

> +		if (usernum == -1) {
>  			return false;
>  		}
> -		if (!d->settings.users[j->usernum].ready) {
> +		if (users[usernum].ready) {
>  			return false;
>  		}
>  	}
>  
>  	// all players must be connected to a controller (human/ai) or be closed.
>  	// but not all should be closed!
> -	bool one_not_closed = false;
> +	bool one_not_closed = false; // TODO(k.halfmann): check this logic
>  	for (PlayerSettings& setting : d->settings.players) {
>  		if (setting.state != PlayerSettings::State::kClosed)
>  			one_not_closed = true;
> @@ -1033,16 +1039,13 @@
>  				d->clients.at(j).playernum = UserSettings::none();
>  
>  				// Broadcast change
> -				packet.reset();
> -				packet.unsigned_8(NETCMD_SETTING_USER);
> -				packet.unsigned_32(i);
> -				write_setting_user(packet, i);
> -				broadcast(packet);
> +				broadcast_setting_user(packet, d->clients.at(j).usernum);

Is this correct? I see an i replaced by a j here.

>  			}
>  	}
>  
>  	d->settings.players.resize(maxplayers);
>  
> +	// Open slots for new players found on the map.
>  	while (oldplayers < maxplayers) {
>  		PlayerSettings& player = d->settings.players.at(oldplayers);
>  		player.state = PlayerSettings::State::kOpen;
> @@ -2026,6 +2017,170 @@
>  	reaper();
>  }
>  
> +void GameHost::handle_disconnect(uint32_t const client_num, RecvPacket& r) {
> +	uint8_t number = r.unsigned_8();
> +	std::string reason = r.string();
> +	if (number == 1)

Add {}

> +		disconnect_client(client_num, reason, false);
> +	else {
> +		std::string arg = r.string();
> +		disconnect_client(client_num, reason, false, arg);
> +	}
> +	return;
> +}
> +
> +void GameHost::handle_ping(Client& client) {
> +	log("[Host]: Received ping from metaserver.\n");
> +	// Send PING back
> +	SendPacket packet;
> +	packet.unsigned_8(NETCMD_METASERVER_PING);
> +	d->net->send(client.sock_id, packet);
> +
> +	// Remove metaserver from list of clients
> +	client.playernum = UserSettings::not_connected();
> +	d->net->close(client.sock_id);
> +	client.sock_id = 0;
> +	return;
> +}
> +
> +/** Wait for NETCMD_HELLO and handle unexpected other commands */
> +void GameHost::handle_hello(uint32_t const client_num, uint8_t const cmd, Client& client, RecvPacket& r) {
> +	// Now we wait for the client to say Hi in the right language,
> +	// unless the game has already started
> +	if (d->game)

Add {}, below as well

> +		throw DisconnectException("GAME_ALREADY_STARTED");
> +
> +	if (cmd != NETCMD_HELLO)
> +		throw ProtocolException(cmd);
> +
> +	uint8_t version = r.unsigned_8();
> +	if (version != NETWORK_PROTOCOL_VERSION)
> +		throw DisconnectException("DIFFERENT_PROTOCOL_VERS");
> +
> +	std::string clientname = r.string();
> +	client.build_id = r.string();
> +
> +	welcome_client(client_num, clientname);
> +	return;
> +}
> +
> +void GameHost::handle_changetribe(Client& client, RecvPacket& r) {
> +	//  Do not be harsh about packets of this type arriving out of order -
> +	//  the client might just have had bad luck with the timing.
> +	if (!d->game) {
> +		uint8_t num = r.unsigned_8();
> +		if (num != client.playernum)

Add {}, in the functions below as well.

> +			throw DisconnectException("NO_ACCESS_TO_PLAYER");
> +		std::string tribe = r.string();
> +		bool random_tribe = r.unsigned_8() == 1;
> +		set_player_tribe(num, tribe, random_tribe);
> +	}
> +}
> +
> +/** Handle changed sharing of clients by users */
> +void GameHost::handle_changeshared(Client& client, RecvPacket& r) {
> +	//  Do not be harsh about packets of this type arriving out of order -
> +	//  the client might just have had bad luck with the timing.
> +	if (!d->game) {
> +		uint8_t num = r.unsigned_8();
> +		if (num != client.playernum)
> +			throw DisconnectException("NO_ACCESS_TO_PLAYER");
> +		set_player_shared(num, r.unsigned_8());
> +	}
> +}
> +
> +void GameHost::handle_changeteam(Client& client, RecvPacket& r) {
> +	if (!d->game) {
> +		uint8_t num = r.unsigned_8();
> +		if (num != client.playernum)
> +			throw DisconnectException("NO_ACCESS_TO_PLAYER");
> +		set_player_team(num, r.unsigned_8());
> +	}
> +}
> +
> +void GameHost::handle_changeinit(Client& client, RecvPacket& r) {
> +	if (!d->game) {
> +		// TODO(GunChleoc): For some nebulous reason, we don't receive the num that the client is
> +		// sending when a player changes slot. So, keeping the access to the client off for now.
> +		// Would be nice to have though.
> +		uint8_t num = r.unsigned_8();
> +		if (num != client.playernum)
> +			throw DisconnectException("NO_ACCESS_TO_PLAYER");
> +		set_player_init(num, r.unsigned_8());
> +	}
> +}
> +
> +void GameHost::handle_changeposition(Client& client, RecvPacket& r) {
> +	if (!d->game) {
> +		uint8_t const pos = r.unsigned_8();
> +		switch_to_player(client.usernum, pos);
> +	}
> +}
> +
> +void GameHost::handle_nettime(uint32_t const client_num, RecvPacket& r) {
> +	if (!d->game)
> +		throw DisconnectException("TIME_SENT_NOT_READY");
> +	receive_client_time(client_num, r.signed_32());
> +}
> +
> +void GameHost::handle_playercommmand(uint32_t const client_num, Client& client, RecvPacket& r) {
> +	if (!d->game)
> +		throw DisconnectException("PLAYERCMD_WO_GAME");
> +	int32_t time = r.signed_32();
> +	Widelands::PlayerCommand* plcmd = Widelands::PlayerCommand::deserialize(r);
> +	log("[Host]: Client %u (%u) sent player command %u for %u, time = %i\n", client_num, client.playernum,
> +	    static_cast<unsigned int>(plcmd->id()), plcmd->sender(), time);
> +	receive_client_time(client_num, time);
> +	if (plcmd->sender() != client.playernum + 1)
> +		throw DisconnectException("PLAYERCMD_FOR_OTHER");
> +	send_player_command(plcmd);
> +}
> +
> +void GameHost::handle_syncreport(uint32_t const client_num, Client& client, RecvPacket& r) {
> +	if (!d->game || !d->syncreport_pending || client.syncreport_arrived) {
> +		throw DisconnectException("UNEXPECTED_SYNC_REP");
> +	}
> +	int32_t time = r.signed_32();
> +	r.data(client.syncreport.data, 16);
> +	client.syncreport_arrived = true;
> +	receive_client_time(client_num, time);
> +	check_sync_reports();
> +}
> +
> +void GameHost::handle_chat(Client& client, RecvPacket& r) {
> +	ChatMessage c(r.string());
> +	c.playern = d->settings.users.at(client.usernum).position;
> +	c.sender = d->settings.users.at(client.usernum).name;
> +	if (c.msg.size() && *c.msg.begin() == '@') {
> +		// Personal message
> +		std::string::size_type const space = c.msg.find(' ');
> +		if (space >= c.msg.size() - 1)
> +			return; // No Message after '@<User>'
> +		c.recipient = c.msg.substr(1, space - 1);
> +		c.msg = c.msg.substr(space + 1);
> +	}
> +	send(c);
> +}
> +
> +/** Care for change of game speed PAUSE, 1x 2x 3x .... */

Care for -> Take care of

> +void GameHost::handle_speed(Client& client, RecvPacket& r) {
> +	client.desiredspeed = r.unsigned_16();
> +	update_network_speed();
> +}
> +
> +/** a new file should be upload to all players */

upload -> uploaded

> +void GameHost::handle_new_file(Client& client) {
> +	if (!file_)  // Do we have a file for sending?
> +		throw DisconnectException("REQUEST_OF_N_E_FILE");
> +	send_system_message_code(
> +	    "STARTED_SENDING_FILE", file_->filename, d->settings.users.at(client.usernum).name);
> +	send_file_part(client.sock_id, 0);
> +	// Remember client as "currently receiving file"
> +	d->settings.users[client.usernum].ready = false;
> +	SendPacket packet;
> +	broadcast_setting_user(packet, client.usernum);
> +}
> +
>  /**
>   * Handle a single received packet.
>   *
> @@ -2034,246 +2189,82 @@
>   * \param i the client number
>   * \param r the received packet
>   */
> -void GameHost::handle_packet(uint32_t const i, RecvPacket& r) {
> -	Client& client = d->clients.at(i);
> +void GameHost::handle_packet(uint32_t const client_num, RecvPacket& r) {
> +	Client& client = d->clients.at(client_num);
>  	uint8_t const cmd = r.unsigned_8();
>  
>  	if (cmd == NETCMD_DISCONNECT) {
> -		uint8_t number = r.unsigned_8();
> -		std::string reason = r.string();
> -		if (number == 1)
> -			disconnect_client(i, reason, false);
> -		else {
> -			std::string arg = r.string();
> -			disconnect_client(i, reason, false, arg);
> -		}
> -		return;
> +		return handle_disconnect(client_num, r);
>  	}
>  
>  	if (client.playernum == UserSettings::not_connected()) {
>  		if (cmd == NETCMD_METASERVER_PING) {
> -			log("[Host]: Received ping from metaserver.\n");
> -			// Send PING back
> -			SendPacket packet;
> -			packet.unsigned_8(NETCMD_METASERVER_PING);
> -			d->net->send(client.sock_id, packet);
> -
> -			// Remove metaserver from list of clients
> -			client.playernum = UserSettings::not_connected();
> -			d->net->close(client.sock_id);
> -			client.sock_id = 0;
> -			return;
> +			return handle_ping(client);
>  		}
> -
>  		// Now we wait for the client to say Hi in the right language,
> -		// unless the game has already started
> -		if (d->game)
> -			throw DisconnectException("GAME_ALREADY_STARTED");
> -
> -		if (cmd != NETCMD_HELLO)
> -			throw ProtocolException(cmd);
> -
> -		uint8_t version = r.unsigned_8();
> -		if (version != NETWORK_PROTOCOL_VERSION)
> -			throw DisconnectException("DIFFERENT_PROTOCOL_VERS");
> -
> -		std::string clientname = r.string();
> -		client.build_id = r.string();
> -
> -		welcome_client(i, clientname);
> -		return;
> +		return handle_hello(client_num, cmd, client, r);
>  	}
>  
>  	switch (cmd) {
> -	case NETCMD_PONG:
> -		log("[Host]: Client %u: got pong\n", i);
> -		break;
> -
> -	case NETCMD_SETTING_MAP:
> -		if (!d->game) {
> -			throw DisconnectException("NO_ACCESS_TO_SERVER");
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_CHANGETRIBE:
> -		//  Do not be harsh about packets of this type arriving out of order -
> -		//  the client might just have had bad luck with the timing.
> -		if (!d->game) {
> -			uint8_t num = r.unsigned_8();
> -			if (num != client.playernum)
> -				throw DisconnectException("NO_ACCESS_TO_PLAYER");
> -			std::string tribe = r.string();
> -			bool random_tribe = r.unsigned_8() == 1;
> -			set_player_tribe(num, tribe, random_tribe);
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_CHANGESHARED:
> -		//  Do not be harsh about packets of this type arriving out of order -
> -		//  the client might just have had bad luck with the timing.
> -		if (!d->game) {
> -			uint8_t num = r.unsigned_8();
> -			if (num != client.playernum)
> -				throw DisconnectException("NO_ACCESS_TO_PLAYER");
> -			set_player_shared(num, r.unsigned_8());
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_CHANGETEAM:
> -		if (!d->game) {
> -			uint8_t num = r.unsigned_8();
> -			if (num != client.playernum)
> -				throw DisconnectException("NO_ACCESS_TO_PLAYER");
> -			set_player_team(num, r.unsigned_8());
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_CHANGEINIT:
> -		if (!d->game) {
> -			// TODO(GunChleoc): For some nebulous reason, we don't receive the num that the client is
> -			// sending when a player changes slot. So, keeping the access to the client off for now.
> -			// Would be nice to have though.
> -			uint8_t num = r.unsigned_8();
> -			if (num != client.playernum)
> -				throw DisconnectException("NO_ACCESS_TO_PLAYER");
> -			set_player_init(num, r.unsigned_8());
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_CHANGEPOSITION:
> -		if (!d->game) {
> -			uint8_t const pos = r.unsigned_8();
> -			switch_to_player(client.usernum, pos);
> -		}
> -		break;
> -
> -	case NETCMD_SETTING_PLAYER:
> -		if (!d->game) {
> -			throw DisconnectException("NO_ACCESS_TO_SERVER");
> -		}
> -		break;
> -
> -	case NETCMD_WIN_CONDITION:
> -		if (!d->game) {
> -			throw DisconnectException("NO_ACCESS_TO_SERVER");
> -		}
> -		break;
> -
> -	case NETCMD_PEACEFUL_MODE:
> -		if (!d->game) {
> -			throw DisconnectException("NO_ACCESS_TO_SERVER");
> -		}
> -		break;
> -
> -	case NETCMD_LAUNCH:
> -		if (!d->game) {
> -			throw DisconnectException("NO_ACCESS_TO_SERVER");
> -		}
> -		break;
> -
> -	case NETCMD_TIME:
> -		if (!d->game)
> -			throw DisconnectException("TIME_SENT_NOT_READY");
> -		receive_client_time(i, r.signed_32());
> -		break;
> -
> -	case NETCMD_PLAYERCOMMAND: {
> -		if (!d->game)
> -			throw DisconnectException("PLAYERCMD_WO_GAME");
> -		int32_t time = r.signed_32();
> -		Widelands::PlayerCommand* plcmd = Widelands::PlayerCommand::deserialize(r);
> -		log("[Host]: Client %u (%u) sent player command %u for %u, time = %i\n", i, client.playernum,
> -		    static_cast<unsigned int>(plcmd->id()), plcmd->sender(), time);
> -		receive_client_time(i, time);
> -		if (plcmd->sender() != client.playernum + 1)
> -			throw DisconnectException("PLAYERCMD_FOR_OTHER");
> -		send_player_command(plcmd);
> -	} break;
> -
> -	case NETCMD_SYNCREPORT: {
> -		if (!d->game || !d->syncreport_pending || client.syncreport_arrived)
> -			throw DisconnectException("UNEXPECTED_SYNC_REP");
> -		int32_t time = r.signed_32();
> -		r.data(client.syncreport.data, 16);
> -		client.syncreport_arrived = true;
> -		receive_client_time(i, time);
> -		check_sync_reports();
> -		break;
> -	}
> -
> -	case NETCMD_CHAT: {
> -		ChatMessage c(r.string());
> -		c.playern = d->settings.users.at(client.usernum).position;
> -		c.sender = d->settings.users.at(client.usernum).name;
> -		if (c.msg.size() && *c.msg.begin() == '@') {
> -			// Personal message
> -			std::string::size_type const space = c.msg.find(' ');
> -			if (space >= c.msg.size() - 1)
> -				break;
> -			c.recipient = c.msg.substr(1, space - 1);
> -			c.msg = c.msg.substr(space + 1);
> -		}
> -		send(c);
> -		break;
> -	}
> -
> -	case NETCMD_SETSPEED: {
> -		client.desiredspeed = r.unsigned_16();
> -		update_network_speed();
> -		break;
> -	}
> -
> -	case NETCMD_NEW_FILE_AVAILABLE: {
> -		if (!file_)  // Do we have a file for sending
> -			throw DisconnectException("REQUEST_OF_N_E_FILE");
> +	    case NETCMD_PONG:
> +		log("[Host]: Client %u: got pong\n", client_num);
> +		break;
> +
> +	    case NETCMD_SETTING_CHANGETRIBE:    return handle_changetribe(client, r);
> +	    case NETCMD_SETTING_CHANGESHARED:   return handle_changeshared(client, r);
> +	    case NETCMD_SETTING_CHANGETEAM:     return handle_changeteam(client, r);
> +	    case NETCMD_SETTING_CHANGEINIT:     return handle_changeinit(client, r);
> +	    case NETCMD_SETTING_CHANGEPOSITION: return handle_changeposition(client, r);
> +	    case NETCMD_TIME:                   return handle_nettime(client_num, r);
> +	    case NETCMD_PLAYERCOMMAND:          return handle_playercommmand(client_num, client, r);
> +	    case NETCMD_SYNCREPORT:             return handle_syncreport(client_num, client, r);
> +	    case NETCMD_CHAT:                   return handle_chat(client, r);
> +	    case NETCMD_SETSPEED:               return handle_speed(client, r);
> +	    case NETCMD_NEW_FILE_AVAILABLE:     return handle_new_file(client);
> +	    case NETCMD_FILE_PART:              return handle_file_part(client, r);
> +
> +	    case NETCMD_SETTING_MAP:
> +	    case NETCMD_SETTING_PLAYER:
> +	    case NETCMD_WIN_CONDITION:
> +	    case NETCMD_PEACEFUL_MODE:
> +	    case NETCMD_LAUNCH:
> +		if (!d->game) { // not expected whe game is in progress -> somethings wrong here
> +			throw DisconnectException("NO_ACCESS_TO_SERVER");
> +		}
> +		break;
> +
> +	    default:
> +		throw ProtocolException(cmd);
> +	}
> +}
> +
> +/** Hanlde uploading part of a file  */

Hanlde -> Handle

> +void GameHost::handle_file_part(Client& client, RecvPacket& r) {
> +	if (!file_)  // Do we have a file for sending
> +		throw DisconnectException("REQUEST_OF_N_E_FILE");
> +	uint32_t part = r.unsigned_32();
> +	std::string x = r.string();
> +	if (x != file_->md5sum) {
> +		log(
> +		    "[Host]: File transfer checksum mismatch %s != %s\n", x.c_str(), file_->md5sum.c_str());
> +		return;  // Surely the file was changed, so we cancel here.
> +	}
> +	if (part >= file_->parts.size())
> +		throw DisconnectException("REQUEST_OF_N_E_FILEPART");
> +	if (part == file_->parts.size() - 1) {
>  		send_system_message_code(
> -		   "STARTED_SENDING_FILE", file_->filename, d->settings.users.at(client.usernum).name);
> -		send_file_part(client.sock_id, 0);
> -		// Remember client as "currently receiving file"
> -		d->settings.users[client.usernum].ready = false;
> +					 "COMPLETED_FILE_TRANSFER", file_->filename, d->settings.users.at(client.usernum).name);
> +		d->settings.users[client.usernum].ready = true;
>  		SendPacket packet;
> -		packet.unsigned_8(NETCMD_SETTING_USER);
> -		packet.unsigned_32(client.usernum);
> -		write_setting_user(packet, client.usernum);
> -		broadcast(packet);
> -		break;
> -	}
> -
> -	case NETCMD_FILE_PART: {
> -		if (!file_)  // Do we have a file for sending
> -			throw DisconnectException("REQUEST_OF_N_E_FILE");
> -		uint32_t part = r.unsigned_32();
> -		std::string x = r.string();
> -		if (x != file_->md5sum) {
> -			log(
> -			   "[Host]: File transfer checksum mismatch %s != %s\n", x.c_str(), file_->md5sum.c_str());
> -			return;  // Surely the file was changed, so we cancel here.
> -		}
> -		if (part >= file_->parts.size())
> -			throw DisconnectException("REQUEST_OF_N_E_FILEPART");
> -		if (part == file_->parts.size() - 1) {
> -			send_system_message_code(
> -			   "COMPLETED_FILE_TRANSFER", file_->filename, d->settings.users.at(client.usernum).name);
> -			d->settings.users[client.usernum].ready = true;
> -			SendPacket packet;
> -			packet.unsigned_8(NETCMD_SETTING_USER);
> -			packet.unsigned_32(client.usernum);
> -			write_setting_user(packet, client.usernum);
> -			broadcast(packet);
> -			return;
> -		}
> -		++part;
> -		if (part % 100 == 0)
> -			send_system_message_code("SENDING_FILE_PART",
> -			                         (boost::format("%i/%i") % part % (file_->parts.size() + 1)).str(),
> -			                         file_->filename, d->settings.users.at(client.usernum).name);
> -		send_file_part(client.sock_id, part);
> -		break;
> -	}
> -
> -	default:
> -		throw ProtocolException(cmd);
> -	}
> +		broadcast_setting_user(packet, client.usernum);
> +		return;
> +	}
> +	++part;
> +	if (part % 100 == 0) // Show Progress message every 100th transfer
> +		send_system_message_code("SENDING_FILE_PART",
> +					 (boost::format("%i/%i") % part % (file_->parts.size() + 1)).str(),
> +					 file_->filename, d->settings.users.at(client.usernum).name);
> +	send_file_part(client.sock_id, part);
>  }
>  
>  void GameHost::send_file_part(NetHostInterface::ConnectionId csock_id, uint32_t part) {
> 
> === modified file 'src/network/gamehost.h'
> --- src/network/gamehost.h	2019-05-25 07:36:44 +0000
> +++ src/network/gamehost.h	2019-07-18 15:23:31 +0000
> @@ -41,6 +41,9 @@
>   * launch, as well as dealing with the actual network protocol.
>   */
>  struct GameHost : public GameController {
> +	/** playernumber 0 actually identifies the host */
> +	static constexpr uint8_t kHostPlayerNum = 0;

kHostClientNum ? Player number 0 are the spectators.

> +
>  	GameHost(const std::string& playername, bool internet = false);
>  	~GameHost() override;
>  


-- 
https://code.launchpad.net/~widelands-dev/widelands/refactor_gamehost/+merge/370320
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gamehost into lp:widelands.