Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use XDG directories #3218

Merged
merged 6 commits into from Jun 2, 2021
Merged

Use XDG directories #3218

merged 6 commits into from Jun 2, 2021

Conversation

qarkai
Copy link
Contributor

@qarkai qarkai commented Apr 17, 2021

Use $XDG_CONFIG_HOME/fheroes2on Linux (~/.config/fheroes2 by default) for fheroes2.bin, fheroes2.cfg.
Use $XDG_DATA_HOME/fheroes2 on Linux (~/.local/share/fheroes2 by default) for data, files/save, maps.
On other platforms use directory returned by GetHomeDirectory.
Close #2860.
Close #3118.

@oleg-derevenetz oleg-derevenetz added Linux Linux OSes improvement New feature, request or improvement labels Apr 17, 2021
@oleg-derevenetz oleg-derevenetz added this to the 0.9.3 milestone Apr 17, 2021
@oleg-derevenetz
Copy link
Collaborator

oleg-derevenetz commented Apr 17, 2021

Hi @qarkai Great, thanks! There is also demo_linux.sh script from script/demo, maybe adapt it to XDG paths as well?

Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @qarkai , I left multiple comments in this pull request. Could you please take a look?

src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/fheroes2/game/fheroes2.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
src/engine/system.cpp Outdated Show resolved Hide resolved
@qarkai qarkai requested a review from ihhub April 18, 2021 11:44
@qarkai
Copy link
Contributor Author

qarkai commented Apr 18, 2021

Hi @qarkai , I left multiple comments in this pull request. Could you please take a look?

Made changes. Don't know how I "close" changes request though.

@ihhub
Copy link
Owner

ihhub commented Apr 18, 2021

Hi @qarkai , I left multiple comments in this pull request. Could you please take a look?

Made changes. Don't know hot to "close" changes request though.

There's no need to close them. Resolving them is enough :)

@karolchmist
Copy link

I tested this branch when building fheroes2 for NixOS, it works great 👍 Hoping to see it in 0.9.3 release.

Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @qarkai , I left two questions. Could you please take a look?

src/engine/system.cpp Show resolved Hide resolved
@Branikolog
Copy link
Collaborator

Hi!
I believe for some platforms, like windows, it will be more reasonably to place all files of fheroes2 project into the same folder. As users can somehow change or even spoil configuration file, and after reinstall can forgot to change previous file, or simply forget the place it is located...

@ihhub
Copy link
Owner

ihhub commented May 3, 2021

Hi @qarkai , I think @Branikolog has a point regarding Windows systems. Putting files somewhere outside initial directory might be misleading for users.

@qarkai
Copy link
Contributor Author

qarkai commented May 3, 2021

Hi @ihhub, I believe that user's files should be kept in user's directories. Consider two people sharing config and saves directory on a computer when one prefers good interface and other prefers evil one.
Configuration file location could be placed in readme/FAQ.
Also let be honest: this project is for geeks and they usually know where their files could be and even expect that.

@ihhub
Copy link
Owner

ihhub commented May 3, 2021

@qarkai , actually the biggest majority of our users are on Windows and they aren't expecting files to be stored anywhere else except the folder where the archive is extracted. Also based on the feedback from Linux based forums many users face problems of running the application so I would say geeks is too much.
Every project should be as much as possible user friendly without assumptions of high OS level from users.

@ihhub
Copy link
Owner

ihhub commented May 3, 2021

The best way is to ask our team to avoid any one side opinions :)

@oleg-derevenetz , @LeHerosInconnu and @idshibanov could you please give us your opinion as well?

@karolchmist
Copy link

karolchmist commented May 3, 2021

Just my 0.02¢ as a geeky Linux user :) Why not checking all the possible locations: first current dir, then XDG dirs? Plus a config option to specify it if needed. This way everyone should be happy.

(On NixOS Linux we can't use the current dir)

@ihhub
Copy link
Owner

ihhub commented May 3, 2021

Just my 0.02¢ as a geeky Linux user :) Why not checking all the possible locations: first current dir, then XDG dirs? Plus a config option to specify it if needed. This way everyone should be happy.

(On NixOS Linux we can't use the current dir)

I totally agree with you. For example, I have Ubuntu machine and I am totally fine to keep all files within the same directory.

@qarkai
Copy link
Contributor Author

qarkai commented May 3, 2021

@karolchmist The problem is with different directories for config and saves. If we search for writeable directory then all files go to which is first in directories list. I believe that config files should be in config dir, since they could be easily regenerated and reconfigured if configure dir is lost but saves should be in data dir.
@ihhub Maybe you should note that in #2860 and close it earlier.

@LeHerosInconnu
Copy link

From the point of view of the average user, it's always better to have everything in one folder; and the majority of users are more average users than geeks.

And for some users, it's already complicated to have to search in subfolders. :D

@ihhub
Copy link
Owner

ihhub commented May 3, 2021

@qarkai , my apologies for this as I need to maintain a lot of things so I came back to this pull request once I got time.

@undef21
Copy link
Contributor

undef21 commented May 3, 2021

My some cents. Why not use both approaches?
Game can use different paths depending command line parameters or executable name. For example, if we start game fheroes2 <game_dir>, then game search and save all data in this dir. If game started without parameters then it use OS-specific paths.

@qarkai
Copy link
Contributor Author

qarkai commented May 3, 2021

@ihhub sorry, I didn't mean to be offensive, I really appreciate your work on this project.

@oleg-derevenetz
Copy link
Collaborator

oleg-derevenetz commented May 3, 2021

Hi all,

How about the following logic: if directory with fheroes2 (or fheroes2.exe) binary is writeable by current user, then use it to load and store config files, maps and savefiles. Otherwise, use $XDG*/$HOME/$APPDATA. This should cover cases when user just downloaded ZIP archive with game files, unpacked it into some directory and run the game from it, as well as cases when game was installed by administrator or package manager into some global directory, like C:\Program Files on Windows or /usr/bin on Linux.

Additionally, it is possible to add command-line switch to manage this behavior explicitly, for example, fheroes2 --config-dir=DIR --data-dir=DIR and mention this in the documentation.

@undef21
Copy link
Contributor

undef21 commented May 4, 2021

if directory with fheroes2 (or fheroes2.exe) binary is writeable by current user, then use it to load and store config files, maps and savefiles.

In this case admins'll break OS-filesystem rules. Admins want to play too.

@oleg-derevenetz
Copy link
Collaborator

In this case admins'll break OS-filesystem rules. Admins want to play too.

Admins will break those rules only if they run app with elevated privileges (through UAC on Windows or as root on Linux). Normally they shouldn't do that.

@undef21
Copy link
Contributor

undef21 commented May 4, 2021

Yes, it's unusual, but possible case. If game try to follow OS-specific rules, then it's better when all accounts follow same rules ( XDG for linux, %APPDATA% for win, something for *BSD).

Also, is there cross-platform way for getting program path? Because if we run fheroes2 from some PATH directory, then game can't detect program dir.

@oleg-derevenetz
Copy link
Collaborator

oleg-derevenetz commented May 4, 2021

Yes, it's unusual, but possible case. If game try to follow OS-specific rules, then it's better when all accounts follow same rules ( XDG for linux, %APPDATA% for win, something for *BSD).

Then we should say bye-bye to storing all user files in the app's folder, at least on Windows. No ordinary user will bother to run fheroes2.exe .. He will just unpack ZIP archive and double-click on EXE.

@qarkai
Copy link
Contributor Author

qarkai commented May 4, 2021

BTW why it's ZIP instead of installer?

@qarkai
Copy link
Contributor Author

qarkai commented May 15, 2021

Also, is there cross-platform way for getting program path? Because if we run fheroes2 from some PATH directory, then game can't detect program dir.

There is SDL_GetBasePath but it requires SDL 2.0.1.
Also there are a bunch of platform-specific methods which could be wrapped in something like System::GetProgramPath.

@ihhub
Copy link
Owner

ihhub commented Jun 1, 2021

Hi @qarkai , could you kindly please resolve merge conflicts as I can't do it by myself?

@ihhub
Copy link
Owner

ihhub commented Jun 1, 2021

Hi @Northfear , could you please test these changes on PS Vita as well?

@Northfear
Copy link
Contributor

Hi @Northfear , could you please test these changes on PS Vita as well?

Checked. Everything seems to be working fine

@ihhub
Copy link
Owner

ihhub commented Jun 2, 2021

Hi @qarkai and @oleg-derevenetz , for fheroes2.bin, fheroes2.cfg and for files/save I have no objections to store outside the current directory but maps is a part of data files of the original game and they shouldn't be separate for each user. We have only one instance of the game which contains the executable file, resources files in data, music, maps and Heroes2 (for animation) directories.

Let me please clarify things so everything will be aligned:

  • we store data files such as data, music, anim and maps within the game's directory
  • we cstore configuration files and saves outside the game's directory.

Please correct me if I'm wrong.

@oleg-derevenetz
Copy link
Collaborator

Hi @ihhub suppose we have a multi-user system with unrelated users, and one of them bought original HoMM2 game, and the other is not. In this case, files of original game should be stored in user's directory.

@ihhub
Copy link
Owner

ihhub commented Jun 2, 2021

Hi @ihhub suppose we have a multi-user system with unrelated users, and one of them bought original HoMM2 game, and the other is not. In this case, files of original game should be stored in user's directory.

Is the original game installed per user? I use GOG version and everything's installed out of C drive obviously.

@qarkai
Copy link
Contributor Author

qarkai commented Jun 2, 2021

Data files could be stored in user's directory. But nothing prevents from storing them in game's directory (or some common system directory on Linux if CONFIGURE_FHEROES2_DATA was used).

@ihhub
Copy link
Owner

ihhub commented Jun 2, 2021

Data files could be stored in user's directory. But nothing prevents from storing them in game's directory (or some common system directory on Linux if CONFIGURE_FHEROES2_DATA was used).

If files could be stored then we should. As a user I expect that all data files will be in the same directory where my installation is. At least in terms on Window's users. Having data files somewhere else might be odd for many.

@ihhub
Copy link
Owner

ihhub commented Jun 2, 2021

Data files could be stored in user's directory. But nothing prevents from storing them in game's directory (or some common system directory on Linux if CONFIGURE_FHEROES2_DATA was used).

If files could be stored then we should. As a user I expect that all data files will be in the same directory where my installation is. At least in terms on Window's users. Having data files somewhere else might be odd for many.

Right now everything works as I described before: demo script downloads files to the same directory where fheroes2.exe locates. While configuration files are in user's directory which is fine.

@oleg-derevenetz
Copy link
Collaborator

@ihhub

Is the original game installed per user?

Original game is shipped as a whole with data files because owners have copyright for them. We don't have a rights to distribute data files, so I'd be careful here and let each user install it's own data files individually. Also, at one moment we should be able to provide fheroes2 packages, and individual data files are fit for this ideally (BTW, in Linux and MacOS shared data should reside somewhere in /usr/local/share anyway). But it's up to you. I personally see only gains from per-user data files: no copyright issues, one user may have full PoL, while another may have just a demo.

@qarkai
Copy link
Contributor Author

qarkai commented Jun 2, 2021

As a user I'd like to have opportunity storing new maps in my data directory thus avoiding UAC/sudo in order to write them in game's directory in C:\Program Files or /usr/whatever.
I kind of agree that data, music, anim should be in game's directory.

@ihhub ihhub merged commit 3a6824b into ihhub:master Jun 2, 2021
@ihhub
Copy link
Owner

ihhub commented Jun 2, 2021

Hi @qarkai , thanks a lot for these changes!

I've decided to go with them as I feel that I'm dragging you guys with the implementation :)

@karolchmist
Copy link

I built it on NixOS Linux and it works, thank you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement New feature, request or improvement Linux Linux OSes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Linux] from 0.9.2 fheroes2.cfg is written to $HOME directory Follow XDG Base Directory specification
8 participants