ShotgunNinja / Kerbalism

Hundreds of Kerbals were killed in the making of this mod.
The Unlicense
43 stars 19 forks source link

UpgradesUIextensions Causes NREs on crew pods #86

Closed KSAMissionCtrl closed 7 years ago

KSAMissionCtrl commented 7 years ago

The new mod UpgradesUIextensions, when installed alongside Kerbalism, causes all pods with crew capability to fire off an NRE when the cursor hovers over them in the VAB

NullReferenceException: Object reference not set to an instance of an object   
at KERBALISM.Configure.Specs () [0x00000] in <filename unknown>:0    
at KERBALISM.Configure.GetInfo () [0x00000] in <filename unknown>:0    
at UpgradesUIExtensions.PatchEditorTooltip.Update () [0x00000] in <filename unknown>:0

output_log.txt

gotmachine commented 7 years ago

Can't investigate right now, but this is most likely an issue on my side. I'm calling modules GetInfo() from a hidden/disabled part, and tried to replicate the same state as the part prefab (GetInfo() is normally called on prefabs). Looks like I failed... Its probably because of one of these :

ShotgunNinja commented 7 years ago

@gotmachine

Code in modules assume that GetInfo() is called during part compilation, on the KSP loading screen. At that time:

Now, most modules don't do anything fancy or use complex data in GetInfo(), and/or have no OnLoad() defined at all. So there will be no issue with how you are doing the part tooltip regeneration.

However, if a module does any of the above, you will get undefined behaviour in their GetInfo(). Such is the case for the Configure module of this mod. There will be probably other cases as well in other mods.

Ideally then you only need to call OnLoad() on each module before regenerating the part tooltip. However, you would also need to somehow set HighLogic.LoadedScene to GameScenes.LOADING to guarantee the exact context that modules expect.

A simpler and maybe better solution would be to wrap the GetInfo() calls you are doing on part tooltip regeneration in a try/catch statement, and skip those that throw exceptions. In that way, in the rare case mentioned above the only effect would be the missing info about that module in the tooltip. Note that this is okay to do only because GetInfo() implementations are all state-less (due to the general constrains in which they are called, explained at the start of the post). So you can assume they are exception-safe (meaning: there is no side effect when an exception is thrown at some point in their execution).