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

Improved search in settings dialogs #33047

Merged
merged 1 commit into from
Jan 31, 2020

Conversation

silvanocerza
Copy link
Contributor

Settings search used to work only on properties, so if a searchbox text
was a substring of a category but not of a property the whole category
would be filtered out and no property would be shown.

Now the behaviour is changed so that when the searchbox text is a
substring of a category all its properties are shown too.

The previous behaviour is still present so that in case the searchbox
text is both a substring of a category and a property of another
category, all properties of the first category are shown and only the
property of the second category is shown.

Fixes #33005

settings-search

@@ -1595,7 +1595,7 @@ void EditorInspector::update_tree() {
if (capitalize_paths)
cat = cat.capitalize();

if (!filter.is_subsequence_ofi(cat) && !filter.is_subsequence_ofi(name))
if (!filter.is_subsequence_ofi(cat) && !filter.is_subsequence_ofi(name) && property_prefix.to_lower().find(filter.to_lower()) == -1)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder why this is needed? The previous code was already checking if the filter is either in the property name or the rest of its path (cat), so here it should have been working fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cat here was always an empty String, probably there's an issue somewhere else?

Copy link
Member

Choose a reason for hiding this comment

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

Judging by the code, it's not meant to be empty, otherwise there wouldn't be a point in testing it. So yeah, would be worth investigating deeper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like when the EditorInspector is used in the engine Inspector cat is correctly set and property_prefix is empty, viceversa when it's used either in the project or editor settings.

Those variable are deduced from the EditorInspector's property list on line 1463 and it has been this way since 005b69cf6e when editor_inspector.h/cpp have been created, so my guess is that it's expected to be like so. Don't know if I'm missing something.

@silvanocerza silvanocerza force-pushed the settings-search branch 3 times, most recently from 0c998c0 to 969de15 Compare October 30, 2019 15:17
Settings search used to work only on properties, so if a searchbox text
was a substring of a category but not of a property the whole category
would be filtered out and no property would be shown.

Now the behaviour is changed so that when the searchbox text is a
substring of a category all its properties are shown too.

The previous behaviour is still present so that in case the searchbox
text is both a substring of a category and a property of another
category, all properties of the first category are shown and only the
property of the second category is shown.
@akien-mga akien-mga self-requested a review November 7, 2019 12:19
@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 21, 2019
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 21, 2019
@akien-mga akien-mga merged commit 1361fa7 into godotengine:master Jan 31, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.1.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 14, 2020
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.

Project Settings search brings weird results
3 participants