Fix windows not being deleted when order changed during deletion

src/window.cpp

@@ -57,6 +57,8 @@ static Window *_last_scroll_window = nullptr; ///< Window of the last scroll eve
 WindowBase *_z_front_window = nullptr;
 /** List of windows opened at the screen sorted from the back. */
 WindowBase *_z_back_window  = nullptr;
+/** List of windows in an arbitrary order, that is not instantaneously changed by bringing windows to the front. */
+WindowBase *_first_window  = nullptr;
 
 /** If false, highlight is white, otherwise the by the widget defined colour. */
 bool _window_highlight_colour = false;
@@ -1192,7 +1194,7 @@ void DeleteWindowById(WindowClass cls, WindowNumber number, bool force)
 void DeleteWindowByClass(WindowClass cls)
 {
 	/* Note: the container remains stable, even when deleting windows. */
-	for (Window *w : Window::IterateFromBack()) {
+	for (Window *w : Window::IterateUnordered()) {
 		if (w->window_class == cls) {
 			delete w;
 		}
@@ -1208,7 +1210,7 @@ void DeleteWindowByClass(WindowClass cls)
 void DeleteCompanyWindows(CompanyID id)
 {
 	/* Note: the container remains stable, even when deleting windows. */
-	for (Window *w : Window::IterateFromBack()) {
+	for (Window *w : Window::IterateUnordered()) {
 		if (w->owner == id) {
 			delete w;
 		}
@@ -1501,6 +1503,8 @@ void Window::InitializeData(WindowNumber window_number)
 
 	/* Insert the window into the correct location in the z-ordering. */
 	AddWindowToZOrdering(this);
+	this->next_window = _first_window;
+	_first_window = this;
 }
 
 /**
@@ -1901,6 +1905,7 @@ void InitWindowSystem()
 
 	_z_back_window = nullptr;
 	_z_front_window = nullptr;
+	_first_window = nullptr;
 	_focused_window = nullptr;
 	_mouseover_last_w = nullptr;
 	_last_scroll_window = nullptr;
@@ -1921,7 +1926,7 @@ void UnInitWindowSystem()
 {
 	UnshowCriticalError();
 
-	for (Window *w : Window::IterateFromFront()) delete w;
+	for (Window *w : Window::IterateUnordered()) delete w;
 
 	for (WindowBase *w = _z_front_window; w != nullptr; /* nothing */) {
 		WindowBase *to_del = w;
@@ -1931,6 +1936,7 @@ void UnInitWindowSystem()
 
 	_z_front_window = nullptr;
 	_z_back_window = nullptr;
+	_first_window = nullptr;
 }
 
 /**
@@ -3152,6 +3158,8 @@ void InputLoop()
 
 	CheckSoftLimit();
 
+	bool reset_window_nexts = false;
+
 	/* Do the actual free of the deleted windows. */
 	for (WindowBase *v = _z_front_window; v != nullptr; /* nothing */) {
 		WindowBase *w = v;
@@ -3161,6 +3169,14 @@ void InputLoop()
 
 		RemoveWindowFromZOrdering(w);
 		free(w);
+		reset_window_nexts = true;
+	}
+
+	if (reset_window_nexts) {
+		_first_window = _z_front_window;
+		for (WindowBase *w = _z_front_window; w != nullptr; w = w->z_back) {
+			w->next_window = w->z_back;
+		}
 	}
 
 	if (_input_events_this_tick != 0) {
@@ -3414,7 +3430,7 @@ void CallWindowGameTickEvent()
 void DeleteNonVitalWindows()
 {
 	/* Note: the container remains stable, even when deleting windows. */
-	for (const Window *w : Window::IterateFromBack()) {
+	for (const Window *w : Window::IterateUnordered()) {
 		if (w->window_class != WC_MAIN_WINDOW &&
 				w->window_class != WC_SELECT_GAME &&
 				w->window_class != WC_MAIN_TOOLBAR &&
@@ -3440,7 +3456,7 @@ void DeleteAllNonVitalWindows()
 	DeleteNonVitalWindows();
 
 	/* Note: the container remains stable, even when deleting windows. */
-	for (const Window *w : Window::IterateFromBack()) {
+	for (const Window *w : Window::IterateUnordered()) {
 		if (w->flags & WF_STICKY) {
 			delete w;
 		}
@@ -3465,7 +3481,7 @@ void DeleteAllMessages()
 void DeleteConstructionWindows()
 {
 	/* Note: the container remains stable, even when deleting windows. */
-	for (const Window *w : Window::IterateFromBack()) {
+	for (const Window *w : Window::IterateUnordered()) {
 		if (w->window_desc->flags & WDF_CONSTRUCTION) {
 			delete w;
 		}

src/window_gui.h

@@ -145,6 +145,7 @@ void DrawCaption(const Rect &r, Colours colour, Owner owner, TextColour text_col
 /* window.cpp */
 extern WindowBase *_z_front_window;
 extern WindowBase *_z_back_window;
+extern WindowBase *_first_window;
 extern Window *_focused_window;
 
 inline uint64 GetWindowUpdateNumber()
@@ -291,6 +292,7 @@ enum TooltipCloseCondition {
 struct WindowBase {
 	WindowBase *z_front;             ///< The window in front of us in z-order.
 	WindowBase *z_back;              ///< The window behind us in z-order.
+	WindowBase *next_window;         ///< The next window in arbitrary iteration order.
 	WindowClass window_class;        ///< Window class
 
 	virtual ~WindowBase() {}
@@ -884,12 +886,18 @@ public:
 	template<class T>
 	using window_base_t = std::conditional_t<std::is_const<T>{}, WindowBase const, WindowBase>;
 
+	enum IterationMode {
+		IM_FROM_FRONT,
+		IM_FROM_BACK,
+		IM_ARBITRARY,
+	};
+
 	/**
 	 * Iterator to iterate all valid Windows
 	 * @tparam T Type of the class/struct that is going to be iterated
-	 * @tparam Tfront Wether we iterate from front
+	 * @tparam Tmode Iteration mode
 	 */
-	template <class T, bool Tfront>
+	template <class T, IterationMode Tmode>
 	struct WindowIterator {
 		typedef T value_type;
 		typedef T *pointer;
@@ -910,7 +918,23 @@ public:
 	private:
 		window_base_t<T> *w;
 		void Validate() { while (this->w != nullptr && this->w->window_class == WC_INVALID) this->Next(); }
-		void Next() { if (this->w != nullptr) this->w = Tfront ? this->w->z_back : this->w->z_front; }
+
+		void Next()
+		{
+			if (this->w != nullptr) {
+				switch (Tmode) {
+					case IM_FROM_FRONT:
+						this->w = this->w->z_back;
+						break;
+					case IM_FROM_BACK:
+						this->w = this->w->z_front;
+						break;
+					case IM_ARBITRARY:
+						this->w = this->w->next_window;
+						break;
+				}
+			}
+		}
 	};
 
 	/**
@@ -918,11 +942,11 @@ public:
 	 * @tparam T Type of the class/struct that is going to be iterated
 	 * @tparam Tfront Wether we iterate from front
 	 */
-	template <class T, bool Tfront>
+	template <class T, IterationMode Tmode>
 	struct Iterate {
 		Iterate(window_base_t<T> *from) : from(from) {}
-		WindowIterator<T, Tfront> begin() { return WindowIterator<T, Tfront>(this->from); }
+		WindowIterator<T, Tmode> begin() { return WindowIterator<T, Tmode>(this->from); }
-		WindowIterator<T, Tfront> end() { return WindowIterator<T, Tfront>(nullptr); }
+		WindowIterator<T, Tmode> end() { return WindowIterator<T, Tmode>(nullptr); }
 		bool empty() { return this->begin() == this->end(); }
 	private:
 		window_base_t<T> *from;
@@ -935,7 +959,7 @@ public:
 	 * @return an iterable ensemble of all valid Window
 	 */
 	template <class T = Window>
-	static Iterate<T, false> IterateFromBack(window_base_t<T> *from = _z_back_window) { return Iterate<T, false>(from); }
+	static Iterate<T, IM_FROM_BACK> IterateFromBack(window_base_t<T> *from = _z_back_window) { return Iterate<T, IM_FROM_BACK>(from); }
 
 	/**
 	 * Returns an iterable ensemble of all valid Window from front to back
@@ -944,7 +968,16 @@ public:
 	 * @return an iterable ensemble of all valid Window
 	 */
 	template <class T = Window>
-	static Iterate<T, true> IterateFromFront(window_base_t<T> *from = _z_front_window) { return Iterate<T, true>(from); }
+	static Iterate<T, IM_FROM_FRONT> IterateFromFront(window_base_t<T> *from = _z_front_window) { return Iterate<T, IM_FROM_FRONT>(from); }
+
+	/**
+	 * Returns an iterable ensemble of all valid Window in an arbitrary order which is safe to use when deleting
+	 * @tparam T Type of the class/struct that is going to be iterated
+	 * @param from index of the first Window to consider
+	 * @return an iterable ensemble of all valid Window
+	 */
+	template <class T = Window>
+	static Iterate<T, IM_ARBITRARY> IterateUnordered(window_base_t<T> *from = _first_window) { return Iterate<T, IM_ARBITRARY>(from); }
 };
 
 /**