(svn r14395) -Fix [FS#2285]: crashes and GUI desyncs when order is deleted/modified while the timetable window is open

-Fix: close any dropdown and child windows in the Order and Timetable windows when selected order is deselected, deleted, ...

src/order_cmd.cpp

@@ -231,9 +231,17 @@ Order UnpackOldOrder(uint16 packed)
  * Updates the widgets of a vehicle which contains the order-data
  *
  */
-void InvalidateVehicleOrder(const Vehicle *v)
+void InvalidateVehicleOrder(const Vehicle *v, int data)
 {
-	InvalidateWindow(WC_VEHICLE_VIEW,      v->index);
+	InvalidateWindow(WC_VEHICLE_VIEW, v->index);
+
+	if (data != 0) {
+		/* Calls SetDirty() too */
+		InvalidateWindowData(WC_VEHICLE_ORDERS,    v->index, data);
+		InvalidateWindowData(WC_VEHICLE_TIMETABLE, v->index, data);
+		return;
+	}
+
 	InvalidateWindow(WC_VEHICLE_ORDERS,    v->index);
 	InvalidateWindow(WC_VEHICLE_TIMETABLE, v->index);
 }
@@ -558,7 +566,7 @@ CommandCost CmdInsertOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 					u->cur_order_index = cur;
 			}
 			/* Update any possible open window of the vehicle */
-			InvalidateVehicleOrder(u);
+			InvalidateVehicleOrder(u, INVALID_VEH_ORDER_ID | (sel_ord << 8));
 		}
 
 		/* As we insert an order, the order to skip to will be 'wrong'. */
@@ -592,7 +600,7 @@ static CommandCost DecloneOrder(Vehicle *dst, uint32 flags)
 {
 	if (flags & DC_EXEC) {
 		DeleteVehicleOrders(dst);
-		InvalidateVehicleOrder(dst);
+		InvalidateVehicleOrder(dst, -1);
 		InvalidateWindowClassesData(GetWindowClassForVehicleType(dst->type), 0);
 	}
 	return CommandCost();
@@ -664,7 +672,7 @@ CommandCost CmdDeleteOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 			}
 
 			/* Update any possible open window of the vehicle */
-			InvalidateVehicleOrder(u);
+			InvalidateVehicleOrder(u, sel_ord | (INVALID_VEH_ORDER_ID << 8));
 		}
 
 		/* As we delete an order, the order to skip to will be 'wrong'. */
@@ -714,7 +722,7 @@ CommandCost CmdSkipToOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 		if (v->current_order.IsType(OT_LOADING)) v->LeaveStation();
 
-		InvalidateVehicleOrder(v);
+		InvalidateVehicleOrder(v, 0);
 	}
 
 	/* We have an aircraft/ship, they have a mini-schedule, so update them all */
@@ -800,7 +808,7 @@ CommandCost CmdMoveOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 			assert(v->orders == u->orders);
 			/* Update any possible open window of the vehicle */
-			InvalidateVehicleOrder(u);
+			InvalidateVehicleOrder(u, moving_order | (target_order << 8));
 		}
 
 		/* As we move an order, the order to skip to will be 'wrong'. */
@@ -1022,7 +1030,7 @@ CommandCost CmdModifyOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 					u->current_order.GetLoadType() != order->GetLoadType()) {
 				u->current_order.SetLoadType(order->GetLoadType());
 			}
-			InvalidateVehicleOrder(u);
+			InvalidateVehicleOrder(u, 0);
 		}
 	}
 
@@ -1080,8 +1088,8 @@ CommandCost CmdCloneOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 				/* Link this vehicle in the shared-list */
 				dst->AddToShared(src);
 
-				InvalidateVehicleOrder(dst);
+				InvalidateVehicleOrder(dst, -1);
-				InvalidateVehicleOrder(src);
+				InvalidateVehicleOrder(src, 0);
 
 				InvalidateWindowClassesData(GetWindowClassForVehicleType(dst->type), 0);
 			}
@@ -1140,7 +1148,7 @@ CommandCost CmdCloneOrder(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 				dst->num_orders = src->num_orders;
 
-				InvalidateVehicleOrder(dst);
+				InvalidateVehicleOrder(dst, -1);
 
 				InvalidateWindowClassesData(GetWindowClassForVehicleType(dst->type), 0);
 			}
@@ -1185,7 +1193,7 @@ CommandCost CmdOrderRefit(TileIndex tile, uint32 flags, uint32 p1, uint32 p2)
 
 		for (Vehicle *u = v->FirstShared(); u != NULL; u = u->NextShared()) {
 			/* Update any possible open window of the vehicle */
-			InvalidateVehicleOrder(u);
+			InvalidateVehicleOrder(u, 0);
 
 			/* If the vehicle already got the current depot set as current order, then update current order as well */
 			if (u->cur_order_index == order_number && u->current_order.GetDepotOrderType() & ODTFB_PART_OF_ORDERS) {
@@ -1450,7 +1458,6 @@ void RemoveOrderFromAllVehicles(OrderType type, DestinationID destination)
 	/* Go through all vehicles */
 	FOR_ALL_VEHICLES(v) {
 		Order *order;
-		bool invalidate;
 
 		/* Forget about this station if this station is removed */
 		if (v->last_station_visited == destination && type == OT_GOTO_STATION) {
@@ -1465,20 +1472,18 @@ void RemoveOrderFromAllVehicles(OrderType type, DestinationID destination)
 		}
 
 		/* Clear the order from the order-list */
-		invalidate = false;
+		int id = -1;
 		FOR_VEHICLE_ORDERS(v, order) {
+			id++;
 			if (order->IsType(OT_GOTO_DEPOT) && (order->GetDepotActionType() & ODATFB_NEAREST_DEPOT) != 0) continue;
 			if ((v->type == VEH_AIRCRAFT && order->IsType(OT_GOTO_DEPOT) ? OT_GOTO_STATION : order->GetType()) == type &&
 					order->GetDestination() == destination) {
 				order->MakeDummy();
-				invalidate = true;
+				for (const Vehicle *w = v->FirstShared(); w != NULL; w = w->NextShared()) {
-			}
+					/* In GUI, simulate by removing the order and adding it back */
-		}
+					InvalidateVehicleOrder(w, id | (INVALID_VEH_ORDER_ID << 8));
-
+					InvalidateVehicleOrder(w, (INVALID_VEH_ORDER_ID << 8) | id);
-		/* Only invalidate once, and if needed */
+				}
-		if (invalidate) {
-			for (const Vehicle *w = v->FirstShared(); w != NULL; w = w->NextShared()) {
-				InvalidateVehicleOrder(w);
 			}
 		}
 	}
@@ -1745,7 +1750,7 @@ bool ProcessOrders(Vehicle *v)
 	/* Otherwise set it, and determine the destination tile. */
 	v->current_order = *order;
 
-	InvalidateVehicleOrder(v);
+	InvalidateVehicleOrder(v, 0);
 	switch (v->type) {
 		default:
 			NOT_REACHED();

src/order_func.h

@@ -31,7 +31,7 @@ void RestoreVehicleOrders(const Vehicle *v, const BackuppedOrders *order = &_bac
 
 /* Functions */
 void RemoveOrderFromAllVehicles(OrderType type, DestinationID destination);
-void InvalidateVehicleOrder(const Vehicle *v);
+void InvalidateVehicleOrder(const Vehicle *v, int data);
 bool VehicleHasDepotOrders(const Vehicle *v);
 void CheckOrders(const Vehicle*);
 void DeleteVehicleOrders(Vehicle *v);

src/order_gui.cpp

@@ -31,6 +31,7 @@
 #include "string_func.h"
 #include "depot_base.h"
 #include "tilehighlight_func.h"
+#include "network/network.h"
 
 #include "table/sprites.h"
 #include "table/strings.h"
@@ -549,20 +550,25 @@ private:
 	{
 		/* Don't skip when there's nothing to skip */
 		if (_ctrl_pressed && w->vehicle->cur_order_index == w->OrderGetSel()) return;
-		if (w->vehicle->num_orders == 0) return;
+		if (w->vehicle->num_orders <= 1) return;
 
 		DoCommandP(w->vehicle->tile, w->vehicle->index, _ctrl_pressed ? w->OrderGetSel() : ((w->vehicle->cur_order_index + 1) % w->vehicle->num_orders),
 				NULL, CMD_SKIP_TO_ORDER | CMD_MSG(_ctrl_pressed ? STR_CAN_T_SKIP_TO_ORDER : STR_CAN_T_SKIP_ORDER));
 	}
 
 	/**
-	 * Handle the click on the unload button.
+	 * Handle the click on the delete button.
 	 *
 	 * @param w current window
 	 */
 	static void OrderClick_Delete(OrdersWindow *w, int i)
 	{
-		DoCommandP(w->vehicle->tile, w->vehicle->index, w->OrderGetSel(), NULL, CMD_DELETE_ORDER | CMD_MSG(STR_8834_CAN_T_DELETE_THIS_ORDER));
+		/* When networking, move one order lower */
+		int selected = w->selected_order + (int)_networking;
+
+		if (DoCommandP(w->vehicle->tile, w->vehicle->index, w->OrderGetSel(), NULL, CMD_DELETE_ORDER | CMD_MSG(STR_8834_CAN_T_DELETE_THIS_ORDER))) {
+			w->selected_order = selected >= w->vehicle->num_orders ? -1 : selected;
+		}
 	}
 
 	/**
@@ -578,7 +584,7 @@ private:
 			/* Cancel refitting */
 			DoCommandP(w->vehicle->tile, w->vehicle->index, (w->OrderGetSel() << 16) | (CT_NO_REFIT << 8) | CT_NO_REFIT, NULL, CMD_ORDER_REFIT);
 		} else {
-			ShowVehicleRefitWindow(w->vehicle, w->OrderGetSel());
+			ShowVehicleRefitWindow(w->vehicle, w->OrderGetSel(), w);
 		}
 	}
 	typedef void Handler(OrdersWindow*, int);
@@ -603,10 +609,54 @@ public:
 		this->FindWindowPlacementAndResize(desc);
 	}
 
-	virtual void OnInvalidateData(int data = 0)
+	virtual void OnInvalidateData(int data)
 	{
-		/* Autoreplace replaced the vehicle */
+		switch (data) {
-		this->vehicle = GetVehicle(this->window_number);
+			case 0:
+				/* Autoreplace replaced the vehicle */
+				this->vehicle = GetVehicle(this->window_number);
+				break;
+
+			case -1:
+				/* Removed / replaced all orders (after deleting / sharing) */
+				if (this->selected_order == -1) break;
+
+				this->DeleteChildWindows();
+				HideDropDownMenu(this);
+				this->selected_order = -1;
+				break;
+
+			default: {
+				/* Moving an order. If one of these is INVALID_VEH_ORDER_ID, then
+				 * the order is being created / removed */
+				if (this->selected_order == -1) break;
+
+				VehicleOrderID from = GB(data, 0, 8);
+				VehicleOrderID to   = GB(data, 8, 8);
+
+				if (from == to) break; // no need to change anything
+
+				if (from != this->selected_order) {
+					/* Moving from preceeding order? */
+					this->selected_order -= (int)(from <= this->selected_order);
+					/* Moving to   preceeding order? */
+					this->selected_order += (int)(to   <= this->selected_order);
+					break;
+				}
+
+				/* Now we are modifying the selected order */
+				if (to == INVALID_VEH_ORDER_ID) {
+					/* Deleting selected order */
+					this->DeleteChildWindows();
+					HideDropDownMenu(this);
+					this->selected_order = -1;
+					break;
+				}
+
+				/* Moving selected order */
+				this->selected_order = to;
+			} break;
+		}
 	}
 
 	virtual void OnPaint()
@@ -753,14 +803,6 @@ public:
 
 				int sel = this->GetOrderFromPt(pt.y);
 
-				if (sel == INVALID_ORDER) {
-					/* This was a click on an empty part of the orders window, so
-					* deselect the currently selected order. */
-					this->selected_order = -1;
-					this->SetDirty();
-					return;
-				}
-
 				if (_ctrl_pressed && sel < this->vehicle->num_orders) {
 					const Order *ord = GetVehicleOrder(this->vehicle, sel);
 					TileIndex xy;
@@ -774,18 +816,22 @@ public:
 
 					if (xy != 0) ScrollMainWindowToTile(xy);
 					return;
-				} else {
+				}
-					if (sel == this->selected_order) {
-						/* Deselect clicked order */
-						this->selected_order = -1;
-					} else {
-						/* Select clicked order */
-						this->selected_order = sel;
 
-						if (this->vehicle->owner == _local_player) {
+				/* This order won't be selected any more, close all child windows and dropdowns */
-							/* Activate drag and drop */
+				this->DeleteChildWindows();
-							SetObjectToPlaceWnd(SPR_CURSOR_MOUSE, PAL_NONE, VHM_DRAG, this);
+				HideDropDownMenu(this);
-						}
+
+				if (sel == INVALID_ORDER || sel == this->selected_order) {
+					/* Deselect clicked order */
+					this->selected_order = -1;
+				} else {
+					/* Select clicked order */
+					this->selected_order = sel;
+
+					if (this->vehicle->owner == _local_player) {
+						/* Activate drag and drop */
+						SetObjectToPlaceWnd(SPR_CURSOR_MOUSE, PAL_NONE, VHM_DRAG, this);
 					}
 				}
 
@@ -979,7 +1025,6 @@ public:
 			if (!cmd.IsValid()) return;
 
 			if (DoCommandP(this->vehicle->tile, this->vehicle->index + (this->OrderGetSel() << 16), cmd.Pack(), NULL, CMD_INSERT_ORDER | CMD_MSG(STR_8833_CAN_T_INSERT_NEW_ORDER))) {
-				if (this->selected_order != -1) this->selected_order++;
 				ResetObjectToPlace();
 			}
 		}

src/ship_cmd.cpp

@@ -627,7 +627,7 @@ static void ShipController(Vehicle *v)
 						UpdateVehicleTimetable(v, true);
 						v->cur_order_index++;
 						v->current_order.MakeDummy();
-						InvalidateVehicleOrder(v);
+						InvalidateVehicleOrder(v, 0);
 					} else {
 						/* Non-buoy orders really need to reach the tile */
 						if (v->dest_tile == gp.new_tile) {
@@ -647,7 +647,7 @@ static void ShipController(Vehicle *v)
 								} else { // leave stations without docks right aways
 									v->current_order.MakeLeaveStation();
 									v->cur_order_index++;
-									InvalidateVehicleOrder(v);
+									InvalidateVehicleOrder(v, 0);
 								}
 							}
 						}

src/timetable_gui.cpp

@@ -49,10 +49,12 @@ void SetTimetableParams(int param1, int param2, uint32 time)
 
 struct TimetableWindow : Window {
 	int sel_index;
+	const Vehicle *vehicle;
 
 	TimetableWindow(const WindowDesc *desc, WindowNumber window_number) : Window(desc, window_number)
 	{
-		this->caption_color = GetVehicle(window_number)->owner;
+		this->vehicle = GetVehicle(window_number);
+		this->caption_color = this->vehicle->owner;
 		this->vscroll.cap = 8;
 		this->resize.step_height = 10;
 		this->sel_index = -1;
@@ -76,9 +78,70 @@ struct TimetableWindow : Window {
 		return (sel < v->num_orders * 2 && sel >= 0) ? sel : INVALID_ORDER;
 	}
 
-	void OnPaint()
+	virtual void OnInvalidateData(int data)
 	{
-		const Vehicle *v = GetVehicle(this->window_number);
+		switch (data) {
+			case 0:
+				/* Autoreplace replaced the vehicle */
+				this->vehicle = GetVehicle(this->window_number);
+				break;
+
+			case -1:
+				/* Removed / replaced all orders (after deleting / sharing) */
+				if (this->sel_index == -1) break;
+
+				this->DeleteChildWindows();
+				this->sel_index = -1;
+				break;
+
+			default: {
+				/* Moving an order. If one of these is INVALID_VEH_ORDER_ID, then
+				 * the order is being created / removed */
+				if (this->sel_index == -1) break;
+
+				VehicleOrderID from = GB(data, 0, 8);
+				VehicleOrderID to   = GB(data, 8, 8);
+
+				if (from == to) break; // no need to change anything
+
+				/* if from == INVALID_VEH_ORDER_ID, one order was added; if to == INVALID_VEH_ORDER_ID, one order was removed */
+				uint old_num_orders = this->vehicle->num_orders - (uint)(from == INVALID_VEH_ORDER_ID) + (uint)(to == INVALID_VEH_ORDER_ID);
+
+				VehicleOrderID selected_order = (this->sel_index + 1) / 2;
+				if (selected_order == old_num_orders) selected_order = 0; // when last travel time is selected, it belongs to order 0
+
+				bool travel = HasBit(this->sel_index, 0);
+
+				if (from != selected_order) {
+					/* Moving from preceeding order? */
+					selected_order -= (int)(from <= selected_order);
+					/* Moving to   preceeding order? */
+					selected_order += (int)(to   <= selected_order);
+				} else {
+					/* Now we are modifying the selected order */
+					if (to == INVALID_VEH_ORDER_ID) {
+						/* Deleting selected order */
+						this->DeleteChildWindows();
+						this->sel_index = -1;
+						break;
+					} else {
+						/* Moving selected order */
+						selected_order = to;
+					}
+				}
+
+				/* recompute new sel_index */
+				this->sel_index = 2 * selected_order - (int)travel;
+				/* travel time of first order needs special handling */
+				if (this->sel_index == -1) this->sel_index = this->vehicle->num_orders * 2 - 1;
+			} break;
+		}
+	}
+
+
+	virtual void OnPaint()
+	{
+		const Vehicle *v = this->vehicle;
 		int selected = this->sel_index;
 
 		SetVScrollCount(this, v->num_orders * 2);
@@ -193,7 +256,7 @@ struct TimetableWindow : Window {
 
 	virtual void OnClick(Point pt, int widget)
 	{
-		const Vehicle *v = GetVehicle(this->window_number);
+		const Vehicle *v = this->vehicle;
 
 		switch (widget) {
 			case TTV_ORDER_VIEW: /* Order view button */
@@ -203,13 +266,8 @@ struct TimetableWindow : Window {
 			case TTV_TIMETABLE_PANEL: { /* Main panel. */
 				int selected = GetOrderFromTimetableWndPt(pt.y, v);
 
-				if (selected == INVALID_ORDER || selected == this->sel_index) {
+				this->DeleteChildWindows();
-					/* Deselect clicked order */
+				this->sel_index = (selected == INVALID_ORDER || selected == this->sel_index) ? -1 : selected;
-					this->sel_index = -1;
-				} else {
-					/* Select clicked order */
-					this->sel_index = selected;
-				}
 			} break;
 
 			case TTV_CHANGE_TIME: { /* "Wait For" button. */
@@ -255,7 +313,7 @@ struct TimetableWindow : Window {
 	{
 		if (str == NULL) return;
 
-		const Vehicle *v = GetVehicle(this->window_number);
+		const Vehicle *v = this->vehicle;
 
 		uint32 p1 = PackTimetableArgs(v, this->sel_index);
 

src/vehicle.cpp

@@ -2574,7 +2574,7 @@ void Vehicle::HandleLoading(bool mode)
 	}
 
 	this->cur_order_index++;
-	InvalidateVehicleOrder(this);
+	InvalidateVehicleOrder(this, 0);
 }
 
 CommandCost Vehicle::SendToDepot(uint32 flags, DepotCommand command)
@@ -2695,7 +2695,7 @@ void Vehicle::RemoveFromShared()
 	if (new_first->NextShared() == NULL) {
 		/* When there is only one vehicle, remove the shared order list window. */
 		DeleteWindowById(GetWindowClassForVehicleType(this->type), old_window_number);
-		InvalidateVehicleOrder(new_first);
+		InvalidateVehicleOrder(new_first, 0);
 	} else if (this->FirstShared() == this) {
 		/* If we were the first one, update to the new first one. */
 		InvalidateWindowData(GetWindowClassForVehicleType(this->type), old_window_number, (new_first->index << 16) | (1 << 15));

src/vehicle_base.h

@@ -657,6 +657,19 @@ static inline Order *GetVehicleOrder(const Vehicle *v, int index)
 	return order;
 }
 
+
+/** Returns VehicleOrderID of selected order */
+static inline VehicleOrderID GetVehicleOrderID(const Vehicle *v, OrderID order)
+{
+	VehicleOrderID ret = 0;
+
+	for (const Order *o = v->orders; o != NULL; o = o->next, ret++) {
+		if (o->index == order) return ret;
+	}
+
+	return INVALID_VEH_ORDER_ID;
+}
+
 /**
  * Returns the last order of a vehicle, or NULL if it doesn't exists
  * @param v Vehicle to query

src/vehicle_gui.cpp

@@ -433,10 +433,11 @@ static const WindowDesc _vehicle_refit_desc = {
 * @param *v The vehicle to show the refit window for
 * @param order of the vehicle ( ? )
 */
-void ShowVehicleRefitWindow(const Vehicle *v, VehicleOrderID order)
+void ShowVehicleRefitWindow(const Vehicle *v, VehicleOrderID order, Window *parent)
 {
 	DeleteWindowById(WC_VEHICLE_REFIT, v->index);
-	new RefitWindow(&_vehicle_refit_desc, v, order);
+	RefitWindow *w = new RefitWindow(&_vehicle_refit_desc, v, order);
+	w->parent = parent;
 }
 
 /** Display additional text from NewGRF in the purchase information window */
@@ -656,7 +657,7 @@ static inline void ChangeVehicleWindow(WindowClass window_class, VehicleID from_
 	if (w != NULL) {
 		w->window_number = to_index;
 		if (w->viewport != NULL) w->viewport->follow_vehicle = to_index;
-		if (to_index != INVALID_VEHICLE) InvalidateThisWindowData(w);
+		if (to_index != INVALID_VEHICLE) InvalidateThisWindowData(w, 0);
 	}
 }
 
@@ -1979,7 +1980,7 @@ struct VehicleViewWindow : Window {
 					_vehicle_command_translation_table[VCT_CMD_GOTO_DEPOT][v->type]);
 				break;
 			case VVW_WIDGET_REFIT_VEH: // refit
-				ShowVehicleRefitWindow(v, INVALID_VEH_ORDER_ID);
+				ShowVehicleRefitWindow(v, INVALID_VEH_ORDER_ID, this);
 				break;
 			case VVW_WIDGET_SHOW_ORDERS: // show orders
 				if (_ctrl_pressed) {

src/vehicle_gui.h

@@ -13,7 +13,7 @@
 #include "waypoint.h"
 
 void DrawVehicleProfitButton(const Vehicle *v, int x, int y);
-void ShowVehicleRefitWindow(const Vehicle *v, VehicleOrderID order);
+void ShowVehicleRefitWindow(const Vehicle *v, VehicleOrderID order, Window *parent);
 
 /** Constants of vehicle view widget indices */
 enum VehicleViewWindowWidgets {

src/window.cpp

@@ -391,6 +391,18 @@ Window **FindWindowZPosition(const Window *w)
 	return NULL;
 }
 
+/**
+ * Delete all children a window might have in a head-recursive manner
+ */
+void Window::DeleteChildWindows() const
+{
+	Window *child = FindChildWindow(this);
+	while (child != NULL) {
+		delete child;
+		child = FindChildWindow(this);
+	}
+}
+
 /**
  * Remove window and all its child windows from the window stack.
  */
@@ -414,12 +426,7 @@ Window::~Window()
 	memmove(wz, wz + 1, (byte*)_last_z_window - (byte*)wz);
 	_last_z_window--;
 
-	/* Delete all children a window might have in a head-recursive manner */
+	this->DeleteChildWindows();
-	Window *child = FindChildWindow(this);
-	while (child != NULL) {
-		delete child;
-		child = FindChildWindow(this);
-	}
 
 	if (this->viewport != NULL) DeleteWindowViewport(this);
 

src/window_gui.h

@@ -251,6 +251,8 @@ public:
 	void DrawViewport() const;
 	void DrawSortButtonState(int widget, SortButtonState state) const;
 
+	void DeleteChildWindows() const;
+
 	void SetDirty() const;
 
 	/*** Event handling ***/