-
Notifications
You must be signed in to change notification settings - Fork 140
feat: support new username system #1025
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
Conversation
727be2f
to
16b0ee7
Compare
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.
quick review, documentation looks fine overall c:
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.
Thoughts on a possible field or helper for determining if a user is on the new system without checking the deprecated discriminator field? Maybe we don't deprecate the discriminator field right now?
This should be ready to go now, I've done a bunch more testing. Since the introduction of |
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.
looks good to me
The user's global display name, if set. | ||
This takes precedence over :attr:`.name` when shown. | ||
|
||
For bots, this is the application 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.
just triple checked and this is currently not true 🦀
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.
yeah :/ It was announced back then, which is why I originally included it in this PR. It'll be correct eventually :p
source
edit: welp.
Summary
Initial implementation of the recently announced username system changes. See discord/discord-api-docs#6130.
The changelog entry should give a pretty solid overview over the specific changes.
Discriminator handling in converters and
Guild.get_member_named
is kept for now, it's planned to be removed once the username migration is complete.There are a couple open issues and things that need to be discussed further:
TBD
discriminator=
field be removed from__repr__
s?~ decided to not remove it, for now
User.__str__
returnname
or@name
?c7f05b0
user#0
in converters +get_member_named
for migrated users?df79046 + c8d8d8d
discriminator
field may disappear in future API versions; we may want to add a"0"
fallback for this already for future proofing, even if it should (ahem) be bound to specific API versions~ also not doing this for now, see feat: support new username system #1025 (comment)
username
already accounts for the three different names a user can haveChecklist
pdm lint
pdm pyright