From: Shane Synan <digitalcircuit36939@gmail.com>
Date: Fri, 19 Feb 2016 09:17:26 +0000 (-0600)
Subject: Send extra modes via non-breaking protocol change
X-Git-Tag: travis-deploy-test~513^2
X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=4aea5769f4c155234394957aa55360f111c12cbb

Send extra modes via non-breaking protocol change

Send extra user mode updates in backwards-compatible manner.

When multiple modes are received in IrcChannel::joinIrcUsers, call
addUserMode one time for each, then call ircuser->addUserModes() with
the new modes.  Perhaps less efficient, but avoids breaking protocol.

Reduce excess network traffic by only syncing IrcUser::setUserModes
and IrcUser::addUserModes when changes are made.

Preliminary test results on Ubuntu (more testing always welcome!)
* New core, new client - works fine (as one might hope)
* New core, old client - client prints warning to console but works
(IrcChannel modes might internally get out of sync, but visible
 channel list updates with mode changes, multiple modes, etc)
* New core, Quasseldroid alpha/NG - works fine
* Old core, new client - works fine

If desired, the warning and other potential risks could be avoided
entirely by implementing a new Feature flag and only sending the
additional mode information to clients that request it.  However,
this seemed excessive to avoid what in limited testing was only a
warning in the client console log, normally hidden away from sight.
---

diff --git a/src/common/ircchannel.cpp b/src/common/ircchannel.cpp
index c111b9fa..6673a799 100644
--- a/src/common/ircchannel.cpp
+++ b/src/common/ircchannel.cpp
@@ -178,7 +178,15 @@ void IrcChannel::joinIrcUsers(const QList<IrcUser *> &users, const QStringList &
     for (int i = 0; i < users.count(); i++) {
         ircuser = users[i];
         if (!ircuser || _userModes.contains(ircuser)) {
-            addUserMode(ircuser, modes[i]);
+            if (modes[i].count() > 1) {
+                // Multiple modes received, do it one at a time
+                // TODO Better way of syncing this without breaking protocol?
+                for (int i_m = 0; i_m < modes[i].count(); ++i_m) {
+                    addUserMode(ircuser, modes[i][i_m]);
+                }
+            } else {
+                addUserMode(ircuser, modes[i]);
+            }
             continue;
         }
 
@@ -190,6 +198,9 @@ void IrcChannel::joinIrcUsers(const QList<IrcUser *> &users, const QStringList &
         // If you wonder why there is no counterpart to ircUserJoined:
         // the joins are propagated by the ircuser. The signal ircUserJoined is only for convenience
 
+        // Also update the IRC user's record of modes; this allows easier tracking
+        ircuser->addUserModes(modes[i]);
+
         newNicks << ircuser->nick();
         newModes << modes[i];
         newUsers << ircuser;
@@ -280,6 +291,8 @@ void IrcChannel::addUserMode(IrcUser *ircuser, const QString &mode)
 
     if (!_userModes[ircuser].contains(mode)) {
         _userModes[ircuser] += mode;
+        // Also update the IRC user's record of modes; this allows easier tracking
+        ircuser->addUserModes(mode);
         QString nick = ircuser->nick();
         SYNC_OTHER(addUserMode, ARG(nick), ARG(mode))
         emit ircUserModeAdded(ircuser, mode);
@@ -301,6 +314,8 @@ void IrcChannel::removeUserMode(IrcUser *ircuser, const QString &mode)
 
     if (_userModes[ircuser].contains(mode)) {
         _userModes[ircuser].remove(mode);
+        // Also update the IRC user's record of modes; this allows easier tracking
+        ircuser->removeUserModes(mode);
         QString nick = ircuser->nick();
         SYNC_OTHER(removeUserMode, ARG(nick), ARG(mode));
         emit ircUserModeRemoved(ircuser, mode);
diff --git a/src/common/ircuser.cpp b/src/common/ircuser.cpp
index 32ff7944..15220f76 100644
--- a/src/common/ircuser.cpp
+++ b/src/common/ircuser.cpp
@@ -346,9 +346,11 @@ void IrcUser::channelDestroyed()
 
 void IrcUser::setUserModes(const QString &modes)
 {
-    _userModes = modes;
-    SYNC(ARG(modes))
-    emit userModesSet(modes);
+    if (_userModes != modes) {
+        _userModes = modes;
+        SYNC(ARG(modes))
+        emit userModesSet(modes);
+    }
 }
 
 
@@ -357,13 +359,19 @@ void IrcUser::addUserModes(const QString &modes)
     if (modes.isEmpty())
         return;
 
+    // Don't needlessly sync when no changes are made
+    bool changesMade = false;
     for (int i = 0; i < modes.count(); i++) {
-        if (!_userModes.contains(modes[i]))
+        if (!_userModes.contains(modes[i])) {
             _userModes += modes[i];
+            changesMade = true;
+        }
     }
 
-    SYNC(ARG(modes))
-    emit userModesAdded(modes);
+    if (changesMade) {
+        SYNC(ARG(modes))
+        emit userModesAdded(modes);
+    }
 }
 
 
diff --git a/src/core/coresessioneventprocessor.cpp b/src/core/coresessioneventprocessor.cpp
index 43f44dba..b5981dd1 100644
--- a/src/core/coresessioneventprocessor.cpp
+++ b/src/core/coresessioneventprocessor.cpp
@@ -987,10 +987,9 @@ void CoreSessionEventProcessor::processIrcEvent353(IrcEvent *e)
             // See: http://ircv3.net/specs/extensions/multi-prefix-3.1.html
             while (e->network()->prefixes().contains(nick[0])) {
                 // Mode found in 1 left-most character, add it to the list.
-                // FIXME Only allow one possible mode to avoid a warning in older clients
-                if (mode.isEmpty())
-                    mode.append(e->network()->prefixToMode(nick[0]));
-                //mode.append(e->network()->prefixToMode(nick[0]));
+                // Note: sending multiple modes may cause a warning in older clients.
+                // In testing, the clients still seemed to function fine.
+                mode.append(e->network()->prefixToMode(nick[0]));
                 // Remove this mode from the nick
                 nick = nick.remove(0, 1);
             }