hikari icon indicating copy to clipboard operation
hikari copied to clipboard

global name handling bug fixes

Open FasterSpeeding opened this issue 1 year ago • 11 comments

Summary

Checklist

  • [ ] I have run nox and all the pipelines have passed.
  • [ ] I have made unittests according to the code I have added/modified/deleted.

Related issues

FasterSpeeding avatar Jun 17 '23 15:06 FasterSpeeding

I disagree with these changes. IMO str(user) should return an user-readable unique ID of the user – the username (previously username + discriminator instead of just username if it was supposed to be the display name).

display_name could be introduced to hikari.User instead of modifying str(user).

Le0Developer avatar Jun 17 '23 15:06 Le0Developer

I would think just their actual unique username would be ideal. if I want their id I'll just get their id from the object.

Jonxslays avatar Jun 17 '23 15:06 Jonxslays

I would think just their actual unique username would be ideal. if I want their id I'll just get their id from the object.

Which is exactly what I meant with "user-readable", sorry for the confusion. Basically I dont want the behavior of str(user) to change.

Le0Developer avatar Jun 17 '23 15:06 Le0Developer

I disagree with these changes. IMO str(user) should return an user-readable unique ID of the user – the username (previously username + discriminator instead of just username if it was supposed to be the display name).

display_name could be introduced to hikari.User instead of modifying str(user).

str(user) isn't really intended as a way to get a unique identifier like that (at the very least that was never like guaranteed/documented behaviour), it's just a way to get a chat friendly display name

On the new system you can just use user.username or user.id to get a globally unique identifier for a user

FasterSpeeding avatar Jun 27 '23 01:06 FasterSpeeding

str(user) isn't really intended as a way to get a unique identifier like that (at the very least that was never like guaranteed/documented behaviour), it's just a way to get a chat friendly display name

I don't believe that the discriminator should be part of "chat friendly display name[s]".

On the new system you can just use user.username or user.id to get a globally unique identifier for a user

It's not fully rolled out yet, and won't for bots for the near future.


I also don't understand how this situation is any different from why Member.display_name exists. This is basically fulfilling the exact same purpose as Member.display_name but on the User level, so why should it be in the __str__ instead of display_name?

Le0Developer avatar Jun 27 '23 13:06 Le0Developer

I agree with @Le0Developer here. Changing this will basically have __str__ function like display_name, at which point why not use just that.

davfsa avatar Jun 29 '23 07:06 davfsa

str(user) wasn't ever intended to be like a unique identifier for the user though (which also didn't even really work due to cases like webhook users having overlapping usernames and also partial user's fallback giving it 2 possible values). And according to what leo's wants it'd just be identical to user.username in the future (but also like users can just change their username and/or discriminator so like their ID would be a way better unique identifier).

It wasn't ever really meant to be a unique identifier (nor was that documented), it was just a chat friendly way to display the user.

FasterSpeeding avatar Jun 29 '23 08:06 FasterSpeeding

Changing this will basically have str function like display_name, at which point why not use just that.

That doesn't exist on User/PartialUser, that's what str(user) was for

FasterSpeeding avatar Jun 29 '23 08:06 FasterSpeeding

By that logic, why does Member.display_name exist? Shouldn't Member.__str__ be used instead?

it was just a chat friendly way to display the user.

Why did it include the discriminator then?

Le0Developer avatar Jun 29 '23 09:06 Le0Developer

By that logic, why does Member.display_name exist?

Member.display_name is actually documented (and thus guaranteed to behave a certain way) unlike str(member)

FasterSpeeding avatar Jun 29 '23 10:06 FasterSpeeding

Also just a thought but .username isn't even always going to be chat friendly on the new system (that's what global_name is for after all) mostly due to the very heavy restrictions around it

The cases I'm thinking of are non-Latin char language speakers (where their global name will likely be in their own lang) as well as mushed together words with no spacing or capitalisation to make it easy to read.

FasterSpeeding avatar Jun 29 '23 13:06 FasterSpeeding