From 54f70eabe86b2656f54037fddf311b4107c104fe Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 27 Jun 2024 12:15:41 +0200 Subject: [PATCH] Refactor AppSettingValue into Variant (#789) --- CHANGELOG.md | 1 + .../Graphics/CustomRenderingOptions.cpp | 12 ++- .../Graphics/CustomRenderingOptions.h | 6 +- .../Graphics/ModelRendererParams.cpp | 21 ++-- .../Graphics/OpenSimDecorationOptions.cpp | 39 ++++--- .../Graphics/OpenSimDecorationOptions.h | 5 +- .../Graphics/OverlayDecorationOptions.cpp | 17 ++- .../Graphics/OverlayDecorationOptions.h | 6 +- .../Platform/OpenSimCreatorApp.cpp | 32 +++--- src/OpenSimCreator/UI/MainUIScreen.cpp | 6 +- src/oscar/CMakeLists.txt | 3 - src/oscar/Platform.h | 2 - src/oscar/Platform/App.cpp | 4 +- src/oscar/Platform/AppSettingValue.cpp | 71 ------------ src/oscar/Platform/AppSettingValue.h | 45 -------- src/oscar/Platform/AppSettingValueType.h | 11 -- src/oscar/Platform/AppSettings.cpp | 67 +++++++----- src/oscar/Platform/AppSettings.h | 8 +- src/oscar/UI/Panels/StandardPanelImpl.cpp | 8 +- src/oscar/UI/ui_context.cpp | 2 +- tests/testoscar/CMakeLists.txt | 1 - .../Platform/TestAppSettingValueType.cpp | 101 ------------------ 22 files changed, 128 insertions(+), 340 deletions(-) delete mode 100644 src/oscar/Platform/AppSettingValue.cpp delete mode 100644 src/oscar/Platform/AppSettingValue.h delete mode 100644 src/oscar/Platform/AppSettingValueType.h delete mode 100644 tests/testoscar/Platform/TestAppSettingValueType.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d46509e9..7c5d1d9a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). plugin code sourced from the internet (e.g. SimTK.org) - Internal: Third-party dependencies were updated to their latest versions, where applicable - Internal: `AppConfig` was dropped and all uses of it were converted to `AppSettings` (#893) +- Internal: `AppSettingValue` was refactored into a `Variant` (#789) ## [0.5.12] - 2024/04/29 diff --git a/src/OpenSimCreator/Graphics/CustomRenderingOptions.cpp b/src/OpenSimCreator/Graphics/CustomRenderingOptions.cpp index d30d339c7..8fe04c304 100644 --- a/src/OpenSimCreator/Graphics/CustomRenderingOptions.cpp +++ b/src/OpenSimCreator/Graphics/CustomRenderingOptions.cpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include @@ -72,21 +74,21 @@ void osc::CustomRenderingOptions::setDrawSelectionRims(bool v) SetOption(m_Flags, CustomRenderingOptionFlags::DrawSelectionRims, v); } -void osc::CustomRenderingOptions::forEachOptionAsAppSettingValue(const std::function& callback) const +void osc::CustomRenderingOptions::forEachOptionAsAppSettingValue(const std::function& callback) const { for (const auto& metadata : GetAllCustomRenderingOptionFlagsMetadata()) { - callback(metadata.id, AppSettingValue{m_Flags & metadata.value}); + callback(metadata.id, static_cast(m_Flags & metadata.value)); } } -void osc::CustomRenderingOptions::tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map& lut) +void osc::CustomRenderingOptions::tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map& lut) { for (const auto& metadata : GetAllCustomRenderingOptionFlagsMetadata()) { std::string key = std::string{keyPrefix} + metadata.id; - if (const auto* v = lookup_or_nullptr(lut, key); v->type() == AppSettingValueType::Bool) { - SetOption(m_Flags, metadata.value, v->to_bool()); + if (const auto* v = lookup_or_nullptr(lut, key); v->type() == VariantType::Bool) { + SetOption(m_Flags, metadata.value, v->to()); } } } diff --git a/src/OpenSimCreator/Graphics/CustomRenderingOptions.h b/src/OpenSimCreator/Graphics/CustomRenderingOptions.h index fd72880b5..d95fc691c 100644 --- a/src/OpenSimCreator/Graphics/CustomRenderingOptions.h +++ b/src/OpenSimCreator/Graphics/CustomRenderingOptions.h @@ -2,8 +2,8 @@ #include -#include #include +#include #include #include @@ -34,8 +34,8 @@ namespace osc bool getDrawSelectionRims() const; void setDrawSelectionRims(bool); - void forEachOptionAsAppSettingValue(const std::function&) const; - void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); + void forEachOptionAsAppSettingValue(const std::function&) const; + void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); void applyTo(SceneRendererParams&) const; diff --git a/src/OpenSimCreator/Graphics/ModelRendererParams.cpp b/src/OpenSimCreator/Graphics/ModelRendererParams.cpp index 23c5fc7d4..65ba75dd1 100644 --- a/src/OpenSimCreator/Graphics/ModelRendererParams.cpp +++ b/src/OpenSimCreator/Graphics/ModelRendererParams.cpp @@ -8,8 +8,8 @@ #include #include #include -#include #include +#include #include #include @@ -19,13 +19,13 @@ using namespace osc; namespace { - std::unordered_map ToValues( + std::unordered_map ToValues( std::string_view prefix, const ModelRendererParams& params) { - std::unordered_map rv; + std::unordered_map rv; std::string subPrefix; - const auto callback = [&subPrefix, &rv](std::string_view subkey, AppSettingValue value) + const auto callback = [&subPrefix, &rv](std::string_view subkey, Variant value) { rv.insert_or_assign(subPrefix + std::string{subkey}, std::move(value)); }; @@ -36,8 +36,8 @@ namespace params.overlayOptions.forEachOptionAsAppSettingValue(callback); subPrefix = std::string{prefix} + std::string{"graphics/"}; params.renderingOptions.forEachOptionAsAppSettingValue(callback); - rv.insert_or_assign(std::string{prefix} + "light_color", AppSettingValue{params.lightColor}); - rv.insert_or_assign(std::string{prefix} + "background_color", AppSettingValue{params.backgroundColor}); + rv.insert_or_assign(std::string{prefix} + "light_color", params.lightColor); + rv.insert_or_assign(std::string{prefix} + "background_color", params.backgroundColor); // TODO: floorLocation return rv; @@ -45,17 +45,17 @@ namespace void UpdFromValues( std::string_view prefix, - const std::unordered_map& values, + const std::unordered_map& values, ModelRendererParams& params) { params.decorationOptions.tryUpdFromValues(std::string{prefix} + "decorations/", values); params.overlayOptions.tryUpdFromValues(std::string{prefix} + "overlays/", values); params.renderingOptions.tryUpdFromValues(std::string{prefix} + "graphics/", values); if (const auto* v = lookup_or_nullptr(values, std::string{prefix} + "light_color")) { - params.lightColor = v->to_color(); + params.lightColor = v->to(); } if (const auto* v = lookup_or_nullptr(values,std::string{prefix} + "background_color")) { - params.backgroundColor = v->to_color(); + params.backgroundColor = v->to(); } // TODO: floorLocation } @@ -66,8 +66,7 @@ osc::ModelRendererParams::ModelRendererParams() : backgroundColor{SceneRendererParams::default_background_color()}, floorLocation{SceneRendererParams::default_floor_location()}, camera{create_camera_with_radius(5.0f)} -{ -} +{} void osc::UpdModelRendererParamsFrom( const AppSettings& settings, diff --git a/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.cpp b/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.cpp index 0216fccd4..ddf718cad 100644 --- a/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.cpp +++ b/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.cpp @@ -1,10 +1,10 @@ #include "OpenSimDecorationOptions.h" -#include -#include #include #include #include +#include +#include #include #include @@ -154,22 +154,20 @@ void osc::OpenSimDecorationOptions::setShouldShowContactForces(bool v) SetOption(m_Flags, OpenSimDecorationOptionFlags::ShouldShowContactForces, v); } -void osc::OpenSimDecorationOptions::forEachOptionAsAppSettingValue(const std::function& callback) const +void osc::OpenSimDecorationOptions::forEachOptionAsAppSettingValue(const std::function& callback) const { - callback("muscle_decoration_style", AppSettingValue{GetMuscleDecorationStyleMetadata(m_MuscleDecorationStyle).id}); - callback("muscle_coloring_style", AppSettingValue{GetMuscleColoringStyleMetadata(m_MuscleColoringStyle).id}); - callback("muscle_sizing_style", AppSettingValue{GetMuscleSizingStyleMetadata(m_MuscleSizingStyle).id}); - for (size_t i = 0; i < num_flags(); ++i) - { + callback("muscle_decoration_style", GetMuscleDecorationStyleMetadata(m_MuscleDecorationStyle).id); + callback("muscle_coloring_style", GetMuscleColoringStyleMetadata(m_MuscleColoringStyle).id); + callback("muscle_sizing_style", GetMuscleSizingStyleMetadata(m_MuscleSizingStyle).id); + for (size_t i = 0; i < num_flags(); ++i) { const auto& meta = GetIthOptionMetadata(i); - const bool v = m_Flags & GetIthOption(i); - callback(meta.id, AppSettingValue{v}); + callback(meta.id, static_cast(m_Flags & GetIthOption(i))); } } void osc::OpenSimDecorationOptions::tryUpdFromValues( std::string_view prefix, - const std::unordered_map& lut) + const std::unordered_map& lut) { // looks up a single element in the lut auto lookup = [ @@ -183,39 +181,38 @@ void osc::OpenSimDecorationOptions::tryUpdFromValues( return lookup_or_nullptr(lut, buf); }; - if (auto* appVal = lookup("muscle_decoration_style"); appVal->type() == AppSettingValueType::String) + if (auto* appVal = lookup("muscle_decoration_style"); appVal->type() == VariantType::String) { const auto metadata = GetAllMuscleDecorationStyleMetadata(); - const auto it = rgs::find(metadata, appVal->to_string(), [](const auto& m) { return m.id; }); + const auto it = rgs::find(metadata, appVal->to(), [](const auto& m) { return m.id; }); if (it != metadata.end()) { m_MuscleDecorationStyle = it->value; } } - if (auto* appVal = lookup("muscle_coloring_style"); appVal->type() == AppSettingValueType::String) + if (auto* appVal = lookup("muscle_coloring_style"); appVal->type() == VariantType::String) { const auto metadata = GetAllMuscleColoringStyleMetadata(); - const auto it = rgs::find(metadata, appVal->to_string(), [](const auto& m) { return m.id; }); + const auto it = rgs::find(metadata, appVal->to(), [](const auto& m) { return m.id; }); if (it != metadata.end()) { m_MuscleColoringStyle = it->value; } } - if (auto* appVal = lookup("muscle_sizing_style"); appVal->type() == AppSettingValueType::String) + if (auto* appVal = lookup("muscle_sizing_style"); appVal->type() == VariantType::String) { const auto metadata = GetAllMuscleSizingStyleMetadata(); - const auto it = rgs::find(metadata, appVal->to_string(), [](const auto& m) { return m.id; }); + const auto it = rgs::find(metadata, appVal->to(), [](const auto& m) { return m.id; }); if (it != metadata.end()) { m_MuscleSizingStyle = it->value; } } - for (size_t i = 0; i < num_flags(); ++i) - { + for (size_t i = 0; i < num_flags(); ++i) { const auto& metadata = GetIthOptionMetadata(i); - if (auto* appVal = lookup(metadata.id); appVal->type() == AppSettingValueType::Bool) + if (auto* appVal = lookup(metadata.id); appVal->type() == VariantType::Bool) { - const bool v = appVal->to_bool(); + const bool v = appVal->to(); SetOption(m_Flags, GetIthOption(i), v); } } diff --git a/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.h b/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.h index b5769ff7c..308742f03 100644 --- a/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.h +++ b/src/OpenSimCreator/Graphics/OpenSimDecorationOptions.h @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -62,8 +63,8 @@ namespace osc bool getShouldShowContactForces() const; void setShouldShowContactForces(bool); - void forEachOptionAsAppSettingValue(const std::function&) const; - void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); + void forEachOptionAsAppSettingValue(const std::function&) const; + void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); friend bool operator==(const OpenSimDecorationOptions&, const OpenSimDecorationOptions&) = default; diff --git a/src/OpenSimCreator/Graphics/OverlayDecorationOptions.cpp b/src/OpenSimCreator/Graphics/OverlayDecorationOptions.cpp index be41cb9e2..566f57f9a 100644 --- a/src/OpenSimCreator/Graphics/OverlayDecorationOptions.cpp +++ b/src/OpenSimCreator/Graphics/OverlayDecorationOptions.cpp @@ -1,10 +1,10 @@ #include "OverlayDecorationOptions.h" -#include -#include #include #include #include +#include +#include #include @@ -95,23 +95,22 @@ void osc::OverlayDecorationOptions::setDrawBVH(bool v) SetOption(m_Flags, OverlayDecorationOptionFlags::DrawBVH, v); } -void osc::OverlayDecorationOptions::forEachOptionAsAppSettingValue(const std::function& callback) const +void osc::OverlayDecorationOptions::forEachOptionAsAppSettingValue(const std::function& callback) const { - for (const auto& metadata : GetAllOverlayDecorationOptionFlagsMetadata()) - { - callback(metadata.id, AppSettingValue{m_Flags & metadata.value}); + for (const auto& metadata : GetAllOverlayDecorationOptionFlagsMetadata()) { + callback(metadata.id, static_cast(m_Flags & metadata.value)); } } -void osc::OverlayDecorationOptions::tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map& lut) +void osc::OverlayDecorationOptions::tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map& lut) { for (size_t i = 0; i < num_flags(); ++i) { const auto& metadata = at(GetAllOverlayDecorationOptionFlagsMetadata(), i); const std::string key = std::string{keyPrefix}+metadata.id; - if (const auto* v = lookup_or_nullptr(lut, key); v and v->type() == osc::AppSettingValueType::Bool) { - SetOption(m_Flags, metadata.value, v->to_bool()); + if (const auto* v = lookup_or_nullptr(lut, key); v and v->type() == VariantType::Bool) { + SetOption(m_Flags, metadata.value, v->to()); } } } diff --git a/src/OpenSimCreator/Graphics/OverlayDecorationOptions.h b/src/OpenSimCreator/Graphics/OverlayDecorationOptions.h index e43744367..02a0a87a8 100644 --- a/src/OpenSimCreator/Graphics/OverlayDecorationOptions.h +++ b/src/OpenSimCreator/Graphics/OverlayDecorationOptions.h @@ -2,8 +2,8 @@ #include -#include #include +#include #include #include @@ -39,8 +39,8 @@ namespace osc bool getDrawBVH() const; void setDrawBVH(bool); - void forEachOptionAsAppSettingValue(const std::function&) const; - void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); + void forEachOptionAsAppSettingValue(const std::function&) const; + void tryUpdFromValues(std::string_view keyPrefix, const std::unordered_map&); friend bool operator==(const OverlayDecorationOptions&, const OverlayDecorationOptions&) = default; diff --git a/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp b/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp index 2088d8c5f..ec8b6229d 100644 --- a/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp +++ b/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp @@ -27,11 +27,13 @@ #include #include +#include #include #include #include #include #include +#include using namespace osc::fd; using namespace osc; @@ -40,6 +42,20 @@ namespace { OpenSimCreatorApp* g_opensimcreator_app_global = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + constexpr auto c_default_panel_states = std::to_array>({ + {"panels/Actions/enabled", true}, + {"panels/Navigator/enabled", true}, + {"panels/Log/enabled", true}, + {"panels/Properties/enabled", true}, + {"panels/Selection Details/enabled", true}, + {"panels/Simulation Details/enabled", false}, + {"panels/Coordinates/enabled", true}, + {"panels/Performance/enabled", false}, + {"panels/Muscle Plot/enabled", false}, + {"panels/Output Watches/enabled", false}, + {"panels/Output Plots/enabled", false}, + }); + // minor alias for setlocale so that any linter complaints about MT unsafety // are all deduped to this one source location // @@ -198,17 +214,9 @@ namespace void InitializeOpenSimCreatorSpecificSettingDefaults(AppSettings& settings) { - settings.set_value("panels/Actions/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Navigator/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Log/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Properties/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Selection Details/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Simulation Details/enabled", AppSettingValue{false}, AppSettingScope::System); - settings.set_value("panels/Coordinates/enabled", AppSettingValue{true}, AppSettingScope::System); - settings.set_value("panels/Performance/enabled", AppSettingValue{false}, AppSettingScope::System); - settings.set_value("panels/Muscle Plot/enabled", AppSettingValue{false}, AppSettingScope::System); - settings.set_value("panels/Output Watches/enabled", AppSettingValue{false}, AppSettingScope::System); - settings.set_value("panels/Output Plots/enabled", AppSettingValue{false}, AppSettingScope::System); + for (const auto& [setting_id, default_state] : c_default_panel_states) { + settings.set_value(setting_id, default_state, AppSettingScope::System); + } } } @@ -267,7 +275,7 @@ osc::OpenSimCreatorApp::~OpenSimCreatorApp() noexcept std::string osc::OpenSimCreatorApp::docs_url() const { if (const auto runtime_url = settings().find_value("docs_url")) { - return runtime_url->to_string(); + return runtime_url->to(); } else { return "https://docs.opensimcreator.com"; diff --git a/src/OpenSimCreator/UI/MainUIScreen.cpp b/src/OpenSimCreator/UI/MainUIScreen.cpp index 8ef61102d..04faec7f3 100644 --- a/src/OpenSimCreator/UI/MainUIScreen.cpp +++ b/src/OpenSimCreator/UI/MainUIScreen.cpp @@ -59,11 +59,11 @@ namespace const ParentPtr& api) { if (auto maybeRequestedTab = settings.find_value("initial_tab")) { - if (std::optional maybeEntry = tabRegistry.find_by_name(maybeRequestedTab->to_string())) { + if (std::optional maybeEntry = tabRegistry.find_by_name(maybeRequestedTab->to())) { return maybeEntry->construct_tab(api); } - log_warn("%s: cannot find a tab with this name in the tab registry: ignoring", maybeRequestedTab->to_string().c_str()); + log_warn("%s: cannot find a tab with this name in the tab registry: ignoring", maybeRequestedTab->to().c_str()); log_warn("available tabs are:"); for (auto&& tabRegistryEntry : tabRegistry) { log_warn(" %s", tabRegistryEntry.name().c_str()); @@ -386,7 +386,7 @@ class osc::MainUIScreen::Impl final : void implAddUserOutputExtractor(const OutputExtractor& output) final { m_UserOutputExtractors.push_back(output); - App::upd().upd_settings().set_value("panels/Output Watches/enabled", AppSettingValue{true}); + App::upd().upd_settings().set_value("panels/Output Watches/enabled", true); } void implRemoveUserOutputExtractor(int idx) final diff --git a/src/oscar/CMakeLists.txt b/src/oscar/CMakeLists.txt index 05d09397d..41fa3bf63 100644 --- a/src/oscar/CMakeLists.txt +++ b/src/oscar/CMakeLists.txt @@ -250,9 +250,6 @@ add_library(oscar STATIC Platform/AppSettings.cpp Platform/AppSettings.h Platform/AppSettingScope.h - Platform/AppSettingValue.cpp - Platform/AppSettingValue.h - Platform/AppSettingValueType.h Platform/FilesystemResourceLoader.cpp Platform/FilesystemResourceLoader.h Platform/ILogSink.h diff --git a/src/oscar/Platform.h b/src/oscar/Platform.h index 2c260728d..4ff77a260 100644 --- a/src/oscar/Platform.h +++ b/src/oscar/Platform.h @@ -4,8 +4,6 @@ #include #include #include -#include -#include #include #include #include diff --git a/src/oscar/Platform/App.cpp b/src/oscar/Platform/App.cpp index 4f1554929..10c60ec1f 100644 --- a/src/oscar/Platform/App.cpp +++ b/src/oscar/Platform/App.cpp @@ -79,7 +79,7 @@ namespace LogLevel get_log_level_from_settings(const AppSettings& settings) { if (const auto v = settings.find_value("log_level")) { - if (auto parsed = try_parse_as_log_level(v->to_string())) { + if (auto parsed = try_parse_as_log_level(v->to())) { return *parsed; } } @@ -117,7 +117,7 @@ namespace constexpr int height = 600; Uint32 flags = SDL_WINDOW_OPENGL | SDL_WINDOW_SHOWN | SDL_WINDOW_RESIZABLE | SDL_WINDOW_MAXIMIZED; - if (auto v = config.find_value("experimental_feature_flags/high_dpi_mode"); v and v->to_bool()) { + if (auto v = config.find_value("experimental_feature_flags/high_dpi_mode"); v and *v) { flags |= SDL_WINDOW_ALLOW_HIGHDPI; enable_highdpi_mode_for_this_process(); } diff --git a/src/oscar/Platform/AppSettingValue.cpp b/src/oscar/Platform/AppSettingValue.cpp deleted file mode 100644 index 2f4e45bd1..000000000 --- a/src/oscar/Platform/AppSettingValue.cpp +++ /dev/null @@ -1,71 +0,0 @@ -#include "AppSettingValue.h" - -#include -#include -#include -#include - -#include -#include - -using namespace osc; -using namespace std::literals; - -AppSettingValueType osc::AppSettingValue::type() const -{ - static_assert(std::variant_size_v == num_options()); - - return std::visit(Overload - { - [](const std::string&) { return AppSettingValueType::String; }, - [](bool) { return AppSettingValueType::Bool; }, - [](const Color&) { return AppSettingValueType::Color; }, - }, value_); -} - -bool osc::AppSettingValue::to_bool() const -{ - return std::visit(Overload{ - [](const std::string& str) - { - if (str.empty()) { - return false; - } - else if (is_equal_case_insensitive(str, "false")) { - return false; - } - else if (is_equal_case_insensitive(str, "0")) { - return false; - } - else { - return true; - } - }, - [](bool v) - { - return v; - }, - [](const Color&) - { - return false; - }, - }, value_); -} - -std::string osc::AppSettingValue::to_string() const -{ - return std::visit(Overload{ - [](const std::string& v) { return std::string{v}; }, - [](bool v) { return v ? "true"s : "false"s; }, - [](const Color& c) { return to_html_string_rgba(c); }, - }, value_); -} - -Color osc::AppSettingValue::to_color() const -{ - return std::visit(Overload{ - [](const std::string& v) { return try_parse_html_color_string(v).value_or(Color::white()); }, - [](bool v) { return v ? Color::white() : Color::black(); }, - [](const Color& c) { return c; }, - }, value_); -} diff --git a/src/oscar/Platform/AppSettingValue.h b/src/oscar/Platform/AppSettingValue.h deleted file mode 100644 index 91f349220..000000000 --- a/src/oscar/Platform/AppSettingValue.h +++ /dev/null @@ -1,45 +0,0 @@ -#pragma once - -#include -#include -#include - -#include -#include -#include - -namespace osc -{ - class AppSettingValue final { - public: - explicit AppSettingValue(std::string value) : - value_{value} - {} - - explicit AppSettingValue(const char* value) : - value_{std::string{value}} - {} - - explicit AppSettingValue(CStringView value) : - value_{std::string{value}} - {} - - explicit AppSettingValue(bool value) : - value_{value} - {} - - explicit AppSettingValue(const Color& value) : - value_{value} - {} - - AppSettingValueType type() const; - bool to_bool() const; - std::string to_string() const; - Color to_color() const; - - friend bool operator==(const AppSettingValue&, const AppSettingValue&) = default; - - private: - std::variant value_; - }; -} diff --git a/src/oscar/Platform/AppSettingValueType.h b/src/oscar/Platform/AppSettingValueType.h deleted file mode 100644 index 23340d55c..000000000 --- a/src/oscar/Platform/AppSettingValueType.h +++ /dev/null @@ -1,11 +0,0 @@ -#pragma once - -namespace osc -{ - enum class AppSettingValueType { - String, - Bool, - Color, - NUM_OPTIONS, - }; -} diff --git a/src/oscar/Platform/AppSettings.cpp b/src/oscar/Platform/AppSettings.cpp index c6f7f8777..de54247d8 100644 --- a/src/oscar/Platform/AppSettings.cpp +++ b/src/oscar/Platform/AppSettings.cpp @@ -1,6 +1,5 @@ #include "AppSettings.h" -#include #include #include #include @@ -8,6 +7,8 @@ #include #include #include +#include +#include #include #include @@ -46,23 +47,23 @@ R"(# configuration options // a value stored in the AppSettings lookup table class AppSettingsLookupValue final { public: - AppSettingsLookupValue(AppSettingScope scope, AppSettingValue value) : + AppSettingsLookupValue(AppSettingScope scope, Variant value) : scope_{scope}, value_{std::move(value)} {} AppSettingScope scope() const { return scope_; } - const AppSettingValue& value() const { return value_; } + const Variant& value() const { return value_; } private: AppSettingScope scope_; - AppSettingValue value_; + Variant value_; }; // a lookup containing all app setting values class AppSettingsLookup final { public: - std::optional find_value(std::string_view key) const + std::optional find_value(std::string_view key) const { if (const auto v = lookup_or_nullptr(hashmap_, key)) { return v->value(); @@ -72,7 +73,7 @@ R"(# configuration options } } - void set_value(std::string_view key, AppSettingScope scope, AppSettingValue value) + void set_value(std::string_view key, AppSettingScope scope, Variant value) { AppSettingsLookupValue scoped_value{scope, std::move(value)}; @@ -237,11 +238,17 @@ R"(# configuration options ++cur.iterator; break; } + else if (const auto* int_value = node.as_integer()) { + out.set_value(key_prefix + std::string{k.str()}, scope, Variant{static_cast(**int_value)}); // TODO: 64-bit int support + } + else if (const auto* float_value = node.as_floating_point()) { + out.set_value(key_prefix + std::string{k.str()}, scope, Variant{static_cast(**float_value)}); // TODO: 64-bit float support + } else if (const auto* string_value = node.as_string()) { - out.set_value(key_prefix + std::string{k.str()}, scope, AppSettingValue{**string_value}); + out.set_value(key_prefix + std::string{k.str()}, scope, Variant{**string_value}); } else if (auto const* bool_value = node.as_boolean()) { - out.set_value(key_prefix + std::string{k.str()}, scope, AppSettingValue{**bool_value}); + out.set_value(key_prefix + std::string{k.str()}, scope, Variant{**bool_value}); } } @@ -323,18 +330,26 @@ R"(# configuration options void insert_into_toml_table( toml::table& table, std::string_view key, - const AppSettingValue& value) + const Variant& value) { - static_assert(num_options() == 3); + static_assert(num_options() == 9); // TODO: support more of them switch (value.type()) { - case AppSettingValueType::Bool: - table.insert(key, value.to_bool()); + case VariantType::None: + case VariantType::Bool: + table.insert(key, value.to()); + break; + case VariantType::Float: + table.insert(key, value.to()); + break; + case VariantType::Int: + table.insert(key, value.to()); break; - case AppSettingValueType::String: - case AppSettingValueType::Color: + case VariantType::Color: + case VariantType::String: + case VariantType::StringName: default: - table.insert(key, value.to_string()); + table.insert(key, value.to()); break; } } @@ -377,18 +392,18 @@ R"(# configuration options return system_config_path_; } - std::optional find_value(std::string_view key) const + std::optional find_value(std::string_view key) const { return app_settings_lookup_.find_value(key); } - void set_value(std::string_view key, AppSettingValue value, AppSettingScope scope) + void set_value(std::string_view key, Variant value, AppSettingScope scope) { app_settings_lookup_.set_value(key, scope, std::move(value)); is_dirty_ = true; } - void set_value_if_not_found(std::string_view key, AppSettingValue value, AppSettingScope scope) + void set_value_if_not_found(std::string_view key, Variant value, AppSettingScope scope) { if (not find_value(key)) { set_value(key, value, scope); @@ -467,17 +482,17 @@ class osc::AppSettings::Impl final { return guarded_data_.lock()->system_configuration_file_location(); } - std::optional find_value(std::string_view key) const + std::optional find_value(std::string_view key) const { return guarded_data_.lock()->find_value(key); } - void set_value(std::string_view key, AppSettingValue value, AppSettingScope scope) + void set_value(std::string_view key, Variant value, AppSettingScope scope) { guarded_data_.lock()->set_value(key, std::move(value), scope); } - void set_value_if_not_found(std::string_view key, AppSettingValue value, AppSettingScope scope) + void set_value_if_not_found(std::string_view key, Variant value, AppSettingScope scope) { guarded_data_.lock()->set_value_if_not_found(key, std::move(value), scope); } @@ -583,7 +598,7 @@ std::filesystem::path osc::get_resource_dir_from_settings(const AppSettings& set return get_resources_dir_fallback_path(settings); } - if (resources_dir_setting_value->type() != AppSettingValueType::String) { + if (resources_dir_setting_value->type() != VariantType::String) { log_error("application setting for '%s' is not a string: falling back", resources_key.c_str()); return get_resources_dir_fallback_path(settings); } @@ -596,7 +611,7 @@ std::filesystem::path osc::get_resource_dir_from_settings(const AppSettings& set else { config_file_dir = current_executable_directory().parent_path(); // assume the `bin/` dir is one-up from the settings } - const std::filesystem::path configured_resources_dir{resources_dir_setting_value->to_string()}; + const std::filesystem::path configured_resources_dir{resources_dir_setting_value->to()}; auto resolved_resource_dir = std::filesystem::weakly_canonical(config_file_dir / configured_resources_dir); if (not std::filesystem::exists(resolved_resource_dir)) { @@ -628,7 +643,7 @@ std::optional osc::AppSettings::system_configuration_file return impl_->system_configuration_file_location(); } -std::optional osc::AppSettings::find_value( +std::optional osc::AppSettings::find_value( std::string_view key) const { return impl_->find_value(key); @@ -636,7 +651,7 @@ std::optional osc::AppSettings::find_value( void osc::AppSettings::set_value( std::string_view key, - AppSettingValue value, + Variant value, AppSettingScope scope) { impl_->set_value(key, std::move(value), scope); @@ -644,7 +659,7 @@ void osc::AppSettings::set_value( void osc::AppSettings::set_value_if_not_found( std::string_view key, - AppSettingValue value, + Variant value, AppSettingScope scope) { impl_->set_value_if_not_found(key, std::move(value), scope); diff --git a/src/oscar/Platform/AppSettings.h b/src/oscar/Platform/AppSettings.h index 67f68d792..f8242dc95 100644 --- a/src/oscar/Platform/AppSettings.h +++ b/src/oscar/Platform/AppSettings.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include @@ -30,9 +30,9 @@ namespace osc // may have deleted it) std::optional system_configuration_file_location() const; - std::optional find_value(std::string_view key) const; - void set_value(std::string_view key, AppSettingValue, AppSettingScope = AppSettingScope::User); - void set_value_if_not_found(std::string_view key, AppSettingValue, AppSettingScope = AppSettingScope::User); + std::optional find_value(std::string_view key) const; + void set_value(std::string_view key, Variant, AppSettingScope = AppSettingScope::User); + void set_value_if_not_found(std::string_view key, Variant, AppSettingScope = AppSettingScope::User); // if available, returns the filesystem path of the configuration file that // provided the given setting value diff --git a/src/oscar/UI/Panels/StandardPanelImpl.cpp b/src/oscar/UI/Panels/StandardPanelImpl.cpp index ae897998c..2cc40f51b 100644 --- a/src/oscar/UI/Panels/StandardPanelImpl.cpp +++ b/src/oscar/UI/Panels/StandardPanelImpl.cpp @@ -2,8 +2,8 @@ #include #include -#include #include +#include #include #include @@ -42,7 +42,7 @@ CStringView osc::StandardPanelImpl::impl_get_name() const bool osc::StandardPanelImpl::impl_is_open() const { if (auto v = App::settings().find_value(panel_enabled_config_key_)) { - return v->to_bool(); + return v->to(); } else { return false; @@ -51,12 +51,12 @@ bool osc::StandardPanelImpl::impl_is_open() const void osc::StandardPanelImpl::impl_open() { - App::upd().upd_settings().set_value(panel_enabled_config_key_, AppSettingValue{true}); + App::upd().upd_settings().set_value(panel_enabled_config_key_, true); } void osc::StandardPanelImpl::impl_close() { - App::upd().upd_settings().set_value(panel_enabled_config_key_, AppSettingValue{false}); + App::upd().upd_settings().set_value(panel_enabled_config_key_, false); } void osc::StandardPanelImpl::impl_on_draw() diff --git a/src/oscar/UI/ui_context.cpp b/src/oscar/UI/ui_context.cpp index 0538acab5..f91b5fa8f 100644 --- a/src/oscar/UI/ui_context.cpp +++ b/src/oscar/UI/ui_context.cpp @@ -89,7 +89,7 @@ void osc::ui::context::init() float vdpi{}; // if the user explicitly enabled high_dpi_mode... - if (auto v = App::settings().find_value("experimental_feature_flags/high_dpi_mode"); v and v->to_bool()) { + if (auto v = App::settings().find_value("experimental_feature_flags/high_dpi_mode"); v and *v) { // and SDL is able to get the DPI of the given window... if (SDL_GetDisplayDPI(SDL_GetWindowDisplayIndex(App::upd().upd_underlying_window()), &dpi, &hdpi, &vdpi) == 0) { return dpi / 96.0f; // then calculate the scaling factor diff --git a/tests/testoscar/CMakeLists.txt b/tests/testoscar/CMakeLists.txt index 6a4694976..774347fa8 100644 --- a/tests/testoscar/CMakeLists.txt +++ b/tests/testoscar/CMakeLists.txt @@ -71,7 +71,6 @@ add_executable(testoscar MetaTests/TestUtilsHeader.cpp MetaTests/TestVariantHeader.cpp - Platform/TestAppSettingValueType.cpp Platform/TestResourceDirectoryEntry.cpp Platform/TestResourceLoader.cpp Platform/TestResourcePath.cpp diff --git a/tests/testoscar/Platform/TestAppSettingValueType.cpp b/tests/testoscar/Platform/TestAppSettingValueType.cpp deleted file mode 100644 index b844cbb6d..000000000 --- a/tests/testoscar/Platform/TestAppSettingValueType.cpp +++ /dev/null @@ -1,101 +0,0 @@ -#include - -#include -#include - -#include - -#include -#include - -using namespace osc; - -TEST(AppSettingValue, CanExplicitlyConstructFromStringRValue) -{ - AppSettingValue v{std::string{"stringrval"}}; - - ASSERT_EQ(v.to_string(), "stringrval"); -} - -TEST(AppSettingValue, CanExplicitlyConstructFromStringLiteral) -{ - AppSettingValue v{"cstringliteral"}; - ASSERT_EQ(v.to_string(), "cstringliteral"); -} - -TEST(AppSettingValue, CanExplicitlyConstructFromCStringView) -{ - AppSettingValue v{CStringView{"cstringview"}}; - ASSERT_EQ(v.to_string(), "cstringview"); -} - -TEST(AppSettingValue, CanExplicitlyContructFromBool) -{ - AppSettingValue vfalse{false}; - ASSERT_EQ(vfalse.to_bool(), false); - AppSettingValue vtrue{true}; - ASSERT_EQ(vtrue.to_bool(), true); -} - -TEST(AppSettingValue, CanExplicitlyConstructFromColor) -{ - AppSettingValue v{Color::red()}; - ASSERT_EQ(v.to_color(), Color::red()); -} - -TEST(AppSettingValue, BoolValueToStringReturnsExpectedStrings) -{ - AppSettingValue vfalse{false}; - ASSERT_EQ(vfalse.to_string(), "false"); - AppSettingValue vtrue{true}; - ASSERT_EQ(vtrue.to_string(), "true"); -} - -TEST(AppSettingValue, StringValueToBoolReturnsExpectedBoolValues) -{ - ASSERT_EQ(AppSettingValue{"false"}.to_bool(), false); - ASSERT_EQ(AppSettingValue{"FALSE"}.to_bool(), false); - ASSERT_EQ(AppSettingValue{"False"}.to_bool(), false); - ASSERT_EQ(AppSettingValue{"FaLsE"}.to_bool(), false); - ASSERT_EQ(AppSettingValue{"0"}.to_bool(), false); - ASSERT_EQ(AppSettingValue{""}.to_bool(), false); - - // all other strings are effectively `true` - ASSERT_EQ(AppSettingValue{"true"}.to_bool(), true); - ASSERT_EQ(AppSettingValue{"non-empty string"}.to_bool(), true); - ASSERT_EQ(AppSettingValue{" "}.to_bool(), true); -} - -TEST(AppSettingValue, ColorValueToStringReturnsSameAsToHtmlStringRGBA) -{ - const auto colors = std::to_array({ - Color::red(), - Color::magenta(), - }); - - for (const auto& color : colors) { - ASSERT_EQ(AppSettingValue{color}.to_string(), to_html_string_rgba(color)); - } -} - -TEST(AppSettingValue, ColorValueToStringReturnsExpectedManualExamples) -{ - ASSERT_EQ(AppSettingValue{Color::yellow()}.to_string(), "#ffff00ff"); - ASSERT_EQ(AppSettingValue{Color::magenta()}.to_string(), "#ff00ffff"); -} - -TEST(AppSettingValue, StringValueToColorWorksIfStringIsAValidHTMLColorString) -{ - ASSERT_EQ(AppSettingValue{"#ff0000ff"}.to_color(), Color::red()); - ASSERT_EQ(AppSettingValue{"#00ff00ff"}.to_color(), Color::green()); - ASSERT_EQ(AppSettingValue{"#ffffffff"}.to_color(), Color::white()); - ASSERT_EQ(AppSettingValue{"#00000000"}.to_color(), Color::clear()); - ASSERT_EQ(AppSettingValue{"#000000ff"}.to_color(), Color::black()); - ASSERT_EQ(AppSettingValue{"#000000FF"}.to_color(), Color::black()); - ASSERT_EQ(AppSettingValue{"#123456ae"}.to_color(), *try_parse_html_color_string("#123456ae")); -} - -TEST(AppSettingValue, StringValueColorReturnsWhiteIfStringIsInvalidHTMLColorString) -{ - ASSERT_EQ(AppSettingValue{"not a color"}.to_color(), Color::white()); -}