`ScreenEvents.afterRender`'s screen null check fails when opening the disconnect screen
Minenash opened this issue ยท 22 comments
Running the following in my screen's render method results in the null check failing, and crashing the game.
client.getInstance().world.disconnect();
client.getInstance().disconnect(new SaveLevelScreen(new TranslatableText("menu.savingLevel")));
My screen's code and the code that calls it
Crash Log
Though, if I strip everything and just have the code in the code block in the render, and nothing else, it still crashes. If I run the code instead of opening the screen, it doesn't crash.
The code in the quit method is practically identical to the code in the vanilla's exit to main menu button
Someone should check if this still happens in 1.18+ given that the code was changed a bit iirc?
java.lang.NullPointerException: Screen cannot be null
at java.base/java.util.Objects.requireNonNull(Objects.java:235)
at net.fabricmc.fabric.api.client.screen.v1.ScreenEvents.afterRender(ScreenEvents.java:135)
at net.minecraft.client.render.GameRenderer.handler$zli000$fabric-screen-api-v1$onAfterRenderScreen(GameRenderer.java:3108)
at net.minecraft.client.render.GameRenderer.render(GameRenderer.java:929)
at net.minecraft.client.MinecraftClient.render(MinecraftClient.java:1193)
at net.minecraft.client.MinecraftClient.run(MinecraftClient.java:781)
at net.minecraft.client.main.Main.main(Main.java:244)
at net.minecraft.client.main.Main.main(Main.java:51)
at net.fabricmc.loader.impl.game.minecraft.MinecraftGameProvider.launch(MinecraftGameProvider.java:462)
at net.fabricmc.loader.impl.launch.knot.Knot.launch(Knot.java:74)
at net.fabricmc.loader.impl.launch.knot.KnotClient.main(KnotClient.java:23)
at net.fabricmc.devlaunchinjector.Main.main(Main.java:86)
That is just not supposed to happen - you should at least provide more context.
I mean, when you exit the world, doesn't it set the screen to null? so you shouldn't touch the world etc. in render method, which is part of a frame in fps. You are supposed to do those logic in client ticks.
You call quit
in render
, after which the screen supposingly should be still there?
I think vanilla calls screen quit in button handling. Button handling I recall is detected in a loop in the tick code. Consider moving your quit code there?
For fixing the mod, I can try that.
After more testing (posted on the discord), I had found out that the current screen was never null during this, but for some reason, the beforeRender didn't get ran once, so therefore the afterRender got called twice in a row.
I mean, when you exit the world, doesn't it set the screen to null?
Not that I could find. The disconnect method changes the screen into a save or progress screen.
Button handling I recall is detected in a loop in the tick code. Consider moving your quit code there?
How would you recommend I do this? I'm using the screen as a guaranteed sync to the very next render that's not the menu screen so it usable for the screenshot.
Even though I can get my mod to work with this, it is still slightly concerning that a inject is just not running in a single instant while another inject that has the same target runs fine.
Like use an atomic boolean that is set to true in render method when the screen needs to quit. In tick method you detect if the boolean is true and quit accordingly if it is.
Like use an atomic boolean that is set to true in render method when the screen needs to quit. In tick method you detect if the boolean is true and quit accordingly if it is.
If that is the case, it may be worth adding a line to the openScreen
and reset
screen methods noting there may be failure if you open a screen during the "render phase"
I think what happens is when you go to disconnect on the client, what will happen is the client will set the new screen and then call render(Z)
until netty has terminated and the integrated server has stopped. What this may do is while you are in a render phase, you send up starting another render phase while you are rendering.
Might be solvable if we track the currently open screen via some sort of stack?
you send up starting another render phase while you are rendering.
Hmm, it's possible, though the original screen's render method fully finishes executive before the null pointer
So after afterRender
is called, the tracking field for the currently open screen is set to null.
So you have this kind of call stack
renderPhase {
renderScreen {
disconnect {
renderPhase {
renderScreen()
setToNull() <== not null
}
}
}
afterRender() <= screen is null
setToNull()
}
Normally you are not nesting render phases with vanilla screens, and minecraft calls it's open/close methods on the tick loop, which is after the render phase.
Since the field to track the current screen is static, you may end up having that field become null before the root render phase calls the after event