Changeset View
Standalone View
source/ps/ConfigDB.h
Show All 23 Lines | /* | ||||
JavaScript: Check this documentation: http://trac.wildfiregames.com/wiki/Exposed_ConfigDB_Functions | JavaScript: Check this documentation: http://trac.wildfiregames.com/wiki/Exposed_ConfigDB_Functions | ||||
*/ | */ | ||||
#ifndef INCLUDED_CONFIGDB | #ifndef INCLUDED_CONFIGDB | ||||
#define INCLUDED_CONFIGDB | #define INCLUDED_CONFIGDB | ||||
#include "lib/file/vfs/vfs_path.h" | #include "lib/file/vfs/vfs_path.h" | ||||
#include "ps/CStr.h" | #include "ps/CStr.h" | ||||
#include "ps/Singleton.h" | |||||
#include <array> | |||||
#include <map> | #include <map> | ||||
#include <memory> | |||||
#include <mutex> | |||||
#include <vector> | #include <vector> | ||||
/** | /** | ||||
* Namespace priorities: | * Namespace priorities: | ||||
* - Command line args override everything | * - Command line args override everything | ||||
* - User supersedes HWDetect (let the user try crashing his system). | * - User supersedes HWDetect (let the user try crashing his system). | ||||
* - HWDetect supersedes mods & default -> mods can mod hwdetect itself. | * - HWDetect supersedes mods & default -> mods can mod hwdetect itself. | ||||
* - SYSTEM is used for local.cfg and is basically for setting custom defaults. | * - SYSTEM is used for local.cfg and is basically for setting custom defaults. | ||||
Show All 11 Lines | |||||
using CConfigValueSet = std::vector<CStr>; | using CConfigValueSet = std::vector<CStr>; | ||||
// Opaque data type so that callers that hook into ConfigDB can delete their hooks. | // Opaque data type so that callers that hook into ConfigDB can delete their hooks. | ||||
// Would be defined in CConfigDB but then it couldn't be forward-declared, which is rather annoying. | // Would be defined in CConfigDB but then it couldn't be forward-declared, which is rather annoying. | ||||
// Actually defined below - requires access to CConfigDB. | // Actually defined below - requires access to CConfigDB. | ||||
class CConfigDBHook; | class CConfigDBHook; | ||||
#define g_ConfigDB CConfigDB::GetSingleton() | #define g_ConfigDB (*CConfigDB::Instance()) | ||||
class CConfigDB : public Singleton<CConfigDB> | class CConfigDB | ||||
vladislavbelov: It doesn't look good, it's better to use a static method:
```lang=cpp
#define g_ConfigDB… | |||||
{ | { | ||||
friend CConfigDBHook; | friend CConfigDBHook; | ||||
public: | public: | ||||
CConfigDB(); | |||||
~CConfigDB(); | |||||
CConfigDB(const CConfigDB&) = delete; | |||||
CConfigDB(CConfigDB&&) = delete; | |||||
static void Initialise(); | |||||
static void Shutdown(); | |||||
static bool IsInitialised(); | |||||
static CConfigDB* Instance(); | |||||
/** | /** | ||||
* Attempt to retrieve the value of a config variable with the given name; | * Attempt to retrieve the value of a config variable with the given name; | ||||
* will search CFG_COMMAND first, and then all namespaces from the specified | * will search CFG_COMMAND first, and then all namespaces from the specified | ||||
* namespace down. | * namespace down. | ||||
*/ | */ | ||||
void GetValue(EConfigNamespace ns, const CStr& name, bool& value); | void GetValue(EConfigNamespace ns, const CStr& name, bool& value); | ||||
///@copydoc CConfigDB::GetValue | ///@copydoc CConfigDB::GetValue | ||||
void GetValue(EConfigNamespace ns, const CStr& name, int& value); | void GetValue(EConfigNamespace ns, const CStr& name, int& value); | ||||
▲ Show 20 Lines • Show All 97 Lines • ▼ Show 20 Lines | public: | ||||
bool WriteValueToFile(EConfigNamespace ns, const CStr& name, const CStr& value, const VfsPath& path); | bool WriteValueToFile(EConfigNamespace ns, const CStr& name, const CStr& value, const VfsPath& path); | ||||
bool WriteValueToFile(EConfigNamespace ns, const CStr& name, const CStr& value); | bool WriteValueToFile(EConfigNamespace ns, const CStr& name, const CStr& value); | ||||
/** | /** | ||||
* Register a simple lambda that will be called anytime the value changes in any namespace | * Register a simple lambda that will be called anytime the value changes in any namespace | ||||
* This is simple on purpose, the hook is responsible for checking if it should do something. | * This is simple on purpose, the hook is responsible for checking if it should do something. | ||||
* When RegisterHookAndCall is called, the hook is immediately triggered. | * When RegisterHookAndCall is called, the hook is immediately triggered. | ||||
* NB: CConfigDBHook will auto-unregister the hook when destroyed, | |||||
* so you can use it to tie the lifetime of the hook to your object. | |||||
* The hook will be deleted alongside ConfigDB anyways. | |||||
*/ | */ | ||||
CConfigDBHook RegisterHookAndCall(const CStr& name, std::function<void()> hook); | [[nodiscard]] CConfigDBHook RegisterHookAndCall(const CStr& name, std::function<void()> hook); | ||||
Not Done Inline ActionsDoes the attribute work for all supported compilers? vladislavbelov: Does the attribute work for all supported compilers? | |||||
Done Inline ActionsYes, this part of C++17 is supported by all compilers (see list at https://trac.wildfiregames.com/wiki/CppSupport). MSVC17 doesn't support no discard constructors, but that's not relevant here. wraitii: Yes, this part of C++17 is supported by all compilers (see list at https://trac.wildfiregames. | |||||
void UnregisterHook(CConfigDBHook&& hook); | void UnregisterHook(CConfigDBHook&& hook); | ||||
void UnregisterHook(std::unique_ptr<CConfigDBHook> hook); | void UnregisterHook(std::unique_ptr<CConfigDBHook> hook); | ||||
private: | private: | ||||
static std::map<CStr, CConfigValueSet> m_Map[]; | std::array<std::map<CStr, CConfigValueSet>, CFG_LAST> m_Map; | ||||
static std::multimap<CStr, std::function<void()>> m_Hooks; | std::multimap<CStr, std::function<void()>> m_Hooks; | ||||
static VfsPath m_ConfigFile[]; | std::array<VfsPath, CFG_LAST> m_ConfigFile; | ||||
static bool m_HasChanges[]; | std::array<bool, CFG_LAST> m_HasChanges; | ||||
mutable std::recursive_mutex m_Mutex; | |||||
Not Done Inline Actions#include <array> Stan: #include <array>
#include <multimap> | |||||
Done Inline Actionsstd::multimap is in <map> wraitii: std::multimap is in <map> | |||||
}; | }; | ||||
class CConfigDBHook | class CConfigDBHook | ||||
{ | { | ||||
friend class CConfigDB; | friend class CConfigDB; | ||||
public: | public: | ||||
CConfigDBHook() = delete; | CConfigDBHook() = delete; | ||||
CConfigDBHook(const CConfigDBHook&) = delete; | |||||
// Point the moved-from hook to end, which is checked for in UnregisterHook, | // Point the moved-from hook to end, which is checked for in UnregisterHook, | ||||
// to avoid a double-erase error. | // to avoid a double-erase error. | ||||
CConfigDBHook(CConfigDBHook&& h) : m_ConfigDB(h.m_ConfigDB) { m_Ptr = std::move(h.m_Ptr); h.m_Ptr = m_ConfigDB.m_Hooks.end(); } | CConfigDBHook(CConfigDBHook&& h) : m_ConfigDB(h.m_ConfigDB) | ||||
CConfigDBHook(const CConfigDBHook&) = delete; | { | ||||
m_Ptr = std::move(h.m_Ptr); | |||||
h.m_Ptr = m_ConfigDB.m_Hooks.end(); | |||||
} | |||||
// Unregisters the hook. Must be called before the original ConfigDB gets deleted. | |||||
~CConfigDBHook() | |||||
{ | |||||
m_ConfigDB.UnregisterHook(std::move(*this)); | |||||
} | |||||
private: | private: | ||||
CConfigDBHook(CConfigDB& cdb, std::multimap<CStr, std::function<void()>>::iterator p) : m_ConfigDB(cdb), m_Ptr(p) {}; | CConfigDBHook(CConfigDB& cdb, std::multimap<CStr, std::function<void()>>::iterator p) | ||||
: m_ConfigDB(cdb), m_Ptr(p) | |||||
{}; | |||||
std::multimap<CStr, std::function<void()>>::iterator m_Ptr; | std::multimap<CStr, std::function<void()>>::iterator m_Ptr; | ||||
Not Done Inline ActionsThat's a bad design if a hook can overlive the config. vladislavbelov: That's a bad design if a hook can overlive the config. | |||||
Done Inline ActionsThe lifetime of our objects are somewhat unpredictable. I'm not sure we can guarantee that the hook owner is always destroyed before / after ConfigDB. wraitii: The lifetime of our objects are somewhat unpredictable. I'm not sure we can guarantee that the… | |||||
Not Done Inline ActionsYou know all places where hooks are living. If you don't know the order of initialization/deinitialization it means that someone can access the config after its deinitialization. The order should be deterministic if there're dependencies. vladislavbelov: You know all places where hooks are living.
If you don't know the order of… | |||||
Done Inline ActionsThe problem is not really an object accessing the configDB after its destruction, it's an object keeping a hook after ConfigDB is destroyed, because the object might outlive ConfigDB. Typically, the global thread pool in D3848 stores a hook, but this hook member variable (not the hook itself) can outlive the configDB - that's normal behaviour. We don't really have an easy way to send a signal to all hook holders to drop their hooks / it seems rather more annoying than just storing a validity token in the hook object. An alternative would be storing a weak_ptr to the ConfigDB object itself, and making ConfigDB a shared_ptr, but I dislike the semantics of that more. wraitii: The problem is not really an object accessing the configDB after its destruction, it's an… | |||||
Not Done Inline ActionsYou don't need to access ConfigDB after its destruction. We also don't need to store hooks in the pool, the number might be determined on a start. You know the point after which you don't need to store hooks anymore, the end of the main loop. vladislavbelov: You don't need to access `ConfigDB` after its destruction.
We also don't need to store hooks… | |||||
Done Inline Actions
ConfigDB is actually reconstructed when mods change, so that's not correct, which is why I'm doing this in the first place. If indeed hooks could live forever, we wouldn't have this problem. wraitii: > You know the point after which you don't need to store hooks anymore, the end of the main… | |||||
Not Done Inline ActionsAnyway we shouldn't have such unpredictable behavior. Especially in case you know when the config is going to destroy. vladislavbelov: Anyway we shouldn't have such unpredictable behavior. Especially in case you know when the… | |||||
CConfigDB& m_ConfigDB; | CConfigDB& m_ConfigDB; | ||||
}; | }; | ||||
// stores the value of the given key into <destination>. this quasi-template | // stores the value of the given key into <destination>. this quasi-template | ||||
// convenience wrapper on top of GetValue simplifies user code | // convenience wrapper on top of GetValue simplifies user code | ||||
#define CFG_GET_VAL(name, destination)\ | #define CFG_GET_VAL(name, destination)\ | ||||
g_ConfigDB.GetValue(CFG_USER, name, destination) | g_ConfigDB.GetValue(CFG_USER, name, destination) | ||||
#endif // INCLUDED_CONFIGDB | #endif // INCLUDED_CONFIGDB |
It doesn't look good, it's better to use a static method:
#define g_ConfigDB *CConfigDB::Instance()
Especially in case you already have a static method IsInitialised.