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

Add Hud Render Events #4119

Open
wants to merge 11 commits into
base: 1.21.1
Choose a base branch
from

Conversation

kevinthegreat1
Copy link
Contributor

@kevinthegreat1 kevinthegreat1 commented Sep 25, 2024

Originally discussed here.

Deprecate HudRenderCallback.

Fixes #3908.

@Fuzss
Copy link

Fuzss commented Sep 25, 2024

Not sure this really goes far enough. The system used by NeoForge has always been pretty ideal (although quite invasive): splitting gui rendering into all the different parts (health, hotbar, status effects, etc.) and allowing mods to render their custom gui layers anywhere in-between.

Also it would be awesome to finally get shared height parameters for bars (health bar + armor bar on the left side, food + air on the right side) rendered above the hotbar in Fabric Api. So that multiple mods adding their own bars know at what height to render without interfering with others.

@kevinthegreat1
Copy link
Contributor Author

Not sure this really goes far enough. The system used by NeoForge has always been pretty ideal (although quite invasive): splitting gui rendering into all the different parts (health, hotbar, status effects, etc.) and allowing mods to render their custom gui layers anywhere in-between.

I agree that Forge's implementation (last when I saw it) is much more versatile and comprehensive. It would be great to be able to cancel certain components too. However as you mentioned it is quite invasive, and in the past, this injection point hasn't exactly been the most stable. A more comprehensive api would need a bit more discussion with the maintainers, but I'd be happy to see it.

Also it would be awesome to finally get shared height parameters for bars (health bar + armor bar on the left side, food + air on the right side) rendered above the hotbar in Fabric Api. So that multiple mods adding their own bars know at what height to render without interfering with others.

The height problem is just reading the code to see the height where bars and components render right? Are you suggesting we provide z constants for different HUD elements?

@Fuzss
Copy link

Fuzss commented Sep 25, 2024

The height problem is just reading the code to see the height where bars and components render right? Are you suggesting we provide z constants for different HUD elements?

No. NeoForge adds Gui#leftHeight and Gui#rightHeight. Those update automatically as vanilla bars are drawn above the hotbar. Mods drawing their own bars should update them as well. So e.g. if I have a mod that draws an armor toughness bar above the vanilla armor bar on the left hotbar side I know at what height to draw my bar at, and then another mod that also draws a bar at the left hotbar side can use the left height variable that has been updated by me to draw above me once more.

Copy link
Member

@modmuss50 modmuss50 left a comment

Choose a reason for hiding this comment

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

Im not sure events like this are the best way to do this. An API that allows you to register LayeredDrawer.Layer's in the desired order may be better?

/**
* Called at the start of HUD rendering, right before anything is rendered.
*/
public static final Event<Start> START = EventFactory.createArrayBacked(Start.class, listeners -> (client, context, tickCounter) -> {
Copy link
Member

Choose a reason for hiding this comment

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

Are events like this the best solution here? Would it be better to allow mods to register their own LayeredDrawer.Layer in a similar way to how the vanilla hud elements work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the code to use one interface, but I don't see a difference with registration methods. Wouldn't it still be implemented with an event?

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking they could just be static? I dont think there is a need for an even unless im missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was under the impression that not using events is more complicated than just using events? So there's no need for registration methods? Are you saying just implement it with a synchronized list? Events also allow phase ordering if some mod wants to render before another.

@modmuss50
Copy link
Member

Please let me know if you want to discuss this more

@modmuss50 modmuss50 added the enhancement New feature or request label Oct 11, 2024
@Fuzss
Copy link

Fuzss commented Oct 15, 2024

Im not sure events like this are the best way to do this. An API that allows you to register LayeredDrawer.Layer's in the desired order may be better?

This please! It would be really great if mods were to properly register their GUI layers with an identifier that allows for ordering with callbacks firing before and after those layers render, as is done on NeoForge. That way other mods can interact with those new layers.
Like the Raised mod (for moving the hotbar up) could selectively move modded layers via translating the pose stack, or a minimap could move a custom effects hud out of the way and so on.

Ideally vanilla layers should also be accessible via an identifier, although not sure how to implement that in a way that’s not too invasive.

@kevinthegreat1
Copy link
Contributor Author

Im not sure events like this are the best way to do this. An API that allows you to register LayeredDrawer.Layer's in the desired order may be better?

This please! It would be really great if mods were to properly register their GUI layers with an identifier that allows for ordering with callbacks firing before and after those layers render, as is done on NeoForge. That way other mods can interact with those new layers. Like the Raised mod (for moving the hotbar up) could selectively move modded layers via translating the pose stack, or a minimap could move a custom effects hud out of the way and so on.

Ideally vanilla layers should also be accessible via an identifier, although not sure how to implement that in a way that’s not too invasive.

I had a discussion with him on discord here, and I think you're misunderstanding his proposal. Under his idea, the API would still have the same capability as the events here, since what you're describing uses many more mixins and is significantly more invasive. I have looked at neo's API, and the number of patches required will probably make it not acceptible for fabric.

@modmuss50
Copy link
Member

Yes, as discussed on discord I think it would be better to statically register (or maybe register in events similar to the ones in the current state of the PR a vanilla LayeredDrawer.Layer.

@kevinthegreat1
Copy link
Contributor Author

Yes, as discussed on discord I think it would be better to statically register (or maybe register in events similar to the ones in the current state of the PR a vanilla LayeredDrawer.Layer.

Done in 0d2bcf2.

@Octol1ttle
Copy link
Contributor

Yes, as discussed on discord I think it would be better to statically register (or maybe register in events similar to the ones in the current state of the PR a vanilla LayeredDrawer.Layer.

Done in 0d2bcf2.

I think what modmuss meant here is that we would have a HudRenderRegistry (or HudRenderRegistrationEvents) where mods would put their LayeredDrawer.Layers. Then, all these layers would be added to the drawer directly with addLayer

@kevinthegreat1
Copy link
Contributor Author

I think what modmuss meant here is that we would have a HudRenderRegistry (or HudRenderRegistrationEvents) where mods would put their LayeredDrawer.Layers. Then, all these layers would be added to the drawer directly with addLayer

He said it’s fine to register in events. I used events because it allows ordering the layers within one phase/event.

Also, directly adding to the vanilla list would require similarly many injection points or rely on the indices which is even more undesirable.

/**
* Called after the entire HUD is rendered.
*/
public static final Event<LayeredDrawer.Layer> LAST = createEventForStage();
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe END? There is START, not FIRST or smth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The names are taken from WorldRenderEvents, which has both LAST and END, and LAST is the last place where you should render stuff in most cases. However, I'm happy to change this if modmuss agrees.

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
None yet
Development

Successfully merging this pull request may close these issues.

6 participants