Fix aerodynamic torque simulations

[BenChung]

This pull request fixes Ferram Aerospace's simulations for torque and force, as well as adding new API endpoints that allow access to real-time force and torque information. Additionally, it refactors the CalculateAeroForces system to reduce code duplication, though it could be refactored further to avoid the lambda and associated generic method invocation overhead.

[BenChung]

@dkavolis Should I open issues for the problems that this PR fixes? I realize in retrospect that I'm fixing one issue and adding a new feature (current aerodynamic torque) in one PR, which may not work well for you.

[dkavolis]

Thanks for the PR, sorry it took so long to have a look at it. Separate PRs would make more sense, I just have a few comments:

1. In FarCenterQuery I would rename pos to origin, keep GetMinTorquePos and TorqueClipFactor (even though they are unused, they might be useful in the future so might as well) and modify TorqueAt to compensate for non-zero origin. I would also provide a default constructor which sets origin to zero.
2. In PhysicsCalcs.cs the totalAeroForceVector and totalAeroTorqueVector could be combined into a single FarCenterQuery, maybe add a Reset(Vector3d origin) method to it and call it before iterating.
3. In FARAPI.cs a lot of the methods have counterparts for active vessel, I would add similar ones. Also, all methods should handle cases when VesselFlightInfo returns a null instead of throwing an NRE. Something like VesselFlightInfo(v)?.InfoParameters.aerodynamicForce ?? 0;. 0 default is in line with other FARAPI methods.
4. In FARAeroSection.cs while the changes make code easier to maintain, the anonymous functions also generate garbage. It may not be important in the editor but I would like to keep the generated garbage to a minimum in flight mode for performance reasons.

[BenChung]

In FarCenterQuery I would keep[...] GetMinTorquePos and TorqueClipFactor (even though they are unused, they might be useful in the future so might as well)

If I recall correctly, FARCenterQuery was making a serious mathematical error that was causing results to be fairly erroneous. However, I need to make sure that I was correct in my original analysis, so let me get back to you on this one.

I would also provide a default constructor which sets origin to zero.

I'm not sure this makes sense; FARCenterQuery is fundamentally asking what the force is around a given point. It makes more sense to me to require that that point be specified rather than setting it to 0 in whatever frame of reference you happen to be using. This is more a design issue, however.

In FARAeroSection.cs while the changes make code easier to maintain, the anonymous functions also generate garbage.

Abstracting over the means of force application is going to require a level of indirection, and it's not clear to me how to do it without allocation. The alternatives that I can come up with are:

• Adding two classes and an interface that provides a consistent interface for force application by AeroSections.
• Accumulating applied forces into a buffer for application by the caller.

I'll make the other changes soon.

[dkavolis]

If I recall correctly, FARCenterQuery was making a serious mathematical error that was causing results to be fairly erroneous. However, I need to make sure that I was correct in my original analysis, so let me get back to you on this one.

I think GetMinTorquePos(Vector3d origin) is correct. At the minimum point, residual torque must be parallel to the force because it can't be obtained from cross product and the comment says is closest to origin. That is the offset to origin to obtain the minimum point is perpendicular to force since all locations of minimum torque are on a line parallel to the force. So the offset is simply Vector3d.Cross(force, TorqueAt(origin)) / fmag. Not sure about TorqueClipFactor.

I'm not sure this makes sense; FARCenterQuery is fundamentally asking what the force is around a given point. It makes more sense to me to require that that point be specified rather than setting it to 0 in whatever frame of reference you happen to be using. This is more a design issue, however.

With the TorqueAt method the default constructor wouldn't make a difference, only a convenient way to initialize FARCenterQuery

Abstracting over the means of force application is going to require a level of indirection, and it's not clear to me how to do it without allocation. The alternatives that I can come up with are:

• Adding two classes and an interface that provides a consistent interface for force application by AeroSections.
• Accumulating applied forces into a buffer for application by the caller.

I think the first alternative would be the easiest and most extendable in the future if needed

[BenChung]

I worked on it more, and FARCenterQuery was actually correct despite what I thought at the time; I've reverted the changes locally.

The bad news is that I've been validating it again and the simulation forces are still broken; I thought I had fixed it, but apparently not. Going to be a bit longer, sadly.

[BenChung]

Update: torque is now applied consistently in the right direction, and total forces are consistently within 10% of actual values. Going to try to get this lower still though.

[BenChung]

The remaining 10% of error is due to issues in calculating local atmospheric density due to not knowing the time of day and therefore the solar-induced atmospheric heating. I'm going to call it good here, and address the usability/stability concerns from here on out.

[BenChung]

I think that's everything; @dkavolis can you please take a look again?

[dkavolis]

Mostly good, just a couple of comments:

1. The new API calls should have similarly named methods for active vessel that take no arguments to make them consistent with other vessel information getters
2. Is there a particular reason why FARCenterQuery and contexts are initialized to null? Why not just initialize them once in the constructor (or constructor equivalent) and skip null checks altogether?

[BenChung]

@dkavolis I think that's everything?

Mostly good, just a couple of comments:

1. The new API calls should have similarly named methods for active vessel that take no arguments to make them consistent with other vessel information getters

Yep, done, apologies for forgetting this originally.

2. Is there a particular reason why FARCenterQuery and contexts are initialized to null? Why not just initialize them once in the constructor (or constructor equivalent) and skip null checks altogether?

It didn't seem to work for mysterious reasons the first time I tried it. I did an inline initalizer and it's fine?

[BenChung]

Apologies, missed those; is that sufficient?

[dkavolis]

Looks like everything is done now, thanks.