Fix animation loop import hints becoming lost#91634
Fix animation loop import hints becoming lost#91634Repiteo merged 1 commit intogodotengine:masterfrom
Conversation
jogly
force-pushed
the
gilley/fix-anim-loop-import
branch
from
6ba3d88 to
72c15c5
Compare
jogly
force-pushed
the
gilley/fix-anim-loop-import
branch
from
72c15c5 to
5337886
Compare
aaronfranke
left a comment
aaronfranke
left a comment
There was a problem hiding this comment.
I spent about 10 minutes looking around this file reviewing this change. Disclaimer: I'm not an expert in the SceneImportSettingsDialog code.
I looked around to see what was the intention here, to see what is the precedent for handling loading the settings, because I was suspicious as to why similar code did not exist next to this code, and why nothing else was altering the data after it was read from _load_default_subresource_settings.
It seems that the precedent is that... there was no existing code handling the same thing. "settings/loop_mode" is the only thing in here that behaves like this. As far as I can tell this is the only case where we need to round-trip read it back into the settings, because in most cases the dialog is the source of truth, but this is a special case where the animation looping can be set to true without the dialog's help.
Anyway, this seems correct, and the code looks very safe (it only changes the dialog's loop mode setting if it was not already set), so I'm approving this PR.
|
This got a bit lost in the folds, could you rebase this PR? See our pull request guidelines for more information |
lyuma
left a comment
lyuma
left a comment
There was a problem hiding this comment.
looks good based on aaron's explanation.
|
Just tested locally, and this will actually merge as-is, so I'm gonna say this is good to go! |
github-project-automation
bot
moved this from Ready for review
to Done
in Animation Team Issue Triage
|
Thanks! Congratulations on your first merged contribution! 🎉 |
7 participants
The scene importer settings dialog was not respecting the inferred loop mode of animation clips. As long as the scene is never reimported via the advanced settings dialog, the import settings for clips would be correct. Opening the advanced importer loads all animation settings with their default values, and confirming serializes those values. Even making zero changes will result in the animation clips reverting to all default values losing any import hints.
This change sets expected loop mode based on the clip information when creating
AnimationDatafor a clip.I confirmed this works as expected now by:
-loopimport hint if set manuallyFixes #75912