From 387d5eb74fb9af45ee93a0c46e6d173ecaa7bc76 Mon Sep 17 00:00:00 2001
From: Patric Stout <truebrain@openttd.org>
Date: Thu, 13 Apr 2023 19:26:17 +0200
Subject: [PATCH] Codechange: validate the developer didn't schedule two timers
 on the same trigger/priority

---
 src/timer/timer_game_calendar.cpp | 17 +++++++++++++++++
 src/timer/timer_game_calendar.h   |  1 +
 src/timer/timer_manager.h         | 19 +++++++++++++++++++
 src/timer/timer_window.cpp        |  7 +++++++
 4 files changed, 44 insertions(+)

diff --git a/src/timer/timer_game_calendar.cpp b/src/timer/timer_game_calendar.cpp
index 6fe804ecc8..5d080b6237 100644
--- a/src/timer/timer_game_calendar.cpp
+++ b/src/timer/timer_game_calendar.cpp
@@ -98,3 +98,20 @@ void TimerManager<TimerGameCalendar>::Elapsed(TimerGameCalendar::TElapsed delta)
 		for (LinkGraph *lg : LinkGraph::Iterate()) lg->ShiftDates(-days_this_year);
 	}
 }
+
+#ifdef WITH_ASSERT
+template<>
+void TimerManager<TimerGameCalendar>::Validate(TimerGameCalendar::TPeriod period)
+{
+	if (period.priority == TimerGameCalendar::Priority::NONE) return;
+
+	/* Validate we didn't make a developer error and scheduled more than one
+	 * entry on the same priority/trigger. There can only be one timer on
+	 * a specific trigger/priority, to ensure we are deterministic. */
+	for (const auto &timer : TimerManager<TimerGameCalendar>::GetTimers()) {
+		if (timer->period.trigger != period.trigger) continue;
+
+		assert(timer->period.priority != period.priority);
+	}
+}
+#endif /* WITH_ASSERT */
diff --git a/src/timer/timer_game_calendar.h b/src/timer/timer_game_calendar.h
index f7dd3dd499..76de11216b 100644
--- a/src/timer/timer_game_calendar.h
+++ b/src/timer/timer_game_calendar.h
@@ -26,6 +26,7 @@
  * For example:
  *   IntervalTimer<TimerGameCalendar>({TimerGameCalendar::DAY, TimerGameCalendar::Priority::NONE}, [](uint count){});
  *
+ * @note Callbacks are executed in the game-thread.
  */
 class TimerGameCalendar {
 public:
diff --git a/src/timer/timer_manager.h b/src/timer/timer_manager.h
index 2efe583ad9..8d1200abea 100644
--- a/src/timer/timer_manager.h
+++ b/src/timer/timer_manager.h
@@ -28,6 +28,7 @@ class BaseTimer;
 template <typename TTimerType>
 class TimerManager {
 public:
+	using TPeriod = typename TTimerType::TPeriod;
 	using TElapsed = typename TTimerType::TElapsed;
 
 	/* Avoid copying this object; it is a singleton object. */
@@ -40,6 +41,9 @@ public:
 	 * @param timer The timer to register.
 	 */
 	static void RegisterTimer(BaseTimer<TTimerType> &timer) {
+#ifdef WITH_ASSERT
+		Validate(timer.period);
+#endif /* WITH_ASSERT */
 		GetTimers().insert(&timer);
 	}
 
@@ -52,6 +56,21 @@ public:
 		GetTimers().erase(&timer);
 	}
 
+#ifdef WITH_ASSERT
+	/**
+	 * Validate that a new period is actually valid.
+	 *
+	 * For most timers this is not an issue, but some want to make sure their
+	 * period is unique, to ensure deterministic game-play.
+	 *
+	 * This is meant purely to protect a developer from making a mistake.
+	 * As such, assert() when validation fails.
+	 *
+	 * @param period The period to validate.
+	 */
+	static void Validate(TPeriod period);
+#endif /* WITH_ASSERT */
+
 	/**
 	 * Called when time for this timer elapsed.
 	 *
diff --git a/src/timer/timer_window.cpp b/src/timer/timer_window.cpp
index dff0b45b8a..f99d6372b3 100644
--- a/src/timer/timer_window.cpp
+++ b/src/timer/timer_window.cpp
@@ -58,3 +58,10 @@ void TimerManager<TimerWindow>::Elapsed(TimerWindow::TElapsed delta)
 		timer->Elapsed(delta);
 	}
 }
+
+#ifdef WITH_ASSERT
+template<>
+void TimerManager<TimerWindow>::Validate(TimerWindow::TPeriod period)
+{
+}
+#endif /* WITH_ASSERT */