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

honor the $XDG_CACHE_HOME env variable #625

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

twolife
Copy link

@twolife twolife commented Oct 7, 2024

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes
    • Render changes
    • Audio changes
    • Input changes
    • Network changes
    • Other changes

Description

First step toward the use XDG directories on Linux

Related Issues

This is related to #373 , but it doesn’t fully fix that issue.

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

Additional Comments

I will try to handle the others XDG directories in another pull request, if you confirm that there is no ideological resistance against the concept. (that is still not clear to me when re-reading #373)

@winterheart
Copy link
Collaborator

I don't think that using environment variables is good idea. XDG specifications are Linux specific, and not all desktop environments are fully support them. Better to use known and controllable locations like path from SDL_GetPrefPath().

@twolife
Copy link
Author

twolife commented Oct 7, 2024

I don't think that using environment variables is good idea.

On unix systems SDL_GetPrefPath() is a wrapper around the $XDG_DATA_HOME environment variable, so I don't understand you stance here.

not all desktop environments are fully support them.

That's why I fall back to $HOME/.cache if the variable doesn't exist (like it's written in the xdg specifications)

It's a shame that SDL doesn't provide a wrapper around $XDG_CACHE_HOME but i fail to see why if SDL doesn't provide it we should not work around that.

I don't think using SDL_GetPrefPath() / $XDG_DATA_HOME is the right thing to do for Descent3_temp_directory , only garbage seems to be written there; that's the purpose of $XDG_CACHE_HOME / $HOME/.cache.

@winterheart
Copy link
Collaborator

SDL_GetPrefPath() is more cross-platform approach, it implements platform-dependent part of path resolving without our intervention.

Fallback to $HOME/.cache is not cross-platform, on Windows no such $HOME variable. This is why environment variables is not good to use - namely each platform has own $HOME naming. SDL_GetPrefPath() / cache is more robust and predictable - we assured to get viable path on every platform where SDL2 even supported.

Don't forget also that Descent3 can be launched in dedicated mode without using desktop environment and where access to environment variables can be limited.

@twolife
Copy link
Author

twolife commented Oct 7, 2024

bottom line: there is indeed an ideological resistance against XDG support in this project; it should be reflected by closing #373

XDG support is a platform specific feature, but you ask for it to be platform-independent ¯_(ツ)_/¯

@Lgt2x
Copy link
Member

Lgt2x commented Oct 7, 2024

Hi @twolife , thanks for opening this pull request! To provide more context, a lot of work has been put lately, especially by @winterheart , into making all target OS code even and unified. Descent 3 used to have a lot of quick-and-dirty tricks to make the game work on Linux and Mac, so we tend to be a little conservative when it comes to adding platform-specific code. That explains some of the push-back you could get here and on #373 when it comes to conforming to XDG specifications.

Moving most of the I/O and system code to SDL2 helped greatly to build a robust cross-platform game, but it still falls short of implementing XDG support. 4 different people in #373 have expressed support for this feature, I think it is reasonable to consider implementing it and assist future Linux package managers. We try to be open to community suggestions and patches.

However, as explained earlier, the current implementation is not correct, as only Linux should implement this logic. Moving the cache directory search to a dedicated and documented function aware of the target OS could be a better way to to it.

@Lgt2x
Copy link
Member

Lgt2x commented Oct 8, 2024

See also #628

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants