sm00th / bitlbee-discord

Bitlbee plugin for Discord (http://discordapp.com)
GNU General Public License v2.0
290 stars 27 forks source link

FRIENDSHIP 🌈🌟 #91

Closed dequis closed 6 years ago

dequis commented 7 years ago

Handle friendship events and the initial batch. Not doing anything interesting for now, just setting a bit in bu->data determining if someone is a friend or not. The idea is to use that bit to filter presence events. Not sure if you want a setting for that.

dequis commented 6 years ago

Addressed stuff, and made it do something useful. It's a setting and it's turned on by default. Obvious behavior change and will "break" never_offline, but probably desirable for most users anyway.

With this mode, friends are always online, non-friends are always offline, and all users are always added to all channels. Away-based presence processing is disabled with this.

friendship_mode is the first name that came to mind, I don't like thinking too much about that, feel free to suggest bikeshed colors.

digitalcircuit commented 6 years ago

@dequis With...

non-friends are always offline, and all users are always added to all channels

By "always offline", you mean "not added to the control channel", correct?

I.e. if your IRC client shows a nick as online if they're in any channel, the end result is your query list will show them as "always online" for those in channels you've chat add'd.

If so, thumbs-up; that sounds nicer than the current situation.

I'm also confused by...

Away-based presence processing is disabled with this.

Presence regarding "existing in the channels or not", or "/away status messages"? It looks like the latter, disabling all indication of Available/Away status, but I'm not sure why you can't have the former.

(And you could call the boolean friendship_is_magic...)

dequis commented 6 years ago

By "always offline", you mean "not added to the control channel", correct?

Yep. That part is the same way twitter contacts are handled, btw.

I.e. if your IRC client shows a nick as online if they're in any channel, the end result is your query list will show them as "always online" for those in channels you've chat add'd.

Yeah, I guess. Not familiar with irc clients attempting to handle presence that way. Bitlbee's offline is more like an away state for the irc client. If the client uses WHO/ISON/WATCH it will consistently get away for those.

Away-based presence processing is disabled with this.

Presence regarding "existing in the channels or not", or "/away status messages"? It looks like the latter, disabling all indication of Available/Away status, but I'm not sure why you can't have the former.

Presence as in discord-level online/idle/offline status. Those status changes won't affect irc level awayness. Friends are always online, non-friends are always offline, all the users are always added to channels they are joined to.

digitalcircuit commented 6 years ago

Yeah, I guess. Not familiar with irc clients attempting to handle presence that way. Bitlbee's offline is more like an away state for the irc client. If the client uses WHO/ISON/WATCH it will consistently get away for those.

In Quassel, there's a distinction between nicknames in a channel, and those you have a conversation with ("Queries"). The query list -also- shows online status, akin to Pidgin's buddy list.

Quassel showing the status of "jwheare" as online in query list despite not being in the same channel

My remark is that Discord Available vs Busy/Away could be reflected in IRC /away status, like XMPP does now. The "nick connected" or "nick offline" in the control channel would still depend on friendship status.

Presence as in discord-level online/idle/offline status. Those status changes won't affect irc level awayness. Friends are always online, non-friends are always offline, all the users are always added to channels they are joined to.

I'd be saddened to lose all Busy/Away status indication since that lets me see if someone's more likely to reply without having to check the web client. I'm not sure where this conflicts, either.

sm00th commented 6 years ago

Finally got to it. This is very nice and works fine. There are a couple of changes I'd like to propose though:

I really don't like the new set_getbool() checks in discord_parse_message(). This breaks the logic and makes it harder to read. I think it is better to call discord_handle_presence() regardless of the setting and check it inside the function. This will allow us to keep "away" statuses for friends.

Additionally if we add new flags var to userinfo struct that would mimic bitlbee's flags (since we are not putting non-friends online anymore) we will be able to track non-friends state and add/remove to channels based on that, so that we won't have to have the whole userlist always present in a channel.

Here's a patch I came up with: https://gist.github.com/sm00th/824a9ac46e2dabac1489f9d756e56b2c Limited testing only but seems to be ok.

digitalcircuit commented 6 years ago

@sm00th From the patch you've listed, that means "away"/Idle status apply to those in groupchat channels too? If so, and if you can still direct-message offline non-friends on mutual servers (as the web UI allows you to do), awesome, that addresses all concerns I had.

(For the rare cases of wanting to see the user count of larger servers, I can temporarily disable FRIENDSHIP MODE.)

sm00th commented 6 years ago

@sm00th From the patch you've listed, that means "away"/Idle status apply to those in groupchat channels too?

That does not and I see no way to implement this as technically those contacts in groupchats are still offline for bitlbee.

dequis commented 6 years ago

Would be nice to be able to do BEE_USER_AWAY without BEE_USER_ONLINE, or setting a status message for someone who is offline. But the "User is offline" message seems to take priority over anything else in /whois, so whatever you do there is pointless.

This sounds like it might need more API over at bitlbee, like some prpl+buddy flag to explicitly require marking buddies as needing to be added to the control channel while still allowing them to have a separate status.

(Also, it sounds like we're avoiding offline_user_quits-related issues that other protocols have - those cases prefer to have people inside channels all the time and also reflect their statuses, but setting them to offline means they quit globally and leave the channel. If i'm interpreting this right, quitting when someone leaves a channel is acceptable for this plugin)

that we won't have to have the whole userlist always present in a channel

Yeah, see, I thought that was desirable. I guess it's just a never_offline thing.

Here's a patch I came up with:

Patch looks okay I guess. You can push it to this PR btw.

Not sure about the removal of the status update when someone is added in discord_handle_relationship

digitalcircuit commented 6 years ago

@sm00th

…technically those contacts in groupchats are still offline for bitlbee.

Noted, unfortunately. Also the reply afterwards.

@dequis

If I'm interpreting this right, quitting when someone leaves a channel is acceptable for this plugin

As far as I know, Discord doesn't let you leave a channel. Do you mean Bitlbee treats it as a global quit when someone only in a group chat (not a friend) goes offline?

Given you can PM folks without having them as a friend, global quits might still cause issues.

Yeah, see, I thought that was desirable. I guess it's just a never_offline thing.

Another consideration for never_offline in channels is Discord lets you mention folks regardless of their status, it'll just show up whenever they next sign in/via notification. Having people always in channel makes tab-complete easier, e.g. if dealing with someone on a spotty connection. Unfortunately I don't think it's possible to emulate the web UI's @[name here] auto-complete without showing offline folks as away...

As usual, if my uninformed remarks are making it more difficult, let me know and I'll step back!

dequis commented 6 years ago

If I'm interpreting this right, quitting when someone leaves a channel is acceptable for this plugin

As far as I know, Discord doesn't let you leave a channel.

Ehhh words are hard and what I said doesn't match what I was thinking

What I meant: Leaving a channel (through a global irc quit) when someone goes offline is acceptable for this plugin.

Do you mean Bitlbee treats it as a global quit when someone only in a group chat (not a friend) goes offline?

Given you can PM folks without having them as a friend, global quits might still cause issues.

offline_user_quits defaults to true. Any change of presence to offline results in a global irc quit. But you can still contact people after they quit, you get away instead of the more traditional "no such nick".

Another consideration for never_offline in channels is Discord lets you mention folks regardless of their status, it'll just show up whenever they next sign in/via notification. Having people always in channel makes tab-complete easier, e.g. if dealing with someone on a spotty connection. Unfortunately I don't think it's possible to emulate the web UI's @[name here] auto-complete without showing offline folks as away...

I think what you're saying just works with never_offline and the latest patch. You can definitely have offline people joined to a channel, that's the main thing this PR does.

dequis commented 6 years ago

I rebased on top of master, applied the patch as a commit (with an admittedly crappy commit message but you're going to squash this anyway so whatever) and tested a bit. I think never_offline isn't doing anything at all.

(wrote the previous comment before pushing, sorta expected the commits to appear here in the middle but I guess i pressed submit too late)

sm00th commented 6 years ago

You can push it to this PR btw.

I have much to learn about github.

Fixed up things for never_offline to retain its functionality in friendship_mode and updated docs accordingly. discord_handle_relationship is a bit clunky right now having to do 2 list searches (bee_user_by_handle and get_user) when adding friends but alternatives require discord_handle_relationship's prototype changes and I am not sure whether it is worth it.

@dequis if you have no objections/additions I think we are ready to merge this.

dequis commented 6 years ago

Pinged the wrong dx there btw.

I think it's fine

Dx commented 6 years ago

Hehehe no problem

On 24 de sep de 2017 12:26 -0500, dx notifications@github.com, wrote:

Pinged the wrong dx there btw. I think it's fine — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.