From 9f588b74062d08e5802e7c7570396c24e98e7ae6 Mon Sep 17 00:00:00 2001 From: pajlada Date: Sat, 24 Aug 2024 15:02:08 +0200 Subject: [PATCH] fix: Fixed account switch not being saved if no other settings were changed (#5558) --- CHANGELOG.md | 1 + benchmarks/src/Highlights.cpp | 3 --- benchmarks/src/main.cpp | 5 +++- mocks/include/mocks/BaseApplication.hpp | 6 +++-- src/RunGui.cpp | 5 +--- src/main.cpp | 2 +- src/singletons/Settings.cpp | 16 +++++++++++-- src/singletons/Settings.hpp | 11 ++++++++- src/widgets/AccountSwitchWidget.cpp | 3 +++ src/widgets/dialogs/SettingsDialog.cpp | 4 +++- tests/src/Filters.cpp | 7 +++--- tests/src/HighlightController.cpp | 9 +++---- tests/src/InputCompletion.cpp | 31 +++++++------------------ tests/src/MessageBuilder.cpp | 14 +++-------- tests/src/ModerationAction.cpp | 11 +++------ tests/src/NotebookTab.cpp | 9 +++---- tests/src/main.cpp | 5 +++- 17 files changed, 69 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5baac202..f250de55c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ - Bugfix: Links with invalid characters in the domain are no longer detected. (#5509) - Bugfix: Fixed janky selection for messages with RTL segments (selection is still wrong, but consistently wrong). (#5525) - Bugfix: Fixed tab visibility being controllable in the emote popup. (#5530) +- Bugfix: Fixed account switch not being saved if no other settings were changed. (#5558) - Dev: Update Windows build from Qt 6.5.0 to Qt 6.7.1. (#5420) - Dev: Update vcpkg build Qt from 6.5.0 to 6.7.0, boost from 1.83.0 to 1.85.0, openssl from 3.1.3 to 3.3.0. (#5422) - Dev: Unsingletonize `ISoundController`. (#5462) diff --git a/benchmarks/src/Highlights.cpp b/benchmarks/src/Highlights.cpp index 9eb4a7370..f6126ecae 100644 --- a/benchmarks/src/Highlights.cpp +++ b/benchmarks/src/Highlights.cpp @@ -6,7 +6,6 @@ #include "messages/Message.hpp" #include "messages/MessageBuilder.hpp" #include "mocks/BaseApplication.hpp" -#include "singletons/Settings.hpp" #include "util/Helpers.hpp" #include @@ -72,8 +71,6 @@ public: static void BM_HighlightTest(benchmark::State &state) { MockApplication mockApplication; - QTemporaryDir settingsDir; - Settings settings(settingsDir.path()); std::string message = R"(@badge-info=subscriber/34;badges=moderator/1,subscriber/24;color=#FF0000;display-name=테스트계정420;emotes=41:6-13,15-22;flags=;id=a3196c7e-be4c-4b49-9c5a-8b8302b50c2a;mod=1;room-id=11148817;subscriber=1;tmi-sent-ts=1590922213730;turbo=0;user-id=117166826;user-type=mod :testaccount_420!testaccount_420@testaccount_420.tmi.twitch.tv PRIVMSG #pajlada :-tags Kreygasm,Kreygasm (no space))"; diff --git a/benchmarks/src/main.cpp b/benchmarks/src/main.cpp index ac3ca2b8e..9c7895a83 100644 --- a/benchmarks/src/main.cpp +++ b/benchmarks/src/main.cpp @@ -1,3 +1,4 @@ +#include "common/Args.hpp" #include "singletons/Resources.hpp" #include "singletons/Settings.hpp" @@ -16,10 +17,12 @@ int main(int argc, char **argv) ::benchmark::Initialize(&argc, argv); + Args args; + // Ensure settings are initialized before any benchmarks are run QTemporaryDir settingsDir; settingsDir.setAutoRemove(false); // we'll remove it manually - chatterino::Settings settings(settingsDir.path()); + chatterino::Settings settings(args, settingsDir.path()); QTimer::singleShot(0, [&]() { ::benchmark::RunSpecifiedBenchmarks(); diff --git a/mocks/include/mocks/BaseApplication.hpp b/mocks/include/mocks/BaseApplication.hpp index b858a87a1..677492130 100644 --- a/mocks/include/mocks/BaseApplication.hpp +++ b/mocks/include/mocks/BaseApplication.hpp @@ -1,5 +1,6 @@ #pragma once +#include "common/Args.hpp" #include "mocks/DisabledStreamerMode.hpp" #include "mocks/EmptyApplication.hpp" #include "providers/bttv/BttvLiveUpdates.hpp" @@ -16,13 +17,13 @@ class BaseApplication : public EmptyApplication { public: BaseApplication() - : settings(this->settingsDir.filePath("settings.json")) + : settings(this->args, this->settingsDir.path()) { } explicit BaseApplication(const QString &settingsData) : EmptyApplication(settingsData) - , settings(this->settingsDir.filePath("settings.json")) + , settings(this->args, this->settingsDir.path()) { } @@ -41,6 +42,7 @@ public: return nullptr; } + Args args; Settings settings; DisabledStreamerMode streamerMode; }; diff --git a/src/RunGui.cpp b/src/RunGui.cpp index 4863f4da7..62e010d9f 100644 --- a/src/RunGui.cpp +++ b/src/RunGui.cpp @@ -266,10 +266,7 @@ void runGui(QApplication &a, const Paths &paths, Settings &settings, app.run(a); app.save(); - if (!args.dontSaveSettings) - { - pajlada::Settings::SettingManager::gSave(); - } + settings.requestSave(); chatterino::NetworkManager::deinit(); diff --git a/src/main.cpp b/src/main.cpp index 0e4d49968..02dc4623e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -117,7 +117,7 @@ int main(int argc, char **argv) IvrApi::initialize(); Helix::initialize(); - Settings settings(paths->settingsDirectory); + Settings settings(args, paths->settingsDirectory); runGui(a, *paths, settings, args, updates); } diff --git a/src/singletons/Settings.cpp b/src/singletons/Settings.cpp index 09e2b410a..a62f46585 100644 --- a/src/singletons/Settings.cpp +++ b/src/singletons/Settings.cpp @@ -1,6 +1,7 @@ #include "singletons/Settings.hpp" #include "Application.hpp" +#include "common/Args.hpp" #include "controllers/filters/FilterRecord.hpp" #include "controllers/highlights/HighlightBadge.hpp" #include "controllers/highlights/HighlightBlacklistUser.hpp" @@ -140,8 +141,9 @@ bool Settings::toggleMutedChannel(const QString &channelName) Settings *Settings::instance_ = nullptr; -Settings::Settings(const QString &settingsDirectory) +Settings::Settings(const Args &args, const QString &settingsDirectory) : prevInstance_(Settings::instance_) + , disableSaving(args.dontSaveSettings) { QString settingsPath = settingsDirectory + "/settings.json"; @@ -153,7 +155,7 @@ Settings::Settings(const QString &settingsDirectory) settingsInstance->setBackupEnabled(true); settingsInstance->setBackupSlots(9); settingsInstance->saveMethod = - pajlada::Settings::SettingManager::SaveMethod::SaveOnExit; + pajlada::Settings::SettingManager::SaveMethod::SaveManually; initializeSignalVector(this->signalHolder, this->highlightedMessagesSetting, this->highlightedMessages); @@ -193,6 +195,16 @@ Settings::~Settings() Settings::instance_ = this->prevInstance_; } +void Settings::requestSave() const +{ + if (this->disableSaving) + { + return; + } + + pajlada::Settings::SettingManager::gSave(); +} + void Settings::saveSnapshot() { BenchmarkGuard benchmark("Settings::saveSnapshot"); diff --git a/src/singletons/Settings.hpp b/src/singletons/Settings.hpp index 889ecb958..39769ad9b 100644 --- a/src/singletons/Settings.hpp +++ b/src/singletons/Settings.hpp @@ -25,6 +25,8 @@ using TimeoutButton = std::pair; namespace chatterino { +class Args; + #ifdef Q_OS_WIN32 # define DEFAULT_FONT_FAMILY "Segoe UI" # define DEFAULT_FONT_SIZE 10 @@ -80,12 +82,19 @@ class Settings static Settings *instance_; Settings *prevInstance_ = nullptr; + const bool disableSaving; + public: - Settings(const QString &settingsDirectory); + Settings(const Args &args, const QString &settingsDirectory); ~Settings(); static Settings &instance(); + /// Request the settings to be saved to file + /// + /// Depending on the launch options, a save might end up not happening + void requestSave() const; + void saveSnapshot(); void restoreSnapshot(); diff --git a/src/widgets/AccountSwitchWidget.cpp b/src/widgets/AccountSwitchWidget.cpp index 01fc161bc..5afd68392 100644 --- a/src/widgets/AccountSwitchWidget.cpp +++ b/src/widgets/AccountSwitchWidget.cpp @@ -5,6 +5,7 @@ #include "controllers/accounts/AccountController.hpp" #include "providers/twitch/TwitchAccount.hpp" #include "providers/twitch/TwitchCommon.hpp" +#include "singletons/Settings.hpp" namespace chatterino { @@ -54,6 +55,8 @@ AccountSwitchWidget::AccountSwitchWidget(QWidget *parent) { app->getAccounts()->twitch.currentUsername = newUsername; } + + getSettings()->requestSave(); } }); } diff --git a/src/widgets/dialogs/SettingsDialog.cpp b/src/widgets/dialogs/SettingsDialog.cpp index fa0d2bd55..2a4af662e 100644 --- a/src/widgets/dialogs/SettingsDialog.cpp +++ b/src/widgets/dialogs/SettingsDialog.cpp @@ -435,8 +435,10 @@ void SettingsDialog::onOkClicked() if (!getApp()->getArgs().dontSaveSettings) { getApp()->getCommands()->save(); - pajlada::Settings::SettingManager::gSave(); } + + getSettings()->requestSave(); + this->close(); } diff --git a/tests/src/Filters.cpp b/tests/src/Filters.cpp index 132bd18cb..dd592b3b6 100644 --- a/tests/src/Filters.cpp +++ b/tests/src/Filters.cpp @@ -4,6 +4,7 @@ #include "controllers/filters/lang/Types.hpp" #include "controllers/highlights/HighlightController.hpp" #include "messages/MessageBuilder.hpp" +#include "mocks/BaseApplication.hpp" #include "mocks/Channel.hpp" #include "mocks/ChatterinoBadges.hpp" #include "mocks/EmptyApplication.hpp" @@ -26,12 +27,11 @@ TypingContext typingContext = MESSAGE_TYPING_CONTEXT; namespace { -class MockApplication : mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: MockApplication() - : settings(this->settingsDir.filePath("settings.json")) - , highlights(this->settings, &this->accounts) + : highlights(this->settings, &this->accounts) { } @@ -75,7 +75,6 @@ public: return &this->highlights; } - Settings settings; AccountController accounts; Emotes emotes; mock::UserDataController userData; diff --git a/tests/src/HighlightController.cpp b/tests/src/HighlightController.cpp index 76f92db18..d36713a90 100644 --- a/tests/src/HighlightController.cpp +++ b/tests/src/HighlightController.cpp @@ -3,12 +3,11 @@ #include "controllers/accounts/AccountController.hpp" #include "controllers/highlights/HighlightPhrase.hpp" #include "messages/MessageBuilder.hpp" // for MessageParseArgs -#include "mocks/EmptyApplication.hpp" +#include "mocks/BaseApplication.hpp" #include "mocks/Helix.hpp" #include "mocks/UserData.hpp" #include "providers/twitch/api/Helix.hpp" #include "providers/twitch/TwitchBadge.hpp" // for Badge -#include "singletons/Settings.hpp" #include "Test.hpp" #include @@ -22,12 +21,11 @@ using ::testing::Exactly; namespace { -class MockApplication : public mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: MockApplication(const QString &settingsBody) - : mock::EmptyApplication(settingsBody) - , settings(this->settingsDir.path()) + : mock::BaseApplication(settingsBody) , highlights(this->settings, &this->accounts) { } @@ -47,7 +45,6 @@ public: return &this->userData; } - Settings settings; AccountController accounts; HighlightController highlights; mock::UserDataController userData; diff --git a/tests/src/InputCompletion.cpp b/tests/src/InputCompletion.cpp index 2b987a5e1..460fb1526 100644 --- a/tests/src/InputCompletion.cpp +++ b/tests/src/InputCompletion.cpp @@ -5,8 +5,8 @@ #include "controllers/completion/strategies/ClassicUserStrategy.hpp" #include "controllers/completion/strategies/Strategy.hpp" #include "messages/Emote.hpp" +#include "mocks/BaseApplication.hpp" #include "mocks/Channel.hpp" -#include "mocks/EmptyApplication.hpp" #include "mocks/Helix.hpp" #include "mocks/TwitchIrcServer.hpp" #include "singletons/Emotes.hpp" @@ -31,9 +31,14 @@ namespace { using namespace chatterino::completion; using ::testing::Exactly; -class MockApplication : mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: + explicit MockApplication(const QString &settingsData) + : BaseApplication(settingsData) + { + } + AccountController *getAccounts() override { return &this->accounts; @@ -110,24 +115,14 @@ class InputCompletionTest : public ::testing::Test protected: void SetUp() override { - // Write default settings to the mock settings json file - this->settingsDir_ = std::make_unique(); - - QFile settingsFile(this->settingsDir_->filePath("settings.json")); - ASSERT_TRUE(settingsFile.open(QIODevice::WriteOnly | QIODevice::Text)); - ASSERT_GT(settingsFile.write(DEFAULT_SETTINGS.toUtf8()), 0); - ASSERT_TRUE(settingsFile.flush()); - settingsFile.close(); - // Initialize helix client this->mockHelix = std::make_unique(); initializeHelix(this->mockHelix.get()); EXPECT_CALL(*this->mockHelix, loadBlocks).Times(Exactly(1)); EXPECT_CALL(*this->mockHelix, update).Times(Exactly(1)); - this->mockApplication = std::make_unique(); - this->settings = std::make_unique(this->settingsDir_->path()); - this->paths = std::make_unique(); + this->mockApplication = + std::make_unique(DEFAULT_SETTINGS); this->mockApplication->accounts.load(); @@ -139,19 +134,11 @@ protected: void TearDown() override { this->mockApplication.reset(); - this->settings.reset(); - this->paths.reset(); this->mockHelix.reset(); this->channelPtr.reset(); - - this->settingsDir_.reset(); } - std::unique_ptr settingsDir_; - std::unique_ptr mockApplication; - std::unique_ptr settings; - std::unique_ptr paths; std::unique_ptr mockHelix; ChannelPtr channelPtr; diff --git a/tests/src/MessageBuilder.cpp b/tests/src/MessageBuilder.cpp index 8a8ea66ac..39f01f196 100644 --- a/tests/src/MessageBuilder.cpp +++ b/tests/src/MessageBuilder.cpp @@ -3,10 +3,10 @@ #include "controllers/accounts/AccountController.hpp" #include "controllers/highlights/HighlightController.hpp" #include "controllers/ignores/IgnorePhrase.hpp" +#include "mocks/BaseApplication.hpp" #include "mocks/Channel.hpp" #include "mocks/ChatterinoBadges.hpp" #include "mocks/DisabledStreamerMode.hpp" -#include "mocks/EmptyApplication.hpp" #include "mocks/TwitchIrcServer.hpp" #include "mocks/UserData.hpp" #include "providers/ffz/FfzBadges.hpp" @@ -28,12 +28,11 @@ using chatterino::mock::MockChannel; namespace { -class MockApplication : mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: MockApplication() - : settings(this->settingsDir.filePath("settings.json")) - , highlights(this->settings, &this->accounts) + : highlights(this->settings, &this->accounts) { } @@ -92,12 +91,6 @@ public: return &this->seventvEmotes; } - IStreamerMode *getStreamerMode() override - { - return &this->streamerMode; - } - - Settings settings; AccountController accounts; Emotes emotes; mock::UserDataController userData; @@ -109,7 +102,6 @@ public: BttvEmotes bttvEmotes; FfzEmotes ffzEmotes; SeventvEmotes seventvEmotes; - DisabledStreamerMode streamerMode; }; } // namespace diff --git a/tests/src/ModerationAction.cpp b/tests/src/ModerationAction.cpp index 75daf8e3e..39118fe5f 100644 --- a/tests/src/ModerationAction.cpp +++ b/tests/src/ModerationAction.cpp @@ -1,10 +1,9 @@ #include "controllers/moderationactions/ModerationAction.hpp" #include "messages/Image.hpp" -#include "mocks/EmptyApplication.hpp" +#include "mocks/BaseApplication.hpp" #include "singletons/Emotes.hpp" #include "singletons/Resources.hpp" -#include "singletons/Settings.hpp" #include "Test.hpp" #include @@ -15,20 +14,16 @@ using namespace std::chrono_literals; namespace { -class MockApplication : mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: - MockApplication() - : settings(this->settingsDir.filePath("settings.json")) - { - } + MockApplication() = default; IEmotes *getEmotes() override { return &this->emotes; } - Settings settings; Emotes emotes; }; diff --git a/tests/src/NotebookTab.cpp b/tests/src/NotebookTab.cpp index 2c6287ba2..a3b751632 100644 --- a/tests/src/NotebookTab.cpp +++ b/tests/src/NotebookTab.cpp @@ -3,9 +3,8 @@ #include "common/Literals.hpp" #include "controllers/hotkeys/HotkeyController.hpp" #include "gmock/gmock.h" -#include "mocks/EmptyApplication.hpp" +#include "mocks/BaseApplication.hpp" #include "singletons/Fonts.hpp" -#include "singletons/Settings.hpp" #include "singletons/Theme.hpp" #include "Test.hpp" #include "widgets/Notebook.hpp" @@ -18,12 +17,11 @@ using ::testing::Exactly; namespace { -class MockApplication : mock::EmptyApplication +class MockApplication : public mock::BaseApplication { public: MockApplication() - : settings(this->settingsDir.filePath("settings.json")) - , theme(this->paths_) + : theme(this->paths_) , fonts(this->settings) { } @@ -42,7 +40,6 @@ public: return &this->fonts; } - Settings settings; Theme theme; HotkeyController hotkeys; Fonts fonts; diff --git a/tests/src/main.cpp b/tests/src/main.cpp index 6c82f632c..75e22571a 100644 --- a/tests/src/main.cpp +++ b/tests/src/main.cpp @@ -1,3 +1,4 @@ +#include "common/Args.hpp" #include "common/network/NetworkManager.hpp" #include "singletons/Resources.hpp" #include "singletons/Settings.hpp" @@ -29,10 +30,12 @@ int main(int argc, char **argv) chatterino::NetworkManager::init(); + Args args; + // Ensure settings are initialized before any tests are run QTemporaryDir settingsDir; settingsDir.setAutoRemove(false); // we'll remove it manually - chatterino::Settings settings(settingsDir.path()); + chatterino::Settings settings(args, settingsDir.path()); QTimer::singleShot(0, [&]() { auto res = RUN_ALL_TESTS();