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

Fix runtime modified graphics settings potentially being saved #222

Merged

Conversation

LostLuma
Copy link
Collaborator

Resolves #213 by reverting any modified settings while the Options.save method is being ran.

This sadly causes an additional world reload if using the minimal graphics state, however since this bug is exceedingly rare and users are already discouraged from using the minimal setting it I think this solution is acceptable.

Other ways of fixing the issue I've considered are way more work to write, maintain, and port to other Minecraft versions.

@juliand665
Copy link
Owner

Do you reckon it might be feasible and/or better to avoid the reload by redirecting whatever method gets the current options to save them to instead receive unmodified options? Oof that's quite the sentence—let me know if it makes sense

@LostLuma
Copy link
Collaborator Author

It is certainly possible and might be considered better, but I don't think it's really worth it.

redirecting whatever method gets the current options to save them to instead receive unmodified options

The issue with this is that the option enums / values aren't just stored as fields in the Options class, but wrapped inside an OptionInstance object, which is pretty difficult to reconstruct as it has only private fields and modifies the arguments passed to the constructor.

@LostLuma
Copy link
Collaborator Author

It might also be good to keep in mind that the game doesn't save your options randomly, but only if some sort of options screen is excited, or a setting is persisted (joining a multiplayer server, or finishing a tutorial step, ..) which should only happen during active gameplay or while the world hasn't rendered yet anyway.

@juliand665
Copy link
Owner

Good points. My last idea would be to inspect the stack trace (lol) to see if we're being persisted, and return unmodified values from our overrides if so

@LostLuma
Copy link
Collaborator Author

That has the same issue as mentioned in my first comment, so I don't think it's really an option.

@LostLuma LostLuma merged commit 2b4dd49 into juliand665:main Sep 20, 2024
2 checks passed
@juliand665
Copy link
Owner

Fair enough—I'm probably working off a less precise understanding of this system than you :)

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.

Options are not restored when improperly quitting the game
2 participants