From c31b3017222edd6e93bdeb02f05a3df7b6b23a1a Mon Sep 17 00:00:00 2001
From: sfan5 <sfan5@live.de>
Date: Mon, 14 Feb 2022 21:01:42 +0100
Subject: [PATCH] Clean up ClientReady packet handling

fixes #12073
---
 src/network/serverpackethandler.cpp | 62 +++++++++++++----------------
 src/remoteplayer.h                  |  4 +-
 src/server.cpp                      | 19 ++++-----
 3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/src/network/serverpackethandler.cpp b/src/network/serverpackethandler.cpp
index a983424ba..8163cb820 100644
--- a/src/network/serverpackethandler.cpp
+++ b/src/network/serverpackethandler.cpp
@@ -380,55 +380,47 @@ void Server::handleCommand_ClientReady(NetworkPacket* pkt)
 {
 	session_t peer_id = pkt->getPeerId();
 
-	PlayerSAO* playersao = StageTwoClientInit(peer_id);
+	// decode all information first
+	u8 major_ver, minor_ver, patch_ver, reserved;
+	u16 formspec_ver = 1; // v1 for clients older than 5.1.0-dev
+	std::string full_ver;
 
-	if (playersao == NULL) {
-		errorstream << "TOSERVER_CLIENT_READY stage 2 client init failed "
+	*pkt >> major_ver >> minor_ver >> patch_ver >> reserved >> full_ver;
+	if (pkt->getRemainingBytes() >= 2)
+		*pkt >> formspec_ver;
+
+	m_clients.setClientVersion(peer_id, major_ver, minor_ver, patch_ver,
+		full_ver);
+
+	// Emerge player
+	PlayerSAO* playersao = StageTwoClientInit(peer_id);
+	if (!playersao) {
+		errorstream << "Server: stage 2 client init failed "
 			"peer_id=" << peer_id << std::endl;
 		DisconnectPeer(peer_id);
 		return;
 	}
 
-
-	if (pkt->getSize() < 8) {
-		errorstream << "TOSERVER_CLIENT_READY client sent inconsistent data, "
-			"disconnecting peer_id: " << peer_id << std::endl;
-		DisconnectPeer(peer_id);
-		return;
-	}
-
-	u8 major_ver, minor_ver, patch_ver, reserved;
-	std::string full_ver;
-	*pkt >> major_ver >> minor_ver >> patch_ver >> reserved >> full_ver;
-
-	m_clients.setClientVersion(peer_id, major_ver, minor_ver, patch_ver,
-		full_ver);
-
-	if (pkt->getRemainingBytes() >= 2)
-		*pkt >> playersao->getPlayer()->formspec_version;
-
-	const std::vector<std::string> &players = m_clients.getPlayerNames();
-	NetworkPacket list_pkt(TOCLIENT_UPDATE_PLAYER_LIST, 0, peer_id);
-	list_pkt << (u8) PLAYER_LIST_INIT << (u16) players.size();
-	for (const std::string &player: players) {
-		list_pkt <<  player;
-	}
-	m_clients.send(peer_id, 0, &list_pkt, true);
-
-	NetworkPacket notice_pkt(TOCLIENT_UPDATE_PLAYER_LIST, 0, PEER_ID_INEXISTENT);
-	// (u16) 1 + std::string represents a pseudo vector serialization representation
-	notice_pkt << (u8) PLAYER_LIST_ADD << (u16) 1 << std::string(playersao->getPlayer()->getName());
-	m_clients.sendToAll(&notice_pkt);
+	playersao->getPlayer()->formspec_version = formspec_ver;
 	m_clients.event(peer_id, CSE_SetClientReady);
 
+	// Send player list to this client
+	{
+		const std::vector<std::string> &players = m_clients.getPlayerNames();
+		NetworkPacket list_pkt(TOCLIENT_UPDATE_PLAYER_LIST, 0, peer_id);
+		list_pkt << (u8) PLAYER_LIST_INIT << (u16) players.size();
+		for (const auto &player : players)
+			list_pkt << player;
+		Send(peer_id, &list_pkt);
+	}
+
 	s64 last_login;
 	m_script->getAuth(playersao->getPlayer()->getName(), nullptr, nullptr, &last_login);
 	m_script->on_joinplayer(playersao, last_login);
 
 	// Send shutdown timer if shutdown has been scheduled
-	if (m_shutdown_state.isTimerRunning()) {
+	if (m_shutdown_state.isTimerRunning())
 		SendChatMessage(peer_id, m_shutdown_state.getShutdownTimerMessage());
-	}
 }
 
 void Server::handleCommand_GotBlocks(NetworkPacket* pkt)
diff --git a/src/remoteplayer.h b/src/remoteplayer.h
index c8991480b..e33630841 100644
--- a/src/remoteplayer.h
+++ b/src/remoteplayer.h
@@ -128,9 +128,7 @@ public:
 	void setDirty(bool dirty) { m_dirty = true; }
 
 	u16 protocol_version = 0;
-
-	// v1 for clients older than 5.1.0-dev
-	u16 formspec_version = 1;
+	u16 formspec_version = 0;
 
 	session_t getPeerId() const { return m_peer_id; }
 
diff --git a/src/server.cpp b/src/server.cpp
index 76345686a..685d8bb25 100644
--- a/src/server.cpp
+++ b/src/server.cpp
@@ -1104,20 +1104,21 @@ PlayerSAO* Server::StageTwoClientInit(session_t peer_id)
 	SendPlayerBreath(playersao);
 
 	/*
-		Print out action
+		Update player list and print action
 	*/
 	{
-		Address addr = getPeerAddress(player->getPeerId());
-		std::string ip_str = addr.serializeString();
-		const std::vector<std::string> &names = m_clients.getPlayerNames();
+		NetworkPacket notice_pkt(TOCLIENT_UPDATE_PLAYER_LIST, 0, PEER_ID_INEXISTENT);
+		notice_pkt << (u8) PLAYER_LIST_ADD << (u16) 1 << std::string(player->getName());
+		m_clients.sendToAll(&notice_pkt);
+	}
+	{
+		std::string ip_str = getPeerAddress(player->getPeerId()).serializeString();
+		const auto &names = m_clients.getPlayerNames();
 
 		actionstream << player->getName() << " [" << ip_str << "] joins game. List of players: ";
-
-		for (const std::string &name : names) {
+		for (const std::string &name : names)
 			actionstream << name << " ";
-		}
-
-		actionstream << player->getName() <<std::endl;
+		actionstream << player->getName() << std::endl;
 	}
 	return playersao;
 }