Bug in SortHeaderGroup

[haggen]

I stumbled upon it when trying to set a default sorting for my add-on.

The bug is: 'OnHeaderClicked' is ignoring the initial direction given by ZO_SortHeader_Initialize.

Let's take a look:

File esoui\libraries\zo_sortheadergroup\zo_sortheadergroup.lua, from line 149 to 165

function ZO_SortHeaderGroup:OnHeaderClicked(header, suppressCallbacks, forceReselect)
    if self:IsEnabled() then
        local resetSortDir = false
        if forceReselect or not self:IsCurrentSelectedHeader(header) then
            self:DeselectHeader()
            resetSortDir = true
        end
 
        self.sortDirection = resetSortDir and header.initialDirection or not self.sortDirection
 
        self:SelectHeader(header)
    
        if(not suppressCallbacks) then
            self:FireCallbacks(self.HEADER_CLICKED, header.key, self.sortDirection)
        end
    end
end

Upon first calling 'OnHeaderClicked' (whether by clicking on the header or calling 'SelectHeaderByKey'), the line that sets 'sortDirection' always gets true, no matter what. I think the problem is the use of 'x and y or z', since sortDirection is a boolean flag, it's tricky to use this ternary-ish conditional instead of a full body if/else.

Edit.

Yes! As I suspected, changing the line

self.sortDirection = resetSortDir and header.initialDirection or not self.sortDirection

to

if resetSortDir then
  self.sortDirection = header.initialDirection
else
  self.sortDirection = not self.sortDirection
end

Fixed the problem!

I always found 'x and y or z' smelly, a mine waiting to be detonated. :P

[merlight]

You're totally right, that line doesn't do what it should. Btw http://www.esoui.com/forums/showpost...64&postcount=8 ^^

[haggen]

Originally Posted by merlight You're totally right, that line doesn't do what it should. Btw http://www.esoui.com/forums/showpost...64&postcount=8 ^^

Oh damn! I tried searching for "Header sort" and missed that =/ Well, it should have been fixed by now.

Another thing that bothers me is: if you check the sort function for comparing rows in a scroll list, it accepts a flag in the sort keys options called "isNumeric" to know when to compare numeric values. But, in esoui\ingame\mail\mailinbox_shared.lua it reads "numeric" (without the prefix 'is'). I think it's misspelled but since I never took the time to look for other instances I never reported. Well, I am now. :P

Edit.

I was reading the link you mentioned. I notice now that there are bugs everywhere around SORT_ORDER_UP and SORT_ORDER_DOWN, and I think it's because they're booleans. Wouldn't it be better if they used 0 and 1?

[merlight]

Originally Posted by haggen Another thing that bothers me is: if you check the sort function for comparing rows in a scroll list, it accepts a flag in the sort keys options called "isNumeric" to know when to compare numeric values. But, in esoui\ingame\mail\mailinbox_shared.lua it reads "numeric" (without the prefix 'is'). I think it's misspelled but since I never took the time to look for other instances I never reported. Well, I am now. :P

It's probably misspelled, but in this case it also doesn't matter. ZO_TableOrderingFunction checks "isNumeric" and converts the argument tonumber; but since those two "numeric" arguments in mailbox already are numbers, "isNumeric" isn't needed there.

Originally Posted by haggen I was reading the link you mentioned. I notice now that there are bugs everywhere around SORT_ORDER_UP and SORT_ORDER_DOWN, and I think it's because they're booleans. Wouldn't it be better if they used 0 and 1?

Maybe, but then inversion (order = not order) would have to be rewritten (order = 1 - order), not only in ZO code, but also in add-ons. Better just fix the bad uses