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

BrettRyland · 2022-05-02T13:52:45Z

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).

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

…all (which doesn't exist in IVA mode).

dkavolis

Thanks for the PR. I've left some comments before merging.

FerramAerospaceResearch/RealChuteLite/RealChuteFAR.cs

FerramAerospaceResearch/LEGACYferram4/FARControllableSurface.cs

FerramAerospaceResearch/FARGUI/FARFlightGUI/PhysicsCalcs.cs

BrettRyland · 2022-05-02T15:09:37Z

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 · 2022-05-02T15:32:17Z

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

Ferram-Aerospace-Research/FerramAerospaceResearch/FARPartGeometry/VehicleVoxel.cs

Lines 95 to 106 in 95e127a

    
           public VoxelCrossSection[] EmptyCrossSectionArray 
        
           { 
        
               get 
        
               { 
        
                   var array = new VoxelCrossSection[MaxArrayLength]; 
        
                   for (int i = 0; i < array.Length; i++) 
        
                       array[i].partSideAreaValues = 
        
                           new Dictionary<Part, VoxelCrossSection.SideAreaValues>(ObjectReferenceEqualityComparer<Part> 
        
                                                                                      .Default); 
        
                   return array; 
        
               } 
        
           }

used in

Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs

Line 398 in 95e127a

_vehicleCrossSection = _voxel.EmptyCrossSectionArray;

BrettRyland · 2022-05-02T18:46:49Z

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 · 2022-05-02T18:53:56Z

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

BrettRyland · 2022-05-02T20:21:37Z

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

Ferram-Aerospace-Research/FerramAerospaceResearch/FARPartGeometry/VehicleVoxel.cs

Lines 95 to 106 in 95e127a

public VoxelCrossSection[] EmptyCrossSectionArray

{

get

{

var array = new VoxelCrossSection[MaxArrayLength];

for (int i = 0; i < array.Length; i++)

array[i].partSideAreaValues =

new Dictionary<Part, VoxelCrossSection.SideAreaValues>(ObjectReferenceEqualityComparer<Part>

.Default);

return array;

}

}

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.

…yGetValue.

dkavolis · 2022-05-02T21:13:36Z

Changing

Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs

Line 74 in 95e127a

private VoxelCrossSection[] _vehicleCrossSection = new VoxelCrossSection[1];

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

BrettRyland · 2022-05-02T21:25:08Z

Changing

Ferram-Aerospace-Research/FerramAerospaceResearch/FARAeroComponents/VehicleAerodynamics.cs

Line 74 in 95e127a

private VoxelCrossSection[] _vehicleCrossSection = new VoxelCrossSection[1];

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 · 2022-05-02T21:29:22Z

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.

…arts).

BrettRyland · 2022-05-02T23:44:55Z

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).

FerramAerospaceResearch/LEGACYferram4/FARControllableSurface.cs

BrettRyland · 2022-05-03T13:58:59Z

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 · 2022-05-03T14:03:31Z

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.

…re the initial CalculateSurfaceFunctions.

dkavolis · 2022-05-03T14:12:24Z

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.

…vative calculation in CalculateSonicPressure.

dkavolis · 2022-05-03T14:21:06Z

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

BrettRyland · 2022-05-03T14:23:56Z

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. 👍

dkavolis · 2022-05-03T14:27:53Z

Thanks!

Fix various NREs. Calculate vessel orientation without using the NavB…

8339147

…all (which doesn't exist in IVA mode).

BrettRyland changed the title ~~Fix various NREs. Calculate vessel orientation without using the NavB…~~ Fix various NREs. Calculate vessel orientation without using the NavBall, which doesn't exist in IVA mode. May 2, 2022

dkavolis reviewed May 2, 2022

View reviewed changes

FerramAerospaceResearch/RealChuteLite/RealChuteFAR.cs Outdated Show resolved Hide resolved

FerramAerospaceResearch/LEGACYferram4/FARControllableSurface.cs Outdated Show resolved Hide resolved

FerramAerospaceResearch/FARGUI/FARFlightGUI/PhysicsCalcs.cs Show resolved Hide resolved

Use the more elegant 'cap?.' instead of 'if (cap != null)'.

b47026d

Fix KeyNotFoundException in FARAeroSection.UpdateAeroSection using Tr…

df35aa7

…yGetValue.

BrettRyland added 2 commits May 2, 2022 23:57

Initialise _vehicleCrossSection as an empty array to avoid an NRE.

a5619dc

Update the ship parts list before iterating through it (avoids null p…

2474742

…arts).

Remove the now unneeded 'p == null' check.

5fc2b1d

dkavolis requested changes May 3, 2022

View reviewed changes

FerramAerospaceResearch/LEGACYferram4/FARControllableSurface.cs Outdated Show resolved Hide resolved

Call UpdateShipPartsList before invoking OnVesselPartsChange and befo…

e45e3b4

…re the initial CalculateSurfaceFunctions.

dkavolis approved these changes May 3, 2022

View reviewed changes

Use backward finite difference for the 'back' index of the first deri…

0c923d5

…vative calculation in CalculateSonicPressure.

dkavolis merged commit 008727e into dkavolis:master May 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

BrettRyland commented May 2, 2022

dkavolis left a comment

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022 •

edited

Loading

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

BrettRyland commented May 3, 2022

BrettRyland commented May 3, 2022

dkavolis commented May 3, 2022

dkavolis commented May 3, 2022

BrettRyland commented May 3, 2022

dkavolis commented May 3, 2022

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

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

Conversation

BrettRyland commented May 2, 2022

dkavolis left a comment

Choose a reason for hiding this comment

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022 • edited Loading

BrettRyland commented May 2, 2022

dkavolis commented May 2, 2022

BrettRyland commented May 2, 2022

BrettRyland commented May 3, 2022

BrettRyland commented May 3, 2022

dkavolis commented May 3, 2022

dkavolis commented May 3, 2022

BrettRyland commented May 3, 2022

dkavolis commented May 3, 2022

dkavolis commented May 2, 2022 •

edited

Loading