taniwha / Extraplanetary-Launchpads

GNU General Public License v3.0
54 stars 45 forks source link

Crash with Principia on CaptureCraft (Separate failure to issue 152) #157

Closed LegoF4 closed 2 years ago

LegoF4 commented 4 years ago

Hello!

I realize there have been multiple issues about this in the past, but I figured since the failure mode within Principia appears different from issue 152 (but perhaps similar to closed issue 137), I created a separate issue.

The problem condition: When finalizing a build in LKO, the game invariably crashes due to Principia failing a check. See FATAL log below.

Relevant base info:

I have cloned the EL source to my own device, and have compiled it with the addition of numerous debug calls. Doing so, I have identified the crash as occurring after the call to OrbitPhysicsManager.HoldVesselUnpack (2); in BuildControl, given that one part was found to be unstarted. The code does not have time to loop twice.

Logs HERE

taniwha commented 4 years ago

It looks very much like the problem exists in Principia: EL's code is working as designed: it is forcing the vessel to stay on rails until all parts have started. On the other hand, Principia is failing to clear one of its lists. My suspicion is that Principia does not know what to do with vessels that are spawned while already in flight mode.

Also, please note that the mod's name is Extraplanetary Launchpads (thus EL), not Extra Planetary Launchpads. This is not just a minor difference in spelling, but a significant difference in grammar and semantics: in the former, "extra" is a prefix meaning "outside" or "off" (as in extracurricular and extraterrestrial) while in the latter "extra" is an adjective meaning "additional". Thus "Extraplanetary Launchpads" means "launchpads off the planet", not "additional launchpads on the planet".

LegoF4 commented 4 years ago

First, I appreciate the quick response! Semantic notes, aside, I suspect you're correct as to what is occurring. I have linked Principia's devs on to this issue. Indeed, the fatal error is clearly thrown from Principia's side.

That said, just to be clear, as my understanding of KSP's source is essentially non-existent: Is it possible that at some point EL creates a vessel with an empty parts List? And is this a reasonable situation (in stock)?

eggrobin commented 4 years ago

Hi @taniwha, it’s been a while :-)

It looks very much like the problem exists in Principia

Well, it exists in the interaction between our mods. We rely on some property which always holds in stock, but no longer does in the presence of EL (we then complain rather loudly if the invariants upon which we rely are violated, which, while being unconventional, has the advantage of making such oddities visible).

This issue/mockingbirdnest/Principia#2582 reminds me of a similar situation with KIS: ihsoft/KIS#221/mockingbirdnest/Principia#1549; see in particular https://github.com/ihsoft/KIS/issues/221#issuecomment-326824829.

LegoF4 commented 4 years ago

@eggrobin Would you mind explaining what the difference between a stock Vessel.parts and your Vessel.parts_ lists is? I ask because I have ascertained that the vessel contains at least one part before the crash, albeit one that is not started (I don't know what this means or what methods enact this).

Then, assuming I've understood correctly, the invariant in question here is that every unpacked vessel must have a rigid body. Is the problem here then that the vessel is "unpacking" (despite the call to HoldVesselUnpacked) before the parts within the newly formed vessel can be started?

I apologize to all of you for the stream of questions here - I respect your work on your respective plugins immensely and am taking this as a learning opportunity for myself.

taniwha commented 4 years ago

EL does not create a vessel with an empty parts list, but it does create and then destroy a vessel in the same frame, but this is when calculating the build cost, not at finalization and capture. However, that is not to say that the create/destroy is not related to the problem: it may be a delayed issue. The build cost is calculated when initially loading the craft configuration, either via "Select Craft" or "Reload", and when initiating the build. Thus, if it is the create/destroy, I'm surprised it didn't give Principia problems until finalization. Regardless, it's still a strong candidate as FAR used to have trouble with it. If I remember correctly, the solution was simply to check that the vessel was still valid before processing it and its parts. Perhaps rather than aborting when a vessel's parts list is empty, Principia should treat the vessel as having been delete (maybe keep an eye on it).

LegoF4 commented 4 years ago

@eggrobin Alright, this isn't looking exactly like the failure in KIS 221. When loading a one-part vessel, I can confirm that part.rb!= null, before the crash. I should also note that before the call to HoldVesselUnpacked, the craft is packed. (With a rigid body on one of its parts).

pleroy commented 4 years ago

@LegoF4: some additional traces might help shed light on the matter.

For context, it is important to understand that Principia has its own representation of vessels and parts, which may differ from the ones seen by KSP. At the point where it crashes, Principia is in the process of scanning its vessels (other than the asteroids there is only one, named Launcher) to remove the parts that are about to be destroyed by KSP. In this instance, it turns out that it removes all parts because it thinks that they play no role in the physics. This leaves us with a (Principia) vessel with no (Principia) parts, hence the failure.

One thing to check is if this part is physically reasonable. The predicate we use is:

    return part.rb != null &&
           !IsNaN(part.rb.position) &&
           !IsNaN(part.rb.velocity) &&
           !IsNaN(part.rb.angularVelocity);

I suggest that you add to your traces (near the place where you detect an "unstarted" part) an indication of whether rb is null or not, and if it is not, that you trace the position, velocity and angularVelocity.

Another thing that is noticeable is that we determine that the vessel is on the launch pad and skip some processing (this is necessary in stock). After the point where the vessel is made active it would be interesting to trace the following:

FlightGlobals.ActiveVessel?.situation
FlightGlobals.ActiveVessel?.orbitDriver?.lastMode
FlightGlobals.ActiveVessel?.orbitDriver?.updateMode
LegoF4 commented 4 years ago

@pleroy I can't seem to replicate the positive rb existence result (i.e. it is null). That said, from the moment the vessel is switched to active vessel, the following are invariant: FlightGlobals.ActiveVessel?.situation = PRELAUNCH FlightGlobals.ActiveVessel?.orbitDriver?.lastMode = TRACK_Phys FlightGlobals.ActiveVessel?.orbitDriver?.updateMode = IDLE

Also, I've updated both the log files and the copy of BuildControl with the latest round of traces, here. Note the CaptureCraft and the BuildAndLaunchCraft methods.

taniwha commented 4 years ago

@LegoF4 Hmm, could that be the source of trouble? I wrote the capture code before I had any understanding of KSP's orbit related code and I remember thrashing around a lot until I got things working. Outside the context of Principia, the situation doesn't seem to matter too much as the vessel is soon destroyed by the coupling process.

I strongly suspect I am missing a few steps that aren't necessary (or have since changed) for standard pad launches as when I release a craft, the crew portraits are missing until I switch to a different craft (mother ship) and back. The reason I haven't looked into that one is I've generally been on the lookout for higher priority issues, bu I have come to wonder if these might be somehow related.

LegoF4 commented 4 years ago

@taniwha That would make sense. Could you point me to where I can find the KSP source code, and more specifically, the launch code?

taniwha commented 4 years ago

ShipConstruct(tion) has most of the logic, but things seem to start in FlightDriver (look for PutShipToGround), But as far as I can tell, all the vessel related stuff is in ShipConsruction.PutShipToGround and ShipConstruction.AssembleForLaunch.

LegoF4 commented 4 years ago

@taniwha Where can I look up the details of those? Also, @eggrobin @pleroy it might be helpful to realize that AT_Utils's version of ship spawning is also incompatible with Principia. What stock method creates that rigid body which you need in a newly spawned craft? Alternatively, could Principia manage without that invariant, maybe as a unit point mass?

taniwha commented 4 years ago

@LegoF4 they are in Assembly-CSharp.dll.

taniwha commented 4 years ago

@LegoF4 @eggrobin @pleroy what is the status of this one?

taniwha commented 2 years ago

Please see if 6.99.1 improves the situation: commits e2e4ed1b75b49c71d176319664d9c3e348ef0086 and d1b10df5497efba44a28e55f7f466c995c4f9e5f seem to me to be highly relevant to this issue.

taniwha commented 2 years ago

I've heard nothing for a while. If this is not fixed, feel free to file a new report or reopen this one.