Skip to content

Conversation

@Delsin-Yu
Copy link
Contributor

@Delsin-Yu Delsin-Yu commented Oct 30, 2025

This PR aims to enhance the usability of the Nullable Reference Type feature by introducing automatic checks for exported members via a new generated override on the virtual method ValidateExportedProperties.

This feature is enabled under the following conditions:

  1. The line <GodotEnableExportNullChecks>true</GodotEnableExportNullChecks> must be included in the csproj file.
  2. The nullable context should be enabled for an exported type, either by including <Nullable>enable</Nullable> in the csproj or by using #nullable enable in your code.
  3. The exported member does not have a nullability annotation (without ?).

With this improvement, users will no longer need to frequently write * = null! for every not-null exported variable or perform null checks at every instance call. The binding will automatically check these values once the scene is created.

To be specific, the following node type:

using Godot;

public partial class Main : Node
{
    [Export] private Node _testField1;
#nullable enable
    [Export] private Resource _testField2EnableNullChk;
    [Export] private Node? _testField3;
    [Export] private Resource? _testField4;
    [Export] private Node _testField5EnableNullChk;
#nullable disable
    [Export] private Resource _testField6;

    [Export] private Node TestProperty1 { get; set; }
#nullable enable
    [Export] private Resource TestProperty2EnableNullChk { get; set; }
    [Export] private Node? TestProperty3 { get; set; }
    [Export] private Resource? TestProperty4 { get; set; }
    [Export] private Node TestProperty5EnableNullChk { get; set; }
#nullable disable
    [Export] private Resource TestProperty6 { get; set; }
}

Would result in the following check code:

using Godot;
using Godot.NativeInterop;

partial class Main
{
#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword
    /// <summary>
    /// Cached StringNames for the methods contained in this class, for fast lookup.
    /// </summary>
    public new class MethodName : global::Godot.Node.MethodName {
    }
#pragma warning restore CS0109
    /// <inheritdoc/>
    [global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
    protected override void ValidateExportedProperties()
    {
        if (this.TestProperty2EnableNullChk == null) throw new global::System.NullReferenceException("The exported property/field 'TestProperty2EnableNullChk' of type 'Godot.Resource' is null.");
        if (this.TestProperty5EnableNullChk == null) throw new global::System.NullReferenceException("The exported property/field 'TestProperty5EnableNullChk' of type 'Godot.Node' is null.");
        if (this._testField2EnableNullChk == null) throw new global::System.NullReferenceException("The exported property/field '_testField2EnableNullChk' of type 'Godot.Resource' is null.");
        if (this._testField5EnableNullChk == null) throw new global::System.NullReferenceException("The exported property/field '_testField5EnableNullChk' of type 'Godot.Node' is null.");
        base.ValidateExportedProperties();
    }
}

And the following resource type:

using Godot;

[GlobalClass]
public partial class MyResource : Resource
{
    [Export] private Resource _resource1;
#nullable enable
    [Export] private Resource _resource2EnableNullChk;
}

Would result in the following check code:

using Godot;
using Godot.NativeInterop;

partial class MyResource
{
#pragma warning disable CS0109 // Disable warning about redundant 'new' keyword
    /// <summary>
    /// Cached StringNames for the methods contained in this class, for fast lookup.
    /// </summary>
    public new class MethodName : global::Godot.Resource.MethodName {
    }
#pragma warning restore CS0109
    /// <inheritdoc/>
    [global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
    protected override void ValidateExportedProperties()
    {
        if (this._resource2EnableNullChk == null) throw new global::System.NullReferenceException("The exported property/field '_resource2EnableNullChk' of type 'Godot.Resource' is null.");
        base.ValidateExportedProperties();
    }
}

@Delsin-Yu Delsin-Yu requested a review from a team as a code owner October 30, 2025 11:29
@Delsin-Yu
Copy link
Contributor Author

Delsin-Yu commented Oct 30, 2025

Currently, the user needs to manually NoWarn the CS8618 in their csproj, as SuppressMessageAttribute doesn't yet support suppressing compiler warnings. This can be fixed either by waiting for upstream support or by communicating with the IDE authors to implement manual suppression at the plugin level.

Thanks for the help from the members of the dotnet/runtime that pointed out the existence of the DiagnosticSuppressor

@zaevi
Copy link
Contributor

zaevi commented Oct 30, 2025

Since NotificationSceneInstantiated is only sent to the scene's root node, this check appears unable to detect the following two scenarios:

  1. Child nodes with attached scripts within the scene
  2. Resources containing [Export] variables

@Delsin-Yu
Copy link
Contributor Author

Since NotificationSceneInstantiated is only sent to the scene's root node, this check appears unable to detect the following two scenarios

That's a good point. I guess we need a better access point in the object initialization workflow to insert these checks.

@Delsin-Yu
Copy link
Contributor Author

The nullability warning now has proper suppressions.

@Delsin-Yu Delsin-Yu requested a review from a team as a code owner October 30, 2025 14:12
@Delsin-Yu
Copy link
Contributor Author

[1/3] Mesa NIR
Downloading Mesa NIR godot-nir-static-arm64-llvm-release.zip ...
Extracting Mesa NIR godot-nir-static-arm64-llvm-release.zip to C:\Users\runneradmin\AppData\Local\Godot\build_deps\mesa-arm64-llvm ...
Downloading Mesa NIR godot-nir-static-arm64-msvc-release.zip ...
TimeoutError: [WinError 10060] A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond

🤔 I guess any web request in CI can fail these days

@Delsin-Yu
Copy link
Contributor Author

Delsin-Yu commented Oct 31, 2025

Now packed array types and string / StringNames are also included, e.g.

[Export] private int[] _intArray;
[Export] private string _str;

Are initialized to Array.Empty and string.Empty if unchanged in inspector.

TLDR:

    [Export] private Node _targetNode;
    [Export] private Resource _targetResource;
    [Export] private string _targetString;
    [Export] private StringName _targetStringName;
    [Export] private int[] _targetIntArray;
    [Export] private Godot.Collections.Array<int> _targetGodotIntArray;
    [Export] private Godot.Collections.Dictionary<string, int> _targetGodotDictionary;

Will have the following checks:

protected override void ValidateExportedProperties()
{
    if (this._targetNode == null) throw new global::System.NullReferenceException("The exported property/field '_targetNode' of type 'Godot.Node' is null.");
    if (this._targetResource == null) throw new global::System.NullReferenceException("The exported property/field '_targetResource' of type 'Godot.Resource' is null.");
    if (this._targetString == null) this._targetString = string.Empty;
    if (this._targetStringName == null) this._targetStringName = new global::Godot.StringName();
    if (this._targetIntArray == null) this._targetIntArray = global::System.Array.Empty<int>();
    if (this._targetGodotIntArray == null) throw new global::System.NullReferenceException("The exported property/field '_targetGodotIntArray' of type 'Godot.Collections.Array<int>' is null.");
    base.ValidateExportedProperties();
}

@Joy-less
Copy link
Contributor

Now packed array types and string / StringNames are also included, e.g.

[Export] private int[] _intArray;
[Export] private string _str;

Are initialized to Array.Empty and string.Empty if unchanged in inspector.

TLDR:

    [Export] private Node _targetNode;
    [Export] private Resource _targetResource;
    [Export] private string _targetString;
    [Export] private StringName _targetStringName;
    [Export] private int[] _targetIntArray;
    [Export] private Godot.Collections.Array<int> _targetGodotIntArray;
    [Export] private Godot.Collections.Dictionary<string, int> _targetGodotDictionary;

Will have the following checks:

protected override void ValidateExportedProperties()
{
    if (this._targetNode == null) throw new global::System.NullReferenceException("The exported property/field '_targetNode' of type 'Godot.Node' is null.");
    if (this._targetResource == null) throw new global::System.NullReferenceException("The exported property/field '_targetResource' of type 'Godot.Resource' is null.");
    if (this._targetString == null) this._targetString = string.Empty;
    if (this._targetStringName == null) this._targetStringName = new global::Godot.StringName();
    if (this._targetIntArray == null) this._targetIntArray = global::System.Array.Empty<int>();
    if (this._targetGodotIntArray == null) throw new global::System.NullReferenceException("The exported property/field '_targetGodotIntArray' of type 'Godot.Collections.Array<int>' is null.");
    base.ValidateExportedProperties();
}

If types other than array and string throw if not assigned, surely array and string should also throw? They are both reference types.

@Delsin-Yu
Copy link
Contributor Author

Delsin-Yu commented Oct 31, 2025

If types other than array and string throw if not assigned, surely array and string should also throw? They are both reference types.

I'm trying to match the GDScript's behavior here, as it is tricky to deterime the nullability for string and packed arrays when operating them in the inspector, see this example:

extends Node

@export var _targetNode : Node;
@export var _targetResource : Resource;
@export var _targetString : String;
@export var _targetStringName : StringName;
@export var _targetIntArray : Array[int];
@export var _targetPackedIntArray : PackedInt32Array;
@export var _targetGodotDictionary : Dictionary[String, int];

func _ready() -> void:
	print(_targetNode);
	print(_targetResource);
	print(_targetString);
	print(_targetStringName);
	print(_targetIntArray);
	print(_targetPackedIntArray);
	print(_targetGodotDictionary);

Prints the following is nothing is assigned in inspector:

Godot Engine v4.5.1.stable.mono.official.f62fdbde1 - https://godotengine.org
Vulkan 1.4.303 - Forward+ - Using Device #0: NVIDIA - NVIDIA GeForce RTX 5070

<null>
<null>


[]
[]
{  }

As you see, unassigned String and StringName will be assigned to empty strings, and arrays, packed arrays, and dictionaries will be assigned to empty collections.

@Delsin-Yu
Copy link
Contributor Author

The reason I left Godot.Collections null checked is that you actually can declare nullable collections with C# script (which will be null unless you expand them in the inspector).

QQ2025111-01728.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants