Improve file sorting

[groud]

Follow up of #42654.

Modifies the way the type sorting is done by also using the class icons in the comparison function. This specifically group similar resources together.

Also adds the possibility to sort according to the last modification date and in reserve order for each mode.

[KoBeWi]

This appears when I try to sort by type:

CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] CowData<char32_t>::_ref (c:\godot_source\core\cowdata.h:351)
[1] String::String (c:\godot_source\core\ustring.h:437)
[2] Resource::get_name (c:\godot_source\core\resource.cpp:112)
[3] FileSystemDock::FileInfoTypeComparator::operator() (c:\godot_source\editor\filesystem_dock.cpp:636)
[4] List<FileSystemDock::FileInfo,DefaultAllocator>::AuxiliaryComparator<FileSystemDock::FileInfoTypeComparator>::operator() (c:\godot_source\core\list.h:622)
[5] SortArray<List<FileSystemDock::FileInfo,DefaultAllocator>::Element *,List<FileSystemDock::FileInfo,DefaultAllocator>::AuxiliaryComparator<FileSystemDock::FileInfoTypeComparator>,1>::partitioner (c:\godot_source\core\sort_array.h:184)
[6] SortArray<List<FileSystemDock::FileInfo,DefaultAllocator>::Element *,List<FileSystemDock::FileInfo,DefaultAllocator>::AuxiliaryComparator<FileSystemDock::FileInfoTypeComparator>,1>::introsort (c:\godot_source\core\sort_array.h:209)
[7] SortArray<List<FileSystemDock::FileInfo,DefaultAllocator>::Element *,List<FileSystemDock::FileInfo,DefaultAllocator>::AuxiliaryComparator<FileSystemDock::FileInfoTypeComparator>,1>::sort_range (c:\godot_source\core\sort_array.h:305)
[8] SortArray<List<FileSystemDock::FileInfo,DefaultAllocator>::Element *,List<FileSystemDock::FileInfo,DefaultAllocator>::AuxiliaryComparator<FileSystemDock::FileInfoTypeComparator>,1>::sort (c:\godot_source\core\sort_array.h:311)
[9] List<FileSystemDock::FileInfo,DefaultAllocator>::sort_custom<FileSystemDock::FileInfoTypeComparator> (c:\godot_source\core\list.h:646)
[10] FileSystemDock::_sort_file_info_list (c:\godot_source\editor\filesystem_dock.cpp:655)
[11] FileSystemDock::_create_tree (c:\godot_source\editor\filesystem_dock.cpp:131)
[12] FileSystemDock::_update_tree (c:\godot_source\editor\filesystem_dock.cpp:276)
[13] FileSystemDock::set_file_sort (c:\godot_source\editor\filesystem_dock.cpp:2599)
[14] FileSystemDock::_file_sort_popup (c:\godot_source\editor\filesystem_dock.cpp:2605)
[15] call_with_variant_args_helper<FileSystemDock,int,0> (c:\godot_source\core\binder_common.h:217)
[16] call_with_variant_args<FileSystemDock,int> (c:\godot_source\core\binder_common.h:301)
[17] CallableCustomMethodPointer<FileSystemDock,int>::call (c:\godot_source\core\callable_method_pointer.h:98)
[18] Callable::call (c:\godot_source\core\callable.cpp:51)
[19] Object::emit_signal (c:\godot_source\core\object.cpp:1077)
[20] Object::emit_signal (c:\godot_source\core\object.cpp:1132)
[21] PopupMenu::activate_item (c:\godot_source\scene\gui\popup_menu.cpp:1142)
[22] PopupMenu::_gui_input (c:\godot_source\scene\gui\popup_menu.cpp:336)
[23] call_with_variant_args_helper<PopupMenu,Ref<InputEvent> const &,0> (c:\godot_source\core\binder_common.h:212)
[24] call_with_variant_args<PopupMenu,Ref<InputEvent> const &> (c:\godot_source\core\binder_common.h:301)
[25] CallableCustomMethodPointer<PopupMenu,Ref<InputEvent> const &>::call (c:\godot_source\core\callable_method_pointer.h:98)
[26] Callable::call (c:\godot_source\core\callable.cpp:51)
[27] Object::emit_signal (c:\godot_source\core\object.cpp:1077)
[28] Object::emit_signal (c:\godot_source\core\object.cpp:1132)
[29] Window::_window_input (c:\godot_source\scene\main\window.cpp:909)
[30] call_with_variant_args_helper<Window,Ref<InputEvent> const &,0> (c:\godot_source\core\binder_common.h:212)
[31] call_with_variant_args<Window,Ref<InputEvent> const &> (c:\godot_source\core\binder_common.h:301)
[32] CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call (c:\godot_source\core\callable_method_pointer.h:98)
[33] Callable::call (c:\godot_source\core\callable.cpp:51)
[34] DisplayServerWindows::_dispatch_input_event (c:\godot_source\platform\windows\display_server_windows.cpp:1792)
[35] DisplayServerWindows::_dispatch_input_events (c:\godot_source\platform\windows\display_server_windows.cpp:1769)
[36] Input::_parse_input_event_impl (c:\godot_source\core\input\input.cpp:614)
[37] Input::parse_input_event (c:\godot_source\core\input\input.cpp:440)
[38] Input::flush_accumulated_events (c:\godot_source\core\input\input.cpp:830)
[39] DisplayServerWindows::process_events (c:\godot_source\platform\windows\display_server_windows.cpp:1531)
[40] OS_Windows::run (c:\godot_source\platform\windows\os_windows.cpp:618)
[41] widechar_main (c:\godot_source\platform\windows\godot_windows.cpp:163)
[42] _main (c:\godot_source\platform\windows\godot_windows.cpp:185)
[43] main (c:\godot_source\platform\windows\godot_windows.cpp:199)
[44] __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
[45] BaseThreadInitThunk
-- END OF BACKTRACE --

Also, is this intended that sorting doesn't affect directories?

[groud]

No this crash is not intended. Let me try something, I suppose there's a wrong pointer somewhere.

[KoBeWi]

No this crash is not intended.

That's not what I asked 🙃

[groud]

That's not what I asked 🙃

Ah yes sorry. Maybe I could sort the directories too when the "Name descending" is chosen. But otherwise, yes, it is intended that directories are always before files.

[groud]

I've updated the PR, could you try if it still crashes on your side ?

[KoBeWi]

Maybe I could sort the directories too when the "Name descending" is chosen.

Yeah that's what I meant. It just looks weird that no matter what you chose the directories don't change at all.

could you try if it still crashes on your side ?

It's fixed.

[groud]

Alright, I updated the PR, the folder are also sorted too now. :)

[KoBeWi]

The PR is good functionality-wise. I didn't check the code thoroughly, but it looks ok too.

[akien-mga]

IMO that might be more readable as two separate for loops in if and else branches.

[akien-mga]

Likely also easier to unroll/optimize for compilers.

[akien-mga]

Well I see you have the same case further down on a bigger for loop, where the duplication might not be wanted. Your call :)

[groud]

Yeah, this is the best solution I came up with for now.
I could have done something like storing the data in a list, like how it's done for files. But it was over-complicating things for little gain. It though duplicating the whole loop (the big one), removing everything to a function, only to be able to iterate over it in reverse was a little bit too much too.
So I went for this solution and kept things consistent for the too places it was used in. I agree it's not very readable, but I think the "reversed" variable name is at least obvious on what this does.

[akien-mga]

Thanks!