-
-
Notifications
You must be signed in to change notification settings - Fork 727
Enhanced user command #1105
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
Enhanced user command #1105
Conversation
That looks neat! |
bot/cogs/information.py
Outdated
badges = "" | ||
|
||
for badge, is_set in user.public_flags: | ||
if is_set and (emoji := getattr(constants.Emojis, f"badge_{badge}")): | ||
badges += emoji + " " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neat way to use walrus. But I think you want to specify a default for getattr
. This way can raise.
I'd probably personally make badges
a list & then join the elements on a " "
, but this is fine too.
emoji = "" | ||
if activity.emoji: | ||
if not activity.emoji.id: | ||
emoji += activity.emoji.name + " " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the activity.emoji.id
check here to filter custom emoji? I think it could use an in-line comment. It's a little indirect.
bot/cogs/information.py
Outdated
@staticmethod | ||
def status_to_emoji(status: Status) -> str: | ||
"""Convert a Discord status into the relevant emoji.""" | ||
if status is Status.offline: | ||
return constants.Emojis.status_offline | ||
elif status is Status.dnd: | ||
return constants.Emojis.status_dnd | ||
elif status is Status.idle: | ||
return constants.Emojis.status_idle | ||
else: | ||
return constants.Emojis.status_online |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using a dictionary to define this mapping? This is probably fine, but it feels a little unnatural to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are very nice enhancements. I'm surprised there's no premium or boost info, but that can be saved for another time.
for field_name, field_content in fields: | ||
embed.add_field(name=field_name, value=field_content, inline=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great refactor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG to the M.
Introduces new information into the
!user
command showing user badges & statuses on all different Discord platforms as well as switching our headers out for embed fields.Resolves #1047.