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

Basic patch for Realms screen NPE #272

Closed
wants to merge 2 commits into from

Conversation

ah-OOG-ah
Copy link
Member

Opening the Realms screen, clicking "Buy Realm", and then hitting Esc to exit and entering a world will eventually cause an NPE+crash when the HTTP request returns and it tries to display a realms GUI. This adds a simple null check to avert that. It still kicks you out of the world, but you can just log back in instead of restarting the client.

Technically the proper fix would be to stop whatever Realms screen is trying to appear from showing in the first place, but that's much harder and would still leave an NPE for someone else to footgun on.

Tested in full pack and it launched, although the actual functionality couldn't be tested as GTNH removes the realms button.

@ah-OOG-ah ah-OOG-ah marked this pull request as draft October 30, 2023 18:43
@ah-OOG-ah
Copy link
Member Author

After fixing the copy-paste error, it seems to only occasionally fix the issue as drawBackground(int) also uses this.mc. I don't think this.mc should ever change; methods that set it should set it to the same thing on every GuiScreen. I would guess that some/all GuiScreens briefly have this.mc uninitialized, so when rendering expects a this.mc the moment the screen changes it NPE's. The patch as written reduces this chance but doesn't eliminate it.

@ah-OOG-ah
Copy link
Member Author

Gonna be a bit busy so I can't complete this in the forseeable future. If someone else wants to pick this into a new PR the code technically works, just isn't as comprehensive a fix as I'd like

@ah-OOG-ah ah-OOG-ah closed this Nov 3, 2023
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