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
Smarter override of version check #2926
Conversation
private const string principia_flags_ = "principia_flags"; | ||
private const string principia_draw_styles_config_name_ = | ||
private const string principia_flags = "principia_flags"; | ||
private const string principia_draw_styles_config_name = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were all very deliberately named that way following an actual bug that would have been caught by that _, see the discussion in #1554.
If we want to revisit that, we should do so in a separate PR, not as a drive-by change in this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted the renaming.
I am not sure that I buy the argument in #1554, actually. The problem there was that the name Δt
was just too short for something declared at the top level (and it's not even clear from the name what was its purpose). Hard to believe that we'd run into hiding issues with principia_override_version_check_config_name
.
Fine with doing that in a separate PR, but please let's come up with a naming convention for constants. Search for private const
and you'll find every.possible.style.
if (version == | ||
$@"{Versioning.version_major}.{Versioning.version_minor}.{ | ||
Versioning.Revision}") { | ||
fail_version_check = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit too double-negativeish, and in both cases we failed the version check; we are just making it non-fatal.
version_check_overriden
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
This will ensure that users don't disable it forever. Example:
Tested in game with 1.11.2.