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 3 commits
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
30 changes: 30 additions & 0 deletions handlers/EveryoneAndHereMentionMessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const Discord = require('discord.js');
const MentionsList = [
Copy link
Member

Choose a reason for hiding this comment

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

Tu bardziej niż tablica pasowało by użyć obiektu, ponieważ jest to jakaś mapa słownikowa. Łatwiej wtedy odczytywać wartości z takiego słownika, bo zamiast MentionsList[0] robi się MentionsList.EVERYONE, co nawet IDE może podpowiedzieć. Tablice nadają się do struktur o uporządkowanej kolejności, a tutaj niwelujemy magic stringi do postaci reużywalnych zmiennych.

W TypeScript tu nadał by się enum (który i tak wyjściowo jest obiektem).

'@everyone',
'@here'
];
const MentionPermission = 'MENTION_EVERYONE';
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.

Ponownie co do konwencji nazewniczej - nazwy z dużej litery są używane dla zapisu konstruktorów i klas. Do oznaczenia stałej używa się tzw. SCREAMING_SNAKE_CASE, a więc na przykład: MENTION_PERMISSION. I ta sugestia dotyczy wszystkich stałych w kodzie.

Copy link
Author

Choose a reason for hiding this comment

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

Dzięki za zwrócenie uwagi, naprawię przy kolejnym (czwartym już, eh..) commicie.

const MentionEmbedColor = '#eb1540';
const MentionEmbedTitle = 'Nie wołaj wszystkich!';
const MentionEmbedThumb = 'https://i.imgur.com/nREiJww.png';
const MentionEmbedDesc = '\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ć.';

module.exports = {

mentionHandler: function (message){
const pingEmbed = new Discord.MessageEmbed()
.setColor(MentionEmbedColor)
.setTitle(MentionEmbedTitle)
.setThumbnail(MentionEmbedThumb)
.setDescription(MentionEmbedDesc);
const mentions = message.content.includes(MentionsList[0]) || message.content.includes(MentionsList[1]);
if (mentions) {
Copy link
Member

Choose a reason for hiding this comment

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

adrian17 pisał o tym na discordzie, jest do tego gotowe property w bibliotece z której korzystamy
https://discord.js.org/#/docs/main/stable/class/MessageMentions
Kwestia kosmetyczna ale skoro już jest to warto z tego korzystać.

Copy link
Author

Choose a reason for hiding this comment

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

d u d e, MessageMentions nie wykrywa wzmianki jeśli jest WYŁĄCZONA, a o to chodzi..

try{
const hasPermission = message.member.hasPermission(MentionPermission);
message.author.send(pingEmbed);
} catch {
Copy link
Member

Choose a reason for hiding this comment

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

Dalej będę naciskał na tego pustego catcha. Odpowiedziałeś na to tylko częściowo, bo zakładasz jeden przypadek. Co z pozostałymi? Nie uważasz że wypadało chociaż dać console.log na błąd który wystąpił? Jakie inne błędy mogą się tu pojawić? Czy niektóre z nich nie wymagają zatrzymania procesu i zrestartowania bota, tak jak np sytuacja gdy zmienia się token i bot traci uprawnienia do wysyłania wiadomości?

Copy link
Author

Choose a reason for hiding this comment

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

Dobra uwaga, naprawię.


}
}
}
}
5 changes: 4 additions & 1 deletion main.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const Discord = require('discord.js');
const commands = require('./commands');
const EveryoneAndHereMentionMessage = require('./handlers/EveryoneAndHereMentionMessage');
Copy link
Member

Choose a reason for hiding this comment

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

Konwencja w JS (i AFAIK nie tylko w nim) jest taka, że z dużej litery nazywa się konstruktory i klasy. Ta funkcja nie jest ani jednym ani drugim, więc powinna być nazwana w formie camelCase everyoneAndHereMentionMessage.

require('dotenv').config();

const client = new Discord.Client();
Expand Down Expand Up @@ -30,5 +31,7 @@ client.on('ready', () => {
}
});
});

client.on('message', message => {
EveryoneAndHereMentionMessage.mentionHandler(message);
Copy link
Member

Choose a reason for hiding this comment

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

Jeśli moduł EveryoneAndHereMentionMessage ma tylko jedną eksportowaną funkcję, to po co ją traktować jako metodę? Prościej użyć po prostu everyoneAndHereMentionMessageHandler - nazwa (ponownie, z małej litery) zawiera w sobie od razu suffix "Handler". Wtedy tę funkcję można wyexportować bezpośrednio z modułu - już bez owijania w obiekt.

});
client.login(process.env.TOKEN);