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: shadow colors #1124

Merged
merged 15 commits into from
Dec 15, 2024
Merged

feat: shadow colors #1124

merged 15 commits into from
Dec 15, 2024

Conversation

kashike
Copy link
Member

@kashike kashike commented Nov 1, 2024

No description provided.

@kashike kashike linked an issue Nov 1, 2024 that may be closed by this pull request
@MiniDigger MiniDigger linked an issue Nov 1, 2024 that may be closed by this pull request
@kezz
Copy link
Member

kezz commented Nov 1, 2024

We need a concept of "no shadow" that exists outside of creating a shadow colour with a magic number value. I'm also not sure I like the ShadowColor object existing as it does, I think I'd prefer it to be an interface implemented by either a singleton "none" object or a textcolor + alpha value. As we need some sort of ARGBLike interface (most platforms already have a colour + alpha object), I think we should try and ensure we don't duplicate API here.

Ideally I think the API would allow for all of the following:

style.shadowColor(ShadowColor.NONE);
style.shadowColor(ShadowColor.shadowColor(NamedTextColor.RED, 0.4);
style.shadowColor(NamedTextColor.RED);
style.shadowColor(myBukkitColorObject);
style.shadowColor(ShadowColor.shadowColor(0xRRGGBBAA);

@zml2008 zml2008 added this to the 4.18.0 milestone Dec 1, 2024
@zml2008
Copy link
Member

zml2008 commented Dec 6, 2024

this will need serializer support at some point too

@zml2008
Copy link
Member

zml2008 commented Dec 7, 2024

another thing we should add: a <!shadow> MM tag to disable shadows

@kashike kashike force-pushed the feature/shadow-color branch from 00fed47 to de37bf6 Compare December 8, 2024 03:50
@zml2008 zml2008 requested review from MiniDigger and kezz December 8, 2024 22:08
@zml2008
Copy link
Member

zml2008 commented Dec 8, 2024

I think this has everything on my list now

@zml2008 zml2008 linked an issue Dec 10, 2024 that may be closed by this pull request
kezz
kezz previously requested changes Dec 10, 2024
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.

Minor changes, otherwise this looks fab

@zml2008 zml2008 requested a review from kezz December 14, 2024 06:19
@zml2008 zml2008 self-assigned this Dec 14, 2024
@zml2008 zml2008 dismissed kezz’s stale review December 15, 2024 17:22

changes made

@zml2008 zml2008 added this pull request to the merge queue Dec 15, 2024
Merged via the queue into main/4 with commit bec02ef Dec 15, 2024
5 checks passed
@zml2008 zml2008 deleted the feature/shadow-color branch December 15, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for shadow_color to MiniMessage Add support for shadow_color to Style New text shadow support
4 participants