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

Wiadomość na wzmiankę @everyone #4

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions handlers/EveryoneAndHereMentionMessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const Discord = require('discord.js');

const color = '#eb1540';
const title = 'Nie wołaj wszystkich!';
const thumbnail = 'LINK'
const description = '\nRozumiemy, że potrzebujesz pomocy, ale nie wszyscy chcą zostać o tym powiadomieni.\n Jest nas tutaj dużo i nie ma sensu, aby każdy dostał bezpośrednio taką informację.\n Nie trudno sobie wyobrazić jak irytujące byłoby, gdyby każdy wołał wszystkich do każdego tematu.\n Dlatego zadaj pytanie i po prostu poczekaj - jeśli ktoś będzie wiedział i mógł, to na pewno spróbuje odpowiedzieć.'

export function mentionHandler(){
const pingEmbed = new Discord.MessageEmbed()
.setColor(color)
.setTitle(title)
.setThumbnail(thumbnail)
.setDescription(description);

const mentionedEveryone = message.content.includes("@everyone") || message.content.includes("@here")
Copy link
Member

Choose a reason for hiding this comment

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

Te stringi @everyone i @here też można wrzucić w zmienne, albo nawet lepiej, zgrupować w jakiś słownik.

Copy link
Member

Choose a reason for hiding this comment

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

Skąd ta funkcja ma dostęp do message? Nie widzę tu podawania referencji w argumentach. Rozumiem że sprawdziłeś to i działa?

Copy link
Author

Choose a reason for hiding this comment

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

Te stringi @everyone i @here też można wrzucić w zmienne, albo nawet lepiej, zgrupować w jakiś słownik.

Myślę, że nie ma takiej potrzeby, gdyż i tak raczej nowe pingi się nie pojawią.

Copy link
Member

@ScriptyChris ScriptyChris Jun 12, 2021

Choose a reason for hiding this comment

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

Myślę, że nie ma takiej potrzeby, gdyż i tak raczej nowe pingi się nie pojawią.

Chodzi o to, że te stringi kontekstowo należą do pewnej grupy - odpowiadają za pingowanie userów. Wyobraź sobie sytuację, że funkcjonalność jest rozszerzana i w kilku miejscach korzystasz z tych samych stringów + dochodzą jakieś inne stringi odpowiadające np. komendom (niech będzie /test). W pewnym momencie trudno jest Ci się połapać w kodzie co kontekstowo oznacza dany string, a jakbyś chciał jednak zmienić go w kilku miejscach to musisz te kilka miejsc edytować. W momencie, gdy wrzucisz takie stringi do słownika, to możesz je sobie zgrupować i wtedy szybciej i wygodniej jest się tym posługiwać, np. DICTIONARY.PINGS.EVERYONE i DICTIONARY.PINGS.HERE są łatwiejsze w interpretacji przez programistę niż jakiś randomowy string '@everyone' w kodzie. Poza tym, jeśli np. chcesz sprawdzić w apce jakie podobne stringi są używane, to szukasz pierwszego lepszego (albo idziesz bezpośrednio do miejsca deklaracji słownika) i odnosisz się do jego obiektu, gdzie masz spis wszystkich kontekstowo podobnych stringów - w jednym miejscu (które można na dodatek sobie udokumentować).

const hasPermission = message.member.hasPermission('MENTION_EVERYONE')
Copy link
Member

Choose a reason for hiding this comment

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

J.w. odnośnie MENTION_EVERYONE

if (mentionedEveryone && !hasPermission) {
try{
Copy link
Member

@ScriptyChris ScriptyChris Jun 11, 2021

Choose a reason for hiding this comment

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

try..catch jest tu zbędny. To już było wspomniane gdzieś w poprzednim review-komentarzu.

Copy link
Author

Choose a reason for hiding this comment

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

Nie, ponieważ jak nie będzie try, catch, to bot by scrashowal jak ktos napisze @here albo @everyone w prywatnej wiad. do bota

Copy link
Member

Choose a reason for hiding this comment

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

A co konkretnie się wtedy crashuje?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, ale to i tak trzeba obsłużyć. Zobacz jaki błąd w takiej sytuacji wyskakuje, wtedy możnabygo ignorować. Ale inne błędy powinny zostać co najmniej zlogowane albo optymalnie wysłane do jakiegos prawilnego loggera albo sentry.

Copy link
Author

Choose a reason for hiding this comment

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

Sprawdziłem i błąd można zignorować, ponieważ dotyczy on tego, że nie może sprawdzić w prywatnej wiadomości czy ktoś ma permisję. Bez try..catch bot wprost się wyłączy.

Copy link
Member

@ScriptyChris ScriptyChris Jun 12, 2021

Choose a reason for hiding this comment

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

nie może sprawdzić w prywatnej wiadomości czy ktoś ma permisję

Czy API Discorda nie pozwala na sprawdzenie czy wiadomość jest prywatna? Można by wtedy zapisać warunek, że bot obsłuży tylko wiadomość publiczną.

Na SO coś o tym piszą i w docsach też coś na ten temat jest:
https://stackoverflow.com/a/48729333/4983840
https://discord.com/developers/docs/resources/channel#channel-object-channel-types

Copy link
Member

Choose a reason for hiding this comment

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

Dobrze, ale to jeden case. Tak jak pisałem wyżej, mogą się pojawiać też inne błędy które tutaj złapiesz ale ich nie obsłużysz. Np. co się stanie jeżeli bot nie będzie mógł wysłać wiadomości na serwer z powodu utraty połączenia/zmiany secretu? Na podstawie tego catcha taka sytuacja będzie nie do odróżnienia od przypadku który opisujesz.

message.author.send(pingEmbed)
} catch {

}
}
}
29 changes: 4 additions & 25 deletions main.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import mentionHandler from './handlers/EveryoneAndHereMentionMessage'
Copy link
Member

@ScriptyChris ScriptyChris Jun 11, 2021

Choose a reason for hiding this comment

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

Póki co, te moduły korzystają ze składni CommonJS, więc dla spójności warto używać funkcji require zamiast zapisu import z ES6.

const Discord = require('discord.js');
const commands = require('./commands');
require('dotenv').config();
Expand Down Expand Up @@ -30,29 +31,7 @@ client.on('ready', () => {
}
});
});
client.on('message', message => { //Ten event wykonuje się, gdy bot wykryje wiadomość
const pingEmbed = new Discord.MessageEmbed()
.setColor('#eb1540')
.setTitle('Nie wołaj wszystkich!')
.setThumbnail('https://cdn.discordapp.com/attachments/617673807213232128/852887484542615572/Pingsock.png')
.setDescription('\nRozumiemy, że potrzebujesz pomocy, ale nie wszyscy chcą zostać o tym powiadomieni.\n Jest nas tutaj dużo i nie ma sensu, aby każdy dostał bezpośrednio taką informację.\n Nie trudno sobie wyobrazić jak irytujące byłoby, gdyby każdy wołał wszystkich do każdego tematu.\n Dlatego zadaj pytanie i po prostu poczekaj - jeśli ktoś będzie wiedział i mógł, to na pewno spróbuje odpowiedzieć.');

if (message.content.includes("@everyone")) { //Jeśli wiadomość zawiera @everyone..
try{
if(!message.member.hasPermission('MENTION_EVERYONE')) //Sprawdzamy, czy wysyłający ma uprawnienia do wzmianki @everyone, jeśli nie to bot wysyła wiadomość
message.author.send(pingEmbed)
}catch{
console.log(message.author.username + " wysłał ping w wiadomości prywatnej do bota, hmm..")
}

} else {
if(message.content.includes("@here")) //Jeśli wiadomość zawiera @here..
try{
if(!message.member.hasPermission('MENTION_EVERYONE')) //Sprawdzamy, czy wysyłający ma uprawnienia do wzmianki @everyone, jeśli nie to bot wysyła wiadomość
message.author.send(pingEmbed)
}catch{
console.log(message.author.username + " wysłał ping w wiadomości prywatnej do bota, hmm..")
}
}
});
client.on('message', message => {
mentionHandler();
});
d-bend marked this conversation as resolved.
Show resolved Hide resolved
client.login(process.env.TOKEN);