dkavolis / Ferram-Aerospace-Research

Aerodynamics model for Kerbal Space Program
Other
80 stars 32 forks source link

Fix various NREs. Calculate vessel orientation without using the NavBall, which doesn't exist in IVA mode. #133

Closed BrettRyland closed 2 years ago

BrettRyland commented 2 years ago

This fixes the massive performance penalty (see profiling pics) and failure to update the vessel orientation (which likely causes many other issues) in IVA mode caused by a call to FindObjectOfType<NavBall>() (which doesn't exist in IVA mode) in GetNavball (which is being called every FixedUpdate since it doesn't find it). It replaces the NavBall code with explicit calculations of the heading, pitch and roll (which I've checked to be consistent with the values from NavBall to around single precision when not in IVA mode).

FAR - FindObjectOfType in GetNavBall in IVA view FAR-FindObjectsOfType

Additionally, this fixes various NullReferenceExceptions related to EVA kerbals' parachutes and parts that have been destroyed.

BrettRyland commented 2 years ago

Found another NRE, this time caused by

Dictionary<Part, VoxelCrossSection.SideAreaValues> partSideAreas =
    crossSections[m].partSideAreaValues;
partSideAreas.Clear();
[[LOG 17:01:04.321] B9.Aero.Wing.Procedural.TypeB Exploded!! - blast awesomeness: 0.1
[LOG 17:01:04.343] [FAR v0.16.0.5]: Updating vessel voxel for Reverse Mosquito Mk Vd_1 Debris
[ERR 17:01:04.348] [FAR v0.16.0.5]: Voxel Volume was infinity; ending voxelization
        UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogFormat(LogType, Object, String, Object[])
        FerramAerospaceResearch.FARLogHandler:Log(LogLevel, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:40)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:81)
[LOG 17:01:04.349] [FAR v0.16.0.5]: Voxel Element CrossSection Area: 0
EXC 17:01:04.350] [FAR v0.16.0.5]: Logged exception:
        UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogFormat(LogType, Object, String, Object[])
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:77)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
[EXC 17:01:04.352] NullReferenceException: Object reference not set to an instance of an object
        FerramAerospaceResearch.FARPartGeometry.VehicleVoxel.CrossSectionData (FerramAerospaceResearch.FARPartGeometry.VoxelCrossSection[] crossSections, UnityEngine.Vector3 orientationVector, System.Int32& frontIndex, System.Int32& backIndex, System.Double& sectionThickness, System.Double& maxCrossSectionArea) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARPartGeometry/VehicleVoxel.cs:461)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CalculateVesselAeroProperties () (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:1018)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CreateVoxel (Vessel vessel) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:403)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:78)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
dkavolis commented 2 years ago

I don't have any ideas why the last NRE is there. crossSections is always created by https://github.com/dkavolis/Ferram-Aerospace-Research/blob/95e127ae140b4be9699da8783d24dd8db726d753/FerramAerospaceResearch/FARPartGeometry/VehicleVoxel.cs#L95-L106 used in https://github.com/dkavolis/Ferram-Aerospace-Research/blob/95e127ae140b4be9699da8783d24dd8db726d753/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs#L398

BrettRyland commented 2 years ago

Yet another exception appeared:

[EXC 19:41:43.748] [FAR v0.16.0.5]: Logged exception:
        UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogFormat(LogType, Object, String, Object[])
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:77)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
[EXC 19:41:43.751] KeyNotFoundException: The given key was not present in the dictionary.
        System.Collections.Generic.Dictionary`2[TKey,TValue].get_Item (TKey key) (at <9577ac7a62ef43179789031239ba8798>:0)
        FerramAerospaceResearch.FARAeroComponents.FARAeroSection.UpdateAeroSection (System.Single potentialFlowNormalForce, System.Single viscCrossflowDrag, System.Single diameter, System.Single flatnessRatio, System.Single hypersonicMomentForward, System.Single hypersonicMomentBackward, UnityEngine.Vector3 centroidWorldSpace, UnityEngine.Vector3 xRefVectorWorldSpace, UnityEngine.Vector3 nRefVectorWorldSpace, UnityEngine.Matrix4x4 vesselToWorldMatrix, UnityEngine.Vector3 vehicleMainAxis, System.Collections.Generic.List`1[T] moduleList, System.Collections.Generic.List`1[T] dragFactor, System.Collections.Generic.Dictionary`2[TKey,TValue] partWorldToLocalMatrixDict) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/FARAeroSection.cs:164)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CalculateVesselAeroProperties () (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:1485)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CreateVoxel (Vessel vessel) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:403)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:78)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)

I think it needs a

if (!partWorldToLocalMatrixDict.ContainsKey(data.aeroModule.part))
                    continue;

between lines 163 and 164 of FARAeroSection.cs like in the block above it (lines 146-153).

dkavolis commented 2 years ago

Good catch, but instead of using Contains use TryGetValue to avoid double lookup, and do the same for the block above.

BrettRyland commented 2 years ago

I don't have any ideas why the last NRE is there. crossSections is always created by

https://github.com/dkavolis/Ferram-Aerospace-Research/blob/95e127ae140b4be9699da8783d24dd8db726d753/FerramAerospaceResearch/FARPartGeometry/VehicleVoxel.cs#L95-L106

Ah, I think I see what's going on here. EmptyCrossSectionArray is called from CreateVoxel if _vehicleCrossSection.Length < _voxel.MaxArrayLength, but VehicleVoxel.CreateNewVoxel returns early due to "Voxel Volume was infinity; ending voxelization" so _voxel.MaxArrayLength is 0 and the if condition is never satisfied. However, _vehicleCrossSection is created with size 1 initially on line 74 of VehicleAeroDynamics.cs, so if it doesn't get updated before the first use, the 0th index will be null and sectionCount from

int sectionCount = yCellLength + (int)(xCellLength * x / y + 1) + (int)(zCellLength * z / y + 1);
sectionCount = Math.Min(sectionCount, crossSections.Length);

on lines 428-429 of VehicleVoxel.cs allows this first index to be accessed.

Line 398 of VehicleAerodynamics.cs seems to be the only place where _vehicleCrossSection is assigned. There probably needs to be an else condition to that if so that it's not reusing old values (or a null value on the first use) in the crossSections array on line 459-461.

dkavolis commented 2 years ago

Changing https://github.com/dkavolis/Ferram-Aerospace-Research/blob/95e127ae140b4be9699da8783d24dd8db726d753/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs#L74 to Array.Empty<VoxelCrossSection>() should work, it's the only place where the contained dict is not created.

BrettRyland commented 2 years ago

Changing

https://github.com/dkavolis/Ferram-Aerospace-Research/blob/95e127ae140b4be9699da8783d24dd8db726d753/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs#L74

to Array.Empty<VoxelCrossSection>() should work, it's the only place where the contained dict is not created.

That would avoid the null, but what about the case when _voxelis smaller than _vehicleCrossSection in

_voxel = VehicleVoxel.CreateNewVoxel(_currentGeoModules, _voxelCount, vessel: vessel);
if (_vehicleCrossSection.Length < _voxel.MaxArrayLength)
    _vehicleCrossSection = _voxel.EmptyCrossSectionArray;

? CalculateVesselAeroProperties would then be running on an old _vehicleCrossSection.

dkavolis commented 2 years ago

I think it's still better than not using anything, at least the old value was valid whereas the new one would blow up the physics due to infinities.

BrettRyland commented 2 years ago

OK, I think I've fixed all the above points now. The only thing is whether you want to try again to get a quaternion approach to the vessel orientation in PhysicsCalcs.cs or just use the vector algebra.

In any case, I'm going to head to bed now. I'll run some more tests tomorrow (SuicidalInsanity is running a new round of BAD-T using FAR, which is the reason I started looking at this in the first place).

BrettRyland commented 2 years ago

Another exception came up:

[LOG 12:24:36.552] [FAR v0.16.0.5]: Updating vessel voxel for CA-15 Kangaroo Debris
[LOG 12:24:36.568] miniFuselage Exploded!! - blast awesomeness: 0.5
[LOG 12:24:36.631] [FAR v0.16.0.5]: Voxel Element CrossSection Area: 0.0149669817358196
[LOG 12:24:36.631] [FAR v0.16.0.5]: Std dev for smoothing: 3 voxel total vol: 0.366210960783051 filled vol: 1.36691105828926
[EXC 12:24:36.633] [FAR v0.16.0.5]: Logged exception:
  UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])  
  ModuleManager.UnityLogHandle.InterceptLogHandler:LogFormat(LogType, Object, String, Object[])  
  FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:77)  
  FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
[EXC 12:24:36.636] IndexOutOfRangeException: Index was outside the bounds of the array.
  FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CalculateSonicPressure (FerramAerospaceResearch.FARPartGeometry.VoxelCrossSection[] vehicleCrossSection, System.Int32 front, System.Int32 back, System.Double sectionThickness) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:1695)  
  FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CalculateVesselAeroProperties () (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:1092)  
  FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CreateVoxel (Vessel vessel) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:403)  
  UnityEngine.DebugLogHandler:LogException(Exception, Object)  
  ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)  
  FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:78)  
  FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
[WRN 12:24:36.696] PlayOneShot was called with a null AudioClip.

This appears to stem from using a forward finite difference to approximate the first derivative on line 1695 of VehicleAerodynamics.cs, where the i + 1 is going beyond the end of the vehicleCrossSection array if back remains at crossSections.Length - 1 from line 410 of VehicleVoxel.cs. You should probably be using a backward finite difference (using i - 1 and i instead of i and i + 1) for the back index. Similarly for line 1654.

BrettRyland commented 2 years ago

And another exception:

[EXC 14:54:23.213] [FAR v0.16.0.5]: Logged exception:
        UnityEngine.DebugLogHandler:LogFormat(LogType, Object, String, Object[])
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogFormat(LogType, Object, String, Object[])
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:77)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)
[EXC 14:54:23.231] ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
        System.ThrowHelper.ThrowArgumentOutOfRangeException (System.ExceptionArgument argument, System.ExceptionResource resource) (at <9577ac7a62ef43179789031239ba8798>:0)
        System.ThrowHelper.ThrowArgumentOutOfRangeException () (at <9577ac7a62ef43179789031239ba8798>:0)
        FerramAerospaceResearch.FARAeroComponents.FARAeroSection.MergeAeroSection (FerramAerospaceResearch.FARAeroComponents.FARAeroSection otherSection) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/FARAeroSection.cs:283)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CalculateVesselAeroProperties () (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:1503)
        FerramAerospaceResearch.FARAeroComponents.VehicleAerodynamics.CreateVoxel (Vessel vessel) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs:403)
        UnityEngine.DebugLogHandler:LogException(Exception, Object)
        ModuleManager.UnityLogHandle.InterceptLogHandler:LogException(Exception, Object)
        FerramAerospaceResearch.FARLogHandler:LogException(Exception, String, Object) (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch.Base/Utils/FARLogHandler.cs:78)
        FerramAerospaceResearch.FARThreading.ThreadSafeDebugLogger:Update() (at /home/brett/github/Ferram-Aerospace-Research/FerramAerospaceResearch/FARThreading/ThreadSafeDebugLogger.cs:79)

I'm not sure how this one occurs. The only potentially questionable indexing is partData[index] where index is found from handledAeroModulesIndexDict.TryGetValue(tmpOtherData.aeroModule, out int index), but the only way I can see that causing a bad index is if partData and handledAeroModulesIndexDict aren't in sync.

dkavolis commented 2 years ago

This appears to stem from using a forward finite difference to approximate the first derivative on line 1695 of VehicleAerodynamics.cs, where the i + 1 is going beyond the end of the vehicleCrossSection array if back remains at crossSections.Length - 1 from line 410 of VehicleVoxel.cs. You should probably be using a backward finite difference (using i - 1 and i instead of i and i + 1) for the back index. Similarly for line 1654.

You're right, even the two branches are identical.

I'm not sure how this one occurs. The only potentially questionable indexing is partData[index] where index is found from handledAeroModulesIndexDict.TryGetValue(tmpOtherData.aeroModule, out int index), but the only way I can see that causing a bad index is if partData and handledAeroModulesIndexDict aren't in sync.

They look to be updated in the same places so they should always be synced unless there was an exception between updating the two.

dkavolis commented 2 years ago

Are you going to try and fix the last NRE? If not, I'll merge the PR.

BrettRyland commented 2 years ago

Are you going to try and fix the last NRE? If not, I'll merge the PR.

Probably not. I've only seen it once, so it must be fairly rare and I don't really see why it occurred. I'll make a new PR if I find other things to fix. :+1:

dkavolis commented 2 years ago

Thanks!