DotNET: Deprecated signal causes build failure

[RedMser]

Tested versions

• Reproducible in latest master (f1c0b5b)

System information

Windows 10 - DotNET SDK v8.0.300

Issue description

See the build failure in https://github.com/godotengine/godot/actions/runs/12914187787/job/36013250581?pr=90049

My PR #90049 deprecates a signal on SceneTree which seems to cause a warning (which is treated as error).

B:\Godot\modules\mono\glue\GodotSharp\GodotSharp\Generated\GodotObjects\SceneTree.cs(856,11): warning CS0618: 'SceneTree.NodeC
onfigurationWarningChangedEventHandler' is obsolete: 'This signal is no longer available. Call 'update_configuration_info' on  
a 'Node' or 'Resource' to notify the editor of configuration info changes.' [B:\Godot\modules\mono\glue\GodotSharp\GodotSharp\
GodotSharp.csproj]

Silencing this warning should be the way to fix this I assume? Generated code snippet for reference:

    /// <summary>
    /// Represents the method that handles the <see cref="Godot.SceneTree.NodeConfigurationWarningChanged"/> event of a <see cref="Godot.SceneTree"/> class.
    /// </summary>
    [Obsolete("This signal is no longer available. Call 'update_configuration_info' on a 'Node' or 'Resource' to notify the editor of configuration info changes.")]
    public delegate void NodeConfigurationWarningChangedEventHandler(Node node);

    private static void NodeConfigurationWarningChangedTrampoline(object delegateObj, NativeVariantPtrArgs args, out godot_variant ret)
    {
        Callable.ThrowIfArgCountMismatch(args, 1);
        ((NodeConfigurationWarningChangedEventHandler)delegateObj)(VariantUtils.ConvertTo<Node>(args[0]));
        // ^^^^ This usage of the EventHandler type emits the warning!
        ret = default;
    }

    /// <summary>
    /// <para>Emitted when the <c>node</c>'s <see cref="Godot.Node.UpdateConfigurationWarnings()"/> is called. Only emitted in the editor.</para>
    /// </summary>
    [Obsolete("This signal is no longer available. Call 'update_configuration_info' on a 'Node' or 'Resource' to notify the editor of configuration info changes.")]
    public unsafe event NodeConfigurationWarningChangedEventHandler NodeConfigurationWarningChanged
    {
        add => Connect(SignalName.NodeConfigurationWarningChanged, Callable.CreateWithUnsafeTrampoline(value, &NodeConfigurationWarningChangedTrampoline));
        remove => Disconnect(SignalName.NodeConfigurationWarningChanged, Callable.CreateWithUnsafeTrampoline(value, &NodeConfigurationWarningChangedTrampoline));
    }

    protected void EmitSignalNodeConfigurationWarningChanged(Node node)
    {
        EmitSignal(SignalName.NodeConfigurationWarningChanged, node);
    }

Steps to reproduce

• Checkout Rework the Configuration Warning system #90049
• Follow the steps on here: https://docs.godotengine.org/en/latest/contributing/development/compiling/compiling_with_dotnet.html
  • Compile with mono module enabled
  • Generate glue
  • Run build_assemblies.py

Minimal reproduction project (MRP)

N/A

[AThousandShips]

Is the configuration to treat warnings as errors something users have control over?

This signal shouldn't be used in generated code if it is deprecated though, so that would be an issue

[RedMser]

Is the configuration to treat warnings as errors something users have control over?

Yes!

godot/modules/mono/build_scripts/build_assemblies.py

Line 409 in 8642e97

parser.add_argument("--werror", action="store_true", default=False, help="Treat compiler warnings as errors.")

But I think in the CI we want to avoid warnings, so treating them as errors is correct.

[AThousandShips]

So this is specific to CI, my bad, the code generation should probably be changed here, the question is if it should generate handlers etc. for a deprecated signal or not, and if it should I assume it should be adding warning supressions appropriately

Has there not been any other deprecated signals so far?

[RedMser]

Has there not been any other deprecated signals so far?

There are EditorPlugin.project_settings_changed and Resource.setup_local_to_scene_requested but their generated code does not include the Trampoline function, so they don't use any deprecated things.
Probably different scenario because SceneTree is a singleton? I'm not a C# user so I'm just guessing :D

[AThousandShips]

Then it looks like we're just missing the correct code in code generation, or it is handled safely for those signals and it just doesn't work for singletons for some reason

I'd suggest someone with C# knowledge should dig through the code generator and see

Or just generate the C# code and see what those signal handlers output, they might just be adding some magic that this case just doesn't because of some edge case

[raulsntos]

The Trampoline function is generated when the signal has parameters, it's not really related to singletons. It just so happens that both EditorPlugin.project_settings_changed and Resource.setup_local_to_scene_requested are parameterless so they don't generate a Trampoline function¹.

The Trampoline function is just a private implementation detail, it's generated to serve as a bridge between the C++ signal and the C# event. So you could say it's part of the code generated to implement the C# event that corresponds to a signal, if that signal is deprecated I think it makes sense to deprecate the Trampoline function as well, thus silencing the warning as suggested.

I went ahead and created a PR to do that: #101970

Footnotes

1. The main purpose of the Trampoline function is to marshal the parameters from C++ objects into C# objects, so when the signal has no parameters, we don't need a Trampoline function. ↩