maneuver.ks mnv_time bugs and improvements

[dewiniaid]

I was adapting mnv_time for my own purposes and noticed a few things about it while making it fit my code style -- so here's a tiny bit of unsolicited code review

• The calculation of average Isp (ens_isp) is incorrect, as it is not weighted to factor in the thrust of the individual engines. The KSP wiki page elaborates on this, including an example of a correct average.

• The calculation of g is incorrect; it's reflecting g at sea level rather than at ship's current altitude. (Additionally, you can use ship:body rather than ship:orbit:body, and also body rather than ship:body). It should probably read:

  body:mu / (body:radius+ship:altitude)^2
  or
  body:mu / body:position:mag^2 // only correct if ship is the active vessel

  EDIT: This is all wrong, because you actually need the altitude where the maneuver node is being executed at, not the ship's current altitude.

• Multiplying f and m by 1000 is unnecessary; the two terms cancel.

• You can use ship:availablethrust rather than adding the available thrust from each engine.

• You can use skip copying engines to a second list and do your processing for average ISP in the first for loop using something like this.

FUNCTION Ship_AverageISP {
    // Returns current average ISP.
    PARAMETER ship IS ship.
    PARAMETER activeOnly IS TRUE.
    PARAMETER includeFlameOut IS FALSE.
    LOCAL engines.
    IF activeonly { SET engines TO Ship_ActiveEngines(ship, includeFlameOut). }
    ELSE { SET engines TO Ship_Engines(ship). }

    LOCAL n IS 0.  // Numerator
    LOCAL d IS 0.  // Denominator

    FOR engine IN engines {
        // might want a check against 0 Isp being reported.
        SET n TO n+engine:availablethrust.
        SET d TO d+(engine:availablethrust / engine:isp).
    }
    RETURN n/d.
}

[Archeagus]

Regarding your 0 ISP reporting comment: Does this version, as is, overcome the issue of having to stage to a new engine during the maneuver? I am using the original version but temporarily return 0.1 if an engine flames out and staging has to occur. (I.E. when isp or thrust = 0)

[dewiniaid]

It does not. To do that, you'd need to go through and figure out remaining dV in the current stage (and subsequent stages until the burn completes) and probably do some form of weighted average to correct for it.

That said, I imagine that most burns involving staging mid-burn are long enough that the resulting inaccuracy is minor (I.e. burning for an interplanetary transfer).

[TaggedYa]

First: The g that is used in this calculation is g sub 0, or g sub bullseye. It is a constant. It has a specific value that never changes equal to the acceleration of gravity at Earth mean sea level as it was accepted when the constant was fixed. It is 9.80665 meters/second^2. I believe that KSP uses 9.81 as the acceleration of gravity at Kerbins surface but that doesn't change the constant.

Second: For computing things with ISP you can use VISP which doesn't seem to go to 0 but returns an engine’s Vacuum ISP even if it isn't active.

Disclamer: I am playing with RSS/RO/RP0 so my experiance with vanilla is limited.

TaggedYa