From d030d17bd6e84e0f311a24e0a354150f972b90e8 Mon Sep 17 00:00:00 2001 From: frosch Date: Mon, 28 Apr 2025 17:48:55 +0200 Subject: [PATCH] Codechange: Do not use a mutable global to return calculated VarAction2 results. --- src/newgrf_engine.cpp | 6 ++--- src/newgrf_engine.h | 2 +- src/newgrf_profiling.cpp | 28 +++++++++++++++--------- src/newgrf_profiling.h | 2 +- src/newgrf_spritegroup.cpp | 34 ++++++++++++++-------------- src/newgrf_spritegroup.h | 45 ++++++++++++++++++++++++-------------- src/newgrf_station.cpp | 6 ++--- src/newgrf_station.h | 2 +- 8 files changed, 71 insertions(+), 54 deletions(-) diff --git a/src/newgrf_engine.cpp b/src/newgrf_engine.cpp index 021ff8a264..cd17a604f4 100644 --- a/src/newgrf_engine.cpp +++ b/src/newgrf_engine.cpp @@ -998,14 +998,14 @@ static uint32_t VehicleGetVariable(Vehicle *v, const VehicleScopeResolver *objec } -/* virtual */ const SpriteGroup *VehicleResolverObject::ResolveReal(const RealSpriteGroup &group) const +/* virtual */ ResolverResult VehicleResolverObject::ResolveReal(const RealSpriteGroup &group) const { const Vehicle *v = this->self_scope.v; if (v == nullptr) { if (!group.loading.empty()) return group.loading[0]; if (!group.loaded.empty()) return group.loaded[0]; - return nullptr; + return std::monostate{}; } const Order &order = v->First()->current_order; @@ -1014,7 +1014,7 @@ static uint32_t VehicleGetVariable(Vehicle *v, const VehicleScopeResolver *objec uint totalsets = static_cast(in_motion ? group.loaded.size() : group.loading.size()); - if (totalsets == 0) return nullptr; + if (totalsets == 0) return std::monostate{}; uint set = (v->cargo.StoredCount() * totalsets) / std::max(1u, v->cargo_cap); set = std::min(set, totalsets - 1); diff --git a/src/newgrf_engine.h b/src/newgrf_engine.h index 92d87a795f..0bf8f3af2f 100644 --- a/src/newgrf_engine.h +++ b/src/newgrf_engine.h @@ -64,7 +64,7 @@ struct VehicleResolverObject : public SpecializedResolverObjectcur_call.time = (uint32_t)time_point_cast(high_resolution_clock::now()).time_since_epoch().count() - this->cur_call.time; - if (result == nullptr) { - this->cur_call.result = 0; - } else if (result->type == SGT_CALLBACK) { - this->cur_call.result = static_cast(result)->result; - } else if (result->type == SGT_RESULT) { - this->cur_call.result = GetSpriteLocalID(static_cast(result)->sprite); - } else { - this->cur_call.result = result->nfo_line; - } + struct visitor { + uint32_t operator()(std::monostate) + { + return 0; + } + uint32_t operator()(CallbackResult cb_result) + { + return cb_result; + } + uint32_t operator()(const SpriteGroup *group) + { + if (group == nullptr) return 0; + if (group->type != SGT_RESULT) return group->nfo_line; + return GetSpriteLocalID(static_cast(group)->sprite); + } + }; + this->cur_call.result = std::visit(visitor{}, result); this->calls.push_back(this->cur_call); } diff --git a/src/newgrf_profiling.h b/src/newgrf_profiling.h index fb7887d63e..89b1222cc9 100644 --- a/src/newgrf_profiling.h +++ b/src/newgrf_profiling.h @@ -24,7 +24,7 @@ struct NewGRFProfiler { ~NewGRFProfiler(); void BeginResolve(const ResolverObject &resolver); - void EndResolve(const SpriteGroup *result); + void EndResolve(const ResolverResult &result); void RecursiveResolve(); void Start(); diff --git a/src/newgrf_spritegroup.cpp b/src/newgrf_spritegroup.cpp index 59d76141be..e867f7b626 100644 --- a/src/newgrf_spritegroup.cpp +++ b/src/newgrf_spritegroup.cpp @@ -31,9 +31,9 @@ TemporaryStorageArray _temp_store; * @param top_level true if this is a top-level SpriteGroup, false if used nested in another SpriteGroup. * @return the resolved group */ -/* static */ const SpriteGroup *SpriteGroup::Resolve(const SpriteGroup *group, ResolverObject &object, bool top_level) +/* static */ ResolverResult SpriteGroup::Resolve(const SpriteGroup *group, ResolverObject &object, bool top_level) { - if (group == nullptr) return nullptr; + if (group == nullptr) return std::monostate{}; const GRFFile *grf = object.grffile; auto profiler = std::ranges::find(_newgrf_profilers, grf, &NewGRFProfiler::grffile); @@ -44,7 +44,7 @@ TemporaryStorageArray _temp_store; } else if (top_level) { profiler->BeginResolve(object); _temp_store.ClearChanges(); - const SpriteGroup *result = group->Resolve(object); + auto result = group->Resolve(object); profiler->EndResolve(result); return result; } else { @@ -120,12 +120,12 @@ static inline uint32_t GetVariable(const ResolverObject &object, ScopeResolver * * @param group Group to get. * @return The available sprite group. */ -/* virtual */ const SpriteGroup *ResolverObject::ResolveReal(const RealSpriteGroup &group) const +/* virtual */ ResolverResult ResolverObject::ResolveReal(const RealSpriteGroup &group) const { if (!group.loaded.empty()) return group.loaded[0]; if (!group.loading.empty()) return group.loading[0]; - return nullptr; + return std::monostate{}; } /** @@ -185,7 +185,7 @@ static bool RangeHighComparator(const DeterministicSpriteGroupRange &range, uint return range.high < value; } -const SpriteGroup *DeterministicSpriteGroup::Resolve(ResolverObject &object) const +/* virtual */ ResolverResult DeterministicSpriteGroup::Resolve(ResolverObject &object) const { uint32_t last_value = 0; uint32_t value = 0; @@ -196,13 +196,9 @@ const SpriteGroup *DeterministicSpriteGroup::Resolve(ResolverObject &object) con /* Try to get the variable. We shall assume it is available, unless told otherwise. */ bool available = true; if (adjust.variable == 0x7E) { - const SpriteGroup *subgroup = SpriteGroup::Resolve(adjust.subroutine, object, false); - if (subgroup == nullptr) { - value = UINT16_MAX; - } else { - value = subgroup->GetCallbackResult(); - if (value == CALLBACK_FAILED) value = UINT16_MAX; - } + auto subgroup = SpriteGroup::Resolve(adjust.subroutine, object, false); + auto *subvalue = std::get_if(&subgroup); + value = subvalue != nullptr ? *subvalue : UINT16_MAX; /* Note: 'last_value' and 'reseed' are shared between the main chain and the procedure */ } else if (adjust.variable == 0x7B) { @@ -230,9 +226,7 @@ const SpriteGroup *DeterministicSpriteGroup::Resolve(ResolverObject &object) con if (this->calculated_result) { /* nvar == 0 is a special case -- we turn our value into a callback result */ - static CallbackResultSpriteGroup nvarzero(0); - nvarzero.result = GB(value, 0, 15); - return &nvarzero; + return static_cast(GB(value, 0, 15)); } if (this->ranges.size() > 4) { @@ -253,7 +247,7 @@ const SpriteGroup *DeterministicSpriteGroup::Resolve(ResolverObject &object) con } -const SpriteGroup *RandomizedSpriteGroup::Resolve(ResolverObject &object) const +/* virtual */ ResolverResult RandomizedSpriteGroup::Resolve(ResolverObject &object) const { ScopeResolver *scope = object.GetScope(this->var_scope, this->count); if (object.callback == CBID_RANDOM_TRIGGER) { @@ -273,8 +267,12 @@ const SpriteGroup *RandomizedSpriteGroup::Resolve(ResolverObject &object) const return SpriteGroup::Resolve(this->groups[index], object, false); } +/* virtual */ ResolverResult CallbackResultSpriteGroup::Resolve(ResolverObject &) const +{ + return this->result; +} -const SpriteGroup *RealSpriteGroup::Resolve(ResolverObject &object) const +/* virtual */ ResolverResult RealSpriteGroup::Resolve(ResolverObject &object) const { return object.ResolveReal(*this); } diff --git a/src/newgrf_spritegroup.h b/src/newgrf_spritegroup.h index 348ff33a26..8af73712ef 100644 --- a/src/newgrf_spritegroup.h +++ b/src/newgrf_spritegroup.h @@ -46,6 +46,15 @@ enum SpriteGroupType : uint8_t { struct SpriteGroup; struct ResolverObject; +using CallbackResult = uint16_t; + +/** + * Result of resolving sprite groups: + * - std::monostate: Failure. + * - CallbackResult: Callback result. + * - SpriteGroup: ResultSpriteGroup, TileLayoutSpriteGroup, IndustryProductionSpriteGroup + */ +using ResolverResult = std::variant; /* SPRITE_WIDTH is 24. ECS has roughly 30 sprite groups per real sprite. * Adding an 'extra' margin would be assuming 64 sprite groups per real @@ -59,7 +68,7 @@ struct SpriteGroup : SpriteGroupPool::PoolItem<&_spritegroup_pool> { protected: SpriteGroup(SpriteGroupType type) : type(type) {} /** Base sprite group resolver */ - virtual const SpriteGroup *Resolve([[maybe_unused]] ResolverObject &object) const { return this; }; + virtual ResolverResult Resolve([[maybe_unused]] ResolverObject &object) const { return this; }; public: virtual ~SpriteGroup() = default; @@ -67,9 +76,7 @@ public: uint32_t nfo_line = 0; SpriteGroupType type{}; - virtual uint16_t GetCallbackResult() const { return CALLBACK_FAILED; } - - static const SpriteGroup *Resolve(const SpriteGroup *group, ResolverObject &object, bool top_level = true); + static ResolverResult Resolve(const SpriteGroup *group, ResolverObject &object, bool top_level = true); }; @@ -89,7 +96,7 @@ struct RealSpriteGroup : SpriteGroup { std::vector loading{}; ///< List of loading groups (can be SpriteIDs or Callback results) protected: - const SpriteGroup *Resolve(ResolverObject &object) const override; + ResolverResult Resolve(ResolverObject &object) const override; }; /* Shared by deterministic and random groups. */ @@ -178,7 +185,7 @@ struct DeterministicSpriteGroup : SpriteGroup { const SpriteGroup *error_group = nullptr; // was first range, before sorting ranges protected: - const SpriteGroup *Resolve(ResolverObject &object) const override; + ResolverResult Resolve(ResolverObject &object) const override; }; enum RandomizedSpriteGroupCompareMode : uint8_t { @@ -200,7 +207,7 @@ struct RandomizedSpriteGroup : SpriteGroup { std::vector groups{}; ///< Take the group with appropriate index: protected: - const SpriteGroup *Resolve(ResolverObject &object) const override; + ResolverResult Resolve(ResolverObject &object) const override; }; @@ -211,10 +218,12 @@ struct CallbackResultSpriteGroup : SpriteGroup { * Creates a spritegroup representing a callback result * @param value The value that was used to represent this callback result */ - explicit CallbackResultSpriteGroup(uint16_t value) : SpriteGroup(SGT_CALLBACK), result(value) {} + explicit CallbackResultSpriteGroup(CallbackResult value) : SpriteGroup(SGT_CALLBACK), result(value) {} - uint16_t result = 0; - uint16_t GetCallbackResult() const override { return this->result; } + CallbackResult result = 0; + +protected: + ResolverResult Resolve(ResolverObject &object) const override; }; @@ -311,7 +320,7 @@ struct ResolverObject { virtual ~ResolverObject() = default; - const SpriteGroup *DoResolve() + ResolverResult DoResolve() { return SpriteGroup::Resolve(this->root_spritegroup, *this); } @@ -342,8 +351,9 @@ public: inline const TSpriteGroup *Resolve() { auto result = this->DoResolve(); - if (result == nullptr || result->type != TSpriteGroup::TYPE) return nullptr; - return static_cast(result); + const auto *group = std::get_if(&result); + if (group == nullptr || *group == nullptr || (*group)->type != TSpriteGroup::TYPE) return nullptr; + return static_cast(*group); } /** @@ -364,13 +374,14 @@ public: * Resolve callback. * @return Callback result. */ - inline uint16_t ResolveCallback() + inline CallbackResult ResolveCallback() { - const SpriteGroup *result = this->DoResolve(); - return result != nullptr ? result->GetCallbackResult() : CALLBACK_FAILED; + auto result = this->DoResolve(); + const auto *value = std::get_if(&result); + return value != nullptr ? *value : CALLBACK_FAILED; } - virtual const SpriteGroup *ResolveReal(const RealSpriteGroup &group) const; + virtual ResolverResult ResolveReal(const RealSpriteGroup &group) const; virtual ScopeResolver *GetScope(VarSpriteGroupScope scope = VSG_SCOPE_SELF, uint8_t relative = 0); diff --git a/src/newgrf_station.cpp b/src/newgrf_station.cpp index a92f455370..53ed441f43 100644 --- a/src/newgrf_station.cpp +++ b/src/newgrf_station.cpp @@ -519,11 +519,11 @@ uint32_t Waypoint::GetNewGRFVariable(const ResolverObject &, uint8_t variable, [ return UINT_MAX; } -/* virtual */ const SpriteGroup *StationResolverObject::ResolveReal(const RealSpriteGroup &group) const +/* virtual */ ResolverResult StationResolverObject::ResolveReal(const RealSpriteGroup &group) const { if (this->station_scope.st == nullptr || !Station::IsExpected(this->station_scope.st)) { if (!group.loading.empty()) return group.loading[0]; - return nullptr; + return std::monostate{}; } uint cargo = 0; @@ -566,7 +566,7 @@ uint32_t Waypoint::GetNewGRFVariable(const ResolverObject &, uint8_t variable, [ } if (!group.loading.empty()) return group.loading[0]; - return nullptr; + return std::monostate{}; } GrfSpecFeature StationResolverObject::GetFeature() const diff --git a/src/newgrf_station.h b/src/newgrf_station.h index 7857df82b6..0a8301387e 100644 --- a/src/newgrf_station.h +++ b/src/newgrf_station.h @@ -75,7 +75,7 @@ struct StationResolverObject : public SpecializedResolverObject