pioneerspacesim / pioneer

A game of lonely space adventure
https://pioneerspacesim.net
1.63k stars 377 forks source link

Fuel Use Rate #3950

Closed mike-f1 closed 7 years ago

mike-f1 commented 7 years ago

Observed behaviour

In ShipType.cpp (and not only there):

float ShipType::GetFuelUseRate() const { const float denominator = fuelTankMass effectiveExhaustVelocity 10; return denominator > 0 ? -linThrust[THRUSTER_FORWARD]/denominator : 1e9; }

Expected behaviour

I think (as stated here: https://en.wikipedia.org/wiki/Specific_impulse#General_definition) the right equation should avoid "fuelTankMass" and "*10":

float ShipType::GetFuelUseRate() const { const float denominator = effectiveExhaustVelocity; return denominator > 0 ? -linThrust[THRUSTER_FORWARD]/denominator : 1e9; }

...or am I wrong?

fluffyfreak commented 7 years ago

@johnbartholomew would you be able to clarify on this one? I see you're name on the Git Blame :)

johnbartholomew commented 7 years ago

Just because I touched the code doesn't mean I wrote the code :-)

My guess is that the units of these values are different to what you would expect.

laarmen commented 7 years ago

Looks that way to me, too. It's like fuel consumption in cars can be expressed in "km/full tank" instead of more correct "m/L".

Le ven. 17 mars 2017 à 16:50, John Bartholomew notifications@github.com a écrit :

Just because I touched the code doesn't mean I wrote the code :-)

My guess is that the units of these values are different to what you would expect.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pioneerspacesim/pioneer/issues/3950#issuecomment-287392398, or mute the thread https://github.com/notifications/unsubscribe-auth/AAkONUgrt_02lA6j-TBxV3WdC2Uwpqj7ks5rmqvRgaJpZM4Mgd4T .

mike-f1 commented 7 years ago

But then in Propulsion.cpp the method UpdateFuel don't works as expected :/ And these together leads to more fuel consuption than expected if m_fuelTankMass>10

About units, here you need a mass flow [Kg/sec], but you get a [Kg^2/sec] that don't have any meaning...

So what's now?

We could change both places, and put on the first the lines I wrote above and on the second (UpdateFuel) remove the 0.01 factor.

ps: eev is Km/sec, right?

bszlrd commented 7 years ago

As far as I know, eev is in m/s.

johnbartholomew commented 7 years ago

The only place I can find that calls GetFuelUseRate() is ShipType.cpp where it's initialising all the ship definition data to make it available to Lua. The value is stored as "thrusterFuelUse", and the doc string for that is "Ship thruster efficiency as a percentage-of-tank-used per second of thrust."

If that's correct then I think it would be: thrust_N = effective_exhaust_velocity_m_s mass_flow_rate_kg_s thrust_N = effective_exhaust_velocity_m_s (tank_mass_tonnes 1000) (fuel_use_percent / 100) thrust_N = effective_exhaust_velocity_m_s tank_mass_tonnes fuel_use_percent 10 fuel_use_percent = thrust_N / (effective_exhaust_velocity_m_s tank_mass_tonnes * 10)

So, the code matches the comment (or I've made another mistake). Quite crazy code and difficult to understand, but not actually "incorrect" :-)

Also I went through the history some more. I believe the relevant code was added here: 60e51241a679680c7d14d9f357a2d7fc5478adf7 (which was a rebase/history fix up from irigi's commit 4ba3f5ba00beca72888846ca175148238ffe9265, from PR #1689)

johnbartholomew commented 7 years ago

Uh, sorry minor mistake: I meant the only place I found that calls GetFuelUseRate is in LuaShipDef.cpp, here: https://github.com/pioneerspacesim/pioneer/blob/master/src/LuaShipDef.cpp#L252

mike-f1 commented 7 years ago

Ok, I think I got it: so it is "pre-multiplied" because it is used in GetSpeedReachedWithFuel, and here m_thrusterFuel is a fraction of 1...

I'm sorry for this, but I think I will add some comments or I change something to make it more understandable.

Thank's

mike-f1 commented 7 years ago

Reason for my doubts are here: #3952... Suggestion are welcome :)

mike-f1 commented 7 years ago

...Ok: I think I can close this, thank you for your support