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

Import scenes as AnimationLibrary #60177

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

reduz
Copy link
Member

@reduz reduz commented Apr 12, 2022

Added the ability to import scenes as AnimationLibrary

Creates a secondary scene importer used only for animations.

Screens of usage:

Simply choose any scene file and change the import type from Scene to AnimationLibrary:
image
Then, the resulting file can be imported into an existing animation player as a library:
image

NOTE: A new flag for scene importer: EditorSceneFormatImporter.IMPORT_DISCARD_MESHES_AND_MATERIALS has been added, to hint importers that they should skip meshes and animations (and hence make importing faster). It is not implemented in any importer yet, this should be done in a separate PR.

Bonus: A couple of bugs fixed in the advanced import settings regarding to option visibility.

bugsquad edit:

Fixes: #60060

@Calinou Calinou added this to the 4.0 milestone Apr 12, 2022
@reduz reduz force-pushed the animation-library-import branch from 59d3324 to 83bd89a Compare April 12, 2022 21:35
@reduz reduz marked this pull request as ready for review April 12, 2022 21:35
@reduz reduz requested review from a team as code owners April 12, 2022 21:35
@reduz reduz force-pushed the animation-library-import branch 2 times, most recently from 1c8dddf to 1b2783b Compare April 13, 2022 07:02
@fire
Copy link
Member

fire commented Apr 13, 2022

reduz/godot@animation-library-import...V-Sekai:animation-library-gltf

I noticed some odd points.

  1. preview pre import always show the meshes
  2. In my branch when I click on the gltf on the file system it changes the import type from animation library back to scene (godot filesytem dock).
  3. Added support for the IMPORT_DISCARD_MESHES_AND_MATERIALS flag to gltf import but preimport doesn't know we want to discard the animation.

Copy link
Member

@fire fire left a comment

Choose a reason for hiding this comment

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

See the comments in the post before.

The filesystem swap isn't blocking work on animation retargeting and since @akien-mga and others will be away for a holiday I'll approve. In my testing the scene does seem to be usable as a resource that can be loaded from an animation player.

Added the ability to import scenes as AnimationLibrary

* Completes implementation of godotengine/godot-proposals#4296
* Helps if you want to export animations to a separate file (say a GLTF) to avoid re-importing/exporting them every time the model changes.
* Helps if you simply want to have animations using a dummy model, which can be shared across multiple models.

Creates a secondary scene importer used only for animations.

**NOTE**: A new flag for scene importer: EditorSceneFormatImporter.IMPORT_DISCARD_MESHES_AND_MATERIALS has been added, to hint importers that they should skip meshes and animations (and hence make importing faster). It is not implemented in any importer yet, this should be done in a separate PR.
@reduz reduz force-pushed the animation-library-import branch from 1b2783b to 6600931 Compare April 13, 2022 13:07
@reduz
Copy link
Member Author

reduz commented Apr 13, 2022

reduz/godot@animation-library-import...V-Sekai:animation-library-gltf

I noticed some odd points.

1. preview pre import always show the meshes

2. In my branch when I click on the gltf on the file system it changes the import type from animation library back to scene (godot filesytem dock).

3. Added support for the IMPORT_DISCARD_MESHES_AND_MATERIALS flag to gltf import but preimport doesn't know we want to discard the animation.

Should be working now, this happened if you click re-import in the advanced dialog.

@fire
Copy link
Member

fire commented Apr 13, 2022

The change to provide the right class looks correct. Not able to compile quickly.

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.

Code looks good.

It seems a bit weird/uncommon for the Godot codebase to have two different singletons for instances of the same class, but I see how it's convenient here.

@reduz
Copy link
Member Author

reduz commented Apr 13, 2022

@akien-mga There are a couple of cases of the same importer for multiple classes (like LayeredTexture) using a single class, but the singleton is indeed strange. I cant think of a way to better workaround it better to be honest.

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

I tested, import is working correctly. Possibly we may need to change later on the dealing with the RESET Track (Context: #60093), but that is no reason to block this PR.

@akien-mga akien-mga merged commit 970debe into godotengine:master Apr 13, 2022
@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.

Blender options should be hidden from non-blender imports
5 participants