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

Fix AnimationPlayer blend_times sorting #93876

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

JacobMillner
Copy link
Contributor

@JacobMillner JacobMillner commented Jul 3, 2024

Fixes #76491

The current operator< for BlendKeys compares to/from pointers resulting in nondeterministic behavior. You can see this represented in the .tscn file of anything with lots of blend_times resulting in file changes which leads to extra noise in your git commits.

This PR usesStringName::AlphaCompare when making to/from comparisons in order preform a lexicographic order sort rather than one determined by memory location.

@JacobMillner JacobMillner requested a review from a team as a code owner July 3, 2024 00:16
scene/animation/animation_player.h Outdated Show resolved Hide resolved
scene/animation/animation_player.h Outdated Show resolved Hide resolved
@JacobMillner
Copy link
Contributor Author

tested and working locally with StringName::AlphaCompare 👍

@TokageItLab
Copy link
Member

Please squash the commits to follow PR workflow https://docs.godotengine.org/en/stable/contributing/workflow/pr_workflow.html#the-interactive-rebase

Modify BlendKey's sort to use  AlphaCompare in order to create a deterministic sort

Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
@akien-mga akien-mga changed the title Fix AnimationPlayer blend_times sorting Fix AnimationPlayer blend_times sorting Jul 4, 2024
@akien-mga akien-mga merged commit 640d815 into godotengine:master Jul 4, 2024
18 checks passed
@akien-mga
Copy link
Member

akien-mga commented Jul 4, 2024

Thanks! And congrats for your first merged Godot contribution 🎉

@JacobMillner
Copy link
Contributor Author

Thanks everyone!

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.

Transition / blend times in AnimationPlayer are reordered upon saving
4 participants