-
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
Display the flight planner and the orbit analyser in the tracking station #2536
Conversation
…ner. This avoids creating a little window stump in the tracking station.
@@ -1587,8 +1591,7 @@ class PartCentredForceHolder { | |||
// The only timing that satisfies these constraints is BetterLateThanNever | |||
// in LateUpdate. | |||
string main_vessel_guid = PredictedVessel()?.id.ToString(); | |||
if (MapView.MapIsEnabled && main_vessel_guid != null && | |||
PluginRunning() && plugin_.HasVessel(main_vessel_guid)) { | |||
if (MapView.MapIsEnabled && main_vessel_guid != null && PluginRunning()) { |
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.
PredictedVessel
requires PlugginRunning
, but we are only testing that after calling it; either it was, and remains, useless here, or this is a fatal failure.
Add PluginRunning
to the conjunction in PredictedVessel
.
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.
} | ||
|
||
// A helper for implementing the RenderButton() method of the subclasses. | ||
protected void RenderButton(string text) { |
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 does not seem specific to vessel-related windows: the plotting frame selectors have a button too.
Should there be RenderButton
in the base class, overriden here (not necessarily in this pull request, but maybe file a cleanup issue).
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.
Look at the code of this function: it is dependent on the existence of a predicted vessel. That's why it belongs here. This was horribly broken in the OrbitAnalyser
, as the decision to hide was made in the middle of RenderWindow
, a sure recipe for layout/repaint inconsistencies. This proves that the logic is non-obvious and belongs to a common place.
These two seem to make the tracking station much more useful.
The implementation involves a bit of code restructuring where we use
PredictedVessel
systematically in many places that were usingActiveVessel
. Overall this factors out a bit of logic and fixes a buglet where the orbit analyser in the tracking station would show a stump of a window.There is a remaining know issue for a Kerbal in EVA: if a flight plan is constructed in the tracking station, that flight plan is lost when switching to the map view. That's because the Kerbal becomes unready for a couple of frames. It doesn't happen in the other direction. We call this "the invariants of KSP". The ultimate fix would be to keep some of the state of unmanaged vessels, but that's a long term effort.
Fix #2531.