[RFC] Secret Channels

[codingHahn]

Context In Garry's Mod's TTT game mode, when you are dead, you are not allowed to speak. It would be nice to have a "private" chat with other dead players. But the alive players should not be able to tell who is dead, so members of the "dead" should also appear in the "alive" channel

Describe the feature you have in mind Another user has already programmed a patch for that here https://github.com/johni0702/mumble/tree/secret-channels but It seems outdated and I don't know the codebase at all.

Describe alternatives you've considered Some sort of murmur plugin but again, I know nothing about the codebase.

[Krzmbrzl]

Hm... I don't like the idea of the UI lying to me.

I do see the use-case for you of course but this is not something I think should be part of the main code :thinking:

Instead this sounds more like a task for a plugin to me. In such a plugin one then could modify the filter n the MainWindow to hide all users in general (and disable the display of member count in channels) while in game. That way you don't even need secret channels.

Plugin support will land in master (sometime soon-ish) via #3743. After that we'd still need support for exposing filtering to plugins though.

[Krzmbrzl]

@Johni0702 do you have additional comments on this particular feature given that you are the one that implemented it in your branch?

[streaps]

I think an optional server-side hiding of private of channels (and users) is an important feature. That wouldn't be lying, it's just privacy / a useful feature.

Having the "dead" players in the "alive" channel is also not lying. I don't think a user has a basic right to track every user on the server. Nothing wrong in denying visibility – access to the server or channel is also not a basic right.

Is it much different than the listener proxy without showing the ear icon?

I also think the secret-channel patch is too specific, but maybe there is a more universal way to implement it that could be useful for other use-cases too.

[codingHahn]

I fully understand that. Maybe @Johni0702 can reimplement the functionality as a plugin, although I doubt that after all this time.

[Johni0702]

Is it much different than the listener proxy without showing the ear icon?

Hm... I don't like the idea of the UI lying to me.

I don't think, at least with the way I'm using it, you'll ever feel betrayed. Maybe a bit confused that some people won't answer you but never betrayed:

• If you're in the dead channel, you get a completely undistorted view anyway.
• If you're in the alive channel, everyone who can hear what you're saying appears to be in your channel (even when they're actually in the dead channel). You just can't hear some of them, as if they were whispering.
• If you're in any other channel which is linked to the alive channel, you can see that link and then the previous point applies.
• If you're in any other channel which isn't linked to the alive channel, you don't really care about the whole thing.

Instead this sounds more like a task for a plugin to me.

As I understand it, plugins are client-side thing, so everyone would have to manually install the plugin which imo is way too inconvenient. In case it wasn't clear: This is all server-side. Once setup by an admin, the individual user merely has to link their identity ingame to their identify in mumble the first time they play (i.e. clicking two buttons and typing a 4-digit code) and after that it just works.

I also think the secret-channel patch is too specific, but maybe there is a more universal way to implement it that could be useful for other use-cases too.

I strongly agree. The current implementation is really use-case specific, it also doesn't handle any edge-cases. Unfortunately I have so far failed to come up with a nice and more universal way of doing the same thing.

The closest thing I could think of would be marking entire channels as hidden-unless-you-are-inside and displaying all the users in their parent channel. Though that still seems awfully specific, especially because now you cannot even make use of it without a gRPC (or ICE) app cause you can't even manually enter the channel (though maybe that's not a bad thing?).

although I doubt that after all this time

I'm still actively using this patch (though not the specific branch on github, just the same patch applied to a more recent but still outdated murmur version).

[codingHahn]

The closest thing I could think of would be marking entire channels as hidden-unless-you-are-inside and displaying all the users in their parent channel. Though that still seems awfully specific, especially because now you cannot even make use of it without a gRPC (or ICE) app cause you can't even manually enter the channel (though maybe that's not a bad thing?).

Maybe make the channel enterable, but don't show who's inside. But I agree, the usecase is quite specific.

Do you have a link to a more recent patch?

[Johni0702]

Do you have a link to a more recent patch?

I do not. For the version I've been using, a simple git cherry-pick 7da5199 does the job but I tried that with a more recent one (current 1.3.x branch) and that's no longer good enough.

[codingHahn]

Were there some large structural changes in 1.3 or did just some definitions change?

I could try to reapply the patch to the 1.3 branch.

[Johni0702]

Were there some large structural changes in 1.3 or did just some definitions change?

I don't think there were, at least to the parts touched by the patch. In fact, if I do a git rebase upstream/1.3.x, then git is able to resolve the conflict in the code and the only conflict I get is in the README (git cherry-pick might work as well, in my test above I was actually downloading the .patch file from github and used git apply). Dunno if it compiles though, don't have a proper development environment set up for it. Seems likely though, so you could totally give it a try.

@Krzmbrzl Do you think it would be feasible to re-use your plugin framework on the server-side (ofc with a different API)? Cause really all that is needed to implement this as a plugin is a hook for Connection::sendMessage(const QByteArray &qbaMsg) (allowing the plugin to inspect, cancel and modify outgoing packets) and a hook in Connection::~Connection() for cleanup. It would ofc in general be preferable to expose any server-side API via gRPC/ICE but given how often sendMessage will be called, I doubt that would be viable here.

[Krzmbrzl]

I was also thinking about server-side plugins before. It's simply that the specific use-case I originally designed the plugin stuff for was purely client-side.

I guess extending it to the server would definitely possible, but I'd definitely only start with that once the client-side is merged and proven to be somewhat stable. After that though I could definitely see plugins for server to be a possibility :)

[Krzmbrzl]

I think an optional server-side hiding of private of channels (and users) is an important feature. That wouldn't be lying, it's just privacy / a useful feature.

That's not what is suggested here though.

Having the "dead" players in the "alive" channel is also not lying. I don't think a user has a basic right to track every user on the server. Nothing wrong in denying visibility – access to the server or channel is also not a basic right.

It's not about "having the right" to anything. If my client shows me that a user is currently in that particular channel, when in fact it is not, then that's lying. It can also cause confusion: If I want to talk to that person, I expect to hop into that channel and be able to do so. With this feature in place this isn't necessarily the case anymore.

Is it much different than the listener proxy without showing the ear icon?

I think it's at least different in the way that the listener proxy problem is due to that user's client not knowing about a new feature yet whereas this would be intentional regardless of the used version (aka this is nothing that'll go away in a few years).

[codingHahn]

@Johni0702 When rebasing and compiling, the build failes at

../Channel.cpp: In member function 'bool Channel::isSecret()':
../Channel.cpp:87:43: error: 'QString::QString(const char*)' is private within this context
   87 |  return qhGroups.contains("members hidden");
      |                                           ^
In file included from /usr/include/qt5/QtCore/qobject.h:47,
                 from /usr/include/qt5/QtCore/qabstractanimation.h:43,
                 from /usr/include/qt5/QtCore/QtCore:6,
                 from mumble_pch.hpp:39:
/usr/include/qt5/QtCore/qstring.h:968:5: note: declared private here
  968 |     QString(const char *ch);
      |     ^~~~~~~

Can you reproduce this?

[Krzmbrzl]

@codingHahn you'll have to use QLatin1String as a wrapper for the const char * in order for this to work

[codingHahn]

Thank you. That worked. Now I have rebased against the 1.3.x release branch. My fork can be found here https://github.com/codingHahn/mumble/tree/secret_rebase