C# Exports ignore default values for indirect getters

[markdibarry]

Tested versions

Godot v4.3.dev.mono (4728ff3)

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 (NVIDIA; 31.0.15.4601) - AMD Ryzen 9 3900X 12-Core Processor (24 Threads)

Issue description

When an export property has a getter that points to a non-constant value, it defaults to the type's default value. It's understandable why this is difficult to do, but makes tool scripts or plugins difficult to make and use, so this would be a good spot to brainstorm workarounds if a solution can't be proposed.

Steps to reproduce

Open MRP. Note that the inspector shows the reset button for the Baz property, but not Qux. When clicking the reset button, it can reset to a possibly invalid value outside the constraints of an export hint.

Minimal reproduction project (MRP)

defaulttest.zip

[raulsntos]

This is expected, the source generator can only figure out the default value for simple properties. See the note in the C# exported properties documentation page that was added in godotengine/godot-docs#8859:

A property's get is not actually executed to determine the default value. Instead, Godot analyzes the C# source code. This works fine for most cases, such as the examples on this page. However, some properties are too complex for the analyzer to understand.

For example, the following property attempts to use math to display the default value as 5 in the property editor, but it doesn't work:

[Export]
public int NumberWithBackingField
{
    get => _number + 3;
    set => _number = value - 3;
}

private int _number = 2;

The analyzer doesn't understand this code and falls back to the default value for int, 0. However, when running the scene or inspecting a node with an attached tool script, _number will be 2, and NumberWithBackingField will return 5. This difference may cause confusing behavior. To avoid this, don't use complex properties.

So in your MRP the default value for Baz is determined to be 0. You can see the generated code adding <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles> to your csproj and looking for the .godot/mono/temp/obj/Debug/generated/Godot.SourceGenerators/DefaultTest.Foo_ScriptPropertyDefVal.generated.cs file:

internal static Dictionary<StringName, Variant> GetGodotPropertyDefaultValues()
{
    var values = new Dictionary<StringName, Variant>();

    int __Baz_default_value = default; // The default value for int is 0.
    values.Add(PropertyName.Baz, Variant.From<int>(__Baz_default_value));

    int __Qux_default_value = 7;
    values.Add(PropertyName.Qux, Variant.From<int>(__Qux_default_value));

    return values;
}

It's understandable why this is difficult to do, but makes tool scripts or plugins difficult to make and use

Why is that difficult? I'm interested in hearing more about your specific use case.

[markdibarry]

Why is that difficult?

Sorry, I meant it makes sense why this is difficult to fix, and would be too expensive to do even if we could. The question is: is there a suggested workaround we could suggest in the docs?

My specific use case is for plugins and tool scripts that are meant to provide an interface in the editor to be used by others. In my current project (a fake 2D lighting system), I have a custom node that needs to add another hidden custom node as a child. The user won't see the child and shouldn't interact with it, except through the main node and its export properties. Unfortunately, that means all of the export properties default values misleadingly display as not the default.

If reset, this change can break the node by resetting to values that aren't supported by the export property hint. Or, at best, break it temporarily until you manually set it to a supported value (which the user wouldn't know of).

I've run into this in a few different projects like a dialog system where a nodes often need to expose properties of their children to prevent further complexity for the user. Or, at least, I've run into it enough times over the last few years that it'd be good to have a workaround for since it's not reasonable to support. I've considered making a C++ module instead for these features, but I doubt that's a good suggestion for others. 😅

[raulsntos]

I see. Would you say explicitly specifying the default value would solve your problem? If so, you can override the _PropertyCanRevert and _PropertyGetRevert methods.

public override bool _PropertyCanRevert(StringName property)
{
    if (property == PropertyName.Baz)
    {
        return true;
    }

    return base._PropertyCanRevert(property);
}

public override Variant _PropertyGetRevert(StringName property)
{
    if (property == PropertyName.Baz)
    {
        return 8;
    }

    return base._PropertyGetRevert(property);
}

These methods, along with _GetPropertyList and _ValidateProperty allow more advanced use cases for exported properties for when the [Export] attribute is not enough.

[markdibarry]

Oh that works great! I wasn't familiar with these two. I think they're pretty self explanatory if we were to link to their definition as part of that note in the docs. I'll make a PR tomorrow.

Thanks for the great insight!