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

Expose 'Reimport' on right-click context menu in the FileSystem panel #75137

Merged
merged 1 commit into from
Aug 7, 2023

Conversation

nongvantinh
Copy link
Contributor

@nongvantinh nongvantinh commented Mar 20, 2023

editor/import_dock.cpp Outdated Show resolved Hide resolved
editor/import_dock.cpp Outdated Show resolved Hide resolved
@KoBeWi
Copy link
Member

KoBeWi commented Apr 30, 2023

_reimport_attempt() for multiple files will reimport all files with the same type as the first selected file. You can e.g. select texture and audio file and clicking Reimport will trigger the warning about changing import type. If you accept, it will corrupt the imported files.

Either the function needs to be changed or _reimport() shouldn't appear if there is more than 1 type of resources selected.

@nongvantinh
Copy link
Contributor Author

I understand. I tested the import tab in the current version and found that when I try to import two different file types, the editor doesn't let me reimport them. To maintain consistency, I'll try to implement the same functionality as the editor.

@KoBeWi
Copy link
Member

KoBeWi commented May 1, 2023

The implementation can be much simpler:

bool resource_valid = true;
String main_extension;

for (int i = 0; i != p_paths.size(); ++i) {
	String extension = p_paths[i].get_extension();
	if (extension_list.has(extension)) {
		if (main_extension.is_empty()) {
			main_extension = extension
		} else if (extension != main_extension) {
			resource_valid = false;
			break;
		}
	} else {
		resource_valid = false;
		break;
	}
}

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Aug 7, 2023
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.

Seems useful.

@akien-mga akien-mga merged commit bbfa74a into godotengine:master Aug 7, 2023
13 checks passed
@akien-mga
Copy link
Member

Thanks!

@nongvantinh nongvantinh deleted the implement-6320 branch August 7, 2023 18:53
@dogboydog
Copy link
Contributor

dogboydog commented Aug 22, 2023

Hey, thank you very much for implementing this! Cool to see a proposal I made actually make it in!

just tried this in the 4.2 dev3 snapshot. It seems like this doesn't work for all types? For instance, when using the https://github.com/Kiamo2/YATI plugin, .tmj files don't show Reimport
image

This was actually the use case that I created the proposal for, to reimport my tilemaps easily. Another plugin I'm using, that I wrote myself, does have the Reimport option
image

Any idea why it wouldn't show up for some resources?

@KoBeWi
Copy link
Member

KoBeWi commented Aug 22, 2023

Maybe there is something wrong with the YATI implementation, like missing function or something. Doesn't it print any errors when used? Can you show the code of your plugin?

@dogboydog
Copy link
Contributor

I don't think I have errors from YATI but I can check later. This is my importer code which does show Reimport:

https://github.com/dogboydog/YarnDonut/blob/godot4/addons/YarnDonut/Editor/YarnImporter.cs

@KoBeWi
Copy link
Member

KoBeWi commented Aug 22, 2023

YATI returns "Node2D" in GetResourceType(), while the method expects Resource type. I think it should be PackedScene in this case.

@dogboydog
Copy link
Contributor

dogboydog commented Aug 22, 2023

Thanks, I'll let the developer know. Edit: confirmed that was the issue, thank you

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.

Expose 'Reimport' on right-click context menu in the FileSystem panel
5 participants