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.

Sign up for GitHub

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

Properly reset the optimizer after each change of the flight plan #3816

Merged
merged 1 commit into from
Dec 7, 2023
Merged

Properly reset the optimizer after each change of the flight plan #3816

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ksp_plugin_adapter/burn_editor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ public enum Event {
Maximized,
}

// Renders the |BurnEditor|. Returns true if and only if the settings were
// changed.
// Renders the |BurnEditor|. Returns |Changed| if and only if the settings
// were changed.
public Event Render(
string header,
bool anomalous,
Expand Down
59 changes: 39 additions & 20 deletions ksp_plugin_adapter/flight_planner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ internal void ReplaceBurn(int i, Burn burn) {
}
var status = plugin.FlightPlanReplace(vessel_guid, burn, i);
UpdateStatus(status, i);
ResetOptimizer(vessel_guid);
if (burn_editors_?.Count > i && burn_editors_[i] is BurnEditor editor) {
editor.Reset(plugin.FlightPlanGetManoeuvre(vessel_guid, i));
}
Expand Down Expand Up @@ -105,6 +106,7 @@ protected override void RenderWindowContents(int window_id) {
GUILayoutWidth(1)) &&
i != selected_flight_plan) {
plugin.FlightPlanSelect(vessel_guid, i);
ResetOptimizer(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.

As discussed, this is not needed.

final_time_.value_if_different =
plugin.FlightPlanGetDesiredFinalTime(vessel_guid);
ClearBurnEditors();
Expand All @@ -123,6 +125,7 @@ protected override void RenderWindowContents(int window_id) {
plugin.FlightPlanCreate(vessel_guid,
plugin.CurrentTime() + 3600,
predicted_vessel.GetTotalMass());
ResetOptimizer(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.

Same.

final_time_.value_if_different =
plugin.FlightPlanGetDesiredFinalTime(vessel_guid);
ClearBurnEditors();
Expand Down Expand Up @@ -190,7 +193,7 @@ private void UpdateVesselAndBurnEditors() {
burn_editors_.Last().Reset(
plugin.FlightPlanGetManoeuvre(vessel_guid, i));
}
UpdateAfterBurnEditorInsertionDeletion(vessel_guid);
UpdateBurnEditorIndices(vessel_guid);
}
}

Expand Down Expand Up @@ -234,6 +237,7 @@ private void RenderFlightPlan(string vessel_guid) {
final_time_.value);
reached_deadline_ = status.is_deadline_exceeded();
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
}
// Always refresh the final time from C++ as it may have changed because
// the last burn changed.
Expand Down Expand Up @@ -276,6 +280,7 @@ private void RenderFlightPlan(string vessel_guid) {
vessel_guid,
parameters);
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
}
UnityEngine.GUILayout.TextArea(
max_steps_[max_steps_index_].ToString(),
Expand All @@ -291,6 +296,7 @@ private void RenderFlightPlan(string vessel_guid) {
vessel_guid,
parameters);
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
}
}
using (new UnityEngine.GUILayout.HorizontalScope()) {
Expand All @@ -311,6 +317,7 @@ private void RenderFlightPlan(string vessel_guid) {
vessel_guid,
parameters);
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
}
UnityEngine.GUILayout.TextArea(
length_integration_tolerances_names_[
Expand All @@ -331,6 +338,7 @@ private void RenderFlightPlan(string vessel_guid) {
vessel_guid,
parameters);
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
}
}
}
Expand Down Expand Up @@ -366,6 +374,7 @@ private void RenderFlightPlan(string vessel_guid) {
new PlannedOrbitAnalyser(adapter_, predicted_vessel_);
plugin.FlightPlanDelete(vessel_guid);
ResetStatus();
ResetOptimizer(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.

And this one probably caused #3819.

ScheduleShrink();
// The state change will happen the next time we go through OnGUI.
} else {
Expand All @@ -376,13 +385,14 @@ private void RenderFlightPlan(string vessel_guid) {
vessel_guid,
predicted_vessel.GetTotalMass());
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
if (status.ok()) {
// The final time does not change, but since it is displayed with
// respect to the beginning of the flight plan, the text must be
// recomputed.
final_time_.ResetValue(
plugin.FlightPlanGetDesiredFinalTime(vessel_guid));
UpdateAfterBurnEditorInsertionDeletion(vessel_guid);
UpdateBurnEditorIndices(vessel_guid);
return;
}
}
Expand Down Expand Up @@ -528,9 +538,10 @@ private void RenderFlightPlan(string vessel_guid) {
case BurnEditor.Event.Deleted: {
var status = plugin.FlightPlanRemove(vessel_guid, i);
UpdateStatus(status, null);
ResetOptimizer(vessel_guid);
burn_editors_[i].Close();
burn_editors_.RemoveAt(i);
UpdateAfterBurnEditorInsertionDeletion(vessel_guid);
UpdateBurnEditorIndices(vessel_guid);
ScheduleShrink();
return;
}
Expand Down Expand Up @@ -679,10 +690,10 @@ private bool RenderCoast(int index, out double? orbital_period) {
minimized = false
};
Burn candidate_burn = editor.Burn();
var status = plugin.FlightPlanInsert(
vessel_guid,
candidate_burn,
index);
var status = plugin.FlightPlanInsert(vessel_guid,
candidate_burn,
index);
ResetOptimizer(vessel_guid);

// The previous call did not necessarily create a manœuvre. Check if
// we need to add an editor.
Expand All @@ -691,7 +702,7 @@ private bool RenderCoast(int index, out double? orbital_period) {
if (number_of_manœuvres > burn_editors_.Count) {
editor.Reset(plugin.FlightPlanGetManoeuvre(vessel_guid, index));
burn_editors_.Insert(index, editor);
UpdateAfterBurnEditorInsertionDeletion(vessel_guid);
UpdateBurnEditorIndices(vessel_guid);
UpdateStatus(status, index);
ScheduleShrink();
return true;
Expand Down Expand Up @@ -736,6 +747,8 @@ internal bool TryParsePlanLength(string text, out double value) {
return true;
}

// Called to rebuild the optimizer based on the current state of the flight
// plan and the given optimization parameters.
private void ResetOptimizer(string vessel_guid,
CelestialBody centre,
double optimization_altitude,
Expand All @@ -753,6 +766,20 @@ private void ResetOptimizer(string vessel_guid,
optimization_reference_frame_parameters_);
}

// Must be called each time the flight plan is changed by the user: the
// optimizer holds a copy of the flight plan, so proceeding with it would be
// wrong or confusing.
private void ResetOptimizer(string vessel_guid) {
// Rebuild the optimizer as whatever it was doing is now irrelevant.
if (adapter_.plotting_frame_selector_.
Centre() is CelestialBody centre) {
ResetOptimizer(vessel_guid,
centre,
optimization_altitude_,
optimization_inclination_in_degrees_);
}
}

private void ResetStatus() {
status_ = Status.OK;
first_error_manœuvre_ = null;
Expand Down Expand Up @@ -878,19 +905,11 @@ private string GetStatusMessage() {
return message;
}

private void UpdateAfterBurnEditorInsertionDeletion(string vessel_guid) {
private void UpdateBurnEditorIndices(string vessel_guid) {
// Adjust the indices of the current burn editors.
for (int j = 0; j < burn_editors_.Count; ++j) {
burn_editors_[j].index = j;
}
// Rebuild the optimizer as whatever it was doing is now irrelevant.
if (adapter_.plotting_frame_selector_.
Centre() is CelestialBody centre) {
ResetOptimizer(vessel_guid,
centre,
default_optimization_altitude,
default_optimization_inclination_in_degrees);
}
}

private void UpdateAdaptiveStepParameters(
Expand Down Expand Up @@ -949,9 +968,9 @@ private void UpdateAdaptiveStepParameters(
private const double default_optimization_altitude = 10e3;
private const double default_optimization_inclination_in_degrees = 0;

// Initialized at flight plan creation by |ResetOptimizer|.
private double optimization_altitude_;
private double? optimization_inclination_in_degrees_;
private double optimization_altitude_ = default_optimization_altitude;
private double? optimization_inclination_in_degrees_ =
default_optimization_inclination_in_degrees;
private NavigationFrameParameters optimization_reference_frame_parameters_;
}

Expand Down