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 animation preview functionality and button to SceneImportSettings dialog #76381

Conversation

jeronimo-schreyer
Copy link
Contributor

Added a button to quickly preview animations in the 3D Preview Viewport

On branch add_animation_playback_in_3d_resource_importer
Changes to be committed:
modified: editor/import/scene_import_settings.cpp
modified: editor/import/scene_import_settings.h

Added a second light for the 3D Preview Viewport and UI toggle buttons which resemble the ones in the EditorMaterialPreviewPlugin

 On branch add_secondary_light_to_3d_resource_importer
 Changes to be committed:
	modified:   editor/import/scene_import_settings.cpp
	modified:   editor/import/scene_import_settings.h
@jeronimo-schreyer jeronimo-schreyer requested a review from a team as a code owner April 23, 2023 20:10
@jeronimo-schreyer jeronimo-schreyer force-pushed the add_animation_playback_in_3d_resource_importer branch 2 times, most recently from 9798a3d to 82ceb0a Compare April 23, 2023 20:15
@Calinou Calinou added this to the 4.x milestone Apr 23, 2023
@fire
Copy link
Member

fire commented Apr 23, 2023

Hi, do you know if thing this cherry-pickable to 4.0?

Also some errors in the cicd.

diff.patch

@jeronimo-schreyer
Copy link
Contributor Author

sorry, getting used to the commit workflow. I'm fixing those right now.

The changes are not a big deal, I'll test it in 4.0

@jeronimo-schreyer jeronimo-schreyer force-pushed the add_animation_playback_in_3d_resource_importer branch from 82ceb0a to 0349416 Compare April 23, 2023 20:37
@jeronimo-schreyer jeronimo-schreyer force-pushed the add_animation_playback_in_3d_resource_importer branch from 0349416 to f0768f1 Compare April 24, 2023 02:58
@clayjohn
Copy link
Member

Related #76367

@fire fire requested a review from a team May 22, 2023 17:55
@fire fire modified the milestones: 4.x, 4.1 May 22, 2023
@TokageItLab
Copy link
Member

TokageItLab commented May 25, 2023

FW from #76367:

I compared #76367 and #76381, both have good and not good points.

#76367 has a seek bar. It is good that you can check the playback position with it. However, the negative is that the loop option is not linked to the import option for the preview. Since users may complain after importing that it is not in the same state as the preview, they should be linked.

image

#76381 does not have a seek bar, but it is preferable that the loop option for preview be linked to the import option. However, I recommend #76367 for animation because #76367 is more valuable for QOL since it has a seek bar.

BTW, the light settings for preview in #76381 are useful. So it is better to omit the animation feature and brush up #76381 for the preview environment option.

image

@fire
Copy link
Member

fire commented Jun 10, 2023

@TokageItLab prefers #76367 because it has better ux but has some wrong implementations

@jeronimo-schreyer
Copy link
Contributor Author

closing then. If anything, I can help with the implementations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Added playback control to animations in the Advanced Import Settings preview
6 participants