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

Add Hex Color Support #3098

Merged
merged 5 commits into from
Jul 7, 2020
Merged

Add Hex Color Support #3098

merged 5 commits into from
Jul 7, 2020

Conversation

APickledWalrus
Copy link
Member

@APickledWalrus APickledWalrus commented Jul 4, 2020

Description

This is a remake of #3078, which I had to close because the conflicts had gotten too complicated for me to fix.
This PR adds hex color support to Skript through a new "color" tag. I have updated the documentation appropriately. Unlike #3078, the tag should work everywhere now. I had to make some changes to EffActionBar for colors to work properly there.

This is kind of outside of this PR, but Skript doesn't allow you to do:
send title "<RED>Hello %player%!"
"RED" must be in lowercase. I'm not sure if this is intentional or not though.
The cause is:

SkriptColor color = SkriptColor.fromName("" + m.group(1));
in that the string isn't converted to lowercase ("RED" doesn't exist in the map, but "red" does).


Target Minecraft Versions: 1.16+
Requirements: 1.16+
Related Issues: Fixes #3106

@APickledWalrus
Copy link
Member Author

APickledWalrus commented Jul 5, 2020

Okay, so I removed the need to format it with "color/hex/c" so you can just write:
send "<#123456>Hey there %player%"

It should be working better now, however it doesn't currently work in tooltips. Not sure if that is something on my end or with Skript/Components

EDIT: Test failed, but it wasn't an issue with this PR.

@FranKusmiruk FranKusmiruk added the feature Pull request adding a new feature. label Jul 6, 2020
@APickledWalrus
Copy link
Member Author

APickledWalrus commented Jul 7, 2020

With that commit, tooltips are now properly colored and the linked issue has been fixed.
This PR should be ready now, although some changes to the new documentation could be made (perhaps use <code> tags?)

FranKusmiruk
FranKusmiruk previously approved these changes Jul 7, 2020
Copy link
Member

@FranKusmiruk FranKusmiruk left a comment

Choose a reason for hiding this comment

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

LGTM but I believe we had an issue by using bungee chat API internally in some Spigot versions that didn't ship with it, are you sure this works all the way down 1.9.4?

src/main/java/ch/njol/skript/util/chat/ChatMessages.java Outdated Show resolved Hide resolved
@APickledWalrus
Copy link
Member Author

APickledWalrus commented Jul 7, 2020

LGTM but I believe we had an issue by using bungee chat API internally in some Spigot versions that didn't ship with it, are you sure this works all the way down 1.9.4?

That will need tested, but I would assume so since EffActionBar uses it. The BungeeConverter class is also used in EffMessage without any sort of version check.

EDIT: Made the change I mentioned above to the documentation and updated the check.

@FranKusmiruk FranKusmiruk added this to the 2.5 milestone Jul 7, 2020
@FranKusmiruk
Copy link
Member

Further testing will be done by users in the future betas, thanks for the contribution.

@Thrasilias
Copy link

Okay, so I removed the need to format it with "color/hex/c" so you can just write:
send "<#123456>Hey there %player%"

It should be working better now, however it doesn't currently work in tooltips. Not sure if that is something on my end or with Skript/Components

EDIT: Test failed, but it wasn't an issue with this PR.

stolen code fom my translator :omegaconcernPanda:

@APickledWalrus
Copy link
Member Author

@Thrasilias wat

@Thrasilias
Copy link

That was sarcasme. I use <#rrggbb> in my own java code :)

@APickledWalrus APickledWalrus deleted the APickledWalrus-feature-morecolors branch September 24, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Pull request adding a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'uncoloured message' doesn't remove §x
3 participants