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

Made hidden ProjectSettings groups more explicit #61818

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Jun 8, 2022

Follow-up to #61751
Changes long if into a more clean list of prefixes.

I experimented with autoloads. At first I tried marking them as internal when added, but the problem is that this isn't saved. The properties are simply loaded with ProjectSettings resource and then re-flagged. So there is no avoiding _get_property_list() or _validate_property() without some bigger changes that just aren't worth it.

@KoBeWi KoBeWi added this to the 4.0 milestone Jun 8, 2022
@KoBeWi KoBeWi requested a review from a team as a code owner June 8, 2022 16:00
@KoBeWi KoBeWi force-pushed the secret_prefix_stash branch from 0b640b0 to d36ebae Compare June 8, 2022 16:44
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 9, 2023
@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@KoBeWi KoBeWi force-pushed the secret_prefix_stash branch from d36ebae to a2700ee Compare August 23, 2023 11:58
@KoBeWi KoBeWi requested review from a team as code owners August 23, 2023 11:58
@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 23, 2023

Rebased. I moved the prefix definition to their respective part of code. I also changed the prefix container from List to LocalVector, which I think is faster.

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.

Note that this doesn't change the editor UI in any way, even when Advanced Settings is enabled.

@AThousandShips
Copy link
Member

It appears to add a lot of extra data to the docs, something broken in the implementation?

@KoBeWi KoBeWi force-pushed the secret_prefix_stash branch from a2700ee to 2cd63a1 Compare August 23, 2023 20:43
@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 23, 2023

Fixed, I had to move input/ prefix back to ProjectSettings.

This is a funny problem. InputMap is created before ProjectSettings, so add_hidden_prefix() can't be used in its constructor. The 2 earliest methods InputMap are load_default(), which is called once in Main when the editor starts, and load_from_project_settings(), which is also called in Main, but is also a bound method, so adding prefix there would cause problems. Apparently doctool runs in non-editor context, hence calling the prefix method in load_default() did not work correctly. I think the best fix would be creating InputMap after ProjectSettings, but it's risky, so I just made an exception.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks much cleaner.

@akien-mga akien-mga merged commit 8ebb347 into godotengine:master Aug 29, 2023
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the secret_prefix_stash branch August 29, 2023 13:32
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.

5 participants