KerbalWeatherProject icon indicating copy to clipboard operation
KerbalWeatherProject copied to clipboard

FAR compatibility

Open dkavolis opened this issue 5 years ago • 4 comments

  • https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L97-L118 ModularFlightIntegrator allows multiple thermodynamics pre and post delegates and FAR only uses pre to calculate exposed and radiative areas so there's no reason not to register it if FAR was found. This way KWP will be mostly compatible with FAR without extra effort as it uses values from vessel for game simulation.

  • Check FARAtmosphere.cs, I've added additional delegates for pressure and temperature which would only be used for prediction. Different from previous versions, FARWind is now FARAtmosphere and its delegate has been changed to Func<CelestialBody, Part, Vector3, Vector3>. Other properties use Func<CelestialBody, Vector3d, double, double> delegates, where Vector3d is used for latitude, longitude and altitude and double - universal time (in case anything is time-dependent). Predictions are not yet wired to use latitude and longitude other than 0, 0 but may in the future.

  • In your UpdateThermodynamicsPre you have 2 calls to fi.BaseFIUpdateThermodynamics() which is not cheap: https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L461 https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L502 They are redundant as MFI will call it after thermodynamics pre. If some values depend on having UpdateThermodynamics completed, you can use thermodynamics post for that. Even if any mods override UpdateThermodynamics, they should still update the same values as stock one.

  • https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L544 https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L564 These should use calculated shock temperatures instead of using the same offset as stock. Stock FI uses Math.Max(this.atmosphericTemperature, this.CalculateShockTemperature()). Also, you update vessel.speedOfSound but not vessel.mach which introduces inconsistencies.

  • https://github.com/cmac994/KerbalWeatherProject/blob/3387f35ccff256832236813b35120be144cce99d/KerbalWeatherProject/ModularFI_Registerer.cs#L445-L453 You can break out of the loop early once you find the root part.

dkavolis avatar Jan 09 '21 06:01 dkavolis

Thanks for pointing inefficiencies in the code! I've corrected the calculation of external shock temperature and have added breaks to part loops. FYI: the vessel Mach number is updated at the end of UpdateAerodynamicsWx() (line 296 in ModularFI_Register.cs). The sound speed is updated during UpdateThermodynamicsPre()

The latest version of KWP (v.1.02) should now be fully compatible with FAR. I've updated the KWP code to register either FARWind or FARAtmosphere, depending on the version of FAR installed. Pressure, wind, and temperature functions have been added to facilitate the incorporation of ambient weather conditions from KWP into FAR.

  • https://github.com/cmac994/KerbalWeatherProject/blob/9091179b068be0f26b5b8e081989c4a9f2eb7de7/KerbalWeatherProject/KerbalWxClimo.cs#L99-L118

cmac994 avatar Jan 12 '21 11:01 cmac994

  • I think
var del1 = Delegate.CreateDelegate(WindFunction, this, typeof(KerbalWxClimo).GetMethod("GetTheWind"), true); // typeof(KerbalWxPoint).GetMethod("GetTheWindPoint"), true);

needs to be simply

WindFunction del1 = GetTheWind;

and similarly for del2 and del3 since Func<> is not a delegate. I got this when running FAR with latest KWP:

KerbalWeatherProject: unable to register with FerramAerospaceResearch. Exception thrown: System.ArgumentException: method arguments are incompatible
  at System.Delegate.CreateDelegate (System.Type type, System.Object firstArgument, System.Reflection.MethodInfo method, System.Boolean throwOnBindFailure, System.Boolean allowClosed) [0x002e3] in <ad04dee02e7e4a85a1299c7ee81c79f6>:0 
  at System.Delegate.CreateDelegate (System.Type type, System.Object firstArgument, System.Reflection.MethodInfo method, System.Boolean throwOnBindFailure) [0x00000] in <ad04dee02e7e4a85a1299c7ee81c79f6>:0 
  at KerbalWeatherProject.KerbalWxPoint.CheckFAR () [0x001d5] in <daccbb5c78b74b0c8d207aff8c879875>:0 

Also, if you don't use thermo you should reset FAR pressure and temperature overrides for consistency between game and predictions as the game will use values obtained directly from the vessel. You can reset overrides by passing null.

  • https://github.com/cmac994/KerbalWeatherProject/blob/9091179b068be0f26b5b8e081989c4a9f2eb7de7/KerbalWeatherProject/KerbalWxClimo.cs#L451-L476 These will be called by prediction routines in FAR so values cannot be cached, particularly latLonAltitude may not be equal to the active vessel position. The performance hit of recalculating those properties on every call is not as important as they should only be called once per simulation point.

  • https://github.com/cmac994/KerbalWeatherProject/blob/9091179b068be0f26b5b8e081989c4a9f2eb7de7/KerbalWeatherProject/ModularFI_Registerer.cs#L68-L72 Aerodynamics override should be disabled if FAR was found, otherwise FAR cannot register its own, MFI only allows one override.

dkavolis avatar Jan 12 '21 11:01 dkavolis

Thanks for noting the issue with delegate creation. In v1.0.3 I've updated KWP to reflect the changes you suggest above. I've also ensured that MFI will not call UpdateAerodynamicsOverride if FAR is present.

In KWP, if thermo is disabled, values for temperature/pressure will default to values from the base game atmosphere.

cmac994 avatar Jan 31 '21 01:01 cmac994

In-flight seems to be working fine now but there are still a few issues. Also, the KWP GUI shows all zeros except for wind and speed.

https://github.com/cmac994/KerbalWeatherProject/blob/d159b9124f25e25b29376fcd384574dc385bf463/KerbalWeatherProject/KerbalWxPoint.cs#L483-L508 https://github.com/cmac994/KerbalWeatherProject/blob/d159b9124f25e25b29376fcd384574dc385bf463/KerbalWeatherProject/KerbalWxClimo.cs#L451-L476

The input arguments are likely to be completely unrelated to the active vessel, so returning cached values breaks FAR simulation routines. The values need to be recomputed from the given arguments but to save some computational cost arguments can be compared to last computed ones to decide whether recomputation is needed or will cached values will work. Wind arguments are different and it's only called in flight so no change needed there.

Also, both KerbalWxClimo and KerbalWxPoint register with FARAtmosphere so one of them gets overwritten. Duplicate code can be removed.

dkavolis avatar Jan 31 '21 08:01 dkavolis