Implementing review: Move _controller pointer to SDLRenderingWindow class

Author: nuess0r

src/RenderLoop.cpp

@@ -30,8 +30,6 @@ void RenderLoop::Run()
 
     _projectMWrapper.DisplayInitialPreset();
 
-    _controller = _sdlRenderingWindow.FindController();
-
     while (!_wantsToQuit)
     {
         limiter.TargetFPS(_projectMWrapper.TargetFPS());
@@ -51,10 +49,6 @@ void RenderLoop::Run()
         _projectMWrapper.UpdateRealFPS(limiter.FPS());
     }
 
-    //Close game controller
-    SDL_GameControllerClose(_controller);
-    _controller = nullptr;
-
     notificationCenter.removeObserver(_quitNotificationObserver);
 
     projectm_playlist_set_preset_switched_event_callback(_playlistHandle, nullptr, nullptr);
@@ -110,12 +104,14 @@ void RenderLoop::PollEvents()
                 break;
 
             case SDL_CONTROLLERDEVICEADDED:
-                ControllerAdd(event.cdevice.which);
+                poco_debug(_logger, "Controller added event received");
+                _sdlRenderingWindow.ControllerAdd(event.cdevice.which);
 
                 break;
 
             case SDL_CONTROLLERDEVICEREMOVED:
-                ControllerRemove(event.cdevice.which);
+                poco_debug(_logger, "Controller remove event received");
+                _sdlRenderingWindow.ControllerRemove(event.cdevice.which);
 
                 break;
 
@@ -350,30 +346,10 @@ void RenderLoop::MouseUpEvent(const SDL_MouseButtonEvent& event)
     }
 }
 
-void RenderLoop::ControllerAdd(const int id )
-{
-    if (!_controller)
-    {
-        _controller = SDL_GameControllerOpen(id);
-    }
-    poco_debug(_logger, "Controller added!");
-}
-
-void RenderLoop::ControllerRemove(const int id )
-{
-    if (_controller && id == SDL_JoystickInstanceID(SDL_GameControllerGetJoystick(_controller)))
-    {
-        SDL_GameControllerClose(_controller);
-        _controller = _sdlRenderingWindow.FindController();
-    }
-    poco_debug(_logger, "Controller removed!");
-}
-
 void RenderLoop::ControllerDownEvent(const SDL_Event& event)
 {
-    if (!_controller || event.cdevice.which != SDL_JoystickInstanceID(SDL_GameControllerGetJoystick(_controller)))
+    if (!_sdlRenderingWindow.ControllerIsOurs(event.cdevice.which) )
     {
-        poco_debug(_logger, "No controller initialized");
         return;
     }
 

src/RenderLoop.h

@@ -61,18 +61,6 @@ class RenderLoop
      */
     void MouseUpEvent(const SDL_MouseButtonEvent& event);
 
-    /**
-     * @brief Handles SDL game controller add events (plugin in a new controller) events.
-     * @param id The added controller id
-     */
-    void ControllerAdd( int id );
-
-    /**
-     * @brief Handles SDL game controller remove events.
-     * @param id The removed controller id
-     */
-    void ControllerRemove( int id );
-
     /**
      * @brief Handles SDL game controller button down events.
      * @param event The controller button event
@@ -112,6 +100,4 @@ class RenderLoop
     ModifierKeyStates _keyStates; //!< Current "pressed" states of modifier keys
 
     Poco::Logger& _logger{Poco::Logger::get("RenderLoop")}; //!< The class logger.
-
-    SDL_GameController *_controller;
 };

src/SDLRenderingWindow.cpp

@@ -35,6 +35,8 @@ void SDLRenderingWindow::initialize(Poco::Util::Application& app)
     // Observe user configuration changes (set via the settings window)
     _userConfig->propertyChanged += Poco::delegate(this, &SDLRenderingWindow::OnConfigurationPropertyChanged);
     _userConfig->propertyRemoved += Poco::delegate(this, &SDLRenderingWindow::OnConfigurationPropertyRemoved);
+
+    _controller = FindController();
 }
 
 void SDLRenderingWindow::uninitialize()
@@ -48,6 +50,12 @@ void SDLRenderingWindow::uninitialize()
         DestroySDLWindow();
         _renderingWindow = nullptr;
     }
+
+    if (_controller)
+    {
+        SDL_GameControllerClose(_controller);
+        _controller = nullptr;
+    }
 }
 
 void SDLRenderingWindow::GetDrawableSize(int& width, int& height) const
@@ -467,15 +475,63 @@ SDL_GameController* SDLRenderingWindow::FindController() {
     if( SDL_NumJoysticks() < 1 )
     {
         poco_debug(_logger, "No joysticks connected");
+        return nullptr;
     }
 
     //For simplicity, we’ll only be setting up and tracking a single
     //controller at a time
     for (int i = 0; i < SDL_NumJoysticks(); i++) {
         if (SDL_IsGameController(i)) {
+            poco_debug(_logger, "Adding first controller");
             return SDL_GameControllerOpen(i);
         }
+        else {
+            poco_debug(_logger, "Connected joystick is not a SDL game controller");
+        }
     }
 
     return nullptr;
 }
+
+void SDLRenderingWindow::ControllerAdd(const int id )
+{
+    if (!_controller)
+    {
+        if (SDL_IsGameController(id)) {
+            _controller = SDL_GameControllerOpen(id);
+            poco_debug(_logger, "Controller added!");
+        }
+        else {
+            poco_debug(_logger, "Connected joystick is not a SDL game controller");
+        }
+    }
+}
+
+void SDLRenderingWindow::ControllerRemove(const int id )
+{
+    if (_controller && id == SDL_JoystickInstanceID(SDL_GameControllerGetJoystick(_controller)))
+    {
+        SDL_GameControllerClose(_controller);
+        poco_debug(_logger, "Controller removed!");
+        _controller = FindController();
+    }
+}
+
+bool SDLRenderingWindow::ControllerIsOurs(const int id )
+{
+    int instance = SDL_JoystickInstanceID(SDL_GameControllerGetJoystick(_controller));
+
+    if (!_controller )
+    {
+        poco_debug(_logger, "No controller initialized");
+        return false;
+    }
+
+    if (id != instance)
+    {
+        poco_debug_f2(_logger, "Use controller %?d instead of %?d. Currently only one controller is supported", instance, id);
+        return false;
+    }
+
+    return true;
+}

src/SDLRenderingWindow.h

@@ -102,6 +102,24 @@ class SDLRenderingWindow : public Poco::Util::Subsystem
      */
     SDL_GameController* FindController();
 
+    /**
+     * @brief Handles SDL game controller add events (plugin in a new controller) events.
+     * @param id The added controller id
+     */
+    void ControllerAdd(const int id );
+
+    /**
+     * @brief Handles SDL game controller remove events.
+     * @param id The removed controller id
+     */
+    void ControllerRemove(const int id );
+
+    /**
+     * @brief Returns true if the given controller is initialized and the one we currently use.
+     * @param id The removed controller id
+     */
+    bool ControllerIsOurs(const int id );
+
 protected:
 
     /**
@@ -162,6 +180,7 @@ class SDLRenderingWindow : public Poco::Util::Subsystem
 
     bool _fullscreen{ false };
 
+    SDL_GameController *_controller;
 };