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

Suggestion for fixing massOverride #126

Merged
merged 3 commits into from
Sep 4, 2021
Merged

Conversation

StonesmileGit
Copy link

Found a way to make massOverride work, but since KSP doesn't like a value being null (the default with the previous float? massOverride), I used float.MinValue instead.

There is likely a much better way to do this.

@dkavolis
Copy link
Owner

dkavolis commented Sep 4, 2021

Thanks, I didn't expect KSP to have a problem with nullable values

@StonesmileGit
Copy link
Author

StonesmileGit commented Sep 4, 2021

MassOverride seems a bit unnecessary now as both the getter and setter are so simple. Would the code be more clear if it was removed?

@dkavolis
Copy link
Owner

dkavolis commented Sep 4, 2021

MassOverride seems a bit unnecessary now as both the getter and setter are so simple. Would the code be more clear if it was removed?

The setter needs to modify field visibility so it has to stay but the modifications are still missing.

@StonesmileGit
Copy link
Author

The setter no longer modifies the fields; it only sets the value to value or null

@dkavolis
Copy link
Owner

dkavolis commented Sep 4, 2021

The setter needs to have

Fields[nameof(curWingMass)].guiActiveEditor = value is not null;
Fields[nameof(massMultiplier)].guiActiveEditor = value is not null;

to maintain correctness

@StonesmileGit
Copy link
Author

Oh, so the Field modifications should be in both the setter and in Initialization?

@dkavolis
Copy link
Owner

dkavolis commented Sep 4, 2021

Yeah, and it may be easier to call setter in Initialization to update field visibility.

@dkavolis
Copy link
Owner

dkavolis commented Sep 4, 2021

Thanks

@dkavolis dkavolis merged commit f9a48d3 into dkavolis:master Sep 4, 2021
@StonesmileGit
Copy link
Author

Glad I could help!

@StonesmileGit
Copy link
Author

@dkavolis Do note that I did not push recompiled .dll's

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.

None yet

2 participants