-
Notifications
You must be signed in to change notification settings - Fork 186
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
feat: allow editing for guild role icons #1558
feat: allow editing for guild role icons #1558
Conversation
Signed-off-by: LordOfPolls <[email protected]>
I don't want to give a review without understanding how this works. Are you able to provide valid client-facing code with your proposal for me? |
already had the functions I added to misc_utils. Undid those changes and tidied up the code in role.py to reflect the already existing converter. Added an error raise if both icon and unicode emojis are used in one edit (the api requires only one is provided.)
Here is a snippet of a listener I have that will update a role to match the icon (if one is present) on a role it's tracking. @i0bs I also updated the branch because I found that there was already a method in interactions.client.utils.serializer that did exactly what I added to misc_utils. Let me know if the snippet below will suffice. async def color_check(self, user):
roles = user.roles # remove @everyone
# highest order of the colour is the one that gets rendered.
# if the highest is the default colour then the next one with a colour
# is chosen instead
role_color = None
# print(user.top_role.color.value)
for role in reversed(roles):
if role.color.value:
role_color = role.color
role_name = role.name
return role_color, role_name
@listen()
async def on_role_update(self, ctx: RoleUpdate):
if int(ctx.after.guild.id) != int(self.GuildID):
return
user = await ctx.after.guild.fetch_member(self.UserID)
roles = user.roles
if int(ctx.after.id) not in roles:
return
try:
self.bot.logger.info(f"Updating Role color to match updated user roles")
my_role = await ctx.after.guild.fetch_role(self.RoleID)
role_color, role_name = await self.color_check(user)
role_icon = None
try:
for role in user.roles:
role = await ctx.guild.fetch_role(role.id)
if role.icon:
role_icon = await role.icon.fetch()
break
except:
role_icon = None
if not role_icon:
await my_role.edit(name=role_name, color=role_color)
else:
await my_role.edit(name=role_name, color=role_color, icon=role_icon)
|
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.
Please revert type signatures for icon
and unicode_emoji
within the .edit()
docstring. This is currently inconsistent with the way we already document argument signatures for methods.
Please rewrite both the documentation for unicode_emoji
to be clearer, simpler and shorter in synopsis. Better choice of verbiage and structure lets us keep things streamlined and straight to the point.
cleaned up the docstring to better align with requirements, moved the icon serialization so the payload's code is a bit easier to follow at a glance.
How do the changes in the latest commit look |
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.
Other than an overly narrow type hint, looks good 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.
lgtm
Pull Request Type
Description
I have implemented handling into the role.edit method to allow for guild.icon updating, as well as adding the required converter methods into the utils.misc_utils.
Changes
Related Issues
Test Scenarios
Python Compatibility
3.10.x
3.11.x
Checklist
pre-commit
code linter over all edited files