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

feat: add Matrix -> Discord reactions #877

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

mangofeet
Copy link

@mangofeet mangofeet commented Feb 13, 2023

Implements reactions from matrix to discord, intended for cases where a single matrix user is using the bot as a personal puppet

Functionality is disabled by default and enable by setting bridge.enableMatrixReactions to true

  • Send reactions to discord from Matrix to Discord
  • Removes reactions when redacted in Matrix
  • Handle thumbs up exception from Matrix to Discord
  • Handle thumbs up exception from Discord to Matrix

@mangofeet mangofeet requested a review from a team as a code owner February 13, 2023 21:39
Copy link
Contributor

@jaller94 jaller94 left a comment

Choose a reason for hiding this comment

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

Looking forward to Discord reactions. 👏
Do you have everything you need to finish the PR?

# install some dependencies needed for the build process
RUN apt update && apt install -y build-essential make gcc g++ python3 ca-certificates libc-dev wget git

COPY . /tmp/src
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice optimisation for having to build the first step less often. 👍

Copy link
Author

@mangofeet mangofeet Feb 14, 2023

Choose a reason for hiding this comment

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

Wasn't sure if this should go in it's own PR, I was going to revert it, but yeah, it does speed up building by quite a bit when trying to test changes on a remote machine instead of locally.

EDIT: There's a way to make it even faster by adding just the package.json and yarn.lock files, doing the yarn install, then adding the source code and doing the build separately

src/bot.ts Outdated Show resolved Hide resolved
config/config.sample.yaml Outdated Show resolved Hide resolved
Co-authored-by: Christian Paul <[email protected]>
Signed-off-by: mangofeet <[email protected]>
@mangofeet
Copy link
Author

mangofeet commented Feb 14, 2023

Looking forward to Discord reactions. clap Do you have everything you need to finish the PR?

I still need to figure out how to respond to the redaction of a reaction in Matrix. Unfortunately it looks like I may have to figure out a way to store the records differently so that the event ID of the reaction that comes back in event.redacts can be used to look up the original Matrix event or at least the original Discord message, and it needs to save the emoji used so that it can remove the correct one I believe.

I'll be checking out the code that's in #862 to maybe pull some implementation details from there (hopefully without depdending on that PR since it's still in review I guess)

@mangofeet
Copy link
Author

mangofeet commented Feb 15, 2023

OK, so I got reactions redacting from matrix to discord. Still need to figure out the edge case of the thumbs up, but otherwise it's working as I intended now.

@mangofeet mangofeet changed the title WIP: Matrix reactions Matrix -> Discord Reactions Feb 15, 2023
@mangofeet mangofeet changed the title Matrix -> Discord Reactions feat: add Matrix -> Discord reactions Feb 15, 2023
@mangofeet
Copy link
Author

Handled the thumbs up exception from Matrix to Discord, but not the other way around yet, not sure if that's strictly necessary to merge this though. It may be better to get this and #862 merged, and then handle any emoji exceptions in a unified manner after the fact.

Comment on lines +729 to +732
let emoji = relatesTo.key
if (emoji.indexOf("👍") >=0 ) {
emoji = "👍"
}

Choose a reason for hiding this comment

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

After reading this if statement, I'm assuming that this is related to the skin-tones? If so, you may want to do proper parsing of those, as more than thumbs up is affected. There's also country flags which use a similar concept.

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