KSPModdingLibs / KSPCommunityFixes

Community patches for bugs in the KSP codebase.
https://github.com/KSPModdingLibs/KSPCommunityFixes/releases/latest
57 stars 19 forks source link

Attitude control overhaul #118

Open gotmachine opened 1 year ago

gotmachine commented 1 year ago

High level overview

4 new patches related to attitude control stuff :

Related changes :

New modding patch : BaseFieldListUseFieldHost, allow BaseField and related features (PAW controls, persistence...) to work when a custom BaseField is added to a BaseFieldList (ie, a Part or PartModule) with a host instance other than the BaseFieldList owner. Allow to dynamically add fields defined in other classes to a Part or PartModule. This patch is used to implement extra fields for the RCSLimiter patch.

TODO

Possibly :

In depth patches documentation

GetPotentialTorqueFixes

This patch is a rewrite of the stock implementations for ITorqueProvider.GetPotentialTorque(out Vector3 pos, out Vector3 neg). All 4 of the stock implementations have various issues and are generally giving unreliable (not to say plain wrong) results. Those issues (and reimplementation details) are further commented in code, but to summarize :

Note that the KSPCF GetPotentialTorque() implementations for ModuleControlSurface and especially for ModuleGimbal are more computationally intensive that the stock ones. Profiling a stock Dynawing with RCS enabled during ascent show a ~30% degradation when summing the vessel total available torque (~250 frames median : 0.31ms vs 0.24ms, frame time : 1.81% vs 1.46% ). Overall this feels acceptable, but this is still is a non-negligible impact that will likely be noticeable in some cases (ie, atmospheric flight with a large vessel having many gimbaling engines and control surfaces). The implementations are pretty naive and could probably be vastly optimized by someone with a better understanding than me of the underlying math and physics. To mitigate the general overhead of those methods, a caching mechanism is implemented for the most performance intensive modules (gimbals and control surface). The general idea is to avoid recomputing the available torque unless there is a significant change in input parameters. Limited profiling show a significant reduction of the average per-frame cost, well below the stock level.

The KSPCF implementations follow these conventions :

So in the context of the KSPCF patch, a correct implementation of a GetVesselPotentialTorque() method is :

 foreach (ITorqueProvider torqueProvider)
 {
   torqueProvider.GetPotentialTorque(out Vector3 pos, out Vector3 neg);
   vesselPosTorque += pos;
   vesselNegTorque += neg;
 }
 if (vesselPosTorque.x < 0f) vesselPosTorque.x = 0f;
 if (vesselPosTorque.y < 0f) vesselPosTorque.y = 0f;
 if (vesselPosTorque.z < 0f) vesselPosTorque.z = 0f;
 if (vesselNegTorque.x < 0f) vesselNegTorque.x = 0f;
 if (vesselNegTorque.y < 0f) vesselNegTorque.y = 0f;
 if (vesselNegTorque.z < 0f) vesselNegTorque.z = 0f;

Review of how the stock implementations are handled in the modding ecosystem

NoLiftInSpace

This is straightforward patch : it prevent ModuleControlSurface.FixedUpdate() and ModuleLiftingSurface.FixedUpdate() from running when the part is neither in atmo or submerged, preventing some expensive calculations from running and pointless actuation from happening. It also take care of putting control surface to their neutral position for visual consistency.

RCSLimiter

This patch implements two extra tweakables in the RCS module "Actuation Toggles" Part Action Window.

By default, ModuleRCS will scale down the actuation level of each nozzle depending on how far the thrust force is from the "ideal" angle for a given actuation request (unless the "always full action" toggle is enabled).

This patch gives the ability to define a separate angle threshold for linear and rotation actuation. If the resulting angle from a nozzle thrust force is below that threshold, that nozzle won't fire at all instead of firing at a reduced level. This allow to optimize efficiency, especially in the case of multi-nozzle RCS parts that are impossible to fine-tune with only the actuation toggles.

The default angle limits can be defined in the ModuleRCS / ModuleRCSFX configuration by adding minRotationAlignement and minlinearAlignement fields (value in degrees). If they aren't defined, they default to 90° (no limit, behavior similar to stock).

To make RCS tweaking easier, the patch also add a potential torque/force readout to the actuation toggles PAW items. In the editor, the actuation orientation is defined by the first found command module, starting from the root part (matching the command module that will be selected as the control point when launching the ship). The readout also takes the RCS module ISP curve into account, and uses the currently selected body and state (sea level/altitude/vacuum) of the stock DeltaV app.

The modification to the RCS control scheme is taken into account by the custom KSPCF ModuleRCS.GetPotentialTorque() implementation. As of writing, all mods reimplement their own version of that method, and all of them are ignoring the stock control scheme anyway, so the behavior change introduced in this patch won't make a significant difference in most cases. Note that RCSBuildAid tries to simulate the stock control scheme, but its implementation doesn't reproduce stock behavior correctly, which is why its torque readout doesn't always match the KSPCF one.

BetterSAS

This patch implement a small tweak to the stock PID attitude controller in how it takes the GetPotentialTorque() results into account to switch from the acceleration to the neutral and coasting phases. This notably prevent it from massively overshooting its target orientation when the vessel has asymmetrical torque capabilities. An example of such a vessel is the stock Dynawing which has very asymmetrical RCS torque capabilities.

The patch also implement an alternative attitude controller more or less copypasted from the MechJeb implementation. The user can switch between controllers with an additional button on the command modules PAW. This PID controller is much more precise and very well suited for in-space operations, reducing RCS fuel consumption massively compared to the stock implementation. However, it can struggle to reach stability, especially when massive external forces are involved, which is usually the case in atmospheric flight situations.

BaseFieldListUseFieldHost

This patch allow BaseField and associated features (PAW controls, persistence, etc) to work when a custom BaseField is added to a BaseFieldList (ie, a Part or PartModule) with a host instance other than the BaseFieldList owner. This allow to dynamically add fields defined in another class to a Part or PartModule and to benefit from all the associated KSP sugar :

The whole thing seems actually designed with such a scenario in mind, but for some reason some BaseField and BaseFieldList methods are using the BaseFieldList.host instance instead of the BaseField.host instance (as for why BaseFieldList has a host at all, I've no idea and this seems to be a design oversight). There is little to no consistency in which host reference is used, they are even sometimes mixed in the same method. For example, BaseFieldList.Load() uses BaseFieldList.host in its main body, then calls BaseFieldList.SetOriginalValue() which is relying on BaseField.host.

Changing every place where a host reference is acquired to ensure the BaseField.host reference is used allow to use a custom host instance, and shouldn't result in any behavior change. This being said, the stock code can theoretically allow a plugin to instantiate a custom BaseField with a null host and have it kinda functional if that field is only used to SetValue() / Getvalue() and as long as the field isn't persistent and doesn't have any associated UI_Control. This feels like an extremely improbable scenario, so this is probably fine.