Use a global WatchManager for settings

Fixes https://github.com/vector-im/riot-web/issues/8936

Watchers are now managed by the SettingsStore itself through a global/default watch manager. As per the included documentation, the watch manager dispatches updates to callbacks which are redirected by the SettingsStore for consumer safety.

docs/settings.md

@@ -143,11 +143,8 @@ class MyComponent extends React.Component {
     settingWatcherRef = null;
     
     componentWillMount() {
-        this.settingWatcherRef = SettingsStore.watchSetting("roomColor", "!example:matrix.org", (settingName, roomId, level, newVal) => {
+        this.settingWatcherRef = SettingsStore.watchSetting("roomColor", "!example:matrix.org", (settingName, roomId, level, newValAtLevel, newVal) => {
-            // Always re-read the setting value from the store to avoid reacting to changes which do not have a consequence. For example, the
+           this.setState({color: newVal});
-            // room color could have been changed at the device level, but an account override prevents that change from making a difference.
-           const actualVal = SettingsStore.getValue(settingName, "!example:matrix.org");
-           if (actualVal !== this.state.color) this.setState({color: actualVal});
         });
     }
     
@@ -188,7 +185,9 @@ If `enableLabs` is true in the configuration, the default for features becomes `
 
 ### Watchers
 
-Watchers can appear complicated under the hood: the request to watch a setting is actually forked off to individual handlers for watching. This means that the handlers need to track their changes and listen for remote changes where possible, but also makes it much easier for the `SettingsStore` to react to changes. The handler is going to know the best things to listen for (specific events, account data, etc) and thus it is left as a responsibility for the handler to track changes. 
+Watchers can appear complicated under the hood: there is a central `WatchManager` which handles the actual invocation of callbacks, and callbacks are managed by the SettingsStore by redirecting the caller's callback to a dedicated callback. This is done so that the caller can reuse the same function as their callback without worrying about whether or not it'll unsubscribe all watchers. 
+
+Setting changes are emitted into the default `WatchManager`, which calculates the new value for the setting. Ideally, we'd also try and suppress updates which don't have a consequence on this value, however there's not an easy way to do this. Instead, we just dispatch an update for all changes and leave it up to the consumer to deduplicate. 
 
 In practice, handlers which rely on remote changes (account data, room events, etc) will always attach a listener to the `MatrixClient`. They then watch for changes to events they care about and send off appropriate updates to the generalized `WatchManager` - a class specifically designed to deduplicate the logic of managing watchers. The handlers which are localized to the local client (device) generally just trigger the `WatchManager` when they manipulate the setting themselves as there's nothing to really 'watch'.
  

src/settings/SettingsStore.js

@@ -27,6 +27,7 @@ import SdkConfig from "../SdkConfig";
 import dis from '../dispatcher';
 import {SETTINGS} from "./Settings";
 import LocalEchoWrapper from "./handlers/LocalEchoWrapper";
+import {WatchManager} from "./WatchManager";
 
 /**
  * Represents the various setting levels supported by the SettingsStore.
@@ -43,6 +44,8 @@ export const SettingLevel = {
     DEFAULT: "default",
 };
 
+const defaultWatchManager = new WatchManager();
+
 // Convert the settings to easier to manage objects for the handlers
 const defaultSettings = {};
 const invertedDefaultSettings = {};
@@ -58,11 +61,11 @@ for (const key of Object.keys(SETTINGS)) {
 }
 
 const LEVEL_HANDLERS = {
-    "device": new DeviceSettingsHandler(featureNames),
+    "device": new DeviceSettingsHandler(featureNames, defaultWatchManager),
-    "room-device": new RoomDeviceSettingsHandler(),
+    "room-device": new RoomDeviceSettingsHandler(defaultWatchManager),
-    "room-account": new RoomAccountSettingsHandler(),
+    "room-account": new RoomAccountSettingsHandler(defaultWatchManager),
-    "account": new AccountSettingsHandler(),
+    "account": new AccountSettingsHandler(defaultWatchManager),
-    "room": new RoomSettingsHandler(),
+    "room": new RoomSettingsHandler(defaultWatchManager),
     "config": new ConfigSettingsHandler(),
     "default": new DefaultSettingsHandler(defaultSettings, invertedDefaultSettings),
 };
@@ -100,32 +103,30 @@ const LEVEL_ORDER = [
  * be enabled).
  */
 export default class SettingsStore {
-    // We support watching settings for changes, and do so only at the levels which are
+    // We support watching settings for changes, and do this by tracking which callbacks have
-    // relevant to the setting. We pass the watcher on to the handlers and aggregate it
+    // been given to us. We end up returning the callbackRef to the caller so they can unsubscribe
-    // before sending it off to the caller. We need to track which callback functions we
+    // at a later point.
-    // provide to the handlers though so we can unwatch it on demand. In practice, we
-    // return a "callback reference" to the caller which correlates to an entry in this
-    // dictionary for each handler's callback function.
     //
     // We also maintain a list of monitors which are special watchers: they cause dispatches
     // when the setting changes. We track which rooms we're monitoring though to ensure we
     // don't duplicate updates on the bus.
-    static _watchers = {}; // { callbackRef => { level => callbackFn } }
+    static _watchers = {}; // { callbackRef => { callbackFn } }
     static _monitors = {}; // { settingName => { roomId => callbackRef } }
 
     /**
      * Watches for changes in a particular setting. This is done without any local echo
-     * wrapping and fires whenever a change is detected in a setting's value. Watching
+     * wrapping and fires whenever a change is detected in a setting's value, at any level.
-     * is intended to be used in scenarios where the app needs to react to changes made
+     * Watching is intended to be used in scenarios where the app needs to react to changes
-     * by other devices. It is otherwise expected that callers will be able to use the
+     * made by other devices. It is otherwise expected that callers will be able to use the
-     * Controller system or track their own changes to settings. Callers should retain
+     * Controller system or track their own changes to settings. Callers should retain the
+     * returned reference to later unsubscribe from updates.
      * @param {string} settingName The setting name to watch
      * @param {String} roomId The room ID to watch for changes in. May be null for 'all'.
      * @param {function} callbackFn A function to be called when a setting change is
-     * detected. Four arguments can be expected: the setting name, the room ID (may be null),
+     * detected. Five arguments can be expected: the setting name, the room ID (may be null),
-     * the level the change happened at, and finally the new value for those arguments. The
+     * the level the change happened at, the new value at the given level, and finally the new
-     * callback may need to do a call to #getValue() to see if a consequential change has
+     * value for the setting regardless of level. The callback is responsible for determining
-     * occurred.
+     * if the change in value is worthwhile enough to react upon.
      * @returns {string} A reference to the watcher that was employed.
      */
     static watchSetting(settingName, roomId, callbackFn) {
@@ -138,21 +139,15 @@ export default class SettingsStore {
         }
 
         const watcherId = `${new Date().getTime()}_${settingName}_${roomId}`;
-        SettingsStore._watchers[watcherId] = {};
 
-        const levels = Object.keys(LEVEL_HANDLERS);
+        const localizedCallback = (changedInRoomId, atLevel, newValAtLevel) => {
-        for (const level of levels) {
+            const newValue = SettingsStore.getValue(originalSettingName);
-            const handler = SettingsStore._getHandler(originalSettingName, level);
+            callbackFn(originalSettingName, changedInRoomId, atLevel, newValAtLevel, newValue);
-            if (!handler) continue;
-
-            const localizedCallback = (changedInRoomId, newVal) => {
-                callbackFn(originalSettingName, changedInRoomId, level, newVal);
         };
 
-            console.log(`Starting watcher for ${settingName}@${roomId || '<null room>'} at level ${level}`);
+        console.log(`Starting watcher for ${settingName}@${roomId || '<null room>'}`);
-            SettingsStore._watchers[watcherId][level] = localizedCallback;
+        SettingsStore._watchers[watcherId] = localizedCallback;
-            handler.watchSetting(settingName, roomId, localizedCallback);
+        defaultWatchManager.watchSetting(settingName, roomId, localizedCallback);
-        }
 
         return watcherId;
     }
@@ -166,12 +161,7 @@ export default class SettingsStore {
     static unwatchSetting(watcherReference) {
         if (!SettingsStore._watchers[watcherReference]) return;
 
-        for (const handlerName of Object.keys(SettingsStore._watchers[watcherReference])) {
+        defaultWatchManager.unwatchSetting(SettingsStore._watchers[watcherReference]);
-            const handler = LEVEL_HANDLERS[handlerName];
-            if (!handler) continue;
-            handler.unwatchSetting(SettingsStore._watchers[watcherReference][handlerName]);
-        }
-
         delete SettingsStore._watchers[watcherReference];
     }
 
@@ -179,7 +169,7 @@ export default class SettingsStore {
      * Sets up a monitor for a setting. This behaves similar to #watchSetting except instead
      * of making a call to a callback, it forwards all changes to the dispatcher. Callers can
      * expect to listen for the 'setting_updated' action with an object containing settingName,
-     * roomId, level, and newValue.
+     * roomId, level, newValueAtLevel, and newValue.
      * @param {string} settingName The setting name to monitor.
      * @param {String} roomId The room ID to monitor for changes in. Use null for all rooms.
      */
@@ -188,12 +178,13 @@ export default class SettingsStore {
 
         const registerWatcher = () => {
             this._monitors[settingName][roomId] = SettingsStore.watchSetting(
-                settingName, roomId, (settingName, inRoomId, level, newValue) => {
+                settingName, roomId, (settingName, inRoomId, level, newValueAtLevel, newValue) => {
                     dis.dispatch({
                         action: 'setting_updated',
                         settingName,
                         roomId: inRoomId,
                         level,
+                        newValueAtLevel,
                         newValue,
                     });
                 },

src/settings/WatchManager.js

@@ -41,7 +41,11 @@ export class WatchManager {
         }
     }
 
-    notifyUpdate(settingName, inRoomId, newValue) {
+    notifyUpdate(settingName, inRoomId, atLevel, newValueAtLevel) {
+        // Dev note: We could avoid raising changes for ultimately inconsequential changes, but
+        // we also don't have a reliable way to get the old value of a setting. Instead, we'll just
+        // let it fall through regardless and let the receiver dedupe if they want to.
+
         if (!this._watchers[settingName]) return;
 
         const roomWatchers = this._watchers[settingName];
@@ -51,7 +55,7 @@ export class WatchManager {
         if (roomWatchers[null]) callbacks.push(...roomWatchers[null]);
 
         for (const callback of callbacks) {
-            callback(inRoomId, newValue);
+            callback(inRoomId, atLevel, newValueAtLevel);
         }
     }
 }

src/settings/handlers/AccountSettingsHandler.js

@@ -16,18 +16,18 @@ limitations under the License.
 */
 
 import MatrixClientPeg from '../../MatrixClientPeg';
-import {WatchManager} from "../WatchManager";
 import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
+import {SettingLevel} from "../SettingsStore";
 
 /**
  * Gets and sets settings at the "account" level for the current user.
  * This handler does not make use of the roomId parameter.
  */
 export default class AccountSettingsHandler extends MatrixClientBackedSettingsHandler {
-    constructor() {
+    constructor(watchManager) {
         super();
 
-        this._watchers = new WatchManager();
+        this._watchers = watchManager;
         this._onAccountData = this._onAccountData.bind(this);
     }
 
@@ -48,11 +48,12 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
                 val = !val;
             }
 
-            this._watchers.notifyUpdate("urlPreviewsEnabled", null, val);
+            this._watchers.notifyUpdate("urlPreviewsEnabled", null, SettingLevel.ACCOUNT, val);
         } else if (event.getType() === "im.vector.web.settings") {
             // We can't really discern what changed, so trigger updates for everything
             for (const settingName of Object.keys(event.getContent())) {
-                this._watchers.notifyUpdate(settingName, null, event.getContent()[settingName]);
+                const val = event.getContent()[settingName];
+                this._watchers.notifyUpdate(settingName, null, SettingLevel.ACCOUNT, val);
             }
         }
     }
@@ -102,14 +103,6 @@ export default class AccountSettingsHandler extends MatrixClientBackedSettingsHa
         return cli !== undefined && cli !== null;
     }
 
-    watchSetting(settingName, roomId, cb) {
-        this._watchers.watchSetting(settingName, roomId, cb);
-    }
-
-    unwatchSetting(cb) {
-        this._watchers.unwatchSetting(cb);
-    }
-
     _getSettings(eventType = "im.vector.web.settings") {
         const cli = MatrixClientPeg.get();
         if (!cli) return null;

src/settings/handlers/ConfigSettingsHandler.js

@@ -47,12 +47,4 @@ export default class ConfigSettingsHandler extends SettingsHandler {
     isSupported() {
         return true; // SdkConfig is always there
     }
-
-    watchSetting(settingName, roomId, cb) {
-        // no-op: no changes possible
-    }
-
-    unwatchSetting(cb) {
-        // no-op: no changes possible
-    }
 }

src/settings/handlers/DefaultSettingsHandler.js

@@ -52,12 +52,4 @@ export default class DefaultSettingsHandler extends SettingsHandler {
     isSupported() {
         return true;
     }
-
-    watchSetting(settingName, roomId, cb) {
-        // no-op: no changes possible
-    }
-
-    unwatchSetting(cb) {
-        // no-op: no changes possible
-    }
 }

src/settings/handlers/DeviceSettingsHandler.js

@@ -18,7 +18,7 @@ limitations under the License.
 import Promise from 'bluebird';
 import SettingsHandler from "./SettingsHandler";
 import MatrixClientPeg from "../../MatrixClientPeg";
-import {WatchManager} from "../WatchManager";
+import {SettingLevel} from "../SettingsStore";
 
 /**
  * Gets and sets settings at the "device" level for the current device.
@@ -29,11 +29,12 @@ export default class DeviceSettingsHandler extends SettingsHandler {
     /**
      * Creates a new device settings handler
      * @param {string[]} featureNames The names of known features.
+     * @param {WatchManager} watchManager The watch manager to notify updates to
      */
-    constructor(featureNames) {
+    constructor(featureNames, watchManager) {
         super();
         this._featureNames = featureNames;
-        this._watchers = new WatchManager();
+        this._watchers = watchManager;
     }
 
     getValue(settingName, roomId) {
@@ -69,22 +70,22 @@ export default class DeviceSettingsHandler extends SettingsHandler {
         // Special case notifications
         if (settingName === "notificationsEnabled") {
             localStorage.setItem("notifications_enabled", newValue);
-            this._watchers.notifyUpdate(settingName, null, newValue);
+            this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
             return Promise.resolve();
         } else if (settingName === "notificationBodyEnabled") {
             localStorage.setItem("notifications_body_enabled", newValue);
-            this._watchers.notifyUpdate(settingName, null, newValue);
+            this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
             return Promise.resolve();
         } else if (settingName === "audioNotificationsEnabled") {
             localStorage.setItem("audio_notifications_enabled", newValue);
-            this._watchers.notifyUpdate(settingName, null, newValue);
+            this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
             return Promise.resolve();
         }
 
         const settings = this._getSettings() || {};
         settings[settingName] = newValue;
         localStorage.setItem("mx_local_settings", JSON.stringify(settings));
-        this._watchers.notifyUpdate(settingName, null, newValue);
+        this._watchers.notifyUpdate(settingName, null, SettingLevel.DEVICE, newValue);
 
         return Promise.resolve();
     }

src/settings/handlers/LocalEchoWrapper.js

@@ -67,12 +67,4 @@ export default class LocalEchoWrapper extends SettingsHandler {
     isSupported() {
         return this._handler.isSupported();
     }
-
-    watchSetting(settingName, roomId, cb) {
-        this._handler.watchSetting(settingName, roomId, cb);
-    }
-
-    unwatchSetting(cb) {
-        this._handler.unwatchSetting(cb);
-    }
 }

src/settings/handlers/RoomAccountSettingsHandler.js

@@ -17,16 +17,16 @@ limitations under the License.
 
 import MatrixClientPeg from '../../MatrixClientPeg';
 import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
-import {WatchManager} from "../WatchManager";
+import {SettingLevel} from "../SettingsStore";
 
 /**
  * Gets and sets settings at the "room-account" level for the current user.
  */
 export default class RoomAccountSettingsHandler extends MatrixClientBackedSettingsHandler {
-    constructor() {
+    constructor(watchManager) {
         super();
 
-        this._watchers = new WatchManager();
+        this._watchers = watchManager;
         this._onAccountData = this._onAccountData.bind(this);
     }
 
@@ -49,13 +49,14 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
                 val = !val;
             }
 
-            this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, val);
+            this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM_ACCOUNT, val);
         } else if (event.getType() === "org.matrix.room.color_scheme") {
-            this._watchers.notifyUpdate("roomColor", roomId, event.getContent());
+            this._watchers.notifyUpdate("roomColor", roomId, SettingLevel.ROOM_ACCOUNT, event.getContent());
         } else if (event.getType() === "im.vector.web.settings") {
             // We can't really discern what changed, so trigger updates for everything
             for (const settingName of Object.keys(event.getContent())) {
-                this._watchers.notifyUpdate(settingName, roomId, event.getContent()[settingName]);
+                const val = event.getContent()[settingName];
+                this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_ACCOUNT, val);
             }
         }
     }
@@ -113,14 +114,6 @@ export default class RoomAccountSettingsHandler extends MatrixClientBackedSettin
         return cli !== undefined && cli !== null;
     }
 
-    watchSetting(settingName, roomId, cb) {
-        this._watchers.watchSetting(settingName, roomId, cb);
-    }
-
-    unwatchSetting(cb) {
-        this._watchers.unwatchSetting(cb);
-    }
-
     _getSettings(roomId, eventType = "im.vector.web.settings") {
         const room = MatrixClientPeg.get().getRoom(roomId);
         if (!room) return null;

src/settings/handlers/RoomDeviceSettingsHandler.js

@@ -17,17 +17,17 @@ limitations under the License.
 
 import Promise from 'bluebird';
 import SettingsHandler from "./SettingsHandler";
-import {WatchManager} from "../WatchManager";
+import {SettingLevel} from "../SettingsStore";
 
 /**
  * Gets and sets settings at the "room-device" level for the current device in a particular
  * room.
  */
 export default class RoomDeviceSettingsHandler extends SettingsHandler {
-    constructor() {
+    constructor(watchManager) {
         super();
 
-        this._watchers = new WatchManager();
+        this._watchers = watchManager;
     }
 
     getValue(settingName, roomId) {
@@ -52,7 +52,7 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
             if (!value["blacklistUnverifiedDevicesPerRoom"]) value["blacklistUnverifiedDevicesPerRoom"] = {};
             value["blacklistUnverifiedDevicesPerRoom"][roomId] = newValue;
             localStorage.setItem("mx_local_settings", JSON.stringify(value));
-            this._watchers.notifyUpdate(settingName, roomId, newValue);
+            this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue);
             return Promise.resolve();
         }
 
@@ -63,7 +63,7 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
             localStorage.setItem(this._getKey(settingName, roomId), newValue);
         }
 
-        this._watchers.notifyUpdate(settingName, roomId, newValue);
+        this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM_DEVICE, newValue);
         return Promise.resolve();
     }
 
@@ -75,14 +75,6 @@ export default class RoomDeviceSettingsHandler extends SettingsHandler {
         return localStorage !== undefined && localStorage !== null;
     }
 
-    watchSetting(settingName, roomId, cb) {
-        this._watchers.watchSetting(settingName, roomId, cb);
-    }
-
-    unwatchSetting(cb) {
-        this._watchers.unwatchSetting(cb);
-    }
-
     _read(key) {
         const rawValue = localStorage.getItem(key);
         if (!rawValue) return null;

src/settings/handlers/RoomSettingsHandler.js

@@ -17,16 +17,16 @@ limitations under the License.
 
 import MatrixClientPeg from '../../MatrixClientPeg';
 import MatrixClientBackedSettingsHandler from "./MatrixClientBackedSettingsHandler";
-import {WatchManager} from "../WatchManager";
+import {SettingLevel} from "../SettingsStore";
 
 /**
  * Gets and sets settings at the "room" level.
  */
 export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandler {
-    constructor() {
+    constructor(watchManager) {
         super();
 
-        this._watchers = new WatchManager();
+        this._watchers = watchManager;
         this._onEvent = this._onEvent.bind(this);
     }
 
@@ -49,11 +49,11 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
                 val = !val;
             }
 
-            this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, val);
+            this._watchers.notifyUpdate("urlPreviewsEnabled", roomId, SettingLevel.ROOM, val);
         } else if (event.getType() === "im.vector.web.settings") {
             // We can't really discern what changed, so trigger updates for everything
             for (const settingName of Object.keys(event.getContent())) {
-                this._watchers.notifyUpdate(settingName, roomId, event.getContent()[settingName]);
+                this._watchers.notifyUpdate(settingName, roomId, SettingLevel.ROOM, event.getContent()[settingName]);
             }
         }
     }
@@ -101,14 +101,6 @@ export default class RoomSettingsHandler extends MatrixClientBackedSettingsHandl
         return cli !== undefined && cli !== null;
     }
 
-    watchSetting(settingName, roomId, cb) {
-        this._watchers.watchSetting(settingName, roomId, cb);
-    }
-
-    unwatchSetting(cb) {
-        this._watchers.unwatchSetting(cb);
-    }
-
     _getSettings(roomId, eventType = "im.vector.web.settings") {
         const room = MatrixClientPeg.get().getRoom(roomId);
         if (!room) return null;

src/settings/handlers/SettingsHandler.js

@@ -69,27 +69,4 @@ export default class SettingsHandler {
     isSupported() {
         return false;
     }
-
-    /**
-     * Watches for a setting change within this handler. The caller should preserve
-     * a reference to the callback so that it may be unwatched. The caller should
-     * additionally provide a unique callback for multiple watchers on the same setting.
-     * @param {string} settingName The setting name to watch for changes in.
-     * @param {String} roomId The room ID to watch for changes in.
-     * @param {function} cb A function taking two arguments: the room ID the setting changed
-     * in and the new value for the setting at this level in the given room.
-     */
-    watchSetting(settingName, roomId, cb) {
-        throw new Error("Invalid operation: watchSetting was not overridden");
-    }
-
-    /**
-     * Unwatches a previously watched setting. If the callback is not associated with
-     * a watcher, this is a no-op.
-     * @param {function} cb A callback function previously supplied to watchSetting
-     * which should no longer be used.
-     */
-    unwatchSetting(cb) {
-        throw new Error("Invalid operation: unwatchSetting was not overridden");
-    }
 }

src/stores/RoomListStore.js

@@ -46,6 +46,20 @@ const LIST_ORDERS = {
     "im.vector.fake.archived": "recent",
 };
 
+/**
+ * Identifier for the "breadcrumb" (or "sort by most important room first") algorithm.
+ * Includes a provision for keeping the currently open room from flying down the room
+ * list.
+ * @type {string}
+ */
+const ALGO_IMPORTANCE = "importance";
+
+/**
+ * Identifier for classic sorting behaviour: sort by the most recent message first.
+ * @type {string}
+ */
+const ALGO_RECENT = "recent";
+
 /**
  * A class for storing application state for categorising rooms in
  * the RoomList.
@@ -60,20 +74,19 @@ class RoomListStore extends Store {
     }
 
     /**
-     * Alerts the RoomListStore to a potential change in how room list sorting should
+     * Changes the sorting algorithm used by the RoomListStore.
-     * behave.
+     * @param {string} algorithm The new algorithm to use. Should be one of the ALGO_* constants.
-     * @param {boolean} forceRegeneration True to force a change in the algorithm
      */
-    updateSortingAlgorithm(forceRegeneration = false) {
+    updateSortingAlgorithm(algorithm) {
-        const byImportance = SettingsStore.getValue("RoomList.orderByImportance");
+        // Dev note: We only have two algorithms at the moment, but it isn't impossible that we want
-        if (byImportance !== this._state.orderRoomsByImportance || forceRegeneration) {
+        // multiple in the future. Also constants make things slightly clearer.
+        const byImportance = algorithm === ALGO_IMPORTANCE;
         console.log("Updating room sorting algorithm: sortByImportance=" + byImportance);
         this._setState({orderRoomsByImportance: byImportance});
 
         // Trigger a resort of the entire list to reflect the change in algorithm
         this._generateInitialRoomLists();
     }
-    }
 
     _init() {
         // Initialise state
@@ -97,6 +110,7 @@ class RoomListStore extends Store {
         };
 
         SettingsStore.monitorSetting('RoomList.orderByImportance', null);
+        SettingsStore.monitorSetting('feature_custom_tags', null);
     }
 
     _setState(newState) {
@@ -120,13 +134,10 @@ class RoomListStore extends Store {
         switch (payload.action) {
             case 'setting_updated': {
                 if (payload.settingName === 'RoomList.orderByImportance') {
-                    this.updateSortingAlgorithm();
+                    this.updateSortingAlgorithm(payload.newValue === true ? ALGO_IMPORTANCE : ALGO_RECENT);
                 } else if (payload.settingName === 'feature_custom_tags') {
-                    const isActive = SettingsStore.isFeatureEnabled("feature_custom_tags");
+                    this._setState({tagsEnabled: payload.newValue});
-                    if (isActive !== this._state.tagsEnabled) {
+                    this._generateInitialRoomLists(); // Tags means we have to start from scratch
-                        this._setState({tagsEnabled: isActive});
-                        this.updateSortingAlgorithm(/*force=*/true);
-                    }
                 }
             }
             break;
@@ -139,7 +150,10 @@ class RoomListStore extends Store {
                 this._setState({tagsEnabled: SettingsStore.isFeatureEnabled("feature_custom_tags")});
 
                 this._matrixClient = payload.matrixClient;
-                this.updateSortingAlgorithm(/*force=*/true);
+
+                const algorithm = SettingsStore.getValue("RoomList.orderByImportance")
+                    ? ALGO_IMPORTANCE : ALGO_RECENT;
+                this.updateSortingAlgorithm(algorithm);
             }
             break;
             case 'MatrixActions.Room.receipt': {
@@ -445,6 +459,11 @@ class RoomListStore extends Store {
     }
 
     _generateInitialRoomLists() {
+        // Log something to show that we're throwing away the old results. This is for the inevitable
+        // question of "why is 100% of my CPU going towards Riot?" - a quick look at the logs would reveal
+        // that something is wrong with the RoomListStore.
+        console.log("Generating initial room lists");
+
         const lists = {
             "m.server_notice": [],
             "im.vector.fake.invite": [],