widelands-dev team mailing list archive

[Notabilis <notabilis27@xxxxxx>]

Notabilis has proposed merging lp:~widelands-dev/widelands/net-remove-tell_ip into lp:widelands.

Commit message:
Removing the obsolete TELL_IP metaserver protocol command.

Requested reviews:
  Widelands Developers (widelands-dev)

For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/net-remove-tell_ip/+merge/343753

Removing the TELL_IP metaserver protocol command. Since online games of the current version are running over the relay server, we no longer have to know which IP versions the client supports, so there is no need to tell the metaserver about another IP address. If it is able to connect to the metaserver, it will also be able to connect to the relay.

Note: Don't merge this, I should do that myself since the protocol version does not change (yet. Will be done in another branch. I want to get in some changes to the network protocol before the feature freeze).


Something to think about: Currently, GAME_OPEN (starting to host a game (on the relay)) and GAME_CONNECT (joining a game) are transmitting up to two IPs of the relay server to the client. Since this IPs are (at least currently) always the same, we could move the IPs to the WELCOME message or simply hardcode them in Widelands (since they are the same as the metaserver address). This would remove some useless data and complexity from the metaserver protocol but we would loose some flexibility for future changes, e.g., when we want to run multiple relays on different servers for our 100000000 players. Of course we could add the IPs back to the protocol then but that would mean a change of the protocol and would require a client update. Any preferences?
-- 
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/net-remove-tell_ip into lp:widelands.

=== modified file 'src/network/gameclient.cc'
--- src/network/gameclient.cc	2018-04-07 16:59:00 +0000
+++ src/network/gameclient.cc	2018-04-21 11:57:23 +0000
@@ -106,7 +106,7 @@
 	if ((!d->net || !d->net->is_connected()) && host.second.is_valid()) {
 		// First IP did not work? Try the second IP
 		if (internet) {
-			d->net = NetClientProxy::connect(host.first, gamename);
+			d->net = NetClientProxy::connect(host.second, gamename);
 		} else {
 			d->net = NetClient::connect(host.second);
 		}

=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc	2018-04-07 16:59:00 +0000
+++ src/network/internet_gaming.cc	2018-04-21 11:57:23 +0000
@@ -177,8 +177,6 @@
 					   "", "", true, _("Users marked with IRC will possibly not react to messages."));
 				}
 
-				// Try to establish a second connection to tell the metaserver about our IPv4 address
-				create_second_connection();
 				return true;
 			} else if (error())
 				return false;
@@ -337,80 +335,6 @@
 	}
 }
 
-void InternetGaming::create_second_connection() {
-	NetAddress addr_net;
-	net->get_remote_address(&addr_net);
-	if (!addr_net.is_ipv6()) {
-		// Primary connection already is IPv4, abort
-		return;
-	}
-
-	// Do real work in thread to reduce freezing of GUI
-	// $this cannot become invalid since it is a global variable
-	// Member variables of $this might change their values but it does not really matter if the
-	// thread fails
-	std::thread([this]() {
-
-		NetAddress addr;
-		if (!NetAddress::resolve_to_v4(&addr, meta_, port_)) {
-			// Could not get the IPv4 address of the metaserver? Strange :-/
-			return;
-		}
-
-		std::unique_ptr<NetClient> tmpNet = NetClient::connect(addr);
-		if (!tmpNet || !tmpNet->is_connected()) {
-			// Connecting by IPv4 doesn't work? Well, nothing to do then
-			return;
-		}
-
-		// Okay, we have a connection. Send the login message and terminate the connection
-		SendPacket s;
-		s.string(IGPCMD_TELL_IP);
-		s.string(boost::lexical_cast<std::string>(kInternetGamingProtocolVersion));
-		s.string(clientname_);
-		s.string(reg_ ? "" : authenticator_);
-		tmpNet->send(s);
-		if (!reg_) {
-			// Not registered: We are done here
-			return;
-		}
-
-		// Wait for the challenge
-		uint32_t const secs = time(nullptr);
-		try {
-			while (kInternetGamingTimeout > time(nullptr) - secs) {
-				// Check if the connection is still open
-				if (!tmpNet->is_connected()) {
-					return;
-				}
-				// Try to get a packet
-				std::unique_ptr<RecvPacket> packet = tmpNet->try_receive();
-				if (!packet) {
-					continue;
-				}
-				const std::string cmd = packet->string();
-				if (cmd != IGPCMD_PWD_CHALLENGE) {
-					// Wrong command, abort
-					return;
-				}
-				const std::string challenge = packet->string();
-				// Got a challenge. Calculate the response and send it
-				SendPacket s2;
-				s2.string(IGPCMD_PWD_CHALLENGE);
-				s2.string(crypto::sha1(challenge + authenticator_));
-				tmpNet->send(s2);
-				// Our work is done
-				return;
-			}
-		} catch (const std::exception& e) {
-			log("InternetGaming: Error when trying to transmit secondary IP.\n");
-			return;
-		}
-
-		log("InternetGaming: Timeout when trying to transmit secondary IP.\n");
-	}).detach();
-}
-
 /// Handle one packet received from the metaserver.
 void InternetGaming::handle_packet(RecvPacket& packet) {
 	std::string cmd = packet.string();

=== modified file 'src/network/internet_gaming_protocol.h'
--- src/network/internet_gaming_protocol.h	2018-04-07 16:59:00 +0000
+++ src/network/internet_gaming_protocol.h	2018-04-21 11:57:23 +0000
@@ -108,24 +108,11 @@
  *
  * Use-cases of the UUID:
  *
- * 1) Linking connections with IPv4 and IPv6
- * The UUID is used on the metaserver to link multiple connections by the same client. This
- * normally happens when the client supports IPv4 and IPv6 and connects with both protocol versions.
- * This way, the metaserver knows that the client supports both versions and can show games / offer
- * its
- * game of/for clients with both protocol versions.
- *
- * When a network client connects to the metaserver with (RE)LOGIN it also sends the UUID.
- * When "another" netclient connects to the metaserver and sends TELL_IP containing the same UUID,
- * it is considered the same game client connecting with another IP. This way, two connections by
- * IPv4 and IPv6 can be matched so the server learns both addresses of the client.
- *
- * 2) Reconnect after crash / network problems.
+ * Reconnect after crash / network problems.
  * When Widelands breaks the connection without logging out, the server still assumes that the old
  * connection is active. So when the player reconnects, another name is chosen. Sending the UUID
- * allows
- * to reclaim the old name, since the server recognizes that there isn't a second player trying to
- * use the same name.
+ * allows to reclaim the old name, since the server recognizes that there isn't a second player
+ * trying to use the same name.
  */
 
 /**
@@ -185,27 +172,7 @@
 static const std::string IGPCMD_LOGIN = "LOGIN";
 
 /**
- * Tells the metaserver about a secondary IP address.
- *
- * Assuming the client already has a connection over IPv6 and tries to establish a secondary
- * connection over IPv4, this is the only message sent.
- * It should be sent as soon as a connection is established.
- * For unregistered users, the connection should be closed immediately following the command. No
- * answer from the server should be expected.
- * For registered users, an exchange of IGPCMD_PWD_CHALLENGE commands is done before closing the
- * connection.
- *
- * Is sent by the client, with the following payload:
- * \li string:    protocol version (see kInternetGamingProtocolVersion)
- * \li string:    client name - the one the metaserver replied at the first login
- * \li string:    for registered accounts: string of length 0
- *                for unregistered users the UUID used on login
- *                for an explanation of the UUID, see above.
- */
-static const std::string IGPCMD_TELL_IP = "TELL_IP";
-
-/**
- * This is sent by the metaserver after a IGPCMD_LOGIN or IGPCMD_TELL_IP by a registered client.
+ * This is sent by the metaserver after a IGPCMD_LOGIN by a registered client.
  * This is the first message of the a protocol similar to the challenge handshake authentication
  * protocol (CHAP) for secure transmission of the users password.
  * The server sends the nonce for hashing:
@@ -215,7 +182,7 @@
  * password:
  * \li string:    HASH_SHA1(nonce | HASH_SHA1(password))
  *
- * If the transmitted value is correct, the normal IGPCMD_LOGIN/IGPCMD_TELL_IP sequence continues.
+ * If the transmitted value is correct, the normal IGPCMD_LOGIN sequence continues.
  * If the
  * value is wrong (e.g., wrong password) the connection is terminated by the servers
  * IGPCMD_DISCONNECT.