Sigma88 / Sigma-Dimensions

The Universal Rescale Mod for KSP
Other
24 stars 7 forks source link

Improper rescaling of homeworld atmospheres #103

Open CashnipLeaf opened 2 months ago

CashnipLeaf commented 2 months ago

Relevant lines: https://github.com/Sigma88/Sigma-Dimensions/blob/2ff65ad2dbd1dccc3c1510aa69c2a93df7bdea34/%5BSource%5D/SigmaDimensions/Dimensions/AtmosphereTopLayer.cs#L44 https://github.com/Sigma88/Sigma-Dimensions/blob/2ff65ad2dbd1dccc3c1510aa69c2a93df7bdea34/%5BSource%5D/SigmaDimensions/Dimensions/AtmosphereTopLayer.cs#L71

If a celestial body has the internal name "Kerbin" (which the homeworld of a system replacer would likely have) and has an atmosphere, then Sig Dim sets its sea level pressure to a hard-coded value of 101.325 kPa (equal to stock Kerbin's sea level pressure), regardless of what the sea level pressure for that homeworld as defined by its atmospherePressureCurve is, while leaving the rest of the body's atmosphere pressure curve untouched. This can lead to the following behaviors:

Fig. 1: Kcalbeloh System's Efil at 2.5x rescale, showing the normal sea level pressure of 75.9938kPa. https://media.discordapp.net/attachments/939249946027245587/1232387089719365683/image.png?ex=6629456a&is=6627f3ea&hm=024e89cd57550b2c8c224db1f71feef8489ca01b21275c2068ffbb6661e1fec0&=&format=webp&quality=lossless&width=1202&height=676

Fig. 2: Kcalbeloh System's Efil at 2.5x rescale, this time with the planet pack's homeswitch setting set to "Efil", which sets Efil's internal name to "Kerbin". Its sea-level pressure has been changed by Sig Dim to the aforementioned hard-coded value of 101.325kPa. https://media.discordapp.net/attachments/939249946027245587/1232390962517381251/image.png?ex=66294906&is=6627f786&hm=2ad8d0abc8594abbbe4e62adcefb0f73e671cf7587b47f2c05e9f76c74a3e1c4&=&format=webp&quality=lossless&width=1200&height=676

Sigma88 commented 1 month ago

tbh I don't recall exactly why this was added, I remember it was necessary at the time but I can't remember why

I will take a look and let you know if I find the original reason for adding it, this way it might be easier to decide how to procede now

Sigma88 commented 1 month ago

was easier to find than I expected:

https://github.com/Sigma88/Sigma-Dimensions/commit/a4d2231c6176ed4dd0d4af842cfb1f1f2a6fcce8#diff-d1ce8890c4ad0ff0bdfbe987bf44d4536ee74284b7a9bc77b91dd01cb768af84L137-L149

in version 10.7 I removed the ISP fix from the .cfg files but forgot to remove those 2 lines you highlighted from the plugin

those can be safely be removed at this point

CashnipLeaf commented 1 month ago

I removed the offending lines as part of the adoption by R-T-B. Thanks for following up, though.