Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C#] Enable nullability for variant structs #82980

Merged

Conversation

Repiteo
Copy link
Contributor

@Repiteo Repiteo commented Oct 7, 2023

Minor update on the road to GodotSharp being nullable by default. This focuses exclusively on the Variant structs, which are probably the one area that could be shifted over in full without issue. The only non-conversion change was in Projection.cs, where a new null check was added to handle a function returning Vector3? instead of Vector3, matching GDScript implementation.

@Repiteo Repiteo requested a review from a team as a code owner October 7, 2023 19:47
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for helping with the nullable annotations. Looks good, only a small nit-pick.

@raulsntos raulsntos added this to the 4.x milestone Oct 8, 2023
@Repiteo Repiteo force-pushed the c#-godotsharp-nullable-structs branch from f39da0d to 6fc74bf Compare October 8, 2023 02:33
Copy link
Member

@raulsntos raulsntos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Repiteo Repiteo force-pushed the c#-godotsharp-nullable-structs branch 2 times, most recently from 7e24c17 to 69ff298 Compare October 9, 2023 13:48
@akien-mga akien-mga changed the title C#: struct variants to nullable environment C#: Struct variants to nullable environment Oct 9, 2023
@Repiteo Repiteo force-pushed the c#-godotsharp-nullable-structs branch from 69ff298 to 05aa0f1 Compare December 1, 2023 15:54
@YuriSizov
Copy link
Contributor

YuriSizov commented Dec 1, 2023

@Repiteo Just a note about contribution style, but we prefer when commit messages start with a verb describing the action in this change. In case of area prefixes the verb would come after them. See this document. This is relevant to your other PRs as well.

@Repiteo
Copy link
Contributor Author

Repiteo commented Dec 1, 2023

@YuriSizov Ah! Thanks for clarifying, don't know how that detail escaped me

Based off the documentation, it seems it'd still be acceptable to prefix with C#: , provided it then follows with the verb syntax. Is that indeed the case, or is no prefix preferable?

@AThousandShips
Copy link
Member

AThousandShips commented Dec 1, 2023

Here you could do [dotnet] or [C#] or similar to make it clear it's a tag of sorts

My bad got mixed up with the used styles 🙂

@Repiteo Repiteo force-pushed the c#-godotsharp-nullable-structs branch from 05aa0f1 to d067c59 Compare December 1, 2023 16:32
@AThousandShips AThousandShips changed the title C#: Struct variants to nullable environment [C#] Enable nullability for variant structs Dec 1, 2023
@YuriSizov
Copy link
Contributor

Based off the documentation, it seems it'd still be acceptable to prefix with C#: , provided it then follows with the verb syntax. Is that indeed the case, or is no prefix preferable?

Yes, both options are acceptable.

@Repiteo Repiteo force-pushed the c#-godotsharp-nullable-structs branch from d067c59 to db7a643 Compare December 9, 2023 18:42
@YuriSizov YuriSizov modified the milestones: 4.x, 4.3 Dec 13, 2023
@YuriSizov YuriSizov merged commit c978f6b into godotengine:master Dec 14, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

Thanks!

@Repiteo Repiteo deleted the c#-godotsharp-nullable-structs branch December 14, 2023 16:47
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