Codechange: Use template specialisation and instantiation for BaseMedia methods.

Specialisations seem to be the correct way to specialise, rather than redefining the base template.

This removes a macro which instantiated methods individually.

src/base_media_base.h

@@ -58,21 +58,18 @@ struct BaseSet {
 	/** BaseSet type name. */
 	static constexpr std::string_view SET_TYPE = BaseSetTraits<T>::set_type;
 
-	/** Internal names of the files in this set. */
-	static const char * const *file_names;
-
 	std::string name;              ///< The name of the base set
 	std::string url;               ///< URL for information about the base set
 	TranslatedStrings description; ///< Description of the base set
-	uint32_t shortname;              ///< Four letter short variant of the name
+	uint32_t shortname = 0; ///< Four letter short variant of the name
-	uint32_t version;                ///< The version of this base set
+	uint32_t version = 0; ///< The version of this base set
-	bool fallback;                 ///< This set is a fallback set, i.e. it should be used only as last resort
+	bool fallback = false; ///< This set is a fallback set, i.e. it should be used only as last resort
 
-	MD5File files[BaseSet<T>::NUM_FILES]; ///< All files part of this set
+	std::array<MD5File, BaseSet<T>::NUM_FILES> files{}; ///< All files part of this set
-	uint found_files;              ///< Number of the files that could be found
+	uint found_files = 0; ///< Number of the files that could be found
-	uint valid_files;              ///< Number of the files that could be found and are valid
+	uint valid_files = 0; ///< Number of the files that could be found and are valid
 
-	T *next;                       ///< The next base set in this list
+	T *next = nullptr; ///< The next base set in this list
 
 	/** Free everything we allocated */
 	~BaseSet()
@@ -154,6 +151,12 @@ struct BaseSet {
 		}
 		return std::nullopt;
 	}
+
+	/**
+	 * Get the internal names of the files in this set.
+	 * @return the internal filenames
+	 */
+	static std::span<const std::string_view> GetFilenames();
 };
 
 /**
@@ -163,9 +166,9 @@ struct BaseSet {
 template <class Tbase_set>
 class BaseMedia : FileScanner {
 protected:
-	static Tbase_set *available_sets; ///< All available sets
+	static inline Tbase_set *available_sets = nullptr; ///< All available sets
-	static Tbase_set *duplicate_sets; ///< All sets that aren't available, but needed for not downloading base sets when a newer version than the one on BaNaNaS is loaded.
+	static inline Tbase_set *duplicate_sets = nullptr; ///< All sets that aren't available, but needed for not downloading base sets when a newer version than the one on BaNaNaS is loaded.
-	static const Tbase_set *used_set; ///< The currently used set
+	static inline const Tbase_set *used_set = nullptr; ///< The currently used set
 
 	bool AddFile(const std::string &filename, size_t basepath_length, const std::string &tar_filename) override;
 
@@ -211,10 +214,6 @@ public:
 	static bool HasSet(const ContentInfo *ci, bool md5sum);
 };
 
-template <class Tbase_set> /* static */ const Tbase_set *BaseMedia<Tbase_set>::used_set;
-template <class Tbase_set> /* static */ Tbase_set *BaseMedia<Tbase_set>::available_sets;
-template <class Tbase_set> /* static */ Tbase_set *BaseMedia<Tbase_set>::duplicate_sets;
-
 /**
  * Check whether there's a base set matching some information.
  * @param ci The content info to compare it to.

src/base_media_func.h

@@ -79,12 +79,14 @@ bool BaseSet<T>::FillSetDetails(const IniFile &ini, const std::string &path, con
 	const IniGroup *files  = ini.GetGroup("files");
 	const IniGroup *md5s   = ini.GetGroup("md5s");
 	const IniGroup *origin = ini.GetGroup("origin");
+	auto file_names = BaseSet<T>::GetFilenames();
+
 	for (uint i = 0; i < BaseSet<T>::NUM_FILES; i++) {
 		MD5File *file = &this->files[i];
 		/* Find the filename first. */
-		item = files != nullptr ? files->GetItem(BaseSet<T>::file_names[i]) : nullptr;
+		item = files != nullptr ? files->GetItem(file_names[i]) : nullptr;
 		if (item == nullptr || (!item->value.has_value() && !allow_empty_filename)) {
-			Debug(grf, 0, "No {} file for: {} (in {})", BaseSet<T>::SET_TYPE, BaseSet<T>::file_names[i], full_filename);
+			Debug(grf, 0, "No {} file for: {} (in {})", BaseSet<T>::SET_TYPE, file_names[i], full_filename);
 			return false;
 		}
 
@@ -406,25 +408,3 @@ template <class Tbase_set>
 {
 	return BaseMedia<Tbase_set>::available_sets;
 }
-
-/**
- * Force instantiation of methods so we don't get linker errors.
- * @param repl_type the type of the BaseMedia to instantiate
- * @param set_type  the type of the BaseSet to instantiate
- */
-#define INSTANTIATE_BASE_MEDIA_METHODS(repl_type, set_type) \
-	template const char *repl_type::GetExtension(); \
-	template bool repl_type::AddFile(const std::string &filename, size_t pathlength, const std::string &tar_filename); \
-	template bool repl_type::HasSet(const struct ContentInfo *ci, bool md5sum); \
-	template bool repl_type::SetSet(const set_type *set); \
-	template bool repl_type::SetSetByName(const std::string &name); \
-	template bool repl_type::SetSetByShortname(uint32_t shortname); \
-	template void repl_type::GetSetsList(std::back_insert_iterator<std::string> &output_iterator); \
-	template int repl_type::GetNumSets(); \
-	template int repl_type::GetIndexOfUsedSet(); \
-	template const set_type *repl_type::GetSet(int index); \
-	template const set_type *repl_type::GetUsedSet(); \
-	template bool repl_type::DetermineBestSet(); \
-	template set_type *repl_type::GetAvailableSets(); \
-	template const char *TryGetBaseSetFile(const ContentInfo *ci, bool md5sum, const set_type *s);
-

src/base_media_graphics.h

@@ -40,7 +40,7 @@ template <> struct BaseSetTraits<struct GraphicsSet> {
 /** All data of a graphics set. */
 struct GraphicsSet : BaseSet<GraphicsSet> {
 private:
-	mutable std::unique_ptr<GRFConfig> extra_cfg = nullptr; ///< Parameters for extra GRF
+	mutable std::unique_ptr<GRFConfig> extra_cfg; ///< Parameters for extra GRF
 public:
 	PaletteType palette{}; ///< Palette of this graphics set
 	BlitterType blitter{}; ///< Blitter of this graphics set

src/bootstrap_gui.cpp

@@ -407,7 +407,7 @@ bool HandleBootstrap()
 	if (_exit_game) return false;
 
 	/* Try to probe the graphics. Should work this time. */
-	if (!BaseGraphics::SetSet({})) goto failure;
+	if (!BaseGraphics::SetSet(nullptr)) goto failure;
 
 	/* Finally we can continue heading for the menu. */
 	_game_mode = GM_MENU;

src/gfxinit.cpp

@@ -137,7 +137,7 @@ void CheckExternalFiles()
 		static_assert(SoundsSet::NUM_FILES == 1);
 		/* No need to loop each file, as long as there is only a single
 		 * sound file. */
-		fmt::format_to(output_iterator, "\t{} is {} ({})\n", sounds_set->files->filename, SoundsSet::CheckMD5(sounds_set->files, BASESET_DIR) == MD5File::CR_MISMATCH ? "corrupt" : "missing", sounds_set->files->missing_warning);
+		fmt::format_to(output_iterator, "\t{} is {} ({})\n", sounds_set->files[0].filename, SoundsSet::CheckMD5(&sounds_set->files[0], BASESET_DIR) == MD5File::CR_MISMATCH ? "corrupt" : "missing", sounds_set->files[0].missing_warning);
 	}
 
 	if (!error_msg.empty()) ShowInfoI(error_msg);
@@ -346,15 +346,11 @@ void GfxLoadSprites()
 	UpdateCursorSize();
 }
 
-GraphicsSet::GraphicsSet()
+// instantiate here, because unique_ptr needs a complete type
-{
+GraphicsSet::GraphicsSet() = default;
-	// instantiate here, because unique_ptr needs a complete type
-}
 
-GraphicsSet::~GraphicsSet()
+// instantiate here, because unique_ptr needs a complete type
-{
+GraphicsSet::~GraphicsSet() = default;
-	// instantiate here, because unique_ptr needs a complete type
-}
 
 bool GraphicsSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename)
 {
@@ -467,18 +463,21 @@ MD5File::ChecksumResult MD5File::CheckMD5(Subdirectory subdir, size_t max_size)
 }
 
 /** Names corresponding to the GraphicsFileType */
-static const char * const _graphics_file_names[] = { "base", "logos", "arctic", "tropical", "toyland", "extra" };
+static const std::string_view _graphics_file_names[] = { "base", "logos", "arctic", "tropical", "toyland", "extra" };
 
 /** Implementation */
-template <class T>
+template <>
-/* static */ const char * const *BaseSet<T>::file_names = _graphics_file_names;
+/* static */ std::span<const std::string_view> BaseSet<GraphicsSet>::GetFilenames()
-
-template <class Tbase_set>
-/* static */ bool BaseMedia<Tbase_set>::DetermineBestSet()
 {
-	if (BaseMedia<Tbase_set>::used_set != nullptr) return true;
+	return _graphics_file_names;
+}
 
-	const Tbase_set *best = nullptr;
+template <>
+/* static */ bool BaseMedia<GraphicsSet>::DetermineBestSet()
+{
+	if (BaseMedia<GraphicsSet>::used_set != nullptr) return true;
+
+	const GraphicsSet *best = nullptr;
 
 	auto IsBetter = [&best] (const auto *current) {
 		/* Nothing chosen yet. */
@@ -495,21 +494,21 @@ template <class Tbase_set>
 		return best->palette != PAL_DOS && current->palette == PAL_DOS;
 	};
 
-	for (const Tbase_set *c = BaseMedia<Tbase_set>::available_sets; c != nullptr; c = c->next) {
+	for (const GraphicsSet *c = BaseMedia<GraphicsSet>::available_sets; c != nullptr; c = c->next) {
 		/* Skip unusable sets */
 		if (c->GetNumMissing() != 0) continue;
 
 		if (IsBetter(c)) best = c;
 	}
 
-	BaseMedia<Tbase_set>::used_set = best;
+	BaseMedia<GraphicsSet>::used_set = best;
-	return BaseMedia<Tbase_set>::used_set != nullptr;
+	return BaseMedia<GraphicsSet>::used_set != nullptr;
 }
 
-template <class Tbase_set>
+template <>
-/* static */ const char *BaseMedia<Tbase_set>::GetExtension()
+/* static */ const char *BaseMedia<GraphicsSet>::GetExtension()
 {
 	return ".obg"; // OpenTTD Base Graphics
 }
 
-INSTANTIATE_BASE_MEDIA_METHODS(BaseMedia<GraphicsSet>, GraphicsSet)
+template class BaseMedia<GraphicsSet>;

src/music.cpp

@@ -66,10 +66,8 @@ std::optional<std::vector<uint8_t>> GetMusicCatEntryData(const std::string &file
 	return data;
 }
 
-INSTANTIATE_BASE_MEDIA_METHODS(BaseMedia<MusicSet>, MusicSet)
-
 /** Names corresponding to the music set's files */
-static const char * const _music_file_names[] = {
+static const std::string_view _music_file_names[] = {
 	"theme",
 	"old_0", "old_1", "old_2", "old_3", "old_4", "old_5", "old_6", "old_7", "old_8", "old_9",
 	"new_0", "new_1", "new_2", "new_3", "new_4", "new_5", "new_6", "new_7", "new_8", "new_9",
@@ -78,22 +76,25 @@ 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 <class T>
+template <>
-/* static */ const char * const *BaseSet<T>::file_names = _music_file_names;
+/* static */ std::span<const std::string_view> BaseSet<MusicSet>::GetFilenames()
+{
+	return _music_file_names;
+}
 
-template <class Tbase_set>
+template <>
-/* static */ const char *BaseMedia<Tbase_set>::GetExtension()
+/* static */ const char *BaseMedia<MusicSet>::GetExtension()
 {
 	return ".obm"; // OpenTTD Base Music
 }
 
-template <class Tbase_set>
+template <>
-/* static */ bool BaseMedia<Tbase_set>::DetermineBestSet()
+/* static */ bool BaseMedia<MusicSet>::DetermineBestSet()
 {
-	if (BaseMedia<Tbase_set>::used_set != nullptr) return true;
+	if (BaseMedia<MusicSet>::used_set != nullptr) return true;
 
-	const Tbase_set *best = nullptr;
+	const MusicSet *best = nullptr;
-	for (const Tbase_set *c = BaseMedia<Tbase_set>::available_sets; c != nullptr; c = c->next) {
+	for (const MusicSet *c = BaseMedia<MusicSet>::available_sets; c != nullptr; c = c->next) {
 		if (c->GetNumMissing() != 0) continue;
 
 		if (best == nullptr ||
@@ -105,10 +106,12 @@ template <class Tbase_set>
 		}
 	}
 
-	BaseMedia<Tbase_set>::used_set = best;
+	BaseMedia<MusicSet>::used_set = best;
-	return BaseMedia<Tbase_set>::used_set != nullptr;
+	return BaseMedia<MusicSet>::used_set != nullptr;
 }
 
+template class BaseMedia<MusicSet>;
+
 bool MusicSet::FillSetDetails(const IniFile &ini, const std::string &path, const std::string &full_filename)
 {
 	bool ret = this->BaseSet<MusicSet>::FillSetDetails(ini, path, full_filename);

src/sound.cpp

@@ -90,7 +90,7 @@ static bool SetBankSource(MixerChannel *mc, SoundEntry *sound, SoundID sound_id)
 void InitializeSound()
 {
 	Debug(misc, 1, "Loading sound effects...");
-	OpenBankFile(BaseSounds::GetUsedSet()->files->filename);
+	OpenBankFile(BaseSounds::GetUsedSet()->files[0].filename);
 }
 
 
@@ -247,28 +247,28 @@ void SndPlayFx(SoundID sound)
 	StartSound(sound, 0.5, UINT8_MAX);
 }
 
-INSTANTIATE_BASE_MEDIA_METHODS(BaseMedia<SoundsSet>, SoundsSet)
-
 /** Names corresponding to the sound set's files */
-static const char * const _sound_file_names[] = { "samples" };
+static const std::string_view _sound_file_names[] = { "samples" };
 
+template <>
+/* static */ std::span<const std::string_view> BaseSet<SoundsSet>::GetFilenames()
+{
+	return _sound_file_names;
+}
 
-template <class T>
+template <>
-/* static */ const char * const *BaseSet<T>::file_names = _sound_file_names;
+/* static */ const char *BaseMedia<SoundsSet>::GetExtension()
-
-template <class Tbase_set>
-/* static */ const char *BaseMedia<Tbase_set>::GetExtension()
 {
 	return ".obs"; // OpenTTD Base Sounds
 }
 
-template <class Tbase_set>
+template <>
-/* static */ bool BaseMedia<Tbase_set>::DetermineBestSet()
+/* static */ bool BaseMedia<SoundsSet>::DetermineBestSet()
 {
-	if (BaseMedia<Tbase_set>::used_set != nullptr) return true;
+	if (BaseMedia<SoundsSet>::used_set != nullptr) return true;
 
-	const Tbase_set *best = nullptr;
+	const SoundsSet *best = nullptr;
-	for (const Tbase_set *c = BaseMedia<Tbase_set>::available_sets; c != nullptr; c = c->next) {
+	for (const SoundsSet *c = BaseMedia<SoundsSet>::available_sets; c != nullptr; c = c->next) {
 		/* Skip unusable sets */
 		if (c->GetNumMissing() != 0) continue;
 
@@ -281,7 +281,8 @@ template <class Tbase_set>
 		}
 	}
 
-	BaseMedia<Tbase_set>::used_set = best;
+	BaseMedia<SoundsSet>::used_set = best;
-	return BaseMedia<Tbase_set>::used_set != nullptr;
+	return BaseMedia<SoundsSet>::used_set != nullptr;
 }
 
+template class BaseMedia<SoundsSet>;