Revert "Revert "DolphinQt: Minor consistency fixes for the Settings window""

This reverts commit e376a985e4.

A beta build has been released now, so there's no reason to keep
it reverted anymore.

Source/Core/DolphinQt/CMakeLists.txt

@@ -287,6 +287,8 @@ add_executable(dolphin-emu
   Settings/AdvancedPane.h
   Settings/AudioPane.cpp
   Settings/AudioPane.h
+  Settings/BroadbandAdapterSettingsDialog.cpp
+  Settings/BroadbandAdapterSettingsDialog.h
   Settings/GameCubePane.cpp
   Settings/GameCubePane.h
   Settings/GeneralPane.cpp

Source/Core/DolphinQt/DolphinQt.vcxproj

@@ -179,6 +179,7 @@
     <ClCompile Include="Settings.cpp" />
     <ClCompile Include="Settings\AdvancedPane.cpp" />
     <ClCompile Include="Settings\AudioPane.cpp" />
+    <ClCompile Include="Settings\BroadbandAdapterSettingsDialog.cpp" />
     <ClCompile Include="Settings\GameCubePane.cpp" />
     <ClCompile Include="Settings\GeneralPane.cpp" />
     <ClCompile Include="Settings\InterfacePane.cpp" />
@@ -346,6 +347,7 @@
     <QtMoc Include="Settings.h" />
     <QtMoc Include="Settings\AdvancedPane.h" />
     <QtMoc Include="Settings\AudioPane.h" />
+    <QtMoc Include="Settings\BroadbandAdapterSettingsDialog.h" />
     <QtMoc Include="Settings\GameCubePane.h" />
     <QtMoc Include="Settings\GeneralPane.h" />
     <QtMoc Include="Settings\InterfacePane.h" />

Source/Core/DolphinQt/Settings/AdvancedPane.cpp

@@ -46,26 +46,27 @@ void AdvancedPane::CreateLayout()
   auto* main_layout = new QVBoxLayout();
   setLayout(main_layout);
 
-  auto* cpu_options = new QGroupBox(tr("CPU Options"));
-  auto* cpu_options_layout = new QVBoxLayout();
-  cpu_options->setLayout(cpu_options_layout);
-  main_layout->addWidget(cpu_options);
+  auto* cpu_options_group = new QGroupBox(tr("CPU Options"));
+  auto* cpu_options_group_layout = new QVBoxLayout();
+  cpu_options_group->setLayout(cpu_options_group_layout);
+  main_layout->addWidget(cpu_options_group);
+
+  auto* cpu_emulation_engine_layout = new QFormLayout;
+  cpu_emulation_engine_layout->setFormAlignment(Qt::AlignLeft | Qt::AlignTop);
+  cpu_emulation_engine_layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
+  cpu_options_group_layout->addLayout(cpu_emulation_engine_layout);
 
-  QGridLayout* cpu_emulation_layout = new QGridLayout();
-  QLabel* cpu_emulation_engine_label = new QLabel(tr("CPU Emulation Engine:"));
   m_cpu_emulation_engine_combobox = new QComboBox(this);
+  cpu_emulation_engine_layout->addRow(tr("CPU Emulation Engine:"), m_cpu_emulation_engine_combobox);
   for (PowerPC::CPUCore cpu_core : PowerPC::AvailableCPUCores())
   {
     m_cpu_emulation_engine_combobox->addItem(tr(CPU_CORE_NAMES.at(cpu_core)));
   }
-  cpu_emulation_layout->addWidget(cpu_emulation_engine_label, 0, 0);
-  cpu_emulation_layout->addWidget(m_cpu_emulation_engine_combobox, 0, 1, Qt::AlignLeft);
-  cpu_options_layout->addLayout(cpu_emulation_layout);
 
   m_enable_mmu_checkbox = new QCheckBox(tr("Enable MMU"));
   m_enable_mmu_checkbox->setToolTip(tr(
       "Enables the Memory Management Unit, needed for some games. (ON = Compatible, OFF = Fast)"));
-  cpu_options_layout->addWidget(m_enable_mmu_checkbox);
+  cpu_options_group_layout->addWidget(m_enable_mmu_checkbox);
 
   auto* clock_override = new QGroupBox(tr("Clock Override"));
   auto* clock_override_layout = new QVBoxLayout();

Source/Core/DolphinQt/Settings/AudioPane.cpp

@@ -87,7 +87,8 @@ void AudioPane::CreateWidgets()
     m_latency_spin = new QSpinBox();
     m_latency_spin->setMinimum(0);
     m_latency_spin->setMaximum(200);
-    m_latency_spin->setToolTip(tr("Sets the latency (in ms). Higher values may reduce audio "
+    m_latency_spin->setToolTip(
+        tr("Sets the latency in milliseconds. Higher values may reduce audio "
            "crackling. Certain backends only."));
   }
 

Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.cpp

@@ -0,0 +1,112 @@
+// Copyright 2021 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "DolphinQt/Settings/BroadbandAdapterSettingsDialog.h"
+
+#include <regex>
+#include <string>
+
+#include <QDialogButtonBox>
+#include <QLabel>
+#include <QLineEdit>
+#include <QString>
+#include <QVBoxLayout>
+
+#include "Common/StringUtil.h"
+#include "Core/ConfigManager.h"
+#include "DolphinQt/QtUtils/ModalMessageBox.h"
+
+BroadbandAdapterSettingsDialog::BroadbandAdapterSettingsDialog(QWidget* parent, Type bba_type)
+    : QDialog(parent)
+{
+  m_bba_type = bba_type;
+  InitControls();
+}
+
+void BroadbandAdapterSettingsDialog::InitControls()
+{
+  QLabel* address_label = nullptr;
+  QLabel* description = nullptr;
+  QString address_placeholder;
+  QString current_address;
+  QString window_title;
+
+  switch (m_bba_type)
+  {
+  case Type::Ethernet:
+    // i18n: MAC stands for Media Access Control. A MAC address uniquely identifies a network
+    // interface (physical) like a serial number. "MAC" should be kept in translations.
+    address_label = new QLabel(tr("Enter new Broadband Adapter MAC address:"));
+    address_placeholder = QString::fromStdString("aa:bb:cc:dd:ee:ff");
+    current_address = QString::fromStdString(SConfig::GetInstance().m_bba_mac);
+    description = new QLabel(tr("For setup instructions, <a "
+                                "href=\"https://wiki.dolphin-emu.org/"
+                                "index.php?title=Broadband_Adapter\">refer to this page</a>."));
+
+    // i18n: MAC stands for Media Access Control. A MAC address uniquely identifies a network
+    // interface (physical) like a serial number. "MAC" should be kept in translations.
+    window_title = tr("Broadband Adapter MAC Address");
+    break;
+
+  case Type::XLinkKai:
+    address_label = new QLabel(tr("Enter IP address of device running the XLink Kai Client:"));
+    address_placeholder = QString::fromStdString("127.0.0.1");
+    current_address = QString::fromStdString(SConfig::GetInstance().m_bba_xlink_ip);
+    description =
+        new QLabel(tr("For setup instructions, <a "
+                      "href=\"https://www.teamxlink.co.uk/wiki/Dolphin\">refer to this page</a>."));
+    window_title = tr("XLink Kai BBA Destination Address");
+    break;
+  }
+
+  setWindowTitle(window_title);
+  setWindowFlags(windowFlags() & ~Qt::WindowContextHelpButtonHint);
+
+  m_address_input = new QLineEdit(current_address);
+  m_address_input->setPlaceholderText(address_placeholder);
+
+  auto buttonbox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
+  connect(buttonbox, &QDialogButtonBox::accepted, this,
+          &BroadbandAdapterSettingsDialog::SaveAddress);
+  connect(buttonbox, &QDialogButtonBox::rejected, this, &BroadbandAdapterSettingsDialog::reject);
+
+  description->setTextFormat(Qt::RichText);
+  description->setWordWrap(true);
+  description->setTextInteractionFlags(Qt::TextBrowserInteraction);
+  description->setOpenExternalLinks(true);
+
+  auto* main_layout = new QVBoxLayout();
+  main_layout->addWidget(address_label);
+  main_layout->addWidget(m_address_input);
+  main_layout->addWidget(description);
+  main_layout->addWidget(buttonbox);
+
+  setLayout(main_layout);
+}
+
+void BroadbandAdapterSettingsDialog::SaveAddress()
+{
+  const std::string bba_new_address(StripSpaces(m_address_input->text().toStdString()));
+
+  switch (m_bba_type)
+  {
+  case Type::Ethernet:
+    if (!std::regex_match(bba_new_address, std::regex("([0-9A-Fa-f]{2}:){5}([0-9A-Fa-f]{2})")))
+    {
+      ModalMessageBox::critical(
+          this, tr("Broadband Adapter Error"),
+          // i18n: MAC stands for Media Access Control. A MAC address uniquely identifies a network
+          // interface (physical) like a serial number. "MAC" should be kept in translations.
+          tr("The entered MAC address is invalid."));
+      return;
+    }
+    SConfig::GetInstance().m_bba_mac = bba_new_address;
+    break;
+
+  case Type::XLinkKai:
+    SConfig::GetInstance().m_bba_xlink_ip = bba_new_address;
+    break;
+  }
+
+  accept();
+}

Source/Core/DolphinQt/Settings/BroadbandAdapterSettingsDialog.h

@@ -0,0 +1,28 @@
+// Copyright 2021 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include <QDialog>
+
+class QLineEdit;
+
+class BroadbandAdapterSettingsDialog final : public QDialog
+{
+  Q_OBJECT
+public:
+  enum class Type
+  {
+    Ethernet,
+    XLinkKai,
+  };
+
+  explicit BroadbandAdapterSettingsDialog(QWidget* target, Type bba_type);
+
+private:
+  QLineEdit* m_address_input;
+  Type m_bba_type;
+
+  void InitControls();
+  void SaveAddress();
+};

Source/Core/DolphinQt/Settings/GameCubePane.cpp

@@ -7,6 +7,7 @@
 #include <QComboBox>
 #include <QFileDialog>
 #include <QFileInfo>
+#include <QFormLayout>
 #include <QGridLayout>
 #include <QGroupBox>
 #include <QInputDialog>
@@ -33,6 +34,7 @@
 #include "DolphinQt/GCMemcardManager.h"
 #include "DolphinQt/QtUtils/ModalMessageBox.h"
 #include "DolphinQt/Settings.h"
+#include "DolphinQt/Settings/BroadbandAdapterSettingsDialog.h"
 
 enum
 {
@@ -55,12 +57,20 @@ void GameCubePane::CreateWidgets()
 
   // IPL Settings
   QGroupBox* ipl_box = new QGroupBox(tr("IPL Settings"), this);
-  QGridLayout* ipl_layout = new QGridLayout(ipl_box);
-  ipl_box->setLayout(ipl_layout);
+  QVBoxLayout* ipl_box_layout = new QVBoxLayout(ipl_box);
+  ipl_box->setLayout(ipl_box_layout);
 
   m_skip_main_menu = new QCheckBox(tr("Skip Main Menu"), ipl_box);
+  ipl_box_layout->addWidget(m_skip_main_menu);
+
+  QFormLayout* ipl_language_layout = new QFormLayout;
+  ipl_language_layout->setFormAlignment(Qt::AlignLeft | Qt::AlignTop);
+  ipl_language_layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
+  ipl_box_layout->addLayout(ipl_language_layout);
+
   m_language_combo = new QComboBox(ipl_box);
   m_language_combo->setCurrentIndex(-1);
+  ipl_language_layout->addRow(tr("System Language:"), m_language_combo);
 
   // Add languages
   for (const auto& entry : {std::make_pair(tr("English"), 0), std::make_pair(tr("German"), 1),
@@ -70,10 +80,6 @@ void GameCubePane::CreateWidgets()
     m_language_combo->addItem(entry.first, entry.second);
   }
 
-  ipl_layout->addWidget(m_skip_main_menu, 0, 0);
-  ipl_layout->addWidget(new QLabel(tr("System Language:")), 1, 0);
-  ipl_layout->addWidget(m_language_combo, 1, 1);
-
   // Device Settings
   QGroupBox* device_box = new QGroupBox(tr("Device Settings"), this);
   QGridLayout* device_layout = new QGridLayout(device_box);
@@ -82,12 +88,12 @@ void GameCubePane::CreateWidgets()
   for (int i = 0; i < SLOT_COUNT; i++)
   {
     m_slot_combos[i] = new QComboBox(device_box);
+    m_slot_combos[i]->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::Fixed);
     m_slot_buttons[i] = new QPushButton(tr("..."), device_box);
     m_slot_buttons[i]->setSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed);
   }
 
   // Add slot devices
-
   for (const auto& entry :
        {std::make_pair(tr("<Nothing>"), ExpansionInterface::EXIDEVICE_NONE),
         std::make_pair(tr("Dummy"), ExpansionInterface::EXIDEVICE_DUMMY),
@@ -102,7 +108,6 @@ void GameCubePane::CreateWidgets()
   }
 
   // Add SP1 devices
-
   std::vector<std::pair<QString, ExpansionInterface::TEXIDevices>> sp1Entries{
       std::make_pair(tr("<Nothing>"), ExpansionInterface::EXIDEVICE_NONE),
       std::make_pair(tr("Dummy"), ExpansionInterface::EXIDEVICE_DUMMY),
@@ -275,26 +280,12 @@ void GameCubePane::OnConfigPressed(int slot)
     return;
   case ExpansionInterface::EXIDEVICE_ETH:
   {
-    bool ok;
-    const auto new_mac = QInputDialog::getText(
-        // i18n: MAC stands for Media Access Control. A MAC address uniquely identifies a network
-        // interface (physical) like a serial number. "MAC" should be kept in translations.
-        this, tr("Broadband Adapter MAC address"), tr("Enter new Broadband Adapter MAC address:"),
-        QLineEdit::Normal, QString::fromStdString(SConfig::GetInstance().m_bba_mac), &ok);
-    if (ok)
-      SConfig::GetInstance().m_bba_mac = new_mac.toStdString();
+    BroadbandAdapterSettingsDialog(this, BroadbandAdapterSettingsDialog::Type::Ethernet).exec();
     return;
   }
   case ExpansionInterface::EXIDEVICE_ETHXLINK:
   {
-    bool ok;
-    const auto new_dest = QInputDialog::getText(
-        this, tr("Broadband Adapter (XLink Kai) Destination Address"),
-        tr("Enter IP address of device running the XLink Kai Client.\nFor more information see"
-           " https://www.teamxlink.co.uk/wiki/Dolphin"),
-        QLineEdit::Normal, QString::fromStdString(SConfig::GetInstance().m_bba_xlink_ip), &ok);
-    if (ok)
-      SConfig::GetInstance().m_bba_xlink_ip = new_dest.toStdString();
+    BroadbandAdapterSettingsDialog(this, BroadbandAdapterSettingsDialog::Type::XLinkKai).exec();
     return;
   }
   default:
@@ -460,11 +451,9 @@ void GameCubePane::LoadSettings()
   }
 
   m_skip_main_menu->setEnabled(have_menu);
-  m_skip_main_menu->setToolTip(have_menu ? QString{} :
-                                           tr("Put Main Menu roms in User/GC/{region}."));
+  m_skip_main_menu->setToolTip(have_menu ? QString{} : tr("Put IPL ROMs in User/GC/<region>."));
 
   // Device Settings
-
   for (int i = 0; i < SLOT_COUNT; i++)
   {
     QSignalBlocker blocker(m_slot_combos[i]);

Source/Core/DolphinQt/Settings/GeneralPane.cpp

@@ -175,16 +175,16 @@ void GeneralPane::CreateBasic()
 void GeneralPane::CreateAutoUpdate()
 {
   auto* auto_update_group = new QGroupBox(tr("Auto Update Settings"));
-  auto* layout = new QFormLayout;
-  auto_update_group->setLayout(layout);
+  auto* auto_update_group_layout = new QFormLayout;
+  auto_update_group->setLayout(auto_update_group_layout);
   m_main_layout->addWidget(auto_update_group);
 
-  layout->setFormAlignment(Qt::AlignLeft | Qt::AlignTop);
-  layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
+  auto_update_group_layout->setFormAlignment(Qt::AlignLeft | Qt::AlignTop);
+  auto_update_group_layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
 
   m_combobox_update_track = new QComboBox(this);
 
-  layout->addRow(tr("&Auto Update:"), m_combobox_update_track);
+  auto_update_group_layout->addRow(tr("&Auto Update:"), m_combobox_update_track);
 
   for (const QString& option : {tr("Don't Update"), tr("Stable (once a year)"),
                                 tr("Beta (once a month)"), tr("Dev (multiple times a day)")})
@@ -194,28 +194,26 @@ void GeneralPane::CreateAutoUpdate()
 void GeneralPane::CreateFallbackRegion()
 {
   auto* fallback_region_group = new QGroupBox(tr("Fallback Region"));
-  auto* layout = new QVBoxLayout;
-  fallback_region_group->setLayout(layout);
+  auto* fallback_region_group_layout = new QVBoxLayout;
+  fallback_region_group->setLayout(fallback_region_group_layout);
   m_main_layout->addWidget(fallback_region_group);
 
-  m_combobox_fallback_region = new QComboBox(this);
+  auto* fallback_region_dropdown_layout = new QFormLayout;
+  fallback_region_dropdown_layout->setFormAlignment(Qt::AlignLeft | Qt::AlignTop);
+  fallback_region_dropdown_layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
+  fallback_region_group_layout->addLayout(fallback_region_dropdown_layout);
 
-  auto* form_widget = new QWidget;
-  auto* form_layout = new QFormLayout;
-  form_widget->setLayout(form_layout);
-  form_layout->setAlignment(Qt::AlignLeft | Qt::AlignTop);
-  form_layout->setFieldGrowthPolicy(QFormLayout::AllNonFixedFieldsGrow);
-  form_layout->addRow(tr("Fallback Region:"), m_combobox_fallback_region);
-  layout->addWidget(form_widget);
+  m_combobox_fallback_region = new QComboBox(this);
+  fallback_region_dropdown_layout->addRow(tr("Fallback Region:"), m_combobox_fallback_region);
+
+  for (const QString& option : {tr("NTSC-J"), tr("NTSC-U"), tr("PAL"), tr("NTSC-K")})
+    m_combobox_fallback_region->addItem(option);
 
   auto* fallback_region_description =
       new QLabel(tr("Dolphin will use this for titles whose region cannot be determined "
                     "automatically."));
   fallback_region_description->setWordWrap(true);
-  layout->addWidget(fallback_region_description);
-
-  for (const QString& option : {tr("NTSC-J"), tr("NTSC-U"), tr("PAL"), tr("NTSC-K")})
-    m_combobox_fallback_region->addItem(option);
+  fallback_region_group_layout->addWidget(fallback_region_description);
 }
 
 #if defined(USE_ANALYTICS) && USE_ANALYTICS

Source/Core/DolphinQt/Settings/USBDeviceAddToWhitelistDialog.cpp

@@ -128,25 +128,13 @@ void USBDeviceAddToWhitelistDialog::AddUSBDeviceToWhitelist()
   if (!IsValidUSBIDString(vid_string))
   {
     // i18n: Here, VID means Vendor ID (for a USB device).
-    ModalMessageBox vid_warning_box(this);
-    vid_warning_box.setIcon(QMessageBox::Warning);
-    vid_warning_box.setWindowTitle(tr("USB Whitelist Error"));
-    // i18n: Here, VID means Vendor ID (for a USB device).
-    vid_warning_box.setText(tr("The entered VID is invalid."));
-    vid_warning_box.setStandardButtons(QMessageBox::Ok);
-    vid_warning_box.exec();
+    ModalMessageBox::critical(this, tr("USB Whitelist Error"), tr("The entered VID is invalid."));
     return;
   }
   if (!IsValidUSBIDString(pid_string))
   {
     // i18n: Here, PID means Product ID (for a USB device).
-    ModalMessageBox pid_warning_box(this);
-    pid_warning_box.setIcon(QMessageBox::Warning);
-    pid_warning_box.setWindowTitle(tr("USB Whitelist Error"));
-    // i18n: Here, PID means Product ID (for a USB device).
-    pid_warning_box.setText(tr("The entered PID is invalid."));
-    pid_warning_box.setStandardButtons(QMessageBox::Ok);
-    pid_warning_box.exec();
+    ModalMessageBox::critical(this, tr("USB Whitelist Error"), tr("The entered PID is invalid."));
     return;
   }
 
@@ -155,7 +143,8 @@ void USBDeviceAddToWhitelistDialog::AddUSBDeviceToWhitelist()
 
   if (SConfig::GetInstance().IsUSBDeviceWhitelisted({vid, pid}))
   {
-    ModalMessageBox::critical(this, tr("Error"), tr("This USB device is already whitelisted."));
+    ModalMessageBox::critical(this, tr("USB Whitelist Error"),
+                              tr("This USB device is already whitelisted."));
     return;
   }
   SConfig::GetInstance().m_usb_passthrough_devices.emplace(vid, pid);

Source/Core/DolphinQt/Settings/WiiPane.cpp

@@ -179,7 +179,7 @@ void WiiPane::CreateWiiRemoteSettings()
   auto* wii_remote_settings_group_layout = new QGridLayout();
   wii_remote_settings_group->setLayout(wii_remote_settings_group_layout);
   m_main_layout->addWidget(wii_remote_settings_group);
-  m_wiimote_motor = new QCheckBox(tr("Wii Remote Rumble"));
+  m_wiimote_motor = new QCheckBox(tr("Enable Rumble"));
 
   m_wiimote_sensor_position_label = new QLabel(tr("Sensor Bar Position:"));
   m_wiimote_ir_sensor_position = new QComboBox();