-
-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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 nullable environment for GodotSharp #83117
base: master
Are you sure you want to change the base?
Conversation
b7f9e49
to
020bd83
Compare
21cf810
to
a99183a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
99d811d
to
1f238f0
Compare
This comment was marked as outdated.
This comment was marked as outdated.
5fb0769
to
3156ddd
Compare
Walked back the above two posts; I won't be adding any new functions nor expanding nullability to |
bf80c79
to
f2f8da3
Compare
f2f8da3
to
e8c926e
Compare
As this PR is now functionally-sound entirely in isolation, I'm comfortable removing the "draft" qualifier. While it can still serve as a means of seeing how individual components would be handled, this should be just as viable to merge as-is There's still no real answer for how best to handle |
I'm not familiar with the specifics of this PR, but this bit sounds like it might be related to this proposal that we were discussing at the GDExtension team meeting today: godotengine/godot-proposals#2241 |
That proposal would indeed be the solution! This PR introduces nullability in an environment that lacks those hints, so |
5b79c73
to
6fed2cd
Compare
6fed2cd
to
bd71b1f
Compare
bd71b1f
to
7acc9a1
Compare
57fda70
to
5ad4a94
Compare
5ad4a94
to
1f830bb
Compare
1f830bb
to
bf87151
Compare
This PR showcases a GodotSharp environment where nullability is enabled. While initially conceived as a means to centralize changes which can be branched off into standalone PRs (eg: #82980, #82983), it also showcases the consequences of trying to retrofit the concept without first undergoing significant refactoring
The biggest culprit is
GodotObject
, and how it's uniquely a reference type that Godot allows for nullability. While this draft explores the idea of returning GodotObject fields as nullable, I'm not convinced that makes for the best approach, because it floods almost every existing method/field with an added nullability check. It might be worth considering an approach similar to Unity Objects, where there's operator overloads & behind-the-scenes null checks to better handlenull
in the context of that environment. Or it could even be as simple as throwing in a bunch of[AllowNull]/[MaybeNull]
annotations in the generation for object filesWhile the majority of areas have warnings accounted for, there's still a handful that have them. This is mainly those that I'm not entirely sure yet how to handle, or areas that feel particularly sensitive to adjustment. It's also entirely possible that many of these warnings/cases simply wouldn't be a thing with future core additions (particularly, some means of knowing if an object return value can be null). Either way, this is provided as-is for discussion and so potential areas can be cherrypicked for implementation as needed