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

fix(PathManager): allow paths to be symlinks #5036

Merged
merged 5 commits into from
Jun 4, 2022

Conversation

keturn
Copy link
Member

@keturn keturn commented May 31, 2022

Useful for directories like cachedModules.

How to test

In your Terasology home directory, replace cachedModules with a symlink. For example

mv cachedModules nowHere
ln -s nowHere cachedModules

Make sure you can still start the game and that it is able to load modules.

Useful for directories like cachedModules.
@keturn keturn added the Size: S Small effort likely only affecting a single area and requiring little to no research label May 31, 2022
@github-actions github-actions bot added the Type: Bug Issues reporting and PRs fixing problems label May 31, 2022
%n is only a newline when run through a Formatter (or printf).
@@ -97,11 +98,11 @@ private static Path findInstallPath() {
}
}

System.err.println(
System.err.printf(
Copy link
Member

Choose a reason for hiding this comment

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

Does PMD have a valid point here in asking to use a logger instead of plain write to console?

Copy link
Member Author

Choose a reason for hiding this comment

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

PathManager sometimes does stuff like this I wouldn't do elsewhere because it's so early in the boot sequence. Do you use the logger while initializing the object that's responsible for providing getLogPath?

(maybe? like, it'll still print WARN and ERROR to the console, right? idk)

Copy link
Member

Choose a reason for hiding this comment

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

I would hope that the logger setup works for whatever code we write "afterwards", including the path manager and all that - but I'm not sure about that either.

No need to change this right now in this PR, but maybe we can keep it in mind/try it out in the future.

@@ -97,11 +98,11 @@ private static Path findInstallPath() {
}
}

System.err.println(
System.err.printf(
Copy link
Member

Choose a reason for hiding this comment

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

I would hope that the logger setup works for whatever code we write "afterwards", including the path manager and all that - but I'm not sure about that either.

No need to change this right now in this PR, but maybe we can keep it in mind/try it out in the future.

@keturn keturn merged commit c42f936 into develop Jun 4, 2022
@keturn keturn deleted the fix/allowSymlinkDirectories branch June 4, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: S Small effort likely only affecting a single area and requiring little to no research Type: Bug Issues reporting and PRs fixing problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants