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

Show altitude and vertical speed even when farther away from planet #2446

Merged
merged 4 commits into from Oct 5, 2013

Conversation

lwho
Copy link
Contributor

@lwho lwho commented Sep 29, 2013

Currently the altitude is only shown when in the rotating frame of the planet. The rotating frame ends beyond atmosphere or highest mountains (whichever is higher). This PR changes this to show the altitude up to 10000km or 0.5 planet radii (whichever is lower). For more than 100km the altitude is shown in km instead of m.

Furthermore, the vertical speed is shown together with the altitude. Note: This is really the vertical speed, not the change in altitude, i.e. it will show zero when you fly horizontal, but the altitude may nevertheless drop if you fly above raising terrain.

@lwho
Copy link
Contributor Author

lwho commented Sep 29, 2013

Hmm, I wonder if it should have special handling for orbital starports to show the height above the parent planet? For surface starports this is not needed as they have no own frame.

Pi::cpan->SetOverlayText(ShipCpanel::OVERLAY_BOTTOM_RIGHT, "");
else {
double vspeed = vel.Dot(Pi::player->GetPosition().Normalized());
Copy link
Contributor

Choose a reason for hiding this comment

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

If you hover above a planet with Set Speed mode to keep your velocity at 0, changing numerical errors make vspeed flicker between positive and negative zero. This needs a threshold (with an explanatory comment) to truncate values that are close to zero to be exactly (positive) zero to stop the display flickering.

@johnbartholomew
Copy link
Contributor

This is mostly fine. I've written a few comments on the diff about some details that could be improved.

@lwho
Copy link
Contributor Author

lwho commented Oct 4, 2013

So, I hope it's much clearer now (and correct ;)). That innocent looking fix for orbital stations did break quite a few places.

@johnbartholomew johnbartholomew merged commit 7b7bcd7 into pioneerspacesim:master Oct 5, 2013
@lwho lwho deleted the altitude_view branch October 5, 2013 10:46
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