fix: priority queue losing order on item removal or reversed inserting

[IXLLEGACYIXL]

PR Details

PriorityQueue loses order on Remove/Reverse Inserting

Related Issue

fix #3398

Types of changes

internal bug fix

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My change requires a change to the documentation.
• I have added tests to cover my changes.
• All new and existing tests passed.
• I have built and run the editor to try this change out.

i was able to run around 4445 tests but it was stuck on bepu so few are missing, but the list queue tests pass now, also the old ones

[xen2]

Doesn't work in all cases:

var q = new PriorityQueue<int>();
foreach (var v in new[] { 26, 38, 33, 54, 69, 64, 21 }) q.Enqueue(v);
q.Remove(54);
// drains: 21, 26, 38, 33, 64, 69   <-- 33 comes after 38

Issue #3398 explains the real problem. When Remove deletes an item, it takes the last item in the queue and puts it in the empty slot. Then it only moves that item down the tree.
But the item can also be smaller than its new parent. In that case it has to move up instead.

[IXLLEGACYIXL]

Doesn't work in all cases:

var q = new PriorityQueue<int>();
foreach (var v in new[] { 26, 38, 33, 54, 69, 64, 21 }) q.Enqueue(v);
q.Remove(54);
// drains: 21, 26, 38, 33, 64, 69   <-- 33 comes after 38

Issue #3398 explains the real problem. When Remove deletes an item, it takes the last item in the queue and puts it in the empty slot. Then it only moves that item down the tree. But the item can also be smaller than its new parent. In that case it has to move up instead.

ok so the current improvements fixes the regression that the old tests pass

i add this case as well

[github-actions]

🤖 Draft PR — automatic CI is skipped to save runner minutes.

• Mark the PR ready for review to run the full automatic CI — or add a ci-run-on-draft label to run it now without leaving draft.
• Or arm a specific opt-in suite: ci-enduser, ci-editor, ci-ios, ci-android.

[IXLLEGACYIXL]

ive added now your test and it passes to decide if order is broken up or downwards on removal

[xen2]

PriorityNodeQueue<T>.Remove has the same bug.
It needs the same "move up if smaller than parent" step.
Please add tests in TestPriorityLinkedQueue too.

Remove and Dequeue now have the same sift-down loop.
Private SiftUp/SiftDown helpers would remove the copy (and maybe make the PriorityNodeQueue fix simple).

[xen2]

Nit: this test RemovalOnBubbleUp not Down

[xen2]

CheckPriorityQueue only checks order. If Remove deleted the wrong item, the tests would still pass.
Could the new tests compare the drained values with the expected sorted list?