Premade Groups Filter

Premade Groups Filter

9M Downloads

Sorting bugs

yourapple opened this issue · 11 comments

commented
2x PremadeGroupsFilter/Main.lua:584: invalid order function for sorting
[string "=[C]"]: in function `?'
[string "@PremadeGroupsFilter/Main.lua"]:584: in function <PremadeGroupsFilter/Main.lua:322>
[string "=[C]"]: in function `LFGListUtil_SortSearchResults'
[string "@FrameXML/LFGList.lua"]:2125: in function `LFGListSearchPanel_UpdateResultList'
[string "@FrameXML/LFGList.lua"]:2033: in function `onEvent'
[string "@FrameXML/LFGList.lua"]:218: in function <FrameXML/LFGList.lua:139>

Locals:
(*temporary) = <table> {
 1 = 2134
 2 = 2075
 3 = 2141
 4 = 1311
 5 = 2142
}
(*temporary) = <function> defined @PremadeGroupsFilter/Main.lua:132
(*temporary) = 1311
(*temporary) = 2075
(*temporary) = nil
commented

Released version 3.5.0, will close issue if no further complaints arise.

commented

since 3.4.2-beta there was no error for me but i've not played "that much" this week.
Will test it next Days or next Week more intensive, but until...

Thank you very much :)

commented

At present, I have changed it like this, and it seems that I have not found this BUG. But I am not sure.

function PGF.SortByFriendsAndAge(searchResultID1, searchResultID2)
    if not searchResultID1 or type(searchResultID1) ~= "number" then return false end
    if not searchResultID2 or type(searchResultID2) ~= "number" then return false end
    if searchResultID1 == searchResultID2 then return false end

    -- sort applications to the top
    local _, appStatus1, pendingStatus1, appDuration1 = C_LFGList.GetApplicationInfo(searchResultID1)
    local _, appStatus2, pendingStatus2, appDuration2 = C_LFGList.GetApplicationInfo(searchResultID2)
    local isApplication1 = appStatus1 ~= "none" or pendingStatus1 or false
    local isApplication2 = appStatus2 ~= "none" or pendingStatus2 or false
    if isApplication1 ~= isApplication2 then return isApplication1 end
    if appDuration1 ~= appDuration2 then return appDuration1 > appDuration2 end
    local searchResultInfo1 = C_LFGList.GetSearchResultInfo(searchResultID1)
    local searchResultInfo2 = C_LFGList.GetSearchResultInfo(searchResultID2)

    local hasRemainingRole1 = HasRemainingSlotsForLocalPlayerRole(searchResultID1)
    local hasRemainingRole2 = HasRemainingSlotsForLocalPlayerRole(searchResultID2)

    if hasRemainingRole1 ~= hasRemainingRole2 then return hasRemainingRole1 end

    if searchResultInfo1.numBNetFriends ~= searchResultInfo2.numBNetFriends then
        return searchResultInfo1.numBNetFriends > searchResultInfo2.numBNetFriends
    end
    if searchResultInfo1.numCharFriends ~= searchResultInfo2.numCharFriends then
        return searchResultInfo1.numCharFriends > searchResultInfo2.numCharFriends
    end
    if searchResultInfo1.numGuildMates ~= searchResultInfo2.numGuildMates then
        return searchResultInfo1.numGuildMates > searchResultInfo2.numGuildMates
    end
    if searchResultInfo1.isWarMode ~= searchResultInfo2.isWarMode then
        return searchResultInfo1.isWarMode == C_PvP.IsWarModeDesired()
    end

    if searchResultInfo1.age == searchResultInfo2.age then
        return false
    end
    
    return searchResultInfo1.age < searchResultInfo2.age
end
commented

I need a lilttle more information as I cannot reproduce the bug. (CC @fubaWoW)

  1. Do you use standard mode (normal window size) or advanced mode (small window)?
  2. What filters and/or sorting do you have configured?
  3. Do you have other addons installed that do something with the group list, e.g. LFM+? When in doubt, please post a complete list of all your addons or re-test with just PGF and BugSack/BugGrabber installed.

@ljlevend claims that it can be reproduced when signing up for multiple groups as a party. I did the same on the 30th with the same PGF version and never got these Lua errors.

Regarding the bug itself: invalid order function for sorting means that the sorting function has an error in the sense that both sortfunc(a, b) and sortfunc(b, a) do return true. I just do not understand why. Also when I think about it, neither searchResultID1 nor searchResultID1 should be ever nil as Lua should ignore those values in a table. Also your locals dump shows that the list of search resuls is a regular list without nil entries (the first (*temporary) = <table>).

For me this looks like the table is being modified and entries are being removed (hence nil) while PGF is trying to sort it. This means that there is another addon involved.

commented

You did not answer my questions… Understanding where the nil searchResultIDs come from is crucial for fixing the issue. I still suspect some kind of race condition.

I saw that you changed the code to return false on both conditions. My idea behind returning false/true was:

  • If searchResultID1 is nil, we return false to sort the first below the second.
  • If searchResultID2 is nil, we return true to sort the first non-nil entry to the top.
  • If both are nil, we return false anyway because the first is nil.
commented

I need a lilttle more information as I cannot reproduce the bug. (CC @fubaWoW)

  1. I use "Standard Mode
  2. My Filters is always set to partyfit and the sometimes instance(s) i want to run like partyfit and (tjs or aa)
    it is also possible that members, tank or heal is limited by the default window
  3. The ONLY AddOn that touches my Group List and i have installed is Raider.io and ElvUI except PGF, nothing else!
    But both do not touch the sorting algorithm
commented

This error, I remember, only appeared when applying for a team for the second time, that is, when there is already an application queue, this error will occur when applying for another team again. But I don't understand the exact logic.

commented

I did some more testing in game. It looks like the sorting function is called twice on a normal Search button click. From the Blizzard interface code, it looks like it can be called even more times when applying as a group. I assume one of the many calls causes a race condition because the sorting function now takes a little bit too long with the two additional method calls for sorting applied groups to the top.

Will have a look tomorrow to see which paths are triggering the sorting and if there is a way to hook on the last call only.

commented

tested now with:

if not searchResultID1 or type(searchResultID1) ~= "number" then return false end
    if not searchResultID2 or type(searchResultID2) ~= "number" then return false end
    if searchResultID1 == searchResultID2 then return false end

the error is GONE with that code!
but sorting of the SECOND and further queues are at the Bottom of the list then, the First "sort" is at the Top ^^

commented

I released version 3.4.2-beta which includes the false/false logic from @fubaWoW and also speeds up the sorting by using pre-filled tables. Hopefully this is enough to remove the problem for now. Please test with this version :)

commented

Problem seems to be resolved with new version with pre-filled tables, closing issue.