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

Wiadomość na wzmiankę @everyone #4

Closed
wants to merge 6 commits into from
Closed

Wiadomość na wzmiankę @everyone #4

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2f3bb7e
Update main.js
Jun 11, 2021
11a72d5
Update
Jun 11, 2021
4a953be
Fix
Jun 12, 2021
0099923
Poprawiono nazewnictwo, polepszono wyłapywanie błędów
Jun 12, 2021
f42d6e8
Fixes from comments (tested)
Aug 12, 2021
93795fc
Rebase #7
Aug 12, 2021
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: 0 additions & 30 deletions handlers/EveryoneAndHereMentionMessage.js
View file
Open in desktop

This file was deleted.

30 changes: 30 additions & 0 deletions handlers/mentionHandler.js
View file
Open in desktop
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.

MentionsList -> mentionsList, albo MENTIONS_LIST (w sumie zamiast "list" bardziej pasuje "dict", bo to nie jest lista - ale to szczegół). Jeśli ten słownik jest traktowany jako stała, to można go też wrzucić w Object.freeze.

const MENTIONS_DICT = Object.freeze({
  EVERYONE: '@everyone',
  HERE: '@here',
});

EVERYONE: '@everyone',
HERE: '@here'
}
const MENTION_PERMISSION = 'MENTION_EVERYONE';
const EMBED_COLOR = '#eb1540';
const EMBED_TITLE = 'Nie wołaj wszystkich!';
const EMBED_THUMBNAIL = 'https://i.imgur.com/nREiJww.png';
const EMBED_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ć.';

module.exports = {

handler: function (message){
Copy link
Member

@ScriptyChris ScriptyChris Jun 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

A czemu nie po prostu:

module.exports = function mentionHandler(message) { ... }

albo:

function mentionHandler(message) { ... }

module.exports = mentionHandler;

I wtedy użycie:

const mentionHandler = require('./handlers/mentionHandler');

client.on('message', (message) => {
  mentionHandler(message);
});

const pingEmbed = new Discord.MessageEmbed()
.setColor(EMBED_COLOR)
.setTitle(EMBED_TITLE)
.setThumbnail(EMBED_THUMBNAIL)
.setDescription(EMBED_DESCRIPTION);
const mentions = message.content.includes(MentionsList.EVERYONE) || message.content.includes(MentionsList.HERE);
try {
const hasPermission = message.member.hasPermission(MENTION_PERMISSION);
if (mentions && !hasPermission) {
message.author.send(pingEmbed);
}
} catch(error) {
Copy link
Member

@ScriptyChris ScriptyChris Jun 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@CodeKid0 upewnij się proszę, czy ten catch łapie błąd, bo wg docsów metoda .send(..) zwraca promisa. Zatem jeśli chce się łapać z niej błędy/wyjątki, to trzeba albo zapiąć się na nią metodą .catch(..) albo użyć na niej await i wtedy strukturalny catch będzie działać.

Moim zdaniem ten catch, w obecnej formie, nie łapie błędu.

console.log(error);
Copy link
Member

Choose a reason for hiding this comment

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

Skoro już jest tu ten try..catch, to dodał bym informację do logowania i użył metody error zamiast log, np. console.error('mentionHandler(..) send error:', error). Ponadto, jeśli oczekiwane jest, że sypnąć może się tylko wywołanie message.author.send, to wyrzuciłbym z bloku try (przeniósł wyżej) tworzenie zmiennej hasPermission (tam się raczej nie ma co wysypać?). Wtedy jeśli się coś sypnie, to będzie można jednoznacznie stwierdzić, że to wysyłanie i informacja w logu na to wskaże. Deklaracja tej zmiennej hasPermission w bloku try - przy założeniu, że ona się nie sypie - IMO trochę zaciemnia przed czym chroni cały try..catch.

Copy link
Author

Choose a reason for hiding this comment

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

Jeśli chodzi o hasPermission w try...catch to jeśli to byłoby poza try...catch to bot, gdy ktoś napisze w wiadomości prywatnej wzmiankę się wysypie

Copy link
Member

Choose a reason for hiding this comment

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

Dlaczego się sypie? Jaki błąd tam jest rzucany? Z tego co kojarzę, to tam chyba jest błąd, że hasPermission is not a function i to by można zaifować albo if (hasPermission) albo if (typeof hasPermission === 'function').

Copy link
Author

Choose a reason for hiding this comment

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

Rzucany jest błąd hasPermission is not defined

Copy link
Member

Choose a reason for hiding this comment

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

No to go zaifuj zamiast robić try..catch.

Copy link
Author

Choose a reason for hiding this comment

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

Problem w tym że discord.js niezbyt pozwala to zifować (albo poprostu jestem zbyt głupi by to zrobić :V)

Copy link
Member

Choose a reason for hiding this comment

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

W jakim sensie nie pozwala?

Zamień:

try{ //needed, if someone sends private message to bot, it crashes without try..catch
    const hasPermission = message.member.hasPermission(MENTION_PERMISSION);

na:

const hasPermission = message.member.hasPermission && message.member.hasPermission(MENTION_PERMISSION);

, albo:

const hasPermission = typeof message.member.hasPermission === 'function' ? 
  message.member.hasPermission(MENTION_PERMISSION) : false;

Copy link
Author

Choose a reason for hiding this comment

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

Moim zdaniem to nie zadziała, discord.js crashuje się jeśli spróbujesz uzyskać message.member.hasPermission bez try..catch

Copy link
Member

Choose a reason for hiding this comment

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

A próbowałeś? message.member.hasPermission to jest property - jeśli .hasPermission nie jest zdefiniowane, to ono zwraca undefined, ale możesz na nim zrobić typeof, żeby sprawdzić jego typ (to docelowo powinna być funkcja, bo ją wołasz). Nie rozumiem dlaczego takie sprawdzenie miało by nie działać - może pod spodem jest tam jakiś getter lub Proxy, które rzuca błąd przy próbie odczytu wartości niezdefiniowanej, ale na normalnym propertisie powinno się naturalnie móc sprawdzić jego obecność i typ. Jeśli natomiast, w jakimś przypadku, nie ma samego message.member, to wtedy trzeba najpierw sprawdzić jego obecność, a potem dopiero message.member.hasPermission.

}
}
}
5 changes: 2 additions & 3 deletions main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
const Discord = require('discord.js');
const commands = require('./commands');
const EveryoneAndHereMentionMessage = require('./handlers/EveryoneAndHereMentionMessage');
require('./handlers/mentionHandler');
require('dotenv').config();

const client = new Discord.Client();

client.on('ready', () => {
Expand Down Expand Up @@ -32,6 +31,6 @@ client.on('ready', () => {
});
});
client.on('message', message => {
EveryoneAndHereMentionMessage.mentionHandler(message);
mentionHandler.handler(message);
});
client.login(process.env.TOKEN);