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.
9.8 KiB
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.).
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.
[[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.
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 *SettingValues the size of the *Setting enum.
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.
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.
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 SettingValues (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.
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.