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

Restructure the rendering of the flight plan #2104

Merged
merged 7 commits into from
Mar 24, 2019

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented Mar 22, 2019

This ensures that we don't change state between Layout and Repaint, and it eliminates a funky null pointer exception when first creating a flight plan.

// Repaint. This means that any state change must occur before Layout or
// after Repaint. This if statement implements the former. It updates the
// vessel and the editors to reflect the current state of the plugin and
// then proceeds with the UI code.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we factor out this if statement in its own function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

RenderFlightPlan(vessel_guid);
} else if (UnityEngine.GUILayout.Button("Create flight plan")) {
plugin_.FlightPlanCreate(vessel_guid,
plugin_.CurrentTime() + 1000,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad indent.

Sorry, something went wrong.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Sorry, something went wrong.


FlightPlanAdaptiveStepParameters parameters =
plugin_.FlightPlanGetAdaptiveStepParameters(vessel_guid);
if (plugin_.FlightPlanExists(vessel_guid)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this not change the set of controls between calls to OnGUI?
the else if branch below may change the value of this condition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment for the uninitiated.

@eggrobin eggrobin added the LGTM label Mar 23, 2019
@pleroy pleroy merged commit 44a01cd into mockingbirdnest:master Mar 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants