From 2fb3b9b951d08da071841a6bb4e02439e7d78698 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 20:56:22 -0400 Subject: [PATCH 1/7] core/telemetry_session: Remove unused include --- src/core/telemetry_session.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/telemetry_session.h b/src/core/telemetry_session.h index cae5a45a00..8229d1333a 100644 --- a/src/core/telemetry_session.h +++ b/src/core/telemetry_session.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include "common/telemetry.h" From 05af9d915ced92e72e20e3a2d6db831ad5a9a8e6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:07:34 -0400 Subject: [PATCH 2/7] core/telemetry_session: Explicitly delete copy and move constructors NonCopyable is misleading here. It also makes the class non-moveable as well, so we can be explicit about this. --- src/core/telemetry_session.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/telemetry_session.h b/src/core/telemetry_session.h index 8229d1333a..7d0c8d4131 100644 --- a/src/core/telemetry_session.h +++ b/src/core/telemetry_session.h @@ -14,11 +14,17 @@ namespace Core { * session, logging any one-time fields. Interfaces with the telemetry backend used for submitting * data to the web service. Submits session data on close. */ -class TelemetrySession : NonCopyable { +class TelemetrySession { public: TelemetrySession(); ~TelemetrySession(); + TelemetrySession(const TelemetrySession&) = delete; + TelemetrySession& operator=(const TelemetrySession&) = delete; + + TelemetrySession(TelemetrySession&&) = delete; + TelemetrySession& operator=(TelemetrySession&&) = delete; + /** * Wrapper around the Telemetry::FieldCollection::AddField method. * @param type Type of the field to add. From 215fd827384904f1cb7fa689ff8cd3f61dbbd007 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:12:23 -0400 Subject: [PATCH 3/7] core/telemetry_session: Remove usages of the global system accessor Makes the dependency explicit in the TelemetrySession's interface instead of making it a hidden dependency. This also revealed a hidden issue with the way the telemetry session was being initialized. It was attempting to retrieve the app loader and log out title-specific information. However, this isn't always guaranteed to be possible. During the initialization phase, everything is being constructed. It doesn't mean an actual title has been selected. This is what the Load() function is for. This potentially results in dead code paths involving the app loader. Instead, we explicitly add this information when we know the app loader instance is available. --- src/core/core.cpp | 2 +- src/core/telemetry_session.cpp | 60 ++++++++++++++++++---------------- src/core/telemetry_session.h | 22 ++++++++++++- 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 7106151bd8..9f9356f53d 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -144,7 +144,6 @@ struct System::Impl { ResultStatus Load(System& system, Frontend::EmuWindow& emu_window, const std::string& filepath) { app_loader = Loader::GetLoader(GetGameFileFromPath(virtual_filesystem, filepath)); - if (!app_loader) { LOG_CRITICAL(Core, "Failed to obtain loader for {}!", filepath); return ResultStatus::ErrorGetLoader; @@ -167,6 +166,7 @@ struct System::Impl { return init_result; } + telemetry_session->AddInitialInfo(*app_loader); auto main_process = Kernel::Process::Create(system, "main"); const auto [load_result, load_parameters] = app_loader->Load(*main_process); if (load_result != Loader::ResultStatus::Success) { diff --git a/src/core/telemetry_session.cpp b/src/core/telemetry_session.cpp index 4b17bada5d..4f8aff8161 100644 --- a/src/core/telemetry_session.cpp +++ b/src/core/telemetry_session.cpp @@ -12,7 +12,6 @@ #include "common/file_util.h" #include "common/logging/log.h" -#include "core/core.h" #include "core/file_sys/control_metadata.h" #include "core/file_sys/patch_manager.h" #include "core/loader/loader.h" @@ -101,7 +100,31 @@ bool VerifyLogin(const std::string& username, const std::string& token) { #endif } -TelemetrySession::TelemetrySession() { +TelemetrySession::TelemetrySession() = default; + +TelemetrySession::~TelemetrySession() { + // Log one-time session end information + const s64 shutdown_time{std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch()) + .count()}; + AddField(Telemetry::FieldType::Session, "Shutdown_Time", shutdown_time); + +#ifdef ENABLE_WEB_SERVICE + auto backend = std::make_unique( + Settings::values.web_api_url, Settings::values.yuzu_username, Settings::values.yuzu_token); +#else + auto backend = std::make_unique(); +#endif + + // Complete the session, submitting to web service if necessary + field_collection.Accept(*backend); + if (Settings::values.enable_telemetry) { + backend->Complete(); + } + backend = nullptr; +} + +void TelemetrySession::AddInitialInfo(Loader::AppLoader& app_loader) { // Log one-time top-level information AddField(Telemetry::FieldType::None, "TelemetryId", GetTelemetryId()); @@ -112,26 +135,28 @@ TelemetrySession::TelemetrySession() { AddField(Telemetry::FieldType::Session, "Init_Time", init_time); u64 program_id{}; - const Loader::ResultStatus res{System::GetInstance().GetAppLoader().ReadProgramId(program_id)}; + const Loader::ResultStatus res{app_loader.ReadProgramId(program_id)}; if (res == Loader::ResultStatus::Success) { const std::string formatted_program_id{fmt::format("{:016X}", program_id)}; AddField(Telemetry::FieldType::Session, "ProgramId", formatted_program_id); std::string name; - System::GetInstance().GetAppLoader().ReadTitle(name); + app_loader.ReadTitle(name); if (name.empty()) { auto [nacp, icon_file] = FileSys::PatchManager(program_id).GetControlMetadata(); - if (nacp != nullptr) + if (nacp != nullptr) { name = nacp->GetApplicationName(); + } } - if (!name.empty()) + if (!name.empty()) { AddField(Telemetry::FieldType::Session, "ProgramName", name); + } } AddField(Telemetry::FieldType::Session, "ProgramFormat", - static_cast(System::GetInstance().GetAppLoader().GetFileType())); + static_cast(app_loader.GetFileType())); // Log application information Telemetry::AppendBuildInfo(field_collection); @@ -162,27 +187,6 @@ TelemetrySession::TelemetrySession() { Settings::values.use_docked_mode); } -TelemetrySession::~TelemetrySession() { - // Log one-time session end information - const s64 shutdown_time{std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) - .count()}; - AddField(Telemetry::FieldType::Session, "Shutdown_Time", shutdown_time); - -#ifdef ENABLE_WEB_SERVICE - auto backend = std::make_unique( - Settings::values.web_api_url, Settings::values.yuzu_username, Settings::values.yuzu_token); -#else - auto backend = std::make_unique(); -#endif - - // Complete the session, submitting to web service if necessary - field_collection.Accept(*backend); - if (Settings::values.enable_telemetry) - backend->Complete(); - backend = nullptr; -} - bool TelemetrySession::SubmitTestcase() { #ifdef ENABLE_WEB_SERVICE auto backend = std::make_unique( diff --git a/src/core/telemetry_session.h b/src/core/telemetry_session.h index 7d0c8d4131..17ac223777 100644 --- a/src/core/telemetry_session.h +++ b/src/core/telemetry_session.h @@ -7,6 +7,10 @@ #include #include "common/telemetry.h" +namespace Loader { +class AppLoader; +} + namespace Core { /** @@ -16,7 +20,7 @@ namespace Core { */ class TelemetrySession { public: - TelemetrySession(); + explicit TelemetrySession(); ~TelemetrySession(); TelemetrySession(const TelemetrySession&) = delete; @@ -25,6 +29,22 @@ public: TelemetrySession(TelemetrySession&&) = delete; TelemetrySession& operator=(TelemetrySession&&) = delete; + /** + * Adds the initial telemetry info necessary when starting up a title. + * + * This includes information such as: + * - Telemetry ID + * - Initialization time + * - Title ID + * - Title name + * - Title file format + * - Miscellaneous settings values. + * + * @param app_loader The application loader to use to retrieve + * title-specific information. + */ + void AddInitialInfo(Loader::AppLoader& app_loader); + /** * Wrapper around the Telemetry::FieldCollection::AddField method. * @param type Type of the field to add. From b1a4ab2ccc82940f9b1cbf18027ce79a3c23fede Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:32:48 -0400 Subject: [PATCH 4/7] core/telemetry_session: Remove unnecessary web service nulling out in destructor This will automatically occur when the backend instance goes out of scope at the end of the destructor's execution. --- src/core/telemetry_session.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/telemetry_session.cpp b/src/core/telemetry_session.cpp index 4f8aff8161..90d06830f3 100644 --- a/src/core/telemetry_session.cpp +++ b/src/core/telemetry_session.cpp @@ -116,12 +116,11 @@ TelemetrySession::~TelemetrySession() { auto backend = std::make_unique(); #endif - // Complete the session, submitting to web service if necessary + // Complete the session, submitting to the web service backend if necessary field_collection.Accept(*backend); if (Settings::values.enable_telemetry) { backend->Complete(); } - backend = nullptr; } void TelemetrySession::AddInitialInfo(Loader::AppLoader& app_loader) { From 84a8fb9264b5018d97e487f3e473b5ebb43b48f1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:42:50 -0400 Subject: [PATCH 5/7] core/loader: Remove LoadKernelSystemMode This is a hold-over from Citra and doesn't apply to yuzu. --- src/core/core.cpp | 9 --------- src/core/core.h | 1 - src/core/loader/loader.h | 11 ----------- src/yuzu/main.cpp | 5 ----- src/yuzu_cmd/yuzu.cpp | 3 --- 5 files changed, 29 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 9f9356f53d..5098bdcff3 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -148,15 +148,6 @@ struct System::Impl { LOG_CRITICAL(Core, "Failed to obtain loader for {}!", filepath); return ResultStatus::ErrorGetLoader; } - std::pair, Loader::ResultStatus> system_mode = - app_loader->LoadKernelSystemMode(); - - if (system_mode.second != Loader::ResultStatus::Success) { - LOG_CRITICAL(Core, "Failed to determine system mode (Error {})!", - static_cast(system_mode.second)); - - return ResultStatus::ErrorSystemMode; - } ResultStatus init_result{Init(system, emu_window)}; if (init_result != ResultStatus::Success) { diff --git a/src/core/core.h b/src/core/core.h index a9a756a4c0..20959de54f 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -98,7 +98,6 @@ public: Success, ///< Succeeded ErrorNotInitialized, ///< Error trying to use core prior to initialization ErrorGetLoader, ///< Error finding the correct application loader - ErrorSystemMode, ///< Error determining the system mode ErrorSystemFiles, ///< Error in finding system files ErrorSharedFont, ///< Error in finding shared font ErrorVideoCore, ///< Error in the video core diff --git a/src/core/loader/loader.h b/src/core/loader/loader.h index f7846db52c..869406b75d 100644 --- a/src/core/loader/loader.h +++ b/src/core/loader/loader.h @@ -153,17 +153,6 @@ public: */ virtual LoadResult Load(Kernel::Process& process) = 0; - /** - * Loads the system mode that this application needs. - * This function defaults to 2 (96MB allocated to the application) if it can't read the - * information. - * @returns A pair with the optional system mode, and and the status. - */ - virtual std::pair, ResultStatus> LoadKernelSystemMode() { - // 96MB allocated to the application. - return std::make_pair(2, ResultStatus::Success); - } - /** * Get the code (typically .code section) of the application * @param buffer Reference to buffer to store data diff --git a/src/yuzu/main.cpp b/src/yuzu/main.cpp index cef2cc1ae8..29beaa3baf 100644 --- a/src/yuzu/main.cpp +++ b/src/yuzu/main.cpp @@ -850,11 +850,6 @@ bool GMainWindow::LoadROM(const QString& filename) { QMessageBox::critical(this, tr("Error while loading ROM!"), tr("The ROM format is not supported.")); break; - case Core::System::ResultStatus::ErrorSystemMode: - LOG_CRITICAL(Frontend, "Failed to load ROM!"); - QMessageBox::critical(this, tr("Error while loading ROM!"), - tr("Could not determine the system mode.")); - break; case Core::System::ResultStatus::ErrorVideoCore: QMessageBox::critical( this, tr("An error occurred initializing the video core."), diff --git a/src/yuzu_cmd/yuzu.cpp b/src/yuzu_cmd/yuzu.cpp index d3734927b9..3cddadd379 100644 --- a/src/yuzu_cmd/yuzu.cpp +++ b/src/yuzu_cmd/yuzu.cpp @@ -199,9 +199,6 @@ int main(int argc, char** argv) { case Core::System::ResultStatus::ErrorNotInitialized: LOG_CRITICAL(Frontend, "CPUCore not initialized"); return -1; - case Core::System::ResultStatus::ErrorSystemMode: - LOG_CRITICAL(Frontend, "Failed to determine system mode!"); - return -1; case Core::System::ResultStatus::ErrorVideoCore: LOG_CRITICAL(Frontend, "Failed to initialize VideoCore!"); return -1; From c6f05b586fa1c31e0415a5368a98c9d61cd79232 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 22:10:51 -0400 Subject: [PATCH 6/7] yuzu_cmd/yuzu: Correct formatting specifier Amends the formatting specifier to obey libfmt. Prevents the application from terminating due to a formatting issue in the error case. --- src/yuzu_cmd/yuzu.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yuzu_cmd/yuzu.cpp b/src/yuzu_cmd/yuzu.cpp index 3cddadd379..05f28ce8d2 100644 --- a/src/yuzu_cmd/yuzu.cpp +++ b/src/yuzu_cmd/yuzu.cpp @@ -191,7 +191,7 @@ int main(int argc, char** argv) { switch (load_result) { case Core::System::ResultStatus::ErrorGetLoader: - LOG_CRITICAL(Frontend, "Failed to obtain loader for %s!", filepath.c_str()); + LOG_CRITICAL(Frontend, "Failed to obtain loader for {}!", filepath); return -1; case Core::System::ResultStatus::ErrorLoader: LOG_CRITICAL(Frontend, "Failed to load ROM!"); From 8bbe930faccc43d85f7890c2b7869c2407a0eef5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 22:14:07 -0400 Subject: [PATCH 7/7] core/core: Remove unnecessary includes The contents of these includes aren't used anywhere in this translation unit. --- src/core/core.cpp | 8 ------ src/core/hle/service/am/applets/applets.cpp | 15 ++++++++++++ src/core/hle/service/am/applets/applets.h | 27 +++++++++++++++++---- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 5098bdcff3..ff07210792 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -18,11 +18,6 @@ #include "core/file_sys/registered_cache.h" #include "core/file_sys/vfs_concat.h" #include "core/file_sys/vfs_real.h" -#include "core/frontend/applets/error.h" -#include "core/frontend/applets/general_frontend.h" -#include "core/frontend/applets/profile_select.h" -#include "core/frontend/applets/software_keyboard.h" -#include "core/frontend/applets/web_browser.h" #include "core/gdbstub/gdbstub.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/kernel.h" @@ -37,9 +32,6 @@ #include "core/settings.h" #include "core/telemetry_session.h" #include "file_sys/cheat_engine.h" -#include "frontend/applets/profile_select.h" -#include "frontend/applets/software_keyboard.h" -#include "frontend/applets/web_browser.h" #include "video_core/debug_utils/debug_utils.h" #include "video_core/renderer_base.h" #include "video_core/video_core.h" diff --git a/src/core/hle/service/am/applets/applets.cpp b/src/core/hle/service/am/applets/applets.cpp index e812c66e9b..14fa923189 100644 --- a/src/core/hle/service/am/applets/applets.cpp +++ b/src/core/hle/service/am/applets/applets.cpp @@ -121,6 +121,21 @@ void Applet::Initialize() { initialized = true; } +AppletFrontendSet::AppletFrontendSet() = default; + +AppletFrontendSet::AppletFrontendSet(ErrorApplet error, PhotoViewer photo_viewer, + ProfileSelect profile_select, + SoftwareKeyboard software_keyboard, WebBrowser web_browser) + : error{std::move(error)}, photo_viewer{std::move(photo_viewer)}, profile_select{std::move( + profile_select)}, + software_keyboard{std::move(software_keyboard)}, web_browser{std::move(web_browser)} {} + +AppletFrontendSet::~AppletFrontendSet() = default; + +AppletFrontendSet::AppletFrontendSet(AppletFrontendSet&&) noexcept = default; + +AppletFrontendSet& AppletFrontendSet::operator=(AppletFrontendSet&&) noexcept = default; + AppletManager::AppletManager() = default; AppletManager::~AppletManager() = default; diff --git a/src/core/hle/service/am/applets/applets.h b/src/core/hle/service/am/applets/applets.h index 7f932672c6..b46e10a4aa 100644 --- a/src/core/hle/service/am/applets/applets.h +++ b/src/core/hle/service/am/applets/applets.h @@ -137,11 +137,28 @@ protected: }; struct AppletFrontendSet { - std::unique_ptr error; - std::unique_ptr photo_viewer; - std::unique_ptr profile_select; - std::unique_ptr software_keyboard; - std::unique_ptr web_browser; + using ErrorApplet = std::unique_ptr; + using PhotoViewer = std::unique_ptr; + using ProfileSelect = std::unique_ptr; + using SoftwareKeyboard = std::unique_ptr; + using WebBrowser = std::unique_ptr; + + AppletFrontendSet(); + AppletFrontendSet(ErrorApplet error, PhotoViewer photo_viewer, ProfileSelect profile_select, + SoftwareKeyboard software_keyboard, WebBrowser web_browser); + ~AppletFrontendSet(); + + AppletFrontendSet(const AppletFrontendSet&) = delete; + AppletFrontendSet& operator=(const AppletFrontendSet&) = delete; + + AppletFrontendSet(AppletFrontendSet&&) noexcept; + AppletFrontendSet& operator=(AppletFrontendSet&&) noexcept; + + ErrorApplet error; + PhotoViewer photo_viewer; + ProfileSelect profile_select; + SoftwareKeyboard software_keyboard; + WebBrowser web_browser; }; class AppletManager {