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 FileSystemDock thumbnails sometimes not displaying #91471

Merged
merged 1 commit into from
May 11, 2024

Conversation

aaronp64
Copy link
Contributor

@aaronp64 aaronp64 commented May 2, 2024

There were (at least) three cases where thumbnails would not display, if they were generated while the FileSystemDock was not visible:
- current_path == "Favorites", due to p_path not starting with "Favorites"
- current_path == "res://", due to current_path having last "/" trimmed for comparison
- current_path pointing to a selected file instead of folder, since it no longer matches p_path's base directory

This change removes the current_path and is_visible_in_tree checks when determining whether to update the file's icon.

Fixes #90801
Fixes #91432

There were (at least) three cases where thumbnails would not display, if they were generated while the FileSystemDock was not visible:
	- current_path == "Favorites", due to p_path not starting with "Favorites"
	- current_path == "res://", due to current_path having last "/" trimmed for comparison
	- current_path pointing to a selected file instead of folder, since it no longer matches p_path's base directory

This change removes the current_path and is_visible_in_tree checks when determining whether to update the file's icon.

Fixes godotengine#90801
Fixes godotengine#91432
@aaronp64 aaronp64 requested a review from a team as a code owner May 2, 2024 17:08
@aaronp64
Copy link
Contributor Author

aaronp64 commented May 2, 2024

There's also the option of adding additional checks for each specific case. It seemed safer to just remove the checks, so we don't have to worry about missing cases, and the work of generating the preview texture has already completed anyway. If there's concerns about performance, I could change this to add checks for each of the listed cases.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Interestingly, last time this line was modified was in #87121, which fixed a similar issue 🤔
The change looks safe. I tried all file dock positions and it works.

@akien-mga akien-mga merged commit ea552e1 into godotengine:master May 11, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@aaronp64 aaronp64 deleted the filesystemdock_previews branch June 4, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants