-
-
Notifications
You must be signed in to change notification settings - Fork 22k
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#: Implement proper generic type name printing for Godot Editor #88363
Conversation
aae2994
to
0095ff4
Compare
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
I removed the cached I wish to minimize the performance impact on runtime by this PR, where cc @raulsntos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .NET team discussed this and we think it's a good addition. The technical aspects look good, this is a code-style review.
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
modules/mono/glue/GodotSharp/GodotSharp/Core/ReflectionUtils.cs
Outdated
Show resolved
Hide resolved
dc65e33
to
96f23f4
Compare
Co-authored-by: Raul Santos <raulsntos@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and works as expected. Thanks!
Thanks! |
The current Godot Editor Implementation fetches a class_name directly from the
System.Type.Name
, which does not return the full definition of a generic type (only the generic parameter count).This PR uses an algorithm created by @ZerxZ, @TML233, and @jinsediaoying for printing the actual underlying generic type definition.
Our tests have convinced us this algorithm is capable of handling all generic use cases, see the following comparison.
This advanced type-printing algorithm only runs when a cached boolean
_isEditorHintCached
reportstrue
, (fallback toSystem.Type.Name
otherwise), so it should not affect runtime performance (only one extra boolean check).Before
After
These are the actual type header definitions in the scripts