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

Fix editor freezing and eventually crashing when importing resources per drag & drop #55373

Closed
wants to merge 1 commit into from

Conversation

and-rad
Copy link
Contributor

@and-rad and-rad commented Nov 27, 2021

Fixes #53871

When scanning for new and modified resource files, the EditorFileSystem class creates duplicate item actions when assets are imported via drag & drop. This causes the import process to fail with an error dialog that freezes the engine. Closing the dialog finally crashes the editor. This PR prevents duplicate file actions to be created.

Might also fix #55087, but more information from OP is necessary to be sure.

Details

The EditorFileSystem class checks the file system for changes due to importing, modifying and deleting resources. This is done on multiple threads that fill the scan_actions list with actions to perform on the affected files. It gets processed in EditorFileSystem::_update_scan_actions. After all actions have been processed, the list is reset at the end of that function.

The problem arises because the function can get called again before it reaches the part that clears scan_actions, leaving all the actions that have already been processed in scan_actions, adding new actions on top, and running the function again. This will perform the actions from the previous run again along with the new ones. This causes numerous problems, the most visible ones are the reported issue and the fact that newly imported resources appear to have been imported twice. This is also mentioned in the issue above.

The PR does two things:

  1. An unnecessary item action was removed from the piece of code that reacts to newly imported assets. The same action is already created immediately after and the way it works now seems more consistent to me.
  2. EditorFileSystem::_update_scan_actions handles the processing of scan_actions like a FIFO queue and removes a scanned action from the list as soon as it is done with it. This way, no action will be proccessed twice even if there are new actions added to the end of the queue before the function is done running.

@and-rad
Copy link
Contributor Author

and-rad commented Nov 27, 2021

I should also clarify why I think it might fix #55087. I was able to reproduce that issue, but not consistently, only about 1 in 4 times. Everytime I did, the cause was always the same: Bulk-imported resources had duplicate uids. The code for the TextureButton class looks good to me, so I strongly suspect that there's no problem there.

I can imagine how the behavior that this PR fixes can, under specific and rare circumstances, cause resources to be created with duplicate ids. The fact that it doesn't occur consistently and seemingly randomly fits the kind of bugs that arise in concurrent scenarios. I was not able to test it thoroughly, though, so I might also be completely wrong on that front.

@and-rad
Copy link
Contributor Author

and-rad commented Jun 12, 2022

It's been a while since this PR was posted and I haven't been able to reproduce either of the issues mentioned, so I'm going to close it.

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.

TextureButton textures doesn't match the expected texture [4.0] Error importing resources by drag and drop
3 participants