From 3a310f180272c03ccb05f5529313be7a0371ec90 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sun, 1 Dec 2024 15:15:21 +0000 Subject: [PATCH] Codechange: Store GRFConfig parameters in a vector. (#13137) All GRFConfigs have space allocated for parameters, but only configured GRFConfigs need them. Using a vector instead means that space is only used when parameters are used. --- src/gamelog.cpp | 2 +- src/gfxinit.cpp | 4 ++-- src/newgrf.cpp | 10 ++++---- src/newgrf_config.cpp | 30 ++++++++++++------------ src/newgrf_config.h | 9 ++++---- src/newgrf_gui.cpp | 18 +++++++-------- src/saveload/newgrf_sl.cpp | 47 ++++++++++++++++++++++++++------------ src/saveload/saveload.h | 3 ++- src/settings.cpp | 2 +- src/settings_gui.cpp | 2 +- src/survey.cpp | 6 ++--- 11 files changed, 76 insertions(+), 57 deletions(-) diff --git a/src/gamelog.cpp b/src/gamelog.cpp index 4384010b1a..7e802cba57 100644 --- a/src/gamelog.cpp +++ b/src/gamelog.cpp @@ -663,7 +663,7 @@ void Gamelog::GRFUpdate(const GRFConfig *oldc, const GRFConfig *newc) this->GRFCompatible(&nl[n]->ident); } - if (og->num_params != ng->num_params || og->param == ng->param) { + if (og->param != ng->param) { this->GRFParameters(ol[o]->ident.grfid); } diff --git a/src/gfxinit.cpp b/src/gfxinit.cpp index a76427c852..b30fbda3af 100644 --- a/src/gfxinit.cpp +++ b/src/gfxinit.cpp @@ -191,7 +191,7 @@ static void LoadSpriteTables() /* Baseset extra graphics */ GRFConfig *extra = new GRFConfig(used_set->GetOrCreateExtraConfig()); - if (extra->num_params == 0) extra->SetParameterDefaults(); + if (extra->param.empty()) extra->SetParameterDefaults(); ClrBit(extra->flags, GCF_INIT_ONLY); extra->next = top; @@ -397,7 +397,7 @@ bool GraphicsSet::IsConfigurable() const void GraphicsSet::CopyCompatibleConfig(const GraphicsSet &src) { const GRFConfig *src_cfg = src.GetExtraConfig(); - if (src_cfg == nullptr || src_cfg->num_params == 0) return; + if (src_cfg == nullptr || src_cfg->param.empty()) return; GRFConfig &dest_cfg = this->GetOrCreateExtraConfig(); if (dest_cfg.IsCompatible(src_cfg->version)) return; dest_cfg.CopyParams(*src_cfg); diff --git a/src/newgrf.cpp b/src/newgrf.cpp index 8dd9044c1d..6049366299 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -8160,7 +8160,7 @@ static bool ChangeGRFNumUsedParams(size_t len, ByteReader &buf) GrfMsg(2, "StaticGRFInfo: expected only 1 byte for 'INFO'->'NPAR' but got {}, ignoring this field", len); buf.Skip(len); } else { - _cur.grfconfig->num_valid_params = std::min(buf.ReadByte(), ClampTo(_cur.grfconfig->param.size())); + _cur.grfconfig->num_valid_params = std::min(buf.ReadByte(), GRFConfig::MAX_NUM_PARAMS); } return true; } @@ -8309,7 +8309,7 @@ static bool ChangeGRFParamMask(size_t len, ByteReader &buf) buf.Skip(len); } else { uint8_t param_nr = buf.ReadByte(); - if (param_nr >= _cur.grfconfig->param.size()) { + if (param_nr >= GRFConfig::MAX_NUM_PARAMS) { GrfMsg(2, "StaticGRFInfo: invalid parameter number in 'INFO'->'PARA'->'MASK', param {}, ignoring this field", param_nr); buf.Skip(len - 1); } else { @@ -8980,8 +8980,10 @@ GRFFile::GRFFile(const GRFConfig *config) /* Copy the initial parameter list * 'Uninitialised' parameters are zeroed as that is their default value when dynamically creating them. */ - this->param = config->param; - this->param_end = config->num_params; + this->param = {}; + + auto last = std::begin(config->param) + std::min(std::size(config->param), GRFConfig::MAX_NUM_PARAMS); + std::copy(std::begin(config->param), last, std::begin(this->param)); } /** diff --git a/src/newgrf_config.cpp b/src/newgrf_config.cpp index 341a796a1c..77364a0ac1 100644 --- a/src/newgrf_config.cpp +++ b/src/newgrf_config.cpp @@ -34,10 +34,7 @@ * Create a new GRFConfig. * @param filename Set the filename of this GRFConfig to filename. */ -GRFConfig::GRFConfig(const std::string &filename) : - filename(filename), num_valid_params(ClampTo(GRFConfig::param.size())) -{ -} +GRFConfig::GRFConfig(const std::string &filename) : filename(filename), num_valid_params(MAX_NUM_PARAMS) {} /** * Create a new GRFConfig that is a deep copy of an existing config. @@ -57,19 +54,17 @@ GRFConfig::GRFConfig(const GRFConfig &config) : flags(config.flags & ~(1 << GCF_COPY)), status(config.status), grf_bugs(config.grf_bugs), - param(config.param), - num_params(config.num_params), num_valid_params(config.num_valid_params), palette(config.palette), + has_param_defaults(config.has_param_defaults), param_info(config.param_info), - has_param_defaults(config.has_param_defaults) + param(config.param) { } -void GRFConfig::SetParams(const std::vector &pars) +void GRFConfig::SetParams(std::span pars) { - this->num_params = static_cast(std::min(this->param.size(), pars.size())); - std::copy(pars.begin(), pars.begin() + this->num_params, this->param.begin()); + this->param.assign(std::begin(pars), std::end(pars)); } /** @@ -86,7 +81,6 @@ bool GRFConfig::IsCompatible(uint32_t old_version) const */ void GRFConfig::CopyParams(const GRFConfig &src) { - this->num_params = src.num_params; this->param = src.param; } @@ -122,8 +116,7 @@ const char *GRFConfig::GetURL() const /** Set the default value for all parameters as specified by action14. */ void GRFConfig::SetParameterDefaults() { - this->num_params = 0; - this->param = {}; + this->param.clear(); if (!this->has_param_defaults) return; @@ -182,6 +175,9 @@ GRFError::GRFError(StringID severity, StringID message) : message(message), seve */ uint32_t GRFConfig::GetValue(const GRFParameterInfo &info) const { + /* If the parameter is not set then it must be 0. */ + if (info.param_nr >= std::size(this->param)) return 0; + /* GB doesn't work correctly with nbits == 32, so handle that case here. */ if (info.num_bit == 32) return this->param[info.param_nr]; @@ -197,6 +193,9 @@ void GRFConfig::SetValue(const GRFParameterInfo &info, uint32_t value) { value = Clamp(value, info.min_value, info.max_value); + /* Allocate the new parameter if it's not already present. */ + if (info.param_nr >= std::size(this->param)) this->param.resize(info.param_nr + 1); + /* SB doesn't work correctly with nbits == 32, so handle that case here. */ if (info.num_bit == 32) { this->param[info.param_nr] = value; @@ -204,7 +203,6 @@ void GRFConfig::SetValue(const GRFParameterInfo &info, uint32_t value) SB(this->param[info.param_nr], info.first_bit, info.num_bit, value); } - this->num_params = std::max(this->num_params, info.param_nr + 1); SetWindowDirty(WC_GAME_OPTIONS, WN_GAME_OPTIONS_NEWGRF_STATE); } @@ -711,9 +709,9 @@ GRFConfig *GetGRFConfig(uint32_t grfid, uint32_t mask) std::string GRFBuildParamList(const GRFConfig *c) { std::string result; - for (uint i = 0; i < c->num_params; i++) { + for (const uint32_t &value : c->param) { if (!result.empty()) result += ' '; - result += std::to_string(c->param[i]); + result += std::to_string(value); } return result; } diff --git a/src/newgrf_config.h b/src/newgrf_config.h index a5e41ff5dc..cab22aa6ec 100644 --- a/src/newgrf_config.h +++ b/src/newgrf_config.h @@ -152,6 +152,8 @@ struct GRFParameterInfo { /** Information about GRF, used in the game and (part of it) in savegames */ struct GRFConfig : ZeroedMemoryAllocator { + static constexpr uint8_t MAX_NUM_PARAMS = 0x80; + GRFConfig(const std::string &filename = std::string{}); GRFConfig(const GRFConfig &config); @@ -171,17 +173,16 @@ struct GRFConfig : ZeroedMemoryAllocator { uint8_t flags; ///< NOSAVE: GCF_Flags, bitset GRFStatus status; ///< NOSAVE: GRFStatus, enum uint32_t grf_bugs; ///< NOSAVE: bugs in this GRF in this run, @see enum GRFBugs - std::array param; ///< GRF parameters - uint8_t num_params; ///< Number of used parameters uint8_t num_valid_params; ///< NOSAVE: Number of valid parameters (action 0x14) uint8_t palette; ///< GRFPalette, bitset - std::vector> param_info; ///< NOSAVE: extra information about the parameters bool has_param_defaults; ///< NOSAVE: did this newgrf specify any defaults for it's parameters + std::vector> param_info; ///< NOSAVE: extra information about the parameters + std::vector param; ///< GRF parameters struct GRFConfig *next; ///< NOSAVE: Next item in the linked list bool IsCompatible(uint32_t old_version) const; - void SetParams(const std::vector &pars); + void SetParams(std::span pars); void CopyParams(const GRFConfig &src); uint32_t GetValue(const GRFParameterInfo &info) const; diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp index 53f286aa50..c706661015 100644 --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -108,7 +108,7 @@ static void ShowNewGRFInfo(const GRFConfig *c, const Rect &r, bool show_params) /* Show GRF parameter list */ if (show_params) { - if (c->num_params > 0) { + if (!c->param.empty()) { SetDParam(0, STR_JUST_RAW_STRING); SetDParamStr(1, GRFBuildParamList(c)); } else { @@ -221,7 +221,7 @@ struct NewGRFParametersWindow : public Window { } case WID_NP_NUMPAR: { - SetDParamMaxValue(0, this->grf_config->param.size()); + SetDParamMaxValue(0, GRFConfig::MAX_NUM_PARAMS); Dimension d = GetStringBoundingBox(this->GetWidget(widget)->widget_data); d.width += padding.width; d.height += padding.height; @@ -335,8 +335,8 @@ struct NewGRFParametersWindow : public Window { { switch (widget) { case WID_NP_NUMPAR_DEC: - if (this->editable && !this->action14present && this->grf_config->num_params > 0) { - this->grf_config->num_params--; + if (this->editable && !this->action14present && !this->grf_config->param.empty()) { + this->grf_config->param.pop_back(); this->InvalidateData(); SetWindowDirty(WC_GAME_OPTIONS, WN_GAME_OPTIONS_NEWGRF_STATE); } @@ -344,8 +344,8 @@ struct NewGRFParametersWindow : public Window { case WID_NP_NUMPAR_INC: { GRFConfig *c = this->grf_config; - if (this->editable && !this->action14present && c->num_params < c->num_valid_params) { - c->param[c->num_params++] = 0; + if (this->editable && !this->action14present && c->param.size() < c->num_valid_params) { + this->grf_config->param.emplace_back(0); this->InvalidateData(); SetWindowDirty(WC_GAME_OPTIONS, WN_GAME_OPTIONS_NEWGRF_STATE); } @@ -487,11 +487,11 @@ struct NewGRFParametersWindow : public Window { { if (!gui_scope) return; if (!this->action14present) { - this->SetWidgetDisabledState(WID_NP_NUMPAR_DEC, !this->editable || this->grf_config->num_params == 0); - this->SetWidgetDisabledState(WID_NP_NUMPAR_INC, !this->editable || this->grf_config->num_params >= this->grf_config->num_valid_params); + this->SetWidgetDisabledState(WID_NP_NUMPAR_DEC, !this->editable || this->grf_config->param.empty()); + this->SetWidgetDisabledState(WID_NP_NUMPAR_INC, !this->editable || std::size(this->grf_config->param) >= this->grf_config->num_valid_params); } - this->vscroll->SetCount(this->action14present ? this->grf_config->num_valid_params : this->grf_config->num_params); + this->vscroll->SetCount(this->action14present ? this->grf_config->num_valid_params : GRFConfig::MAX_NUM_PARAMS); if (this->clicked_row != INT32_MAX && this->clicked_row >= this->vscroll->GetCount()) { this->clicked_row = INT32_MAX; this->CloseChildWindows(WC_QUERY_STRING); diff --git a/src/saveload/newgrf_sl.cpp b/src/saveload/newgrf_sl.cpp index 934a642741..187eaf442d 100644 --- a/src/saveload/newgrf_sl.cpp +++ b/src/saveload/newgrf_sl.cpp @@ -61,44 +61,61 @@ void NewGRFMappingChunkHandler::Load() const } } - -static const SaveLoad _grfconfig_desc[] = { - SLE_SSTR(GRFConfig, filename, SLE_STR), - SLE_VAR(GRFConfig, ident.grfid, SLE_UINT32), - SLE_ARR(GRFConfig, ident.md5sum, SLE_UINT8, 16), - SLE_CONDVAR(GRFConfig, version, SLE_UINT32, SLV_151, SL_MAX_VERSION), - SLE_ARR(GRFConfig, param, SLE_UINT32, 0x80), - SLE_VAR(GRFConfig, num_params, SLE_UINT8), - SLE_CONDVAR(GRFConfig, palette, SLE_UINT8, SLV_101, SL_MAX_VERSION), -}; - - struct NGRFChunkHandler : ChunkHandler { NGRFChunkHandler() : ChunkHandler('NGRF', CH_TABLE) {} + static inline std::array param; + static inline uint8_t num_params; + + static inline const SaveLoad description[] = { + SLE_SSTR(GRFConfig, filename, SLE_STR), + SLE_VAR(GRFConfig, ident.grfid, SLE_UINT32), + SLE_ARR(GRFConfig, ident.md5sum, SLE_UINT8, 16), + SLE_CONDVAR(GRFConfig, version, SLE_UINT32, SLV_151, SL_MAX_VERSION), + SLEG_ARR("param", param, SLE_UINT32, std::size(param)), + SLEG_VAR("num_params", num_params, SLE_UINT8), + SLE_CONDVAR(GRFConfig, palette, SLE_UINT8, SLV_101, SL_MAX_VERSION), + }; + + void SaveParameters(const GRFConfig &config) const + { + /* Transfer config to fixed array, ensure unused entries are blanked. */ + param.fill(0); + num_params = static_cast(std::size(config.param)); + std::copy(std::begin(config.param), std::end(config.param), std::begin(param)); + } + void Save() const override { - SlTableHeader(_grfconfig_desc); + SlTableHeader(description); int index = 0; for (GRFConfig *c = _grfconfig; c != nullptr; c = c->next) { if (HasBit(c->flags, GCF_STATIC) || HasBit(c->flags, GCF_INIT_ONLY)) continue; + this->SaveParameters(*c); SlSetArrayIndex(index++); - SlObject(c, _grfconfig_desc); + SlObject(c, description); } } + void LoadParameters(GRFConfig &config) const + { + /* Transfer used part of fixed array to config. */ + auto last = std::begin(param) + std::min(std::size(param), num_params); + config.param.assign(std::begin(param), last); + } void LoadCommon(GRFConfig *&grfconfig) const { - const std::vector slt = SlCompatTableHeader(_grfconfig_desc, _grfconfig_sl_compat); + const std::vector slt = SlCompatTableHeader(description, _grfconfig_sl_compat); ClearGRFConfigList(&grfconfig); while (SlIterateArray() != -1) { GRFConfig *c = new GRFConfig(); SlObject(c, slt); if (IsSavegameVersionBefore(SLV_101)) c->SetSuitablePalette(); + this->LoadParameters(*c); AppendToGRFConfigList(&grfconfig, c); } } diff --git a/src/saveload/saveload.h b/src/saveload/saveload.h index e90d040187..7bb1c6d8d0 100644 --- a/src/saveload/saveload.h +++ b/src/saveload/saveload.h @@ -1196,8 +1196,9 @@ inline constexpr bool SlCheckVarSize(SaveLoadType cmd, VarType type, size_t leng * @param name The name of the field. * @param variable Name of the global variable. * @param type Storage of the data in memory and in the savegame. + * @param length Number of elements in the array. */ -#define SLEG_ARR(name, variable, type) SLEG_CONDARR(name, variable, type, lengthof(variable), SL_MIN_VERSION, SL_MAX_VERSION) +#define SLEG_ARR(name, variable, type, length) SLEG_CONDARR(name, variable, type, length, SL_MIN_VERSION, SL_MAX_VERSION) /** * Storage of a global \c std::string in every savegame version. diff --git a/src/settings.cpp b/src/settings.cpp index d238ec97cb..ba77acecc1 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -1220,7 +1220,7 @@ static void GraphicsSetSaveConfig(IniFile &ini) group.GetOrCreateItem("shortname").SetValue(fmt::format("{:08X}", BSWAP32(used_set->shortname))); const GRFConfig *extra_cfg = used_set->GetExtraConfig(); - if (extra_cfg != nullptr && extra_cfg->num_params > 0) { + if (extra_cfg != nullptr && !extra_cfg->param.empty()) { group.GetOrCreateItem("extra_version").SetValue(fmt::format("{}", extra_cfg->version)); group.GetOrCreateItem("extra_params").SetValue(GRFBuildParamList(extra_cfg)); } diff --git a/src/settings_gui.cpp b/src/settings_gui.cpp index 5d8ca16c54..893dbca177 100644 --- a/src/settings_gui.cpp +++ b/src/settings_gui.cpp @@ -814,7 +814,7 @@ struct GameOptionsWindow : Window { auto *used_set = BaseGraphics::GetUsedSet(); if (used_set == nullptr || !used_set->IsConfigurable()) break; GRFConfig &extra_cfg = used_set->GetOrCreateExtraConfig(); - if (extra_cfg.num_params == 0) extra_cfg.SetParameterDefaults(); + if (extra_cfg.param.empty()) extra_cfg.SetParameterDefaults(); OpenGRFParameterWindow(true, &extra_cfg, _game_mode == GM_MENU); if (_game_mode == GM_MENU) this->reload = true; break; diff --git a/src/survey.cpp b/src/survey.cpp index 123c82563e..8154b7e62a 100644 --- a/src/survey.cpp +++ b/src/survey.cpp @@ -273,8 +273,8 @@ void SurveyConfiguration(nlohmann::json &survey) if (BaseGraphics::GetUsedSet() != nullptr) { survey["graphics_set"] = fmt::format("{}.{}", BaseGraphics::GetUsedSet()->name, BaseGraphics::GetUsedSet()->version); const GRFConfig *extra_cfg = BaseGraphics::GetUsedSet()->GetExtraConfig(); - if (extra_cfg != nullptr && extra_cfg->num_params > 0) { - survey["graphics_set_parameters"] = std::span(extra_cfg->param.data(), extra_cfg->num_params); + if (extra_cfg != nullptr && !extra_cfg->param.empty()) { + survey["graphics_set_parameters"] = std::span(extra_cfg->param); } else { survey["graphics_set_parameters"] = std::span(); } @@ -370,7 +370,7 @@ void SurveyGrfs(nlohmann::json &survey) if ((c->palette & GRFP_BLT_MASK) == GRFP_BLT_32BPP) grf["blitter"] = "32bpp"; grf["is_static"] = HasBit(c->flags, GCF_STATIC); - grf["parameters"] = std::span(c->param.data(), c->num_params); + grf["parameters"] = std::span(c->param); } }