(svn r26020) [1.3] -Backport from trunk:

- Fix: [Windows] Conditional expression with enumeral with non-enumeral type (r26009)
- Fix: Game script showing vehicle on e.g. a goal, then the vehicle being removed and eventually being replaced by a non-user vehicle (most likely smoke) causing an assertion to trigger [FS#5804] (r26007, r26006)
- Fix: Crash when transferring savegame from server to client [FS#5478] (r26005)

src/lang/english.txt

@@ -2804,6 +2804,9 @@ STR_NEWGRF_INVALID_CARGO_QUANTITY                               :{COMMA} of <inv
 STR_NEWGRF_INVALID_ENGINE                                       :<invalid vehicle model>
 STR_NEWGRF_INVALID_INDUSTRYTYPE                                 :<invalid industry>
 
+# Placeholders for other invalid stuff, e.g. vehicles that have gone (Game Script).
+STR_INVALID_VEHICLE                                             :<invalid vehicle>
+
 # NewGRF scanning window
 STR_NEWGRF_SCAN_CAPTION                                         :{WHITE}Scanning NewGRFs
 STR_NEWGRF_SCAN_MESSAGE                                         :{BLACK}Scanning NewGRFs. Depending on the amount this can take a while...

src/network/network_server.cpp

@@ -57,30 +57,90 @@ struct PacketWriter : SaveFilter {
 	ServerNetworkGameSocketHandler *cs; ///< Socket we are associated with.
 	Packet *current;                    ///< The packet we're currently writing to.
 	size_t total_size;                  ///< Total size of the compressed savegame.
+	Packet *packets;                    ///< Packet queue of the savegame; send these "slowly" to the client.
+	ThreadMutex *mutex;                 ///< Mutex for making threaded saving safe.
 
 	/**
 	 * Create the packet writer.
 	 * @param cs The socket handler we're making the packets for.
 	 */
-	PacketWriter(ServerNetworkGameSocketHandler *cs) : SaveFilter(NULL), cs(cs), current(NULL), total_size(0)
+	PacketWriter(ServerNetworkGameSocketHandler *cs) : SaveFilter(NULL), cs(cs), current(NULL), total_size(0), packets(NULL)
 	{
-		this->cs->savegame_mutex = ThreadMutex::New();
+		this->mutex = ThreadMutex::New();
 	}
 
 	/** Make sure everything is cleaned up. */
 	~PacketWriter()
 	{
-		/* Prevent double frees. */
+		if (this->mutex != NULL) this->mutex->BeginCritical();
-		if (this->cs != NULL) {
-			if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical();
-			this->cs->savegame = NULL;
-			if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical();
 
-			delete this->cs->savegame_mutex;
+		if (this->cs != NULL && this->mutex != NULL) {
-			this->cs->savegame_mutex = NULL;
+			this->mutex->WaitForSignal();
+		}
+
+		/* This must all wait until the Destroy function is called. */
+
+		while (this->packets != NULL) {
+			Packet *p = this->packets->next;
+			delete this->packets;
+			this->packets = p;
 		}
 
 		delete this->current;
+
+		if (this->mutex != NULL) this->mutex->EndCritical();
+
+		delete this->mutex;
+		this->mutex = NULL;
+	}
+
+	/**
+	 * Begin the destruction of this packet writer. It can happen in two ways:
+	 * in the first case the client disconnected while saving the map. In this
+	 * case the saving has not finished and killed this PacketWriter. In that
+	 * case we simply set cs to NULL, triggering the appending to fail due to
+	 * the connection problem and eventually triggering the destructor. In the
+	 * second case the destructor is already called, and it is waiting for our
+	 * signal which we will send. Only then the packets will be removed by the
+	 * destructor.
+	 */
+	void Destroy()
+	{
+		if (this->mutex != NULL) this->mutex->BeginCritical();
+
+		this->cs = NULL;
+
+		if (this->mutex != NULL) this->mutex->SendSignal();
+
+		if (this->mutex != NULL) this->mutex->EndCritical();
+	}
+
+	/**
+	 * Checks whether there are packets.
+	 * It's not 100% threading safe, but this is only asked for when checking
+	 * whether there still is something to send. Then another call will be made
+	 * to actually get the Packet, which will be the only one popping packets
+	 * and thus eventually setting this on false.
+	 */
+	bool HasPackets()
+	{
+		return this->packets != NULL;
+	}
+
+	/**
+	 * Pop a single created packet from the queue with packets.
+	 */
+	Packet *PopPacket()
+	{
+		if (this->mutex != NULL) this->mutex->BeginCritical();
+
+		Packet *p = this->packets;
+		this->packets = p->next;
+		p->next = NULL;
+
+		if (this->mutex != NULL) this->mutex->EndCritical();
+
+		return p;
 	}
 
 	/** Append the current packet to the queue. */
@@ -88,7 +148,7 @@ struct PacketWriter : SaveFilter {
 	{
 		if (this->current == NULL) return;
 
-		Packet **p = &this->cs->savegame_packets;
+		Packet **p = &this->packets;
 		while (*p != NULL) {
 			p = &(*p)->next;
 		}
@@ -104,7 +164,7 @@ struct PacketWriter : SaveFilter {
 
 		if (this->current == NULL) this->current = new Packet(PACKET_SERVER_MAP_DATA);
 
-		if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical();
+		if (this->mutex != NULL) this->mutex->BeginCritical();
 
 		byte *bufe = buf + size;
 		while (buf != bufe) {
@@ -119,7 +179,7 @@ struct PacketWriter : SaveFilter {
 			}
 		}
 
-		if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical();
+		if (this->mutex != NULL) this->mutex->EndCritical();
 
 		this->total_size += size;
 	}
@@ -129,7 +189,7 @@ struct PacketWriter : SaveFilter {
 		/* We want to abort the saving when the socket is closed. */
 		if (this->cs == NULL) SlError(STR_NETWORK_ERROR_LOSTCONNECTION);
 
-		if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->BeginCritical();
+		if (this->mutex != NULL) this->mutex->BeginCritical();
 
 		/* Make sure the last packet is flushed. */
 		this->AppendQueue();
@@ -143,7 +203,7 @@ struct PacketWriter : SaveFilter {
 		p->Send_uint32((uint32)this->total_size);
 		this->cs->NetworkTCPSocketHandler::SendPacket(p);
 
-		if (this->cs->savegame_mutex != NULL) this->cs->savegame_mutex->EndCritical();
+		if (this->mutex != NULL) this->mutex->EndCritical();
 	}
 };
 
@@ -172,9 +232,10 @@ ServerNetworkGameSocketHandler::~ServerNetworkGameSocketHandler()
 	if (_redirect_console_to_client == this->client_id) _redirect_console_to_client = INVALID_CLIENT_ID;
 	OrderBackup::ResetUser(this->client_id);
 
-	if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical();
+	if (this->savegame != NULL) {
-	if (this->savegame != NULL) this->savegame->cs = NULL;
+		this->savegame->Destroy();
-	if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical();
+		this->savegame = NULL;
+	}
 
 	/* Make sure the saving is completely cancelled.
 	 * Yes, we need to handle the save finish as well
@@ -182,14 +243,6 @@ ServerNetworkGameSocketHandler::~ServerNetworkGameSocketHandler()
 	 * just be requesting the map and such. */
 	WaitTillSaved();
 	ProcessAsyncSaveFinish();
-
-	while (this->savegame_packets != NULL) {
-		Packet *p = this->savegame_packets->next;
-		delete this->savegame_packets;
-		this->savegame_packets = p;
-	}
-
-	delete this->savegame_mutex;
 }
 
 Packet *ServerNetworkGameSocketHandler::ReceivePacket()
@@ -205,13 +258,6 @@ Packet *ServerNetworkGameSocketHandler::ReceivePacket()
 	return p;
 }
 
-void ServerNetworkGameSocketHandler::SendPacket(Packet *packet)
-{
-	if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical();
-	this->NetworkTCPSocketHandler::SendPacket(packet);
-	if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical();
-}
-
 NetworkRecvStatus ServerNetworkGameSocketHandler::CloseConnection(NetworkRecvStatus status)
 {
 	assert(status != NETWORK_RECV_STATUS_OKAY);
@@ -556,18 +602,14 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap()
 	}
 
 	if (this->status == STATUS_MAP) {
-		if (this->savegame_mutex != NULL) this->savegame_mutex->BeginCritical();
-
 		bool last_packet = false;
+		bool has_packets = false;
 
-		for (uint i = 0; i < sent_packets && this->savegame_packets != NULL; i++) {
+		for (uint i = 0; (has_packets = this->savegame->HasPackets()) && i < sent_packets; i++) {
-			Packet *p = this->savegame_packets;
+			Packet *p = this->savegame->PopPacket();
 			last_packet = p->buffer[2] == PACKET_SERVER_MAP_DONE;
 
-			/* Remove the packet from the savegame queue and put it in the real queue. */
+			this->SendPacket(p);
-			this->savegame_packets = p->next;
-			p->next = NULL;
-			this->NetworkTCPSocketHandler::SendPacket(p);
 
 			if (last_packet) {
 				/* There is no more data, so break the for */
@@ -575,10 +617,11 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap()
 			}
 		}
 
-		if (this->savegame_mutex != NULL) this->savegame_mutex->EndCritical();
-
 		if (last_packet) {
 			/* Done reading, make sure saving is done as well */
+			this->savegame->Destroy();
+			this->savegame = NULL;
+
 			WaitTillSaved();
 
 			/* Set the status to DONE_MAP, no we will wait for the client
@@ -615,7 +658,7 @@ NetworkRecvStatus ServerNetworkGameSocketHandler::SendMap()
 
 			case SPS_ALL_SENT:
 				/* All are sent, increase the sent_packets */
-				if (this->savegame_packets != NULL) sent_packets *= 2;
+				if (has_packets) sent_packets *= 2;
 				break;
 
 			case SPS_PARTLY_SENT:

src/network/network_server.h

@@ -75,16 +75,13 @@ public:
 	CommandQueue outgoing_queue; ///< The command-queue awaiting delivery
 	int receive_limit;           ///< Amount of bytes that we can receive at this moment
 
-	Packet *savegame_packets;      ///< Packet queue of the savegame; send these "slowly" to the client.
 	struct PacketWriter *savegame; ///< Writer used to write the savegame.
-	ThreadMutex *savegame_mutex;   ///< Mutex for making threaded saving safe.
 	NetworkAddress client_address; ///< IP-address of the client (so he can be banned)
 
 	ServerNetworkGameSocketHandler(SOCKET s);
 	~ServerNetworkGameSocketHandler();
 
 	virtual Packet *ReceivePacket();
-	virtual void SendPacket(Packet *packet);
 	NetworkRecvStatus CloseConnection(NetworkRecvStatus status);
 	void GetClientName(char *client_name, size_t size) const;
 

src/strings.cpp

@@ -233,7 +233,10 @@ char *GetStringWithArgs(char *buffr, StringID string, StringParameters *args, co
 
 		case 15:
 			/* Old table for custom names. This is no longer used */
+			if (!game_script) {
 				error("Incorrect conversion of custom name string.");
+			}
+			break;
 
 		case GAME_TEXT_TAB:
 			return FormatString(buffr, GetGameStringPtr(index), args, last, case_index, true);
@@ -254,9 +257,6 @@ char *GetStringWithArgs(char *buffr, StringID string, StringParameters *args, co
 
 		case 30:
 			return FormatString(buffr, GetGRFStringPtr(index + 0x1000), args, last, case_index);
-
-		case 31:
-			NOT_REACHED();
 	}
 
 	if (index >= _langtab_num[tab]) {
@@ -1478,7 +1478,7 @@ static char *FormatString(char *buff, const char *str_arg, StringParameters *arg
 
 					StringID str;
 					switch (v->type) {
-						default: NOT_REACHED();
+						default:           str = STR_INVALID_VEHICLE; break;
 						case VEH_TRAIN:    str = STR_SV_TRAIN_NAME; break;
 						case VEH_ROAD:     str = STR_SV_ROAD_VEHICLE_NAME; break;
 						case VEH_SHIP:     str = STR_SV_SHIP_NAME; break;

src/video/cocoa/event.mm

@@ -275,7 +275,7 @@ static void QZ_KeyEvent(unsigned short keycode, unsigned short unicode, BOOL dow
 	if (down) {
 		uint32 pressed_key = QZ_MapKey(keycode);
 		/* Don't handle normal characters if an edit box has the focus. */
-		if (!EditBoxInGlobalFocus() || (!IsInsideMM(pressed_key, 'A', 'Z' + 1) && !IsInsideMM(pressed_key, '0', '9' + 1))) {
+		if (!EditBoxInGlobalFocus() || ((pressed_key & ~WKC_SPECIAL_KEYS) <= WKC_TAB) || IsInsideMM(pressed_key & ~WKC_SPECIAL_KEYS, WKC_F1, WKC_PAUSE + 1)) {
 			HandleKeypress(pressed_key, unicode);
 		}
 		DEBUG(driver, 2, "cocoa_v: QZ_KeyEvent: %x (%x), down, mapping: %x", keycode, unicode, pressed_key);

src/video/win32_v.cpp

@@ -670,7 +670,7 @@ static LRESULT CALLBACK WndProcGdi(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lP
 		case WM_KEYDOWN: {
 			/* No matter the keyboard layout, we will map the '~' to the console. */
 			uint scancode = GB(lParam, 16, 8);
-			keycode = scancode == 41 ? WKC_BACKQUOTE : MapWindowsKey(wParam);
+			keycode = scancode == 41 ? (uint)WKC_BACKQUOTE : MapWindowsKey(wParam);
 
 			/* Silently drop all messages handled by WM_CHAR. */
 			MSG msg;