NeoForge icon indicating copy to clipboard operation
NeoForge copied to clipboard

[1.21] Overhaul output of tps command to be more colorful

Open ApexModder opened this issue 1 year ago • 4 comments

The /neoforge tps <dim> command output has been overhauled to be much more colorful, making it easier to read and understand at a glance.

Both the TPS and Tick Time components now have a “hover event” to better explain the values. Additionally, the mean TPS fades from green to red, making it quicker and easier to identify laggy dimensions.

Before

image

After

image

ApexModder avatar Aug 05 '24 19:08 ApexModder

  • [x] Publish PR to GitHub Packages

Last commit published: c2f464d7f7659ebf131a2fc30ad813fccd74685b.

PR Publishing

The artifacts published by this PR:

Repository Declaration

In order to use the artifacts published by the PR, add the following repository to your buildscript:

repositories {
    maven {
        name 'Maven for PR #1422' // https://github.com/neoforged/NeoForge/pull/1422
        url 'https://prmaven.neoforged.net/NeoForge/pr1422'
        content {
            includeModule('net.neoforged', 'neoforge')
            includeModule('net.neoforged', 'testframework')
        }
    }
}

MDK installation

In order to setup a MDK using the latest PR version, run the following commands in a terminal.
The script works on both *nix and Windows as long as you have the JDK bin folder on the path.
The script will clone the MDK in a folder named NeoForge-pr1422.
On Powershell you will need to remove the -L flag from the curl invocation.

mkdir NeoForge-pr1422
cd NeoForge-pr1422
curl -L https://prmaven.neoforged.net/NeoForge/pr1422/net/neoforged/neoforge/21.1.42-pr-1422-pr-tps-command/mdk-pr1422.zip -o mdk.zip
jar xf mdk.zip
rm mdk.zip || del mdk.zip

To test a production environment, you can download the installer from here.

Please don't drop the "ms" unit.

Please revert those commas back to periods or use lowercase after them. ", Mean" is just mean ;)

Also, I would suggest using ResourceLocation.toLanguageKey() instead of the raw RL (and putting the raw RL into the hover text). That way, it will translate automatically if we ever manage to put those language keys in for vanilla dimensions.

The "Dimension:" prefix seems unnecessary to me. I would drop it to avoid line breaking.

HenryLoenwind avatar Aug 05 '24 22:08 HenryLoenwind

Added back the ms unit; I didn’t actually intend on removing it.

Dropped the dimension prefix and updated to auto-translate if a matching translation key exists (defaults to the raw registry name if missing): dimension.<namespace>.<path>

I have included translations for the vanilla dimensions, but modders will have to include their own.

The raw dimension registry name has also been added as a hover tooltip when hovering over the dimension component.

Below is an example of these changes. Note how the Nether is minecraft:the_nether; this is because no dimension.minecraft.the_nether translation exists.

Example

image

ApexModder avatar Aug 06 '24 12:08 ApexModder

Locked behind #1428, Will need updating if that PR goes through

ApexModder avatar Aug 06 '24 14:08 ApexModder

@ApexModder, this pull request has conflicts, please resolve them for this PR to move forward.

I think it's better if the colours are part of the translation and use vanilla's weird character instead of being hard-coded, as it would be more customizable, especially for people that have a hard time with the built in colours.

Matyrobbrt avatar Aug 23 '24 05:08 Matyrobbrt

I think it's better if the colours are part of the translation and use vanilla's weird character instead of being hard-coded, as it would be more customizable, especially for people that have a hard time with the built in colours.

While this would work for the coloring, how would I go about applying the hover tooltips that way? afaik it is not possible to apply hover effects other than code. But I do agree having the colors being configurable in some way would be better (even if its just the translation defining the colors).

ApexModder avatar Aug 23 '24 07:08 ApexModder

Just a side note: The json-array format probably can do hover, but it should not be used as it falls apart on Crowdin. The SGML-format could be extended to support hover and even parameterised hovers; I just never thought about this use case.

HenryLoenwind avatar Aug 23 '24 14:08 HenryLoenwind

afaik it is not possible to apply hover effects other than code.

That's correct, that much can stay code, but the colouring should still be part of the lang entry.

Matyrobbrt avatar Aug 23 '24 17:08 Matyrobbrt

🚀 This PR has been released as NeoForge version 21.1.32.

neoforged-releases[bot] avatar Aug 31 '24 16:08 neoforged-releases[bot]