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

Script classes make the CreateDialog search filter prohibitively slow #27333

Closed
willnationsdev opened this issue Mar 22, 2019 · 23 comments · Fixed by #86447
Closed

Script classes make the CreateDialog search filter prohibitively slow #27333

willnationsdev opened this issue Mar 22, 2019 · 23 comments · Fixed by #86447

Comments

@willnationsdev
Copy link
Contributor

willnationsdev commented Mar 22, 2019

Godot version:
Godot 3.1

Issue description:
The CreateDialog is incredibly laggy (like, prohibitively laggy) when you have many script classes defined in your project and you try to type into the search filter.

Steps to reproduce:

  1. Create a new project.
  2. Download godot-next from GitHub, extract the .zip, and add the plugin.
  3. Turn the plugin on.
  4. Open the CreateDialog and try typing in the search field.

It will lag incredibly long.

Suggested solution:
I believe this is happening because of runtime checks that the CreateDialog has to perform to track inheritance relationships between script classes, as well as a host of additional logic that allows it to handle engine types, script classes, and custom types. Every time the text_changed signal of the TextEdit fires (so every add or delete of a character), it rebuilds the entire tree, even though none of the data is really changing.

Ideally, we should instead be able to not change any of the TreeItem structure, but toggle the display of certain ones. I'm not sure yet if that is a feature, but if not, we can add it, only update the visualization during text_changed, but rebuild the tree only when script classes have been updated by the EditorFileSystem. This will guarantee fast opening of the CreateDialog, and fast updating of the structure as users type text, resulting in a much more responsive tool.

Bugsquad edit (keywords for easier searching): named class, class_name

@Jeremi360
Copy link

So what you say is that to writer our one CreateDialog or Hack existing one?
Getting all classes is simple just do: ClassDB.get_class_list()

@willnationsdev
Copy link
Contributor Author

So what you say is that to writer our one CreateDialog or Hack existing one?

No, this requires changes to the Tree class and/or the CreateDialog editor class, in the engine source code. (this Issue is for godotengine/godot, not one of our plugins @jebedaia360).

Getting all classes is simple just do: ClassDB.get_class_list()

But this doesn't account for script classes or custom types. A list of custom types isn't even something that is accessible to the scripting API as it is all locked away behind editor code. Furthermore, the issue is about fixing the existing CreateDialog that comes with the editor.

@Jeremi360
Copy link

That is sad. :(
But this doesn't account for script classes or custom types. A list of custom types isn't even something that is accessible to the scripting API as it is all locked away behind editor code. Furthermore, the issue is about fixing the existing CreateDialog that comes with the editor.
All custom classes are added to project settings - we can read it from there.

@willnationsdev
Copy link
Contributor Author

All custom classes are added to project settings - we can read it from there.

This is true of script classes, but not custom types. CustomTypes are registered directly by EditorPlugins and are not serialized anywhere, but rather are stored in an internal HashMap kept inside the EditorNode::EditorData class.

@Calinou
Copy link
Member

Calinou commented May 11, 2019

I can confirm this on fedf9cd.

As a workaround, we could try moving the processing to a thread, or add a small debounce timer after opening the dialog and typing each character, so that input doesn't feel blocked as much.

@pouleyKetchoupp
Copy link
Contributor

I've made this PR that doesn't completely solve this issue, but improves the time it takes to update the tree (~2 times faster) so it makes the filter usable again: #33387

@willnationsdev
Copy link
Contributor Author

willnationsdev commented Nov 6, 2019

The thing that I want to try next is having CreateDialog instances connect to the EditorFileSystem to be notified whenever script classes update and then they can immediately rebuild their Tree. Then the "update" that happens when users type out text would just show/hide various sections of the Tree without actually needing to create a new TreeItem hierarchy with every key press. We would also need to do the same or similar when building the list of items in the Resource dropdown in the Inspector, when adding a Resource instance to an exported resource type (So, CreateDialog and EditorPropertyResource would both need to be notified of script class updates). (Edit: The EditorPropertyResource need is contingent on adding a search filter to it, as suggested in godotengine/godot-proposals#43).

So, @pouleyKetchoupp, your solution will help things out a lot, but I think there's also more we can do here.

@ericdsw
Copy link

ericdsw commented Jan 6, 2020

Can confirm that this is happening to me as well. My project has several script with defined class_names, and every time I try to add a child node to a scene the search dialogue lags horribly for every letter I type.

@PolarisJunior
Copy link

Is this confirmed for 3.2? We have an indie team nearing the final stages of development for a game, and the slowdown on the CreateDialog is causing significant slowdown and development friction. If this won't be fixed by the 3.2 release, then we may have to reduce the usage of of script classes.

@willnationsdev
Copy link
Contributor Author

@PolarisJunior There is a change in 3.2 that caches all the scripts that are used while building the CreateDialog's internal Tree. It significantly improves its performance.

I was going to also start making changes (for 4.0) that would leave the Tree fully created unless script classes change and just toggle visibility of the TreeItems, but it doesn't seem like TreeItem visibility is even a configurable setting (only whether they are collapsed or not). I'll have to add that as a feature if I want to optimize it further.

@akien-mga
Copy link
Member

I tried with the steps to reproduce in OP and it doesn't seem to lag much, if at all, in the current master branch.

Does anyone have a more heavy project to test against?

@willnationsdev
Copy link
Contributor Author

Could probably write a quick EditorScript that saves a new GDScript script class in a loop. Make, like, 1000.

@Calinou
Copy link
Member

Calinou commented Jan 15, 2020

To create 1000 named classes, create a new project and paste this in a terminal:

for i in {1..1000}; do
  echo -e "extends Node \nclass_name MyClass$i\n\nfunc _ready():\n\tpass" > "$i.gd"
done

(On Windows, you can use the WSL to do this.)

@akien-mga
Copy link
Member

Yeah with 1000 named classes it's a bit laggy, it takes ~1 s to show the CreateDialog, and searches including many of the named classes (like in @Calinou's example "MyClass") are laggy.

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Jan 15, 2020
@vnen
Copy link
Member

vnen commented Jan 16, 2020

Yeah, this can get quite laggy, in particular if the scripts have parsing errors and print to the console. My plan is to eventually split GDScript parsing and loading steps, so this could also be sped up by not fully parsing nor looking at dependent resources.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 22, 2020

@willnationsdev I'm curious how much performance gains #27566 provided, I want to resolve this issue in 3.2, at least locally.

The searching part also has to be taken into account. Can easily take around 3-6 seconds just to find a class of interest, depending on the verbosity of a class name.

The merged PR #33387 did provide some performance improvement, but still too slow.

Having around 100 global scripts, Windows.

There are no parsing errors either which would affect this.

@koodikulma-fi
Copy link

This also happens when simply just adding or removing nodes whose classes have been defined in this way (custom classes that show up in add new -popup). Without opening up the add dialog.

@pixaline
Copy link

This also happens to me with 71 class_names. I get up to 2 seconds lag on dialog start up and for each key stroke. In my workflow, I only use Create new node to add base types while dragging and dropping my own custom scripts to nodes - could an option be to add a switch to turn this custom type search on and off? Something like: [X] Show custom classes in editor class lists

@Calinou
Copy link
Member

Calinou commented Jul 12, 2021

I think a decent way to work around this in 3.x would be to implement a dynamic debounce timer based on the time the last search took. If more than 0.3 seconds have elapsed between the keystroke was made and the tree was done updating, set the timer's wait time to 0.25 seconds. By default, set the timer's wait time to a low value like 0.05 seconds (just to ensure typing is always responsive enough, even on small projects).

If anyone is interested on working on this, you can find an example of a LineEdit search field with a debounce timer here: #42402

I recommend opening a pull request against the 3.x branch first, then we can look into forward-porting this to the master branch. The CreateDialog's source code is here: https://github.com/godotengine/godot/blob/3.x/editor/create_dialog.cpp

@Anyeos
Copy link

Anyeos commented Sep 19, 2021

I think it is the same for dropdown dialogs where there are objects inside (Texture, CollisionShapes) because they become very very slow when I have a lot of inherited class_name
I need to inherit the classes so I can code one time and implement only modifications. Actually my project make all dialogs to delay near 10 seconds to show (and freezes all the editor). I need to found a solution for it because it becomes very slow to edit anything.
Already related with #34205

@reduz
Copy link
Member

reduz commented Jan 19, 2023

I tested with 1500 custom classes and while its a tiny bit slow (like 0.2s) its not terrible. It probably should be changed to use the new API I added in #71683 but the performance is not bad. This could be moved to 4.1.

@AncientSion
Copy link

I have this issue with maybe 15 custom classes. Like 2-3 seconds delay until the search updates according to the dropbox. 3.5.2

@Gnumaru
Copy link
Contributor

Gnumaru commented Jul 21, 2023

I had this issue back in 3.x and I continue having it on 4.1.

This is in an empty project, on an Intel Core i7-7700:

2023-07-21_17-20-54.mp4

This is with my current project, on an Intel Core i7-7700:

2023-07-21_17-19-29.mp4

And this is with my current project, on an AMD A10-7850K:

2023-07-22_05-38-52.mp4

I have a lot of scripts but certainly much less than 1500

image

And several of these do not have a class_name, so they don't show up in the create node dialog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment