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.

Sign up for GitHub

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

Jump to bottom

replace f*cks with hugs #308

Merged
merged 8 commits into from Jul 21, 2022
Merged

replace f*cks with hugs #308

merged 8 commits into from Jul 21, 2022

Conversation

danthe1st
Copy link
Member

@danthe1st danthe1st requested a review from a team as a code owner June 16, 2022 20:49
@danthe1st
Copy link
Member Author

I thought it would be funny to check every incoming message for the content fuck and replaces it with hug.
This is an anology to an attempt to replacing all fucks with hugs in the Linux kernel.

@danthe1st
Copy link
Member Author

Note that the username/avatar cannot be set as JDA5 currently doesn't have webhook support.

@danthe1st danthe1st marked this pull request as draft June 16, 2022 21:33
@jasonlessenich
Copy link
Member

you might want to take a look at this: https://github.com/MinnDevelopment/discord-webhooks

@danthe1st danthe1st marked this pull request as ready for review June 17, 2022 16:36
@jasonlessenich
Copy link
Member

It would be great if you (or someone else) could test this functionality tho

@jasonlessenich jasonlessenich added enhancement Improvement of an already existing feature new feature needs to be tested For code that needs to be tested before it can be merged to the production branch. labels Jun 17, 2022
@danthe1st
Copy link
Member Author

danthe1st commented Jun 17, 2022 •
edited

I tested it with normal text, images, images devlared as spoilers and in a thread. However, i still think someone else should test it in isolation.

Note that obtaining non-image attachments currently fail due to a bug (fixed but the fix is not yet published) in JDA. See discord-jda/JDA#2139 for details.

@MoonTM-GIT
Copy link
Member

I'll test it a bit

@MoonTM-GIT
Copy link
Member

Okay, so something that is kind of expected but I would still like to mention, is that voice-text channels are not supported by JDA yet (discord-jda/JDA#2072), so it doesn't work in these obviously

Other than that I haven't found anything that didn't work as expected, except for non-image attachments which you already mentioned

MoonTM-GIT
MoonTM-GIT approved these changes Jun 17, 2022
View reviewed changes
Copy link
Member

@MoonTM-GIT MoonTM-GIT left a comment

Choose a reason for hiding this comment

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

Not tested

@MoonTM-GIT
Copy link
Member

Not sure if we still want someone else to test this though

@jasonlessenich
Copy link
Member

would be cool if you cloud extract this webhook stuff into a separate method/class tho

@jasonlessenich
Copy link
Member

(for things like the messagelink listener)

@MoonTM-GIT
Copy link
Member

MoonTM-GIT commented Jun 18, 2022 •
edited

Oh, something I just noticed, if I ping someone in my initial message the ping will (as it shouldn't) not go through to the message the webhook sends. This might lead to confusion as someone might be looking for the message he was pinged in, which he can't find that easily since it doesn't have that orange mark, so we might want to disable this functionality for messages that include any pings

@danthe1st
Copy link
Member Author

Yes but if that feature would accept pings, it should be capped to e.g. a maximum of three mentions per message.
Aside from that, I might need to explicitely exclude the suggestion channel.

@MoonTM-GIT
Copy link
Member

I wasn't suggesting to let pings through, that would lead to more confusion as people would he shown two pings, instead it could just not replace anything in messages that include pings to cause no confusion about pings

@danthe1st
Copy link
Member Author

would be cool if you cloud extract this webhook stuff into a separate method/class tho

Done

I wasn't suggesting to let pings through, that would lead to more confusion as people would he shown two pings, instead it could just not replace anything in messages that include pings to cause no confusion about pings

Done

@danthe1st
remove hugs in suggestion channels
e865e65 
Replacing fucks with hugs would cause incompatibilities with the
suggestion listener.

However, fucks are still replaced with hugs in suggestion threads.
@danthe1st
add spaces around == in HugListener
c6c0a27
@MoonTM-GIT
Copy link
Member

So what's up with this?

@danthe1st
Copy link
Member Author

danthe1st commented Jul 18, 2022 •
edited

The functionality is implemented and has been tested against the old version of the bot.

@danthe1st danthe1st changed the title replace fucks with hugs replace f*cks with hugs Jul 20, 2022
@jasonlessenich jasonlessenich merged commit ef4af86 into Java-Discord:main Jul 21, 2022
@MoonTM-GIT
Copy link
Member

yay

@jasonlessenich
Copy link
Member

this is a huge step for the java community

@danthe1st danthe1st deleted the hugs branch July 21, 2022 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonlessenich jasonlessenich jasonlessenich approved these changes

@MoonTM-GIT MoonTM-GIT Awaiting requested review from MoonTM-GIT

Assignees

@danthe1st danthe1st

Labels
enhancement Improvement of an already existing feature needs to be tested For code that needs to be tested before it can be merged to the production branch. new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@danthe1st @jasonlessenich @MoonTM-GIT