matrix-org / matrix-appservice-discord

A bridge between Matrix and Discord.
Apache License 2.0
813 stars 152 forks source link

Change to upstream `better-discord.js` #768

Open LecrisUT opened 2 years ago

LecrisUT commented 2 years ago

Looking through Suronume's changes to the better-discord.js, I see that it simply adds editMessages, however this is already present in upstream https://github.com/discordjs/discord.js/blob/b2dfb2ec70c33d30cf9b4a0b6757fbc6c750826c/packages/discord.js/src/structures/Webhook.js#L315 The upstream also includes threaded interface for future use, so how about refactoring this dependency? Api is different, but it shouldn't be hard to change.

Sorunome commented 2 years ago

sorus thing does way more, it also e.g. adds usertoken support

    1. 2022 8:36:49 LecrisUT @.***>:

Looking through Suronume's changes to the better-discord.js, I see that it simply adds editMessages, however this is already present in upstream https://github.com/discordjs/discord.js/blob/b2dfb2ec70c33d30cf9b4a0b6757fbc6c750826c/packages/discord.js/src/structures/Webhook.js#L315 The upstream also includes threaded interface for future use, so how about refactoring this dependency? Api is different, but it shouldn't be hard to change.

— Reply to this email directly, view it on GitHub[https://github.com/Half-Shot/matrix-appservice-discord/issues/768], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AASSEVB33JBX42ED7T4YJHLUVKEA7ANCNFSM5LS63FZQ]. Triage notifications on the go with GitHub Mobile for iOS[https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675] or Android[https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub]. You are receiving this because you are subscribed to this thread. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEgAAABICAYAAABV7bNHAAAAAXNSR0IArs4c6QAAAARzQklUCAgICHwIZIgAAAArSURBVHic7cEBDQAAAMKg909tDjegAAAAAAAAAAAAAAAAAAAAAAAAAAA+DFFIAAEctgHwAAAAAElFTkSuQmCC###24x24:true###][Sledovací obrázek][https://github.com/notifications/beacon/AASSEVGCSMELDPE5BQ7SWTTUVKEA7A5CNFSM5LS63FZ2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4QLLIGGQ.gif]

LecrisUT commented 2 years ago

Do you have a list of changes or PRs so we can check if they are implemented upstream? I see that there are differences in implementation so simply merging upstream will be difficult.

Sorunome commented 2 years ago

nope, but the big things are usertoken and friends management. Also, plop into the mx-puppet-bridge, daniel is also tackeling the aame issue there.

    1. 2022 8:44:57 LecrisUT @.***>:

Do you have a list of changes or PRs so we can check if they are implemented upstream? I see that there are differences in implementation so simply merging upstream will be difficult.

— Reply to this email directly, view it on GitHub[https://github.com/Half-Shot/matrix-appservice-discord/issues/768#issuecomment-1008611168], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AASSEVBFHA6NEIRXA3BS3DTUVKE7NANCNFSM5LS63FZQ]. Triage notifications on the go with GitHub Mobile for iOS[https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675] or Android[https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub]. You are receiving this because you commented. [data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEgAAABICAYAAABV7bNHAAAAAXNSR0IArs4c6QAAAARzQklUCAgICHwIZIgAAAArSURBVHic7cEBDQAAAMKg909tDjegAAAAAAAAAAAAAAAAAAAAAAAAAAA+DFFIAAEctgHwAAAAAElFTkSuQmCC###24x24:true###][Sledovací obrázek][https://github.com/notifications/beacon/AASSEVFDAGOKNGEIFQZTCYDUVKE7NA5CNFSM5LS63FZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOHQPC6YA.gif]

LecrisUT commented 2 years ago

Kay, thanks for the info.

erkinalp commented 2 years ago

it also e.g. adds usertoken support

What is bad with that?

dsonck92 commented 2 years ago

If logging in as regular users is out of scope for this bridge (which I would suggest it is), then I second the decision to go to upstream again. In my eyes, the appservice series of bridges are best off focusing on bridging in a bot/webhook way, intended for "other chat"-admins to invite to their equivalent of rooms. Essentially the way some IRC<->Discord bridges are made. While bridges dedicated to user bridging through puppeting should focus on doing that, by any means necessary (user tokens, talking client API-like, etc.)

Essentially I see them as "official" and "personal" bridges. The "official" bridges being targeted to moderators/admins of projects that happen to have multiple chats (e.g. software projects on Slack/Discord). The "personal" bridges being targeted to users that happen to require access to Discord/Slack/etc. because some of their peers, or products (e.g. games, or software projects on Slack/Discord) haven't seen Matrix yet (or refuse to use it).

But that's just my view on it. If this project feels it should implement puppeting on top of bot usage, then there's little possibility to change this as it requires the fork.

erkinalp commented 2 years ago

@dsonck92 If you want to support both, you can use fosscord/fosscord.js. It is a fork of discord.js that allows changing the endpoint and use with either user or bot access tokens.

dsonck92 commented 2 years ago

Oh I don't maintain either project anymore (for a puppeting bridge I just lift on mautrix-discord which isn't node.js based), and this project I just use for bridging my own server over to Matrix as I have the power to do so. But fosscord/fosscordjs sounds like a good idea then.