Allow C# SignalAttribute to take a name argument

bbugh commented 2 years ago

Describe the project you are working on

2D space exploration game, but it affects anything with C#

Describe the problem or limitation you are having in your project

The standard naming convention for Godot editor signals is snake case, but naming conventions for C# uses PascalCase. There is no conversion between the two, so either the editor or the C# code has an unusual naming scheme.

[Signal]
delegate void area_entered(PhysicsBody2D body);

results in the signal being named area_entered, but doesn't follow naming conventions in C#.

vs

[Signal]
delegate void AreaEntered(PhysicsBody2D body);

results in the signal being named and accessed as AreaEntered.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

By allowing a specified name, like:

[Signal("my_signal")]
delegate void MySignal();

results in

my_signal in the editor and in usage, matching the current pattern.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Some options:

Names are automatically converted to snake case. Not backwards compatible, would have to be Godot 4 if this is desired.
SignalAttribute gets a new argument name that is used first where SignalAttribute is automatically named.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I don't know any workarounds that don't change one side or the other's expectations.

Is there a reason why this should be core and not an add-on in the asset library?

I don't think it can be an add-on.

P.S. I have a background in pro game dev and can create a PR if this feature is accepted.

Calinou commented 2 years ago

cc @neikeq @raulsntos

raulsntos commented 2 years ago

My personal opinion is allowing to customize the name gives too much flexibility. For example this would break the assumption that the name of the signal is the name of the delegate so you can no longer use nameof expressions.

Currently we recommend using nameof because that way you get compilation errors if you mistype it and since it references the member you can get the delegate signature on hover, ctrl+click to go to the definition and refactor renames all occurrences.

[Signal]
delegate void AreaEntered(PhysicsBody2D body);

Connect(nameof(AreaEntered), area, nameof(OnAreaEntered));

If it can't be assumed that the signal name is always going to be the name of the delegate you can no longer rely on nameof and must use a string (although you can always use a const string defined somewhere to try and avoid typos, but you still lose all the other benefits of using nameof).

const string AreaEnteredName = "area_entered";

[Signal(AreaEnteredName)]
delegate void AreaEntered(PhysicsBody2D body);

Connect(AreaEnteredName, area, nameof(OnAreaEntered));

This would be easier to do in 4.0 since signals are C# events so you can continue referring to the C# event regardless of the signal name used in Godot.

delegate void AreaEnteredHandler(PhysicsBody2D body);

[Signal("area_entered")]
event AreaEnteredHandler AreaEntered;

area.AreaEntered += OnAreaEntered;

However, we could do this automatically in 4.0 so you don't have to specify a name and it always gets converted to snake case. Not sure if there would be much of an use case for specifying a different name other than the event name converted to a different case.

bbugh commented 2 years ago

My personal opinion is allowing to customize the name gives too much flexibility.

FWIW I agree with that. I had also considered some other attribute or a "case" argument, but that seemed weirder than customizing a name.

It seems like the way to go is to set Godot 4 to auto convert without any arguments. Is that right?

godotengine / godot-proposals