From 674642f9cccbb4beac4fd37554f17a2f516064f9 Mon Sep 17 00:00:00 2001 From: Jonathan G Rennison Date: Sat, 20 Apr 2024 21:29:21 +0100 Subject: [PATCH] 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. --- src/company_cmd.cpp | 4 ++-- src/newgrf_profiling.cpp | 4 ++-- src/saveload/afterload.cpp | 2 +- src/saveload/misc_sl.cpp | 6 +++--- src/sl/misc_sl.cpp | 10 +++++----- src/sl/oldloader_sl.cpp | 2 +- src/timer/timer_game_tick.cpp | 21 +++++++++++++++------ src/timer/timer_game_tick.h | 29 ++++++++++++++++++++++++++++- 8 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/company_cmd.cpp b/src/company_cmd.cpp index b9abefdf1d..559ec2a5cb 100644 --- a/src/company_cmd.cpp +++ b/src/company_cmd.cpp @@ -668,7 +668,7 @@ Company *DoStartupNewCompany(DoStartupNewCompanyFlag flags, CompanyID company) } /** Start a new competitor company if possible. */ -TimeoutTimer _new_competitor_timeout(0, []() { +TimeoutTimer _new_competitor_timeout({ TimerGameTick::Priority::COMPETITOR_TIMEOUT, 0 }, []() { if (_game_mode == GM_MENU || !AI::CanStartNew()) return; if (_networking && Company::GetNumItems() >= _settings_client.network.max_companies) return; @@ -850,7 +850,7 @@ void OnTick_Companies(bool main_tick) /* 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; - _new_competitor_timeout.Reset(std::max(1, timeout)); + _new_competitor_timeout.Reset({ TimerGameTick::Priority::COMPETITOR_TIMEOUT, static_cast(std::max(1, timeout)) }); } } diff --git a/src/newgrf_profiling.cpp b/src/newgrf_profiling.cpp index 9d7d5f9316..aa9c374c80 100644 --- a/src/newgrf_profiling.cpp +++ b/src/newgrf_profiling.cpp @@ -164,7 +164,7 @@ std::string NewGRFProfiler::GetOutputFilename() const /** * Check whether profiling is active and should be finished. */ -static TimeoutTimer _profiling_finish_timeout(0, []() +static TimeoutTimer _profiling_finish_timeout({ TimerGameTick::Priority::NONE, 0 }, []() { NewGRFProfiler::FinishAll(); }); @@ -174,7 +174,7 @@ static TimeoutTimer _profiling_finish_timeout(0, []() */ /* static */ void NewGRFProfiler::StartTimer(uint64_t ticks) { - _profiling_finish_timeout.Reset(ticks); + _profiling_finish_timeout.Reset({ TimerGameTick::Priority::NONE, static_cast(ticks) }); } /** diff --git a/src/saveload/afterload.cpp b/src/saveload/afterload.cpp index 154fc234ef..ae097bed72 100644 --- a/src/saveload/afterload.cpp +++ b/src/saveload/afterload.cpp @@ -4410,7 +4410,7 @@ bool AfterLoadGame() /* We did load the "period" of the timer, but not the fired/elapsed. We can deduce that here. */ extern TimeoutTimer _new_competitor_timeout; _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 (SlXvIsFeatureMissing(XSLFI_SAVEGAME_ID) && IsSavegameVersionBefore(SLV_SAVEGAME_ID)) { diff --git a/src/saveload/misc_sl.cpp b/src/saveload/misc_sl.cpp index fd0aba070e..3bd3a679e6 100644 --- a/src/saveload/misc_sl.cpp +++ b/src/saveload/misc_sl.cpp @@ -61,9 +61,9 @@ static const SaveLoad _date_desc[] = { 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), /* 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, 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("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.value, SLE_UINT32, SLV_109, SLV_AI_START_DATE), + 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_fired", _new_competitor_timeout.fired, SLE_BOOL, SLV_AI_START_DATE, SL_MAX_VERSION), }; diff --git a/src/sl/misc_sl.cpp b/src/sl/misc_sl.cpp index f1b1eeb8e3..484de97bbf 100644 --- a/src/sl/misc_sl.cpp +++ b/src/sl/misc_sl.cpp @@ -105,8 +105,8 @@ static const NamedSaveLoad _date_desc[] = { NSL("", SLE_CONDNULL(1, SL_MIN_VERSION, SLV_10)), NSL("", SLE_CONDNULL(4, SLV_10, SLV_120)), NSL("company_tick_counter", SLEG_VAR(_cur_company_tick_index, SLE_FILE_U8 | SLE_VAR_U32)), - NSL("", SLEG_CONDVAR(_new_competitor_timeout.period, SLE_FILE_U16 | SLE_VAR_U32, SL_MIN_VERSION, SLV_109)), - NSL("", SLEG_CONDVAR_X(_new_competitor_timeout.period, SLE_UINT32, SLV_109, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE, 0, 0))), + NSL("", SLEG_CONDVAR(_new_competitor_timeout.period.value, SLE_FILE_U16 | SLE_VAR_U32, SL_MIN_VERSION, SLV_109)), + NSL("", SLEG_CONDVAR_X(_new_competitor_timeout.period.value, SLE_UINT32, SLV_109, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE, 0, 0))), NSL("trees_tick_counter", SLEG_VAR(_trees_tick_ctr, SLE_UINT8)), NSL("pause_mode", SLEG_CONDVAR(_pause_mode, SLE_UINT8, SLV_4, SL_MAX_VERSION)), NSL("game_events_overall", SLEG_CONDVAR_X(_game_events_overall, SLE_UINT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_GAME_EVENTS))), @@ -115,7 +115,7 @@ static const NamedSaveLoad _date_desc[] = { NSL("aspect_cfg_hash", SLEG_CONDVAR_X(_aspect_cfg_hash, SLE_UINT64, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_REALISTIC_TRAIN_BRAKING, 7))), NSL("aux_tileloop_tile", SLEG_CONDVAR_X(_aux_tileloop_tile, SLE_UINT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AUX_TILE_LOOP))), NSL("", SLE_CONDNULL(4, SLV_11, SLV_120)), - NSL("competitors_interval", SLEG_CONDVAR_X(_new_competitor_timeout.period, SLE_UINT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE))), + NSL("competitors_interval", SLEG_CONDVAR_X(_new_competitor_timeout.period.value, SLE_UINT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE))), NSL("competitors_interval_elapsed", SLEG_CONDVAR_X(_new_competitor_timeout.storage.elapsed, SLE_UINT32, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE))), NSL("competitors_interval_fired", SLEG_CONDVAR_X(_new_competitor_timeout.fired, SLE_BOOL, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE))), @@ -151,8 +151,8 @@ static const NamedSaveLoad _date_check_desc[] = { NSL("", SLE_CONDNULL(1, SL_MIN_VERSION, SLV_10)), NSL("", SLE_CONDNULL(4, SLV_10, SLV_120)), NSL("", SLE_NULL(1)), // _cur_company_tick_index - NSL("", SLE_CONDNULL(2, SL_MIN_VERSION, SLV_109)), // _new_competitor_timeout.period - NSL("", SLE_CONDNULL_X(4, SLV_109, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE, 0, 0))), // _new_competitor_timeout.period + NSL("", SLE_CONDNULL(2, SL_MIN_VERSION, SLV_109)), // _new_competitor_timeout.period.value + NSL("", SLE_CONDNULL_X(4, SLV_109, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_AI_START_DATE, 0, 0))), // _new_competitor_timeout.period.value NSL("", SLE_NULL(1)), // _trees_tick_ctr NSL("", SLE_CONDNULL(1, SLV_4, SL_MAX_VERSION)), // _pause_mode NSL("", SLE_CONDNULL_X(4, SL_MIN_VERSION, SL_MAX_VERSION, SlXvFeatureTest(XSLFTO_AND, XSLFI_GAME_EVENTS))), // _game_events_overall diff --git a/src/sl/oldloader_sl.cpp b/src/sl/oldloader_sl.cpp index 406859e5eb..2169a90224 100644 --- a/src/sl/oldloader_sl.cpp +++ b/src/sl/oldloader_sl.cpp @@ -1701,7 +1701,7 @@ static const OldChunks main_chunk[] = { 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 diff --git a/src/timer/timer_game_tick.cpp b/src/timer/timer_game_tick.cpp index cc3bbaa68e..34a1a3227c 100644 --- a/src/timer/timer_game_tick.cpp +++ b/src/timer/timer_game_tick.cpp @@ -19,13 +19,13 @@ template<> void IntervalTimer::Elapsed(TimerGameTick::TElapsed delta) { - if (this->period == 0) return; + if (this->period.value == 0) return; this->storage.elapsed += delta; uint count = 0; - while (this->storage.elapsed >= this->period) { - this->storage.elapsed -= this->period; + while (this->storage.elapsed >= this->period.value) { + this->storage.elapsed -= this->period.value; count++; } @@ -38,11 +38,11 @@ template<> void TimeoutTimer::Elapsed(TimerGameTick::TElapsed delta) { if (this->fired) return; - if (this->period == 0) return; + if (this->period.value == 0) return; this->storage.elapsed += delta; - if (this->storage.elapsed >= this->period) { + if (this->storage.elapsed >= this->period.value) { this->callback(); this->fired = true; } @@ -58,7 +58,16 @@ void TimerManager::Elapsed(TimerGameTick::TElapsed delta) #ifdef WITH_ASSERT template<> -void TimerManager::Validate(TimerGameTick::TPeriod) +void TimerManager::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::GetTimers()) { + assert(timer->period.priority != period.priority); + } } #endif /* WITH_ASSERT */ diff --git a/src/timer/timer_game_tick.h b/src/timer/timer_game_tick.h index 2a8a8cc103..599b328226 100644 --- a/src/timer/timer_game_tick.h +++ b/src/timer/timer_game_tick.h @@ -21,7 +21,34 @@ */ class TimerGameTick { public: - 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; struct TStorage { uint elapsed;