godotengine / godotengine/godot
C# exported StringName property set to null is assigned a non-null empty StringName with a null native value.
- 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
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