Upgrade Aquatic

Upgrade Aquatic

26M Downloads

Crash if RenderPlayerEvent.Pre is canceled

maxanier opened this issue ยท 6 comments

commented

Forge's RenderPlayerEvent.Pre is cancelable. This can be used to prevent the player from being rendered (and optionally render something else).
This is e.g. used by Vampirism to render a bat instead when the player is transformed to one.
TeamLapen/Vampirism#673

In the Pre event you push on the given matrix stack and in post you pop again.
https://github.com/minecraftabnormals/Upgrade-Aquatic/blob/7d277eed065fc12a7eb7e4baf790316a1bdc4e6b/src/main/java/com/teamabnormals/upgrade_aquatic/core/events/ClientEvents.java#L38
This works fine if both events are called.
However, when the Pre event is canceled, post is never called and the game crashes with a rather (not very helpful) error message

java.lang.IllegalStateException: Pose stack not empty
	at net.minecraft.client.renderer.WorldRenderer.func_228423_a_(WorldRenderer.java:1085) ~[?:?] {re:classloading,pl:runtimedistcleaner:A}
	at net.minecraft.client.renderer.WorldRenderer.func_228426_a_(WorldRenderer.java:952) ~[?:?] {re:classloading,pl:runtimedistcleaner:A}
	at net.minecraft.client.renderer.GameRenderer.func_228378_a_(GameRenderer.java:600) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
	at net.minecraft.client.renderer.GameRenderer.func_195458_a(GameRenderer.java:422) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
	at net.minecraft.client.Minecraft.func_195542_b(Minecraft.java:924) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
	at net.minecraft.client.Minecraft.func_99999_d(Minecraft.java:553) ~[?:?] {re:classloading,pl:accesstransformer:B,pl:runtimedistcleaner:A}
	at net.minecraft.client.main.Main.main(SourceFile:204) ~[minecraft-1.15.2-client.jar:?] {re:classloading}

Not sure what the best way to fix this is.
Maybe use lowest priority on the Pre event handler.
If the transformation has to be applied before everything else, maybe add a second handler at lowest priority that receives canceled events and use that to pop the stack if the event is canceled.

Or maybe there is a different solution without modifying the stack at all.

commented

There doesn't look like there's a solution to this that's safe. If you want to make a PR to fix it go ahead.

commented

Also, I know many other mods use this same method

commented

I've decided to just set the EventPriority to lowest, hopefully that should resolve the issues.

commented

Fixed by 1.6.1

commented

I don't see any way of doing in a safe way, but subscribing to the event at the lowest priority would at least reduce the number of incompatibilities as most mod use normal priority or higher to cancel the event.

commented

Also, even though other mods might use this as well (I haven't seen any), it still is a bug.
If the event is cancelable it will be cancelled sometimes and with the current solution this directly leads to the mentioned crash.

(Let me know which other mods are using this, so I can create an issue there as well)