File Dialogs allow sorting by modified time

[nishantjr]

I've thrown together a quick PR that adds an extra button to the file dialogs, allowing sorting by modified time. This allows you to bring the most recent save to the top of the list.

Caveats:

• Since I only have access to a linux system, I have only implemented the platform specific parts for that platform. I think this will also work for other Unix-like platforms (MacOS, maaaybe Windows?)
• I picked an arbitrary image for the sorting button
• I've not written any automated tests.

Going forward, we could implement ascending/descending order, and displaying the modified date

[crudelios]

@nishantjr I really like your idea and implementation and will gladly merge it into the code, but please sort of the compiling problems first.

Windows/mac seem to have a typo in a variable name and one of the struct's names seem to be clashing with Switch and Vita's folder contents cache struct.

I'll also want to test these on Vita, Switch, Windows and add support for Android.

@bvschaik Could you test on mac, please? Also, what do you think about adding this to Julius?

[nishantjr]

Thanks for the review @crudelios Is it possible to define USE_FILE_CACHE on Linux so I can test it without a Vita/Switch?

[crudelios]

Yes it should be possible, all you need to do is to define it and it should work. I think defining it temporarily on file_manager_cache.h will do the trick.

Also, Vita and Switch don't always have stat information, so in that case the sort button is useless/shouldn't even appear.

[nishantjr]

@crudelios Looks like I lost workflow approval because I rebased and force-pushed. I think this should be pretty much complete.

[crudelios]

Hey, I'm finally going to look at this.

Can you please fix the conflicts so I can look at the code?

Thanks!

[nishantjr]

@crudelios Done!

[nishantjr]

@crudelios Regarding merging ok_cancel_buttons and sort_button, I find that:

image_buttons_draw(0, 0, image_buttons, 2 + platform_file_manager_has_stat());

is clever code that I need to take time to read and understand. I also need to know/guess from context that the scroll button is last in the array. Since code is write once, read many times I feel it is better to optimize for reading. The code below is more verbose, but obvious in its intent.

    image_buttons_draw(0, 0, ok_cancel_buttons, 2);                                                                     
    if (platform_file_manager_has_stat()) {                                                                             
        image_buttons_draw(0, 0, sort_button, 1);                                                                       
    }

Although, if you prefer merging them, I'm fine with merging them.

[nishantjr]

Edit: It's also not working on Windows. Strange..

@crudelios Hmm, that's interesting. Looks like windows supports stat/mtime. So something else must be broken. What is the behaviour you are observing? I've tested on Linus, with and without cache enabled. I unfortunately do not have a windows machine to test on.

[nishantjr]

Merged all other suggestions.

[nishantjr]

Also, since right now android isn't supported, you should also disable sorting on Android.

Added an #ifdef that disables sorting for android.

[crudelios]

@crudelios Regarding merging ok_cancel_buttons and sort_button, I find that:

image_buttons_draw(0, 0, image_buttons, 2 + platform_file_manager_has_stat());

is clever code that I need to take time to read and understand. I also need to know/guess from context that the scroll button is last in the array. Since code is write once, read many times I feel it is better to optimize for reading. The code below is more verbose, but obvious in its intent.

    image_buttons_draw(0, 0, ok_cancel_buttons, 2);                                                                     
    if (platform_file_manager_has_stat()) {                                                                             
        image_buttons_draw(0, 0, sort_button, 1);                                                                       
    }

Although, if you prefer merging them, I'm fine with merging them.

That's true, but in that case you could always do something like:

image_buttons_draw(0, 0, ok_cancel_sort_buttons, platform_file_manager_has_stat() ? 3 : 2);

[nishantjr]

@crudelios Sorry about the delay, had a deadline. Fixed.

[crudelios]

I finally had a proper look at this. Sorry for the long delay but both my personal and professional life have been hectic these past few months.

The windows build is not working properly yet. It does reorder the files but not by last modified, I'm not really sure what it's doing.

However, my biggest problem is how it looks in game. The purpose of the added button is unclear and it has no tooltip.

I'm thinking about adding the PR as-is and then improving upon it if you don't mind, mainly by replacing the current arrow button with an "order by" drop down menu, increasing the "load/save/delete game" window size if I need to. I also want to add support for android and try to fix whatever is going on with Windows.

[nishantjr]

@crudelios No problem. That sounds like a good plan. Sorry, I don't have a windows machine so cannot debug the issues on Windows. I will be getting one in a few weeks so maybe I can help then.

[crudelios]

Thanks. I'm waiting for v3.1 to be released as I think this can't be added as-is to a stable release.

I'm going to overhaul the load/save/delete menu for v4.0 (improving it and adding support for separate directories), so this is a nice first step.

[crudelios]

I completely forgot about this.

Since we have a new improved file dialog, I'm going to add this right after v3.2.0 is released.

[nishantjr]

Great!

[crudelios]

This PR is nearing two years old!

I didn't forget about it though, having this pending PR has been itching me for a while now.

I've been thinking about it and I'm actually thinking about using a modern file filter where you can type a part of the filename and the list of files will be reduced. This should work very well using strstr.

Eventually I'll look into it, but I'm really pressed for time unfortunately.

[nishantjr]

No problem at all. Wish I could help out, but I've been a bit strapped for time too.

[crudelios]

Two years later and I'm finally looking at this.

If you don't mind, I'm going to liberally use your code regarding the "modified time" addition to the directory structure list.

I'll obviously credit you on the commit, but I won't be using this PR as it is now very outdated.

[nishantjr]

Of course. I’m just glad it’s being used.

I’ve not being playing C3 lately but have been watching Zach’s videos. Great work by everyone, on the game and community building!

On Thu, Jul 13, 2023 at 06:10, José Cadete @.***> wrote:

Two years later and I'm finally looking at this.

If you don't mind, I'm going to liberally use your code regarding the "modified time" addition to the directory structure list.

I'll obviously credit you on the commit, but I won't be using this PR as it is now very outdated.

— Reply to this email directly, view it on GitHub https://github.com/Keriew/augustus/pull/401#issuecomment-1634054025, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAE3ABOHTHMNP5BFMIGMHTDXP7JSFANCNFSM45K3MMSA . You are receiving this because you were mentioned.Message ID: @.***>

[crudelios]

As you can see I finally started implementing this.

Right now I've only done the background work, but already the timestamp is working on every build, including android.

Now it's just a matter of actually adding the UI to support the new data, but I have a few pending issues to do first.

Since I've finally started adding this, I'll close this PR.

Thank you very much for your idea and contribution!

[crudelios]

@nishantjr It's done.

If you're interested please have a look at the new dialog 👍