With FAR installed SHIP:TERMVELOCITY puts infinity to the stack.

[ghost]

While the ship is in motion and in the atmosphere SHIP:TERMVELOCITY returns infinity. It doesn't do this without FAR installed.

[Dunbaratu]

Terminal velocity only returns infinity when your altitude is higher than the body's real atmosphere cutoff altitude based on the calculations that KSP itself uses, which is:

-body.atmosphereScaleHeight_1000_Math.Log(1e-6);

So I'm going to guess that FAR is messing with the body's atmosphereScaleHeight and thus throwing this calculation off.

Is there a way we can directly ask the KSP API "is there air here?" instead of running the calculation ourselves? Perhaps whatever FAR is doing to KSP would be reflected in such an API call.

[ghost]

When the rocket is sitting at the launch pad SHIP:TERMVELOCITY outputs 908 or so. I don't know if Fer ram Aerospace Research is changing Kerbins atmosphereScaleHeight though? If you need anything else just ask.

[Dunbaratu]

I haven't used FAR so I have to ask. Does it tend to make the atmosphere behave as if it was a bit thinner when close to the surface? By Default KSP makes the atmosphere VERY thick when at low altitude. I could easily imagine that they've changed up the formula for atmosphere thickness in FAR.

In which case, the fact that we're following KSP's default formula rather than FAR's formula would be the problem. The only solution I can imagine working would be if we could query KSP directly for the atmosphere data so that whatever FAR is doing gets exposed to kOS too.

[ddproxy]

MechJeb had the same issue, they were supposed to write a shim to integrate FARs aerodynamics into their calculations. May be worth a look to see if they did that yet. On Jul 7, 2014 7:04 PM, "Steven Mading" notifications@github.com wrote:

I haven't used FAR so I have to ask. Does it tend to make the atmosphere behave as if it was a bit thinner when close to the surface? By Default KSP makes the atmosphere VERY thick when at low altitude. I could easily imagine that they've changed up the formula for atmosphere thickness in FAR.

In which case, the fact that we're following KSP's default formula rather than FAR's formula would be the problem. The only solution I can imagine working would be if we could query KSP directly for the atmosphere data so that whatever FAR is doing gets exposed to kOS too.

— Reply to this email directly or view it on GitHub https://github.com/KSP-KOS/KOS/issues/188#issuecomment-48257015.

[ghost]

From locking at @sarbian's patch for mechjeb it seems that FAR adds in its own terminal velocity. Considering FAR uses so many different variables like drag, I guess its possible to have kOS check if FAR is installed and when SHIP:TERMVELOCITY is called, it just outputs FAR's terminal velocity variable(which if I am correct is TerminalVelocityFAR).

[Dunbaratu]

I'm always in favor of having the least possible special cases for mods, just because with the plethora of mods out there the total amount of code needed for mods making exceptions for other mods would quickly grow to an N factorial level - the classic direct connection networking formula.

It's better to have mods work with each other via one unified way.

[ddproxy]

I agree in principal. However, since FAR is such an overhaul of this system, I would like to argue that this special case would merit a special check.

Just my two cents. On Jul 7, 2014 8:33 PM, "Steven Mading" notifications@github.com wrote:

I'm always in favor of having the least possible special cases for mods, just because with the plethora of mods out there the total amount of code needed for mods making exceptions for other mods would quickly grow to an N factorial level - the classic direct connection networking formula.

It's better to have mods work with each other via one unified way.

— Reply to this email directly or view it on GitHub https://github.com/KSP-KOS/KOS/issues/188#issuecomment-48262395.

[north1]

FAR is such an essential mod for so many people, myself included. The stock aerodynamics are absolutely atrocious. And yes, FAR does make the atmosphere feel quite a lot thinner. It's also thins exponentially rather than linearly like in the stock model.

I would not be surprised if, somewhere between 50 and 70km (just guessing, haven't tested this) it is so close to zero (but not zero!) that rounding errors are creating the div-by-zero (infinity) in the terminal velocity calculation.

So I think this should be fixed, but I should also make a note that Ferram himself has said that you should never need to throttle back below terminal velocity if you're launching a properly aerodynamic rocket, which is what I assume you're trying to do, @nuclearphoenixegg.

[Dunbaratu]

The claim that the stock game thins the atmosphere linearly is utterly false. In the stock game it follows a natural log curve.

The reason for the infinity is that in the stock game the atmosphere is rounded down to zero when it falls below 0.000001 ATM's (since otherwise technically atmospheres never actually end, they just get thinner and thinner and thinner until they're just as thin as the "atmosphere" of the solar system itself, to save on wasting time making calculations that are so small they won't make much difference, the game just ignores atmosphere entirely at altitudes higher than that which would cause it to be less than 0.000001 ATM's ) What must be happening is that with FAR changing the calculation curve, the altitude at which it falls below 0.000001 ATMs is not the same.)

When the stock game says the atmosphere ends at 70km, it's lying to you. It actually ends at 69,078m, as thats the point where it falls below 0.000001 ATM's. FAR is probably making it fall below 0.000001 ATM's at a different altitude.

At any rate the solution is NOT to make special exceptions for FAR but to find a way to query the game itself and ask it what the atmosphere is at the current altitude, rather than making the same calculation ourselves. When we make the same calculation the stock game makes we open ourselves up to a problem if the stock game changes the formula in a future release. Presumably what FAR is doing will cause the query to change the answer because it's altering the body's atmo stats.

[ghost]

@north1 I was using terminal velocity for some space plane testing that I could not do my self with the same amount of precision and consistency.

[ghost]

I just had a bit of extra time and found out that it only returns infinity if you are using a liquid fuel oxidizer engine it works fine with jet engines.(haven't tested ion engines) (I actually found this out by accident.) :P

[teleksterling]

@nuclearphoenixegg is this at all altitudes? Even immediately after takeoff?

[Dunbaratu]

That sounds like a bad bug in FAR -I mean how much air there is is not a function of what engine you have. Is it somehow using the jet's air intakes as a measuring device?

[sarbian]

... Maybe you should look into your code and how FAR works before saying it's a bug in FAR ? FAR replace the stock drag and lift system of KSP. To avoid KSP stock drag interference FAR set the drag of all parts to 0. Now if you look into your code you ll see that you divide by the "Mass Drag" to get the term velocity, which will always be equal to 0 when FAR is running.

[ghost]

@teleksterling It is at all altitudes even if I land it on the ground but with the exact same craft with the jet engines active and the liquid fuel oxidizer engines inactive it out puts about 900 at group level and goes up like it should.

[erendrake]

@sarbian agreed, with FAR being such an overhaul of the aerodynamic model im sure there will be fun API changes we just need to mask from scripters. Thanks for the input :)

[sarbian]

@erendrake Let me just add how I solved it in MJ. I use a delegate to get the terminal velocity, and a separate dll that links both MJ and FAR points that delegate to a new function when FAR is here.

[erendrake]

@sarbian that sounds pretty groovy ill have to steal it :)

[Dunbaratu]

Okay so maybe it was harsh to call it a bug in FAR because when FAR is used entirely on its own, this isn't a bug. But the fact that it causes bugs when used with other mods is not the fault of those other mods. It's not their fault that FAR is implemented in a way that falsifies the data coming from the KSP API. FAR is, effectively, causing an API change. "Other mods can alter the KSP API however they like and we'll accommodate those changes" sounds like a dangerous precedent to set given how many mods there are. The reason I'm nervous is I see the potential N factorial problem surfacing if we adopt that policy.

[erendrake]

@Dunbaratu you are right, its technical dept that FAR is forcing on us which is a bummer. Its not a "bug" in FAR and not a "bug" in kOS.

At least there isnt an outright lie in the drag field :)

[Dunbaratu]

Are you willing to set the precedent that we'll support any mods' alteration of the API in general, or is it just FAR that gets that exception because its big and popular?

[erendrake]

@Dunbaratu Any mod? I dont think so, not as a blanket statement. Mostly because I dont think we will ever have the bandwidth for it.

I am likely going to be friendly towards mods that add something really special to the game and ones that i use in my saves like FAR or RealChutes. That being said if the mod changes the API in a way that doesnt make sense or is unnecessary i would rather send that developer a pull request than patch it on our end.

There are really so many mods that i want scripters to have access to in kOS that we should sit down and develop a more sensible API.

[Dunbaratu]

Well I suppose the reason I am reluctant is that I have no interest in installing FAR myself, so it becomes a question of "Am I going to have to start installing lots of mods I don't use just to test how kOS works with all the other mods?" That creates an enormous testing burden. I'm very reluctant to set the precedent that if one mod does something unorthodox to the API that breaks kOS, that we'll treat it as being kOS's problem to fix it. Once its done for one mod, that opens it up for other people to make the argument that their favorite mod should get similar treatment.

If I make an edit to the code and submit it, I don't want to start getting bug reports back claiming that my code has a bug in need of fixing because it didn't account for a mod I don't have installed.

[erendrake]

@Dunbaratu it is a tricky situation, we will just have to take it one mod at a time :)

[dcrockwell]

+1 for FAR support in kOS. It is extremely popular to the point that it warrants special support.

Perhaps if there were just a way to get atmospheric density at sea level from KerboScript? Then there would be no special support needed and we could do our own calculations based upon this, scale height, and atmospheric height.

[Dunbaratu]

-1 to FAR support in kOS. In addition to my usual reasons that FAR is being a bad citizen when it breaks the API and demands that other mods change to do things its way, there's also the fact that we have no idea yet what to expect when KSP 1.0 changes the stock aero system. Whatever that does is going to require a change anyway and maybe invalidate anything we do this close to its release.

[gillonba]

Also -1 to FAR support. I would be inclined to agree with dcrockwell based on FAR's popularity, with Squad reworking the aero model there is a good chance that FAR will soon become much less relevant in the near future. If that is the case, effort spent supporting it will have been wasted effort that could have been better spent elsewhere

[dcrockwell]

with Squad reworking the aero model there is a good chance that FAR will soon become much less relevant in the near future.

I was unaware. I think you're right. -1 FAR support.

[futrtrubl]

I don't think FAR is going anywhere. While the stock aero system is becoming more intuitive it is still not nearly as realistic as FAR. For one I don't believe it changes drag due to mach effects. Also I don't think it sets a precedent as FAR is a huge change to the system so it really had to change the underlying behavior. So I think special support is warranted and I believe just a matter of getting the terminal velocity straight from FAR.

[erendrake]

@futrtrubl I dont think FAR is going anywhere either but i would advise letting the smoke clear from the 1.0 release before we continue with this.

[ferram4]

I fail to see how FAR is being a bad citizen by breaking a non-existent API; it's not like there's a "vesselDrag" parameter that I can set. In fact, it doesn't falsify the data at all; you're calculating the terminal velocity based on the drag of the stock massdrag system and getting the expected result when that is disabled with FAR.

I have the option of trying to provide support by applying drag forces through the massdrag system, but given that it adds a fair bit more calculations to apply forces I question whether the overhead is necessary.

I will also note that even attempting that will result in unintended behavior in FAR when handling physicsless (massless) parts that shouldn't be; this means that taking steps to make FAR compatible with the massdrag-assuming scheme you're using will result in parts like the 3.75m decoupler having no drag forces whatsoever. Obviously that's bad, so the next step would be to attempt to apply forces to the nearest physics-having part, but adjust things to keep it behaving properly.

So I don't know what you guys want me to do short of filling FAR with workarounds. I can add any function you guys want to make interfacing easier, but trying to fix it by fitting FAR into the massdrag assumptions that you're working on will inevitably cause bugs / require convoluted workarounds in FAR.

Edit: That said, we'll have to see how the KSP aero update handles things. I somewhat doubt that it will provide an easy way to read off total drag or whatever, but we can hope for something.

[Dunbaratu]

Keep in mind that the bug report here wasn't "Installing FAR caused the stock terminal velocity value to be wrong" - THAT would be expected, and fine. The bug report here is that installing FAR made the script crash (because the drag was changed from 0.02 to zero.)

That's where I'm coming from. I'm not talking about the work that would be necessary to return FAR's terminal velocity - of course that would have to be a case of making special exception code for FAR if we were to try doing that. I'm talking about the work that is necessary to keep FAR's changes to the values returned by the stock API from crashing our math.

In order to suppress stock KSP's aerodynamics, FAR found a very roundabout way to do it - it does it by changing the stock API's values such that the sum of all p.maximum_drag's for an entire vessel will always be zero. That cannot happen in stock. Although there are some stock parts that don't interact with physics and don't have drag, at least one part on a stock vessel is going to have to be something with drag - the probe core or command pod at the very least. Thus the assumption that's correct with the numbers you get in a stock install becomes incorrect with FAR installed, and it's a change that caused a crash.

Solving it generically for all mods (treating them like equal citizens and not giving special status to FAR) is nearly impossible due to the the fact that the generic solution is to say that we can never assume anything about any properties or fields coming from the KSP API that are used in a formula. ANY of them could be changed into something utterly unexpected by another mod, so protect against potential bogus values for all of them. <-- that would be unworkably messy.

Thus my claim that fixing this means giving special exceptions to FAR, and I don't feel comfortable with having to do that, but I can see I'm getting outvoted on that.

But, given that the reason FAR is resorting to falsifying the stock values for the parts is to find a roundabout way to zero out the effects of stock aero calculations, and KSP 1.0 seems like it might be providing a better way to accomplish that (a flag to turn off stock aero without having to do that to the part data), that's all the more reason not to try to fix this now. If 1.0 gives Ferram a better way to do this, the entire problem may go away on its own and whatever we do NOW would be irrelevant.

@erendrake, There's also place in our code where an Infinity is being deliberately put on the stack to report an error condition back to the script, file KOS/Utilities/VesselUtils.cs, line 359. That should be changed to a maxdouble instead. I don't know how old that code is but I suspect it predates marianoapp's stack-forbids-infinity change from ages ago.

[hvacengi]

As we have deprecated the suffix TERMINALVELOCITY, I'd say that the original "bug" is at least gone. Do we want to continue this discussion as an attempt to access the data from FAR directly?

[blnk2007]

Looks like FAR is still relevant.

[dcrockwell]

Indeed ;)

[Dunbaratu]

The claim was never "far will be irrelevant", but that supporting the old FAR code would be irrelevant because it cannot stay the same and still be compatible with 1.0. The FAR code had to have a major overhaul - anything we did to support what it was doing pre-1.0 would have been wasted effort to throw away and start over. Post-1.0 FAR is not the same animal as pre-1.0 FAR was.

[blnk2007]

What's the status on this now?

[erendrake]

@blnk2007 As far as i know. "Terminal velocity" is not a very useful stat for either stock or FAR. Which is why i deprecated it. I have seen quite a few people asking about a dynamic pressure binding but i dont have the maths on hand to add it.

[sarbian]

vessel.dynamicPressurekPa * 1000 ?

[erendrake]

@sarbian perfect :) I havent spent very much quality time with the stock aero part of the API so i didnt know we had that available. Thank you!

[blnk2007]

Awesome! Now if only MechJeb could add this! it could be used instead of "Limit Terminal Velocity". Maybe?

[hvacengi]

Since this was converted into the dynamic pressure suffix, and terminal velocity itself has been abandoned, I'm going to close this issue. We can reopen it if more discussion is required.