From 267f3c7905972ad78713cd92f96e3073ea1c8996 Mon Sep 17 00:00:00 2001 From: lat9nq <22451773+lat9nq@users.noreply.github.com> Date: Tue, 18 Jul 2023 15:36:20 -0400 Subject: [PATCH] settings: Cleanup Addresses review feedback Co-authored-by: Morph <39850852+Morph1984@users.noreply.github.com> --- src/common/CMakeLists.txt | 1 + src/common/settings.cpp | 5 ++- src/common/settings_setting.h | 77 +++++++++++++++++++++-------------- 3 files changed, 51 insertions(+), 32 deletions(-) diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt index e205055e62..bf97d9ba28 100644 --- a/src/common/CMakeLists.txt +++ b/src/common/CMakeLists.txt @@ -205,6 +205,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") -Werror=unreachable-code-aggressive ) target_compile_definitions(common PRIVATE + # Clang 14 and earlier have errors when explicitly instantiating Settings::Setting $<$,15>:CANNOT_EXPLICITLY_INSTANTIATE> ) endif() diff --git a/src/common/settings.cpp b/src/common/settings.cpp index 78fa991134..59d24a0534 100644 --- a/src/common/settings.cpp +++ b/src/common/settings.cpp @@ -25,6 +25,7 @@ namespace Settings { +// Clang 14 and earlier have errors when explicitly instantiating these classes #ifndef CANNOT_EXPLICITLY_INSTANTIATE #define SETTING(TYPE, RANGED) template class Setting #define SWITCHABLE(TYPE, RANGED) template class SwitchableSetting @@ -113,11 +114,11 @@ void LogSettings() { for (const auto& setting : settings) { if (setting->Id() == values.yuzu_token.Id()) { - // Hide the token secret, which could be used to share patreon builds + // Hide the token secret, for security reasons. continue; } - std::string name = fmt::format( + const auto name = fmt::format( "{:c}{:c} {}.{}", setting->ToString() == setting->DefaultToString() ? '-' : 'M', setting->UsingGlobal() ? '-' : 'C', TranslateCategory(category), setting->GetLabel()); diff --git a/src/common/settings_setting.h b/src/common/settings_setting.h index 2e708fa0d1..959b4f3f9e 100644 --- a/src/common/settings_setting.h +++ b/src/common/settings_setting.h @@ -34,6 +34,10 @@ public: * @param default_val Initial value of the setting, and default value of the setting * @param name Label for the setting * @param category_ Category of the setting AKA INI group + * @param specialization_ Suggestion for how frontend implemetations represent this in a config + * @param save_ Suggests that this should or should not be saved to a frontend config file + * @param runtime_modifiable_ Suggests whether this is modifiable while a guest is loaded + * @param other_setting_ A second Setting to associate to this one in metadata */ explicit Setting(Linkage& linkage, const Type& default_val, const std::string& name, enum Category category_, u32 specialization_ = Specialization::Default, @@ -54,6 +58,10 @@ public: * @param max_val Sets the maximum allowed value of the setting * @param name Label for the setting * @param category_ Category of the setting AKA INI group + * @param specialization_ Suggestion for how frontend implemetations represent this in a config + * @param save_ Suggests that this should or should not be saved to a frontend config file + * @param runtime_modifiable_ Suggests whether this is modifiable while a guest is loaded + * @param other_setting_ A second Setting to associate to this one in metadata */ explicit Setting(Linkage& linkage, const Type& default_val, const Type& min_val, const Type& max_val, const std::string& name, enum Category category_, @@ -93,18 +101,19 @@ public: } [[nodiscard]] constexpr bool IsEnum() const override { - return std::is_enum::value; + return std::is_enum_v; } protected: - std::string ToString(const Type& value_) const { - if constexpr (std::is_same()) { + [[nodiscard]] std::string ToString(const Type& value_) const { + if constexpr (std::is_same_v) { return value_; - } else if constexpr (std::is_same>()) { + } else if constexpr (std::is_same_v>) { return value_.has_value() ? std::to_string(*value_) : "none"; - } else if constexpr (std::is_same()) { + } else if constexpr (std::is_same_v) { return value_ ? "true" : "false"; - } else if constexpr (std::is_same()) { + } else if constexpr (std::is_same_v) { + // Compatibility with old AudioEngine setting being a string return CanonicalizeEnum(value_); } else { return std::to_string(static_cast(value_)); @@ -118,7 +127,7 @@ public: * * @returns The current setting as a std::string */ - std::string ToString() const override { + [[nodiscard]] std::string ToString() const override { return ToString(this->GetValue()); } @@ -127,7 +136,7 @@ public: * * @returns The default value as a string. */ - std::string DefaultToString() const override { + [[nodiscard]] std::string DefaultToString() const override { return ToString(default_value); } @@ -159,19 +168,19 @@ public: * * @param input The desired value */ - void LoadString(const std::string& input) override { + void LoadString(const std::string& input) override final { if (input.empty()) { this->SetValue(this->GetDefault()); return; } try { - if constexpr (std::is_same()) { + if constexpr (std::is_same_v) { this->SetValue(input); - } else if constexpr (std::is_same>()) { + } else if constexpr (std::is_same_v>) { this->SetValue(static_cast(std::stoul(input))); - } else if constexpr (std::is_same()) { + } else if constexpr (std::is_same_v) { this->SetValue(input == "true"); - } else if constexpr (std::is_same()) { + } else if constexpr (std::is_same_v) { this->SetValue(ToEnum(input)); } else { this->SetValue(static_cast(std::stoll(input))); @@ -181,8 +190,8 @@ public: } } - [[nodiscard]] std::string constexpr Canonicalize() const override { - if constexpr (std::is_enum::value) { + [[nodiscard]] std::string constexpr Canonicalize() const override final { + if constexpr (std::is_enum_v) { return CanonicalizeEnum(this->GetValue()); } else { return ToString(this->GetValue()); @@ -194,26 +203,26 @@ public: * * @returns the type_index of the setting's type */ - virtual std::type_index TypeId() const override { + [[nodiscard]] std::type_index TypeId() const override final { return std::type_index(typeid(Type)); } - constexpr u32 EnumIndex() const override { - if constexpr (std::is_enum()) { + [[nodiscard]] constexpr u32 EnumIndex() const override final { + if constexpr (std::is_enum_v) { return EnumMetadata::Index(); } else { return std::numeric_limits::max(); } } - virtual std::string MinVal() const override { + [[nodiscard]] std::string MinVal() const override final { return this->ToString(minimum); } - virtual std::string MaxVal() const override { + [[nodiscard]] std::string MaxVal() const override final { return this->ToString(maximum); } - constexpr bool Ranged() const override { + [[nodiscard]] constexpr bool Ranged() const override { return ranged; } @@ -242,6 +251,10 @@ public: * @param default_val Initial value of the setting, and default value of the setting * @param name Label for the setting * @param category_ Category of the setting AKA INI group + * @param specialization_ Suggestion for how frontend implemetations represent this in a config + * @param save_ Suggests that this should or should not be saved to a frontend config file + * @param runtime_modifiable_ Suggests whether this is modifiable while a guest is loaded + * @param other_setting_ A second Setting to associate to this one in metadata */ explicit SwitchableSetting(Linkage& linkage, const Type& default_val, const std::string& name, Category category_, u32 specialization_ = Specialization::Default, @@ -264,6 +277,10 @@ public: * @param max_val Sets the maximum allowed value of the setting * @param name Label for the setting * @param category_ Category of the setting AKA INI group + * @param specialization_ Suggestion for how frontend implemetations represent this in a config + * @param save_ Suggests that this should or should not be saved to a frontend config file + * @param runtime_modifiable_ Suggests whether this is modifiable while a guest is loaded + * @param other_setting_ A second Setting to associate to this one in metadata */ explicit SwitchableSetting(Linkage& linkage, const Type& default_val, const Type& min_val, const Type& max_val, const std::string& name, Category category_, @@ -284,7 +301,7 @@ public: * * @param to_global Whether to use the global or custom setting. */ - void SetGlobal(bool to_global) override { + void SetGlobal(bool to_global) override final { use_global = to_global; } @@ -293,7 +310,7 @@ public: * * @returns The global state */ - [[nodiscard]] bool UsingGlobal() const override { + [[nodiscard]] bool UsingGlobal() const override final { return use_global; } @@ -305,13 +322,13 @@ public: * * @returns The required value of the setting */ - [[nodiscard]] virtual const Type& GetValue() const override { + [[nodiscard]] const Type& GetValue() const override final { if (use_global) { return this->value; } return custom; } - [[nodiscard]] virtual const Type& GetValue(bool need_global) const { + [[nodiscard]] const Type& GetValue(bool need_global) const { if (use_global || need_global) { return this->value; } @@ -323,7 +340,7 @@ public: * * @param val The new value */ - void SetValue(const Type& val) override { + void SetValue(const Type& val) override final { Type temp{ranged ? std::clamp(val, this->minimum, this->maximum) : val}; if (use_global) { std::swap(this->value, temp); @@ -332,11 +349,11 @@ public: } } - [[nodiscard]] virtual constexpr bool Switchable() const override { + [[nodiscard]] constexpr bool Switchable() const override final { return true; } - [[nodiscard]] virtual std::string ToStringGlobal() const override { + [[nodiscard]] std::string ToStringGlobal() const override final { return this->ToString(this->value); } @@ -347,7 +364,7 @@ public: * * @returns A reference to the current setting value */ - const Type& operator=(const Type& val) override { + const Type& operator=(const Type& val) override final { Type temp{ranged ? std::clamp(val, this->minimum, this->maximum) : val}; if (use_global) { std::swap(this->value, temp); @@ -362,7 +379,7 @@ public: * * @returns A reference to the current setting value */ - virtual explicit operator const Type&() const override { + explicit operator const Type&() const override final { if (use_global) { return this->value; }