From 5aa4ad513506ce80ea28567361fd42b163e354a7 Mon Sep 17 00:00:00 2001 From: frosch Date: Sun, 23 Mar 2025 19:23:31 +0100 Subject: [PATCH] Codechange: Pass unformatted strings from GetStringPtr as std::string_view. (#13871) --- src/game/game_text.cpp | 4 +- src/game/game_text.hpp | 2 +- src/newgrf_config.cpp | 16 +++---- src/newgrf_gui.cpp | 26 ++++++----- src/newgrf_text.cpp | 33 +++++++------- src/newgrf_text.h | 6 +-- src/strings.cpp | 100 ++++++++++++++++++++++------------------- src/strings_func.h | 2 +- 8 files changed, 97 insertions(+), 92 deletions(-) diff --git a/src/game/game_text.cpp b/src/game/game_text.cpp index bf8192ed97..ff337f5e44 100644 --- a/src/game/game_text.cpp +++ b/src/game/game_text.cpp @@ -315,10 +315,10 @@ GameStrings *_current_data = nullptr; * @param id The ID of the game string. * @return The encoded string. */ -const char *GetGameStringPtr(StringIndexInTab id) +std::string_view GetGameStringPtr(StringIndexInTab id) { if (_current_data == nullptr || _current_data->cur_language == nullptr || id.base() >= _current_data->cur_language->lines.size()) return GetStringPtr(STR_UNDEFINED); - return _current_data->cur_language->lines[id].c_str(); + return _current_data->cur_language->lines[id]; } /** diff --git a/src/game/game_text.hpp b/src/game/game_text.hpp index e12851a928..60a10e2dcc 100644 --- a/src/game/game_text.hpp +++ b/src/game/game_text.hpp @@ -29,7 +29,7 @@ struct StringParam { using StringParams = std::vector; using StringParamsList = std::vector; -const char *GetGameStringPtr(StringIndexInTab id); +std::string_view GetGameStringPtr(StringIndexInTab id); const StringParams &GetGameStringParams(StringIndexInTab id); const std::string &GetGameStringName(StringIndexInTab id); void RegisterGameTranslation(class Squirrel *engine); diff --git a/src/newgrf_config.cpp b/src/newgrf_config.cpp index 15d8f91ee5..9eca36c24c 100644 --- a/src/newgrf_config.cpp +++ b/src/newgrf_config.cpp @@ -84,8 +84,8 @@ void GRFConfig::CopyParams(const GRFConfig &src) */ std::string GRFConfig::GetName() const { - const char *name = GetGRFStringFromGRFText(this->name); - return StrEmpty(name) ? this->filename : name; + auto name = GetGRFStringFromGRFText(this->name); + return name.has_value() ? std::string(*name) : this->filename; } /** @@ -94,9 +94,9 @@ std::string GRFConfig::GetName() const */ std::optional GRFConfig::GetDescription() const { - const char *str = GetGRFStringFromGRFText(this->info); - if (StrEmpty(str)) return std::nullopt; - return std::string(str); + auto str = GetGRFStringFromGRFText(this->info); + if (!str.has_value()) return std::nullopt; + return std::string(*str); } /** @@ -105,9 +105,9 @@ std::optional GRFConfig::GetDescription() const */ std::optional GRFConfig::GetURL() const { - const char *str = GetGRFStringFromGRFText(this->url); - if (StrEmpty(str)) return std::nullopt; - return std::string(str); + auto str = GetGRFStringFromGRFText(this->url); + if (!str.has_value()) return std::nullopt; + return std::string(*str); } /** Set the default value for all parameters as specified by action14. */ diff --git a/src/newgrf_gui.cpp b/src/newgrf_gui.cpp index 07db9487fe..607614366b 100644 --- a/src/newgrf_gui.cpp +++ b/src/newgrf_gui.cpp @@ -232,9 +232,9 @@ struct NewGRFParametersWindow : public Window { Dimension suggestion = {500U - WidgetDimensions::scaled.frametext.Horizontal(), (uint)GetCharacterHeight(FS_NORMAL) * 4 + WidgetDimensions::scaled.frametext.Vertical()}; for (const auto &par_info : this->grf_config.param_info) { if (!par_info.has_value()) continue; - const char *desc = GetGRFStringFromGRFText(par_info->desc); - if (desc == nullptr) continue; - Dimension d = GetStringMultiLineBoundingBox(desc, suggestion); + auto desc = GetGRFStringFromGRFText(par_info->desc); + if (!desc.has_value()) continue; + Dimension d = GetStringMultiLineBoundingBox(*desc, suggestion); d.height += WidgetDimensions::scaled.frametext.Vertical(); suggestion = maxdim(d, suggestion); } @@ -260,7 +260,7 @@ struct NewGRFParametersWindow : public Window { auto it = std::ranges::lower_bound(par_info.value_names, value, std::less{}, &GRFParameterInfo::ValueName::first); if (it != std::end(par_info.value_names) && it->first == value) { - if (const char *label = GetGRFStringFromGRFText(it->second); label != nullptr) return {STR_JUST_RAW_STRING, label}; + if (auto label = GetGRFStringFromGRFText(it->second); label.has_value()) return {STR_JUST_RAW_STRING, std::string(*label)}; } return {STR_JUST_INT, value}; @@ -269,10 +269,10 @@ struct NewGRFParametersWindow : public Window { std::string GetSettingString(const GRFParameterInfo &par_info, int i, uint32_t value) const { auto [param1, param2] = this->GetValueParams(par_info, value); - const char *name = GetGRFStringFromGRFText(par_info.name); - return name == nullptr - ? GetString(STR_NEWGRF_PARAMETERS_SETTING, STR_NEWGRF_PARAMETERS_DEFAULT_NAME, i + 1, param1, param2) - : GetString(STR_NEWGRF_PARAMETERS_SETTING, STR_JUST_RAW_STRING, name, param1, param2); + auto name = GetGRFStringFromGRFText(par_info.name); + return name.has_value() + ? GetString(STR_NEWGRF_PARAMETERS_SETTING, STR_JUST_RAW_STRING, std::string(*name), param1, param2) + : GetString(STR_NEWGRF_PARAMETERS_SETTING, STR_NEWGRF_PARAMETERS_DEFAULT_NAME, i + 1, param1, param2); } void DrawWidget(const Rect &r, WidgetID widget) const override @@ -280,9 +280,9 @@ struct NewGRFParametersWindow : public Window { if (widget == WID_NP_DESCRIPTION) { if (!this->HasParameterInfo(this->clicked_row)) return; const GRFParameterInfo &par_info = this->GetParameterInfo(this->clicked_row); - const char *desc = GetGRFStringFromGRFText(par_info.desc); - if (desc == nullptr) return; - DrawStringMultiLine(r.Shrink(WidgetDimensions::scaled.framerect), desc, TC_BLACK); + auto desc = GetGRFStringFromGRFText(par_info.desc); + if (!desc.has_value()) return; + DrawStringMultiLine(r.Shrink(WidgetDimensions::scaled.framerect), *desc, TC_BLACK); return; } else if (widget != WID_NP_BACKGROUND) { return; @@ -385,7 +385,9 @@ struct NewGRFParametersWindow : public Window { DropDownList list; for (const auto &[value, name] : par_info.value_names) { - list.push_back(MakeDropDownListStringItem(GetString(STR_JUST_RAW_STRING, GetGRFStringFromGRFText(name)), value)); + auto text = GetGRFStringFromGRFText(name); + assert(text.has_value()); // ensured by "complete_labels" + list.push_back(MakeDropDownListStringItem(GetString(STR_JUST_RAW_STRING, std::string(*text)), value)); } ShowDropDownListAt(this, std::move(list), old_val, WID_NP_SETTING_DROPDOWN, wi_rect, COLOUR_ORANGE); diff --git a/src/newgrf_text.cpp b/src/newgrf_text.cpp index 2ee0165534..f90310911b 100644 --- a/src/newgrf_text.cpp +++ b/src/newgrf_text.cpp @@ -184,7 +184,7 @@ struct UnmappedChoiceList { } /* "" */ - dest << this->strings[0].rdbuf() << '\0'; + dest << this->strings[0].rdbuf(); } else { if (this->type == SCC_PLURAL_LIST) { *d++ = lm->plural_form; @@ -611,18 +611,18 @@ StringID GetGRFStringID(uint32_t grfid, GRFStringID stringid) * current language nullptr is returned. * @param text_list The GRFTextList to get the string from. */ -const char *GetGRFStringFromGRFText(const GRFTextList &text_list) +std::optional GetGRFStringFromGRFText(const GRFTextList &text_list) { - const char *default_text = nullptr; + std::optional default_text; /* Search the list of lang-strings of this stringid for current lang */ for (const auto &text : text_list) { - if (text.langid == _currentLangID) return text.text.c_str(); + if (text.langid == _currentLangID) return text.text; /* If the current string is English or American, set it as the * fallback language if the specific language isn't available. */ - if (text.langid == GRFLX_UNSPECIFIED || (default_text == nullptr && (text.langid == GRFLX_ENGLISH || text.langid == GRFLX_AMERICAN))) { - default_text = text.text.c_str(); + if (text.langid == GRFLX_UNSPECIFIED || (!default_text.has_value() && (text.langid == GRFLX_ENGLISH || text.langid == GRFLX_AMERICAN))) { + default_text = text.text; } } @@ -636,21 +636,21 @@ const char *GetGRFStringFromGRFText(const GRFTextList &text_list) * current language nullptr is returned. * @param text The GRFTextList to get the string from. */ -const char *GetGRFStringFromGRFText(const GRFTextWrapper &text) +std::optional GetGRFStringFromGRFText(const GRFTextWrapper &text) { - return text ? GetGRFStringFromGRFText(*text) : nullptr; + return text ? GetGRFStringFromGRFText(*text) : std::nullopt; } /** * Get a C-string from a stringid set by a newgrf. */ -const char *GetGRFStringPtr(StringIndexInTab stringid) +std::string_view GetGRFStringPtr(StringIndexInTab stringid) { assert(stringid.base() < _grf_text.size()); assert(_grf_text[stringid].grfid != 0); - const char *str = GetGRFStringFromGRFText(_grf_text[stringid].textholder); - if (str != nullptr) return str; + auto str = GetGRFStringFromGRFText(_grf_text[stringid].textholder); + if (str.has_value()) return *str; /* Use the default string ID if the fallback string isn't available */ return GetStringPtr(_grf_text[stringid].def_string); @@ -758,7 +758,7 @@ struct TextRefStack { } }; -static void HandleNewGRFStringControlCodes(const char *str, TextRefStack &stack, std::vector ¶ms); +static void HandleNewGRFStringControlCodes(std::string_view str, TextRefStack &stack, std::vector ¶ms); /** * Process NewGRF string control code instructions. @@ -961,11 +961,9 @@ char32_t RemapNewGRFStringControlCode(char32_t scc, const char **str) * @param[in,out] stack Stack to use. * @param[out] params Parameters to fill. */ -static void HandleNewGRFStringControlCodes(const char *str, TextRefStack &stack, std::vector ¶ms) +static void HandleNewGRFStringControlCodes(std::string_view str, TextRefStack &stack, std::vector ¶ms) { - if (str == nullptr) return; - - for (const char *p = str; *p != '\0'; /* nothing */) { + for (const char *p = str.data(), *end = str.data() + str.size(); p < end; /* nothing */) { char32_t scc; p += Utf8Decode(&scc, p); ProcessNewGRFStringControlCode(scc, p, stack, params); @@ -983,8 +981,7 @@ std::vector GetGRFSringTextStackParameters(const GRFFile *grffi { if (stringid == INVALID_STRING_ID) return {}; - const char *str = GetStringPtr(stringid); - if (str == nullptr) return {}; + auto str = GetStringPtr(stringid); std::vector params; params.reserve(20); diff --git a/src/newgrf_text.h b/src/newgrf_text.h index afbc01cfe9..b9dffd81ad 100644 --- a/src/newgrf_text.h +++ b/src/newgrf_text.h @@ -16,9 +16,9 @@ StringID AddGRFString(uint32_t grfid, GRFStringID stringid, uint8_t langid, bool new_scheme, bool allow_newlines, std::string_view text_to_add, StringID def_string); StringID GetGRFStringID(uint32_t grfid, GRFStringID stringid); -const char *GetGRFStringFromGRFText(const GRFTextList &text_list); -const char *GetGRFStringFromGRFText(const GRFTextWrapper &text); -const char *GetGRFStringPtr(StringIndexInTab stringid); +std::optional GetGRFStringFromGRFText(const GRFTextList &text_list); +std::optional GetGRFStringFromGRFText(const GRFTextWrapper &text); +std::string_view GetGRFStringPtr(StringIndexInTab stringid); void CleanUpStrings(); void SetCurrentGrfLangID(uint8_t language_id); std::string TranslateTTDPatchCodes(uint32_t grfid, uint8_t language_id, bool allow_newlines, std::string_view str, StringControlCode byte80 = SCC_NEWGRF_PRINT_WORD_STRING_ID); diff --git a/src/strings.cpp b/src/strings.cpp index ffc93330d3..005cecab18 100644 --- a/src/strings.cpp +++ b/src/strings.cpp @@ -258,7 +258,7 @@ uint64_t GetParamMaxValue(uint64_t max_value, uint min_count, FontSize size) static void StationGetSpecialString(StringBuilder &builder, StationFacilities x); static bool GetSpecialNameString(StringBuilder &builder, StringID string, StringParameters &args); -static void FormatString(StringBuilder &builder, const char *str, StringParameters &args, uint case_index = 0, bool game_script = false, bool dry_run = false); +static void FormatString(StringBuilder &builder, std::string_view str, StringParameters &args, uint case_index = 0, bool game_script = false, bool dry_run = false); /** * Parse most format codes within a string and write the result to a buffer. @@ -270,7 +270,7 @@ static void FormatString(StringBuilder &builder, const char *str, StringParamete * @param game_script True when doing GameScript text processing. * @param dry_run True when the args' type data is not yet initialized. */ -static void FormatString(StringBuilder &builder, const char *str, std::span params, uint case_index = 0, bool game_script = false, bool dry_run = false) +static void FormatString(StringBuilder &builder, std::string_view str, std::span params, uint case_index = 0, bool game_script = false, bool dry_run = false) { StringParameters tmp_params{params}; FormatString(builder, str, tmp_params, case_index, game_script, dry_run); @@ -291,7 +291,7 @@ struct LanguagePackDeleter { struct LoadedLanguagePack { std::unique_ptr langpack; - std::vector offsets; + std::vector strings; std::array langtab_num; ///< Offset into langpack offs std::array langtab_start; ///< Offset into langpack offs @@ -312,7 +312,7 @@ std::string_view GetListSeparator() return _langpack.list_separator; } -const char *GetStringPtr(StringID string) +std::string_view GetStringPtr(StringID string) { switch (GetStringTab(string)) { case TEXT_TAB_GAMESCRIPT_START: return GetGameStringPtr(GetStringIndex(string)); @@ -321,8 +321,8 @@ const char *GetStringPtr(StringID string) case TEXT_TAB_NEWGRF_START: return GetGRFStringPtr(GetStringIndex(string)); default: { const size_t offset = _langpack.langtab_start[GetStringTab(string)] + GetStringIndex(string).base(); - if (offset < _langpack.offsets.size()) return _langpack.offsets[offset]; - return nullptr; + if (offset < _langpack.strings.size()) return _langpack.strings[offset]; + return "(undefined string)"; } } } @@ -780,16 +780,19 @@ static const char *ParseStringChoice(const char *b, uint form, StringBuilder &bu { /* {Length of each string} {each string} */ uint n = (uint8_t)*b++; - uint pos, i, mypos = 0; - - for (i = pos = 0; i != n; i++) { + size_t form_offset = 0, form_len = 0, total_len = 0; + for (uint i = 0; i != n; i++) { uint len = (uint8_t)*b++; - if (i == form) mypos = pos; - pos += len; + if (i == form) { + form_offset = total_len; + form_len = len; + } + total_len += len; } - builder += b + mypos; - return b + pos; + assert(form_len > 0); // len includes a null terminator + builder += std::string_view(b + form_offset, form_len - 1); + return b + total_len; } /** Helper for unit conversion. */ @@ -1075,7 +1078,7 @@ static const char *DecodeEncodedString(const char *str, bool game_script, String * @param args Pointer to extra arguments used by various string codes. * @param dry_run True when the args' type data is not yet initialized. */ -static void FormatString(StringBuilder &builder, const char *str_arg, StringParameters &args, uint orig_case_index, bool game_script, bool dry_run) +static void FormatString(StringBuilder &builder, std::string_view str_arg, StringParameters &args, uint orig_case_index, bool game_script, bool dry_run) { size_t orig_first_param_offset = args.GetOffset(); @@ -1094,25 +1097,31 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara /* We have to restore the original offset here to to read the correct values. */ args.SetOffset(orig_first_param_offset); } - char32_t b = '\0'; uint next_substr_case_index = 0; struct StrStackItem { const char *str; + const char *end; size_t first_param_offset; uint case_index; + + StrStackItem(std::string_view view, size_t first_param_offset, uint case_index) + : str(view.data()), end(view.data() + view.size()), first_param_offset(first_param_offset), case_index(case_index) + {} }; std::stack> str_stack; str_stack.emplace(str_arg, orig_first_param_offset, orig_case_index); for (;;) { try { - while (!str_stack.empty() && (b = Utf8Consume(&str_stack.top().str)) == '\0') { + while (!str_stack.empty() && str_stack.top().str >= str_stack.top().end) { str_stack.pop(); } if (str_stack.empty()) break; const char *&str = str_stack.top().str; const size_t ref_param_offset = str_stack.top().first_param_offset; const uint case_index = str_stack.top().case_index; + char32_t b = Utf8Consume(&str); + assert(b != 0); if (SCC_NEWGRF_FIRST <= b && b <= SCC_NEWGRF_LAST) { /* We need to pass some stuff as it might be modified. */ @@ -1134,24 +1143,16 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara case SCC_NEWGRF_STRINL: { StringID substr = Utf8Consume(&str); - const char *ptr = GetStringPtr(substr); - if (ptr == nullptr) { - builder += "(invalid NewGRF string)"; - } else { - str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str" - } + std::string_view ptr = GetStringPtr(substr); + str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str" next_substr_case_index = 0; break; } case SCC_NEWGRF_PRINT_WORD_STRING_ID: { StringID substr = args.GetNextParameter(); - const char *ptr = GetStringPtr(substr); - if (ptr == nullptr) { - builder += "(invalid NewGRF string)"; - } else { - str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str" - } + std::string_view ptr = GetStringPtr(substr); + str_stack.emplace(ptr, args.GetOffset(), next_substr_case_index); // this may invalidate "str" next_substr_case_index = 0; break; } @@ -1226,16 +1227,22 @@ static void FormatString(StringBuilder &builder, const char *str_arg, StringPara /* <0x9E> * Each LEN is printed using 2 bytes in big endian order. */ uint num = (uint8_t)*str++; - while (num) { - if ((uint8_t)str[0] == case_index) { - /* Found the case, adjust str pointer and continue */ - str += 3; - break; + std::optional found; + for (; num > 0; --num) { + uint8_t index = static_cast(str[0]); + uint16_t len = (static_cast(str[1]) << 8) + static_cast(str[2]); + assert(len > 0); // len includes a null terminator + str += 3; + if (index == case_index) { + /* Found the case */ + found.emplace(str, len - 1); } - /* Otherwise skip to the next case */ - str += 3 + (static_cast(str[1]) << 8) + static_cast(str[2]); - num--; + str += len; } + const char *end = str_stack.top().end; + if (!found.has_value()) found.emplace(str, end - str); + str = end; + str_stack.emplace(*found, ref_param_offset, case_index); // this may invalidate "str" break; } @@ -1990,12 +1997,12 @@ bool LanguagePackHeader::IsReasonablyFinished() const bool ReadLanguagePack(const LanguageMetadata *lang) { /* Current language pack */ - size_t len = 0; - std::unique_ptr lang_pack(reinterpret_cast(ReadFileToMem(FS2OTTD(lang->file), len, 1U << 20).release())); + size_t total_len = 0; + std::unique_ptr lang_pack(reinterpret_cast(ReadFileToMem(FS2OTTD(lang->file), total_len, 1U << 20).release())); if (!lang_pack) return false; /* End of read data (+ terminating zero added in ReadFileToMem()) */ - const char *end = (char *)lang_pack.get() + len + 1; + const char *end = (char *)lang_pack.get() + total_len + 1; /* We need at least one byte of lang_pack->data */ if (end <= lang_pack->data || !lang_pack->IsValid()) { @@ -2015,26 +2022,25 @@ bool ReadLanguagePack(const LanguageMetadata *lang) } /* Allocate offsets */ - std::vector offs(count); + std::vector strings; /* Fill offsets */ char *s = lang_pack->data; - len = (uint8_t)*s++; for (uint i = 0; i < count; i++) { + size_t len = static_cast(*s++); if (s + len >= end) return false; if (len >= 0xC0) { - len = ((len & 0x3F) << 8) + (uint8_t)*s++; + len = ((len & 0x3F) << 8) + static_cast(*s++); if (s + len >= end) return false; } - offs[i] = s; + strings.emplace_back(s, len); s += len; - len = (uint8_t)*s; - *s++ = '\0'; // zero terminate the string } + assert(strings.size() == count); _langpack.langpack = std::move(lang_pack); - _langpack.offsets = std::move(offs); + _langpack.strings = std::move(strings); _langpack.langtab_num = tab_num; _langpack.langtab_start = tab_start; @@ -2298,7 +2304,7 @@ class LanguagePackGlyphSearcher : public MissingGlyphSearcher { { if (this->i >= TEXT_TAB_END) return std::nullopt; - const char *ret = _langpack.offsets[_langpack.langtab_start[this->i] + this->j]; + std::string_view ret = _langpack.strings[_langpack.langtab_start[this->i] + this->j]; this->j++; while (this->i < TEXT_TAB_END && this->j >= _langpack.langtab_num[this->i]) { diff --git a/src/strings_func.h b/src/strings_func.h index 740e76321c..7f53eeec1e 100644 --- a/src/strings_func.h +++ b/src/strings_func.h @@ -70,7 +70,7 @@ static inline void PrepareArgsForNextRun(std::span args) std::string GetStringWithArgs(StringID string, std::span args); std::string GetString(StringID string); -const char *GetStringPtr(StringID string); +std::string_view GetStringPtr(StringID string); void AppendStringInPlace(std::string &result, StringID string); void AppendStringWithArgsInPlace(std::string &result, StringID string, std::span params);