From ddc8093b34a91e1c9ce68fb27510a3160fc6053b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Fri, 13 Jun 2025 19:09:35 +0200 Subject: [PATCH] RetroAchievements: Better error messages on login, don't delete the username on failure --- Common/System/Request.h | 4 ++-- Core/RetroAchievements.cpp | 24 +++++++++++++++--------- UI/RetroAchievementScreens.cpp | 10 +++++----- UI/RetroAchievementScreens.h | 1 - Windows/InputBox.cpp | 18 ++++++++++-------- Windows/main.cpp | 2 +- 6 files changed, 33 insertions(+), 26 deletions(-) diff --git a/Common/System/Request.h b/Common/System/Request.h index 47cb1e52ae..e9fab4aa75 100644 --- a/Common/System/Request.h +++ b/Common/System/Request.h @@ -123,8 +123,8 @@ inline void System_BrowseForFileSave(RequesterToken token, std::string_view titl void System_BrowseForFolder(RequesterToken token, std::string_view title, const Path &initialPath, RequestCallback callback, RequestFailedCallback failedCallback = nullptr); // The returned string is username + '\n' + password. -inline void System_AskUsernamePassword(RequesterToken token, std::string_view title, RequestCallback callback, RequestFailedCallback failedCallback = nullptr) { - g_requestManager.MakeSystemRequest(SystemRequestType::ASK_USERNAME_PASSWORD, token, callback, failedCallback, title, "", 0); +inline void System_AskUsernamePassword(RequesterToken token, std::string_view title, std::string_view defaultUsername, RequestCallback callback, RequestFailedCallback failedCallback = nullptr) { + g_requestManager.MakeSystemRequest(SystemRequestType::ASK_USERNAME_PASSWORD, token, callback, failedCallback, title, defaultUsername, 0); } inline void System_CopyStringToClipboard(std::string_view string) { diff --git a/Core/RetroAchievements.cpp b/Core/RetroAchievements.cpp index 4d5017876e..fa516a56be 100644 --- a/Core/RetroAchievements.cpp +++ b/Core/RetroAchievements.cpp @@ -496,18 +496,25 @@ static void login_token_callback(int result, const char *error_message, rc_clien ERROR_LOG(Log::Achievements, "Callback: Failure logging in via token: %d, %s", result, error_message); if (isInitialAttempt) { auto ac = GetI18NCategory(I18NCat::ACHIEVEMENTS); - g_OSD.Show(OSDType::MESSAGE_WARNING, ac->T("Failed logging in to RetroAchievements"), "", g_RAImageID); + char message[512]; + snprintf(message, sizeof(message), "%d: %s", result, error_message); + g_OSD.Show(OSDType::MESSAGE_WARNING, ac->T("Failed logging in to RetroAchievements"), message, g_RAImageID); } - // Clear the token. - if (result == RC_INVALID_CREDENTIALS || result == RC_EXPIRED_TOKEN) { - g_Config.sAchievementsUserName.clear(); + // Take some action. + switch (result) { + case RC_INVALID_CREDENTIALS: + g_loginResult = RC_OK; // why? + break; + case RC_EXPIRED_TOKEN: + WARN_LOG(Log::Achievements, "Clearing token since it was expired"); NativeClearSecret(RA_TOKEN_SECRET_NAME); - g_loginResult = RC_OK; - } else { + g_loginResult = RC_OK; // why? + break; + default: g_loginResult = result; + break; } - OnAchievementsLoginStateChange(); g_isLoggingIn = false; return; @@ -722,8 +729,7 @@ bool LoginAsync(const char *username, const char *password) { void Logout() { rc_client_logout(g_rcClient); - // remove from config - g_Config.sAchievementsUserName.clear(); + // remove secret from config NativeClearSecret(RA_TOKEN_SECRET_NAME); g_Config.Save("Achievements logout"); g_activeChallenges.clear(); diff --git a/UI/RetroAchievementScreens.cpp b/UI/RetroAchievementScreens.cpp index b5f16a408f..e29c59de1b 100644 --- a/UI/RetroAchievementScreens.cpp +++ b/UI/RetroAchievementScreens.cpp @@ -319,7 +319,7 @@ void RetroAchievementsSettingsScreen::CreateAccountTab(UI::ViewGroup *viewGroup) } else if (System_GetPropertyBool(SYSPROP_HAS_LOGIN_DIALOG)) { viewGroup->Add(new Choice(di->T("Log in")))->OnClick.Add([=](UI::EventParams &) -> UI::EventReturn { std::string title = StringFromFormat("RetroAchievements: %s", di->T_cstr("Log in")); - System_AskUsernamePassword(GetRequesterToken(), title, [](const std::string &value, int) { + System_AskUsernamePassword(GetRequesterToken(), title, g_Config.sAchievementsUserName, [](const std::string &value, int) { std::vector parts; SplitString(value, '\n', parts); if (parts.size() == 2 && !parts[0].empty() && !parts[1].empty()) { @@ -330,19 +330,19 @@ void RetroAchievementsSettingsScreen::CreateAccountTab(UI::ViewGroup *viewGroup) }); } else { // Hack up a temporary quick login-form-ish-thing - viewGroup->Add(new PopupTextInputChoice(GetRequesterToken(), &username_, di->T("Username"), "", 128, screenManager())); + viewGroup->Add(new PopupTextInputChoice(GetRequesterToken(), &g_Config.sAchievementsUserName, di->T("Username"), "", 128, screenManager())); viewGroup->Add(new PopupTextInputChoice(GetRequesterToken(), &password_, di->T("Password"), "", 128, screenManager()))->SetPasswordDisplay(); Choice *loginButton = viewGroup->Add(new Choice(di->T("Log in"))); loginButton->OnClick.Add([=](UI::EventParams &) -> UI::EventReturn { - if (!username_.empty() && !password_.empty()) { - Achievements::LoginAsync(username_.c_str(), password_.c_str()); + if (!g_Config.sAchievementsUserName.empty() && !password_.empty()) { + Achievements::LoginAsync(g_Config.sAchievementsUserName.c_str(), password_.c_str()); memset(&password_[0], 0, password_.size()); password_.clear(); } return UI::EVENT_DONE; }); loginButton->SetEnabledFunc([&]() { - return !username_.empty() && !password_.empty(); + return !g_Config.sAchievementsUserName.empty() && !password_.empty(); }); } viewGroup->Add(new Choice(ac->T("Register on www.retroachievements.org")))->OnClick.Add([&](UI::EventParams &) -> UI::EventReturn { diff --git a/UI/RetroAchievementScreens.h b/UI/RetroAchievementScreens.h index bc84f3109d..6beb314554 100644 --- a/UI/RetroAchievementScreens.h +++ b/UI/RetroAchievementScreens.h @@ -47,7 +47,6 @@ private: void CreateCustomizeTab(UI::ViewGroup *viewGroup); void CreateDeveloperToolsTab(UI::ViewGroup *viewGroup); - std::string username_; std::string password_; }; diff --git a/Windows/InputBox.cpp b/Windows/InputBox.cpp index d539798774..14b7fb47d1 100644 --- a/Windows/InputBox.cpp +++ b/Windows/InputBox.cpp @@ -8,7 +8,6 @@ struct DialogBoxParams { std::wstring textBoxContents; - std::wstring passwordContents; std::wstring out; std::wstring windowTitle; bool defaultSelected; @@ -117,22 +116,25 @@ static INT_PTR CALLBACK UserPasswordBoxFunc(HWND hDlg, UINT message, WPARAM wPar { switch (message) { case WM_INITDIALOG: - SetWindowText(GetDlgItem(hDlg, IDC_INPUTBOX), L""); + SetWindowText(GetDlgItem(hDlg, IDC_INPUTBOX), g_params.textBoxContents.c_str()); SetWindowText(GetDlgItem(hDlg, IDC_PASSWORDBOX), L""); SetWindowText(hDlg, g_params.windowTitle.c_str()); PostMessage(GetDlgItem(hDlg, IDC_INPUTBOX), EM_SETSEL, -1, -1); PostMessage(GetDlgItem(hDlg, IDC_PASSWORDBOX), EM_SETSEL, -1, -1); W32Util::CenterWindow(hDlg); + if (!g_params.textBoxContents.empty()) { + SetFocus(GetDlgItem(hDlg, IDC_PASSWORDBOX)); + return FALSE; // We don't want the caller to set the focus for us. + } return TRUE; case WM_COMMAND: - switch (wParam) - { + switch (wParam) { case IDOK: { wchar_t temp[256]; - GetWindowText(GetDlgItem(hDlg, IDC_INPUTBOX), temp, sizeof(temp)); + GetWindowText(GetDlgItem(hDlg, IDC_INPUTBOX), temp, ARRAY_SIZE(temp)); g_params.userName = ConvertWStringToUTF8(temp); - GetWindowText(GetDlgItem(hDlg, IDC_PASSWORDBOX), temp, sizeof(temp)); + GetWindowText(GetDlgItem(hDlg, IDC_PASSWORDBOX), temp, ARRAY_SIZE(temp)); g_params.passWord = ConvertWStringToUTF8(temp); EndDialog(hDlg, IDOK); return TRUE; @@ -149,9 +151,9 @@ static INT_PTR CALLBACK UserPasswordBoxFunc(HWND hDlg, UINT message, WPARAM wPar } bool UserPasswordBox_GetStrings(HINSTANCE hInst, HWND hParent, const wchar_t *title, std::string *username, std::string *password) { - g_params.windowTitle = title; + g_params.textBoxContents = ConvertUTF8ToWString(*username); + g_params.userName = *username; INT_PTR value = DialogBox(hInst, (LPCWSTR)IDD_USERPASSWORDBOX, hParent, UserPasswordBoxFunc); - if (value == IDOK) { *username = g_params.userName; *password = g_params.passWord; diff --git a/Windows/main.cpp b/Windows/main.cpp index 00bdf4ad84..4c976b9591 100644 --- a/Windows/main.cpp +++ b/Windows/main.cpp @@ -619,7 +619,7 @@ bool System_MakeRequest(SystemRequestType type, int requestId, const std::string return true; case SystemRequestType::ASK_USERNAME_PASSWORD: std::thread([=] { - std::string username; + std::string username = param2; std::string password; if (UserPasswordBox_GetStrings(MainWindow::GetHInstance(), MainWindow::GetHWND(), ConvertUTF8ToWString(param1).c_str(), &username, &password)) { g_requestManager.PostSystemSuccess(requestId, (username + '\n' + password).c_str());