Files
Vasily 1465c4a8bd Convert getSettingsController() to global static object
After looking at the assembly output it became apparent that the Windows
compiler did not compile out the `getSettingsController()` function
call.

This led to an unnecessary function call in order to get the
SettingsController. Turning it into a static object in the compilation
unit scope removes this function call and turns getSettings call into a
single instruction (not counting zeroing the register for comparison and
the jump instruction afterwards).

----

Adding the following code to the `main` function:
```cpp
f ( settings::getSetting( settings::BoolSetting::UTILITY_vrcDebug ) )
{
        LOG( INFO ) << "Enabled";
}
else
{
        LOG( INFO ) << "Disabled";
}
```

Leads to this with the `getSettingsController` function:
```assembly
[...]
    47            if ( settings::getSetting( settings::BoolSetting::UTILITY_vrcDebug ) )
0x7ff6c96819a0  <+ 1024>         call AdvancedSettings!settings::getSettingsController (00007ff6`c96cd280)
0x7ff6c96819a5  <+ 1029>         xorps   xmm0,xmm0
0x7ff6c96819a8  <+ 1032>         cmp     byte ptr [rax+5FCh],0
0x7ff6c96819af  <+ 1039>         je AdvancedSettings!main+0x4fd (00007ff6`c9681a9d)
    49                LOG( INFO ) << "Enabled";
0x7ff6c96819b5  <+ 1045>         mov     qword ptr
[rsp+288h],r15
0x7ff6c96819bd  <+ 1053>         mov     qword ptr
[rsp+290h],rsi
[...]
```

And this without the function:
```assembly
[...]
    47            if ( settings::getSetting( settings::BoolSetting::UTILITY_vrcDebug ) )
0x7ff7d97a19bb  <+ 1019>         xorps   xmm0,xmm0
0x7ff7d97a19be  <+ 1022>         cmp     byte ptr [AdvancedSettings!settings::settingController+0x5fc (00007ff7`d9854d4c)],0
0x7ff7d97a19c5  <+ 1029>         je AdvancedSettings!main+0x4f3 (00007ff7`d97a1ab3)
    49                LOG( INFO ) << "Enabled";
0x7ff7d97a19cb  <+ 1035>         mov     qword ptr
[rsp+288h],r15
0x7ff7d97a19d3  <+ 1043>         mov     qword ptr
[rsp+290h],rsi
[...]
```

Notice the missing `call AdvancedSettings![...]` without the function
and that the settings call literally cannot be made any faster.
2019-12-10 08:28:41 +01:00

229 lines
9.8 KiB
Markdown

## Problem
The current solution for storing and getting settings is to directly use the Qt QSettings object.
This leads to extremely verbose code for something that really should just be a single function call.
While the Qt settings management abstracts low level details very well, an even higher level API should be sought after.
The current system has stringly typed settings, which means it's possible to save the `desktopWidth` setting, but attempt to use the `deskWidth` setting somewhere else without noticing.
Currently, there are many different ways for a variable to be initialized depending on what goes wrong (QML, header file initialization, constructor, QSettings loading).
There should only default option when none other are available.
## Considerations for the new Settings API
The public facing API should be decoupled from the Qt API, in case we want to use another API later.
There should be a single correct way of referring to a setting.
The public API should not concern itself with how the settings are stored (.ini file/database/PROM).
The public API should be simple, and return error checked, ready values.
The settings should be logically grouped.
Ideally, the type system is used to ensure that a setting isn't used as the wrong type (at compile time?).
Ideally, it should not be possible to attempt to retrieve non-existent settings.
## Requirements
The API MUST transparently allow the storage of persistent values. The performance impact during runtime SHOULD be as small as possible.
Settings set using the API MUST be recoverable in the event of an unexpected program termination.
The API MUST allow for easy addition of new settings, with minimal vectors for error.
The API MUST type check the values, so that it is not possible to misinterpret data.
The API MUST have a default value for all settings that is used in case previously stored values are unavailable.
## Public API
The public API is in the `settings.h` file in the `settings` namespace.
The settings are represented as enums according to type.
Each option is prefixed with their general category (`PLAYSPACE_`, etc.).
```cpp
enum class BoolSetting
{
PLAYSPACE_lockXToggle,
PLAYSPACE_lockYToggle,
PLAYSPACE_lockZToggle,
[...]
CHAPERONE_disableChaperone,
// LAST_ENUMERATOR must always be set to the last value
LAST_ENUMERATOR = CHAPERONE_disableChaperone,
};
enum class DoubleSetting
{
UTILITY_desktopWidth,
[...]
CHAPERONE_fadeDistanceRemembered,
// LAST_ENUMERATOR must always be set to the last value
LAST_ENUMERATOR = CHAPERONE_fadeDistanceRemembered,
};
enum class StringSetting
{
KEYBOARDSHORTCUT_keyboardOne,
KEYBOARDSHORTCUT_keyboardTwo,
KEYBOARDSHORTCUT_keyboardThree,
// LAST_ENUMERATOR must always be set to the last value
LAST_ENUMERATOR = KEYBOARDSHORTCUT_keyboardThree,
};
enum class IntSetting
{
UTILITY_alarmHour,
UTILITY_alarmMinute,
// LAST_ENUMERATOR must always be set to the last value
LAST_ENUMERATOR = UTILITY_alarmMinute,
};
```
The order of the of the enumerators matter, and they can't be reordered without leading to a runtime error.
The `LAST_ENUMERATOR` is a result of C++ enums not knowing how many enumerators they have, and it is required for the implementation to work correctly.
Values can be accessed with imperative functions.
```cpp
[[nodiscard]] bool getSetting( const BoolSetting setting );
void setSetting( const BoolSetting setting, const bool value );
[[nodiscard]] double getSetting( const DoubleSetting setting );
void setSetting( const DoubleSetting setting, const double value );
[[nodiscard]] int getSetting( const IntSetting setting );
void setSetting( const IntSetting setting, const int value );
[[nodiscard]] std::string getSetting( const StringSetting setting );
void setSetting( const StringSetting setting, const std::string value );
```
The `[[nodiscard]]` means that it's a compiler error to not use the return value of the function.
Since none of the getters have side effects it would always be wrong to call the function without using the value.
Additionally, there are three utility functions.
```cpp
std::string initializeAndGetSettingsPath();
void saveChangedSettings();
void saveAllSettings();
```
`saveChangedSettings()` only writes settings to disk that have had their values changed through the API.
`saveAllSettings()` writes all known settings to disk.
`initializeAndGetSettingsPath()` initializes the API and returns the full path of the settings file.
It is not a requirement for this function to be called for the API to work.
If it is not called, the API will be initialized with the first call to the API.
The function is used as a means of controlling when the API will be initialized, as well as getting the current file path.
## Implementation
The public imperative API is an abstraction for the implementation, which is contained in a `SettingsController`.
The `SettingsController` object is a `static` object inside the `settings.cpp` compilation unit.
This is done to remove the limitations that an object oriented design would bring, namely lifetime and scope management.
Since the API is supposed to be globally available throughout the lifetime of the program, creating it as an object in the `OverlayController` and passing it to subcontrollers would only introduce unnecessary parameter passing.
The `SettingsController` also relies on containing the settings as mutable state, and doing so in a purely imperative way would be cumbersome.
The API currently supports `int`, `double`, `std::string` and `bool` values. Below the `int` version is examined but the others are identical in implementation.
Settings are stored as an array of `*SettingValue`s the size of the `*Setting` enum.
```cpp
constexpr static auto intSettingsSize
= static_cast<int>( IntSetting::LAST_ENUMERATOR ) + 1;
std::array<IntSettingValue, intSettingsSize> m_intSettings{
IntSettingValue{ IntSetting::UTILITY_alarmHour,
SettingCategory::Utility,
QtInfo{ "alarmHour" },
0 },
IntSettingValue{ IntSetting::UTILITY_alarmMinute,
SettingCategory::Utility,
QtInfo{ "alarmMinute" },
0 },
};
```
Here, for each setting, the following is defined:
* `setting`: Which enumerator the setting has.
* `category`: Which category the setting belongs to (used for QSettings categories).
* `QtInfo`: A struct that contains Qt specific information. For now only a string value containing the name of the setting.
* The default value of the setting.
The `*SettingValue` classes are template specializations of the `SpecificSettingValue` class.
```cpp
using IntSettingValue = SpecificSettingValue<int, IntSetting>;
```
The `SpecificSettingValue` class contains getters for the `setting`, `category` and `qtInfo` fields, as well as a getter+setter for the `value` field.
```cpp
template <typename Value, typename Setting>
class SpecificSettingValue : public SettingValue
{
public:
SpecificSettingValue( const Setting setting,
const SettingCategory category,
const QtInfo qtInfo,
const Value defaultValue )
: SettingValue( category, qtInfo ), m_setting( setting ),
m_value( defaultValue )
{
// Attempt to load saved setting
// Use default if none saved
// Save setting if not saved to make it appear in .ini file
}
```
The templates used are not very complex. For the `IntSettingValue` you would simply mentally replace `Setting` with `IntSetting` and `Value` with `int`.
The templates reduce code duplication by a very large amount, which is why they are used.
The `SpecificSettingValue` inherits from the `SettingValue` base class. This class only contains fields for `category` and `qtInfo`.
The inheritance from `SettingValue` is necessary in order to make the `saveChangedValues()` easier to implement.
When a setting has changed value it is added to an array of `SettingValue`s (the base class).
It is possible to call `saveValue()` on all derived classes since it has been defined as a `virtual` function in the base class.
This is done in the `saveChangedValues()` function.
Templates might seem daunting if when they are encountered for the first time, but the usage here is not very advanced.
```cpp
template <typename ReturnType, typename Setting>
[[nodiscard]] ReturnType getSetting( const Setting setting ) const noexcept
{
const auto index = static_cast<std::size_t>( setting );
if constexpr ( std::is_same<Setting, BoolSetting>::value )
{
return m_boolSettings[index].value();
}
else if constexpr ( std::is_same<Setting, DoubleSetting>::value )
{
return m_doubleSettings[index].value();
}
else if constexpr ( std::is_same<Setting, IntSetting>::value )
{
return m_intSettings[index].value();
}
else if constexpr ( std::is_same<Setting, StringSetting>::value )
{
return m_stringSettings[index].value();
}
}
```
The above is the generalized `getSetting()` function that takes a generalized `Setting` as a parameter, and returns a generalized `ReturnType`.
Following the `IntSettingValue` example we would pass an `IntSetting` enum to `Setting` and `int` to `ReturnType`.
All our enums can be cast as integers, so we do that outside of the big `if` block.
Next we need to specify which array we index in to get the relevant setting, so we use `if constexpr` and `std::is_same<A, B>::value` to find which type we are dealing with.
This will compile different functions for the different types, so the `if constexpr` statement would not get executed.
The alternative to this would be having `n` amount of specific functions with predefined types.
This is done in the public API for explicitness, but internally it is much easier to maintain a single function.
The above function compiles down to the equivalent of four manually typed functions.