Skip to content

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jun 24, 2025

Addresses #107944 (comment)
Allows displaying raw property names, regardless of inspector settings.

The only problem is that the permission properties are defined as lowercase 🙃
image

@KoBeWi KoBeWi added this to the 4.x milestone Jun 24, 2025
@KoBeWi KoBeWi requested review from a team as code owners June 24, 2025 20:28
Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Looks good!

@KoBeWi KoBeWi force-pushed the it's_friggin_raw branch from 3f392da to 6e17ecc Compare June 25, 2025 09:02
@KoBeWi KoBeWi requested a review from a team as a code owner June 25, 2025 09:02
@timothyqiu
Copy link
Member

The only problem is that the permission properties are defined as lowercase

For regular properties, we can deprecate properties by making them setter-only. But it guess it's not easy for export options.

Copy link
Member

@Calinou Calinou left a 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.

Copy link
Member

@timothyqiu timothyqiu left a 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.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 27, 2025

I can change the properties to upper case and add compatibility system, shouldn't be too difficult.

@akien-mga
Copy link
Member

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 AndroidManifest.xml.

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 snake_case) get converted to "Title Case" in the editor by default - they can now even be localized. I don't see how priori how those are particularly different from other properties to warrant a dedicated core change to support them.

@syntaxerror247
Copy link
Member

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).

@AThousandShips
Copy link
Member

I'd say the better solution for this specific case is to clarify it in the documentation

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.

7 participants