Codechange: Un-bitstuff commands taking a ClientID (i.e. CMD_CLIENT_ID).

4 files changed, 42 insertions, 18 deletions

diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp
index cc7cd8336..1868f5e60 100644
--- a/src/network/network_client.cpp
+++ b/src/network/network_client.cpp
@@ -840,7 +840,7 @@ NetworkRecvStatus ClientNetworkGameSocketHandler::Receive_SERVER_MAP_DONE(Packet
 			 * the server will give us a client-id and let us in */
 			_network_join_status = NETWORK_JOIN_STATUS_REGISTERING;
 			ShowJoinStatusWindow();
-			Command<CMD_COMPANY_CTRL>::SendNet(STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {});
+			Command<CMD_COMPANY_CTRL>::SendNet(STR_NULL, nullptr, _local_company, CCA_NEW, INVALID_COMPANY, CRR_NONE, INVALID_CLIENT_ID);
 		}
 	} else {
 		/* take control over an existing company */
diff --git a/src/network/network_command.cpp b/src/network/network_command.cpp
index f99c71b97..58de73b19 100644
--- a/src/network/network_command.cpp
+++ b/src/network/network_command.cpp
@@ -89,15 +89,17 @@ static CommandCallback * const _callback_table[] = {
 
 template <Commands Tcmd> static CommandDataBuffer SanitizeCmdStrings(const CommandDataBuffer &data);
 template <Commands Tcmd> static void UnpackNetworkCommand(const CommandPacket *cp);
+template <Commands Tcmd> static void NetworkReplaceCommandClientId(CommandPacket &cp, ClientID client_id);
 struct CommandDispatch {
 	CommandDataBuffer(*Sanitize)(const CommandDataBuffer &);
+	void (*ReplaceClientId)(CommandPacket &, ClientID);
 	void (*Unpack)(const CommandPacket *);
 };
 
 template<typename T, T... i>
 inline constexpr auto MakeDispatchTable(std::integer_sequence<T, i...>) noexcept
 {
-	return std::array<CommandDispatch, sizeof...(i)>{{ { &SanitizeCmdStrings<static_cast<Commands>(i)>, &UnpackNetworkCommand<static_cast<Commands>(i)> }... }};
+	return std::array<CommandDispatch, sizeof...(i)>{{ { &SanitizeCmdStrings<static_cast<Commands>(i)>, &NetworkReplaceCommandClientId<static_cast<Commands>(i)>, &UnpackNetworkCommand<static_cast<Commands>(i)> }... }};
 }
 static constexpr auto _cmd_dispatch = MakeDispatchTable(std::make_integer_sequence<std::underlying_type_t<Commands>, CMD_END>{});
 
@@ -383,23 +385,45 @@ void NetworkGameSocketHandler::SendCommand(Packet *p, const CommandPacket *cp)
 	p->Send_uint8 (callback);
 }
 
-/**
- * Insert a client ID into the command data in a command packet.
- * @param cp Command packet to modify.
- * @param client_id Client id to insert.
- */
-void NetworkReplaceCommandClientId(CommandPacket &cp, ClientID client_id)
+/** Helper to process a single ClientID argument. */
+template <class T>
+static inline void SetClientIdHelper(T &data, [[maybe_unused]] ClientID client_id)
+{
+	if constexpr (std::is_same_v<ClientID, T>) {
+		data = client_id;
+	}
+}
+
+/** Set all invalid ClientID's to the proper value. */
+template<class Ttuple, size_t... Tindices>
+static inline void SetClientIds(Ttuple &values, ClientID client_id, std::index_sequence<Tindices...>)
+{
+	((SetClientIdHelper(std::get<Tindices>(values), client_id)), ...);
+}
+
+template <Commands Tcmd>
+static void NetworkReplaceCommandClientId(CommandPacket &cp, ClientID client_id)
 {
 	/* Unpack command parameters. */
-	auto params = EndianBufferReader::ToValue<std::tuple<TileIndex, uint32, uint32, std::string>>(cp.data);
+	auto params = EndianBufferReader::ToValue<typename CommandTraits<Tcmd>::Args>(cp.data);
 
 	/* Insert client id. */
-	std::get<2>(params) = client_id;
+	SetClientIds(params, client_id, std::make_index_sequence<std::tuple_size_v<decltype(params)>>{});
 
 	/* Repack command parameters. */
 	cp.data = EndianBufferWriter<CommandDataBuffer>::FromValue(params);
 }
 
+/**
+ * Insert a client ID into the command data in a command packet.
+ * @param cp Command packet to modify.
+ * @param client_id Client id to insert.
+ */
+void NetworkReplaceCommandClientId(CommandPacket &cp, ClientID client_id)
+{
+	_cmd_dispatch[cp.cmd].ReplaceClientId(cp, client_id);
+}
+
 
 /** Validate a single string argument coming from network. */
 template <class T>
diff --git a/src/network/network_gui.cpp b/src/network/network_gui.cpp
index ac12402ea..0c7e361dd 100644
--- a/src/network/network_gui.cpp
+++ b/src/network/network_gui.cpp
@@ -1396,7 +1396,7 @@ static void AdminCompanyResetCallback(Window *w, bool confirmed)
 {
 	if (confirmed) {
 		if (NetworkCompanyHasClients(_admin_company_id)) return;
-		Command<CMD_COMPANY_CTRL>::Post(0, CCA_DELETE | _admin_company_id << 16 | CRR_MANUAL << 24, 0, {});
+		Command<CMD_COMPANY_CTRL>::Post(CCA_DELETE, _admin_company_id, CRR_MANUAL, INVALID_CLIENT_ID);
 	}
 }
 
@@ -1536,9 +1536,9 @@ private:
 	static void OnClickCompanyNew(NetworkClientListWindow *w, Point pt, CompanyID company_id)
 	{
 		if (_network_server) {
-			Command<CMD_COMPANY_CTRL>::Post(0, CCA_NEW, _network_own_client_id, {});
+			Command<CMD_COMPANY_CTRL>::Post(CCA_NEW, INVALID_COMPANY, CRR_NONE, _network_own_client_id);
 		} else {
-			Command<CMD_COMPANY_CTRL>::SendNet(STR_NULL, nullptr, _local_company, 0, CCA_NEW, 0, {});
+			Command<CMD_COMPANY_CTRL>::SendNet(STR_NULL, nullptr, _local_company, CCA_NEW, INVALID_COMPANY, CRR_NONE, INVALID_CLIENT_ID);
 		}
 	}
 
diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp
index ad891aef8..197d398bf 100644
--- a/src/network/network_server.cpp
+++ b/src/network/network_server.cpp
@@ -1050,15 +1050,15 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::Receive_CLIENT_COMMAND(Packet
 	 * to match the company in the packet. If it doesn't, the client has done
 	 * something pretty naughty (or a bug), and will be kicked
 	 */
-	uint32 company_p1 = cp.cmd == CMD_COMPANY_CTRL ? std::get<1>(EndianBufferReader::ToValue<CommandTraits<CMD_COMPANY_CTRL>::Args>(cp.data)) : 0;
-	if (!(cp.cmd == CMD_COMPANY_CTRL && company_p1 == 0 && ci->client_playas == COMPANY_NEW_COMPANY) && ci->client_playas != cp.company) {
+	CompanyCtrlAction cca = cp.cmd == CMD_COMPANY_CTRL ? std::get<0>(EndianBufferReader::ToValue<CommandTraits<CMD_COMPANY_CTRL>::Args>(cp.data)) : CCA_NEW;
+	if (!(cp.cmd == CMD_COMPANY_CTRL && cca == CCA_NEW && ci->client_playas == COMPANY_NEW_COMPANY) && ci->client_playas != cp.company) {
 		IConsolePrint(CC_WARNING, "Kicking client #{} (IP: {}) due to calling a command as another company {}.",
 		               ci->client_playas + 1, this->GetClientIP(), cp.company + 1);
 		return this->SendError(NETWORK_ERROR_COMPANY_MISMATCH);
 	}
 
 	if (cp.cmd == CMD_COMPANY_CTRL) {
-		if (company_p1 != 0 || cp.company != COMPANY_SPECTATOR) {
+		if (cca != CCA_NEW || cp.company != COMPANY_SPECTATOR) {
 			return this->SendError(NETWORK_ERROR_CHEATER);
 		}
 
@@ -1556,7 +1556,7 @@ static void NetworkAutoCleanCompanies()
 			/* Is the company empty for autoclean_unprotected-months, and is there no protection? */
 			if (_settings_client.network.autoclean_unprotected != 0 && _network_company_states[c->index].months_empty > _settings_client.network.autoclean_unprotected && _network_company_states[c->index].password.empty()) {
 				/* Shut the company down */
-				Command<CMD_COMPANY_CTRL>::Post(0, CCA_DELETE | c->index << 16 | CRR_AUTOCLEAN << 24, 0, {});
+				Command<CMD_COMPANY_CTRL>::Post(CCA_DELETE, c->index, CRR_AUTOCLEAN, INVALID_CLIENT_ID);
 				IConsolePrint(CC_INFO, "Auto-cleaned company #{} with no password.", c->index + 1);
 			}
 			/* Is the company empty for autoclean_protected-months, and there is a protection? */
@@ -1570,7 +1570,7 @@ static void NetworkAutoCleanCompanies()
 			/* Is the company empty for autoclean_novehicles-months, and has no vehicles? */
 			if (_settings_client.network.autoclean_novehicles != 0 && _network_company_states[c->index].months_empty > _settings_client.network.autoclean_novehicles && vehicles_in_company[c->index] == 0) {
 				/* Shut the company down */
-				Command<CMD_COMPANY_CTRL>::Post(0, CCA_DELETE | c->index << 16 | CRR_AUTOCLEAN << 24, 0, {});
+				Command<CMD_COMPANY_CTRL>::Post(CCA_DELETE, c->index, CRR_AUTOCLEAN, INVALID_CLIENT_ID);
 				IConsolePrint(CC_INFO, "Auto-cleaned company #{} with no vehicles.", c->index + 1);
 			}
 		} else {