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

Set Legendary's config path to a Heroic-specific location #2803

Merged
merged 2 commits into from Jun 30, 2023

Conversation

CommandMC
Copy link
Collaborator

Legendary's configuration will now be stored in a Heroic-specific location. This avoids issues where users change their config.ini and in turn change Heroic's behavior.
Existing configuration data will be migrated automatically, meaning anyone testing this PR should not notice anything out of the ordinary.

The original issue I've aimed to solve here was reported on our Discord (here): When launching a game, we used to run legendary launch <AppName> --json --offline to get Legendary's original launch configuration for the game. This is problematic on macOS systems without CrossOver installed, as Legendary will crash when trying to auto-detect a version.
With us now controlling the config file, we simply don't have to check for user modifications at all.


Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

Legendary derives its configuration path using `XDG_CONFIG_HOME`. By
setting this environment variable to a Heroic-specific location, we can
rule out the user influencing Heroic's functionality by changing
Legendary's config file.

Legendary's configuration now lives in
`<heroic config>/legendaryConfig`, and Heroic will migrate an already
existent config directory to this new path if it's found.
The purpose of this was making sure the user didn't modify `config.ini`
in a problematic way. Now that we alone control the config file, we
don't have to worry about this anymore.

As a side effect, this also resolves issues for MacOS users without a
CrossOver installation (Legendary would try to auto-detect CX & crash if
nothing is found)
@CommandMC CommandMC added the pr:ready-for-review Feature-complete, ready for the grind! :P label Jun 16, 2023
@CommandMC CommandMC requested a review from a team June 16, 2023 21:36
@CommandMC CommandMC self-assigned this Jun 16, 2023
@CommandMC CommandMC requested review from arielj, flavioislima, Nocccer and imLinguin and removed request for a team June 16, 2023 21:36
@arielj
Copy link
Collaborator

arielj commented Jun 20, 2023

I wonder if we want to do this or not, because there are some settings that can be configured in Legendary's .ini file that we can't set in Heroic (like a pre-launch script, max memory, preferred cdn) and maybe some users are using that and if we duplicate this configure somewhere else this will be confusing if they go and try to edit things (they'll edit the original file without knowing we copied them inside heroic's settings).

Maybe it's safer to just catch that error so it's not showed to the user and skip the config logging?

That way we remove the error for the user without duplicating Legendary's settings.

It sounds like it's a bug in Legendary that it fails in those cases and it would go away eventually when fixed, and we can ignore it since it's just something we run to have more logs.

@flavioislima
Copy link
Member

I wonder if we want to do this or not, because there are some settings that can be configured in Legendary's .ini file that we can't set in Heroic (like a pre-launch script, max memory, preferred cdn) and maybe some users are using that and if we duplicate this configure somewhere else this will be confusing if they go and try to edit things (they'll edit the original file without knowing we copied them inside heroic's settings).

Maybe it's safer to just catch that error so it's not showed to the user and skip the config logging?

That way we remove the error for the user without duplicating Legendary's settings.

It sounds like it's a bug in Legendary that it fails in those cases and it would go away eventually when fixed, and we can ignore it since it's just something we run to have more logs.

I believe we should do this change because it not only fixes the issue on Mac but also makes the launching faster.
I really don't think it is common for Heroic users to use Legendary-specific configurations. Also, the features you mentioned, like running a script before the game, it is on our Roadmap and I want to work on it soon.

Rodney actually developed this feature of using a specific configuration because I asked to a long time ago since it was causing issues with Heroic. This also makes the usage of Legendary within Heroic more predictable since we know exactly the configuration we will use instead of dealing with some modified config in Legendary that can conflict with another config Heroic passes.

Copy link
Member

@flavioislima flavioislima left a comment

Choose a reason for hiding this comment

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

LGTM

@arielj
Copy link
Collaborator

arielj commented Jun 21, 2023

Rodney actually developed this feature of using a specific configuration because I asked to a long time ago since it was causing issues with Heroic. This also makes the usage of Legendary within Heroic more predictable since we know exactly the configuration we will use instead of dealing with some modified config in Legendary that can conflict with another config Heroic passes.

is this using a legendary feature? we are changing the XDG_CONFIG_HOME env variable, maybe that's how the feature should work (I don't see any docs about it) but I would expect a flag passed to the legendary command to only affect legendary and nothing else. I think my question is, will this env variable change affect only legendary or other things like a wrapper or a native linux game looking for the home folder using this env too?

about the game starting faster, I think the log should be done async instead of sync to not only don't delay the game launching but also not block it if it fails

my concern is that I don't know the repercussions of changing this env variable

@CommandMC
Copy link
Collaborator Author

Linux applications are supposed to respect XDG_CONFIG_HOME (see the XDG Base Directory Spec), with Legendary adding support for it in this commit.
The only other applications that could honor this would be other Linux-native applications, but Legendary will never launch one of those, so this should be fine

@flavioislima flavioislima merged commit c0bab68 into main Jun 30, 2023
13 checks passed
@flavioislima flavioislima deleted the legendary-config branch June 30, 2023 11:19
@butterbutts
Copy link

It seems like the Epic Games alternative login method still uses the old .config directory for legendary. Not sure if this is supposed to be the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants