(svn r14915) -Codechange: unify the receiving and sending of commands, and thus unifying most of the validity checking too.

7 files changed, 113 insertions, 119 deletions

diff --git a/src/command_type.h b/src/command_type.h
index ccc0df26c..e61464504 100644
--- a/src/command_type.h
+++ b/src/command_type.h
@@ -389,4 +389,16 @@ struct Command {
  */
 typedef void CommandCallback(bool success, TileIndex tile, uint32 p1, uint32 p2);
 
+/**
+ * Structure for buffering the build command when selecting a station to join.
+ */
+struct CommandContainer {
+	TileIndex tile;            ///< tile command being executed on
+	uint32 p1;                 ///< parameter p1
+	uint32 p2;                 ///< parameter p2
+	uint32 cmd;                ///< command being executed
+	CommandCallback *callback; ///< any callback function executed upon successful completion of the command
+	char text[80];             ///< possible text sent for name changes etc
+};
+
 #endif /* COMMAND_TYPE_H */
diff --git a/src/network/core/tcp.h b/src/network/core/tcp.h
index 1f5be946e..a124337a0 100644
--- a/src/network/core/tcp.h
+++ b/src/network/core/tcp.h
@@ -12,7 +12,6 @@
 #include "os_abstraction.h"
 #include "core.h"
 #include "packet.h"
-#include "../../tile_type.h"
 
 /**
  * Enum with all types of UDP packets.
@@ -59,18 +58,7 @@ enum {
 };
 
 /** Packet that wraps a command */
-struct CommandPacket {
-	CommandPacket *next; ///< the next command packet (if in queue)
-	CompanyByte company; ///< company that is executing the command
-	uint32 cmd;        ///< command being executed
-	uint32 p1;         ///< parameter p1
-	uint32 p2;         ///< parameter p2
-	TileIndex tile;    ///< tile command being executed on
-	char text[80];     ///< possible text sent for name changes etc
-	uint32 frame;      ///< the frame in which this packet is executed
-	byte callback;     ///< any callback function executed upon successful completion of the command
-	bool my_cmd;       ///< did the command originate from "me"
-};
+struct CommandPacket;
 
 /** Status of a client */
 enum ClientStatus {
@@ -120,6 +108,9 @@ public:
 	inline bool IsValid() const { return this->IsConnected(); }
 	inline void SetInfo(NetworkClientInfo *info) { assert(info != NULL && this->info == NULL); this->info = info; }
 	inline NetworkClientInfo *GetInfo() const { return this->info; }
+
+	const char *Recv_Command(Packet *p, CommandPacket *cp);
+	void Send_Command(Packet *p, const CommandPacket *cp);
 };
 
 static inline bool IsValidNetworkClientSocketIndex(ClientIndex index)
diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp
index 77d18d291..23d112ce1 100644
--- a/src/network/network_client.cpp
+++ b/src/network/network_client.cpp
@@ -205,7 +205,7 @@ DEF_CLIENT_SEND_COMMAND(PACKET_CLIENT_ACK)
 }
 
 // Send a command packet to the server
-DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_COMMAND)(CommandPacket *cp)
+DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_COMMAND)(const CommandPacket *cp)
 {
 	//
 	// Packet: CLIENT_COMMAND
@@ -221,14 +221,7 @@ DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_COMMAND)(CommandPacket *cp)
 	//
 
 	Packet *p = NetworkSend_Init(PACKET_CLIENT_COMMAND);
-
-	p->Send_uint8 (cp->company);
-	p->Send_uint32(cp->cmd);
-	p->Send_uint32(cp->p1);
-	p->Send_uint32(cp->p2);
-	p->Send_uint32((uint32)cp->tile);
-	p->Send_string(cp->text);
-	p->Send_uint8 (cp->callback);
+	MY_CLIENT->Send_Command(p, cp);
 
 	MY_CLIENT->Send_Packet(p);
 }
@@ -674,29 +667,13 @@ DEF_CLIENT_RECEIVE_COMMAND(PACKET_SERVER_SYNC)
 DEF_CLIENT_RECEIVE_COMMAND(PACKET_SERVER_COMMAND)
 {
 	CommandPacket cp;
-	cp.company  = (CompanyID)p->Recv_uint8();
-	cp.cmd      = p->Recv_uint32();
-	cp.p1       = p->Recv_uint32();
-	cp.p2       = p->Recv_uint32();
-	cp.tile     = p->Recv_uint32();
-	p->Recv_string(cp.text, sizeof(cp.text));
-	cp.callback = p->Recv_uint8();
+	const char *err = MY_CLIENT->Recv_Command(p, &cp);
 	cp.frame    = p->Recv_uint32();
 	cp.my_cmd   = p->Recv_bool();
 	cp.next     = NULL;
 
-	if (!IsValidCommand(cp.cmd)) {
-		IConsolePrintF(CC_ERROR, "WARNING: invalid command from server, dropping...");
-		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
-	}
-
-	if (GetCommandFlags(cp.cmd) & CMD_OFFLINE) {
-		IConsolePrintF(CC_ERROR, "WARNING: offline only command from server, dropping...");
-		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
-	}
-
-	if ((cp.cmd & CMD_FLAGS_MASK) != 0) {
-		IConsolePrintF(CC_ERROR, "WARNING: invalid command flag from server, dropping...");
+	if (err != NULL) {
+		IConsolePrintF(CC_ERROR, "WARNING: %s from server, dropping...", err);
 		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
 	}
 
diff --git a/src/network/network_client.h b/src/network/network_client.h
index b85ad9d5b..28ab3affe 100644
--- a/src/network/network_client.h
+++ b/src/network/network_client.h
@@ -9,7 +9,7 @@
 
 DEF_CLIENT_SEND_COMMAND(PACKET_CLIENT_GAME_INFO);
 DEF_CLIENT_SEND_COMMAND(PACKET_CLIENT_COMPANY_INFO);
-DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_COMMAND)(CommandPacket *cp);
+DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_COMMAND)(const CommandPacket *cp);
 DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_ERROR)(NetworkErrorCode errorno);
 DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_QUIT)();
 DEF_CLIENT_SEND_COMMAND_PARAM(PACKET_CLIENT_CHAT)(NetworkAction action, DestType desttype, int dest, const char *msg, int64 data);
diff --git a/src/network/network_command.cpp b/src/network/network_command.cpp
index 31df84819..52f680aaa 100644
--- a/src/network/network_command.cpp
+++ b/src/network/network_command.cpp
@@ -54,22 +54,13 @@ void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comma
 	assert((cmd & CMD_FLAGS_MASK) == 0);
 
 	CommandPacket c;
-	c.company = _local_company;
-	c.next    = NULL;
-	c.tile    = tile;
-	c.p1      = p1;
-	c.p2      = p2;
-	c.cmd     = cmd;
-
-	c.callback = 0;
-	while (c.callback < _callback_table_count && _callback_table[c.callback] != callback) {
-		c.callback++;
-	}
-
-	if (c.callback == _callback_table_count) {
-		DEBUG(net, 0, "Unknown callback. (Pointer: %p) No callback sent", callback);
-		c.callback = 0; // _callback_table[0] == NULL
-	}
+	c.company  = _local_company;
+	c.next     = NULL;
+	c.tile     = tile;
+	c.p1       = p1;
+	c.p2       = p2;
+	c.cmd      = cmd;
+	c.callback = callback;
 
 	strecpy(c.text, (text != NULL) ? text : "", lastof(c.text));
 
@@ -81,6 +72,7 @@ void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comma
 		 *   which gives about the same speed as most clients.
 		 */
 		c.frame = _frame_counter_max + 1;
+		c.my_cmd = true;
 
 		NetworkAddCommandQueue(c);
 
@@ -107,13 +99,7 @@ void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comma
 static void NetworkExecuteCommand(CommandPacket *cp)
 {
 	_current_company = cp->company;
-	/* cp->callback is unsigned. so we don't need to do lower bounds checking. */
-	if (cp->callback > _callback_table_count) {
-		DEBUG(net, 0, "Received out-of-bounds callback (%d)", cp->callback);
-		cp->callback = 0;
-	}
-
-	DoCommandP(cp->tile, cp->p1, cp->p2, cp->cmd | CMD_NETWORK_COMMAND, _callback_table[cp->callback], cp->text, cp->my_cmd);
+	DoCommandP(cp->tile, cp->p1, cp->p2, cp->cmd | CMD_NETWORK_COMMAND, cp->callback, cp->text, cp->my_cmd);
 }
 
 /**
@@ -155,4 +141,56 @@ void NetworkFreeLocalCommandQueue()
 	}
 }
 
+/**
+ * Receives a command from the network.
+ * @param p the packet to read from.
+ * @param cp the struct to write the data to.
+ * @return an error message. When NULL there has been no error.
+ */
+const char *NetworkClientSocket::Recv_Command(Packet *p, CommandPacket *cp)
+{
+	cp->company = (CompanyID)p->Recv_uint8();
+	cp->cmd     = p->Recv_uint32();
+	cp->p1      = p->Recv_uint32();
+	cp->p2      = p->Recv_uint32();
+	cp->tile    = p->Recv_uint32();
+	p->Recv_string(cp->text, lengthof(cp->text));
+
+	byte callback = p->Recv_uint8();
+
+	if (!IsValidCommand(cp->cmd))               return "invalid command";
+	if (GetCommandFlags(cp->cmd) & CMD_OFFLINE) return "offline only command";
+	if ((cp->cmd & CMD_FLAGS_MASK) != 0)        return "invalid command flag";
+	if (callback > _callback_table_count)       return "invalid callback";
+
+	cp->callback = _callback_table[callback];
+	return NULL;
+}
+
+/**
+ * Sends a command over the network.
+ * @param p the packet to send it in.
+ * @param cp the packet to actually send.
+ */
+void NetworkClientSocket::Send_Command(Packet *p, const CommandPacket *cp)
+{
+	p->Send_uint8 (cp->company);
+	p->Send_uint32(cp->cmd);
+	p->Send_uint32(cp->p1);
+	p->Send_uint32(cp->p2);
+	p->Send_uint32(cp->tile);
+	p->Send_string(cp->text);
+
+	byte callback = 0;
+	while (callback < _callback_table_count && _callback_table[callback] != cp->callback) {
+		callback++;
+	}
+
+	if (callback == _callback_table_count) {
+		DEBUG(net, 0, "Unknown callback. (Pointer: %p) No callback sent", callback);
+		callback = 0; // _callback_table[0] == NULL
+	}
+	p->Send_uint8 (callback);
+}
+
 #endif /* ENABLE_NETWORK */
diff --git a/src/network/network_internal.h b/src/network/network_internal.h
index 5512528c3..d3bb63584 100644
--- a/src/network/network_internal.h
+++ b/src/network/network_internal.h
@@ -16,6 +16,8 @@
 #include "core/packet.h"
 #include "core/tcp.h"
 
+#include "../command_type.h"
+
 /**
  * If this line is enable, every frame will have a sync test
  *  this is not needed in normal games. Normal is like 1 sync in 100
@@ -129,6 +131,16 @@ void UpdateNetworkGameWindow(bool unselect);
 bool IsNetworkCompatibleVersion(const char *version);
 
 /* From network_command.cpp */
+/**
+ * Everything we need to know about a command to be able to execute it.
+ */
+struct CommandPacket : CommandContainer {
+	CommandPacket *next; ///< the next command packet (if in queue)
+	CompanyByte company; ///< company that is executing the command
+	uint32 frame;        ///< the frame in which this packet is executed
+	bool my_cmd;         ///< did the command originate from "me"
+};
+
 void NetworkAddCommandQueue(CommandPacket cp, NetworkClientSocket *cs = NULL);
 void NetworkExecuteLocalCommandQueue();
 void NetworkFreeLocalCommandQueue();
diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp
index 896569497..051c97148 100644
--- a/src/network/network_server.cpp
+++ b/src/network/network_server.cpp
@@ -467,7 +467,7 @@ DEF_SERVER_SEND_COMMAND(PACKET_SERVER_SYNC)
 	cs->Send_Packet(p);
 }
 
-DEF_SERVER_SEND_COMMAND_PARAM(PACKET_SERVER_COMMAND)(NetworkClientSocket *cs, CommandPacket *cp)
+DEF_SERVER_SEND_COMMAND_PARAM(PACKET_SERVER_COMMAND)(NetworkClientSocket *cs, const CommandPacket *cp)
 {
 	//
 	// Packet: SERVER_COMMAND
@@ -485,13 +485,7 @@ DEF_SERVER_SEND_COMMAND_PARAM(PACKET_SERVER_COMMAND)(NetworkClientSocket *cs, Co
 
 	Packet *p = NetworkSend_Init(PACKET_SERVER_COMMAND);
 
-	p->Send_uint8 (cp->company);
-	p->Send_uint32(cp->cmd);
-	p->Send_uint32(cp->p1);
-	p->Send_uint32(cp->p2);
-	p->Send_uint32(cp->tile);
-	p->Send_string(cp->text);
-	p->Send_uint8 (cp->callback);
+	cs->Send_Command(p, cp);
 	p->Send_uint32(cp->frame);
 	p->Send_bool  (cp->my_cmd);
 
@@ -815,38 +809,6 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_MAP_OK)
 	}
 }
 
-/** Enforce the command flags.
- * Eg a server-only command can only be executed by a server, etc.
- * @param cp the commandpacket that is going to be checked
- * @param ci client information for debugging output to console
- */
-static bool CheckCommandFlags(CommandPacket cp, const NetworkClientInfo *ci)
-{
-	byte flags = GetCommandFlags(cp.cmd);
-
-	if (flags & CMD_SERVER && ci->client_id != CLIENT_ID_SERVER) {
-		IConsolePrintF(CC_ERROR, "WARNING: server only command from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
-		return false;
-	}
-
-	if (flags & CMD_OFFLINE) {
-		IConsolePrintF(CC_ERROR, "WARNING: offline only command from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
-		return false;
-	}
-
-	if (cp.cmd != CMD_COMPANY_CTRL && !IsValidCompanyID(cp.company) && ci->client_id != CLIENT_ID_SERVER) {
-		IConsolePrintF(CC_ERROR, "WARNING: spectator issueing command from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
-		return false;
-	}
-
-	if ((cp.cmd & CMD_FLAGS_MASK) != 0) {
-		IConsolePrintF(CC_ERROR, "WARNING: invalid command flag from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
-		return false;
-	}
-
-	return true;
-}
-
 /** The client has done a command and wants us to handle it
  * @param *cs the connected client that has sent the command
  * @param *p the packet in which the command was sent
@@ -863,27 +825,27 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 	}
 
 	CommandPacket cp;
-	cp.company = (CompanyID)p->Recv_uint8();
-	cp.cmd     = p->Recv_uint32();
-	cp.p1      = p->Recv_uint32();
-	cp.p2      = p->Recv_uint32();
-	cp.tile    = p->Recv_uint32();
-	p->Recv_string(cp.text, lengthof(cp.text));
-
-	byte callback = p->Recv_uint8();
+	const char *err = cs->Recv_Command(p, &cp);
 
 	if (cs->has_quit) return;
 
 	const NetworkClientInfo *ci = cs->GetInfo();
 
-	/* Check if cp->cmd is valid */
-	if (!IsValidCommand(cp.cmd)) {
-		IConsolePrintF(CC_ERROR, "WARNING: invalid command from client %d (IP: %s).", ci->client_id, GetClientIP(ci));
+	if (err != NULL) {
+		IConsolePrintF(CC_ERROR, "WARNING: %s from client %d (IP: %s).", err, ci->client_id, GetClientIP(ci));
 		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_NOT_EXPECTED);
 		return;
 	}
 
-	if (!CheckCommandFlags(cp, ci)) {
+
+	if (GetCommandFlags(cp.cmd) & CMD_SERVER && ci->client_id != CLIENT_ID_SERVER) {
+		IConsolePrintF(CC_ERROR, "WARNING: server only command from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
+		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_KICKED);
+		return;
+	}
+
+	if (cp.cmd != CMD_COMPANY_CTRL && !IsValidCompanyID(cp.company) && ci->client_id != CLIENT_ID_SERVER) {
+		IConsolePrintF(CC_ERROR, "WARNING: spectator issueing command from client %d (IP: %s), kicking...", ci->client_id, GetClientIP(ci));
 		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_KICKED);
 		return;
 	}
@@ -922,19 +884,21 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 	cp.frame = _frame_counter_max + 1;
 	cp.next  = NULL;
 
+	CommandCallback *callback = cp.callback;
+
 	// Queue the command for the clients (are send at the end of the frame
 	//   if they can handle it ;))
 	FOR_ALL_CLIENT_SOCKETS(new_cs) {
 		if (new_cs->status >= STATUS_MAP) {
 			// Callbacks are only send back to the client who sent them in the
 			//  first place. This filters that out.
-			cp.callback = (new_cs != cs) ? 0 : callback;
+			cp.callback = (new_cs != cs) ? NULL : callback;
 			cp.my_cmd = (new_cs == cs);
 			NetworkAddCommandQueue(cp, new_cs);
 		}
 	}
 
-	cp.callback = 0;
+	cp.callback = NULL;
 	cp.my_cmd = false;
 	NetworkAddCommandQueue(cp);
 }