Rosalie241 / RMG

Rosalie's Mupen GUI
GNU General Public License v3.0
610 stars 52 forks source link

RMG's default path of the screenshot directory #223

Closed orbea closed 8 months ago

orbea commented 8 months ago

RMG: 0.5.6

When starting the RMG frontend it creates an empty ~/RMG on one system and ~/Pictures/RMG on the other system, looking at the settings in the gui this corresponds to the screenshots directory.

From the perspective of at least some users it is nice to by default not create directories in the user's $HOME directory outside of the prescribed places such as ~/.config/ and ~/.local/share/. Additionally there is already a ~/.local/share/RMG/ for both saves and savestates, are there reasons why to not have the default be ~/.local/share/RMG/Screenshots?

For example a patch like this would work for me.

--- a/Source/RMG-Core/Directories.cpp
+++ b/Source/RMG-Core/Directories.cpp
@@ -428,15 +428,8 @@ std::filesystem::path CoreGetDefaultScreenshotDirectory(void)
         directory = get_appdata_directory("Screenshots");

 #else
-        directory = get_command_output("xdg-user-dir PICTURES");
-        if (!directory.empty())
-        {
-            directory += "/RMG";
-        }
-        else
-        {
-            directory = get_var_directory("XDG_PICTURES_DIR", "/RMG", "HOME", "/Pictures/RMG");
-        }
+        directory = CoreGetDefaultUserDataDirectory();
+        directory += "/Screenshots";
 #endif // _WIN32
     }
     return directory.make_preferred();

I also noticed this is the only use of xdg-user-dir which is an unique dependency in Gentoo and probably other distros.

Rosalie241 commented 8 months ago

I agree that making a directory isn't ideal, but preferably I'd put the screenshots in a place where the user can easily find them, so ~/.local/share/RMG/Screenshots doesn't seem ideal either for end-users who might not be very technical like steam deck users.

maybe putting the screenshots in XDG_PICTURES_DIR without making a new directory would be better?

what do you think?

orbea commented 8 months ago

I can see how these are opposing use cases which are both valid in the correct context.

What would happen if XDG_PICTURES_DIR is an unset or empty variable? I presume the directory would need to be created when the RMG gui saves any screenshots. It seems to correct to respect that variable if its set, but would it be wrong to default to CoreGetDefaultUserDataDirectory() + /Screenshots" if its not set? I don't really know much of what Steamdeck users may expects.

Rosalie241 commented 8 months ago

What would happen if XDG_PICTURES_DIR is an unset or empty variable?

That's a good question, don't some applications make a ~/Pictures directory when that variable is empty(i.e screenshot applications)?

but would it be wrong to default to CoreGetDefaultUserDataDirectory() + /Screenshots" if its not set? I don't really know much of what Steamdeck users may expects.

well it wouldn't be wrong, I'm just not sure what other applications do regarding such thing, it seems that every application does it's own thing which makes it difficult to choose what RMG should do!

orbea commented 8 months ago

That's a good question, don't some applications make a ~/Pictures directory when that variable is empty(i.e screenshot applications)?

My system is rather minimal so I haven't experienced this and I use scrot(1) as my screenshot tool. However I imagine that may be different for GNOME or KDE users.

I'm just not sure what other applications do regarding such thing, it seems that every application does it's own thing which makes it difficult to choose what RMG should do!

In the case of emulators I am unaware of anyone supporting something beyond the XDG Base Directory Specification, but I certainly haven't tried every emulator.

Unfortunately I may be the wrong person to ask about what the desktop environment users want. :)

Rosalie241 commented 8 months ago

My system is rather minimal so I haven't experienced this and I use scrot(1) as my screenshot tool. However I imagine that may be different for GNOME or KDE users.

that makes sense, I'm a KDE user myself :)

In the case of emulators I am unaware of anyone supporting something beyond the XDG Base Directory Specification, but I certainly haven't tried every emulator.

Unfortunately I may be the wrong person to ask about what the desktop environment users want. :)

I'll think about it for a bit, but adding a build time option to change the default directory could be an option aswell

orbea commented 8 months ago

I realized that if xdg-user-dir is run without xdg-user-dirs-update being run sometime earlier it will print only $HOME.

$ xdg-user-dir PICTURES
/home/orbea

While...

$ xdg-user-dirs-update
$ xdg-user-dir PICTURES
/home/orbea/Pictures

This is because the ~/.config/user-dirs.dirs file may not exist until its created by xdg-user-dirs-update which I presume is run as part some desktop environments? While someone with just a window manager may never run that even if the command is installed on their system.

Maybe something like this would be workable?

  1. If XDG_PICTURES_DIR is set in the user's environment use that.
  2. If xdg-user-dir exists in the user's $PATH and then use it if it outputs a different value than $HOME.
  3. If XDG_DATA_HOME is set in the user's environment use that.
  4. Default to ~/.local/share/ as specified in the XDG Base Directory Specification for XDG_DATA_HOME.

I am not sure if its worth using xdg-user-dir at all and will defer to your judgement, but it would be nice if it was at least an optional dependency (And didn't print command not found when its not installed).

Rosalie241 commented 8 months ago

Yeah that seems annoying to deal with, I've pushed your patch to github (forgot to make you the author though, woops) https://github.com/Rosalie241/RMG/commit/84681fdd0d326e400524c422f182f7fa60e6493f so that should fix this issue!

I also looked at what pcsx2 does and it also uses ~/.local/share so it makes sense to do what other emulators are doing in that regard.

Thank you for your feedback :heart:

orbea commented 8 months ago

Sounds good enough for me, thanks!