ferram4 / Ferram-Aerospace-Research

Aerodynamics model for Kerbal Space Program
Other
238 stars 131 forks source link

KSPWheel compatibility #165

Closed AkiraR closed 7 years ago

AkiraR commented 7 years ago

New mod by Shadowmage that adds a physics based wheel collider is not playing nicely with FAR at the moment.

Repro:

Logs:

Here is a link to logs and some screenshots, for whatever reason imgur is not letting me upload anything :/ https://www.dropbox.com/sh/07oyi1h8mh7m1ph/AACd0z5YT1cUxF2CvJ-_cd4Ka?dl=0

Link to issue on KSPWheel git https://github.com/shadowmage45/KSPWheel/issues/6

shadowmage45 commented 7 years ago

Just to add -- if Ferram is not aware of anything off the top of his head that would cause conflicts, I'm more than willing to dig in to do some debugging to find where the error is (which mod / what function / etc).

It could well be that there is something in KSP wheel that I'm doing that is causing the problem (but does not effect stock drag code); but I'm at a loss as to what it could be, as none of the modules do anything with physics except for the wheel-collider itself (which merely uses rigidbody.addforce() ).

blowfishpro commented 7 years ago

(which merely uses rigidbody.addforce() )

I doubt this affects FAR, but isn't everything supposed to use part.AddForce now?

shadowmage45 commented 7 years ago

NathanKell informed me that 'collision forces' can still use rigidBody.AddForce(); this would include anything wheel related; it was the first question I asked when they first revealed the part.addforce methods.

Edit: Add link to post ( http://forum.kerbalspaceprogram.com/index.php?/topic/146533-devnote-thursday-tweaking-and-turning-gears/&do=findComment&comment=2729643 )

(Note that the stock/Unity wheels do not use the part.addforce method) (Also note that there is no way to link the wheel-collider class to use the part.addforce methods without creating a KSP dependency in the wheel-collider, removing posibility of use for other games/projects).

Edit: Would be an interesting test to comment out the wheel-collider rigidbody force code in KSPWheel and see if that 'fixes' the problem. (I do not have access to FAR dev versions though, so this will have to be done by someone else)

ferram4 commented 7 years ago

The "addForce" minutiae isn't affecting any of this. Likely the deal is that either one of the colliders on the part or one of the meshes on the part is either not closed (unlikely to cause an issue like this, I think) or is somehow creating a voxel that is not contiguous, which is normally the result of some faces / verts floating around off in space somewhere, as was happening with some VSR parts. I'll take a look at it, but I think there's a good chance this requires fixes on the model side as most of these tend to do.

shadowmage45 commented 7 years ago

Hmm.. that seems to be the likely cause (disconnected colliders).

Specifically I add a sphere collider to the models at runtime to prevent suspension overcompression (bump-stop collider); this collider will often not be contiguous with the rest of the model, as it is most often 'dangling' where the wheels fully-compressed position would be.

Is there a specific tag or layer I can set these colliders to that would inform FAR to not include them in the voxelization process? (Or some other manner to tell FAR to ignore specific transforms, either config or API solutions would work)

shadowmage45 commented 7 years ago

After some quite lengthy investigation I have narrowed this down to being caused by one specific code-related action. (has nothing to do with the run-time added 'bump-stop' colliders as I had thought)

If I set the part.collider field to be null, voxelization fails.

I am currently intentionally setting the field to null, otherwise stock code has a tendency to randomly explode parts when they are anywhere close to the ground and moving at reasonable speeds (not sure if it is still necessary, this was back in KSP 1.1.3, have not investigated in 1.2+).

The field can also 'accidentally' become null when removing 'bounds' colliders or 'collisionEnhancer' colliders from legacy parts at run-time that also happen to be currently assigned to part.collider (which then becomes null). (lots of the wheel part models I'm working with are only available in .mu format from prior KSP versions, and so cannot be edited further without very substantial work in re-rigging; so run-time manipulation is the only efficient option available for fixing the models)

The only place in FAR code that I see part.collider field referenced is here: https://github.com/ferram4/Ferram-Aerospace-Research/blob/ea259af5989ee0826b18c346cc748741672c81f5/FerramAerospaceResearch/FARPartGeometry/GeometryPartModule.cs#L192 but I'm unsure why it is being examined.... (there must be a reason, however I cannot think of anything specifically aero or drag related; all that field does in stock code is tell the part what single collider can trigger explosions)

What options do I have to allow for setting of the part.collider field to null? When can this safely be done that it will not break voxelization?

I am also going to investigate whether it is still necessary for me to null that field; perhaps the KSP code that was exploding things erroneously has been fixed and it is no longer explicitly needed.

I can confirm however that if I ensure that (part.collider != null) then voxelization works, and lift/drag/aero data is properly displayed both in-editor and in-flight. This appears to be the entirety of the problem.

ferram4 commented 7 years ago

The purpose of examining part.collider != null is that often the mesh exists before the colliders are set; without that FAR will often grab the meshes to voxelize (on the basis that they are much larger than the 0-bound-size non-existent colliders) adding much more work when the colliders will do.

Functionally, allowing part.collider to become null one FixedUpdate after the colliders are set should fix the issue, unless part.collider == null is something that's saved for the Part and will cause problems on a vessel reload.

If you've got a better way to determine when all the colliders have been created, then I'll use that method, but currently this has to remain or the number of math calculations that FAR needs to do risks becoming untenable.

On another note, if you can list the name of the transform that your sphere collider uses in the newly-created FARPartModuleTransformExceptions.cfg that should ensure that the collider isn't voxelized. I can either add it on my end, or you should be able to set it using a MM config on yours.

shadowmage45 commented 7 years ago

Understood and thanks for the explanation; I was thinking that examination was likely a check for if things had initialized, it was about all that made sense with it being referenced in only that one spot, I just wasn't seeing it from that little bit of context in the code.

I'm going to do some investigation on if that bit of hackery is even still needed in the KSPWheels code. It was originally a workaround to collision problems that I was encountering. It may have been fixed in stock code updates, or I may have fixed it in other ways and forgotten to remove the original hack... (I think the explosion problem I was having I may have been fixed by adjusting the CollisionEnhancer settings for the part)

On the note of bounds colliders removal (which need to be removed before the first physics update, and can also cause the part.collider to be null); being aware of the problem it can cause I can specifically check the part.collider field and reassign it to a valid collider if needed.

Collider exceptions list -- I'll make up a quick MM patch to ship with KSPWheels if needed. These colliders are only added in the flight scene, and only after physics has started so they should not be present for the voxelization. Good to know either way though as I may have a few parts in other mods that could use exclusions for specific plugin-created meshes.

Thanks again for the info and explanations. You can probably consider this issue to be solved; I can fix it all fairly easily on the KSPWheels end now that I'm aware what is going on.

ferram4 commented 7 years ago

Collider exceptions list -- I'll make up a quick MM patch to ship with KSPWheels if needed. These colliders are only added in the flight scene, and only after physics has started so they should not be present for the voxelization.

In an ideal world, that would be true. However, depending on what other PartModules are on the Part or if any other mod decides to interact with FAR it is possible for there to be a situation where FAR rebuilds the collider and mesh information it needs for voxelization from scratch. I don't believe there are any current bits of code in anyone's mods that would cause that but the code is there to cause that issue should anyone decide to make use of it.

And also, as you say you can address it, I'll close this.