Skip to content

Improve handling of archives when installing assets - #81358

Merged
akien-mga merged 1 commit into
godotengine:masterfrom
YuriSizov:assets-install-however-you-want
Sep 8, 2023
Merged

akien-mga merged 1 commit into
godotengine:masterfrom
YuriSizov:assets-install-however-you-want

Conversation

@YuriSizov

@YuriSizov YuriSizov commented Sep 5, 2023

Copy link
Copy Markdown
Contributor

Fixes #81311. Closes #75712.

So #75712 has been a problem for quite some time now. As discussed, there are two key points:

  • Installing ZIPs manually is treated differently than installing assets from the AssetLib (despite handling the same ZIPs internally);
  • All ZIPs are expected to have a root directory in them, because that's how GitHub, etc pack their ZIPs.

And #81311 is a new finding, as far as I know. It seems some ZIP archives can skip the directory structure information and just contain record of files without intermediate folders. We fail at handling those gracefully.


So I set to fix these problems, and along the way I tweaks smaller details here and there. Here's a list of changes:

  • Implicitly create folders when parsing ZIP files to make sure we have the entire structure available regardless of the source ZIP.
  • Add a checkbox that allows to skip the root directory when extracting files, if such directory exists.
  • Fix formatting of the "Files failed to extract" error message that was missing a line break.
  • Fix some errors not being displayed at all (also remove the unnecessary dialog and use toasts instead).
  • Improve the status bar to better highlight the title of the asset and the status on the conflicting files (also use TTRN for that last message).
  • Rename the window to explain what it's for.
  • Various refactoring for code clarity and slightly better organization (this is clearly a very old file, and it leaves a lot to be desired, so I kept the changes to rather minimal).
godot windows editor dev x86_64_2023-09-05_23-54-53 godot windows editor dev x86_64_2023-09-05_23-54-28 godot windows editor dev x86_64_2023-09-06_00-05-32

Most of it should be self explanatory. Regarding the checkbox, it tries to be smart. First of all, it's disabled if there is no single root folder in the asset archive. It only becomes available if your asset has a folder structure similar to what GitHub creates. This may backfire when your ZIP file contains, for example, only the addons folder — but if that becomes an issue we should be able to easily add an exception.

I've decided to preserve the current dual logic, where assets installed through the AssetLib have their root folder skipped, while archives installed manually don't behave this way. However, this is no longer a hardcoded behavior. Instead, the checkbox is either checked by default or not, but can still be changed by the user.

Here's everything in action (note that the image loading error is related to the broken asset and is not a part of this PR):

godot.windows.editor.dev.x86_64_2023-09-05_23-53-18.mp4

@Trey2k

Trey2k commented Sep 6, 2023

Copy link
Copy Markdown

Would it be possible to modify what info the editor retries from the asset library to verify if the repo host is custom or not? I think that could potentially be a cleaner approach as then we would know for certain how it should be handled in either case.

EDIT: This would not resolve issues with manual imports still, but it would help the checkbox be more accurate in assumptions for direct downloads.