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

Allow to specify target folder when installing assets #81620

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented Sep 13, 2023

Closes godotengine/godot-proposals#554 with a user-facing solution. We can allow to pre-populate this new property from some asset configuration file in the future, but it should be good enough for now (you can just tell users where to extract your files in the instruction, if that matters).

I've rearranged some things and split the tree in two. On the left we show the contents of the asset archive, intact. On the right we show the end result of the installation/extraction.

godot windows editor dev x86_64_2023-09-19_20-13-00 godot windows editor dev x86_64_2023-09-19_20-15-21
Old screenshot godot windows editor dev x86_64_2023-09-13_19-07-54

The code is a bit messy, so feel free to tell me I've made some bad mistakes 🙃 For some reason I'm having a bit of trouble with it today, and juggling two trees, remappings, and the installation itself in an efficient manner proved to me a confusing challenge. But I think I got it in order and after a few tests I don't see any immediate issues. I also fixed a couple of existing mistakes with caches not being cleaned up properly or methods being called in a wrong place. So it should overall be a bit better now.

Here's a bit of a walkthrough (with a slightly older version, see updates in comments):

godot.windows.editor.dev.x86_64_2023-09-13_18-57-43.mp4

@aaronfranke
Copy link
Member

aaronfranke commented Sep 13, 2023

I'm not a fan of the default folder being at the root with the name being the asset name with a long UUID at the end. I would prefer if the asset path was something like addons/something. Maybe this could be specified on the asset library, or maybe it can be detected (if the asset has addons/ in it, extract at the root to keep the folder structure as-is, if not, extract into addons/something).

@YuriSizov
Copy link
Contributor Author

I'm not a fan of the default folder being at the root with the name being the asset name with a long UUID at the end.

That's the contents of the archive generated by GitHub. That's not what is going to be extracted to the project.

@YuriSizov YuriSizov force-pushed the assets-remap-install-folder branch from 5e6ca9b to 644f780 Compare September 13, 2023 17:38
@YuriSizov
Copy link
Contributor Author

I think I want to hide the left tree by default let the user open it if they want to customize the files installed (via a dedicated button, but also via the "N conflicts" status). I don't think selecting individual files is the behavior needed by default most of the time, and having two trees like that adds to the confusion.

I'll try to implement the change today or tomorrow.

This also changes the layout of the installer window to
better separate configuration of the installation and the
expected output.
@YuriSizov YuriSizov force-pushed the assets-remap-install-folder branch from 644f780 to ef80a2b Compare September 19, 2023 18:10
@YuriSizov
Copy link
Contributor Author

Okay, done. It should be less confusing now:

godot.windows.editor.dev.x86_64_2023-09-19_20-07-59.mp4

@Mickeon
Copy link
Contributor

Mickeon commented Sep 25, 2023

The "Ignore asset root" toggle is very, very welcome. No more accidental READMEs or LICENSE files being dropped where I don't want them to be.

@YuriSizov
Copy link
Contributor Author

YuriSizov commented Sep 25, 2023

The "Ignore asset root" toggle is very, very welcome. No more accidental READMEs or LICENSE files being dropped where I don't want them to be.

That's already merged in 4.2 :) See #81358. But also, it's not exactly for that. It's to account for the ZIP-file structure of GitHub archives.

@Mickeon
Copy link
Contributor

Mickeon commented Sep 25, 2023

It's much better than outright not having this feature. Although, I do feel like when both Trees are displayed, the "Installation Preview" is a bit useless. It shows a lot of of the same information that can be gathered from looking at the Tree on the left. The only "new" thing that cannot be discerned without it, is the installation root path.

Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@akien-mga akien-mga merged commit 8f0609c into godotengine:master Sep 26, 2023
@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.

Allow extracting asset library assets into subfolders
4 participants