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

Add get_remapped_files() method to Directory #59334

Closed

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Mar 19, 2022

Resolves #25672

I clarified the description of get_files() to mention what happens on export and added get_remapped_files() method. The method returns the list of original files based on remaps. The list is consistent between editor and exported project.

For example, the original list of files returned by get_files() when running from editor:

[Chicken.png, Chicken.png.import, Chicken.tscn, Chicken.wav, Chicken.wav.import, Egg.png, Egg.png.import, Rooster.tscn]

List from get_remapped_files():

[Chicken.png, Chicken.wav, Egg.png]

get_files() in exported project:

[Chicken.png.import, Chicken.tscn.remap, Chicken.wav.import, Egg.png.import, Rooster.tscn.remap]

^ this is different than in editor (hence infinite confusion for some users)
List from get_remapped_files():

[Chicken.png, Chicken.tscn, Chicken.wav, Egg.png, Rooster.tscn]

^ same as in editor.

get_remapped_files() handles .import, .remap and .gdc files (the latter is probably not needed anymore, see #59241), but maybe there are more files that differ in export?

@KoBeWi KoBeWi added this to the 4.0 milestone Mar 19, 2022
@KoBeWi KoBeWi requested review from a team as code owners March 19, 2022 23:58
@KoBeWi KoBeWi force-pushed the nobody_expects_the_icon.png.import branch from d790037 to 2e743e2 Compare March 20, 2022 00:45
@reduz
Copy link
Member

reduz commented Mar 29, 2022

I would approach this a bit different. Since the exporter knows everything that is removed and remapped, the exporter could keep track of all this and then add a file to the project like

res://.godot/file_remaps.cfg

The Godot directories API can then try to open this file and if found, then it will simply remap them. It would be only useful for directories (files does not need except for File::exists).

@KoBeWi
Copy link
Member Author

KoBeWi commented Apr 3, 2022

You mean e.g. loading icon.png.import should load the icon.png? This doesn't solve the problem that the exported directory content is different and confuses the users.

@akien-mga
Copy link
Member

Check references to remap in EditorExportPlatform::export_project_files in editor/editor_export.cpp. This function keeps track of the remappings and this data could be serialized in a file that can be reused by Directory to expose the remaps in a user-friendly way.

@reduz
Copy link
Member

reduz commented Apr 5, 2022

I will be doing a few changes to the exporter soon because I need to complete the server export target based on the work by @Faless. I can take the chance and implement this as I described as it should not be so complex.

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 20, 2022

Ah I think I now understand what you meant in the first comment 😅 (partially at least)

I get that I can obtain remaps in a more reliable way, but it can't be used by get_files(). From what I understand, you suggest to hijack files returned by get_files() and turn .import files into their original paths using the available remap database. The thing is, this won't work when running from the editor, because in editor you get list of both original files and imports. Unless I should just change the implementation of my method, but the way it works is actually ok 🤔

@YuriSizov YuriSizov modified the milestones: 4.0, 4.x Feb 6, 2023
@nonchip
Copy link

nonchip commented Feb 17, 2023

yeah this "simply rename .import to nothing and .gdc to .gd" thing does the opposite of get_remapped_files(), instead it remaps (badly) again the already remapped files. i think we can all agree this is not a solution but rather an ugly hack :P

especially as demonstrated by:

As you can see, get_remapped_files() skips .tscn, because it's not remappable.

being clearly untrue, given .scn (and .remap; globally) existing ;)

also:

but maybe there are more files that differ in export?

yes, potentially every one. that's why hardcoding string replacements will never cut it. you'll need to access the current map somehow.

@KoBeWi

I get that I can obtain remaps in a more reliable way, but it can't be used by get_files().

can you explain that? why can't you just simply ask the ResourceCache or similar about the actual list of files being remapped right now, and then filter that "backwards" onto the result of get_files? because the fact load, Resource.take_over_path and the likes can figure out the forward mapping should very much mean you should be able to apply it in reverse? and the fact you can load packs at runtime which were built with varying export options should mean each one should have some idea of how it was remapped, right?

see also ResourceLoader::[_]path_remap and ResourceLoader::import_remap/ResourceFormatLoader::recognize_path, I think you "just" need to do the reverse of that logic as applicable.

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 19, 2023

I tried using path_remap() and import_remap() and e.g. for icon.png it returns the .ctex file in .godot/imported, while it doesn't touch icon.png.import. I think the .import files are not recognized at all, i.e. they are just metadata that doesn't map to the original file.

I'll rebase the PR and make it include all files in the directory, mapping imports to original files and removing duplicates. So it will be the same as get_files(), but consistent between editor and exported project.

being clearly untrue, given .scn (and .remap; globally) existing ;)

It was true at the time of writing this .-.

@KoBeWi KoBeWi force-pushed the nobody_expects_the_icon.png.import branch from 2e743e2 to 123f8d6 Compare February 19, 2023 13:43
@nonchip
Copy link

nonchip commented Feb 20, 2023

@KoBeWi

I tried using path_remap() and import_remap() and e.g. for icon.png it returns the .ctex file in .godot/imported, while it doesn't touch icon.png.import. I think the .import files are not recognized at all, i.e. they are just metadata that doesn't map to the original file.

that is kinda expected yeah, the .import file just contains the import settings for that asset, and the .godot/imported/... path that contains the actual data.

I'll rebase the PR and make it include all files in the directory, mapping imports to original files and removing duplicates. So it will be the same as get_files(), but consistent between editor and exported project.

yeah i think that's probably the best approach, just "unmapping" as much as possible from the export process in the result. that's not what you're doing in 123f8d6 though, still renaming according to a list.

being clearly untrue, given .scn (and .remap; globally) existing ;)

It was true at the time of writing this .-.

was it? arbitrary asset remapping (e.g. per-language and per-buildprofile) and binary export formats have existed in some form since 2.x iirc?

and the former (especially the even worse case: completely arbitrary and nondeterministic per-exportplugin! i can totally see game projects run their own remapper plugins to customize builds/etc) is the reason a "remapping dictionary" like you're building can't ever work reliably, i'm afraid. you'll somehow have to either grab that info from the runtime VFS, or, if not available there, maybe export the metadata as part of the rewriting going on during said export and then read that back in? essentially providing Path.map_for_export and Path.unmap_from_export functions, which you can then also easily map your get_files result through.

@KoBeWi
Copy link
Member Author

KoBeWi commented Feb 20, 2023

So again, when you list files from the editor, you get icon.png, icon.png.import. When listing from exported project, you get icon.png.import. From what I gathered, there is no information outside of the .import file that relates it to the original and reading the file to obtain the original path is as arbitrary as removing the suffix.

A proper unmapping requires a new system that would handle these files. I'm not particularly interested in implementing it, as @reduz clearly has some idea and I was waiting for his implementation.

@horizoncarlo
Copy link

horizoncarlo commented Sep 3, 2024

This would be great to get merged in. In the meantime I'm using a wrapper function that behaves consistently between in-editor and in-exported versions of my project:

static func get_remapped_files(directory: String) -> Array:
	var allFiles = DirAccess.get_files_at(directory)
	var remappedFiles = []
	for file in allFiles:
		if file.ends_with('.import'):
			remappedFiles.append(file.replace(".import", ""))
	return remappedFiles

alternatively with fewer lines and variables:

static func get_remapped_files(directory: String) -> Array:
	var allFiles = DirAccess.get_files_at(directory)
	return allFiles.filter((file) -> file.ends_with('.import')).map((file) -> file.replace(".import", ""))

@nonchip
Copy link

nonchip commented Sep 4, 2024

for the record, @horizoncarlo found it necessary to send me a private email from a non-replyable address, ordering me to tell him "the alternative to DirAccess.get_files_at", to which I will refer to literally this whole thread as to why a) get_files_at isn't the problem here and b) his whole approach is.

even though clearly he read it because he thumbsdown-reacted multiple correct posts (interestingly exclusively and all of mine, so totally not petty). someone needs to read the CoC.

EDIT: and now he's chosen to contact me via the same email directly again, apparently not being able to get any hint.
@horizoncarlo if you have something to say about this issue, comment in the issue. rage-emoting and personally harrassing me won't get you anywhere else than my spam filter.

@KoBeWi
Copy link
Member Author

KoBeWi commented Oct 7, 2024

Superseded by #96590

@KoBeWi KoBeWi closed this Oct 7, 2024
@KoBeWi KoBeWi removed this from the 4.x milestone Oct 7, 2024
@KoBeWi KoBeWi added the archived label Oct 7, 2024
@KoBeWi KoBeWi deleted the nobody_expects_the_icon.png.import branch October 7, 2024 10:28
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.

Listing directories in exported project returns only *.import files
7 participants