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

[3.x] Add dedicated macros for property name extraction #61141

Merged
merged 1 commit into from
May 19, 2022

Conversation

timothyqiu
Copy link
Member

  • Replaces case-by-case extraction with PNAME & GNAME
  • Fixes group handling when group hint begins with property name
  • Excludes properties that are PROPERTY_USAGE_NOEDITOR

I have a feeling that these are not good macro names. But I can't find a suitable short name for them currently. Any suggestions?

@timothyqiu timothyqiu added this to the 3.5 milestone May 18, 2022
@timothyqiu timothyqiu requested review from a team as code owners May 18, 2022 05:45
@fire-forge
Copy link
Contributor

Is this also needed for subgroups and arrays? There are a few macros that don't seem to be included by extract.py:

  • ADD_ARRAY
  • ADD_ARRAY_COUNT
  • ADD_ARRAY_COUNT_WITH_USAGE_FLAGS
  • ADD_GROUP_INDENT
  • ADD_SUBGROUP
  • ADD_SUBGROUP_INDENT

@timothyqiu
Copy link
Member Author

@fire-forge This is for 3.x so these are not available. I'll add them in a master PR later.

* Replace case-by-case extraction with PNAME & GNAME
* Fix group handling when group hint begins with property name
* Exclude properties that are PROPERTY_USAGE_NOEDITOR
@timothyqiu timothyqiu force-pushed the property-extract-3.x branch from c65b718 to b657d0c Compare May 19, 2022 03:31
@@ -785,13 +785,13 @@ void register_scene_types() {
OS::get_singleton()->yield(); //may take time to init

for (int i = 0; i < 20; i++) {
GLOBAL_DEF("layer_names/2d_render/layer_" + itos(i + 1), "");
GLOBAL_DEF("layer_names/3d_render/layer_" + itos(i + 1), "");
GLOBAL_DEF(vformat("%s/layer_%d", PNAME("layer_names/2d_render"), i + 1), "");
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't you localize layer too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since it's the formated string (layer_1 / Layer 1) that will be sent for localization. Translating layer alone won't work. It's currently a limitation.

@akien-mga
Copy link
Member

Thanks!

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