Use C# 8 and the nullable reference type specification in the adapter #2312
Conversation
ksp_plugin_adapter/flight_planner.cs
Outdated
| @@ -77,7 +77,7 @@ class FlightPlanner : SupervisedWindowRenderer { | |||
| } else if (UnityEngine.GUILayout.Button("Create flight plan")) { | |||
| plugin.FlightPlanCreate(vessel_guid, | |||
| plugin.CurrentTime() + 1000, | |||
| vessel_.GetTotalMass()); | |||
| vessel_!.GetTotalMass()); | |||
| @@ -21,7 +21,7 @@ internal class DisposableIteratorMarshaller : ICustomMarshaler { | |||
| if (managed_object == null) { | |||
| return IntPtr.Zero; | |||
| } | |||
| var disposable_iterator = managed_object as DisposableIterator; | |||
| var disposable_iterator = (managed_object as DisposableIterator)!; | |||
There was a problem hiding this comment.
(DisposableIterator)managed_object, which checks.
| @@ -50,7 +50,7 @@ internal class DisposablePlanetariumMarshaller : ICustomMarshaler { | |||
| if (managed_object == null) { | |||
| return IntPtr.Zero; | |||
| } | |||
| var disposable_planetarium = managed_object as DisposablePlanetarium; | |||
| var disposable_planetarium = (managed_object as DisposablePlanetarium)!; | |||
There was a problem hiding this comment.
(DisposablePlanetarium)managed_object.
| private KSP.UI.Screens.DebugToolbar.DebugScreen debug_screen_; | ||
| private KSP.UI.Screens.DebugToolbar.Screens.Cheats.HackGravity hack_gravity_; | ||
| private KSP.UI.Screens.DebugToolbar.DebugScreen? debug_screen_; | ||
| private KSP.UI.Screens.DebugToolbar.Screens.Cheats.HackGravity? hack_gravity_; | ||
| private KSP.UI.Screens.DebugToolbar.Screens.Cheats.HackGravity hack_gravity { |
There was a problem hiding this comment.
The property is also a HackGravity?, because there only is a HackGravity once someone opens the cheats menu; this is why we get the debug_screen_ and search within it.
See also line 420:
Principia/ksp_plugin_adapter/ksp_plugin_adapter.cs
Lines 420 to 422 in a3b2358
| @@ -457,7 +457,7 @@ private enum UnityLayers { | |||
| } | |||
|
|
|||
| // Returns false and nulls |texture| if the file does not exist. | |||
| private bool LoadTextureIfExists(out UnityEngine.Texture texture, | |||
| private bool LoadTextureIfExists(out UnityEngine.Texture? texture, | |||
There was a problem hiding this comment.
Apparently there is [NotNullWhen(true)] for that: https://devblogs.microsoft.com/dotnet/try-out-nullable-reference-types/#conditional-postconditions-maybenullwhenbool-and-notnullwhenbool.
| @@ -789,7 +791,7 @@ private enum UnityLayers { | |||
| var normal = | |||
| (Vector3d)plugin_.VesselBinormal(active_vessel.id.ToString()); | |||
|
|
|||
| SetNavballVector(navball_.progradeVector, prograde); | |||
| SetNavballVector(navball_!.progradeVector, prograde); | |||
There was a problem hiding this comment.
Comment why this is not null (I think because RenderNavball set it above; we might want to make it a property like the Krakensbane etc. in the future for clarity).
| @@ -1186,7 +1188,7 @@ private enum UnityLayers { | |||
| new Origin{reference_part_is_at_origin = | |||
| FloatingOrigin.fetch.continuous, | |||
| reference_part_is_unmoving = | |||
| krakensbane_.FrameVel != Vector3d.zero, | |||
| krakensbane!.FrameVel != Vector3d.zero, | |||
| @@ -1603,7 +1606,7 @@ private enum UnityLayers { | |||
| // Texture the ball. | |||
| navball_changed_ = false; | |||
| if (plotting_frame_selector_.target_override) { | |||
| set_navball_texture(target_navball_texture_); | |||
| set_navball_texture(target_navball_texture_!); | |||
There was a problem hiding this comment.
Comment that the textures are set in OnAwake (we could write lazy-load properties for them so that things are clearer).
| ConfigNode ephemeris_parameters = | ||
| numerics_blueprint.GetAtMostOneNode("ephemeris"); | ||
| ConfigNode? ephemeris_parameters = | ||
| numerics_blueprint!.GetAtMostOneNode("ephemeris"); |
There was a problem hiding this comment.
Is this ! necessary with the null check above?
| @@ -2121,7 +2124,7 @@ private enum UnityLayers { | |||
| node => node.GetUniqueValue("name")); | |||
| BodyProcessor insert_body = body => { | |||
| Log.Info("Inserting " + body.name + "..."); | |||
| if (!name_to_gravity_model.TryGetValue( | |||
| if (!name_to_gravity_model!.TryGetValue( | |||
There was a problem hiding this comment.
Change lines 2111 .. 2113 from
if (name_to_gravity_model == null) {
Log.Fatal("Cartesian config without gravity models");
}to
if (name_to_gravity_model == null) {
throw Log.Fatal("Cartesian config without gravity models");
}so that control flow analysis understands that name_to_gravity_model is not null here.
| if (name_to_gravity_model?.TryGetValue( | ||
| body.name, | ||
| out body_gravity_model) == true) { | ||
| Log.Info("using custom gravity model"); | ||
| } | ||
| Orbit orbit = unmodified_orbits_.GetValueOrNull(body); | ||
| Orbit? orbit = unmodified_orbits_!.GetValueOrNull(body); |
There was a problem hiding this comment.
Comment that unmodified_orbits_ has been set in OnAwake (we should probably create the dictionary at construction so that unmodified_orbits_ is not null, and just fill it in OnAwake, but we can clean that up later).
| @@ -114,7 +114,7 @@ internal abstract class BaseWindowRenderer : ScalingRenderer, IConfigNode { | |||
| var old_skin = UnityEngine.GUI.skin; | |||
| if (skin_ == null) { | |||
| UnityEngine.GUI.skin = null; | |||
| skin_ = MakeSkin(UnityEngine.GUI.skin); | |||
| skin_ = MakeSkin(UnityEngine.GUI.skin!); | |||
There was a problem hiding this comment.
This is a case where we would really need to annotate the engine declarations:
the property UnityEngine.GUI.skin is UnityEngine.GUISkin, not UnityEngine.GUISkin?, but it is [AllowNull] (set accepts null, get never returns null).
No description provided.