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.
This commit is contained in:
Joshua Goins
2026-01-16 15:14:54 -05:00
parent 3c77711417
commit 4c37dcf518
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())
@@ -320,17 +342,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;
@@ -343,16 +354,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)
@@ -509,7 +518,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;
@@ -529,25 +538,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
@@ -607,21 +617,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);
}
}
}