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

Prevent zero length direction vector #5569

Merged
merged 1 commit into from May 18, 2023

Conversation

Gliese852
Copy link
Contributor

Approximately once every one and a half in-game years (for the Sol system), the following state may occur:

  • some ship is directed towards the planet
  • it has AICmdFlyTo command active
  • its target is on course
  • gravity, respectively, is directed along the the ship, and accelerates it
  • its reverse thrusters are weaker than gravity
  • it has zero speed

The combination of these circumstances leads to the fact that both coefficients in the calculation of the head vector are equal to 0. A vector of zero length is passed to Propulsion, where it is normalized and SIGFPE occurs.

Fixes #5516

It would be nice to do a thorough reworking of the autopilot here. For example, to ban ships from flying and stopping with their nose down in gravity, it looks strange. @azieba you worked next to this code not so long ago, I would like to know if you have any plans here?

Approximately once every one and a half in-game years (for the Sol
system), the following state may occur:

  - some ship is directed towards the planet
  - it has AICmdFlyTo command active
  - its target is on course
  - gravity, respectively, is directed along the the ship, and
    accelerates it
  - its reverse thrusters are _weaker than gravity_
  - it has zero speed

The combination of these circumstances leads to the fact that both
coefficients in the calculation of the head vector are equal to 0.
A vector of zero length is passed to Propulsion, where it is normalized
and SIGFPE occurs.
@azieba
Copy link
Contributor

azieba commented Mar 9, 2023

It would be nice to do a thorough reworking of the autopilot here. For example, to ban ships from flying and stopping with their nose down in gravity, it looks strange. @azieba you worked next to this code not so long ago, I would like to know if you have any plans here?

Does this really happen? Do you have 'how to reproduce' maybe? I would gladly take a look, unless you want to work on it.
I would not rework it entirely though, I am not sure I understand all the concepts in this code.

@Gliese852
Copy link
Contributor Author

@azieba to reproduce the SIGFPE, you can launch on Mars, turn on time acceleration, and skip a year or two. (one year on x10000 is 53 minutes of real time).

I cheated a little to create a situation quickly. If built from this branch,
https://github.com/Gliese852/pioneer/tree/test-sigfpe
2 buttons appear near the speed limiter, "1" and "2".
You need to start on Mars, press "Undock", "1", "2" and SIGFPE happens.

And the "weird nose-down hover" happens every time you land on autopilot at a surface space station - the ship hangs nose-down at an altitude of 500 m, only then it turns belly down, releases the landing gear and begins to descend onto the pad.

@azieba
Copy link
Contributor

azieba commented Mar 9, 2023

And the "weird nose-down hover" happens every time you land on autopilot at a surface space station - the ship hangs nose-down at an altitude of 500 m, only then it turns belly down, releases the landing gear and begins to descend onto the pad.

Oh, you mean the phase when the ship goes from 15km above the station down to 500m nose down. Well I assumed that this was originally intended, but when I think about it, yes it seems a bit strange. It might be the case that if the gravity is greater then retro thrusters the autopilot would flip the ship around, but this would be rare. Do you think that the ship should descend using main thrusters in all cases?

@Web-eWorks
Copy link
Member

Web-eWorks commented Mar 9, 2023

It might be the case that if the gravity is greater then retro thrusters the autopilot would flip the ship around, but this would be rare. Do you think that the ship should descend using main thrusters in all cases?

The ship should always descend with the belly or rear thrusters pointing in the direction of the ship's velocity (ideally the autopilot code queries the ship's Propulsion for the correct descent orientation, for e.g. future tail-sitter craft), depending on the gravity. This is for two reasons - it looks more correct to a human audience, and allows for emergency burns to avoid impacting the planet, especially if the player were to take over the autopilot.

I'd generally expect a descent using rear thrusters on airless worlds, and a descent using belly thrusters on atmospheric worlds, as our craft generally produce the most drag when descending straight up/down. Ships that cannot fight the world's gravity with their belly thrusters should be incapable of landing on planets (as ships generally do not have landing gear in such a way they could land with the rear thruster facing the pad).

@azieba
Copy link
Contributor

azieba commented Mar 9, 2023

I'd generally expect a descent using rear thrusters on airless worlds, and a descent using belly thrusters on atmospheric worlds, as our craft generally produce the most drag when descending straight up/down. Ships that cannot fight the world's gravity with their belly thrusters should be incapable of landing on planets (as ships generally do not have landing gear in such a way they could land with the rear thruster facing the pad).

This should not be too hard. I'll try to implement it.

Copy link
Contributor

@azieba azieba left a comment

Choose a reason for hiding this comment

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

The fix is fine for me. I just wonder if we should have protection against null dir vectors in the propulsion code?

@Web-eWorks
Copy link
Member

As long as a vector's length is greater than epsilon, usable direction information (of arguable quality) can be extracted from it - I'm not sure it's worth attempting to "fix" zero-length vectors inside of Propulsion, as there's no useful semantic information left and the calling code is almost always more qualified to determine what the effect should be if the vector is zero-length.

@impaktor impaktor merged commit 859257b into pioneerspacesim:master May 18, 2023
@Gliese852 Gliese852 deleted the sigfpe-zero-head branch July 1, 2023 20:58
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.

SIGFPE if flying long enough
4 participants