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

Changed the track type inserted into the animation by the Inspector key button #54187

Closed

Conversation

TokageItLab
Copy link
Member

@TokageItLab TokageItLab commented Oct 24, 2021

Note: RotationTrack insertion is broken. Euler and Quaternion conversions are needed, but maybe #54084 merge is needed for them.


For performance considerations, the following 4 track types have been added to the animation by #53689 and #53865.

  • Position3DTrack
  • Rotation3DTrack
  • Scale3DTrack
  • BlendShapeTrack

However, the insertion of tracks from the Inspector's key buttons is still only allowed for ValueTrack or BezierTrack. BezierTrack still has its place, but for certain properties, we will change it to insert a new type of track instead of ValueTrack.

Although, there is no foolproof mechanism implemented to prevent ValueTrack from being directly generated or the pathname of ValueTrack from being rewritten by user interaction, it is outside the scope of this PR.

@TokageItLab TokageItLab requested review from a team as code owners October 24, 2021 10:48
@TokageItLab TokageItLab changed the title Fixed animation insertion in Inspector Changed the track type inserted into the animation by the Inspector key button Oct 24, 2021
@TokageItLab TokageItLab force-pushed the fix-key-insertion branch 2 times, most recently from b7447bb to ede074a Compare October 24, 2021 10:58
@Rubonnek Rubonnek added this to the 4.0 milestone Oct 24, 2021
@TokageItLab TokageItLab force-pushed the fix-key-insertion branch 4 times, most recently from 18afa7c to 8fd3600 Compare October 26, 2021 04:15
@TokageItLab TokageItLab requested a review from a team as a code owner October 26, 2021 04:15
@TokageItLab TokageItLab marked this pull request as draft October 26, 2021 05:22
@TokageItLab TokageItLab marked this pull request as ready for review October 26, 2021 06:43
@TokageItLab
Copy link
Member Author

Bezier tracks parse a single property into multiple tracks and insert them into the animation. So it was a bit of a hacky implementation to check if a Bezier track already exists. I think it should be refactored in the future when Bezier tracks are more organized.

@TokageItLab TokageItLab force-pushed the fix-key-insertion branch 2 times, most recently from fb41ef1 to e7018c6 Compare October 26, 2021 08:32
@TokageItLab TokageItLab marked this pull request as draft October 26, 2021 10:20
@TokageItLab TokageItLab marked this pull request as ready for review October 26, 2021 10:33
@reduz
Copy link
Member

reduz commented Nov 9, 2021

I would just harcode the property paths that require these special keys in the editor, its anvery special case, not worth really doing it flexible.

@TokageItLab
Copy link
Member Author

TokageItLab commented Nov 9, 2021

@reduz If there is no way to choose between Bezier tracks like in Skeleton bone's pose, simple hardcoding is fine.
However, if we want to allow the choice of Bezier track, such as in Node3D Transform, hard coding will cause code duplication, to avoid it, I ended up with a slightly larger implementation.

@TokageItLab TokageItLab force-pushed the fix-key-insertion branch 2 times, most recently from 83f8472 to c251f38 Compare November 10, 2021 06:57
@TokageItLab
Copy link
Member Author

@reduz If we expect to insert Bezier tracks in TRS/Blends tracks, you need to implement this.
But, if we don't want to insert Bezier tracks in the TRS/Blends track, but want to access the Bezier editor from the TRS/Blends track, you are right, a simpler implementation is possible.

@TokageItLab
Copy link
Member Author

It is old. Perhaps the Transform3D Value/Bezier track will not be obsolete in Godot 4. Also, the rotation issue will be superseded by the QuaternionEditor implementation.

@TokageItLab TokageItLab removed this from the 4.0 milestone Jul 19, 2022
@TokageItLab TokageItLab deleted the fix-key-insertion branch September 16, 2022 21:02
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.

5 participants