Skip to content

C#: Parameterless constructors for Variant structs#83207

Open
Repiteo wants to merge 1 commit intogodotengine:masterfrom
Repiteo:c#-struct-parameterless-constructors
Open

C#: Parameterless constructors for Variant structs#83207
Repiteo wants to merge 1 commit intogodotengine:masterfrom
Repiteo:c#-struct-parameterless-constructors

Conversation

@Repiteo
Copy link
Copy Markdown
Contributor

@Repiteo Repiteo commented Oct 12, 2023

Currently, it's documented that one of the differences between GDScript and C# is that certain struct types shouldn't use their constructors, instead opting for static readonly equivalents. This was because, at the time of writing, it was impossible for C# to define a parameterless constructor for a struct. However, starting with .NET6 (C#10), we now have that functionality!

Old implementation

This PR provides parity with GDScript by assigning parameterless constructors to the 6 problem types, setting their values to the GDScript equivalents. Theoretically, this shouldn't break compatability, as it's already suggested by the documentation to always avoid these constructors. However, in the case that a zeroed-value is what's desired, the docstrings mention the relevant C#10 specification where providing default results in exactly that. The one area where parity is arguable is fields, as their implicit value is default rather than new(); but in a C# context this is expected behavior (mirrors reference types and nullable value types being null, their default, implicitly)


Input:

GD.Print(new Color());
GD.Print((Color)default);
GD.Print(new Quaternion());
GD.Print((Quaternion)default);
GD.Print(new Transform2D());
GD.Print((Transform2D)default);
GD.Print(new Transform3D());
GD.Print((Transform3D)default);
GD.Print(new Basis());
GD.Print((Basis)default);
GD.Print(new Projection());
GD.Print((Projection)default);

Output:

(0, 0, 0, 1)
(0, 0, 0, 0)
(0, 0, 0, 1)
(0, 0, 0, 0)
[X: (1, 0), Y: (0, 1), O: (0, 0)]
[X: (0, 0), Y: (0, 0), O: (0, 0)]
[X: (1, 0, 0), Y: (0, 1, 0), Z: (0, 0, 1), O: (0, 0, 0)]
[X: (0, 0, 0), Y: (0, 0, 0), Z: (0, 0, 0), O: (0, 0, 0)]
[X: (1, 0, 0), Y: (0, 1, 0), Z: (0, 0, 1)]
[X: (0, 0, 0), Y: (0, 0, 0), Z: (0, 0, 0)]
1, 0, 0, 0
0, 1, 0, 0
0, 0, 1, 0
0, 0, 0, 1

0, 0, 0, 0
0, 0, 0, 0
0, 0, 0, 0
0, 0, 0, 0

EDIT: Change of plans. The parameterless constructor will instead be used as a means of adding further documentation. It'll remain functionally identical to default, but with an added note on how to emulate GDScript behavior if used as a constructor. Also removes the portion of documentation that explicitly discourages a plain constructor

@Repiteo Repiteo requested review from a team as code owners October 12, 2023 18:59
@paulloz
Copy link
Copy Markdown
Member

paulloz commented Oct 12, 2023

IINW this was discussed in #65051.

@Repiteo
Copy link
Copy Markdown
Contributor Author

Repiteo commented Oct 12, 2023

Now I feel like a goober, I was looking for this in proposals but not in archived PRs

I'm not entirely convinced that this is 1-1 with that though, because it didn't seem to present the idea of parameterless constructors in parity with default. Unless I'm missing something, I don't see any harm in giving users the option to use a constructor

@raulsntos
Copy link
Copy Markdown
Member

The default keyword was also discussed: #65051 (comment) and #65051 (comment).

I feel like having parameterless constructors in structs is confusing for C# users. And my impression from the original discussion is that there was agreement on this.

Unless I'm missing something, I don't see any harm in giving users the option to use a constructor

The harm is that users may not realize that the constructor does something different to what they expect. I'd be OK with giving users the option, but that option already exists: the Identity property does exactly what the GDScript constructors do.

Theoretically, this shouldn't break compatability, as it's already suggested by the documentation to always avoid these constructors.

That's not how compatibility works. Just because we discourage users from using an API, that doesn't mean we can then break that API. That said, I don't think we discourage users from using parameterless constructors, we just mention it in the documentation as one of the differences from GDScript, but if users need a zeroed struct they are free to use the parameterless constructor to get one.

@Repiteo
Copy link
Copy Markdown
Contributor Author

Repiteo commented Oct 12, 2023

I don't think we discourage users from using parameterless constructors, we just mention it in the documentation as one of the differences from GDScript

Maybe not in the main documentation, but several Identity docstrings certainly discourage it:

Do not use <c>new Basis()</c> with no arguments in C#, because it sets all values to zero.

I don't know if I'm completely sold on the concept being confusing, but I'm not gonna die on that hill. If we wanna let the behavior remain as-is, then that's totally fine by me


So instead, let's flip this on its head: we can use a parameterless constructor to define default values instead. At first that sounds redundant, as it's functionally identical to how it worked before, but there's one key difference: it can have a docstring

As such, instead of relying on the Identity docstring to specify it should be used, which a user might miss, we can specify in the base constructor that using it will result in a zeroed-value result. It keeps old behavior 100% intact, while gaining the potential to specify for unaware users that the use-case might not be what they expect

• Functionally identical to old behavior, but with docstrings to help guide potentially uninformed users
@Repiteo Repiteo force-pushed the c#-struct-parameterless-constructors branch from 5d523a9 to 68ec192 Compare October 12, 2023 23:45
youfch added a commit to youfch/godot that referenced this pull request Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants