Skip to content

Commit

Permalink
Ignore NREs from FlightGlobals.ActiveVessel (fixes #33)
Browse files Browse the repository at this point in the history
Happens during compilation of internal spaces according to @JonnyOThan
  • Loading branch information
HebaruSan committed Jun 24, 2024
1 parent 158f55d commit 5002506
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
5 changes: 5 additions & 0 deletions GameData/Astrogator/Astrogator-Changelog.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ KERBALCHANGELOG
version = 1.0.1
versionName = Vanish in black smoke
versionKSP = 1.12
CHANGE
{
change = Ignore NREs from FlightGlobals.ActiveVessel
type = Fix
}
}
VERSION
{
Expand Down
4 changes: 2 additions & 2 deletions Source/AstrogatorMenu.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public class AstrogatorMenu : InternalModule {
: base()
{
model = new AstrogationModel(
(ITargetable)FlightGlobals.ActiveVessel
?? (ITargetable)FlightGlobals.getMainBody());
DefaultIfThrows<ITargetable>(() => FlightGlobals.ActiveVessel)
?? FlightGlobals.getMainBody());
loader = new AstrogationLoadBehaviorette(model, null);
timeToWait = new List<DateTimeParts>();
cursorTransfer = 0;
Expand Down
19 changes: 19 additions & 0 deletions Source/KerbalTools.cs
Original file line number Diff line number Diff line change
Expand Up @@ -316,5 +316,24 @@ public static string FilePath(string filename, bool GameDataRelative = true)
}
}

/// <summary>Discard annoying exceptions</summary>
/// <param name="func">The function to evaluate</param>
/// <returns>
/// The return value of func
/// or the default of its return type (typically null)
/// if it throws an exception
/// </returns>
public static T DefaultIfThrows<T>(Func<T> func)
{
try
{
return func();
}
catch
{
return default;
}
}

}
}

6 comments on commit 5002506

@JonnyOThan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhh you probably just should have checked the current game scene

@HebaruSan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? What? Why?

@JonnyOThan
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flightglobals don’t exist during the loading scene. Allowing the exception to be thrown and just silently swallowing an exception is just…not great.

Doing logic inside constructors is also generally discouraged in unity. Awake or Start is better depending on what you need. There’s also a a few virtual methods on InternalModel but I’d have to go review the best way to use them.

@HebaruSan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My class is being instantiated at a time when it doesn't make sense for it to exist, and a simple accessor property that can be null is instead throwing an exception because it has a weird initialization dependency and its use probably wasn't thought through very deeply. A whole lot of things are "not great" here already.

@HebaruSan
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More importantly, FlightGlobals.getMainBody() apparently calls ActiveVessel internally, so the exception is probably still thrown. Sigh.

@JonnyOThan
Copy link

@JonnyOThan JonnyOThan commented on 5002506 Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything gets instantiated during loading for prefabs.

if this initialization code were in Start instead, it wouldn’t run in the flight scene. The loading scene is meant for setting up the prefab (in Awake mostly) so that it can be cloned for each instance.

Please sign in to comment.