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

Node create dialog filtering optimization #33387

Merged

Conversation

pouleyKetchoupp
Copy link
Contributor

Avoid loading the same scripts several times to parse them in order to update the node type tree.

Improvement for #27333
(~2 times faster to update tree)

Copy link
Contributor

@willnationsdev willnationsdev left a comment

Choose a reason for hiding this comment

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

These changes look good to me. Thanks for getting this change into 3.2!
I have more optimizations I'd like to do here for 4.0, but this should definitely help with the load for now.

@@ -321,6 +325,9 @@ void CreateDialog::_update_search() {

TreeItem *to_select = search_box->get_text() == base_type ? root : NULL;

// Keep references to loaded scripts to avoid loading and parsing them several times
HashMap<String, RES> loaded_scripts;
Copy link
Contributor

Choose a reason for hiding this comment

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

While not necessary @pouleyKetchoupp, would it be possible to have this be a variable in the class itself rather than a local variable? And then just clear it whenever the "config" method, or maybe the "popup" method, is called? That way it doesn't have to rebuild this map on every key press, but rather keeps the scripts loaded the entire time the user has it open.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, let me look into that.

Avoid loading the same scripts again and parse them when updating the node type tree.
@pouleyKetchoupp pouleyKetchoupp force-pushed the faster-create-dialog-filter branch from ded32f4 to fb9ff92 Compare November 6, 2019 16:21
@pouleyKetchoupp
Copy link
Contributor Author

Thanks @willnationsdev for your feedback.
I've pushed the change to keep loaded scripts while the popup is open, so now it's completely smooth while typing in the search filter box!

@willnationsdev
Copy link
Contributor

Great! I think things here are good to go @akien-mga.

@akien-mga akien-mga merged commit 74c4543 into godotengine:master Nov 6, 2019
@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