mockingbirdnest / Principia

𝑛-Body and Extended Body Gravitation for Kerbal Space Program
MIT License
774 stars 70 forks source link

Crashes with: "Check failed: height_above_terrain_at_time(min_radius_time)" on landings, reproducible save included #3826

Closed R-T-B closed 11 months ago

R-T-B commented 11 months ago

My personal modset + a savegame that will reproduce this issue shortly if in mapview returning to Kerbin at the poles on Flight "Shuttle Flight 14." Download and extract to fresh KSP 1.12.5 install root.

If it is not reproducible, I also had parallax installed and blackracks private eve build. I removed both for size + copyright concerns. I doubt they are related. Link to everything below:

[Link removed due to licensing afterthoughts, see below]

log follows:

E1228 22:03:47.925518  9252 interface.cpp:728] Initialized Google logging for Principia
E1228 22:03:47.926522  9252 interface.cpp:729] Principia version 2023121300-Jordan-0-ge51928f832188fd4eeedd50e33d11218e1cd44eb built on 2023-12-14T12:13:27Z by Microsoft Visual C++ version 193732822 for Windows x86-64
E1228 22:03:47.926522  9252 interface.cpp:742] Base address is 00007FFDE8E70000
E1228 22:06:26.575793 17588 matrix_computations_body.hpp:479] Difficult diagonalization: rows: 4 columns: 4
{-1.87214160877916438e+04 kg, -1.11275071104159906e-02 kg, -1.10869058377716101e-03 kg, +2.71562462651951364e+01 kg}
{-1.11275071104159906e-02 kg, -1.87214133621546862e+04 kg, +1.25276614431868438e-03 kg, -3.06850472464330721e+01 kg}
{-1.10869058377716101e-03 kg, +1.25276614431868438e-03 kg, -1.87214258107967253e+04 kg, -3.05730029114141644e+00 kg}
{+2.71562462651951364e+01 kg, -3.06850472464330721e+01 kg, -3.05730029114141644e+00 kg, +5.61642552607430625e+04 kg}
, stopping with: rows: 4 columns: 4
{-1.87214259356260154e+04 kg, -1.81898940354585648e-12 kg, +5.24846355783497041e-13 kg, -1.09050665955013425e-12 kg}
{+1.81898940354585648e-12 kg, -1.87214259356260154e+04 kg, +2.87639271922818661e-13 kg, -4.79516854523367871e-13 kg}
{+3.06293985778974682e-13 kg, +6.86424947071341139e-13 kg, -1.87214259356025323e+04 kg, -1.19089900617334780e-12 kg}
{-1.09128106315728343e-12 kg, -4.77940478041498333e-13 kg, -1.19061072539370256e-12 kg, +5.61642778068545595e+04 kg}
E1228 22:06:41.219045 16796 matrix_computations_body.hpp:479] Difficult diagonalization: rows: 4 columns: 4
{-1.87214174082352692e+04 kg, -9.62126363873494483e-03 kg, -1.02757680412679520e-03 kg, +2.52701088558172273e+01 kg}
{-9.62126363873494483e-03 kg, -1.87214150801696378e+04 kg, +1.15939180926716290e-03 kg, -2.85117051565647515e+01 kg}
{-1.02757680412679520e-03 kg, +1.15939180926716290e-03 kg, -1.87214258117998615e+04 kg, -3.04512529376341945e+00 kg}
{+2.52701088558172273e+01 kg, -2.85117051565647515e+01 kg, -3.04512529376341945e+00 kg, +5.61642583002047613e+04 kg}
, stopping with: rows: 4 columns: 4
{-1.87214259355985305e+04 kg, +1.68878318656108460e-13 kg, -5.05918128579483400e-13 kg, +5.85143988179790399e-18 kg}
{-1.63810507081217011e-12 kg, -1.87214259356260227e+04 kg, -1.81898940354585648e-12 kg, -2.05323166971010605e-14 kg}
{-6.25075773358486714e-13 kg, +1.81898940354585648e-12 kg, -1.87214259356260227e+04 kg, +3.11252338986282555e-13 kg}
{-5.97090367216505023e-16 kg, -1.98803320622860366e-14 kg, +3.11236910852713865e-13 kg, +5.61642778068505868e+04 kg}
F1228 22:06:45.032204  2856 apsides_body.hpp:216] Check failed: height_above_terrain_at_time(min_radius_time) <= Length{} (+3.93302324484102428e+02 m vs. +0.00000000000000000e+00 m) 
R-T-B commented 11 months ago

I've just been advised that a.) a modzip is not a good thing re licensing, and b.) a journaling system exists and I should use it. Stand by.

EDIT: Journal link below:

https://drive.google.com/file/d/1j9Rk3HU6eC9_7HRC-hJTjXLQDsJcCLoY/view?usp=sharing

pleroy commented 11 months ago

These checks don't make sense because max_radius_time and min_radius_time are not necessarily the times when we cross the max or min radius, because of these statements.

eggrobin commented 11 months ago

These checks don't make sense

They don’t, but given how this is called it should work out. This looks fishy though: https://github.com/mockingbirdnest/Principia/blob/eebee0c928597fb35883b345a4fdf0885ca7c442/ksp_plugin_adapter/map_node_pool.cs#L189-L192

R-T-B commented 11 months ago

These checks don't make sense

They don’t, but given how this is called it should work out. This looks fishy though:

https://github.com/mockingbirdnest/Principia/blob/eebee0c928597fb35883b345a4fdf0885ca7c442/ksp_plugin_adapter/map_node_pool.cs#L189-L192

Interesting you should say that, because reverting that exact set of changes via dnspy (the map_node_pool.cs changes) fixed it for me.

eggrobin commented 11 months ago

Interesting you should say that, because reverting that exact set of changes via dnspy (the map_node_pool.cs changes) fixed it for me.

You mentioned that on Discord already; it is certainly true that ripping out the feature means you do not hit the bugs in that feature, but I would not say that it is interesting. But look more closely at lines 190 and 191 of the quoted code.

R-T-B commented 11 months ago

Interesting you should say that, because reverting that exact set of changes via dnspy (the map_node_pool.cs changes) fixed it for me.

You mentioned that on Discord already; it is certainly true that ripping out the feature means you do not hit the bugs in that feature, but I would not say that it is interesting. But look more closely at lines 190 and 191 of the quoted code.

It becomes interesting when you have no knowledge of C++ and don't know if that's doing something unseen.

I did however, indeed miss the "Latitude Latitude" mistake. That is kind of hilarious tbh.

Thanks for your attention on this anyways. :)

Proxima-b commented 11 months ago

Log file created at: 2024/01/09 00:00:54 Running on machine: PROXIMACOMMAND Log line format: [IWEF]mmdd hh:mm:ss.uuuuuu threadid file:line] msg @ 00007FFDFFE144FF google::LogMessageFatal::~LogMessageFatal [0x00007FFDFFE144FE+46] @ 00007FFD5A7FB23F principiaVesselVelocity [0x00007FFD5A7FB23E+646798] @ 00007FFD5A893319 principiaVesselVelocity [0x00007FFD5A893318+1269608] @ 00007FFD5A72BD95 principiaUpdatePrediction [0x00007FFD5A72BD94+7604] @ 00007FFD5A72AACD principiaUpdatePrediction [0x00007FFD5A72AACC+2796] @ 00007FFE16A49363 recalloc [0x00007FFE16A49362+162] @ 00007FFE17E3257D BaseThreadInitThunk [0x00007FFE17E3257C+28] @ 00007FFE1902AA58 RtlUserThreadStart [0x00007FFE1902AA57+39] F0109 00:00:54.627553 22764 apsides_body.hpp:215] Check failed: Length{} <= height_above_terrain_at_time(max_radius_time) (+0.00000000000000000e+00 m vs. -7.84768058301880956e+02 m)

I still get a related crash after I dnspyed the change into ksp plugin adapter, but it doesn't seem to be reproducable now, I changed the inclination before the deorbit burn and the crash stooped

pleroy commented 11 months ago

Shrug. We are not going to debug hacked binaries, so no point in dumping more crashes here.

If someone builds Principia from head and they still see the crash, then we want to hear about it. Otherwise, no.

R-T-B commented 11 months ago

Yeah you can't really expect them to try to debug anything hacked together with dnSpy. Good news is there should be a new release addressing this in just a few days, if the new moon schedule holds.