Skip to content
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: Add Guild Member Cover & Accent Color support #1117

Merged
merged 9 commits into from Mar 2, 2022

Conversation

42atomys
Copy link
Contributor

I will add Guild Member Cover & Accent Color support to the library

Resolve #1093

@42atomys 42atomys marked this pull request as draft February 27, 2022 17:23
@42atomys
Copy link
Contributor Author

Tested manually : works with static and animated user banner.

@FedorLap2006 Ready for review

@42atomys 42atomys marked this pull request as ready for review February 27, 2022 17:47
Copy link
Collaborator

@FedorLap2006 FedorLap2006 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall PR looks good to me, except a couple of things. Additionally, I see inconsistent naming of cover and banner. Please fix that.

user.go Outdated Show resolved Hide resolved
user.go Outdated Show resolved Hide resolved
user.go Outdated Show resolved Hide resolved
42atomys and others added 4 commits February 28, 2022 17:16
Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
@FedorLap2006 FedorLap2006 added the feature Feature implementation label Feb 28, 2022
util.go Outdated Show resolved Hide resolved
user.go Outdated
// size: The size of the desired banner image as a power of two
// Image size can be any power of two between 16 and 4096.
func (u *User) BannerURL(size string) string {
// The default/empty avatar case should be handled by the above condition
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this belongs to Member.BannerURL

structs.go Outdated
// BannerURL returns the URL of the member's banner image.
// size: The size of the desired banner image as a power of two
// Image size can be any power of two between 16 and 4096.
func (m *Member) BannerURL(size string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this would be necessary, since Member doesn't have Banner property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actually implemented on Discord API v9 but custom banner per server is actually a feature of the Discord client (Available on Edit Server Profile). In Discord Client app, the app fallback to the user banner if the field banner of Member is null.

I just added the fallback strategy now and wait the banner field to be released

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to just implement it when it goes out. DiscordGo is 1:1 mapping, so if API doesn't have it, we don't as well.

Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
@FedorLap2006
Copy link
Collaborator

I'll give it a look tomorrow

@FedorLap2006 FedorLap2006 added this to the v0.24.0 milestone Mar 1, 2022
@FedorLap2006
Copy link
Collaborator

FedorLap2006 commented Mar 2, 2022

Thank you for your contribution! Due to the fact that v0.24.0 is planned for today, I decided to make a couple of changes which I proposed to the PR myself.

I think that would be best to open a new PR and implement Member.BannerURL when it's going to be shipped. If we would have included it now, users might get confused on why it doesn't return the customized url, without having it however, there won't be any confusion.

@FedorLap2006 FedorLap2006 merged commit df7555c into bwmarrin:master Mar 2, 2022
@42atomys 42atomys deleted the feat/member_banner_support branch March 2, 2022 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding User Banners?
2 participants