-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update for RGB #17
Update for RGB #17
Conversation
} | ||
for (char color : ansiCodes.keySet()) { | ||
String cString = COLOR_CHAR + String.valueOf(color); | ||
String escCode = ansiCodes.get(color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be changed with looping ansiCodes.entrySet instead.
public void testRgbColor() { | ||
assertEquals("World",format("§x§F§F§0§0§5§5World",false)); | ||
assertEquals("\u001B[38;2;255;0;85mWorld",format("§x§F§F§0§0§5§5World",true)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty line
I'm not a fan of adding support for spigot's non-standard format directly to TCA -- a TCA PR should really just remove TCA's dependence on MC's legacy text formatting. |
What if Paper forks TCA |
I'd really rather not -- a number of projects use TCA -- forge for one -- so any improvements made should really be designed to benefit them as well. |
I agree with @zml2008. Unless other projects like Sponge, Forge, ... agree on the same convention I would rather not support the non-standard Spigot format here. Actually, the only reason If the code can no longer be shared in a reasonable way it's really better to just put special converters into the projects itself - there is really not much of a difference because it's so easy to hook into Log4j2 with the annotations. |
I'm inclined to ask for this, 'lest I go break the existing log4j configs people are using to add a bukkit RGB aware impl of this class |
For the reasons already mentioned above (and also mentioned in #18) I will close this. However, @Narimm thanks a lot for making the PR! I believe something like your code made it into Paper as part of PaperMC/Paper#5205. As inspiration for all others who also want to add RGB console color to their platform, I've also documented a potential quite different approach in #18 that would avoid legacy color codes entirely. I think this would be the ideal solution but I don't really have time at the moment to investigate it myself. |
Updates to support rgb