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

Cog handler & error logging #286

Closed
wants to merge 4 commits into from

Conversation

ultralegendary
Copy link

Hi, I tried to explain the added commands and its workflows here. Before making this PR, I tested this to make sure nothing is breaking.

The cog loading feature helps developers and contributors to play with code and test the cog and its commands without having to retart the bot everytime. You can now easily add other cogs to the cogs/ directory and load them without having to restart the bot.

Let me know if I have missed to explain somthing. or somthing needs to be modified.

Things changed:

Added commands

  • cog list
    List the set of loaded and available cogs.

  • cog load cogname
    Load a cog from the available cog list. You need to register a cog before loading a cog.
    Successfully loaded cog will be added to loaded cog list and the status will be updated to cogs.json

  • cog register cogname description
    A new cog needs to be registered. The cog file needs to be present under cogs directory. Once successfully registerd, the cog will be added to cogs.json. A newly registerd cog is not loaded by default.

  • cog reload cogname
    A loaded cog will be reloaded again.

  • cog unload cogname
    A loaded cog can be unloaded using this command. All the commands present inside this cog will be disabled. Cog status and the cogs.json will be updated when the unload is successful.
    Unloading the admin cog is not allowed. If you want, I can remove this condition.

  • cog unregister cogname
    A registerd cog can be unregisterd. The cog should unloaded before unregistering. Unregistering will remove the entry from cogs.json.

  • shutdown
    Allow the bot to shutdown. This comand is to avoid terminating this from terminal.
    You will need to click the shutdown bot as confirmation for shutdown. Timeout for the button is set to 15 seconds.

  • logerrors
    This command enabled to be used only on channel. When used in a channel, that channel will be used to log the errors.
    If the logerrors is already enabled in another channel, it will be overridden. Logerrors are only enabled to one channel.

Added files

  • Added cogs/cogs.json file to store the available cogs and its status.
    Structure of the file:
    cogs.json is array of Json objects.
    Each Json object represent one single cog. This file will be read when the bot is loaded. All the cogs with is_enabled=true will be load the cog. If for some reason, the loaded of cog fails, the status will be updated to false.

Example of single object:

{
    "name": "moderation",
    "description": "Moderation commands for the bot.",
    "is_enabled": true
} 

name: Name of the cog. This should match with the file name of the cog. The above example will check for moderation.py file in /cogs directory
description: A short description about what the cog is about.
is_enabled: Describes if the cog is loaded or not loaded.

  • Added cogs/exceptionhandler.json file to store the channel to which the logerrors is enabled.
    Structure of the file:
    a single json object which has the details of the channel.
    Example of the file:
{
    "guild_id": 781133714306105092,
    "guild_name": "Hackclub - hacktoberfest",
    "channel_id": 818839322790894382,
    "channel_name": "errorlogs"
}
  • modified cogs/__init__.py to support automatic cog loading. this file contains the variables cogs, loaded_cogs, all_cogs. these are loaded here and these are used in bot.py to load the cogs when the bot loads.

  • cogs/igknite_ascii.txt I thought of adding a ascii art to the logging terminal when a bot loads. I am leaving this upto you. You can read the file and display the ascii art to the logger

Issues that are covered: #280 #281

Thanks.

cogs/inspection.py Outdated Show resolved Hide resolved
cogs/general.py Outdated
@@ -140,7 +144,7 @@ async def _ping(self, inter: disnake.CommandInter) -> None:
async def help(self, inter: disnake.CommandInter):
embed = core.TypicalEmbed(
inter=inter,
title="Hey there! I'm IgKnite.",
title="Hey there! I'm IgKnite help.",
Copy link
Author

Choose a reason for hiding this comment

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

one more typo

Copy link
Member

Choose a reason for hiding this comment

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

@ultralegendary Uh, this is not a typo though. It's supposed to be an introductory line.

Copy link
Author

Choose a reason for hiding this comment

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

i added an "help" at the end.

Copy link
Member

Choose a reason for hiding this comment

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

@ultralegendary It isn't meant to be that way. Could you please remove it?

core/ui.py Outdated Show resolved Hide resolved
@ultralegendary
Copy link
Author

hey @hitblast. did a overall review and modified some statments.
Can you review and let me know if any changes required.

Copy link
Member

@furtidev furtidev left a comment

Choose a reason for hiding this comment

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

@ultralegendary yo! Thanks for the PR but unfortunately it cannot be merged with main.

It adds a lot of baggage and maintenance hell to the project. The code standard doesn't match our own (take a deeper look into how to write readable code, and further more read IgKnite's source code). There's a lot of global usage (see here why it's a bad idea). And further more, this is not how I'd do it. (see below)

The logger doesn't seem to persist to disk, which makes it inherently not very helpful for debugging purposes (see here for better logging practices and see here for Python logging fundamentals).

shutdown command is redundant. And the cog hot reloading idea is indeed cool, but I think the implementation is poorly done. If I was asked to do this, I'd probably look at incorporating it within the development environment and not rely on Discord commands (e.g look at cogwatch).

@furtidev furtidev closed this Oct 14, 2024
@ultralegendary
Copy link
Author

hi @furtidev ,
Thanks for the suggestion. I will take a look at them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants