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

Update examples #11

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Update examples #11

wants to merge 6 commits into from

Conversation

novanai
Copy link

@novanai novanai commented Nov 13, 2022

Various updates to examples, including

  • requirements
  • commands
  • intents
  • consistency

@novanai novanai marked this pull request as draft November 13, 2022 11:37
Copy link
Owner

@parafoxia parafoxia left a comment

Choose a reason for hiding this comment

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

Some general comments. I know we briefly discussed whether to include prefix command support in the server, but if it helps, this intro was originally designed to help people create slash commands using the various frameworks. Prefix commands were not considered at the time. What are your current thoughts on adding prefix command stuff?

Comment on lines +50 to +51
.add_field(name="Bot?", value=str(self.target.is_bot), inline=True)
.add_field(name="No. of roles", value=str(len(roles)), inline=True)
Copy link
Owner

Choose a reason for hiding this comment

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

I believe these string casts are unnecessary.

Copy link
Author

@novanai novanai Nov 16, 2022

Choose a reason for hiding this comment

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

They are unnecessary (it's just to stop vscode's linter showing an error), but I'll remove them

Copy link
Owner

Choose a reason for hiding this comment

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

They don't normally show errors for me. Which linter are you using?

colour=hikari.Colour(0x563275),
# Doing it like this is important.
timestamp=dt.datetime.now().astimezone(),
)
.set_author(name="Information")
.set_author(name=str(self.target))
Copy link
Owner

Choose a reason for hiding this comment

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

Should this not use the display_name attribute?

Copy link
Author

Choose a reason for hiding this comment

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

It'll show the user's username and discriminator, but I can use the display_name property if you'd like

Copy link
Owner

Choose a reason for hiding this comment

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

Ah okay. In that case it's fine as is.

roles = (await self.target.fetch_roles())[1:] # All but @everyone.

# Create a list of roles, formatting them into role mentions.
# The @everyone role's ID is the guild's ID, which we filter out.
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
# The @everyone role's ID is the guild's ID, which we filter out.
# The @everyone role's ID is the guild's ID, which we filter out.


# Function calls can be chained when creating embeds.
embed = (
hikari.Embed(
title="User information",
description=f"ID: {self.target.id}",
description=f"ID: `{self.target.id}`",
Copy link
Owner

Choose a reason for hiding this comment

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

Imo, using backticks around Discord IDs is non-standard.

Copy link
Author

Choose a reason for hiding this comment

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

I always use them, but I'll remove them

Comment on lines +9 to +14
INTENTS = (
Intents.ALL_MESSAGES
| Intents.MESSAGE_CONTENT
| Intents.GUILD_MEMBERS
| Intents.GUILDS
)
Copy link
Owner

Choose a reason for hiding this comment

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

Why are these intents different from the ones in the Crescent bot?

Copy link
Author

Choose a reason for hiding this comment

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

Because for prefix commands to work, ALL_MESSAGES and MESSAGE_CONTENT was necessary, but since I'll be removing prefix commands I'll remove those intents too

Copy link
Owner

Choose a reason for hiding this comment

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

Cool, sounds good.

Comment on lines +3 to +4
hikari-lightbulb~=2.2.0
hikari-tanjun~=2.9.0
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
hikari-lightbulb~=2.2.0
hikari-tanjun~=2.9.0
hikari-lightbulb>=2.2.0,<3.0
hikari-tanjun>=2.9.0,<3.0

It may be worth opening these constraints up to accept future updates like Crescent does.

Copy link
Author

Choose a reason for hiding this comment

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

Smort, ty

@novanai
Copy link
Author

novanai commented Nov 16, 2022

What are your current thoughts on adding prefix command stuff?

I think it might make the guide a bit too cluttered, so I was going to stick with slash commands only after some consideration

@parafoxia
Copy link
Owner

What are your current thoughts on adding prefix command stuff?

I think it might make the guide a bit too cluttered, so I was going to stick with slash commands only after some consideration

Okay cool, that sounds good to me.

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.

2 participants