Change OverlayController from singleton pattern to regular RAII

The singleton pattern was poorly implemented and didn't provide the
benefits usually associated with the singleton pattern, with all the
negatives.

Refer to this github issue:
https://github.com/OpenVR-Advanced-Settings/OpenVR-AdvancedSettings/issues/29

src/main.cpp

@@ -408,16 +408,8 @@ int main( int argc, char* argv[] )
 
         // The OverlayController handles the majority of the application specfic
         // things. It contains all the other tabs.
-        // It is created using a singleton pattern that destructs the old
-        // instance whenever createInstance is called. This is obviously not
-        // ideal since it can lead to dangling points and use after free.
-        // The constructor only sets two member vars called desktopMode and
-        // noSound, the rest of the initialization is done in Init. It is
-        // unknown why this is done.
-        advsettings::OverlayController* controller
-            = advsettings::OverlayController::createInstance( desktopMode,
-                                                              noSound );
-        controller->Init( &qmlEngine );
+        advsettings::OverlayController controller(
+            desktopMode, noSound, qmlEngine );
 
         QQmlComponent component(
             &qmlEngine, QUrl::fromLocalFile( "res/qml/mainwidget.qml" ) );
@@ -428,7 +420,7 @@ int main( int argc, char* argv[] )
                          << std::endl;
         }
         auto quickObj = component.create();
-        controller->SetWidget(
+        controller.SetWidget(
             qobject_cast<QQuickItem*>( quickObj ),
             advsettings::OverlayController::applicationDisplayName,
             advsettings::OverlayController::applicationKey );

src/overlaycontroller.cpp

@@ -24,18 +24,14 @@
 // application namespace
 namespace advsettings
 {
-std::unique_ptr<OverlayController> OverlayController::singleton;
-
 constexpr const char* OverlayController::applicationVersionString;
 
 QSettings* OverlayController::_appSettings = nullptr;
 
-OverlayController::~OverlayController()
-{
-    Shutdown();
-}
-
-void OverlayController::Init( QQmlEngine* qmlEngine )
+OverlayController::OverlayController( bool desktopMode,
+                                      bool noSound,
+                                      QQmlEngine& qmlEngine )
+    : QObject(), desktopMode( desktopMode ), noSound( noSound )
 {
     // Loading the OpenVR Runtime
     auto initError = vr::VRInitError_None;
@@ -237,29 +233,39 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
     accessibilityTabController.initStage1();
 
     // Set qml context
-    qmlEngine->rootContext()->setContextProperty( "applicationVersion",
-                                                  getVersionString() );
-    qmlEngine->rootContext()->setContextProperty( "vrRuntimePath",
-                                                  getVRRuntimePathUrl() );
+    qmlEngine.rootContext()->setContextProperty( "applicationVersion",
+                                                 getVersionString() );
+    qmlEngine.rootContext()->setContextProperty( "vrRuntimePath",
+                                                 getVRRuntimePathUrl() );
 
-    // Register qml singletons
+    // Pretty disgusting trick to allow qmlRegisterSingletonType to continue
+    // working with the lambdas that were already there. The callback function
+    // in qmlRegisterSingletonType won't work with any lambdas that capture the
+    // environment. The alternative to making a static pointer to this was
+    // rewriting all QML to not be singletons, which should probably be done
+    // whenever possible.
+    static OverlayController* const objectAddress = this;
     qmlRegisterSingletonType<OverlayController>(
         "matzman666.advsettings",
         1,
         0,
         "OverlayController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = getInstance();
+            QObject* obj = objectAddress;
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
+    // It is unknown if it it intended for the generic in
+    // qmlRegisterSingletonType to be <SteamVRTabController> in all the
+    // remaining function calls, or if it's just a copy paste accident that
+    // happens to work.
     qmlRegisterSingletonType<SteamVRTabController>(
         "matzman666.advsettings",
         1,
         0,
         "SteamVRTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->steamVRTabController;
+            QObject* obj = &( objectAddress->steamVRTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -269,7 +275,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "ChaperoneTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->chaperoneTabController;
+            QObject* obj = &( objectAddress->chaperoneTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -279,7 +285,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "MoveCenterTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->moveCenterTabController;
+            QObject* obj = &( objectAddress->moveCenterTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -289,7 +295,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "FixFloorTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->fixFloorTabController;
+            QObject* obj = &( objectAddress->fixFloorTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -299,7 +305,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "AudioTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->audioTabController;
+            QObject* obj = &( objectAddress->audioTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -309,7 +315,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "StatisticsTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->statisticsTabController;
+            QObject* obj = &( objectAddress->statisticsTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -319,7 +325,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "SettingsTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->settingsTabController;
+            QObject* obj = &( objectAddress->settingsTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -329,7 +335,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "ReviveTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->reviveTabController;
+            QObject* obj = &( objectAddress->reviveTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -339,7 +345,7 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "UtilitiesTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->utilitiesTabController;
+            QObject* obj = &( objectAddress->utilitiesTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
@@ -349,12 +355,17 @@ void OverlayController::Init( QQmlEngine* qmlEngine )
         0,
         "AccessibilityTabController",
         []( QQmlEngine*, QJSEngine* ) {
-            QObject* obj = &getInstance()->accessibilityTabController;
+            QObject* obj = &( objectAddress->accessibilityTabController );
             QQmlEngine::setObjectOwnership( obj, QQmlEngine::CppOwnership );
             return obj;
         } );
 }
 
+OverlayController::~OverlayController()
+{
+    Shutdown();
+}
+
 void OverlayController::Shutdown()
 {
     if ( m_pPumpEventsTimer )

src/overlaycontroller.h

@@ -94,16 +94,12 @@ public: // I know it's an ugly hack to make them public to enable external
     AccessibilityTabController accessibilityTabController;
 
 private:
-    OverlayController( bool desktopMode, bool noSound )
-        : QObject(), desktopMode( desktopMode ), noSound( noSound )
-    {
-    }
     QPoint getMousePositionForEvent( vr::VREvent_Mouse_t mouse );
 
 public:
+    OverlayController( bool desktopMode, bool noSound, QQmlEngine& qmlEngine );
     virtual ~OverlayController();
 
-    void Init( QQmlEngine* qmlEngine );
     void Shutdown();
 
     bool isDashboardVisible()
@@ -174,21 +170,8 @@ signals:
 
 private:
     static QSettings* _appSettings;
-    static std::unique_ptr<OverlayController> singleton;
 
 public:
-    static OverlayController* getInstance()
-    {
-        return singleton.get();
-    }
-
-    static OverlayController* createInstance( bool desktopMode, bool noSound )
-    {
-        singleton.reset(
-            new advsettings::OverlayController( desktopMode, noSound ) );
-        return singleton.get();
-    }
-
     static QSettings* appSettings()
     {
         return _appSettings;