-
Notifications
You must be signed in to change notification settings - Fork 69
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
Change the Dialog class to persist its data using a ConfigNode #2096
Conversation
retest this please |
[KSPField(isPersistant = true)] | ||
private int bad_installation_dialog_x_ = Dialog.XCentre; | ||
[KSPField(isPersistant = true)] | ||
private int bad_installation_dialog_y_ = Dialog.YCentre; | ||
private Dialog bad_installation_dialog_ = new Dialog(); |
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.
As discussed, the following are unsightly:
- a non-
null
invalidDialog
; - a
bool
field equivalent to thenull
ness of another field.
Make the Dialog
s null
by default, remove the is_post_apocalyptic_
field, create the Dialog
s when the bool
s would be set.
Take a message at construction in Dialog
, so that there is no need for the "SHOW WITHOUT MESSAGE"
thing.
For abstraction and readability, replace the is_post_apocalyptic_
field with a property:
private bool is_post_apocalyptic { get { return apocalypse_dialog_ != null; } }
is_bad_installation_
remains inequivalent to bad_installation_dialog != null
; it can either become an auto-implemented property or remain a field.
if (is_post_apocalyptic_) {
apocalypse_dialog_.Show();
}
becomes
apocalypse_dialog_?.Show();
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, except that is_post_apocalyptic_
is really not needed anymore.
plugin_.HasEncounteredApocalypse(out revelation_); | ||
if (apocalypse_dialog_ == null) { | ||
String revelation = ""; | ||
if (plugin_.HasEncounteredApocalypse(out revelation)) { |
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.
if (plugin_.HasEncounteredApocalypse(out string revelation)) {
This reverts commit 6ad5a5f.
Also centre the dialog by default.