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.

Sign up for GitHub

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

Jump to bottom

feat(api): #453 Introduce Font #648

Open
wants to merge 1 commit into
base: main/4
Choose a base branch
from
Open

feat(api): #453 Introduce Font #648

wants to merge 1 commit into from

Conversation

kashike
Copy link
Member

@kashike kashike commented Dec 19, 2021

No description provided.

@kashike kashike added this to the 4.10.0 milestone Dec 19, 2021
@kashike kashike linked an issue Dec 19, 2021 that may be closed by this pull request
Create a Font interface for use as a StyleBuilderApplicable #453
Open
@KingOfSquares
Copy link
Contributor

Maybe expand a little more than just Font. in the Java doc? Something that distinguishes between the representation of the Minecraft font(the class) and its resource location(the key) could be something to expand on. I know the Key is regarded as the font all the other places in the api, so maybe just something about the class not being the same as the contained Key…?

@KingOfSquares
Copy link
Contributor

KingOfSquares commented Dec 19, 2021 •
edited
Loading

Lightning speed 😎👍

zml2008
zml2008 previously requested changes Dec 19, 2021
View reviewed changes
Copy link
Member

@zml2008 zml2008 left a comment

Choose a reason for hiding this comment

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

Maybe add an overload of StyleSetter.font that accepts instances of Font?

@kezz kezz self-assigned this Dec 21, 2021
kezz
kezz requested changes Dec 21, 2021
View reviewed changes
Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

Actually, the ambiguous overload is a bit of a deal breaker for this - we don't want to force developers to change code/recompile if they are removing a font.

@kezz kezz removed their assignment Dec 21, 2021
@zml2008 zml2008 removed this from the 4.10.0 milestone Feb 4, 2022
@zml2008
Copy link
Member

zml2008 commented Feb 14, 2022

hmm, how awful would it be to bring this in with just Font, and not the added method on StyleSetter?

@zml2008 zml2008 added the status: needs discussion An issue or PR that requires design decisions or further consensus building before it can be merged label Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kezz kezz kezz requested changes

@lucko lucko lucko approved these changes

@zml2008 zml2008 zml2008 left review comments

Assignees
No one assigned
Labels
area: api status: needs discussion An issue or PR that requires design decisions or further consensus building before it can be merged type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a Font interface for use as a StyleBuilderApplicable
5 participants
@kashike @KingOfSquares @zml2008 @kezz @lucko