This gist will outline all the bad practices shown by the youtuber Lucas in his Making a discord bot series
Lucas is a youtuber that posts programming videos on youtube. he has a whole series on how to make a discord bot that has 14 videos currently. His code often contains badly written or outright wrong code
In programming, there are several ways to achieve the same thing. Some are good, some are not good. most of the time the method lucas shows in his videos are not optimal, and are considered bad practice
Even the creator of discord.py (Danny#0007) discourages tutorials
Now I do not want to spread hate, someone in the official discord.py server asked for a list of what was wrong with lucas' videos so he can fix them, so here it is
I myself watched lucas when I first started coding bots (around 23 Apr 2020). and I have found many problems in my code because of that.
The part one of his series shows mainly the installation and setup but it still has some problems out of which one is naming the bot, client which is in my opinion the biggest mistake in the whole series
Required python Version
At 2:22he says that you need to have Python 3.5.3 in order to use rewrite. Which at the time of making the video was correct, but as of the time writing this guide, the required python version is changed to python 3.8 in discord.py version 2.0
pip install
At 6:04he tells you to install discord.py using pip install discord.py which only works if the python scripts folder is added to path. if it is not added to path then you will see a message like this
'pip' is not recognized as an internal or external command,
operable program or batch file.To fix this you would either need to add the scripts folder to path or use py -m pip install discord.py on windows and python3 -m pip install discord.py on Mac/Linux
from discord.ext import commands
At 8:27he states that this line is really important and tells his viewers to make sure they have that line.
from discord.ext import commandsNow whether it is important or not depends on what your definition important is. if you do not add that line you could still do everything you can do with that line, they would just take a bit more words. for example
import discord
from discord.ext import commands
bot = commands.Bot(command_prefix="!")is the same as
import discord
bot = discord.ext.commands.Bot(command_prefix="!")client variable
At 8:38“We're gonna add a client variable. we're gonna set that equal to commands.Bot”
which is extremely misleading since he has a bot but he named it client. a bot and a client are not the same thing. A bot (commands.Bot) is like a client but has access to commands and bot specific features, a client (discord.Client) is just a client that can accept events and do stuff a normal user can
The whole video is unnecessary
Instead of doing everything he shows, you can just use the following command in a folder and the discord.py will automatically create the files he created with even better codepython -m discord newbotThe code he showed does not work after discord.py version 1.5.0 since discord added something called intents and he provides a huge misinformation
Intents
This is due to a breaking change introduced by Discord which is now being enforced. Bots will no longer receive certain information from Discord unless they subscribe to those specific Intents. Here is what you must do:
-
Log on to your developer portal at https://discord.com/developers/applications. Click on your bot, then the "Bot" tab on the left, and scroll down to the Privileged Gateway Intents section. There will be two toggles, one for PRESENCE INTENT and one for SERVER MEMBERS INTENT. Switch the second one on to allow your bot to receive server members. Bots in more than 100 guilds will need to complete the verification process and apply for these intents.
-
Update your discord.py to version 1.5.0 or newer. Make sure you're installing it to the correct environment
-
Modify your commands.Bot constructor and pass an instance of discord.Intents. Here is an example -- READ AND UNDERSTAND, DO NOT BLINDLY COPY AND PASTE INTO YOUR CODE!!!!
intents = discord.Intents.default() # All but the two privileged ones intents.members = True # Subscribe to the Members intent bot = commands.Bot(command_prefix = '!', intents=intents) # add the intents= part to your existing constructor call
Can't access guild in on_member_join/on_member_remove
At 5:56“We can't exactly access the server”
which is wrong and misleading. and after that, he says
“So if we wanted to use a server inside that function You'd have to use different methods that, again, discord.py... You can read through the documentation and look through all the various methods you could use to obtain a server object *but it's not given to us unfortunately so we can't manipulate it just by using the function parameters here”
Which is not even 1% correct.
you can ge the server just by using member.guild and the member is provided in the function parameters therefore everything he said there was wrong
In this part he shows 2 commands one of which is taken straight from the docs so is pretty good and the 2nd one is not as good
Using aliases instead of name
At 7:29He names his 8ball command, _8ball. which is not necessarily bad as per se. but then he proceeds to use the aliases kwarg to make sure people can use 8ball. now the aliases keyword argument was not made for that and a name kwarg already exists for that. so if you want to do it the right way
change
@bot.command(aliases=['8ball'])
async def _8ball...to
@bot.command(name="8ball")
async def _8ball...This part is pretty short where he just shows a purge/clear command
Directly using the amount in the clear command
At 1:50He uses limit=amount in purge() but setting the limit to amount will not delete the actual amount and will delete 1 less message due to the message sent that invoked the command. to fix this you could use either
- use
limit=amount+1instead oflimit=amount - use
await ctx.message.delete()before thepurge()
In this part he shows a kick and a ban command example
No permission checks
At 1:00 and at 3:43He does not do any permission checks and just straight up bans/kicks the user. this is not good because then anyone can ban or kick anyone in the server which you most likely do not want to happen
Ah yes, the notorious lucas unban
No permission checks
At 0:47 He does not do any permission checks and just straight up unbans the user. this is not good because then anyone can make a alt then unban themselvesUnbanning logic
At literally the whole videoThe code he uses for unbanning is
banned_users = await ctx.guild.bans()
member_name, member_discriminator = member.split('#')
for ban_entry in banned_users:
user = ban_entry.user
if (user.name, user.discriminator) == (member_name, member_discriminator):
await ctx.guild.unban(user)which is bad for several reasons
-
it fetches the bans which is a api call and can be avoided
-
it uses
member_name, member_discriminator = member.split('#')
when you can just remove that line and use it's id instead of all this hassle and even if you want to use name#discrim, this whole line is unnecessary
-
it loops through all the banned users. a server can have hundreds of banned users and this just tries to loop through all of them, although this will probably not be necessarily slow, it's still inefficient
-
it does
if (user.name, user.discriminator) == (member_name, member_discriminator):
when you can just cast the user to a str and compare it against member like so:
if str(user) == member:
If you do this then the line with
split("#")becomes unnecessary
await ctx.guild.unban(discord.Object(id=member_id))This video shows cogs and is also a source of bad practices and misleading names
No permission checks
At 0:54 and 2:54He does not do any permission checks and just loads the cog whenever someone uses the command. this is not good because then anyone can load and unload the cogs for the bot and cogs are not server specific if someone unloads a cog in one server your whole bot won't respond to the commands in that sever
Misleading name again
At 8:09Here he takes his bot as a parameter and names the parameter client, not only this, he also names the attribute self.client which is also misleading. in his setup function he does the same thing. we already discussed why this is bad previously
if you look around the docs enough, you may know that we can get the bot instance by using ctx.bot so the self.client there is mostly unnecessary
This video only has one line of code and even this video has a big issue
Changing presence in on_ready
At Basically the whole videoYou should not use change_presence (or make API calls) in on_ready within your Bot or Client.
Discord has a high chance to completely disconnect you during the READY or GUILD_CREATE events (1006 close code) and there is nothing you can do to prevent it.
Also as noted in the docs, on_ready is triggered multiple times, not just once.
Instead, we should use the activity and status kwargs in the constructor of these Classes. for example
bot = commands.Bot(command_prefix="!", activity=..., status=...)description
Changing the status very often
At timeChanging the status every 10 seconds is considered api abuse
What is api abuse?
Take a look at this chat in the discord api server
Deleted User: Question, if I run the Discord API every five minutes, is that considered API Abuse? And will that lead to rate limiting? Jake (Discord Staff): automating the API in that way /is/ abuse. Automatically doing "X" every N is generally not a good idea. Where X could be posting a message, changing someone's nickname, renaming a role, changing a channel topic, etc... Generally bots should only react to user actions... Although, for very large N we generally don't care. But for small N, we do care. Think rainbow bots, etc....
in this video he shows error example that is bad
His error handler is eating all errors other than CommandNotFound
At 9:42His error handler example is really bad because it handles CommandNotFound error but does not do anything if the error is something else.because of that, other errors will not show up in the console and will just be ignored and the person watching the video will spend hours trying to find what's wrong but the problem is not even being shown
And the funny thing is that he seems to understand that doing
@bot.event
async def on_command_error(ctx, error):
passis a noob trap and is not gonna let any errors be printed on the console.
but he still does something that only catches a single error and ignores the others. he also does it in his local error handler for the clear command
For a proper error handler you should either raise the error or print the exception
@bot.event
async def on_command_error(ctx, error):
if isinstance(...):
...
else:
# All other errors not caught come here. And we can just print the default traceBack.
print(f"Ignoring exception in command {ctx.command)}:", file=sys.stderr)
traceback.print_exception(type(error), error, error.__traceback__, file=sys.stderr)In this video he shows how to use checks and a custom check example that is taken from the docs
Making a useless check
At 6:46Here he writes the following check that is taken from the docs
def is_it_me(ctx):
return ctx.author.id == 395737777368465410Now you may say that this check is shown in the docs why is this bad?
The docs show this check as an example, he shows this check in his tutorial people are gonna see his video and think that that's the only way to have a owner check. but that's not, in fact you can do
@commands.command()
@commands.is_owner()
async def command_name(ctx):
await ctx.send("You are my owner")assuming you passed your owner_id to the bot constructor
but even if we ignore that, he does not show how to transform a check into it's own decorator which the docs do and is far easier than doing @commands.check every time.
In this video he shows how to make per server prefixes and this was his last video till a few days ago
Using json as a database
At 3:54Json is a data serialization format, that turns primitive data structures into a string.
A database is a formalized system designed to store and retrieve data, with a substantial
number of features and functionalities designed to support doing so at various scales,
and with a large number of guarantees.
A Json file as-a-db is fundamentally flawed due to its lack of atomic writes
which results in data corruption and loss when:
- The disk runs out of storage
- The program crashes during the write
- The computer crashes during write
- Two programs try to write to the file at the same time
Additionally:
- All reads require the entire file to be read
- All reads require the entire structure to be loaded into memory this does make it fast!
- All writes require the entire file to be re-written
The Json module makes no effort to ameliorate any of these concerns, because json-as-a-db falls
completely and entirely out of the scope of the standard.
If you need the file to be portable and want minimum setup, use sqlite, a standard designed to
solve all these issues.
Reading a file on every message
At 4:42Now whether this is good or bad is debatable but the best practice would be to read the file when the bot starts and save it to a variable, then when the prefix is needed, getting the prefix from the variable
This video is totally misleading
Using commands.MemberConverter instead of discord.Member
At timeIf you see the discord.py source code, you would know that the discord.Member converter uses the commands.MemberConverter to convert members, in the video lucas states the opposite and says we shouuld use commands.MemberConverter in order to get the features of that which is in fact, wrong
Bad DurationConverter implementation
At timein the video lucas uses the following code to give an example of a converter
class DurationConverter(commands.Converter):
async def convert(seLf, ctx, argument):
amount = argument[: -1]
unit = argument[-1]
if amount.isdigit() and unit in ['s','m']:
return (int(amount), unit)
raise commands.BadArgument(message='Not a valid duration')This is bad for several reasons
- This does not work with time that has multiple characters like
15m - This does not work with unit that has multiple characters like
mo
You should use regex to make a robust DurationConverter
time_regex = re.compile(r"(\d{1,5}(?:[.,]?\d{1,5})?)([smhd])")
time_dict = {"h":3600, "s":1, "m":60, "d":86400}
class DurationConverter(commands.Converter):
async def convert(self, ctx, argument):
matches = time_regex.findall(argument.lower())
time = 0
for v, k in matches:
try:
time += time_dict[k]*float(v)
except KeyError:
raise commands.BadArgument("{} is an invalid time-key! h/m/s/d are valid!".format(k))
except ValueError:
raise commands.BadArgument("{} is not a number!".format(v))
return timeIn this video he tries to show a help command example but the examples are not well
The video is just not worth 16 minutes
The whole video could have been 5 minutes or even a short (1 minute). What he shows is nothing helpful and he did not even show some methods we can use or anything really, he just showed the main 4 methods that we could override and that's it, nothing else. that could be gotten from the docs very easily, it would have been better if he actually wrote a good, clean, beautiful help by subclassing HelpCommand.
For a proper guide see https://gist.github.com/InterStella0/b78488fb28cadf279dfd3164b9f0cf96