Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable self messages #103

Merged
merged 3 commits into from
Sep 22, 2017
Merged

Enable self messages #103

merged 3 commits into from
Sep 22, 2017

Conversation

mickael9
Copy link
Contributor

@mickael9 mickael9 commented Sep 20, 2017

Messages sent from another discord client will now appear as if you sent
them from Bitlbee.

See https://wiki.bitlbee.org/SelfMessages for more information about
this feature.

@dequis
Copy link
Contributor

dequis commented Sep 20, 2017

Have you tested this? What are you trying to do? That flag does nothing for groupchat messages

@mickael9
Copy link
Contributor Author

I tested this on channels, it allows messages sent by myself from another discord client to appear as self messages on IRC (https://wiki.bitlbee.org/SelfMessages)

@dequis
Copy link
Contributor

dequis commented Sep 20, 2017

Nevermind the flag does do something, confused it with something else.

It's not strictly correct to apply it to all messages, but I guess it doesn't hurt either.

@mickael9
Copy link
Contributor Author

mickael9 commented Sep 20, 2017

Yes, I noticed that, I'm trying to improve it now since I'm not actually sure it doesn't hurt having it unconditionally. I'd also like it to work with private messages!

@mickael9 mickael9 changed the title Enable self messages WIP: Enable self messages Sep 20, 2017
@sm00th
Copy link
Owner

sm00th commented Sep 20, 2017

The patch I had for this was a bit more complicated. The problem I had with this is described in #7: this results in echoed messages when sending from bitlbee which I see as the main usecase.

@mickael9
Copy link
Contributor Author

mickael9 commented Sep 20, 2017

I see what this is trying to do but it seems to have a race conditon between the HTTP post response and the echo from the gateway (in the debug log, the gateway echo appears first).

I see there is a nonce field that can be set when sending a message which gets repeated in the echo from the gateway. We could set it to a random string and use that instead of the message id which can't be predicted.

Perhaps we could do even simpler, setting the nonce to a fixed value per connection so we can ignore all self messages that we generated ourselves without having to keep track of them.

@sm00th
Copy link
Owner

sm00th commented Sep 20, 2017

I see what this is trying to do but it seems you have a race conditon between the time you receive the message info (with its id) from the HTTP POST request and the time you receive the message from the gateway (in the debug log, the gateway response appears first).

Yeah, that was written a while ago, things probably have changed.

I see there is a nonce field that you can set in the message (and which gets repeated in the echo), we could set this to a random string and use that instead of the message id which we can't predict.

Oh so they did add something we can use for this, nice. Haven't looked at discord's API in a while. I think that at this point we don't even need the nonce string to be unique per-session, it can just be hardcoded as "bitblee" or something like that. It could be later changed to random ids if it is decided to use this field for it's original purpose.

Messages sent from another discord client will now appear as if you sent
them from Bitlbee.

See https://wiki.bitlbee.org/SelfMessages for more information about
this feature.
@mickael9
Copy link
Contributor Author

mickael9 commented Sep 20, 2017

PR updated with some inspiration from @sm00th's patch

For some reason, self messages don't work in private chats, but I think this might be an issue on the bitlbee side

Edit: actually, this was my mistake, it seems to work flawlessly now ! 🎉

Copy link
Owner

@sm00th sm00th left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that one comment I am pretty happy with this code.

@@ -494,8 +509,8 @@ static gboolean discord_prepare_message(struct im_connection *ic,
}

if (cinfo->type == CHANNEL_PRIVATE) {
if (g_strcmp0(author, cinfo->to.handle.name) == 0) {
posted = discord_post_message(cinfo, cinfo->to.handle.name, msg);
if (is_self || g_strcmp0(author, cinfo->to.handle.name) == 0) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check is not needed anymore. It is originally here to mute selfmessages, but now one of the conditions will always be true (unless I'm missing something) so we can just drop it and always call discord_post_message()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I fixed this, let me know if you'd rather have my squash my commits

@mickael9 mickael9 changed the title WIP: Enable self messages Enable self messages Sep 22, 2017
@sm00th sm00th merged commit 0dd7727 into sm00th:master Sep 22, 2017
@mickael9 mickael9 deleted the self-messages branch September 24, 2017 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants