MuMech / MechJeb2

MechJeb2 - KSP mod
Other
992 stars 250 forks source link

Crew mass is not included in deltaV calculations #1157

Closed Genhis closed 4 years ago

Genhis commented 5 years ago

MechJeb Version

2.8.3.0

KSP Version

1.7

Description

I set kerbalCrewMass = 0.09375 in KSP physics file. KSP reports total mass correctly, both in VAB and during flight (MechJeb Delta-V Stats window doesn't). image

Replication Case

  1. Edit kerbalCrewMass to to a positive number.
  2. Launch a ship with at least one kerbal.
  3. Compare KSP and MJ stats and see the difference.
lamont-granquist commented 5 years ago

does the MJ vessel info window display the mass correctly and is vessel.totalMass coming from KSP at least correct?

it was suggested that in the FuelFlowSimulation that the mass of the crew needs to be accounted for manually but i have no idea if that is correct or not. e.g. something like:

if (part.ModuleCommand.crewCount > 0)
{
    crewMass = part.ModuleCommand.crewCount * kerbalCrewMass
    actualPartMass = part.mass + crewMass
}
lamont-granquist commented 5 years ago

cross reference on the prior bug:

https://bugs.kerbalspaceprogram.com/issues/14679

877

The bug report states "The KerbalCrewMass varaible was never implemented, so whatever behavior it has is unsupported anyway."

The suggested code is apparently from the 1.2.x modders notes) which i can't seem to find anywhere.

searching around for "kerbalCrewMass" as a keyword in google / kerbal forums hasn't turned up anything useful.

lamont-granquist commented 5 years ago

KER implemented a workaround before which is now broken:

https://github.com/jrbudda/KerbalEngineer/issues/35

lamont-granquist commented 5 years ago

In discussion on slack, it looks like KSP updates the part.mass in FixedUpdate to include the mass of the kerbals.

Either the FuelFlowSimulation bit that copies part data from the part is not being run in FixedUpdate, or this is another aspect of FuelFlowSimulation that I broke back in #1077 because we're accessing the part.mass directly in the thread.

So hacking up the FuelFlowSimulation to just add the part mass is not obviously the long term solution (although still could be if its determined that the root cause is the main thread bit runs before FixedUpdate and has to, then the kerbal just needs to get added manually to the part mass there).

lamont-granquist commented 4 years ago

Validated this is still a bug after the fuelflowsim fixes for 1.8 on current dev branch

lamont-granquist commented 4 years ago

Took another look at it and the total mass is correct, so the problem shouldn't be coming from VesselState.cs and as far as I can see the FuelFlowSimulation is running in FixedUpdate() so I'm not sure what could be patched.

Also the Kerbalism author has this to say about it:

// adding mass to a kerbal leads to all sorts of oddities in KSP.
// 1. the crew mass isn't accounted for in the editor immediately after adding/removing crew.
//    you have to make an additional change to the vessel for the numbers to add up.
// 2. a kerbal on EVA already has a mass of > 100kg in stock. the crew mass would be ADDED to that.
//    this would produce 95k of mass out of nothing for each EVA.
// it's convoluted, and I won't bother messing around with that any more.
//@PHYSICSGLOBALS:FOR[KerbalismDefault]
//{
//    %kerbalCrewMass = 0.09375
//}

Plus apparently heavy Kerbals drown.

lamont-granquist commented 4 years ago

So just hooking FixedUpdate() may run before or after KSP's FixedUpdate(). The way to assert running after KSP's FixedUpdate() is to hook into the script order:

https://kerbalspaceprogram.com/api/script_order.html

It looks like calling FixedUpdateAdd() with a timing stage of FashionablyLate or later will schedule it to run after the part.mass has been updated this happens in Part.FixedUpdate() which is not in the script order list explicitly but happens in the default order (0) in the order index.

The timings work out like:

ObscenelyEarly, = Timing0
        Early, = Timing1
        Precalc,
        Earlyish, = Timing2
        Normal, = 0
        FashionablyLate, = timing3
        FlightIntegrator, = TimingFI
        Late, = Timing4
        BetterLateThanNever = Timing5 (edited) 

(since we don't care at all it may be best to run it in BetterLateThanNever?)

lamont-granquist commented 4 years ago

Not going to fix this bug, kerbalCrewMass has been removed from RO:

https://github.com/KSP-RO/RealismOverhaul/pull/2290