-
-
Notifications
You must be signed in to change notification settings - Fork 22.7k
Add property hint for displaying raw names #107953
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
base: master
Are you sure you want to change the base?
Conversation
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!
For regular properties, we can deprecate properties by making them setter-only. But it guess it's not easy for export options. |
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.
Code looks good to me. The use case makes sense.
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 rationale for this PR was to address the issue of Android permission enums being capitalized in export settings. However, by only adding a property hint, the permissions are still displayed in an undesirable way (lower cased).
I don't think this should be added separately unless there are other properties that can benefit from it.
I can change the properties to upper case and add compatibility system, shouldn't be too difficult. |
I wonder if this is a problem really worth solving though. We haven't had reports so far of users being confused by the apparent mismatch between our human-readable names in the editor and the Android permission names in I get @m4gr3d's point that this would be technically more correct, but is it really solving a problem? Godot users are familiar with the fact that properties names (usually |
It’s not a major issue, but I don’t think it's too much work to change it. Personally, I’d prefer if the permission names used upper snake case, for consistency with how permissions are referenced to request them (android.permission.READ_EXTERNAL_STORAGE). |
I'd say the better solution for this specific case is to clarify it in the documentation |
Addresses #107944 (comment)
Allows displaying raw property names, regardless of inspector settings.
The only problem is that the permission properties are defined as lowercase 🙃
