MuMech / MechJeb2

MechJeb2 - KSP mod
Other
997 stars 251 forks source link

Maneuver Node Editor does irreversible changes #1002

Open mmithril opened 6 years ago

mmithril commented 6 years ago

After upgrade to 1.4, Maneuver Node Editor does irreversible changes to the trajectory. Both CKAN version and latest DEV

Steps to reproduce:

  1. Launch a vehicle with MechJeb on board
  2. Use Advanced Transfer... tool to create injection to Moho
  3. Focus on Moho and display periapsis details
  4. Open Maneuver Node Editor and click Update several times
  5. Adjust Prograde by +0.01 & -0.01
  6. Add maneuver node to periapsis

Expected:

Actual:

Which logs should I provide?

lamont-granquist commented 6 years ago

I've seen this in at least 1.3.1 as well, i don't think its 1.4.x specific, dunno how long it has been around.

I thought it might have been that somehow the dV vector in the maneuver node was being converted to world coordinates, been edited for a few seconds while the "world rotated" and then being written back but I couldn't see where a bug like that might be happening. When I looked at the code the math seemed to be in the maneuver node coordinate basis which should be non-rotating from frame-to-frame, so IDK.

lamont-granquist commented 6 years ago

I wonder if it has something to do with the code changes to be able to merge/add two maneuver nodes together?

sarbian commented 6 years ago

I re-read the code and I don't see how the stock code does not do the same thing.... I ll try to investigate it more in the coming days

mmithril commented 6 years ago

Observation: when I click e.g. +0.1 fast, periapsis to Moho goes lower (as expected). When I click slow, it goes higher (wrong). It really looks like some time-dependent rounding / truncatation happens.

Dňa ut, 13. mar 2018 10:07 sarbian notifications@github.com napísal(a):

I re-read the code and I don't see how the stock code does not do the same thing.... I ll try to investigate it more in the coming days

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MuMech/MechJeb2/issues/1002#issuecomment-372594864, or mute the thread https://github.com/notifications/unsubscribe-auth/AK60a3yHsDVZL2Wr_MtFSJ53oL0SdRn5ks5td4xdgaJpZM4SmtgB .

sarbian commented 6 years ago

Could you test with Precise Node to see if it has the same problem ? If not then it may be some weird interaction with the EditableDouble

sarbian commented 6 years ago

The only mods that do things differently is Better_Maneuvering and I do not understand why he does it... https://github.com/DMagic1/KSP_Better_Maneuvering/blob/master/Source/BetterManeuvering/ManeuverController.cs#L1055

sarbian commented 6 years ago

So... all the mods exhibits the same problem. It seems there is an instability somewhere in the stock intercept code and I can't do much about it. At best I could make sure the update button do nothing when the value did not change... Other ideas ?

mmithril commented 6 years ago

Ideas

Another observations:

Dňa ut, 13. mar 2018 20:25 sarbian notifications@github.com napísal(a):

So... all the mods exhibits the same problem. It seems there is an instability somewhere in the stock intercept code and I can't do much about it. At best I could make sure the update button do nothing when the value did not change... Other ideas ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MuMech/MechJeb2/issues/1002#issuecomment-372788537, or mute the thread https://github.com/notifications/unsubscribe-auth/AK60a-2NHzjMvGmNOVQB21qsecGHe3Naks5teB0QgaJpZM4SmtgB .

lamont-granquist commented 6 years ago

I'm pretty certain that the problem is more trivially reproducible:

And MJ is saving the node with the same Vector3d and UT values that it read off the node.

lamont-granquist commented 4 years ago

This is likely just due to loss of precision due to the KSP ManeuverNode object using a Quaternion instead of a QuaternionD.

For the editor, if we try to stay in the Frenet frame and not round-trip through the Burn vector frame (which rotates with the inverse rotation of the universe) then we might at least be able to make the node editor idempotent.

lamont-granquist commented 4 years ago

reported https://bugs.kerbalspaceprogram.com/issues/24582

lamont-granquist commented 4 years ago

So we don't round-trip through the Burn vector and keep everything in the frenet frame, so the errors are likely just from the loss of precision packing into and out of the Quaternion.