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

HUD Text API #122

Open
wants to merge 9 commits into
base: 2.0-alpha.3
Choose a base branch
from
Open

Conversation

zr0n1
Copy link

@zr0n1 zr0n1 commented Jul 15, 2024

A simple API for adding (debug) text to the game HUD.

@EventListener
private static void renderHudText(HudTextRenderEvent event) {
    // The position will change based on whether the debug HUD is enabled
    event.left.add(new HudTextLine("This text displays regardless of whether the debug HUD is enabled!"));
    if (event.debug) {
        event.left.add(new HudTextLine("This text only displays on the debug HUD!", 0xE0E0E0));
    } else {
        // This will only succeed if the priority exceeds other mods with version text or other mods' version texts are disabled
        event.setVersion(new HudTextLine("StationAPI v2.0-alpha.3"));
        Collections.addAll(event.right,
                new HudTextLine("Hello " + event.minecraft.player.name + "!"),
                new HudTextLine("This line is doubly offset from the last. Perchance.", 0xFFFFFF, 20)
        );
    }
}

2024-07-15_02 02 29
2024-07-15_02 02 32

Copy link
Member

@calmilamsy calmilamsy left a comment

Choose a reason for hiding this comment

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

Mostly surface-level concerns. The implementation is otherwise very good, and I'm more than happy to merge this once my issues are addressed.

Copy link
Member

@calmilamsy calmilamsy left a comment

Choose a reason for hiding this comment

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

Last concern.

Copy link
Author

@zr0n1 zr0n1 left a comment

Choose a reason for hiding this comment

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

done

@zr0n1 zr0n1 requested a review from calmilamsy July 15, 2024 18:23
@mineLdiver mineLdiver added this to the 2.0-alpha.3 milestone Jul 16, 2024
@mineLdiver mineLdiver added the enhancement New feature or request label Jul 16, 2024
Copy link
Member

@mineLdiver mineLdiver left a comment

Choose a reason for hiding this comment

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

Surface level review, but I have some big concerns:

  1. HudTextLine being instantiated dozens of times each frame.
  2. Vanilla debug renderer being completely skipped.

In my opinion, HudTextLine should instead contain a template string and an array of callbacks for formatting, so that it can be instantiated once and always be up-to-date.

For example: Template "Used memory: %d\% (%dMB) of %dMB", the array of callbacks being:

var callbacks = new Supplier[] {
  () -> (Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory()) * 100L / Runtime.getRuntime().maxMemory(),
  () -> (Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory()) / 1024L / 1024L,
  () -> Runtime.getRuntime().maxMemory() / 1024L / 1024L
}
This is just a working example, this could be optimized by using a single class with its methods registered as callbacks, so the calculated values could be shared.

Making HudTextLine use callbacks also enables the debug HUD to just be built once instead of each frame.

As for the vanilla lines being completely canceled, I feel like a more surgical approach must be taken where there's a way to cancel each line (maybe by using something like EnumSet<VanillaHudLine> canceled?) and then overwrite the freed space while building the debug HUD. StAPI shouldn't have to render Minecraft's debug if it's unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants