neoforged / NeoForge

Neo Modding API for Minecraft, based on Forge
https://projects.neoforged.net/neoforged/neoforge
Other
1.17k stars 172 forks source link

ScreenEvent.BackgroundRendered is not fired from most GUIs in 1.20.6 #1077

Closed mezz closed 2 months ago

mezz commented 3 months ago

Minecraft Version: 1.20.6

NeoForge Version: 20.6.113-beta

Logs: N/A

Steps to Reproduce: N/A

Description of issue: NeoForge fires the ScreenEvent.BackgroundRendered event at the end of Screen.renderBackground Unfortunately, most (all?) vanilla GUIs override the renderBackground method and do not call super.renderBackground, so the event is not fired from most GUIs.

There seems to be a similar problem with ContainerScreenEvent.Render.Background, which is only fired from AbstractContainerScreen.render and not always fired from child classes like AbstractFurnaceScreen, CraftingScreen, etc.

It seems like the events need to be fired from many new places in order to be consistent. I do not have a great solution for this problem, so I opened an issue instead of a PR.

Soaryn commented 3 months ago
net.neoforged.neoforge.common.NeoForge.EVENT_BUS.post(new net.neoforged.neoforge.client.event.ScreenEvent.BackgroundRendered(this,  graphics));

likely needs to be called in the renderBackground after the renderBg call in AbstractContainerScreen. given AbstractContainerScreen doesn't call super since it shouldn't be doing the blurring that happens on something like the esc menu

That being said, I don't know what all is really expected to be called in the event.

mezz commented 3 months ago

I think I created this event originally so that JEI could render below GUI tooltips in like 1.8.9

It seems like it's no longer necessary because Mojang's rendering is smarter now, so I removed the need for it in JEI entirely https://github.com/mezz/JustEnoughItems/commit/b13d85c2091402afbfb17addab2c18ad7255bf20

You may want to evaluate the current needs for this event (if any) and consider removing it.

mezz commented 3 months ago

Ok here, I found the original PR justification for the event: https://github.com/MinecraftForge/MinecraftForge/pull/2370

JEI needs to draw above the gui's background but below the gui's tooltips. https://github.com/mezz/JustEnoughItems/issues/86 By adding this event, anything that needs to draw beside a gui correctly can do so.

This reasoning no longer holds, so I think the event should be considered for removal.