Codechange: Add a priority field to TimerGameTick::TPeriod

Use this as the primary sort key for TimerGameTick::TPeriod,
to avoid container sort order changes on timer period saveload.
See: #12509
This commit is contained in:
Jonathan G Rennison 2024-04-20 15:39:36 +01:00 committed by rubidium42
parent 57f5d27427
commit 11ec156b64
8 changed files with 53 additions and 17 deletions

View File

@ -637,7 +637,7 @@ Company *DoStartupNewCompany(bool is_ai, CompanyID company = INVALID_COMPANY)
} }
/** Start a new competitor company if possible. */ /** Start a new competitor company if possible. */
TimeoutTimer<TimerGameTick> _new_competitor_timeout(0, []() { TimeoutTimer<TimerGameTick> _new_competitor_timeout({ TimerGameTick::Priority::COMPETITOR_TIMEOUT, 0 }, []() {
if (_game_mode == GM_MENU || !AI::CanStartNew()) return; if (_game_mode == GM_MENU || !AI::CanStartNew()) return;
if (_networking && Company::GetNumItems() >= _settings_client.network.max_companies) return; if (_networking && Company::GetNumItems() >= _settings_client.network.max_companies) return;
@ -778,7 +778,7 @@ void OnTick_Companies()
/* Randomize a bit when the AI is actually going to start; ranges from 87.5% .. 112.5% of indicated value. */ /* Randomize a bit when the AI is actually going to start; ranges from 87.5% .. 112.5% of indicated value. */
timeout += ScriptObject::GetRandomizer(OWNER_NONE).Next(timeout / 4) - timeout / 8; timeout += ScriptObject::GetRandomizer(OWNER_NONE).Next(timeout / 4) - timeout / 8;
_new_competitor_timeout.Reset(std::max(1, timeout)); _new_competitor_timeout.Reset({ TimerGameTick::Priority::COMPETITOR_TIMEOUT, static_cast<uint>(std::max(1, timeout)) });
} }
_cur_company_tick_index = (_cur_company_tick_index + 1) % MAX_COMPANIES; _cur_company_tick_index = (_cur_company_tick_index + 1) % MAX_COMPANIES;

View File

@ -157,7 +157,7 @@ std::string NewGRFProfiler::GetOutputFilename() const
/** /**
* Check whether profiling is active and should be finished. * Check whether profiling is active and should be finished.
*/ */
static TimeoutTimer<TimerGameTick> _profiling_finish_timeout(0, []() static TimeoutTimer<TimerGameTick> _profiling_finish_timeout({ TimerGameTick::Priority::NONE, 0 }, []()
{ {
NewGRFProfiler::FinishAll(); NewGRFProfiler::FinishAll();
}); });
@ -167,7 +167,7 @@ static TimeoutTimer<TimerGameTick> _profiling_finish_timeout(0, []()
*/ */
/* static */ void NewGRFProfiler::StartTimer(uint64_t ticks) /* static */ void NewGRFProfiler::StartTimer(uint64_t ticks)
{ {
_profiling_finish_timeout.Reset(ticks); _profiling_finish_timeout.Reset({ TimerGameTick::Priority::NONE, static_cast<uint>(ticks) });
} }
/** /**

View File

@ -3255,7 +3255,7 @@ bool AfterLoadGame()
/* We did load the "period" of the timer, but not the fired/elapsed. We can deduce that here. */ /* We did load the "period" of the timer, but not the fired/elapsed. We can deduce that here. */
extern TimeoutTimer<TimerGameTick> _new_competitor_timeout; extern TimeoutTimer<TimerGameTick> _new_competitor_timeout;
_new_competitor_timeout.storage.elapsed = 0; _new_competitor_timeout.storage.elapsed = 0;
_new_competitor_timeout.fired = _new_competitor_timeout.period == 0; _new_competitor_timeout.fired = _new_competitor_timeout.period.value == 0;
} }
if (IsSavegameVersionBefore(SLV_NEWGRF_LAST_SERVICE)) { if (IsSavegameVersionBefore(SLV_NEWGRF_LAST_SERVICE)) {

View File

@ -99,9 +99,9 @@ static const SaveLoad _date_desc[] = {
SLEG_CONDVAR("pause_mode", _pause_mode, SLE_UINT8, SLV_4, SL_MAX_VERSION), SLEG_CONDVAR("pause_mode", _pause_mode, SLE_UINT8, SLV_4, SL_MAX_VERSION),
SLEG_CONDSSTR("id", _game_session_stats.savegame_id, SLE_STR, SLV_SAVEGAME_ID, SL_MAX_VERSION), SLEG_CONDSSTR("id", _game_session_stats.savegame_id, SLE_STR, SLV_SAVEGAME_ID, SL_MAX_VERSION),
/* For older savegames, we load the current value as the "period"; afterload will set the "fired" and "elapsed". */ /* For older savegames, we load the current value as the "period"; afterload will set the "fired" and "elapsed". */
SLEG_CONDVAR("next_competitor_start", _new_competitor_timeout.period, SLE_FILE_U16 | SLE_VAR_U32, SL_MIN_VERSION, SLV_109), SLEG_CONDVAR("next_competitor_start", _new_competitor_timeout.period.value, SLE_FILE_U16 | SLE_VAR_U32, SL_MIN_VERSION, SLV_109),
SLEG_CONDVAR("next_competitor_start", _new_competitor_timeout.period, SLE_UINT32, SLV_109, SLV_AI_START_DATE), SLEG_CONDVAR("next_competitor_start", _new_competitor_timeout.period.value, SLE_UINT32, SLV_109, SLV_AI_START_DATE),
SLEG_CONDVAR("competitors_interval", _new_competitor_timeout.period, SLE_UINT32, SLV_AI_START_DATE, SL_MAX_VERSION), SLEG_CONDVAR("competitors_interval", _new_competitor_timeout.period.value, SLE_UINT32, SLV_AI_START_DATE, SL_MAX_VERSION),
SLEG_CONDVAR("competitors_interval_elapsed", _new_competitor_timeout.storage.elapsed, SLE_UINT32, SLV_AI_START_DATE, SL_MAX_VERSION), SLEG_CONDVAR("competitors_interval_elapsed", _new_competitor_timeout.storage.elapsed, SLE_UINT32, SLV_AI_START_DATE, SL_MAX_VERSION),
SLEG_CONDVAR("competitors_interval_fired", _new_competitor_timeout.fired, SLE_BOOL, SLV_AI_START_DATE, SL_MAX_VERSION), SLEG_CONDVAR("competitors_interval_fired", _new_competitor_timeout.fired, SLE_BOOL, SLV_AI_START_DATE, SL_MAX_VERSION),
}; };

View File

@ -1701,7 +1701,7 @@ static const OldChunks main_chunk[] = {
OCL_ASSERT( OC_TTO, 0x496CE ), OCL_ASSERT( OC_TTO, 0x496CE ),
OCL_VAR ( OC_FILE_U16 | OC_VAR_U32, 1, &_new_competitor_timeout.period ), OCL_VAR ( OC_FILE_U16 | OC_VAR_U32, 1, &_new_competitor_timeout.period.value ),
OCL_CNULL( OC_TTO, 2 ), ///< available monorail bitmask OCL_CNULL( OC_TTO, 2 ), ///< available monorail bitmask

View File

@ -21,13 +21,13 @@ TimerGameTick::TickCounter TimerGameTick::counter = 0;
template<> template<>
void IntervalTimer<TimerGameTick>::Elapsed(TimerGameTick::TElapsed delta) void IntervalTimer<TimerGameTick>::Elapsed(TimerGameTick::TElapsed delta)
{ {
if (this->period == 0) return; if (this->period.value == 0) return;
this->storage.elapsed += delta; this->storage.elapsed += delta;
uint count = 0; uint count = 0;
while (this->storage.elapsed >= this->period) { while (this->storage.elapsed >= this->period.value) {
this->storage.elapsed -= this->period; this->storage.elapsed -= this->period.value;
count++; count++;
} }
@ -40,11 +40,11 @@ template<>
void TimeoutTimer<TimerGameTick>::Elapsed(TimerGameTick::TElapsed delta) void TimeoutTimer<TimerGameTick>::Elapsed(TimerGameTick::TElapsed delta)
{ {
if (this->fired) return; if (this->fired) return;
if (this->period == 0) return; if (this->period.value == 0) return;
this->storage.elapsed += delta; this->storage.elapsed += delta;
if (this->storage.elapsed >= this->period) { if (this->storage.elapsed >= this->period.value) {
this->callback(); this->callback();
this->fired = true; this->fired = true;
} }
@ -64,7 +64,16 @@ bool TimerManager<TimerGameTick>::Elapsed(TimerGameTick::TElapsed delta)
#ifdef WITH_ASSERT #ifdef WITH_ASSERT
template<> template<>
void TimerManager<TimerGameTick>::Validate(TimerGameTick::TPeriod) void TimerManager<TimerGameTick>::Validate(TimerGameTick::TPeriod period)
{ {
if (period.priority == TimerGameTick::Priority::NONE) return;
/* Validate we didn't make a developer error and scheduled more than one
* entry on the same priority. There can only be one timer on
* a specific priority, to ensure we are deterministic, and to avoid
* container sort order invariant issues with timer period saveload. */
for (const auto &timer : TimerManager<TimerGameTick>::GetTimers()) {
assert(timer->period.priority != period.priority);
}
} }
#endif /* WITH_ASSERT */ #endif /* WITH_ASSERT */

View File

@ -24,7 +24,34 @@ public:
using Ticks = int32_t; ///< The type to store ticks in using Ticks = int32_t; ///< The type to store ticks in
using TickCounter = uint64_t; ///< The type that the tick counter is stored in using TickCounter = uint64_t; ///< The type that the tick counter is stored in
using TPeriod = uint; enum Priority {
NONE, ///< These timers can be executed in any order; the order is not relevant.
/* For all other priorities, the order is important.
* For safety, you can only setup a single timer on a single priority. */
COMPETITOR_TIMEOUT,
};
struct TPeriod {
Priority priority;
uint value;
TPeriod(Priority priority, uint value) : priority(priority), value(value)
{}
bool operator < (const TPeriod &other) const
{
/* Sort by priority before value, such that changes in value for priorities other than NONE do not change the container order */
if (this->priority != other.priority) return this->priority < other.priority;
return this->value < other.value;
}
bool operator == (const TPeriod &other) const
{
return this->priority == other.priority && this->value == other.value;
}
};
using TElapsed = uint; using TElapsed = uint;
struct TStorage { struct TStorage {
uint elapsed; uint elapsed;

View File

@ -799,7 +799,7 @@ struct TimetableWindow : Window {
/** /**
* In real-time mode, the timetable GUI shows relative times and needs to be redrawn every second. * In real-time mode, the timetable GUI shows relative times and needs to be redrawn every second.
*/ */
IntervalTimer<TimerGameTick> redraw_interval = {Ticks::TICKS_PER_SECOND, [this](auto) { IntervalTimer<TimerGameTick> redraw_interval = { { TimerGameTick::Priority::NONE, Ticks::TICKS_PER_SECOND }, [this](auto) {
if (_settings_client.gui.timetable_mode == TimetableMode::Seconds) { if (_settings_client.gui.timetable_mode == TimetableMode::Seconds) {
this->SetDirty(); this->SetDirty();
} }