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

Fixed malilib#163 - onRenderGameOverlayPost not actually rendering above all Hud Elements #164

Open
wants to merge 2 commits into
base: pre-rewrite/fabric/1.21.1-masa
Choose a base branch
from

Conversation

EnricoMessall
Copy link

  • Adjusted RenderEventHandler to be added to layeredDrawer

 + Adjusted RenderEventHandler to be added to layeredDrawer
@EnricoMessall EnricoMessall changed the title Fixes #163 - onRenderGameOverlayPost not actually rendering above all Hud Elements Fixed malilib#163 - onRenderGameOverlayPost not actually rendering above all Hud Elements Sep 18, 2024
 + Adjusted naming to match malilib
@sakura-ryoko
Copy link

Applied to --> sakura-ryoko@6861afe

@EnricoMessall
Copy link
Author

Hey @sakura-ryoko

i've noticed you reverted the PR from your project:

"Revert PR #164 from upstream, as this code causes rendering problems that you might not immediately notice."

can you explain which rendering problems happen there?

@sakura-ryoko
Copy link

sakura-ryoko commented Sep 24, 2024

Inventory Overlay problems, Info Lines problems, etc. It causes the "Equipment" overlay to not display properly, and the Info Lines not display properly (Text is darkened out behind the BG), among other issues; perhaps.

I am referring to Snapshot; because under Snapshot, more of these elements needed to be moved to the DrawContext rendering away from the Global Stack rendering; such as with various textures; and it causes things rendered with the Global Stack to "Overlay" them.

@sakura-ryoko
Copy link

If you want to test your code under Snapshot; go ahead and see the issues presented.

@EnricoMessall
Copy link
Author

I see, for AdvancedChat its actually no problem, there you have all the Chat stuff above anything else as soon as you open the chat window, but for other modules that might be not wanted. Maybe its possible to inject into different layeredDrawer variables like the drawers that are private in the InGameHud at other positions then and its questionable if the registerGameOverlayRenderer is labeled correct with:

/**
* Registers a renderer which will have its {@link IRenderer.onRenderGameOverlayPost}
* method called after the vanilla rendering is done
* @param renderer
*/

@sakura-ryoko
Copy link

It would need a second IRenderer.onRenderGameOverlayXXXXXX

@sakura-ryoko
Copy link

Which malilib based menus are you having trouble with ?

@EnricoMessall
Copy link
Author

EnricoMessall commented Sep 24, 2024

You can see it in the corresponding issue #163 its about my 1.21.1 port of AdvancedChatHUD https://github.com/Arematics/AdvancedChatHUD

There I resolved it in the release by replacing the malilib injection of onRenderGameOverlayPost with the suggestion here.

@sakura-ryoko
Copy link

Does 'AdvancedChatHud' use MaLiLib ?

@EnricoMessall
Copy link
Author

Yes, all AdvancedChat Mods depend on it

@sakura-ryoko
Copy link

Yes, all AdvancedChat Mods depend on it

Then I can just add an additional Interface for you to insert the rendering into the Drawer, while leaving the existing untouched.
Does that work?

@EnricoMessall
Copy link
Author

EnricoMessall commented Sep 24, 2024

Sorry, i can also use it like that and keep the MaliLib Mixin overwritten as it is, my current AdvancedChatHUD release works as expected, i dont need a specific endpoint from Malilib :D Just wanted to mention earlier that maybe the JavaDoc for the onRenderGameOverlayPost might not be fully accurate when it says its after all Minecraft renders but then renders behind the InGameHUD elements as seen on the screenshot in #163

@sakura-ryoko
Copy link

sakura-ryoko commented Sep 24, 2024

They perhaps added this to the code (Depth Test) and the Mixin wasn't changed. Under Snapshot, there is no enable/disable depth test at this location.

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