(svn r14910) -Codechange: merge the command queue handling into a single location

7 files changed, 126 insertions, 155 deletions

diff --git a/src/core/random_func.cpp b/src/core/random_func.cpp
index ae3fcf95f..b64d5b9d6 100644
--- a/src/core/random_func.cpp
+++ b/src/core/random_func.cpp
@@ -36,7 +36,6 @@ void SetRandomSeed(uint32 seed)
 
 #ifdef RANDOM_DEBUG
 #include "../network/network_internal.h"
-#include "../variables.h" /* _frame_counter */
 #include "../company_func.h"
 
 uint32 DoRandom(int line, const char *file)
diff --git a/src/network/network.cpp b/src/network/network.cpp
index 7bc562a86..6b3137e82 100644
--- a/src/network/network.cpp
+++ b/src/network/network.cpp
@@ -64,6 +64,7 @@ char *_network_host_list[10];
 char *_network_ban_list[25];
 uint32 _frame_counter_server; // The frame_counter of the server, if in network-mode
 uint32 _frame_counter_max; // To where we may go with our clients
+uint32 _frame_counter;
 uint32 _last_sync_frame; // Used in the server to store the last time a sync packet was sent to clients.
 uint32 _broadcast_list[MAX_INTERFACES + 1];
 uint32 _network_server_bind_ip;
@@ -79,9 +80,6 @@ uint8 _network_advertise_retries;
 assert_compile((int)NETWORK_NUM_LANDSCAPES == (int)NUM_LANDSCAPE);
 assert_compile((int)NETWORK_COMPANY_NAME_LENGTH == MAX_LENGTH_COMPANY_NAME_BYTES);
 
-// global variables (declared in network_data.h)
-CommandPacket *_local_command_queue;
-
 extern NetworkUDPSocketHandler *_udp_client_socket; ///< udp client socket
 extern NetworkUDPSocketHandler *_udp_server_socket; ///< udp server socket
 extern NetworkUDPSocketHandler *_udp_master_socket; ///< udp master socket
@@ -619,13 +617,6 @@ static void NetworkClose()
 	}
 	NetworkUDPCloseAll();
 
-	/* Free all queued commands */
-	while (_local_command_queue != NULL) {
-		CommandPacket *p = _local_command_queue;
-		_local_command_queue = _local_command_queue->next;
-		free(p);
-	}
-
 	_networking = false;
 	_network_server = false;
 
@@ -639,8 +630,6 @@ static void NetworkClose()
 // Inits the network (cleans sockets and stuff)
 static void NetworkInitialize()
 {
-	_local_command_queue = NULL;
-
 	_NetworkClientSocket_pool.CleanPool();
 	_NetworkClientSocket_pool.AddBlockToPool();
 	_NetworkClientInfo_pool.CleanPool();
@@ -912,46 +901,11 @@ static void NetworkSend()
 	}
 }
 
-// Handle the local-command-queue
-static void NetworkHandleLocalQueue()
-{
-	CommandPacket *cp, **cp_prev;
-
-	cp_prev = &_local_command_queue;
-
-	while ( (cp = *cp_prev) != NULL) {
-
-		// The queue is always in order, which means
-		// that the first element will be executed first.
-		if (_frame_counter < cp->frame) break;
-
-		if (_frame_counter > cp->frame) {
-			// If we reach here, it means for whatever reason, we've already executed
-			// past the command we need to execute.
-			error("[net] Trying to execute a packet in the past!");
-		}
-
-		// We can execute this command
-		NetworkExecuteCommand(cp);
-
-		*cp_prev = cp->next;
-		free(cp);
-	}
-
-	// Just a safety check, to be removed in the future.
-	// Make sure that no older command appears towards the end of the queue
-	// In that case we missed executing it. This will never happen.
-	for (cp = _local_command_queue; cp; cp = cp->next) {
-		assert(_frame_counter < cp->frame);
-	}
-
-}
-
 static bool NetworkDoClientLoop()
 {
 	_frame_counter++;
 
-	NetworkHandleLocalQueue();
+	NetworkExecuteLocalCommandQueue();
 
 	StateGameLoop();
 
@@ -1050,7 +1004,7 @@ void NetworkGameLoop()
 			send_frame = true;
 		}
 
-		NetworkHandleLocalQueue();
+		NetworkExecuteLocalCommandQueue();
 
 		// Then we make the frame
 		StateGameLoop();
diff --git a/src/network/network_client.cpp b/src/network/network_client.cpp
index 1c56fda7c..77d18d291 100644
--- a/src/network/network_client.cpp
+++ b/src/network/network_client.cpp
@@ -673,47 +673,36 @@ DEF_CLIENT_RECEIVE_COMMAND(PACKET_SERVER_SYNC)
 
 DEF_CLIENT_RECEIVE_COMMAND(PACKET_SERVER_COMMAND)
 {
-	CommandPacket *cp = MallocT<CommandPacket>(1);
-	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();
-	cp->frame    = p->Recv_uint32();
-	cp->my_cmd   = p->Recv_bool();
-	cp->next     = NULL;
-
-	if (!IsValidCommand(cp->cmd)) {
+	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();
+	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...");
-		free(cp);
 		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
 	}
 
-	if (GetCommandFlags(cp->cmd) & CMD_OFFLINE) {
+	if (GetCommandFlags(cp.cmd) & CMD_OFFLINE) {
 		IConsolePrintF(CC_ERROR, "WARNING: offline only command from server, dropping...");
-		free(cp);
 		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
 	}
 
-	if ((cp->cmd & CMD_FLAGS_MASK) != 0) {
+	if ((cp.cmd & CMD_FLAGS_MASK) != 0) {
 		IConsolePrintF(CC_ERROR, "WARNING: invalid command flag from server, dropping...");
-		free(cp);
 		return NETWORK_RECV_STATUS_MALFORMED_PACKET;
 	}
 
 	// The server did send us this command..
 	//  queue it in our own queue, so we can handle it in the upcoming frame!
-
-	if (_local_command_queue == NULL) {
-		_local_command_queue = cp;
-	} else {
-		// Find last packet
-		CommandPacket *c = _local_command_queue;
-		while (c->next != NULL) c = c->next;
-		c->next = cp;
-	}
+	NetworkAddCommandQueue(cp);
 
 	return NETWORK_RECV_STATUS_OKAY;
 }
diff --git a/src/network/network_data.cpp b/src/network/network_command.cpp
index 4bfc12ec8..9839b2372 100644
--- a/src/network/network_data.cpp
+++ b/src/network/network_command.cpp
@@ -15,29 +15,45 @@
 #include "../date_func.h"
 #include "../company_func.h"
 
-// Add a command to the local command queue
-void NetworkAddCommandQueue(NetworkClientSocket *cs, CommandPacket *cp)
+/** Local queue of packets */
+static CommandPacket *_local_command_queue = NULL;
+
+/**
+ * Add a command to the local or client socket command queue,
+ * based on the socket.
+ * @param cp the command packet to add
+ * @param cs the socket to send to (NULL = locally)
+ */
+void NetworkAddCommandQueue(CommandPacket cp, NetworkClientSocket *cs)
 {
-	CommandPacket* new_cp = MallocT<CommandPacket>(1);
+	CommandPacket *new_cp = MallocT<CommandPacket>(1);
+	*new_cp = cp;
 
-	*new_cp = *cp;
+	CommandPacket **begin = (cs == NULL ? &_local_command_queue : &cs->command_queue);
 
-	if (cs->command_queue == NULL) {
-		cs->command_queue = new_cp;
+	if (*begin == NULL) {
+		*begin = new_cp;
 	} else {
-		CommandPacket *c = cs->command_queue;
+		CommandPacket *c = *begin;
 		while (c->next != NULL) c = c->next;
 		c->next = new_cp;
 	}
 }
 
-// Prepare a DoCommand to be send over the network
+/**
+ * Prepare a DoCommand to be send over the network
+ * @param tile The tile to perform a command on (see #CommandProc)
+ * @param p1 Additional data for the command (see #CommandProc)
+ * @param p2 Additional data for the command (see #CommandProc)
+ * @param cmd The command to execute (a CMD_* value)
+ * @param callback A callback function to call after the command is finished
+ * @param text The text to pass
+ */
 void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, CommandCallback *callback, const char *text)
 {
 	assert((cmd & CMD_FLAGS_MASK) == 0);
 
 	CommandPacket c;
-
 	c.company = _local_company;
 	c.next    = NULL;
 	c.tile    = tile;
@@ -66,24 +82,14 @@ void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comma
 		 */
 		c.frame = _frame_counter_max + 1;
 
-		CommandPacket *new_cp = MallocT<CommandPacket>(1);
-		*new_cp = c;
-		new_cp->my_cmd = true;
-		if (_local_command_queue == NULL) {
-			_local_command_queue = new_cp;
-		} else {
-			/* Find last packet */
-			CommandPacket *cp = _local_command_queue;
-			while (cp->next != NULL) cp = cp->next;
-			cp->next = new_cp;
-		}
+		NetworkAddCommandQueue(c);
 
 		/* Only the local client (in this case, the server) gets the callback */
 		c.callback = 0;
 		/* And we queue it for delivery to the clients */
 		NetworkClientSocket *cs;
 		FOR_ALL_CLIENT_SOCKETS(cs) {
-			if (cs->status > STATUS_MAP_WAIT) NetworkAddCommandQueue(cs, &c);
+			if (cs->status > STATUS_MAP_WAIT) NetworkAddCommandQueue(c, cs);
 		}
 		return;
 	}
@@ -94,8 +100,11 @@ void NetworkSend_Command(TileIndex tile, uint32 p1, uint32 p2, uint32 cmd, Comma
 	SEND_COMMAND(PACKET_CLIENT_COMMAND)(&c);
 }
 
-// Execute a DoCommand we received from the network
-void NetworkExecuteCommand(CommandPacket *cp)
+/**
+ * Execute a DoCommand we received from the network
+ * @param cp the command to execute
+ */
+static void NetworkExecuteCommand(CommandPacket *cp)
 {
 	_current_company = cp->company;
 	/* cp->callback is unsigned. so we don't need to do lower bounds checking. */
@@ -107,4 +116,43 @@ void NetworkExecuteCommand(CommandPacket *cp)
 	DoCommandP(cp->tile, cp->p1, cp->p2, cp->cmd | CMD_NETWORK_COMMAND, _callback_table[cp->callback], cp->text, cp->my_cmd);
 }
 
+/**
+ * Execute all commands on the local command queue that ought to be executed this frame.
+ */
+void NetworkExecuteLocalCommandQueue()
+{
+	while (_local_command_queue != NULL) {
+
+		/* The queue is always in order, which means
+		 * that the first element will be executed first. */
+		if (_frame_counter < _local_command_queue->frame) break;
+
+		if (_frame_counter > _local_command_queue->frame) {
+			/* If we reach here, it means for whatever reason, we've already executed
+			 * past the command we need to execute. */
+			error("[net] Trying to execute a packet in the past!");
+		}
+
+		/* We can execute this command */
+		NetworkExecuteCommand(_local_command_queue);
+
+		CommandPacket *cp = _local_command_queue;
+		_local_command_queue = _local_command_queue->next;
+		free(cp);
+	}
+}
+
+/**
+ * Free the local command queue.
+ */
+void NetworkFreeLocalCommandQueue()
+{
+	/* Free all queued commands */
+	while (_local_command_queue != NULL) {
+		CommandPacket *p = _local_command_queue;
+		_local_command_queue = _local_command_queue->next;
+		free(p);
+	}
+}
+
 #endif /* ENABLE_NETWORK */
diff --git a/src/network/network_internal.h b/src/network/network_internal.h
index 229428a90..5512528c3 100644
--- a/src/network/network_internal.h
+++ b/src/network/network_internal.h
@@ -94,6 +94,7 @@ enum NetworkLanguage {
 
 extern uint32 _frame_counter_server; // The frame_counter of the server, if in network-mode
 extern uint32 _frame_counter_max; // To where we may go with our clients
+extern uint32 _frame_counter;
 
 extern uint32 _last_sync_frame; // Used in the server to store the last time a sync packet was sent to clients.
 
@@ -119,9 +120,6 @@ extern uint16 _network_udp_broadcast;
 
 extern uint8 _network_advertise_retries;
 
-// following externs are instantiated at network.cpp
-extern CommandPacket *_local_command_queue;
-
 void NetworkTCPQueryServer(const char* host, unsigned short port);
 
 void NetworkAddServer(const char *b);
@@ -130,8 +128,10 @@ void UpdateNetworkGameWindow(bool unselect);
 
 bool IsNetworkCompatibleVersion(const char *version);
 
-void NetworkExecuteCommand(CommandPacket *cp);
-void NetworkAddCommandQueue(NetworkClientSocket *cs, CommandPacket *cp);
+/* From network_command.cpp */
+void NetworkAddCommandQueue(CommandPacket cp, NetworkClientSocket *cs = NULL);
+void NetworkExecuteLocalCommandQueue();
+void NetworkFreeLocalCommandQueue();
 
 // from network.c
 void NetworkCloseClient(NetworkClientSocket *cs);
diff --git a/src/network/network_server.cpp b/src/network/network_server.cpp
index e496bb6a5..896569497 100644
--- a/src/network/network_server.cpp
+++ b/src/network/network_server.cpp
@@ -817,12 +817,12 @@ 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
+ * @param cp the commandpacket that is going to be checked
+ * @param ci client information for debugging output to console
  */
-static bool CheckCommandFlags(const CommandPacket *cp, const NetworkClientInfo *ci)
+static bool CheckCommandFlags(CommandPacket cp, const NetworkClientInfo *ci)
 {
-	byte flags = GetCommandFlags(cp->cmd);
+	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));
@@ -834,12 +834,12 @@ static bool CheckCommandFlags(const CommandPacket *cp, const NetworkClientInfo *
 		return false;
 	}
 
-	if (cp->cmd != CMD_COMPANY_CTRL && !IsValidCompanyID(cp->company) && ci->client_id != CLIENT_ID_SERVER) {
+	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) {
+	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;
 	}
@@ -854,8 +854,6 @@ static bool CheckCommandFlags(const CommandPacket *cp, const NetworkClientInfo *
 DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 {
 	NetworkClientSocket *new_cs;
-	const NetworkClientInfo *ci;
-	byte callback;
 
 	// The client was never joined.. so this is impossible, right?
 	//  Ignore the packet, give the client a warning, and close his connection
@@ -864,34 +862,29 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 		return;
 	}
 
-	CommandPacket *cp = MallocT<CommandPacket>(1);
-	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));
+	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));
 
-	callback = p->Recv_uint8();
+	byte callback = p->Recv_uint8();
 
-	if (cs->has_quit) {
-		free(cp);
-		return;
-	}
+	if (cs->has_quit) return;
 
-	ci = cs->GetInfo();
+	const NetworkClientInfo *ci = cs->GetInfo();
 
 	/* Check if cp->cmd is valid */
-	if (!IsValidCommand(cp->cmd)) {
+	if (!IsValidCommand(cp.cmd)) {
 		IConsolePrintF(CC_ERROR, "WARNING: invalid command from client %d (IP: %s).", ci->client_id, GetClientIP(ci));
 		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_NOT_EXPECTED);
-		free(cp);
 		return;
 	}
 
 	if (!CheckCommandFlags(cp, ci)) {
 		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_KICKED);
-		free(cp);
 		return;
 	}
 
@@ -899,11 +892,10 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 	 * 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
 	 */
-	if (!(cp->cmd == CMD_COMPANY_CTRL && cp->p1 == 0 && ci->client_playas == COMPANY_NEW_COMPANY) && ci->client_playas != cp->company) {
+	if (!(cp.cmd == CMD_COMPANY_CTRL && cp.p1 == 0 && ci->client_playas == COMPANY_NEW_COMPANY) && ci->client_playas != cp.company) {
 		IConsolePrintF(CC_ERROR, "WARNING: client %d (IP: %s) tried to execute a command as company %d, kicking...",
-		               ci->client_playas + 1, GetClientIP(ci), cp->company + 1);
+		               ci->client_playas + 1, GetClientIP(ci), cp.company + 1);
 		SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_COMPANY_MISMATCH);
-		free(cp);
 		return;
 	}
 
@@ -912,24 +904,23 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 	 * is prohibited. Pretty ugly and should be redone together with its function.
 	 * @see CmdCompanyCtrl()
 	 */
-	if (cp->cmd == CMD_COMPANY_CTRL) {
-		if (cp->p1 != 0) {
+	if (cp.cmd == CMD_COMPANY_CTRL) {
+		if (cp.p1 != 0) {
 			SEND_COMMAND(PACKET_SERVER_ERROR)(cs, NETWORK_ERROR_CHEATER);
-			free(cp);
 			return;
 		}
 
 		/* XXX - Execute the command as a valid company. Normally this would be done by a
 		 * spectator, but that is not allowed any commands. So do an impersonation. The drawback
 		 * of this is that the first company's last_built_tile is also updated... */
-		cp->company = OWNER_BEGIN;
-		cp->p2 = cs->client_id;
+		cp.company = OWNER_BEGIN;
+		cp.p2 = cs->client_id;
 	}
 
 	// The frame can be executed in the same frame as the next frame-packet
 	//  That frame just before that frame is saved in _frame_counter_max
-	cp->frame = _frame_counter_max + 1;
-	cp->next  = NULL;
+	cp.frame = _frame_counter_max + 1;
+	cp.next  = NULL;
 
 	// Queue the command for the clients (are send at the end of the frame
 	//   if they can handle it ;))
@@ -937,23 +928,15 @@ DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_COMMAND)
 		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->my_cmd = (new_cs == cs);
-			NetworkAddCommandQueue(new_cs, cp);
+			cp.callback = (new_cs != cs) ? 0 : callback;
+			cp.my_cmd = (new_cs == cs);
+			NetworkAddCommandQueue(cp, new_cs);
 		}
 	}
 
-	cp->callback = 0;
-	cp->my_cmd = false;
-	// Queue the command on the server
-	if (_local_command_queue == NULL) {
-		_local_command_queue = cp;
-	} else {
-		// Find last packet
-		CommandPacket *c = _local_command_queue;
-		while (c->next != NULL) c = c->next;
-		c->next = cp;
-	}
+	cp.callback = 0;
+	cp.my_cmd = false;
+	NetworkAddCommandQueue(cp);
 }
 
 DEF_SERVER_RECEIVE_COMMAND(PACKET_CLIENT_ERROR)
diff --git a/src/variables.h b/src/variables.h
index cd8c16e33..968864214 100644
--- a/src/variables.h
+++ b/src/variables.h
@@ -36,8 +36,6 @@ VARDEF byte _trees_tick_ctr;
 /* NOSAVE: Used in palette animations only, not really important. */
 VARDEF int _palette_animation_counter;
 
-
-VARDEF uint32 _frame_counter;
 VARDEF uint32 _realtime_tick;
 
 VARDEF bool _is_old_ai_company; // current company is an oldAI company? (enables a lot of cheats..)