-
-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Add type_string()
utility
#69624
Add type_string()
utility
#69624
Conversation
Regarding the PR itself: I love this feature and I'll be using it a ton in debugging. But since 4.0 is in feature freeze and this doesn't fix any bugs, it will likely not make the cut for now. |
bcf2800
to
d92c3d3
Compare
Since this is a small feature, the commits need to be squashed into one. Here's a link to our tutorial on this: https://docs.godotengine.org/en/stable/community/contributing/pr_workflow.html#the-interactive-rebase |
Thanks! I always find a way to mess my branch up |
@Kubulambula Is this PR ready? I would love this feature, and it could make 4.1. |
Yes it should be ready as it does not change anything and just exposes an already present function and is just waiting for approval. |
After looking at it though, maybe Because you get the name of type that you get from Any thoughts? |
I think it would, yes. |
This is clearly the correct implementation, I think it's a matter of whether we approve of the feature and if so, when we get to merging this. And I also prefer |
b861210
to
e9388b7
Compare
c3c975d
to
f61cfc3
Compare
Need to rebase the branch and resolve conflicts. |
c7d7cba
to
594d195
Compare
Should be good now. |
a519812
to
7729caa
Compare
5e42346
to
fcbc50e
Compare
@AThousandShips I think that I somehow fixed it. Thank you for your help and patience with me ❤️ |
2a28316
to
aa98e31
Compare
aa98e31
to
7fd79f8
Compare
b1d99dd
to
5d4adc8
Compare
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.
Could you please remove the variant_get_type_name()
function in GDScript tests by the way:
godot/modules/gdscript/tests/scripts/utils.notest.gd
Lines 69 to 76 in 7e67b49
static func variant_get_type_name(type: Variant.Type) -> String: | |
match type: | |
TYPE_NIL: | |
return "Nil" # `Nil` in core, `null` in GDScript. | |
TYPE_BOOL: | |
return "bool" | |
TYPE_INT: | |
return "int" |
And replace it here:
return variant_get_type_name(property.type) |
- return variant_get_type_name(property.type)
+ return type_string(property.type)
aef2c4e
to
e74ba27
Compare
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!
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.
Other than the note, it looks good!
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.
Seems useful to me.
e74ba27
to
74c9370
Compare
Thanks! And congrats for your first merged Godot contribution 🎉 |
Closes #1860
Adds
typeof_string()
method, usingVariant::get_type_name()
effectively just exposing this method to GDScript.This method goes hand in hand with the already implemented
error_string()
utility. Both of these methods are extremely useful for plugin development where user interaction is expected.The proposal describes a method that prints the name of the variable directly without the
typeof()
step between. But I believe that this makes this function more versatile and a method like that can be simply made like this: