From 2fb3b9b951d08da071841a6bb4e02439e7d78698 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 20:56:22 -0400 Subject: 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 cae5a45a0..8229d1333 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" -- cgit v1.2.3 From 05af9d915ced92e72e20e3a2d6db831ad5a9a8e6 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:07:34 -0400 Subject: 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 8229d1333..7d0c8d413 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. -- cgit v1.2.3 From 215fd827384904f1cb7fa689ff8cd3f61dbbd007 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:12:23 -0400 Subject: 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 7106151bd..9f9356f53 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 4b17bada5..4f8aff816 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 7d0c8d413..17ac22377 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. -- cgit v1.2.3 From b1a4ab2ccc82940f9b1cbf18027ce79a3c23fede Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:32:48 -0400 Subject: 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 4f8aff816..90d06830f 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) { -- cgit v1.2.3 From 84a8fb9264b5018d97e487f3e473b5ebb43b48f1 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 21:42:50 -0400 Subject: 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 9f9356f53..5098bdcff 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 a9a756a4c..20959de54 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 f7846db52..869406b75 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 cef2cc1ae..29beaa3ba 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 d3734927b..3cddadd37 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; -- cgit v1.2.3 From c6f05b586fa1c31e0415a5368a98c9d61cd79232 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 22:10:51 -0400 Subject: 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 3cddadd37..05f28ce8d 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!"); -- cgit v1.2.3 From 8bbe930faccc43d85f7890c2b7869c2407a0eef5 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 28 May 2019 22:14:07 -0400 Subject: 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 5098bdcff..ff0721079 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 e812c66e9..14fa92318 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 7f932672c..b46e10a4a 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 { -- cgit v1.2.3