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

Implemented banners and banner patterns #588

Merged
merged 8 commits into from
Aug 1, 2022
Merged

Implemented banners and banner patterns #588

merged 8 commits into from
Aug 1, 2022

Conversation

JustTalDevelops
Copy link
Member

No description provided.

@JustTalDevelops JustTalDevelops added the feature New feature or request label Jul 23, 2022
server/block/banner_pattern_type_register.go Outdated Show resolved Hide resolved
server/block/banner_pattern_type_register.go Outdated Show resolved Hide resolved
server/block/banner_pattern_type_register.go Show resolved Hide resolved
Copy link
Contributor

@abimek abimek left a comment

Choose a reason for hiding this comment

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

thing is gives more info, dk though, feel free to decline

server/block/banner_pattern_type.go Show resolved Hide resolved
@DaPigGuy DaPigGuy mentioned this pull request Jul 27, 2022
89 tasks
Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

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

Got two comments. Should be easily resolved.

type BannerPattern struct {
// Pattern represents the type of banner pattern. These types do not include all patterns that can be applied to a
// banner.
Pattern BannerPatternType
Copy link
Member

Choose a reason for hiding this comment

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

Would name this field Type probably.


// BannerPatternLayer is a wrapper over BannerPatternType with a colour property.
type BannerPatternLayer struct {
BannerPatternType
Copy link
Member

Choose a reason for hiding this comment

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

Would probably not embed BannerPatternType in here, but have a Type name.

Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

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

This looks alright to me. Thanks for the PR!

@Sandertv Sandertv merged commit 5e4704e into master Aug 1, 2022
@Sandertv Sandertv deleted the feature/banners branch August 1, 2022 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants