ESCA111/neochat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Improve reliability of restoring the last space and room (again)

Browse Source 
I found that 50% of the time, NeoChat won't restore the last space but
instead get stuck at Home. Even worse, it will overwrite Home's last
opened room with the one from the space - resulting in really buggy
behavior.

The reason why this happens is partly due to the space hierarchy cache
(I think) but that's not the real problem in my opinion. During
setCurrentSpace, we needlessly update the last space & room config
despite us being the ones already reading it.

In addition to that I also refactored this code a bit to be more
consolidated and readable.

(cherry picked from commit 4c37dcf518)
This commit is contained in:
Joshua Goins
2026-01-16 15:14:54 -05:00
parent e965e1680f
commit b6a4b67c22
2 changed files with 71 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

106
src/app/roommanager.cpp
View File

@@ -29,6 +29,28 @@
#include <KIO/OpenUrlJob>
#endif
/**
* @brief Stops RoomManager from updating the last room and space config.
*/
class LastRoomBlocker
{
public:
explicit LastRoomBlocker(RoomManager *manager)
: m_manager(manager)
{
Q_ASSERT(manager);
m_manager->m_dontUpdateLastRoom = true;
}
~LastRoomBlocker()
{
m_manager->m_dontUpdateLastRoom = false;
}
private:
RoomManager *m_manager;
};
RoomManager::RoomManager(QObject *parent)
: QObject(parent)
, m_config(KSharedConfig::openStateConfig())
@@ -324,17 +346,6 @@ void RoomManager::loadInitialRoom()
resolveResource(m_arg);
}
if (m_isMobile) {
QString lastSpace = m_lastRoomConfig.readEntry(u"lastSpace"_s, QString());
// We can't have empty keys in KConfig, so we stored it as "Home"
if (lastSpace == u"Home"_s) {
lastSpace.clear();
}
setCurrentSpace(lastSpace, false);
// We don't want to open a room on startup on mobile
return;
}
if (m_currentRoom) {
// we opened a room with the arg parsing already
return;
@@ -347,16 +358,14 @@ void RoomManager::loadInitialRoom()
void RoomManager::openRoomForActiveConnection()
{
if (!m_connection) {
setCurrentRoom({});
setCurrentSpace({}, false);
return;
}
Q_ASSERT(m_connection);
auto lastSpace = m_lastRoomConfig.readEntry(u"lastSpace"_s, QString());
if (lastSpace == u"Home"_s) {
lastSpace.clear();
}
setCurrentSpace(lastSpace, true);
// We don't want to open a room on startup on mobile
setCurrentSpace(lastSpace, !m_isMobile);
}
UriResolveResult RoomManager::visitUser(User *user, const QString &action)
@@ -513,7 +522,7 @@ void RoomManager::setConnection(NeoChatConnection *connection)
Q_EMIT connectionChanged();
}
void RoomManager::setCurrentSpace(const QString &spaceId, bool setRoom)
void RoomManager::setCurrentSpace(const QString &spaceId, bool goToLastUsedRoom)
{
m_currentSpaceId = spaceId;
@@ -533,25 +542,26 @@ void RoomManager::setCurrentSpace(const QString &spaceId, bool setRoom)
m_lastRoomConfig.writeEntry(u"lastSpace"_s, spaceId.isEmpty() ? u"Home"_s : spaceId);
}
if (!setRoom) {
return;
}
// If we requested to change to the last opened room, do so:
if (goToLastUsedRoom) {
// We don't want to needlessly update the last room config here, that should only be done during explicit user action.
LastRoomBlocker blocker(this);
// We intentionally don't want to open the last room on mobile
if (m_isMobile) {
return;
}
// We can't have empty keys in KConfig, so it's stored as "Home":
if (const auto &lastRoom = m_lastRoomConfig.readEntry(spaceId.isEmpty() ? u"Home"_s : spaceId, QString()); !lastRoom.isEmpty()) {
resolveResource(lastRoom, "no_join"_L1);
return;
}
// We can't have empty keys in KConfig, so it's stored as "Home"
if (const auto &lastRoom = m_lastRoomConfig.readEntry(spaceId.isEmpty() ? u"Home"_s : spaceId, QString()); !lastRoom.isEmpty()) {
resolveResource(lastRoom, "no_join"_L1);
return;
// If no last room was opened, go to the space home:
if (!spaceId.isEmpty() && spaceId != u"DM"_s) {
resolveResource(spaceId, "no_join"_L1);
return;
}
// Fallback to no room opened:
setCurrentRoom({});
}
if (!spaceId.isEmpty() && spaceId != u"DM"_s) {
resolveResource(spaceId, "no_join"_L1);
return;
}
setCurrentRoom({});
}
QString RoomManager::findSpaceIdForCurrentRoom() const
@@ -611,21 +621,23 @@ void RoomManager::setCurrentRoom(const QString &roomId)
Q_EMIT currentRoomChanged();
if (roomId.isEmpty()) {
m_lastRoomConfig.deleteEntry(m_currentSpaceId);
return;
}
if (!m_dontUpdateLastRoom) {
if (roomId.isEmpty()) {
m_lastRoomConfig.deleteEntry(m_currentSpaceId);
return;
}
const auto spaceIdForRoom = findSpaceIdForCurrentRoom();
// We can't have empty keys in KConfig, so name it "Home"
if (spaceIdForRoom.isEmpty()) {
m_lastRoomConfig.writeEntry(u"Home"_s, roomId);
} else {
m_lastRoomConfig.writeEntry(spaceIdForRoom, roomId);
}
const auto spaceIdForRoom = findSpaceIdForCurrentRoom();
// We can't have empty keys in KConfig, so name it "Home"
if (spaceIdForRoom.isEmpty()) {
m_lastRoomConfig.writeEntry(u"Home"_s, roomId);
} else {
m_lastRoomConfig.writeEntry(spaceIdForRoom, roomId);
}
if (m_currentSpaceId != spaceIdForRoom) {
setCurrentSpace(spaceIdForRoom, false);
if (m_currentSpaceId != spaceIdForRoom) {
setCurrentSpace(spaceIdForRoom, false);
}
}
}

14
src/app/roommanager.h
View File

@@ -337,6 +337,11 @@ Q_SIGNALS:
void currentSpaceChanged();
protected:
bool m_dontUpdateLastRoom = false; // Don't set directly, use LastRoomBlocker.
friend class LastRoomBlocker;
private:
bool m_isMobile = false;
@@ -382,8 +387,13 @@ private:
*/
QString findSpaceIdForCurrentRoom() const;
// Space ID, "DM", or empty string
void setCurrentSpace(const QString &spaceId, bool setRoom = true);
/**
* @brief Sets the current space.
*
* @param spaceId The ID of the space, "DM" for direct messages or an empty string for Home.
* @param goToLastUsedRoom If true, we will navigate to the last opened room in this space.
*/
void setCurrentSpace(const QString &spaceId, bool goToLastUsedRoom = true);
/**
* @brief Resolve a user URI.