From 3375dc3095d89e2a17d84a089b89668b9973f0c9 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Wed, 26 Mar 2025 18:11:55 +0000 Subject: [PATCH] Codechange: Use traits to define BaseSet parameters. --- src/base_media_base.h | 20 ++++++++++++-------- src/base_media_func.h | 29 +++++++++++++++-------------- src/base_media_graphics.h | 14 ++++++++++---- src/base_media_music.h | 10 ++++++++-- src/base_media_sounds.h | 10 +++++++--- src/gfxinit.cpp | 10 +++------- src/music.cpp | 10 +++------- src/sound.cpp | 7 ++----- 8 files changed, 60 insertions(+), 50 deletions(-) diff --git a/src/base_media_base.h b/src/base_media_base.h index e347006624..4378c4673b 100644 --- a/src/base_media_base.h +++ b/src/base_media_base.h @@ -38,21 +38,25 @@ struct MD5File { ChecksumResult CheckMD5(Subdirectory subdir, size_t max_size) const; }; +/** Defines the traits of a BaseSet type. */ +template struct BaseSetTraits; + /** * Information about a single base set. * @tparam T the real class we're going to be - * @tparam Tnum_files the number of files in the set - * @tparam Tsearch_in_tars whether to search in the tars or not */ -template +template struct BaseSet { typedef std::unordered_map TranslatedStrings; /** Number of files in this set */ - static const size_t NUM_FILES = Tnum_files; + static constexpr size_t NUM_FILES = BaseSetTraits::num_files; /** Whether to search in the tars or not. */ - static const bool SEARCH_IN_TARS = Tsearch_in_tars; + static constexpr bool SEARCH_IN_TARS = BaseSetTraits::search_in_tars; + + /** BaseSet type name. */ + static constexpr std::string_view SET_TYPE = BaseSetTraits::set_type; /** Internal names of the files in this set. */ static const char * const *file_names; @@ -64,7 +68,7 @@ struct BaseSet { uint32_t version; ///< The version of this base set bool fallback; ///< This set is a fallback set, i.e. it should be used only as last resort - MD5File files[NUM_FILES]; ///< All files part of this set + MD5File files[BaseSet::NUM_FILES]; ///< All files part of this set uint found_files; ///< Number of the files that could be found uint valid_files; ///< Number of the files that could be found and are valid @@ -82,7 +86,7 @@ struct BaseSet { */ int GetNumMissing() const { - return Tnum_files - this->found_files; + return BaseSet::NUM_FILES - this->found_files; } /** @@ -92,7 +96,7 @@ struct BaseSet { */ int GetNumInvalid() const { - return Tnum_files - this->valid_files; + return BaseSet::NUM_FILES - this->valid_files; } bool FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename, bool allow_empty_filename = true); diff --git a/src/base_media_func.h b/src/base_media_func.h index 2d68caaa6b..204ccfa818 100644 --- a/src/base_media_func.h +++ b/src/base_media_func.h @@ -7,7 +7,6 @@ /** * @file base_media_func.h Generic function implementations for base data (graphics, sounds). - * @note You should _never_ include this file due to the SET_TYPE define. */ #include "base_media_base.h" @@ -25,7 +24,7 @@ extern void CheckExternalFiles(); #define fetch_metadata(name) \ item = metadata->GetItem(name); \ if (item == nullptr || !item->value.has_value() || item->value->empty()) { \ - Debug(grf, 0, "Base " SET_TYPE "set detail loading: {} field missing.", name); \ + Debug(grf, 0, "Base {}set detail loading: {} field missing.", BaseSet::SET_TYPE, name); \ Debug(grf, 0, " Is {} readable for the user running OpenTTD?", full_filename); \ return false; \ } @@ -38,12 +37,12 @@ extern void CheckExternalFiles(); * @param allow_empty_filename empty filenames are valid * @return true if loading was successful. */ -template -bool BaseSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename, bool allow_empty_filename) +template +bool BaseSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename, bool allow_empty_filename) { const IniGroup *metadata = ini.GetGroup("metadata"); if (metadata == nullptr) { - Debug(grf, 0, "Base " SET_TYPE "set detail loading: metadata missing."); + Debug(grf, 0, "Base {}set detail loading: metadata missing.", BaseSet::SET_TYPE); Debug(grf, 0, " Is {} readable for the user running OpenTTD?", full_filename); return false; } @@ -80,12 +79,12 @@ bool BaseSet::FillSetDetails(const IniFile &ini, const IniGroup *files = ini.GetGroup("files"); const IniGroup *md5s = ini.GetGroup("md5s"); const IniGroup *origin = ini.GetGroup("origin"); - for (uint i = 0; i < Tnum_files; i++) { + for (uint i = 0; i < BaseSet::NUM_FILES; i++) { MD5File *file = &this->files[i]; /* Find the filename first. */ - item = files != nullptr ? files->GetItem(BaseSet::file_names[i]) : nullptr; + item = files != nullptr ? files->GetItem(BaseSet::file_names[i]) : nullptr; if (item == nullptr || (!item->value.has_value() && !allow_empty_filename)) { - Debug(grf, 0, "No " SET_TYPE " file for: {} (in {})", BaseSet::file_names[i], full_filename); + Debug(grf, 0, "No {} file for: {} (in {})", BaseSet::SET_TYPE, BaseSet::file_names[i], full_filename); return false; } @@ -164,7 +163,7 @@ template bool BaseMedia::AddFile(const std::string &filename, size_t basepath_length, const std::string &) { bool ret = false; - Debug(grf, 1, "Checking {} for base " SET_TYPE " set", filename); + Debug(grf, 1, "Checking {} for base {} set", filename, BaseSet::SET_TYPE); Tbase_set *set = new Tbase_set(); IniFile ini{}; @@ -190,7 +189,8 @@ bool BaseMedia::AddFile(const std::string &filename, size_t basepath_ /* The more complete set takes precedence over the version number. */ if ((duplicate->valid_files == set->valid_files && duplicate->version >= set->version) || duplicate->valid_files > set->valid_files) { - Debug(grf, 1, "Not adding {} ({}) as base " SET_TYPE " set (duplicate, {})", set->name, set->version, + Debug(grf, 1, "Not adding {} ({}) as base {} set (duplicate, {})", set->name, set->version, + BaseSet::SET_TYPE, duplicate->valid_files > set->valid_files ? "less valid files" : "lower version"); set->next = BaseMedia::duplicate_sets; BaseMedia::duplicate_sets = set; @@ -209,7 +209,8 @@ bool BaseMedia::AddFile(const std::string &filename, size_t basepath_ * version number until a new game is started which isn't a big problem */ if (BaseMedia::used_set == duplicate) BaseMedia::used_set = set; - Debug(grf, 1, "Removing {} ({}) as base " SET_TYPE " set (duplicate, {})", duplicate->name, duplicate->version, + Debug(grf, 1, "Removing {} ({}) as base {} set (duplicate, {})", duplicate->name, duplicate->version, + BaseSet::SET_TYPE, duplicate->valid_files < set->valid_files ? "less valid files" : "lower version"); duplicate->next = BaseMedia::duplicate_sets; BaseMedia::duplicate_sets = duplicate; @@ -223,7 +224,7 @@ bool BaseMedia::AddFile(const std::string &filename, size_t basepath_ ret = true; } if (ret) { - Debug(grf, 1, "Adding {} ({}) as base " SET_TYPE " set", set->name, set->version); + Debug(grf, 1, "Adding {} ({}) as base {} set", set->name, set->version, BaseSet::SET_TYPE); } } else { delete set; @@ -296,7 +297,7 @@ template template /* static */ void BaseMedia::GetSetsList(std::back_insert_iterator &output_iterator) { - fmt::format_to(output_iterator, "List of " SET_TYPE " sets:\n"); + fmt::format_to(output_iterator, "List of {} sets:\n", BaseSet::SET_TYPE); for (const Tbase_set *s = BaseMedia::available_sets; s != nullptr; s = s->next) { fmt::format_to(output_iterator, "{:>18}: {}", s->name, s->GetDescription({})); int invalid = s->GetNumInvalid(); @@ -383,7 +384,7 @@ template if (index == 0) return s; index--; } - FatalError("Base" SET_TYPE "::GetSet(): index {} out of range", index); + FatalError("Base{}::GetSet(): index {} out of range", BaseSet::SET_TYPE, index); } /** diff --git a/src/base_media_graphics.h b/src/base_media_graphics.h index ada00a72dc..72f2ca36e5 100644 --- a/src/base_media_graphics.h +++ b/src/base_media_graphics.h @@ -31,13 +31,19 @@ enum BlitterType : uint8_t { struct GRFConfig; +template <> struct BaseSetTraits { + static constexpr size_t num_files = MAX_GFT; + static constexpr bool search_in_tars = true; + static constexpr std::string_view set_type = "graphics"; +}; + /** All data of a graphics set. */ -struct GraphicsSet : BaseSet { +struct GraphicsSet : BaseSet { private: - mutable std::unique_ptr extra_cfg; ///< Parameters for extra GRF + mutable std::unique_ptr extra_cfg = nullptr; ///< Parameters for extra GRF public: - PaletteType palette; ///< Palette of this graphics set - BlitterType blitter; ///< Blitter of this graphics set + PaletteType palette{}; ///< Palette of this graphics set + BlitterType blitter{}; ///< Blitter of this graphics set GraphicsSet(); ~GraphicsSet(); diff --git a/src/base_media_music.h b/src/base_media_music.h index 3d6acf5a19..86b3e8cad7 100644 --- a/src/base_media_music.h +++ b/src/base_media_music.h @@ -43,12 +43,18 @@ struct MusicSongInfo { int override_end; ///< MIDI tick to end the song at (0 if no override) }; +template <> struct BaseSetTraits { + static constexpr size_t num_files = NUM_SONGS_AVAILABLE; + static constexpr bool search_in_tars = false; + static constexpr std::string_view set_type = "music"; +}; + /** All data of a music set. */ -struct MusicSet : BaseSet { +struct MusicSet : BaseSet { /** Data about individual songs in set. */ MusicSongInfo songinfo[NUM_SONGS_AVAILABLE]; /** Number of valid songs in set. */ - uint8_t num_available; + uint8_t num_available = 0; bool FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename); }; diff --git a/src/base_media_sounds.h b/src/base_media_sounds.h index b089fdfa2d..d7a5cd0ffa 100644 --- a/src/base_media_sounds.h +++ b/src/base_media_sounds.h @@ -12,16 +12,20 @@ #include "base_media_base.h" -/** All data of a sounds set. */ -struct SoundsSet : BaseSet { +template <> struct BaseSetTraits { + static constexpr size_t num_files = 1; + static constexpr bool search_in_tars = true; + static constexpr std::string_view set_type = "sounds"; }; +/** All data of a sounds set. */ +struct SoundsSet : BaseSet {}; + /** All data/functions related with replacing the base sounds */ class BaseSounds : public BaseMedia { public: /** The set as saved in the config file. */ static inline std::string ini_set; - }; #endif /* BASE_MEDIA_SOUNDS_H */ diff --git a/src/gfxinit.cpp b/src/gfxinit.cpp index c3b014551d..7e4ebd196b 100644 --- a/src/gfxinit.cpp +++ b/src/gfxinit.cpp @@ -18,9 +18,6 @@ #include "video/video_driver.hpp" #include "window_func.h" #include "palette_func.h" - -/* The type of set we're replacing */ -#define SET_TYPE "graphics" #include "base_media_func.h" #include "base_media_graphics.h" #include "base_media_sounds.h" @@ -350,7 +347,6 @@ void GfxLoadSprites() } GraphicsSet::GraphicsSet() - : BaseSet{}, palette{}, blitter{} { // instantiate here, because unique_ptr needs a complete type } @@ -362,7 +358,7 @@ GraphicsSet::~GraphicsSet() bool GraphicsSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename) { - bool ret = this->BaseSet::FillSetDetails(ini, path, full_filename, false); + bool ret = this->BaseSet::FillSetDetails(ini, path, full_filename, false); if (ret) { const IniGroup *metadata = ini.GetGroup("metadata"); assert(metadata != nullptr); /* ret can't be true if metadata isn't present. */ @@ -474,8 +470,8 @@ MD5File::ChecksumResult MD5File::CheckMD5(Subdirectory subdir, size_t max_size) static const char * const _graphics_file_names[] = { "base", "logos", "arctic", "tropical", "toyland", "extra" }; /** Implementation */ -template -/* static */ const char * const *BaseSet::file_names = _graphics_file_names; +template +/* static */ const char * const *BaseSet::file_names = _graphics_file_names; template /* static */ bool BaseMedia::DetermineBestSet() diff --git a/src/music.cpp b/src/music.cpp index e10373949b..f713fdc991 100644 --- a/src/music.cpp +++ b/src/music.cpp @@ -9,10 +9,6 @@ #include "stdafx.h" #include "string_func.h" - - -/** The type of set we're replacing */ -#define SET_TYPE "music" #include "base_media_func.h" #include "base_media_music.h" #include "random_access_file_type.h" @@ -82,8 +78,8 @@ static const char * const _music_file_names[] = { /** Make sure we aren't messing things up. */ static_assert(lengthof(_music_file_names) == NUM_SONGS_AVAILABLE); -template -/* static */ const char * const *BaseSet::file_names = _music_file_names; +template +/* static */ const char * const *BaseSet::file_names = _music_file_names; template /* static */ const char *BaseMedia::GetExtension() @@ -115,7 +111,7 @@ template bool MusicSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename) { - bool ret = this->BaseSet::FillSetDetails(ini, path, full_filename); + bool ret = this->BaseSet::FillSetDetails(ini, path, full_filename); if (ret) { this->num_available = 0; const IniGroup *names = ini.GetGroup("names"); diff --git a/src/sound.cpp b/src/sound.cpp index 965fea6102..94e67bc720 100644 --- a/src/sound.cpp +++ b/src/sound.cpp @@ -17,9 +17,6 @@ #include "window_func.h" #include "window_gui.h" #include "vehicle_base.h" - -/* The type of set we're replacing */ -#define SET_TYPE "sounds" #include "base_media_func.h" #include "base_media_sounds.h" @@ -256,8 +253,8 @@ INSTANTIATE_BASE_MEDIA_METHODS(BaseMedia, SoundsSet) static const char * const _sound_file_names[] = { "samples" }; -template -/* static */ const char * const *BaseSet::file_names = _sound_file_names; +template +/* static */ const char * const *BaseSet::file_names = _sound_file_names; template /* static */ const char *BaseMedia::GetExtension()