Skip to content

Fixed array tooltips, removed \n from lang#47

Open
JackOfNoneTrades wants to merge 4 commits intoCarbon-Config-Project:1.7.10from
JackOfNoneTrades:1.7.10
Open

Fixed array tooltips, removed \n from lang#47
JackOfNoneTrades wants to merge 4 commits intoCarbon-Config-Project:1.7.10from
JackOfNoneTrades:1.7.10

Conversation

@JackOfNoneTrades
Copy link

Title says it all. The \n removal is because yes no guis only support a title and one line of text. I thought about subclassing it and adding support for an arbitrary amount of lines (+ custom background), but it would require more effort + approval.

I am aware that this instanceof ArrayElement && ((ArrayElement) this).node != null might not be the most idiomatic way for the array tooltip fix, if I should change it please tell.

@Speiger
Copy link
Contributor

Speiger commented May 2, 2025

@JackOfNoneTrades could you please show the difference in both?

@JackOfNoneTrades
Copy link
Author

image
vs
image

Otherwise for the array, without the fix no tooltip appears when the entry name is hovered.

@Speiger
Copy link
Contributor

Speiger commented May 2, 2025

@JackOfNoneTrades yeah i would rather fix the underlying issue instead of removing new lines.

@JackOfNoneTrades
Copy link
Author

image
image
Ok I think I managed to add support for \n newlines in a non invasive way, by subclassing GuiYesNo.

if (code == 'r') {
active.setLength(0);
resetFound = true;
} else if ("0123456789abcdefklmnor".indexOf(code) != -1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please make that a char >= '0' && char <= 'r' instead of an indexOf

Copy link
Author

@JackOfNoneTrades JackOfNoneTrades May 4, 2025

Choose a reason for hiding this comment

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

hmm I want to know if the char is a valid code, 0123456789abcdefklmnor are three separate ranges put together. do I do three range checks?

Copy link
Contributor

Choose a reason for hiding this comment

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

I know you stroke this through, but 3 checks would be still faster than a indexOf which is a brute force iteration.

active.setLength(0);
resetFound = true;
} else if ("0123456789abcdefklmnor".indexOf(code) != -1) {
if (!resetFound && active.indexOf("§" + code) == -1) {
Copy link
Contributor

@Speiger Speiger May 4, 2025

Choose a reason for hiding this comment

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

Please track this separatly. Instead of doing indexOf

for (int i = 0; i < rawLines.length; ++i) {
String line = rawLines[i];

if (!line.matches("^§[0-9a-frk-orA-FK-OR].*")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Author

Choose a reason for hiding this comment

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

so when a style is set, for example here:
new ChatComponentTranslation("gui.carbonconfig.warn.changed").getFormattedText(), new ChatComponentTranslation("gui.carbonconfig.warn.changed.desc").setChatStyle(new ChatStyle().setColor(EnumChatFormatting.GRAY)).getFormattedText(), 0));
if the formatted line is split, minecrafts style syntax doesn't carry to remaining lines. It needs to be carried, plus needs to work if any amount of styles is applied to the text.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah ok, makes sense, but i would add a check before hand if the format isn't "empty" because why check for missing formatting if no formatting would be applied anyways.

public void drawScreen(int mouseX, int mouseY, float partialTicks) {
super.drawScreen(mouseX, mouseY, partialTicks);

String[] rawLines = this.multiLineMessage.split("\\\\n");
Copy link
Contributor

@Speiger Speiger May 4, 2025

Choose a reason for hiding this comment

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

why is that using \\\\?

Copy link
Author

@JackOfNoneTrades JackOfNoneTrades May 4, 2025

Choose a reason for hiding this comment

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

It's what works. I tried evaluating different expressions under the debugger and "\n", "\n" etc don't work 🤷. Must be something with regex

String line = lines[i];

if (!line.matches("^§[0-9a-frk-orA-FK-OR].*")) {
line = activeFormatting + line;
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be applied here. Since the listFormattedStringToWidth would destroy this anyways again.
You can keep track of the flag but other than that this is to early to apply

@Speiger
Copy link
Contributor

Speiger commented May 4, 2025

@JackOfNoneTrades i would suggest you make this a helper function that inserts a input string and converts multiple lines out of it as a output, that way we have consistent implementations between both usecases.
You can throw in the fontrenderer as well and a max width to ensure max sizes.
(Also you might want to cache the output like the multichoice screen that way we have to generate this info only once)

Overall thank you :)

@JackOfNoneTrades
Copy link
Author

JackOfNoneTrades commented May 4, 2025

image
I finally looked up what the font renderer does to split formatted strings and adapted the listFormattedStringToWidth function. In the screenshot I added a series of A's to the lang to test width splitting.

int lineHeight = this.fontRendererObj.FONT_HEIGHT + 2;
int startY = 90;
String activeFormatting = "";
String[] messages = FormattingUtil.listFormattedStringToWidthRespectingNewlines(this.fontRendererObj, message, this.width - 50);
Copy link
Contributor

Choose a reason for hiding this comment

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

throw that line into the init function, font renderer and width are accessible once the init is called.

Copy link
Author

Choose a reason for hiding this comment

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

I tried, width is 0

@Speiger
Copy link
Contributor

Speiger commented May 4, 2025

i will integrate this pr next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants