the connect method flags in c#

[behroozbc]

Godot version

3.5.1

System information

Windows 10

Issue description

The connect method in c# does not have any type a flag parameter which is hard to know and code it. This hard to code because this parameter only gets unit and does not contain any information the flags value. you can see in this sample

_player.Connect("DiedEventHandler", this, nameof(_OnPlayerDied),flags:1);

Steps to reproduce

start to coding with c# and use connect method.

_player.Connect("DiedEventHandler", this, nameof(_OnPlayerDied),flags:1);

Minimal reproduction project

N/A

[behroozbc]

I suggest this flags become an enum and in the body of the connect function cast it to a unit.

[KoBeWi]

I think this is resolved in 4.0.

[behroozbc]

Yes, In version 4 this problem was fixed by adding the ConnectFlags enum. However, the connect function signature does not have changed, which is

        public Error Connect(StringName signal, Callable callable, uint flags = 0u)
        {
            return (Error)NativeCalls.godot_icall_3_690(MethodBind31, GetPtr(this), (godot_string_name)(signal?.NativeValue ?? default(godot_string_name.movable)), in callable, flags);
        }

Also, I see this code in my visual studio editor, but I cannot find it in the git repository.
I suggest This signature change to like this

        public Error Connect(StringName signal, Callable callable, ConnectFlags flags= ConnectFlags.None)
        {
            return (Error)NativeCalls.godot_icall_3_690(MethodBind31, GetPtr(this), (godot_string_name)(signal?.NativeValue ?? default(godot_string_name.movable)), in callable, flags);
        }

[behroozbc]

I want to help and contribute to the Godot engine, If it's possible tell me about how I can fix this issue and where is the connect method to change its signature?

[RedworkDE]

That method is generated from the original declaration in https://github.com/godotengine/godot/blob/master/core/object/object.cpp#L1232 there the arg would need to be changed to BitField<ConnectFlags>, not sure how much of a breaking change this is, if this could be included in a 4.x version or if this would have to wait for 5.0.

[behroozbc]

I think this change need a major update which will go to version 5.

[dannflor]

Can't we change ConnectFlags to be a uint backed enum in C#? I'm pretty surprised that the Connect method signature can't accept the enum specifically made for that one case.

[raulsntos]

Changing the underlying type of the ConnectFlags enum won't solve this issue, since the method still takes uint and C# considers them different types (even if the enum's underlying type is the same).

Adding a method overload that takes the ConnectFlags enum instead of uint would create ambiguity for the case where the user was passing a number literal¹. I don't know how common this case is, it's probably very rare, so we may want to break source compatibility to fix the issue.

So, if we decide to break source compatibility we can fix this in Godot 4.x, no need to wait for 5.0. We would change the Connect method signature in Core, then add a compat method for GDExtension and C# to avoid breaking binary compatibility.

Footnotes

1. I think it's considered a System.Int32 which is convertible to both uint and enums. ↩

[dannflor]

This sort of tiny targeted change seems like a perfect time to cut my teeth on the engine source code. Can you explain further what the “compat” method would be? The idea is to change the signature of the main one to take the enum, right? Wouldn’t the compat method cause the ambiguity you mentioned?

[raulsntos]

A compat method is a method with the signature of the old method, it's bound using ClassDB::bind_compatibility_method¹ instead of ClassDB::bind_method.

Wouldn’t the compat method cause the ambiguity you mentioned?

Yes, in C# the compat method is just an overload with the old signature, so it would cause ambiguity. But this ambiguity is only a source breaking change so maybe it's worth breaking compat for this, not sure.

Before you do anything, I'd advise waiting for a decision to be made. I'd like to know what @godotengine/production thinks of breaking source compatibility for this.

Footnotes

1. To add compat methods to GDExtension, see https://github.com/godotengine/godot/pull/76577.
   To add compat methods to C#, see https://github.com/godotengine/godot/pull/78452. ↩

[alex-everitt-2277]

This is a pretty old thread but im still seeing this issue in latest stable Godot version.

Did anyone ever get a chance to look into this?