Rework client connecting and enable fallback address use

src/client/client.cpp

@@ -97,7 +97,6 @@ void PacketCounter::print(std::ostream &o) const
 Client::Client(
 		const char *playername,
 		const std::string &password,
-		const std::string &address_name,
 		MapDrawControl &control,
 		IWritableTextureSource *tsrc,
 		IWritableShaderSource *shsrc,
@@ -106,7 +105,6 @@ Client::Client(
 		ISoundManager *sound,
 		MtEventManager *event,
 		RenderingEngine *rendering_engine,
-		bool ipv6,
 		GameUI *game_ui,
 		ELoginRegister allow_login_or_register
 ):
@@ -123,8 +121,6 @@ Client::Client(
 		tsrc, this
 	),
 	m_particle_manager(std::make_unique<ParticleManager>(&m_env)),
-	m_con(new con::Connection(PROTOCOL_ID, 512, CONNECTION_TIMEOUT, ipv6, this)),
-	m_address_name(address_name),
 	m_allow_login_or_register(allow_login_or_register),
 	m_server_ser_ver(SER_FMT_VER_INVALID),
 	m_last_chat_message_sent(time(NULL)),
@@ -338,7 +334,8 @@ bool Client::isShutdown()
 Client::~Client()
 {
 	m_shutdown = true;
-	m_con->Disconnect();
+	if (m_con)
+		m_con->Disconnect();
 
 	deleteAuthData();
 
@@ -381,13 +378,32 @@ Client::~Client()
 	m_sounds_client_to_server.clear();
 }
 
-void Client::connect(Address address, bool is_local_server)
+void Client::connect(const Address &address, const std::string &address_name,
+	bool is_local_server)
 {
-	initLocalMapSaving(address, m_address_name, is_local_server);
+	if (m_con) {
+		// can't do this if the connection has entered auth phase
+		sanity_check(m_state == LC_Created && m_proto_ver == 0);
+		infostream << "Client connection will be recreated" << std::endl;
+
+		m_access_denied = false;
+		m_access_denied_reconnect = false;
+		m_access_denied_reason.clear();
+	}
+
+	m_address_name = address_name;
+	m_con.reset(new con::Connection(PROTOCOL_ID, 512, CONNECTION_TIMEOUT,
+		address.isIPv6(), this));
+
+	infostream << "Connecting to server at ";
+	address.print(infostream);
+	infostream << std::endl;
 
 	// Since we use TryReceive() a timeout here would be ineffective anyway
 	m_con->SetTimeoutMs(0);
 	m_con->Connect(address);
+
+	initLocalMapSaving(address, m_address_name, is_local_server);
 }
 
 void Client::step(float dtime)
@@ -908,6 +924,10 @@ void Client::initLocalMapSaving(const Address &address,
 	if (!g_settings->getBool("enable_local_map_saving") || is_local_server) {
 		return;
 	}
+	if (m_localdb) {
+		infostream << "Local map saving already running" << std::endl;
+		return;
+	}
 
 	std::string world_path;
 #define set_world_path(hostname) \
@@ -935,6 +955,8 @@ void Client::ReceiveAll()
 	NetworkPacket pkt;
 	u64 start_ms = porting::getTimeMs();
 	const u64 budget = 10;
+
+	FATAL_ERROR_IF(!m_con, "Networking not initialized");
 	for(;;) {
 		// Limit time even if there would be huge amounts of data to
 		// process
@@ -1767,7 +1789,7 @@ ClientEvent *Client::getClientEvent()
 
 const Address Client::getServerAddress()
 {
-	return m_con->GetPeerAddress(PEER_ID_SERVER);
+	return m_con ? m_con->GetPeerAddress(PEER_ID_SERVER) : Address();
 }
 
 float Client::mediaReceiveProgress()
@@ -1873,11 +1895,13 @@ void Client::afterContentReceived()
 
 float Client::getRTT()
 {
+	assert(m_con);
 	return m_con->getPeerStat(PEER_ID_SERVER,con::AVG_RTT);
 }
 
 float Client::getCurRate()
 {
+	assert(m_con);
 	return (m_con->getLocalStat(con::CUR_INC_RATE) +
 			m_con->getLocalStat(con::CUR_DL_RATE));
 }

src/client/client.h

@@ -122,7 +122,6 @@ public:
 	Client(
 			const char *playername,
 			const std::string &password,
-			const std::string &address_name,
 			MapDrawControl &control,
 			IWritableTextureSource *tsrc,
 			IWritableShaderSource *shsrc,
@@ -131,7 +130,6 @@ public:
 			ISoundManager *sound,
 			MtEventManager *event,
 			RenderingEngine *rendering_engine,
-			bool ipv6,
 			GameUI *game_ui,
 			ELoginRegister allow_login_or_register
 	);
@@ -155,11 +153,8 @@ public:
 
 	bool isShutdown();
 
-	/*
+	void connect(const Address &address, const std::string &address_name,
-		The name of the local player should already be set when
+		bool is_local_server);
-		calling this, as it is sent in the initialization.
-	*/
-	void connect(Address address, bool is_local_server);
 
 	/*
 		Stuff that references the environment is valid only as
@@ -351,7 +346,7 @@ public:
 	bool activeObjectsReceived() const
 	{ return m_activeobjects_received; }
 
-	u16 getProtoVersion()
+	u16 getProtoVersion() const
 	{ return m_proto_ver; }
 
 	bool m_simple_singleplayer_mode;
@@ -363,6 +358,10 @@ public:
 
 	float getRTT();
 	float getCurRate();
+	// has the server ever replied to us, used for connection retry/fallback
+	bool hasServerReplied() const {
+		return getProtoVersion() != 0; // (set in TOCLIENT_HELLO)
+	}
 
 	Minimap* getMinimap() { return m_minimap; }
 	void setCamera(Camera* camera) { m_camera = camera; }
@@ -415,8 +414,10 @@ public:
 
 	void showMinimap(bool show = true);
 
+	// IP and port we're connected to
 	const Address getServerAddress();
 
+	// Hostname of the connected server (but can also be a numerical IP)
 	const std::string &getAddressName() const
 	{
 		return m_address_name;

src/client/game.cpp

@@ -1613,13 +1613,15 @@ bool Game::connectToServer(const GameStartData &start_data,
 	*connect_ok = false;	// Let's not be overly optimistic
 	*connection_aborted = false;
 	bool local_server_mode = false;
+	const auto &address_name = start_data.address;
 
 	showOverlayMessage(N_("Resolving address..."), 0, 15);
 
 	Address connect_address(0, 0, 0, 0, start_data.socket_port);
+	Address fallback_address;
 
 	try {
-		connect_address.Resolve(start_data.address.c_str());
+		connect_address.Resolve(address_name.c_str(), &fallback_address);
 
 		if (connect_address.isAny()) {
 			// replace with localhost IP
@@ -1639,45 +1641,58 @@ bool Game::connectToServer(const GameStartData &start_data,
 		return false;
 	}
 
-	if (connect_address.isIPv6() && !g_settings->getBool("enable_ipv6")) {
+	// this shouldn't normally happen since Address::Resolve() checks for enable_ipv6
+	if (g_settings->getBool("enable_ipv6")) {
+		// empty
+	} else if (connect_address.isIPv6()) {
 		*error_message = fmtgettext("Unable to connect to %s because IPv6 is disabled", connect_address.serializeString().c_str());
 		errorstream << *error_message << std::endl;
 		return false;
+	} else if (fallback_address.isIPv6()) {
+		fallback_address = Address();
 	}
 
+	fallback_address.setPort(connect_address.getPort());
+	if (fallback_address.isValid()) {
+		infostream << "Resolved two addresses for \"" << address_name
+			<< "\" isIPv6[0]=" << connect_address.isIPv6()
+			<< " isIPv6[1]=" << fallback_address.isIPv6() << std::endl;
+	} else {
+		infostream << "Resolved one address for \"" << address_name
+			<< "\" isIPv6=" << connect_address.isIPv6() << std::endl;
+	}
+
+
 	try {
 		client = new Client(start_data.name.c_str(),
-				start_data.password, start_data.address,
+				start_data.password,
 				*draw_control, texture_src, shader_src,
 				itemdef_manager, nodedef_manager, sound_manager.get(), eventmgr,
-				m_rendering_engine, connect_address.isIPv6(), m_game_ui.get(),
+				m_rendering_engine, m_game_ui.get(),
 				start_data.allow_login_or_register);
-		client->migrateModStorage();
 	} catch (const BaseException &e) {
 		*error_message = fmtgettext("Error creating client: %s", e.what());
 		errorstream << *error_message << std::endl;
 		return false;
 	}
 
+	client->migrateModStorage();
 	client->m_simple_singleplayer_mode = simple_singleplayer_mode;
 
-	infostream << "Connecting to server at ";
-	connect_address.print(infostream);
-	infostream << std::endl;
-
-	client->connect(connect_address,
-		simple_singleplayer_mode || local_server_mode);
-
 	/*
 		Wait for server to accept connection
 	*/
 
+	client->connect(connect_address, address_name,
+		simple_singleplayer_mode || local_server_mode);
+
 	try {
 		input->clear();
 
 		FpsControl fps_control;
 		f32 dtime;
 		f32 wait_time = 0; // in seconds
+		bool did_fallback = false;
 
 		fps_control.reset();
 
@@ -1712,8 +1727,15 @@ bool Game::connectToServer(const GameStartData &start_data,
 			}
 
 			wait_time += dtime;
-			// Only time out if we aren't waiting for the server we started
+			if (local_server_mode) {
-			if (!start_data.address.empty() && wait_time > 10) {
+				// never time out
+			} else if (wait_time > GAME_FALLBACK_TIMEOUT && !did_fallback) {
+				if (!client->hasServerReplied() && fallback_address.isValid()) {
+					client->connect(fallback_address, address_name,
+						simple_singleplayer_mode || local_server_mode);
+				}
+				did_fallback = true;
+			} else if (wait_time > GAME_CONNECTION_TIMEOUT) {
 				*error_message = gettext("Connection timed out.");
 				errorstream << *error_message << std::endl;
 				break;
@@ -1723,8 +1745,7 @@ bool Game::connectToServer(const GameStartData &start_data,
 			showOverlayMessage(N_("Connecting to server..."), dtime, 20);
 		}
 	} catch (con::PeerNotFoundException &e) {
-		// TODO: Should something be done here? At least an info/error
+		warningstream << "This should not happen. Please report a bug." << std::endl;
-		// message?
 		return false;
 	}
 

src/client/game.h

@@ -43,6 +43,8 @@ struct CameraOrientation {
 	f32 camera_pitch;  // "up/down"
 };
 
+#define GAME_FALLBACK_TIMEOUT 1.8f
+#define GAME_CONNECTION_TIMEOUT 10.0f
 
 void the_game(bool *kill,
 		InputHandler *input,

src/network/clientpackethandler.cpp

@@ -163,6 +163,7 @@ void Client::handleCommand_AuthAccept(NetworkPacket* pkt)
 
 	m_state = LC_Init;
 }
+
 void Client::handleCommand_AcceptSudoMode(NetworkPacket* pkt)
 {
 	deleteAuthData();