refactor: some more Application refactors (#5551)

CHANGELOG.md

@@ -74,6 +74,7 @@
 - Dev: Refactored a few `#define`s into `const(expr)` and cleaned includes. (#5527)
 - Dev: Added `FlagsEnum::isEmpty`. (#5550)
 - Dev: Prepared for Qt 6.8 by addressing some deprecations. (#5529)
+- Dev: Moved some responsibility away from Application into WindowManager. (#5551)
 - Dev: Fixed benchmarks segfaulting on run. (#5559)
 - Dev: Refactored `MessageBuilder` to be a single class. (#5548)
 - Dev: Recent changes are now shown in the nightly release description. (#5553, #5554)

mocks/include/mocks/BaseApplication.hpp

@@ -4,7 +4,9 @@
 #include "mocks/DisabledStreamerMode.hpp"
 #include "mocks/EmptyApplication.hpp"
 #include "providers/bttv/BttvLiveUpdates.hpp"
+#include "singletons/Fonts.hpp"
 #include "singletons/Settings.hpp"
+#include "singletons/Theme.hpp"
 
 #include <QString>
 
@@ -18,12 +20,16 @@ class BaseApplication : public EmptyApplication
 public:
     BaseApplication()
         : settings(this->args, this->settingsDir.path())
+        , theme(this->paths_)
+        , fonts(this->settings)
     {
     }
 
     explicit BaseApplication(const QString &settingsData)
         : EmptyApplication(settingsData)
         , settings(this->args, this->settingsDir.path())
+        , theme(this->paths_)
+        , fonts(this->settings)
     {
     }
 
@@ -32,6 +38,16 @@ public:
         return &this->streamerMode;
     }
 
+    Theme *getThemes() override
+    {
+        return &this->theme;
+    }
+
+    Fonts *getFonts() override
+    {
+        return &this->fonts;
+    }
+
     BttvLiveUpdates *getBttvLiveUpdates() override
     {
         return nullptr;
@@ -45,6 +61,8 @@ public:
     Args args;
     Settings settings;
     DisabledStreamerMode streamerMode;
+    Theme theme;
+    Fonts fonts;
 };
 
 }  // namespace chatterino::mock

src/Application.cpp

@@ -153,7 +153,7 @@ Application::Application(Settings &_settings, const Paths &paths,
     , emotes(new Emotes)
     , accounts(new AccountController)
     , hotkeys(new HotkeyController)
-    , windows(new WindowManager(paths))
+    , windows(new WindowManager(paths, _settings, *this->themes, *this->fonts))
     , toasts(new Toasts)
     , imageUploader(new ImageUploader)
     , seventvAPI(new SeventvAPI)
@@ -184,11 +184,6 @@ Application::Application(Settings &_settings, const Paths &paths,
 #endif
     , updates(_updates)
 {
-    // We can safely ignore this signal's connection since the Application will always
-    // be destroyed after fonts
-    std::ignore = this->fonts->fontChanged.connect([this]() {
-        this->windows->layoutChannelViews();
-    });
 }
 
 Application::~Application()
@@ -225,9 +220,15 @@ void Application::initialize(Settings &settings, const Paths &paths)
 
     this->accounts->load();
 
-    this->windows->initialize(settings);
+    this->windows->initialize();
 
     this->ffzBadges->load();
+
+    // Load global emotes
+    this->bttvEmotes->loadEmotes();
+    this->ffzEmotes->loadEmotes();
+    this->seventvEmotes->loadGlobalEmotes();
+
     this->twitch->initialize();
 
     // Load live status
@@ -266,8 +267,6 @@ void Application::initialize(Settings &settings, const Paths &paths)
     }
 #endif
 
-    this->windows->updateWordTypeMask();
-
     if (!this->args_.isFramelessEmbed)
     {
         this->initNm(paths);
@@ -297,34 +296,9 @@ int Application::run(QApplication &qtApp)
         },
         false);
 
-    // We can safely ignore the signal connections since Application will always live longer than
-    // everything else, including settings. right?
-    // NOTE: SETTINGS_LIFETIME
-    std::ignore =
-        getSettings()->moderationActions.delayedItemsChanged.connect([this] {
-            this->windows->forceLayoutChannelViews();
-        });
-
-    std::ignore =
-        getSettings()->highlightedMessages.delayedItemsChanged.connect([this] {
-            this->windows->forceLayoutChannelViews();
-        });
-    std::ignore =
-        getSettings()->highlightedUsers.delayedItemsChanged.connect([this] {
-            this->windows->forceLayoutChannelViews();
-        });
-    std::ignore =
-        getSettings()->highlightedBadges.delayedItemsChanged.connect([this] {
-            this->windows->forceLayoutChannelViews();
-        });
-
-    getSettings()->removeSpacesBetweenEmotes.connect([this] {
-        this->windows->forceLayoutChannelViews();
-    });
-
     getSettings()->enableBTTVGlobalEmotes.connect(
         [this] {
-            this->twitch->reloadBTTVGlobalEmotes();
+            this->bttvEmotes->loadEmotes();
         },
         false);
     getSettings()->enableBTTVChannelEmotes.connect(
@@ -334,7 +308,7 @@ int Application::run(QApplication &qtApp)
         false);
     getSettings()->enableFFZGlobalEmotes.connect(
         [this] {
-            this->twitch->reloadFFZGlobalEmotes();
+            this->ffzEmotes->loadEmotes();
         },
         false);
     getSettings()->enableFFZChannelEmotes.connect(
@@ -344,7 +318,7 @@ int Application::run(QApplication &qtApp)
         false);
     getSettings()->enableSevenTVGlobalEmotes.connect(
         [this] {
-            this->twitch->reloadSevenTVGlobalEmotes();
+            this->seventvEmotes->loadGlobalEmotes();
         },
         false);
     getSettings()->enableSevenTVChannelEmotes.connect(

src/CMakeLists.txt

@@ -498,6 +498,7 @@ set(SOURCE_FILES
         util/SampleData.cpp
         util/SampleData.hpp
         util/SharedPtrElementLess.hpp
+        util/SignalListener.hpp
         util/StreamLink.cpp
         util/StreamLink.hpp
         util/ThreadGuard.hpp

src/common/ChatterinoSetting.hpp

@@ -141,4 +141,14 @@ public:
     using pajlada::Settings::Setting<QString>::operator QString;
 };
 
+template <typename T>
+struct IsChatterinoSettingT : std::false_type {
+};
+template <typename T>
+struct IsChatterinoSettingT<ChatterinoSetting<T>> : std::true_type {
+};
+
+template <typename T>
+concept IsChatterinoSetting = IsChatterinoSettingT<T>::value;
+
 }  // namespace chatterino

src/providers/twitch/TwitchIrcServer.cpp

@@ -230,10 +230,6 @@ void TwitchIrcServer::initialize()
             this->connect();
         });
     });
-
-    this->reloadBTTVGlobalEmotes();
-    this->reloadFFZGlobalEmotes();
-    this->reloadSevenTVGlobalEmotes();
 }
 
 void TwitchIrcServer::initializeConnection(IrcConnection *connection,
@@ -806,11 +802,6 @@ void TwitchIrcServer::setLastUserThatWhisperedMe(const QString &user)
     this->lastUserThatWhisperedMe.set(user);
 }
 
-void TwitchIrcServer::reloadBTTVGlobalEmotes()
-{
-    getApp()->getBttvEmotes()->loadEmotes();
-}
-
 void TwitchIrcServer::reloadAllBTTVChannelEmotes()
 {
     this->forEachChannel([](const auto &chan) {
@@ -821,11 +812,6 @@ void TwitchIrcServer::reloadAllBTTVChannelEmotes()
     });
 }
 
-void TwitchIrcServer::reloadFFZGlobalEmotes()
-{
-    getApp()->getFfzEmotes()->loadEmotes();
-}
-
 void TwitchIrcServer::reloadAllFFZChannelEmotes()
 {
     this->forEachChannel([](const auto &chan) {
@@ -836,11 +822,6 @@ void TwitchIrcServer::reloadAllFFZChannelEmotes()
     });
 }
 
-void TwitchIrcServer::reloadSevenTVGlobalEmotes()
-{
-    getApp()->getSeventvEmotes()->loadGlobalEmotes();
-}
-
 void TwitchIrcServer::reloadAllSevenTVChannelEmotes()
 {
     this->forEachChannel([](const auto &chan) {

src/providers/twitch/TwitchIrcServer.hpp

@@ -95,11 +95,8 @@ public:
     std::shared_ptr<Channel> getChannelOrEmptyByID(
         const QString &channelID) override;
 
-    void reloadBTTVGlobalEmotes();
     void reloadAllBTTVChannelEmotes();
-    void reloadFFZGlobalEmotes();
     void reloadAllFFZChannelEmotes();
-    void reloadSevenTVGlobalEmotes();
     void reloadAllSevenTVChannelEmotes();
 
     /** Calls `func` with all twitch channels that have `emoteSetId` added. */

src/singletons/WindowManager.cpp

@@ -10,6 +10,7 @@
 #include "singletons/Settings.hpp"
 #include "singletons/Theme.hpp"
 #include "util/CombinePath.hpp"
+#include "util/SignalListener.hpp"
 #include "widgets/AccountSwitchPopup.hpp"
 #include "widgets/dialogs/SettingsDialog.hpp"
 #include "widgets/FramelessEmbedWindow.hpp"
@@ -86,29 +87,67 @@ void WindowManager::showAccountSelectPopup(QPoint point)
     w->setFocus();
 }
 
-WindowManager::WindowManager(const Paths &paths)
+WindowManager::WindowManager(const Paths &paths, Settings &settings,
-    : windowLayoutFilePath(combinePath(paths.settingsDirectory,
+                             Theme &themes_, Fonts &fonts)
+    : themes(themes_)
+    , windowLayoutFilePath(combinePath(paths.settingsDirectory,
                                        WindowManager::WINDOW_LAYOUT_FILENAME))
+    , updateWordTypeMaskListener([this] {
+        this->updateWordTypeMask();
+    })
+    , forceLayoutChannelViewsListener([this] {
+        this->forceLayoutChannelViews();
+    })
+    , layoutChannelViewsListener([this] {
+        this->layoutChannelViews();
+    })
+    , invalidateChannelViewBuffersListener([this] {
+        this->invalidateChannelViewBuffers();
+    })
+    , repaintVisibleChatWidgetsListener([this] {
+        this->repaintVisibleChatWidgets();
+    })
 {
     qCDebug(chatterinoWindowmanager) << "init WindowManager";
 
-    auto *settings = getSettings();
+    this->updateWordTypeMaskListener.add(settings.showTimestamps);
+    this->updateWordTypeMaskListener.add(settings.showBadgesGlobalAuthority);
+    this->updateWordTypeMaskListener.add(settings.showBadgesPredictions);
+    this->updateWordTypeMaskListener.add(settings.showBadgesChannelAuthority);
+    this->updateWordTypeMaskListener.add(settings.showBadgesSubscription);
+    this->updateWordTypeMaskListener.add(settings.showBadgesVanity);
+    this->updateWordTypeMaskListener.add(settings.showBadgesChatterino);
+    this->updateWordTypeMaskListener.add(settings.showBadgesFfz);
+    this->updateWordTypeMaskListener.add(settings.showBadgesSevenTV);
+    this->updateWordTypeMaskListener.add(settings.enableEmoteImages);
+    this->updateWordTypeMaskListener.add(settings.lowercaseDomains);
+    this->updateWordTypeMaskListener.add(settings.showReplyButton);
 
-    this->wordFlagsListener_.addSetting(settings->showTimestamps);
+    this->forceLayoutChannelViewsListener.add(
-    this->wordFlagsListener_.addSetting(settings->showBadgesGlobalAuthority);
+        settings.moderationActions.delayedItemsChanged);
-    this->wordFlagsListener_.addSetting(settings->showBadgesPredictions);
+    this->forceLayoutChannelViewsListener.add(
-    this->wordFlagsListener_.addSetting(settings->showBadgesChannelAuthority);
+        settings.highlightedMessages.delayedItemsChanged);
-    this->wordFlagsListener_.addSetting(settings->showBadgesSubscription);
+    this->forceLayoutChannelViewsListener.add(
-    this->wordFlagsListener_.addSetting(settings->showBadgesVanity);
+        settings.highlightedUsers.delayedItemsChanged);
-    this->wordFlagsListener_.addSetting(settings->showBadgesChatterino);
+    this->forceLayoutChannelViewsListener.add(
-    this->wordFlagsListener_.addSetting(settings->showBadgesFfz);
+        settings.highlightedBadges.delayedItemsChanged);
-    this->wordFlagsListener_.addSetting(settings->showBadgesSevenTV);
+    this->forceLayoutChannelViewsListener.add(
-    this->wordFlagsListener_.addSetting(settings->enableEmoteImages);
+        settings.removeSpacesBetweenEmotes);
-    this->wordFlagsListener_.addSetting(settings->lowercaseDomains);
+    this->forceLayoutChannelViewsListener.add(settings.emoteScale);
-    this->wordFlagsListener_.addSetting(settings->showReplyButton);
+    this->forceLayoutChannelViewsListener.add(settings.timestampFormat);
-    this->wordFlagsListener_.setCB([this] {
+    this->forceLayoutChannelViewsListener.add(settings.collpseMessagesMinLines);
-        this->updateWordTypeMask();
+    this->forceLayoutChannelViewsListener.add(settings.enableRedeemedHighlight);
-    });
+    this->forceLayoutChannelViewsListener.add(settings.colorUsernames);
+    this->forceLayoutChannelViewsListener.add(settings.boldUsernames);
+
+    this->layoutChannelViewsListener.add(settings.timestampFormat);
+    this->layoutChannelViewsListener.add(fonts.fontChanged);
+
+    this->invalidateChannelViewBuffersListener.add(settings.alternateMessages);
+    this->invalidateChannelViewBuffersListener.add(settings.separateMessages);
+
+    this->repaintVisibleChatWidgetsListener.add(
+        this->themes.repaintVisibleChatWidgets_);
 
     this->saveTimer = new QTimer;
 
@@ -117,6 +156,8 @@ WindowManager::WindowManager(const Paths &paths)
     QObject::connect(this->saveTimer, &QTimer::timeout, [] {
         getApp()->getWindows()->save();
     });
+
+    this->updateWordTypeMask();
 }
 
 WindowManager::~WindowManager() = default;
@@ -338,18 +379,10 @@ void WindowManager::setEmotePopupBounds(QRect bounds)
     }
 }
 
-void WindowManager::initialize(Settings &settings)
+void WindowManager::initialize()
 {
     assertInGuiThread();
 
-    // We can safely ignore this signal connection since both Themes and WindowManager
-    // share the Application state lifetime
-    // NOTE: APPLICATION_LIFETIME
-    std::ignore =
-        getApp()->getThemes()->repaintVisibleChatWidgets_.connect([this] {
-            this->repaintVisibleChatWidgets();
-        });
-
     {
         WindowLayout windowLayout;
 
@@ -391,37 +424,6 @@ void WindowManager::initialize(Settings &settings)
             this->mainWindow_->hide();
         }
     }
-
-    settings.timestampFormat.connect([this](auto, auto) {
-        this->layoutChannelViews();
-    });
-
-    settings.emoteScale.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
-
-    settings.timestampFormat.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
-    settings.alternateMessages.connect([this](auto, auto) {
-        this->invalidateChannelViewBuffers();
-    });
-    settings.separateMessages.connect([this](auto, auto) {
-        this->invalidateChannelViewBuffers();
-    });
-    settings.collpseMessagesMinLines.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
-    settings.enableRedeemedHighlight.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
-
-    settings.colorUsernames.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
-    settings.boldUsernames.connect([this](auto, auto) {
-        this->forceLayoutChannelViews();
-    });
 }
 
 void WindowManager::save()

src/singletons/WindowManager.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "common/FlagsEnum.hpp"
+#include "util/SignalListener.hpp"
 #include "widgets/splits/SplitContainer.hpp"
 
 #include <pajlada/settings/settinglistener.hpp>
@@ -23,6 +24,8 @@ using ChannelPtr = std::shared_ptr<Channel>;
 struct Message;
 using MessagePtr = std::shared_ptr<const Message>;
 class WindowLayout;
+class Theme;
+class Fonts;
 
 enum class MessageElementFlag : int64_t;
 using MessageElementFlags = FlagsEnum<MessageElementFlag>;
@@ -33,10 +36,13 @@ class FramelessEmbedWindow;
 
 class WindowManager final
 {
+    Theme &themes;
+
 public:
     static const QString WINDOW_LAYOUT_FILENAME;
 
-    explicit WindowManager(const Paths &paths);
+    explicit WindowManager(const Paths &paths, Settings &settings,
+                           Theme &themes_, Fonts &fonts);
     ~WindowManager();
 
     WindowManager(const WindowManager &) = delete;
@@ -102,7 +108,7 @@ public:
     void setEmotePopupBounds(QRect bounds);
 
     // Set up some final signals & actually show the windows
-    void initialize(Settings &settings);
+    void initialize();
     void save();
     void closeAll();
 
@@ -164,10 +170,17 @@ private:
     Window *selectedWindow_{};
 
     MessageElementFlags wordFlags_{};
-    pajlada::SettingListener wordFlagsListener_;
 
     QTimer *saveTimer;
 
+    pajlada::Signals::SignalHolder signalHolder;
+
+    SignalListener updateWordTypeMaskListener;
+    SignalListener forceLayoutChannelViewsListener;
+    SignalListener layoutChannelViewsListener;
+    SignalListener invalidateChannelViewBuffersListener;
+    SignalListener repaintVisibleChatWidgetsListener;
+
     friend class Window;  // this is for selectedWindow_
 };
 

src/util/SignalListener.hpp

@@ -0,0 +1,73 @@
+#pragma once
+
+#include "common/ChatterinoSetting.hpp"
+
+#include <pajlada/signals/scoped-connection.hpp>
+#include <pajlada/signals/signal.hpp>
+
+#include <concepts>
+#include <memory>
+#include <mutex>
+#include <utility>
+#include <vector>
+
+namespace chatterino {
+
+class SignalListener
+{
+    std::mutex cbMutex;
+    std::function<void()> cb;
+
+public:
+    using Callback = std::function<void()>;
+
+    explicit SignalListener(Callback callback)
+        : cb(std::move(callback))
+    {
+    }
+
+    ~SignalListener() = default;
+
+    SignalListener(SignalListener &&other) = delete;
+    SignalListener &operator=(SignalListener &&other) = delete;
+    SignalListener(const SignalListener &) = delete;
+    SignalListener &operator=(const SignalListener &) = delete;
+
+    template <typename T>
+        requires IsChatterinoSetting<T>
+    void add(T &setting)
+    {
+        setting.connectSimple(
+            [this](auto) {
+                this->invoke();
+            },
+            this->managedConnections, false);
+    }
+
+    template <typename T>
+        requires std::derived_from<T, pajlada::Signals::NoArgSignal>
+    void add(T &signal)
+    {
+        this->managedConnections.emplace_back(
+            std::make_unique<pajlada::Signals::ScopedConnection>(
+                signal.connect([this] {
+                    this->invoke();
+                })));
+    }
+
+    void invoke()
+    {
+        std::unique_lock lock(this->cbMutex);
+
+        if (this->cb)
+        {
+            this->cb();
+        }
+    }
+
+private:
+    std::vector<std::unique_ptr<pajlada::Signals::ScopedConnection>>
+        managedConnections;
+};
+
+}  // namespace chatterino

tests/src/MessageLayout.cpp

@@ -21,26 +21,14 @@ using namespace chatterino;
 
 namespace {
 
-class MockApplication : mock::BaseApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : theme(this->paths_)
+        : windowManager(this->paths_, this->settings, this->theme, this->fonts)
-        , fonts(this->settings)
-        , windowManager(this->paths_)
     {
     }
 
-    Theme *getThemes() override
-    {
-        return &this->theme;
-    }
-
-    Fonts *getFonts() override
-    {
-        return &this->fonts;
-    }
-
     WindowManager *getWindows() override
     {
         return &this->windowManager;
@@ -52,8 +40,6 @@ public:
     }
 
     AccountController accounts;
-    Theme theme;
-    Fonts fonts;
     WindowManager windowManager;
 };
 

tests/src/Scrollbar.cpp

@@ -17,32 +17,19 @@ using namespace chatterino;
 
 namespace {
 
-class MockApplication : mock::BaseApplication
+class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : theme(this->paths_)
+        : windowManager(this->paths_, this->settings, this->theme, this->fonts)
-        , fonts(this->settings)
-        , windowManager(this->paths_)
     {
     }
-    Theme *getThemes() override
-    {
-        return &this->theme;
-    }
-
-    Fonts *getFonts() override
-    {
-        return &this->fonts;
-    }
 
     WindowManager *getWindows() override
     {
         return &this->windowManager;
     }
 
-    Theme theme;
-    Fonts fonts;
     WindowManager windowManager;
 };
 

tests/src/SeventvEventAPI.cpp

@@ -28,16 +28,16 @@ TEST(SeventvEventAPI, AllEvents)
     std::optional<EmoteRemoveDispatch> removeDispatch;
     std::optional<UserConnectionUpdateDispatch> userDispatch;
 
-    eventAPI.signals_.emoteAdded.connect([&](const auto &d) {
+    std::ignore = eventAPI.signals_.emoteAdded.connect([&](const auto &d) {
         addDispatch = d;
     });
-    eventAPI.signals_.emoteUpdated.connect([&](const auto &d) {
+    std::ignore = eventAPI.signals_.emoteUpdated.connect([&](const auto &d) {
         updateDispatch = d;
     });
-    eventAPI.signals_.emoteRemoved.connect([&](const auto &d) {
+    std::ignore = eventAPI.signals_.emoteRemoved.connect([&](const auto &d) {
         removeDispatch = d;
     });
-    eventAPI.signals_.userUpdated.connect([&](const auto &d) {
+    std::ignore = eventAPI.signals_.userUpdated.connect([&](const auto &d) {
         userDispatch = d;
     });
 

tests/src/SplitInput.cpp

@@ -28,27 +28,16 @@ class MockApplication : public mock::BaseApplication
 {
 public:
     MockApplication()
-        : theme(this->paths_)
+        : windowManager(this->paths_, this->settings, this->theme, this->fonts)
-        , fonts(this->settings)
-        , windowManager(this->paths_)
         , commands(this->paths_)
     {
     }
-    Theme *getThemes() override
-    {
-        return &this->theme;
-    }
 
     HotkeyController *getHotkeys() override
     {
         return &this->hotkeys;
     }
 
-    Fonts *getFonts() override
-    {
-        return &this->fonts;
-    }
-
     WindowManager *getWindows() override
     {
         return &this->windowManager;
@@ -69,9 +58,7 @@ public:
         return &this->emotes;
     }
 
-    Theme theme;
     HotkeyController hotkeys;
-    Fonts fonts;
     WindowManager windowManager;
     AccountController accounts;
     CommandController commands;