Merge pull request #11803 from flodavid/improve-controller-applet-click

yuzu: Improve behavior when clicking on controller box in Controller applet

src/yuzu/applets/qt_controller.cpp

@@ -155,18 +155,27 @@ QtControllerSelectorDialog::QtControllerSelectorDialog(
         UpdateBorderColor(i);
 
         connect(player_groupboxes[i], &QGroupBox::toggled, [this, i](bool checked) {
-            if (checked) {
-                // Hide eventual error message about number of controllers
-                ui->labelError->setVisible(false);
-                for (std::size_t index = 0; index <= i; ++index) {
-                    connected_controller_checkboxes[index]->setChecked(checked);
-                }
-            } else {
-                for (std::size_t index = i; index < NUM_PLAYERS; ++index) {
-                    connected_controller_checkboxes[index]->setChecked(checked);
-                }
+            // Reconnect current controller if it was the last one checked
+            // (player number was reduced by more than one)
+            const bool reconnect_first = !checked && i < player_groupboxes.size() - 1 &&
+                                         player_groupboxes[i + 1]->isChecked();
+
+            // Ensures that connecting a controller changes the number of players
+            if (connected_controller_checkboxes[i]->isChecked() != checked) {
+                // Ensures that the players are always connected in sequential order
+                PropagatePlayerNumberChanged(i, checked, reconnect_first);
             }
         });
+        connect(connected_controller_checkboxes[i], &QCheckBox::clicked, [this, i](bool checked) {
+            // Reconnect current controller if it was the last one checked
+            // (player number was reduced by more than one)
+            const bool reconnect_first = !checked &&
+                                         i < connected_controller_checkboxes.size() - 1 &&
+                                         connected_controller_checkboxes[i + 1]->isChecked();
+
+            // Ensures that the players are always connected in sequential order
+            PropagatePlayerNumberChanged(i, checked, reconnect_first);
+        });
 
         connect(emulated_controllers[i], qOverload<int>(&QComboBox::currentIndexChanged),
                 [this, i](int) {
@@ -668,6 +677,29 @@ void QtControllerSelectorDialog::UpdateDockedState(bool is_handheld) {
     }
 }
 
+void QtControllerSelectorDialog::PropagatePlayerNumberChanged(size_t player_index, bool checked,
+                                                              bool reconnect_current) {
+    connected_controller_checkboxes[player_index]->setChecked(checked);
+    // Hide eventual error message about number of controllers
+    ui->labelError->setVisible(false);
+
+    if (checked) {
+        // Check all previous buttons when checked
+        if (player_index > 0) {
+            PropagatePlayerNumberChanged(player_index - 1, checked);
+        }
+    } else {
+        // Unchecked all following buttons when unchecked
+        if (player_index < connected_controller_checkboxes.size() - 1) {
+            PropagatePlayerNumberChanged(player_index + 1, checked);
+        }
+    }
+
+    if (reconnect_current) {
+        connected_controller_checkboxes[player_index]->setCheckState(Qt::Checked);
+    }
+}
+
 void QtControllerSelectorDialog::DisableUnsupportedPlayers() {
     const auto max_supported_players = parameters.enable_single_mode ? 1 : parameters.max_players;
 

src/yuzu/applets/qt_controller.h

@@ -100,6 +100,10 @@ private:
     // Updates the console mode.
     void UpdateDockedState(bool is_handheld);
 
+    // Enable preceding controllers or disable following ones
+    void PropagatePlayerNumberChanged(size_t player_index, bool checked,
+                                      bool reconnect_current = false);
+
     // Disables and disconnects unsupported players based on the given parameters.
     void DisableUnsupportedPlayers();
 

src/yuzu/configuration/configure_input.cpp

@@ -101,13 +101,13 @@ void ConfigureInput::Initialize(InputCommon::InputSubsystem* input_subsystem,
         ui->tabPlayer5, ui->tabPlayer6, ui->tabPlayer7, ui->tabPlayer8,
     };
 
-    player_connected = {
+    connected_controller_checkboxes = {
         ui->checkboxPlayer1Connected, ui->checkboxPlayer2Connected, ui->checkboxPlayer3Connected,
         ui->checkboxPlayer4Connected, ui->checkboxPlayer5Connected, ui->checkboxPlayer6Connected,
         ui->checkboxPlayer7Connected, ui->checkboxPlayer8Connected,
     };
 
-    std::array<QLabel*, 8> player_connected_labels = {
+    std::array<QLabel*, 8> connected_controller_labels = {
         ui->label,   ui->label_3, ui->label_4, ui->label_5,
         ui->label_6, ui->label_7, ui->label_8, ui->label_9,
     };
@@ -115,23 +115,37 @@ void ConfigureInput::Initialize(InputCommon::InputSubsystem* input_subsystem,
     for (std::size_t i = 0; i < player_tabs.size(); ++i) {
         player_tabs[i]->setLayout(new QHBoxLayout(player_tabs[i]));
         player_tabs[i]->layout()->addWidget(player_controllers[i]);
-        connect(player_connected[i], &QCheckBox::clicked, [this, i](int checked) {
-            // Ensures that the controllers are always connected in sequential order
-            this->propagateMouseClickOnPlayers(i, checked, true);
+        connect(player_controllers[i], &ConfigureInputPlayer::Connected, [this, i](bool checked) {
+            // Ensures that connecting a controller changes the number of players
+            if (connected_controller_checkboxes[i]->isChecked() != checked) {
+                // Ensures that the players are always connected in sequential order
+                PropagatePlayerNumberChanged(i, checked);
+            }
+        });
+        connect(connected_controller_checkboxes[i], &QCheckBox::clicked, [this, i](bool checked) {
+            // Reconnect current controller if it was the last one checked
+            // (player number was reduced by more than one)
+            const bool reconnect_first = !checked &&
+                                         i < connected_controller_checkboxes.size() - 1 &&
+                                         connected_controller_checkboxes[i + 1]->isChecked();
+
+            // Ensures that the players are always connected in sequential order
+            PropagatePlayerNumberChanged(i, checked, reconnect_first);
         });
         connect(player_controllers[i], &ConfigureInputPlayer::RefreshInputDevices, this,
                 &ConfigureInput::UpdateAllInputDevices);
         connect(player_controllers[i], &ConfigureInputPlayer::RefreshInputProfiles, this,
                 &ConfigureInput::UpdateAllInputProfiles, Qt::QueuedConnection);
-        connect(player_connected[i], &QCheckBox::stateChanged, [this, i](int state) {
+        connect(connected_controller_checkboxes[i], &QCheckBox::stateChanged, [this, i](int state) {
+            // Keep activated controllers synced with the "Connected Controllers" checkboxes
             player_controllers[i]->ConnectPlayer(state == Qt::Checked);
         });
 
         // Remove/hide all the elements that exceed max_players, if applicable.
         if (i >= max_players) {
             ui->tabWidget->removeTab(static_cast<int>(max_players));
-            player_connected[i]->hide();
-            player_connected_labels[i]->hide();
+            connected_controller_checkboxes[i]->hide();
+            connected_controller_labels[i]->hide();
         }
     }
     // Only the first player can choose handheld mode so connect the signal just to player 1
@@ -175,28 +189,25 @@ void ConfigureInput::Initialize(InputCommon::InputSubsystem* input_subsystem,
     LoadConfiguration();
 }
 
-void ConfigureInput::propagateMouseClickOnPlayers(size_t player_index, bool checked, bool origin) {
-    // Origin has already been toggled
-    if (!origin) {
-        player_connected[player_index]->setChecked(checked);
-    }
+void ConfigureInput::PropagatePlayerNumberChanged(size_t player_index, bool checked,
+                                                  bool reconnect_current) {
+    connected_controller_checkboxes[player_index]->setChecked(checked);
 
     if (checked) {
         // Check all previous buttons when checked
         if (player_index > 0) {
-            propagateMouseClickOnPlayers(player_index - 1, checked, false);
+            PropagatePlayerNumberChanged(player_index - 1, checked);
         }
     } else {
         // Unchecked all following buttons when unchecked
-        if (player_index < player_tabs.size() - 1) {
-            // Reconnect current player if it was the last one checked
-            // (player number was reduced by more than one)
-            if (origin && player_connected[player_index + 1]->checkState() == Qt::Checked) {
-                player_connected[player_index]->setCheckState(Qt::Checked);
-            }
-            propagateMouseClickOnPlayers(player_index + 1, checked, false);
+        if (player_index < connected_controller_checkboxes.size() - 1) {
+            PropagatePlayerNumberChanged(player_index + 1, checked);
         }
     }
+
+    if (reconnect_current) {
+        connected_controller_checkboxes[player_index]->setCheckState(Qt::Checked);
+    }
 }
 
 QList<QWidget*> ConfigureInput::GetSubTabs() const {
@@ -249,17 +260,17 @@ void ConfigureInput::LoadConfiguration() {
 }
 
 void ConfigureInput::LoadPlayerControllerIndices() {
-    for (std::size_t i = 0; i < player_connected.size(); ++i) {
+    for (std::size_t i = 0; i < connected_controller_checkboxes.size(); ++i) {
         if (i == 0) {
             auto* handheld =
                 system.HIDCore().GetEmulatedController(Core::HID::NpadIdType::Handheld);
             if (handheld->IsConnected()) {
-                player_connected[i]->setChecked(true);
+                connected_controller_checkboxes[i]->setChecked(true);
                 continue;
             }
         }
         const auto* controller = system.HIDCore().GetEmulatedControllerByIndex(i);
-        player_connected[i]->setChecked(controller->IsConnected());
+        connected_controller_checkboxes[i]->setChecked(controller->IsConnected());
     }
 }
 

src/yuzu/configuration/configure_input.h

@@ -56,7 +56,9 @@ private:
     void UpdateDockedState(bool is_handheld);
     void UpdateAllInputDevices();
     void UpdateAllInputProfiles(std::size_t player_index);
-    void propagateMouseClickOnPlayers(size_t player_index, bool origin, bool checked);
+    // Enable preceding controllers or disable following ones
+    void PropagatePlayerNumberChanged(size_t player_index, bool checked,
+                                      bool reconnect_current = false);
 
     /// Load configuration settings.
     void LoadConfiguration();
@@ -71,7 +73,8 @@ private:
 
     std::array<ConfigureInputPlayer*, 8> player_controllers;
     std::array<QWidget*, 8> player_tabs;
-    std::array<QCheckBox*, 8> player_connected;
+    // Checkboxes representing the "Connected Controllers".
+    std::array<QCheckBox*, 8> connected_controller_checkboxes;
     ConfigureInputAdvanced* advanced;
 
     Core::System& system;

src/yuzu/configuration/configure_input_player.h

@@ -75,7 +75,7 @@ public:
     void ClearAll();
 
 signals:
-    /// Emitted when this controller is connected by the user.
+    /// Emitted when this controller is (dis)connected by the user.
     void Connected(bool connected);
     /// Emitted when the Handheld mode is selected (undocked with dual joycons attached).
     void HandheldStateChanged(bool is_handheld);
@@ -183,9 +183,6 @@ private:
     /// Stores a pair of "Connected Controllers" combobox index and Controller Type enum.
     std::vector<std::pair<int, Core::HID::NpadStyleIndex>> index_controller_type_pairs;
 
-    static constexpr int PLAYER_COUNT = 8;
-    std::array<QCheckBox*, PLAYER_COUNT> player_connected_checkbox;
-
     /// This will be the the setting function when an input is awaiting configuration.
     std::optional<std::function<void(const Common::ParamPackage&)>> input_setter;