From 781d4b787a02552f4ef03059dd3d68c1a4f59c51 Mon Sep 17 00:00:00 2001 From: fearlessTobi Date: Fri, 15 Feb 2019 19:20:06 +0100 Subject: [PATCH] Address first batch of review comments --- src/citra_qt/applets/mii_selector.cpp | 92 +++++++------------ src/citra_qt/applets/mii_selector.h | 8 +- src/citra_qt/applets/swkbd.cpp | 16 ++-- src/citra_qt/applets/swkbd.h | 2 +- src/citra_qt/main.cpp | 4 +- src/core/file_sys/archive_backend.h | 3 + src/core/frontend/applets/default_applets.cpp | 5 +- src/core/frontend/applets/mii_selector.cpp | 11 +-- src/core/frontend/applets/mii_selector.h | 20 ++-- src/core/frontend/applets/swkbd.cpp | 10 +- src/core/frontend/applets/swkbd.h | 21 ++--- src/core/hle/applets/mii_selector.cpp | 16 ++-- src/core/hle/applets/swkbd.cpp | 12 +-- src/core/hle/service/ptm/ptm.h | 2 +- 14 files changed, 91 insertions(+), 131 deletions(-) diff --git a/src/citra_qt/applets/mii_selector.cpp b/src/citra_qt/applets/mii_selector.cpp index 3b905eb33b..811804ea5f 100644 --- a/src/citra_qt/applets/mii_selector.cpp +++ b/src/citra_qt/applets/mii_selector.cpp @@ -11,66 +11,58 @@ #include "common/file_util.h" #include "core/file_sys/archive_extsavedata.h" #include "core/file_sys/file_backend.h" -#include "core/hle/applets/buttons.h" #include "core/hle/service/ptm/ptm.h" QtMiiSelectorDialog::QtMiiSelectorDialog(QWidget* parent, QtMiiSelector* mii_selector_) : QDialog(parent), mii_selector(mii_selector_) { - Frontend::MiiSelectorConfig config = mii_selector->config; + using namespace Frontend; + const auto config = mii_selector->config; layout = new QVBoxLayout; combobox = new QComboBox; buttons = new QDialogButtonBox; // Initialize buttons - buttons->addButton(tr(AppletButton::Ok), QDialogButtonBox::ButtonRole::AcceptRole); + buttons->addButton(tr(MII_BUTTON_OKAY), QDialogButtonBox::ButtonRole::AcceptRole); if (config.enable_cancel_button) { - buttons->addButton(tr(AppletButton::Cancel), QDialogButtonBox::ButtonRole::RejectRole); + buttons->addButton(tr(MII_BUTTON_CANCEL), QDialogButtonBox::ButtonRole::RejectRole); } setWindowTitle(config.title.empty() ? tr("Mii Selector") : QString::fromStdU16String(config.title)); + miis.push_back(HLE::Applets::MiiSelector::GetStandardMiiResult().selected_mii_data); + combobox->addItem(tr("Standard Mii")); + std::string nand_directory{FileUtil::GetUserPath(FileUtil::UserPath::NANDDir)}; FileSys::ArchiveFactory_ExtSaveData extdata_archive_factory(nand_directory, true); auto archive_result = extdata_archive_factory.Open(Service::PTM::ptm_shared_extdata_id); - if (!archive_result.Succeeded()) { - ShowNoMiis(); - return; - } + if (archive_result.Succeeded()) { + auto archive = std::move(archive_result).Unwrap(); - auto archive = std::move(archive_result).Unwrap(); + FileSys::Path file_path = "/CFL_DB.dat"; + FileSys::Mode mode{}; + mode.read_flag.Assign(1); - FileSys::Path file_path = "/CFL_DB.dat"; - FileSys::Mode mode{}; - mode.read_flag.Assign(1); + auto file_result = archive->OpenFile(file_path, mode); + if (file_result.Succeeded()) { + auto file = std::move(file_result).Unwrap(); - auto file_result = archive->OpenFile(file_path, mode); - if (!file_result.Succeeded()) { - ShowNoMiis(); - return; - } - - auto file = std::move(file_result).Unwrap(); - - u32 saved_miis_offset = 0x8; - // The Mii Maker has a 100 Mii limit on the 3ds - for (int i = 0; i < 100; ++i) { - HLE::Applets::MiiData mii; - std::array mii_raw; - file->Read(saved_miis_offset, sizeof(mii), mii_raw.data()); - std::memcpy(&mii, mii_raw.data(), sizeof(mii)); - if (mii.mii_id != 0) { - std::u16string name(sizeof(mii.mii_name), '\0'); - std::memcpy(&name[0], mii.mii_name.data(), sizeof(mii.mii_name)); - miis.emplace(combobox->count(), mii); - combobox->addItem(QString::fromStdU16String(name)); + u32 saved_miis_offset = 0x8; + // The Mii Maker has a 100 Mii limit on the 3ds + for (int i = 0; i < 100; ++i) { + HLE::Applets::MiiData mii; + std::array mii_raw; + file->Read(saved_miis_offset, sizeof(mii), mii_raw.data()); + std::memcpy(&mii, mii_raw.data(), sizeof(mii)); + if (mii.mii_id != 0) { + std::u16string name(sizeof(mii.mii_name), '\0'); + std::memcpy(name.data(), mii.mii_name.data(), sizeof(mii.mii_name)); + miis.push_back(mii); + combobox->addItem(QString::fromStdU16String(name)); + } + saved_miis_offset += sizeof(mii); + } } - saved_miis_offset += sizeof(mii); - } - - if (miis.empty()) { - ShowNoMiis(); - return; } if (combobox->count() > static_cast(config.initially_selected_mii_index)) { @@ -87,22 +79,9 @@ QtMiiSelectorDialog::QtMiiSelectorDialog(QWidget* parent, QtMiiSelector* mii_sel setLayout(layout); } -void QtMiiSelectorDialog::ShowNoMiis() { - Frontend::MiiSelectorConfig config = mii_selector->config; - QMessageBox::StandardButton answer = QMessageBox::question( - nullptr, - config.title.empty() ? tr("Mii Selector") : QString::fromStdU16String(config.title), - tr("You don't have any Miis.\nUse standard Mii?"), QMessageBox::Yes | QMessageBox::No); - - if (answer == QMessageBox::No) - return_code = 1; - - QMetaObject::invokeMethod(this, "close", Qt::QueuedConnection); -} - QtMiiSelector::QtMiiSelector(QWidget& parent_) : parent(parent_) {} -void QtMiiSelector::Setup(const Frontend::MiiSelectorConfig* config) { +void QtMiiSelector::Setup(const Frontend::MiiSelectorConfig& config) { MiiSelector::Setup(config); QMetaObject::invokeMethod(this, "OpenDialog", Qt::BlockingQueuedConnection); } @@ -114,12 +93,11 @@ void QtMiiSelector::OpenDialog() { dialog.setWindowModality(Qt::WindowModal); dialog.exec(); - int index = dialog.combobox->currentIndex(); + const auto index = dialog.combobox->currentIndex(); LOG_INFO(Frontend, "Mii Selector dialog finished (return_code={}, index={})", dialog.return_code, index); - HLE::Applets::MiiData mii_data = - index == -1 ? HLE::Applets::MiiSelector::GetStandardMiiResult().selected_mii_data - : dialog.miis.at(dialog.combobox->currentIndex()); - Finalize(dialog.return_code, dialog.return_code == 0 ? mii_data : HLE::Applets::MiiData{}); + const auto mii_data = dialog.miis.at(index); + Finalize(dialog.return_code, + dialog.return_code == 0 ? std::move(mii_data) : HLE::Applets::MiiData{}); } diff --git a/src/citra_qt/applets/mii_selector.h b/src/citra_qt/applets/mii_selector.h index 3135b35fb8..c3b6eabb25 100644 --- a/src/citra_qt/applets/mii_selector.h +++ b/src/citra_qt/applets/mii_selector.h @@ -8,8 +8,8 @@ #include #include "core/frontend/applets/mii_selector.h" -class QDialogButtonBox; class QComboBox; +class QDialogButtonBox; class QVBoxLayout; class QtMiiSelector; @@ -20,14 +20,12 @@ public: QtMiiSelectorDialog(QWidget* parent, QtMiiSelector* mii_selector_); private: - void ShowNoMiis(); - QDialogButtonBox* buttons; QComboBox* combobox; QVBoxLayout* layout; QtMiiSelector* mii_selector; u32 return_code = 0; - std::unordered_map miis; + std::vector miis; friend class QtMiiSelector; }; @@ -37,7 +35,7 @@ class QtMiiSelector final : public QObject, public Frontend::MiiSelector { public: explicit QtMiiSelector(QWidget& parent); - void Setup(const Frontend::MiiSelectorConfig* config) override; + void Setup(const Frontend::MiiSelectorConfig& config) override; private: Q_INVOKABLE void OpenDialog(); diff --git a/src/citra_qt/applets/swkbd.cpp b/src/citra_qt/applets/swkbd.cpp index a9237389a6..7514524b3e 100644 --- a/src/citra_qt/applets/swkbd.cpp +++ b/src/citra_qt/applets/swkbd.cpp @@ -25,7 +25,7 @@ QtKeyboardValidator::State QtKeyboardValidator::validate(QString& input, int& po QtKeyboardDialog::QtKeyboardDialog(QWidget* parent, QtKeyboard* keyboard_) : QDialog(parent), keyboard(keyboard_) { using namespace Frontend; - KeyboardConfig config = keyboard->config; + const auto config = keyboard->config; layout = new QVBoxLayout; label = new QLabel(QString::fromStdString(config.hint_text)); line_edit = new QLineEdit; @@ -36,31 +36,31 @@ QtKeyboardDialog::QtKeyboardDialog(QWidget* parent, QtKeyboard* keyboard_) case ButtonConfig::Triple: buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[2]) - : tr(BUTTON_OKAY), + : tr(SWKBD_BUTTON_OKAY), QDialogButtonBox::ButtonRole::AcceptRole); buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[1]) - : tr(BUTTON_FORGOT), + : tr(SWKBD_BUTTON_FORGOT), QDialogButtonBox::ButtonRole::HelpRole); buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[0]) - : tr(BUTTON_CANCEL), + : tr(SWKBD_BUTTON_CANCEL), QDialogButtonBox::ButtonRole::RejectRole); break; case ButtonConfig::Dual: buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[1]) - : tr(BUTTON_OKAY), + : tr(SWKBD_BUTTON_OKAY), QDialogButtonBox::ButtonRole::AcceptRole); buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[0]) - : tr(BUTTON_CANCEL), + : tr(SWKBD_BUTTON_CANCEL), QDialogButtonBox::ButtonRole::RejectRole); break; case ButtonConfig::Single: buttons->addButton(config.has_custom_button_text ? QString::fromStdString(config.button_text[0]) - : tr(BUTTON_OKAY), + : tr(SWKBD_BUTTON_OKAY), QDialogButtonBox::ButtonRole::AcceptRole); break; case ButtonConfig::None: @@ -109,7 +109,7 @@ void QtKeyboardDialog::HandleValidationError(Frontend::ValidationError error) { QtKeyboard::QtKeyboard(QWidget& parent_) : parent(parent_) {} -void QtKeyboard::Setup(const Frontend::KeyboardConfig* config) { +void QtKeyboard::Setup(const Frontend::KeyboardConfig& config) { SoftwareKeyboard::Setup(config); if (this->config.button_config != Frontend::ButtonConfig::None) { ok_id = static_cast(this->config.button_config); diff --git a/src/citra_qt/applets/swkbd.h b/src/citra_qt/applets/swkbd.h index cfcbcb45e7..5fcbd71127 100644 --- a/src/citra_qt/applets/swkbd.h +++ b/src/citra_qt/applets/swkbd.h @@ -48,7 +48,7 @@ class QtKeyboard final : public QObject, public Frontend::SoftwareKeyboard { public: explicit QtKeyboard(QWidget& parent); - void Setup(const Frontend::KeyboardConfig* config) override; + void Setup(const Frontend::KeyboardConfig& config) override; private: Q_INVOKABLE void OpenInputDialog(); diff --git a/src/citra_qt/main.cpp b/src/citra_qt/main.cpp index e54bdfb854..f3d1d6582c 100644 --- a/src/citra_qt/main.cpp +++ b/src/citra_qt/main.cpp @@ -1897,8 +1897,8 @@ int main(int argc, char* argv[]) { // Register frontend applets Frontend::RegisterDefaultApplets(); - Frontend::RegisterMiiSelector(std::make_shared(main_window)); - Frontend::RegisterSoftwareKeyboard(std::make_shared(main_window)); + Core::System::GetInstance().RegisterMiiSelector(std::make_shared(main_window)); + Core::System::GetInstance().RegisterSoftwareKeyboard(std::make_shared(main_window)); main_window.show(); int result = app.exec(); diff --git a/src/core/file_sys/archive_backend.h b/src/core/file_sys/archive_backend.h index 259ece84ae..da05fcb7b9 100644 --- a/src/core/file_sys/archive_backend.h +++ b/src/core/file_sys/archive_backend.h @@ -40,6 +40,9 @@ public: Path() : type(LowPathType::Invalid) {} Path(const char* path) : type(LowPathType::Char), string(path) {} Path(std::vector binary_data) : type(LowPathType::Binary), binary(std::move(binary_data)) {} + template + Path(std::array binary_data) + : type(LowPathType::Binary), binary(binary_data.begin(), binary_data.end()) {} Path(LowPathType type, const std::vector& data); LowPathType GetType() const { diff --git a/src/core/frontend/applets/default_applets.cpp b/src/core/frontend/applets/default_applets.cpp index 8fb07484c4..38a6c48dd2 100644 --- a/src/core/frontend/applets/default_applets.cpp +++ b/src/core/frontend/applets/default_applets.cpp @@ -2,13 +2,14 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "core/core.h" #include "core/frontend/applets/default_applets.h" #include "core/frontend/applets/mii_selector.h" #include "core/frontend/applets/swkbd.h" namespace Frontend { void RegisterDefaultApplets() { - RegisterSoftwareKeyboard(std::make_shared()); - RegisterMiiSelector(std::make_shared()); + Core::System::GetInstance().RegisterSoftwareKeyboard(std::make_shared()); + Core::System::GetInstance().RegisterMiiSelector(std::make_shared()); } } // namespace Frontend diff --git a/src/core/frontend/applets/mii_selector.cpp b/src/core/frontend/applets/mii_selector.cpp index ca39788697..2ca23f1db7 100644 --- a/src/core/frontend/applets/mii_selector.cpp +++ b/src/core/frontend/applets/mii_selector.cpp @@ -2,7 +2,6 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. -#include "core/core.h" #include "core/frontend/applets/mii_selector.h" namespace Frontend { @@ -11,17 +10,9 @@ void MiiSelector::Finalize(u32 return_code, HLE::Applets::MiiData mii) { data = {return_code, mii}; } -void DefaultMiiSelector::Setup(const Frontend::MiiSelectorConfig* config) { +void DefaultMiiSelector::Setup(const Frontend::MiiSelectorConfig& config) { MiiSelector::Setup(config); Finalize(0, HLE::Applets::MiiSelector::GetStandardMiiResult().selected_mii_data); } -void RegisterMiiSelector(std::shared_ptr applet) { - Core::System::GetInstance().RegisterMiiSelector(applet); -} - -std::shared_ptr GetRegisteredMiiSelector() { - return Core::System::GetInstance().GetMiiSelector(); -} - } // namespace Frontend diff --git a/src/core/frontend/applets/mii_selector.h b/src/core/frontend/applets/mii_selector.h index 2913c2040f..8cb59d9d22 100644 --- a/src/core/frontend/applets/mii_selector.h +++ b/src/core/frontend/applets/mii_selector.h @@ -4,13 +4,16 @@ #pragma once -#include #include #include #include "core/hle/applets/mii_selector.h" namespace Frontend { +/// Default English button text mappings. Frontends may need to copy this to internationalize it. +constexpr char MII_BUTTON_OKAY[] = "Ok"; +constexpr char MII_BUTTON_CANCEL[] = "Cancel"; + /// Configuration that's relevant to frontend implementation of applet. Anything missing that we /// later learn is needed can be added here and filled in by the backend HLE applet struct MiiSelectorConfig { @@ -26,11 +29,12 @@ struct MiiSelectorData { class MiiSelector { public: - virtual void Setup(const MiiSelectorConfig* config) { - this->config = MiiSelectorConfig(*config); + virtual void Setup(const MiiSelectorConfig& config) { + this->config = MiiSelectorConfig(config); } - const MiiSelectorData* ReceiveData() { - return &data; + + const MiiSelectorData& ReceiveData() const { + return data; } /** @@ -46,11 +50,7 @@ protected: class DefaultMiiSelector final : public MiiSelector { public: - void Setup(const MiiSelectorConfig* config) override; + void Setup(const MiiSelectorConfig& config) override; }; -void RegisterMiiSelector(std::shared_ptr applet); - -std::shared_ptr GetRegisteredMiiSelector(); - } // namespace Frontend diff --git a/src/core/frontend/applets/swkbd.cpp b/src/core/frontend/applets/swkbd.cpp index 6c74429138..d1513e20c8 100644 --- a/src/core/frontend/applets/swkbd.cpp +++ b/src/core/frontend/applets/swkbd.cpp @@ -135,7 +135,7 @@ ValidationError SoftwareKeyboard::Finalize(const std::string& text, u8 button) { return ValidationError::None; } -void DefaultKeyboard::Setup(const Frontend::KeyboardConfig* config) { +void DefaultKeyboard::Setup(const Frontend::KeyboardConfig& config) { SoftwareKeyboard::Setup(config); auto cfg = Service::CFG::GetModule(Core::System::GetInstance()); @@ -157,12 +157,4 @@ void DefaultKeyboard::Setup(const Frontend::KeyboardConfig* config) { } } -void RegisterSoftwareKeyboard(std::shared_ptr applet) { - Core::System::GetInstance().RegisterSoftwareKeyboard(applet); -} - -std::shared_ptr GetRegisteredSoftwareKeyboard() { - return Core::System::GetInstance().GetSoftwareKeyboard(); -} - } // namespace Frontend diff --git a/src/core/frontend/applets/swkbd.h b/src/core/frontend/applets/swkbd.h index 5c07eb12d6..f678c0f48f 100644 --- a/src/core/frontend/applets/swkbd.h +++ b/src/core/frontend/applets/swkbd.h @@ -30,9 +30,9 @@ enum class ButtonConfig { }; /// Default English button text mappings. Frontends may need to copy this to internationalize it. -constexpr char BUTTON_OKAY[] = "Ok"; -constexpr char BUTTON_CANCEL[] = "Cancel"; -constexpr char BUTTON_FORGOT[] = "I Forgot"; +constexpr char SWKBD_BUTTON_OKAY[] = "Ok"; +constexpr char SWKBD_BUTTON_CANCEL[] = "Cancel"; +constexpr char SWKBD_BUTTON_FORGOT[] = "I Forgot"; /// Configuration thats relevent to frontend implementation of applets. Anything missing that we /// later learn is needed can be added here and filled in by the backend HLE applet @@ -82,11 +82,12 @@ enum class ValidationError { class SoftwareKeyboard { public: - virtual void Setup(const KeyboardConfig* config) { - this->config = KeyboardConfig(*config); + virtual void Setup(const KeyboardConfig& config) { + this->config = KeyboardConfig(config); } - const KeyboardData* ReceiveData() { - return &data; + + const KeyboardData& ReceiveData() const { + return data; } /** @@ -121,11 +122,7 @@ protected: class DefaultKeyboard final : public SoftwareKeyboard { public: - void Setup(const KeyboardConfig* config) override; + void Setup(const KeyboardConfig& config) override; }; -void RegisterSoftwareKeyboard(std::shared_ptr applet); - -std::shared_ptr GetRegisteredSoftwareKeyboard(); - } // namespace Frontend diff --git a/src/core/hle/applets/mii_selector.cpp b/src/core/hle/applets/mii_selector.cpp index 1cfa6ea1c0..c606c95f8b 100644 --- a/src/core/hle/applets/mii_selector.cpp +++ b/src/core/hle/applets/mii_selector.cpp @@ -60,11 +60,11 @@ ResultCode MiiSelector::StartImpl(const Service::APT::AppletStartupParameter& pa memcpy(&config, parameter.buffer.data(), parameter.buffer.size()); using namespace Frontend; - frontend_applet = GetRegisteredMiiSelector(); - if (frontend_applet) { - MiiSelectorConfig frontend_config = ToFrontendConfig(config); - frontend_applet->Setup(&frontend_config); - } + frontend_applet = Core::System::GetInstance().GetMiiSelector(); + ASSERT(frontend_applet); + + MiiSelectorConfig frontend_config = ToFrontendConfig(config); + frontend_applet->Setup(frontend_config); is_running = true; return RESULT_SUCCESS; @@ -72,9 +72,9 @@ ResultCode MiiSelector::StartImpl(const Service::APT::AppletStartupParameter& pa void MiiSelector::Update() { using namespace Frontend; - const MiiSelectorData* data = frontend_applet->ReceiveData(); - result.return_code = data->return_code; - result.selected_mii_data = data->mii; + const MiiSelectorData& data = frontend_applet->ReceiveData(); + result.return_code = data.return_code; + result.selected_mii_data = data.mii; // Calculate the checksum of the selected Mii, see https://www.3dbrew.org/wiki/Mii#Checksum result.mii_data_checksum = boost::crc<16, 0x1021, 0, 0, false, false>( &result.selected_mii_data, sizeof(HLE::Applets::MiiData) + sizeof(result.unknown1)); diff --git a/src/core/hle/applets/swkbd.cpp b/src/core/hle/applets/swkbd.cpp index 3f209e9f4f..4e4dd1b90c 100644 --- a/src/core/hle/applets/swkbd.cpp +++ b/src/core/hle/applets/swkbd.cpp @@ -68,11 +68,11 @@ ResultCode SoftwareKeyboard::StartImpl(Service::APT::AppletStartupParameter cons DrawScreenKeyboard(); using namespace Frontend; - frontend_applet = GetRegisteredSoftwareKeyboard(); - if (frontend_applet) { - KeyboardConfig frontend_config = ToFrontendConfig(config); - frontend_applet->Setup(&frontend_config); - } + frontend_applet = Core::System::GetInstance().GetSoftwareKeyboard(); + ASSERT(frontend_applet); + + KeyboardConfig frontend_config = ToFrontendConfig(config); + frontend_applet->Setup(frontend_config); is_running = true; return RESULT_SUCCESS; @@ -80,7 +80,7 @@ ResultCode SoftwareKeyboard::StartImpl(Service::APT::AppletStartupParameter cons void SoftwareKeyboard::Update() { using namespace Frontend; - KeyboardData data(*frontend_applet->ReceiveData()); + KeyboardData data(frontend_applet->ReceiveData()); std::u16string text = Common::UTF8ToUTF16(data.text); memcpy(text_memory->GetPointer(), text.c_str(), text.length() * sizeof(char16_t)); switch (config.num_buttons_m1) { diff --git a/src/core/hle/service/ptm/ptm.h b/src/core/hle/service/ptm/ptm.h index ee29eb2e8c..549e693538 100644 --- a/src/core/hle/service/ptm/ptm.h +++ b/src/core/hle/service/ptm/ptm.h @@ -16,7 +16,7 @@ class System; namespace Service::PTM { /// Id of the SharedExtData archive used by the PTM process -static const std::vector ptm_shared_extdata_id = {0, 0, 0, 0, 0x0B, 0, 0, 0xF0, 0, 0, 0, 0}; +constexpr std::array ptm_shared_extdata_id = {0, 0, 0, 0, 0x0B, 0, 0, 0xF0, 0, 0, 0, 0}; /// Charge levels used by PTM functions enum class ChargeLevels : u32 {