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

Reduce compact wool scoreboard component length #1084

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

Pugzy
Copy link
Contributor

@Pugzy Pugzy commented Oct 17, 2022

The recent changes migrating the scoreboard to use components has an unintended side effect of increasing the character count of the lines due to the use of space(). When these components are translated back to legacy format the extra spaces add unnecessary resets to the string. This causes the line to be sliced in order to be compact enough for scoreboard display causing wools to be cut off where they had previously not.

An example of this is below.
§1⬜§r §3⬜§r §2⬜§r §a⬜§r §6⬜§r §e⬜

Each set of spaces causes the §r characters to be added after each wool which increases its final display length.

This change appends the spaces in a way that does not require the resets and produces the below.
§1 ⬜§2 ⬜§6 ⬜§c ⬜§5 ⬜§4 ⬜§7 ⬜ which is 10 characters shorter.

Examples of compact scoreboards with many wools.

image

This commit also removes an extra space that was added at the start of the wool line which caused it to start with two spaces rather than the previous one. Looking back at the commits prior to this change and old OCN videos one space seems to be correct.

If there's a better way to do this let me know.

@Pugzy
Copy link
Contributor Author

Pugzy commented Oct 17, 2022

@KingOfSquares thoughts?

If there's a way to keep the spaces after the wool box I think that would be nicer but to the end user they both look the same.

@Electroid Electroid self-requested a review October 17, 2022 18:00
Comment on lines +487 to +489
TextComponent spacer = space();
if (!firstWool && !horizontalCompact) {
woolText.append(space()).append(space());
spacer = spacer.append(space()).append(space());
Copy link
Contributor

@KingOfSquares KingOfSquares Oct 19, 2022

Choose a reason for hiding this comment

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

Maybe weird suggestion but since the sidebar stuff might get rendered a lot maybe we can create the second spacer statically? space() itself does not create any objects, but the triple space could be created once.

Component spacer = (!firstWool && !horizontalCompact) ? TRIPLE_SPACE : space()

You could go a step further and just check for this when you are appending the rendered text(To not create the reference at all), but I think that will be harder to read than you gain in performance...

@Electroid Electroid merged commit aa4cd1d into PGMDev:dev Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants