Sorting: stable tiebreak so group frames don't reshuffle mid-M+#142
Open
Krathe82 wants to merge 1 commit into
Open
Sorting: stable tiebreak so group frames don't reshuffle mid-M+#142Krathe82 wants to merge 1 commit into
Krathe82 wants to merge 1 commit into
Conversation
The sort comparators ended with `return false` for equal-key members (same role with Alphabetical off, or tied names), and Lua's table.sort is not stable. A roster-driven re-sort during an encounter (pet summon/dismiss, role/assist churn -> GROUP_ROSTER_UPDATE) could then land same-role players in a different order, changing the rebuilt nameList so the secure header reorders the frames — the "group frames rearrange mid-M+ after pulls" report. Add a deterministic final tiebreak by name (invariant per session, realm- qualified in the nameList) so every re-sort of the same roster yields an identical order: - Headers.lua SortMembers — the live nameList path; covers the player too, which is re-sorted through the same comparator in SORTED mode. - Sort.lua CompareUnits + CompareTestData — legacy/test path, same pattern. Trade-off: with Alphabetical off, members of the same role now order by name (previously undefined/shuffling) rather than an arbitrary order.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Group/raid frame order rearranges itself during M+ pulls. The sort comparator (
SortMembersinBuildSortedNameList) ends withreturn falsefor members with equal sort keys — same role with Alphabetical off, or tied names — and Lua'stable.sortis not stable. A roster-driven re-sort during an encounter (pet summon/dismiss, role/assist churn →GROUP_ROSTER_UPDATE) can then land same-role players in a different order, changing the rebuilt nameList. The secure header faithfully applies the new order via its Hide/Show cycle, and the frames visibly shuffle.(The skip-if-unchanged guard on the header attribute is intact and not the cause — it was applying a genuinely different nameList. The non-determinism is upstream of it.)
Fix
Add a deterministic final tiebreak by name so every re-sort of the same roster yields an identical order. Name is invariant for the session and realm-qualified in the nameList, so distinct players never tie:
Frames/Headers.luaSortMembers— the live nameList path; covers the player too, which is re-sorted through the same comparator in SORTED mode.Features/Sort.luaCompareUnits+CompareTestData— samereturn falsepattern (legacy/test path).Trade-off
With Alphabetical off, members of the same role now order by name (previously undefined/arbitrary) instead of reshuffling. That's the deterministic behaviour required to stop the churn.