Skip to content
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

Expose all existing PropertyHint global enums #67360

Merged

Conversation

jordo
Copy link
Contributor

@jordo jordo commented Oct 13, 2022

As disucssed on RC, exposes additional global enums in 3.x that have been exposed in master. Allows for elimination of using magic numbers for these.

@jordo jordo requested a review from a team as a code owner October 13, 2022 17:18
@Mickeon
Copy link
Contributor

Mickeon commented Oct 13, 2022

I am not confident on this, but one of the checks may soon fail because there's no documentation defined for these in @GlobalScope, I believe.

@jordo
Copy link
Contributor Author

jordo commented Oct 13, 2022

I am not confident on this, but one of the checks may soon fail because there's no documentation defined for these in @GlobalScope, I believe.

I can add the documentation if required. I'm assuming whats on master is applicable here as well.

@jordo jordo requested a review from a team as a code owner October 13, 2022 17:42
@jordo
Copy link
Contributor Author

jordo commented Oct 13, 2022

Added documentation. Matches what's currently in @GlobalScope.xml in master.

BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_HINT_PROPERTY_OF_SCRIPT);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_HINT_OBJECT_TOO_BIG);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_HINT_NODE_PATH_VALID_TYPES);
BIND_GLOBAL_ENUM_CONSTANT(PROPERTY_HINT_SAVE_FILE);
Copy link
Member

Choose a reason for hiding this comment

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

Missing PROPERTY_HINT_ENUM_SUGGESTION.

Copy link
Member

Choose a reason for hiding this comment

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

And the docs check will likely still fail as PROPERTY_HINT_MAX is bound but not documented.
The best is to run --doctool to generate the template from your compiled binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing PROPERTY_HINT_ENUM_SUGGESTION.

Not missed it's just included in a different place from @YuriSizov.
image
image

If you like I can move this around in @GlobalScope.xml to re-organize, or just leave it as is.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see. I guess it was put there because ENUM and ENUM_SUGGESTION are linked, but were not made consecutive in the enum to avoid breaking compat. Can probably be kept as is.

@akien-mga akien-mga added this to the 3.6 milestone Oct 13, 2022
@akien-mga akien-mga added enhancement topic:core cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Oct 13, 2022
@akien-mga akien-mga merged commit 64635af into godotengine:3.x Oct 31, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.2.

@akien-mga akien-mga removed the cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release label Dec 12, 2022
@akien-mga akien-mga changed the title expose additional global enums in 3.x which are available in master Expose all existing PropertyHint global enums Dec 14, 2022
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.

3 participants