-
Notifications
You must be signed in to change notification settings - Fork 802
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
adds default avatar generation to AvatarURL method #488
Conversation
user.go
Outdated
@@ -34,7 +35,10 @@ func (u *User) Mention() string { | |||
// be added to the URL. | |||
func (u *User) AvatarURL(size string) string { | |||
var URL string | |||
if strings.HasPrefix(u.Avatar, "a_") { | |||
if u.Avatar == "" { | |||
discriminatorInt, _ := strconv.Atoi(u.Discriminator) |
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.
Instead of throwing away this error, just use Discriminator as a string in endpoints.go
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.
Wasn't sure how much logic I should put into endpoints.go, I will change it 👍
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.
Endpoints shouldn't need any real logic, if you just use strings (it's kinda gross, but we don't use discriminators as ints anywhere else in the codebase)
endpoints.go
Outdated
return EndpointCDN + "embed/avatars/" + strconv.Itoa(uDiscriminator%5) + ".png" | ||
EndpointDefaultUserAvatar = func(uDiscriminator string) string { | ||
uDiscriminatorInt, _ := strconv.Atoi(uDiscriminator) | ||
return EndpointCDN + "embed/avatars/" + strconv.Itoa(uDiscriminatorInt%5) + ".png" |
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.
Oh you need the int for modulus, guh.
I regret asking you for the previous change now, sorry!
But now that we need to do the Atoi, can you check the error in user.go and return "" if it fails?
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.
if it fails it returns 0, the final url will be https://cdn.discordapp.com/embed/avatars/0.png, which is a valid url. I think that might be better to return instead of just ""?
EndpointUserNotes = func(uID string) string { return EndpointUsers + "@me/notes/" + uID } | ||
EndpointDefaultUserAvatar = func(uDiscriminator string) string { | ||
uDiscriminatorInt, _ := strconv.Atoi(uDiscriminator) | ||
return EndpointCDN + "embed/avatars/" + strconv.Itoa(uDiscriminatorInt%5) + ".png" |
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.
Are you missing the uID here somewhere? embed/avatars/1682.png doesn't seem right for my avatar?
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.
It's the default avatar url, which is used if you don't have a custom avatar set. And it's discriminator modulo 5, not just the discriminator in the url. 1682 modulo 5 = 2: Your default avatar is https://cdn.discordapp.com/embed/avatars/2.png .
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.
* adds default avatar generation to AvatarURL method * converts discriminator string to integer in endpoints.go
AvatarURL will return a link to the default avatars instead of an invalid link if the user has no avatar set.
As described here: https://discordapp.com/developers/docs/reference#image-formatting-cdn-endpoints