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 an import preset stored in project settings for textures #55

Merged
merged 2 commits into from
Jul 20, 2022

Conversation

TheOrioli
Copy link
Contributor

This PR will add a new feature, the ability to use a custom texture import preset.

It adds a new checkbox to the config_dialog.tscn
image

The preset is stored in the project settings:
image

It is a replica of the dictionary stored in the project settings when selecting 2D Pixel as saving that as the default import preset.

I couldn't find a good official way of modifying resource import settings without writing a custom plugin that handles every resource of that type. This is worked around by creating a partial .import file for the resource before a filesystem scan is triggered. That way the engine just fills out the unspecified sections of the import file, leaving the wanted parameters untouched.

I also couldn't find an official way of accessing the import presets, so instead there is now an additional file called 2d_pixel_preset.cfg which is read by the plugin and copied into the project settings when needed. It seems that ConfigFile demands an absolute path when loading from file so it's referenced as such.

@viniciusgerevini
Copy link
Owner

This looks great. Thank you.

I also couldn't find an official way of accessing the import presets, so instead there is now an additional file called 2d_pixel_preset.cfg which is read by the plugin and copied into the project settings when needed.

Yeah, I'm also not a big fan of keeping this file, but I can't really think of any better way. It's a good workaround.

I'll just give it a try locally here before I merge, but it looks good to go.

@viniciusgerevini
Copy link
Owner

Hello @TheOrioli . This is working like charm. I have two observations to make.

The first one is that this currently only works for AnimationPlayers. For it to work for SpriteFrames as well, a similar implementation needs to be added to sprite_frames_creator.gd. I like to keep feature parity between both., but I understand you don't use SpriteFrames at the moment. I don't mind doing it myself in case you don't have time for it, as it would be pretty much extracting it and reusing the same thing.

The second thing is that any manual change done via Godot's default Import dock is lost when re-importing the animation. I'm not sure what is the preferable behaviour, always overwrite with the preset or only using it on the first time? As you've been using this feature I believe you have a better understanding what would work best.

@TheOrioli
Copy link
Contributor Author

@viniciusgerevini

implementation needs to be added to sprite_frames_creator.gd

I can give it a pass, no problem 👍

any manual change done via Godot's default Import dock is lost when re-importing the animation. I'm not sure what is the preferable behaviour, always overwrite with the preset or only using it on the first time?

IMO if the user decided to change the import settings for a specific resource it should be respected, but the process of actually checking the import settings for a resource and comparing it is a total nightmare from everything I can tell so far. Of the top of my head, the best we could do is check if a .import file exists for that resource and not delete/overwrite it. I will give it a pass as well 👍

@TheOrioli TheOrioli marked this pull request as ready for review July 19, 2022 12:04
@TheOrioli
Copy link
Contributor Author

PR updated 👍

@@ -59,6 +59,10 @@ func _create_animations_from_file(sprite: AnimatedSprite, options: Dictionary):

if output.empty():
return result_code.ERR_ASEPRITE_EXPORT_FAILED

if _config.is_import_preset_enabled():
Copy link
Owner

Choose a reason for hiding this comment

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

this looks so clean!! 👍

@viniciusgerevini
Copy link
Owner

That's awesome. Thanks a lot. 🎉

I'll merge this PR now, but I'll actually hold a few days before releasing it to the asset lib. I have another feature I'm working on at the moment and I would like to batch them together.

@viniciusgerevini viniciusgerevini merged commit b03bf96 into viniciusgerevini:master Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants