(svn r22135) -Fix [FS#4523]: When commands need to invalidate windows, process these events asynchronously before the next redraw. Calling window code directly from command scope uses wrong _current_company and might issue nested DoCommands() which interfer with the running command.

src/main_gui.cpp

@@ -435,7 +435,7 @@ struct MainWindow : Window
 	virtual void OnInvalidateData(int data)
 	{
 		/* Forward the message to the appropiate toolbar (ingame or scenario editor) */
-		InvalidateWindowData(WC_MAIN_TOOLBAR, 0, data);
+		InvalidateWindowData(WC_MAIN_TOOLBAR, 0, data, true);
 	}
 
 	static Hotkey<MainWindow> global_hotkeys[];

src/misc_gui.cpp

@@ -199,6 +199,7 @@ public:
 		if (c != NULL) {
 			Money old_money = c->money;
 			c->money = INT64_MAX;
+			assert(_current_company == _local_company);
 			CommandCost costclear = DoCommand(tile, 0, 0, DC_NONE, CMD_LANDSCAPE_CLEAR);
 			c->money = old_money;
 			if (costclear.Succeeded()) {

src/newgrf_debug_gui.cpp

@@ -518,7 +518,9 @@ void DeleteNewGRFInspectWindow(GrfSpecFeature feature, uint index)
 	WindowNumber wno = GetInspectWindowNumber(feature, index);
 	DeleteWindowById(WC_NEWGRF_INSPECT, wno);
 
-	/* Reinitialise the land information window to remove the "debug" sprite if needed. */
+	/* Reinitialise the land information window to remove the "debug" sprite if needed.
+	 * Note: Since we might be called from a command here, it is important to not execute
+	 * the invalidation immediatelly. The landinfo window tests commands itself. */
 	InvalidateWindowData(WC_LAND_INFO, 0, 1);
 }
 

src/train_cmd.cpp

@@ -264,7 +264,7 @@ void Train::ConsistChanged(bool same_length)
 	if (this->IsFrontEngine()) {
 		this->UpdateAcceleration();
 		SetWindowDirty(WC_VEHICLE_DETAILS, this->index);
-		InvalidateWindowData(WC_VEHICLE_REFIT, this->index);
+		InvalidateWindowData(WC_VEHICLE_REFIT, this->index); // Important, do not invalidate immediatelly. The refit window tests commands.
 	}
 }
 
@@ -1089,7 +1089,7 @@ static void NormaliseTrainHead(Train *head)
 	if (!head->IsFrontEngine()) return;
 
 	/* Update the refit button and window */
-	InvalidateWindowData(WC_VEHICLE_REFIT, head->index);
+	InvalidateWindowData(WC_VEHICLE_REFIT, head->index); // Important, do not invalidate immediatelly. The refit window tests commands.
 	SetWindowWidgetDirty(WC_VEHICLE_VIEW, head->index, VVW_WIDGET_REFIT_VEH);
 
 	/* If we don't have a unit number yet, set one. */

src/vehicle.cpp

@@ -2270,7 +2270,7 @@ void Vehicle::RemoveFromShared()
 	} else if (were_first) {
 		/* If we were the first one, update to the new first one.
 		 * Note: FirstShared() is already the new first */
-		InvalidateWindowData(GetWindowClassForVehicleType(this->type), vli.Pack(), this->FirstShared()->index | (1U << 31));
+		InvalidateWindowData(GetWindowClassForVehicleType(this->type), vli.Pack(), this->FirstShared()->index | (1U << 31), true);
 	}
 
 	this->next_shared     = NULL;

src/vehicle_gui.cpp

@@ -585,6 +585,7 @@ struct RefitWindow : public Window {
 	 */
 	StringID GetCapacityString(RefitOption *option) const
 	{
+		assert(_current_company == _local_company);
 		Vehicle *v = Vehicle::Get(this->window_number);
 		CommandCost cost = DoCommand(v->tile, this->selected_vehicle, option->cargo | option->subtype << 8 |
 				this->num_vehicles << 16, DC_QUERY_COST, GetCmdRefitVeh(v->type));
@@ -1119,7 +1120,7 @@ static inline void ChangeVehicleWindow(WindowClass window_class, VehicleID from_
 			_thd.window_number = to_index;
 		}
 
-		/* Notify the window */
+		/* Notify the window immediatelly, without scheduling. */
 		w->InvalidateData();
 	}
 }

src/window.cpp

@@ -2420,6 +2420,7 @@ void UpdateWindows()
 
 			if (!(w->flags4 & WF_WHITE_BORDER_MASK)) w->SetDirty();
 		}
+		w->ProcessScheduledInvalidations();
 	}
 
 	DrawDirtyBlocks();
@@ -2476,29 +2477,47 @@ void SetWindowClassesDirty(WindowClass cls)
 
 /**
  * Mark window data of the window of a given class and specific window number as invalid (in need of re-computing)
+ * Note that by default the invalidation is not executed immediatelly but is scheduled till the next redraw.
+ * The asynchronous execution is important to prevent GUI code being executed from command scope.
  * @param cls Window class
  * @param number Window number within the class
  * @param data The data to invalidate with
+ * @param immediatelly If true then do not schedule the event, but execute immediatelly.
  */
-void InvalidateWindowData(WindowClass cls, WindowNumber number, int data)
+void InvalidateWindowData(WindowClass cls, WindowNumber number, int data, bool immediatelly)
 {
 	Window *w;
 	FOR_ALL_WINDOWS_FROM_BACK(w) {
-		if (w->window_class == cls && w->window_number == number) w->InvalidateData(data);
+		if (w->window_class == cls && w->window_number == number) {
+			if (immediatelly) {
+				w->InvalidateData(data);
+			} else {
+				w->ScheduleInvalidateData(data);
+			}
+		}
 	}
 }
 
 /**
  * Mark window data of all windows of a given class as invalid (in need of re-computing)
+ * Note that by default the invalidation is not executed immediatelly but is scheduled till the next redraw.
+ * The asynchronous execution is important to prevent GUI code being executed from command scope.
  * @param cls Window class
  * @param data The data to invalidate with
+ * @param immediatelly If true then do not schedule the event, but execute immediatelly.
  */
-void InvalidateWindowClassesData(WindowClass cls, int data)
+void InvalidateWindowClassesData(WindowClass cls, int data, bool immediatelly)
 {
 	Window *w;
 
 	FOR_ALL_WINDOWS_FROM_BACK(w) {
-		if (w->window_class == cls) w->InvalidateData(data);
+		if (w->window_class == cls) {
+			if (immediatelly) {
+				w->InvalidateData(data);
+			} else {
+				w->ScheduleInvalidateData(data);
+			}
+		}
 	}
 }
 

src/window_func.h

@@ -34,8 +34,8 @@ void ResetWindowSystem();
 void SetupColoursAndInitialWindow();
 void InputLoop();
 
-void InvalidateWindowData(WindowClass cls, WindowNumber number, int data = 0);
-void InvalidateWindowClassesData(WindowClass cls, int data = 0);
+void InvalidateWindowData(WindowClass cls, WindowNumber number, int data = 0, bool immediatelly = false);
+void InvalidateWindowClassesData(WindowClass cls, int data = 0, bool immediatelly = false);
 
 void DeleteNonVitalWindows();
 void DeleteAllNonVitalWindows();

src/window_gui.h

@@ -17,6 +17,7 @@
 #include "company_type.h"
 #include "tile_type.h"
 #include "widget_type.h"
+#include "core/smallvec_type.hpp"
 
 /** State of handling an event. */
 enum EventState {
@@ -221,6 +222,8 @@ protected:
 	void InitializePositionSize(int x, int y, int min_width, int min_height);
 	void FindWindowPlacementAndResize(int def_width, int def_height);
 
+	SmallVector<int, 4> scheduled_invalidation_data;  ///< Data of scheduled OnInvalidateData() calls.
+
 public:
 	Window();
 
@@ -438,6 +441,28 @@ public:
 		this->OnInvalidateData(data);
 	}
 
+	/**
+	 * Schedule a invalidation call for next redraw.
+	 * Important for asynchronous invalidation from commands.
+	 * @param data The data to invalidate with
+	 */
+	void ScheduleInvalidateData(int data = 0)
+	{
+		this->SetDirty();
+		*this->scheduled_invalidation_data.Append() = data;
+	}
+
+	/**
+	 * Process all scheduled invalidations.
+	 */
+	void ProcessScheduledInvalidations()
+	{
+		for (int *data = this->scheduled_invalidation_data.Begin(); this->window_class != WC_INVALID && data != this->scheduled_invalidation_data.End(); data++) {
+			this->OnInvalidateData(*data);
+		}
+		this->scheduled_invalidation_data.Clear();
+	}
+
 	/*** Event handling ***/
 
 	/**