gotmachine
commented
1 year ago Gave a quick look at this, and pinging @R-T-B for awareness.
Indeed, the code does clamp the minAirPressureToOpen field in the editor, but fails to do so in flight, this is an easy fix.
However, note that from a config perspective, minAirPressureToOpen only define the default value of the field.
The minimum pressure is actually defined by setting the clampMinAirPressure field.
Also, saw the convo on the Koperncius discord, you also talked about another issue, ie, chutes deploying when they reach the altitude threshold even if they haven't reached the min pressure threshold. This is fixable as well.
All this being said, there is a notable argument against doing any of those fixes. clampMinAirPressure is a static pressure threshold, making it unreliable and pretty much meaningless. What should condition if a chute can or cannot deploy is the dynamic pressure (ie, air density combined with air speed).
In that context, I would advise against doing those changes. I can see how it can be somewhat exploited, but the reverse is also true, there are cases where chutes should deploy and taking advantage of those (weird) behaviors can help.
In my opinion, what is needed is a mod that revamp the chutes deployment behavior, using dynamic pressure thresholds instead of static, ideally with 3 user-facing configurable values :
- Semi-deployment pressure (clamped to min pressure, defaults to min pressure)
- Full-deployment pressure (clamped to min pressure, defaults to 1 atm)
- Full-deployment altitude With full deployment happening only if min pressure is reached, and at the pressure or altitude threshold, whichever happens first.
I can see such a mod being part of KSPCF, however I think it should be optional (ideally activated from the in-game options), so maybe releasing it as a standalone mod will offer better visibility, and avoid bloating the KSPCF codebase with things that aren't strictly fixes.
For reference, here is an (untested) patch for the two "issues" :
internal class ChuteMinPressure : BasePatch
{
protected override Version VersionMin => new Version(1, 10, 0);
protected override void ApplyPatches(List<PatchInfo> patches)
{
patches.Add(new PatchInfo(
PatchMethodType.Prefix,
AccessTools.Method(typeof(ModuleParachute), nameof(ModuleParachute.OnStart)),
this));
patches.Add(new PatchInfo(
PatchMethodType.Prefix,
AccessTools.Method(typeof(ModuleParachute), nameof(ModuleParachute.ShouldDeploy)),
this));
}
// fix the "Min Pressure" field being allowed to be set lower than the clampMinAirPressure configured value in the flight PAW
private static void ModuleParachute_OnStart_Prefix(ModuleParachute __instance)
{
if (__instance.minAirPressureToOpen < __instance.clampMinAirPressure)
__instance.minAirPressureToOpen = __instance.clampMinAirPressure;
((UI_FloatRange)__instance.Fields[nameof(ModuleParachute.minAirPressureToOpen)].uiControlFlight).minValue = __instance.clampMinAirPressure;
}
// prevent deployment if static pressure is lower than clampMinAirPressure, even if the altitude threshold is reached
private static bool ModuleParachute_ShouldDeploy_Prefix(ModuleParachute __instance, ref bool __result)
{
if (FlightGlobals.getStaticPressure(__instance.part.transform.position, __instance.vessel.mainBody) * ModuleParachute.pressureRecip > __instance.minAirPressureToOpen)
{
__result = false;
return false;
}
return true;
}
}
R-T-B
OhioBob
NathanKell
I have a request for a bug fix related to parachutes. Each parachute has a minAirPressureToOpen setting in its config. As the name implies, this is the minimum ambient air pressure required for a parachute to semi-deploy. It is generally set to 0.04 atm for main parachutes and 0.02 atm for drogue parachutes.
In game, the player can right-click on a parachute and set the minimum semi-deployment pressure via a slider. While in the editor (VAB or SPH), the slider will not go below the minAirPressureToOpen as defined in the part’s config. This is what we should expect. However, once a vehicle is sent to the launch pad, or while in flight, the slider we get when right-clicking on a parachute allows the pressure to be set all the way down to 0.01 atm. This shouldn’t be. At no time should we be able to set the pressure to less than the minAirPressureToOpen. I request this be corrected as it is clearly unintended behavior.
I have spoken with R-T-B about this issue and he is willing to add a fix for it in Kopernicus, but he would like to give you first opportunity should you think KSPCF is a better place to handle it. Either way, we need to keep R-T-B informed so he knows how to proceed. Thank you.