dkavolis / Ferram-Aerospace-Research

Aerodynamics model for Kerbal Space Program
Other
80 stars 32 forks source link

Bad mod interaction with ReStock #26

Closed Zuthal closed 5 years ago

Zuthal commented 5 years ago

As mentioned here, having both FAR and ReStock installed causes certain engines overhauled by the ReStock mod to have much more drag than they should have.

I myself experienced this when building a spaceplane, with the Rapier engine causing the plane to have about six and a half times the maximum cross-section area and about sixty times the Mach 1 wave drag area than with a Whiplash engine installed in its place.

drewcassidy commented 5 years ago

This is mainly caused by FAR not supporting skinned meshes or parts with inconsistent scaling. The latter could possibly be fixed on Restock’s end, but skinned meshes can not, not without significantly changing the way the engines are rigged

dkavolis commented 5 years ago

Sorry, I'm not very good at this but can you explain how skinned meshes differ from every other mesh used in KSP? If I knew how they were handled it would provide a starting point in resolving this issue.

drewcassidy commented 5 years ago

Skinned meshes use bones to determine the position of different vertices. My guess is that FAR is using the “rest” position of the skinned mesh vertices, which is not where the mesh is necessarily oriented when using the bones.

Unfortunately there doesn’t seem to be an easy way to get the “result” mesh deformed by the bones in the Unity Mesh class, which might make this tricky if you’re just reading the vertices directly

EDIT: looks like you can use SkinnedMeshRenderer.BakeMesh() to get the deformed mesh, which returns just a regular mesh you can send to the voxelization function

dkavolis commented 5 years ago

Would this do the trick https://docs.unity3d.com/ScriptReference/SkinnedMeshRenderer.BakeMesh.html?

drewcassidy commented 5 years ago

Yeah sorry couldn’t remember if that was in 2017.1, turns out I was just looking in the wrong place. Just call that on the skinned mesh renderers you encounter in the compiled part to get a “normal” mesh

dkavolis commented 5 years ago

I'll have a try later this evening if I find some time.

blowfishpro commented 5 years ago

It looks like FAR is already baking SMRs. I think the problem is that when it gets baked hierarchy scale gets baked into that, whereas for regular mesh renderers, it grabs the shared mesh and applies hierarchy scale later. So in effect scale gets applied twice.

blowfishpro commented 5 years ago

I think there may be another problem in that FAR doesn't re-voxelize on part variant changes.

dkavolis commented 5 years ago

Variant changes are another issue. I had a look at ModulePartVariants API but there doesn't seem to be a way to set up a callback on variant change unless I'm missing something.

DMagic1 commented 5 years ago

You could try GameEvents.onVariantApplied.

dkavolis commented 5 years ago

https://github.com/dkavolis/Ferram-Aerospace-Research/blob/f602aff3ff7ba28db9bb5cd6cae5c507fe783dde/FerramAerospaceResearch/FARPartGeometry/GeometryPartModule.cs#L653-L692

@blowfishpro is right, FAR already handles skinned meshes. @drewcassidy can you try forcing voxelization to be based on mesh with

@PART[<Part>]:AFTER[FerramAerospaceResearch]
{
    @MODULE[GeometryPartModule]
    {
        %forceUseMeshes = True
    }
}

for affected <Part>s?

dkavolis commented 5 years ago

I think there may be another problem in that FAR doesn't re-voxelize on part variant changes.

@blowfishpro FAR already handles variant changes by default, GameEvents.onEditorShipModified gets triggered by any part field/slider change so there is no need to connect to GameEvents.onVariantApplied or GameEvents.onEditorVariantApplied.

blowfishpro commented 5 years ago

Okay, glad variants are working.

I think we actually want forceUseColliders - FAR is currently using the visible mesh (with the messed up scaling)

dkavolis commented 5 years ago

Try either of those, if you replace FAR dll and assets bundle with ones from the repo you won't get performance drop with debug voxels. It should make debugging less painful.

dkavolis commented 5 years ago

Forcing to voxelize based on colliders works better though voxelization doesn't completely match the visual mesh

screenshot32

dkavolis commented 5 years ago

@drewcassidy @blowfishpro I've narrowed down the cause to https://github.com/dkavolis/Ferram-Aerospace-Research/blob/f602aff3ff7ba28db9bb5cd6cae5c507fe783dde/FerramAerospaceResearch/FARPartGeometry/GeometryMesh.cs#L80 The meshTransform.localToWorldMatrix (obtained from part.PartModelTransformList()) is garbage for skinned meshes. If you know how to get the correct transform, the fix would be easy.

Zuthal commented 5 years ago

Forcing to voxelize based on colliders works better though voxelization doesn't completely match the visual mesh

Still much better than the broken mesh-based voxelisation though. Might be worth publishing a hotfix that just forces collider-based voxelisation for all parts affected by the issue, if a proper fix is going to take some time to develop.

Zuthal commented 5 years ago

I have compiled a list of engines that are affected by this issue:

List of broken engines:

I have also made a MM config that should force use colliders on all these engines.

https://pastebin.com/d9XUZ5c2

blowfishpro commented 5 years ago

@dkavolis that makes sense based on the research I've seen. In effect, scale and rotation are getting applied twice, though I'm less sure about position. Would probably need to mess around with it a bit. But you could try just not applying any transform, or just the position transform, and see how that goes.

drewcassidy commented 5 years ago

you could also manually un-apply the transformation as you read through the mesh

dkavolis commented 5 years ago

I'm just testing a potential fix by building a TRS matrix with a scale of 1, seems to work. screenshot33 From the meshes, the pipes seem to be skinned mesh and voxelization is applied correctly. Will try no rotation next to see what happens.

dkavolis commented 5 years ago

Okay, rotation is needed so only the scale was applied twice. Only need to clean up the code now. screenshot34

dkavolis commented 5 years ago

Fixed by #30