Skip to content

AnimationCurveI17F15.Keys セット時にソートされない不具合を修正 - #350

Merged
agate-pris merged 1 commit into
developfrom
fix-sort-animation-curve-keys
Nov 9, 2025
Merged

agate-pris merged 1 commit into
developfrom
fix-sort-animation-curve-keys

Conversation

@agate-pris

Copy link
Copy Markdown
Owner
  • AnimationCurveI17F15.Keys セット時に挿入ソートを行うように修正
  • 上記の実装のために Utiliity.InsertionSort を追加
  • Unity.AnimationCurve.keys と動作が同等であることを確認するテストを追加

@agate-pris agate-pris self-assigned this Nov 9, 2025
@agate-pris
agate-pris requested a review from Copilot November 9, 2025 15:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements stable sorting for AnimationCurve keyframes by introducing a custom insertion sort algorithm. The change ensures that keyframes with equal time values maintain their original relative order after sorting.

  • Added a new Utility class with a stable InsertionSort implementation
  • Updated AnimationCurveI17F15.Keys setter to use the stable insertion sort
  • Added comprehensive tests to verify stable sorting behavior for both Unity's AnimationCurve and the custom AnimationCurveI17F15

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
Intar/Utility.cs New utility class containing stable insertion sort implementation
Intar/Utility.cs.meta Unity metadata file for the new Utility.cs
Intar/AnimationCurveI17F15.cs Updated Keys setter to use stable insertion sort
Intar.Tests/AnimationCurveTest.cs Added test case verifying stable sort behavior for AnimationCurveI17F15
Intar.Tests/AnimationCurveTest.Unity.cs Added test case verifying stable sort behavior for Unity's AnimationCurve
Comments suppressed due to low confidence (1)

Intar/AnimationCurveI17F15.cs:117

  • The MoveKey method uses List<T>.Sort which is not stable, while the Keys setter uses Utility.InsertionSort for stable sorting. For consistency and to maintain stable sort behavior throughout the API, consider using Utility.InsertionSort(keys, (a, b) => a.Time.CompareTo(b.Time)) here as well.
            keys.Sort((a, b) => a.Time.CompareTo(b.Time));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@agate-pris
agate-pris requested a review from Copilot November 9, 2025 15:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@agate-pris
agate-pris force-pushed the fix-sort-animation-curve-keys branch from 55cd7e2 to d24e0e6 Compare November 9, 2025 16:19
Repository owner deleted a comment from Copilot AI Nov 9, 2025
@agate-pris
agate-pris requested a review from Copilot November 9, 2025 16:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

return;
}
keys = new List<KeyframeI17F15>(value);
Utility.InsertionSort(keys, (a, b) => a.Time.CompareTo(b.Time));

Copilot AI Nov 9, 2025

Copy link

Choose a reason for hiding this comment

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

MoveKeyメソッド(117行目)ではList<T>.Sortを使用していますが、KeysセッターではInsertionSortを使用しており、ソートアルゴリズムが統一されていません。MoveKeyも安定ソートにするべきではないでしょうか。一貫性のため、MoveKeyでもUtility.InsertionSortを使用することを検討してください。

Copilot uses AI. Check for mistakes.
Comment thread Intar/Utility.cs
@agate-pris
agate-pris merged commit 382acea into develop Nov 9, 2025
6 checks passed
@agate-pris
agate-pris deleted the fix-sort-animation-curve-keys branch November 9, 2025 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants