godotengine / godotengine/godot

C# exported StringName property set to null is assigned a non-null empty StringName with a null native value.

Open
#96,051 0 comments 0 reactions 0 assignees View on GitHub
bug topic:dotnet
Dominant language
C++
Stars
117k
Forks
26.8k
PR merge metrics
PR metrics pending

Description

### Tested versions

- Reproducible in 4.2 stable and 4.3 stable

### System information

Windows 10 - Godot 4.3

### Issue description

When a node or resource has an exported StringName property and that property is set to null in the tscn or tres file, upon running the game an empty StringName object will be assigned to that property.

### Steps to reproduce

It is specifically these functions from the [C# Glue StringName.cs](https://github.com/godotengine/godot/blob/e3550cb20f5d6a61befaafb7d9cbdb57b24870e4/modules/mono/glue/GodotSharp/GodotSharp/Core/StringName.cs#L55) and [C# Glue VariantUtils.cs](https://github.com/godotengine/godot/blob/e3550cb20f5d6a61befaafb7d9cbdb57b24870e4/modules/mono/glue/GodotSharp/GodotSharp/Core/NativeInterop/VariantUtils.cs#L520) files that cause this behavior:
```csharp
// StringName.cs

private StringName(godot_string_name nativeValueToOwn)
{
NativeValue = (godot_string_name.movable)nativeValueToOwn;
_weakReferenceToSelf = DisposablesTracker.RegisterDisposable(this);
}

// Explicit name to make it very clear
internal static StringName CreateTakingOwnershipOfDisposableValue(godot_string_name nativeValueToOwn)
=> new StringName(nativeValueToOwn);
```
```csharp
// VariantUtils.cs

public static StringName ConvertToStringName(in godot_variant p_var)
=> StringName.CreateTakingOwnershipOfDisposableValue(ConvertToNativeStringName(p_var));
```
This behavior can be observed when Godot is setting the properties defined within a tscn or tres file that has a StringName property with a value of null. When attempting to convert a Variant whose type is null, the `godot_string_name` that `ConvertToNativeStringName` returns does not have its `IsAllocated` property checked before creating a new StringName in `CreateTakingOwnershipOfDisposableValue`. This results in an empty StringName being created with an internal NativeValue wrapping a null IntPtr.

This _feels_ like unintended behavior. When other exported reference types are set to null through the editor, they are assigned null at runtime. If this is in fact unintended, I've already written a [small fix](https://github.com/operation404/godot/commit/7e52ce776351a23f3c3d547290707bdda8404862) and can open a pull request with it. I just added a ternary check to see if the native value was allocated before creating a new managed StringName.

If this is _intended_, then I think it should be documented somewhere explicitly. I have many StringName properties set to null by default, and if they aren't specified within a tscn or tres file, they stay null at runtime. If those properties are declared as null within those files, they instead have an empty StringName assigned. Even if a property isn't modified from its default and doesn't need to be specified in the resource file, Godot will sometimes add it anyways, which results in different behavior than if it were excluded.

### Minimal reproduction project (MRP)

-

Contributor guide

Open the contributing guide

Research direction

Start with the cited StringName.cs constructor and CreateTakingOwnershipOfDisposableValue method, then follow VariantUtils.cs ConvertToStringName and ConvertToNativeStringName for null variants. Reproduce the difference between an omitted and explicitly null StringName in a tscn or tres file; done means the intended null behavior is consistent and covered by regression testing.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
game-dev
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.