From 90ca3515da31c44813b340c865058fd000a8b471 Mon Sep 17 00:00:00 2001
From: Peter Nelson <peter1138@openttd.org>
Date: Tue, 9 Apr 2024 21:46:29 +0100
Subject: [PATCH] Fix #12459, f6a88e4: Crashes when deleting news messages.
 (#12460)

The updated logic in f6a88e4 for deleting news messages did things in the wrong order.
---
 src/news_gui.cpp | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/src/news_gui.cpp b/src/news_gui.cpp
index 8e8d98f261..24827d0f19 100644
--- a/src/news_gui.cpp
+++ b/src/news_gui.cpp
@@ -685,13 +685,14 @@ static bool ReadyForNextNewsItem()
 /** Move to the next ticker item */
 static void MoveToNextTickerItem()
 {
-	assert(!std::empty(_news));
-
 	/* There is no status bar, so no reason to show news;
 	 * especially important with the end game screen when
 	 * there is no status bar but possible news. */
 	if (FindWindowById(WC_STATUS_BAR, 0) == nullptr) return;
 
+	/* No news to move to. */
+	if (std::empty(_news)) return;
+
 	/* if we're not at the latest item, then move on */
 	while (_statusbar_news != std::begin(_news)) {
 		--_statusbar_news;
@@ -719,8 +720,6 @@ static void MoveToNextTickerItem()
 /** Move to the next news item */
 static void MoveToNextNewsItem()
 {
-	assert(!std::empty(_news));
-
 	/* There is no status bar, so no reason to show news;
 	 * especially important with the end game screen when
 	 * there is no status bar but possible news. */
@@ -729,6 +728,9 @@ static void MoveToNextNewsItem()
 	CloseWindowById(WC_NEWS_WINDOW, 0); // close the newspapers window if shown
 	_forced_news = std::end(_news);
 
+	/* No news to move to. */
+	if (std::empty(_news)) return;
+
 	/* if we're not at the latest item, then move on */
 	while (_current_news != std::begin(_news)) {
 		--_current_news;
@@ -755,28 +757,38 @@ static void MoveToNextNewsItem()
 /** Delete a news item from the queue */
 static std::list<NewsItem>::iterator DeleteNewsItem(std::list<NewsItem>::iterator ni)
 {
-	if (_forced_news == ni || _current_news == ni) {
-		/* When we're the current news, go to the previous item first;
-		 * we just possibly made that the last news item. */
-		if (_current_news == ni) _current_news = (_current_news == std::begin(_news)) ? std::end(_news) : std::prev(_current_news);
+	bool updateCurrentNews = (_forced_news == ni || _current_news == ni);
+	bool updateStatusbarNews = (_statusbar_news == ni);
 
+	if (updateCurrentNews) {
+		/* When we're the current news, go to the next older item first;
+		 * we just possibly made that the last news item. */
+		if (_current_news == ni) ++_current_news;
+		if (_forced_news == ni) _forced_news = std::end(_news);
+	}
+
+	if (updateStatusbarNews) {
+		/* When we're the current news, go to the next older item first;
+		 * we just possibly made that the last news item. */
+		++_statusbar_news;
+	}
+
+	/* Delete the news from the news queue. */
+	ni = _news.erase(ni);
+
+	if (updateCurrentNews) {
 		/* About to remove the currently forced item (shown as newspapers) ||
 		 * about to remove the currently displayed item (newspapers) */
 		MoveToNextNewsItem();
 	}
 
-	if (_statusbar_news == ni) {
-		/* When we're the current news, go to the previous item first;
-		 * we just possibly made that the last news item. */
-		 if (_statusbar_news == ni) _statusbar_news = (_statusbar_news == std::begin(_news)) ? std::end(_news) : std::prev(_statusbar_news);
-
+	if (updateStatusbarNews) {
 		/* About to remove the currently displayed item (ticker, or just a reminder) */
 		InvalidateWindowData(WC_STATUS_BAR, 0, SBI_NEWS_DELETED); // invalidate the statusbar
 		MoveToNextTickerItem();
 	}
 
-	/* Delete the news from the news queue. */
-	return _news.erase(ni);
+	return ni;
 }
 
 /**