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.

Sign up for GitHub

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 dev.dirs to calculate terasology directories #5284

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

use dev.dirs to calculate terasology directories #5284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions engine/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ dependencies {
api(libs.guava)
api(libs.gson)
api("net.sf.trove4j:trove4j:3.0.3")
implementation("dev.dirs:directories:26")
implementation("io.netty:netty-all:4.1.77.Final")
implementation("com.google.protobuf:protobuf-java:${libs.versions.protobuf.get().toString()}")
implementation("org.lz4:lz4-java:1.8.0")
Expand Down
55 changes: 16 additions & 39 deletions engine/src/main/java/org/terasology/engine/core/PathManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
package org.terasology.engine.core;

import com.google.common.collect.ImmutableList;
import com.sun.jna.platform.win32.KnownFolders;
import com.sun.jna.platform.win32.Shell32Util;
import dev.dirs.ProjectDirectories;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.terasology.engine.context.Context;
Expand Down Expand Up @@ -33,15 +32,14 @@
*/
public final class PathManager {
private static final Logger LOGGER = LoggerFactory.getLogger(PathManager.class);
private static final String TERASOLOGY_FOLDER_NAME = "Terasology";
private static final Path LINUX_HOME_SUBPATH = Paths.get(".local", "share", "terasology");

private static final ProjectDirectories PROJECT_DIRS = ProjectDirectories.from("org", "terasology", "terasology");
private static final Path PROJECT_PATH = Paths.get(PROJECT_DIRS.dataDir);
private static final String SAVED_GAMES_DIR = "saves";
private static final String RECORDINGS_LIBRARY_DIR = "recordings";
private static final String LOG_DIR = "logs";
private static final String SHADER_LOG_DIR = "shaders";
private static final String LOG_DIR = PROJECT_DIRS.dataLocalDir + "/logs";
private static final String SHADER_LOG_DIR = PROJECT_DIRS.dataLocalDir + "/shaders";
private static final String MODULE_DIR = "modules";
private static final String MODULE_CACHE_DIR = "cachedModules";
private static final String MODULE_CACHE_DIR = PROJECT_DIRS.cacheDir + "/cachedModules";
private static final String SCREENSHOT_DIR = "screenshots";
private static final String NATIVES_DIR = "natives";
private static final String CONFIGS_DIR = "configs";
Copy link

Choose a reason for hiding this comment

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

Suggested change
private static final String CONFIGS_DIR = "configs";
private static final String CONFIGS_DIR = PROJECT_DIRS.configDir;

Expand All @@ -67,7 +65,7 @@ public final class PathManager {

private PathManager() {
installPath = findInstallPath();
homePath = installPath;
homePath = PROJECT_PATH;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
homePath = PROJECT_PATH;
homePath = installPath;

findInstallPath() provides a fallback path for unusual scenarios that I think we should keep. Defaulting to the current directory is normal for development anyway, I think. You can default to PROJECT_PATH in findInstallPath if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

returning projectpath in a function called findinstallpath i thought is too much :) but let me digest your suggestion ...

}

private static Path findInstallPath() {
Expand Down Expand Up @@ -159,7 +157,11 @@ static PathManager setInstance(PathManager pathManager) {
}

/**
* Uses the given path as the home instead of the default home path.
* Uses the given path as the home instead of the default home path. Especially interesting for unit tests, as java>17 does not
* make it easy to set environment variables. see: https://www.baeldung.com/java-unit-testing-environment-variables .
*
* Currently LOG_DIR and LOG_SHADER_DIR are not affected here, as not based on homePath.
*
* @param rootPath Path to use as the home path.
* @throws IOException Thrown when required directories cannot be accessed.
*/
Expand All @@ -173,33 +175,8 @@ public void useOverrideHomePath(Path rootPath) throws IOException {
* @throws IOException Thrown when required directories cannot be accessed.
*/
public void useDefaultHomePath() throws IOException {
switch (OS.get()) {
case LINUX:
homePath = Paths.get(System.getProperty("user.home")).resolve(LINUX_HOME_SUBPATH);
break;
case MACOSX:
homePath = Paths.get(System.getProperty("user.home"), "Library", "Application Support", TERASOLOGY_FOLDER_NAME);
break;
case WINDOWS:
String savedGamesPath = Shell32Util
.getKnownFolderPath(KnownFolders.FOLDERID_SavedGames);
if (savedGamesPath == null) {
savedGamesPath = Shell32Util
.getKnownFolderPath(KnownFolders.FOLDERID_Documents);
}
Path rawPath;
if (savedGamesPath != null) {
rawPath = Paths.get(savedGamesPath);
} else {
rawPath = new JFileChooser().getFileSystemView().getDefaultDirectory()
.toPath();
}
homePath = rawPath.resolve(TERASOLOGY_FOLDER_NAME);
break;
default:
homePath = Paths.get(System.getProperty("user.home")).resolve(LINUX_HOME_SUBPATH);
break;
}
// use datadir, .local/share for linux e.g.
homePath = PROJECT_PATH;
updateDirs();
}

Expand Down Expand Up @@ -316,8 +293,8 @@ public Path getSandboxPath() {
private void updateDirs() throws IOException {
savesPath = homePath.resolve(SAVED_GAMES_DIR);
recordingsPath = homePath.resolve(RECORDINGS_LIBRARY_DIR);
logPath = homePath.resolve(LOG_DIR);
shaderLogPath = logPath.resolve(SHADER_LOG_DIR);
logPath = Paths.get(LOG_DIR);
shaderLogPath = Paths.get(SHADER_LOG_DIR);
Comment on lines -319 to +297
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are logPath and shaderLogPath absolute paths when all the other paths are relative to the home directory?

Copy link
Contributor Author

@soloturn soloturn Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why are logPath and shaderLogPath absolute paths when all the other paths are relative to the home directory?

the reason why i like the new appdir / xdg / macos directory structure so much is that one can properly separate stuff which should be in a backup and what not. config i want to backup. shared data as well. local data not, but depends on the use case. cache and logs i never want a backup. coming back to your question: i did not see any harm at the moment to leave e.g. config in homedirectory, even it is not xdg compliant.

screenshotPath = homePath.resolve(SCREENSHOT_DIR);
nativesPath = installPath.resolve(NATIVES_DIR);
configsPath = homePath.resolve(CONFIGS_DIR);
Copy link

Choose a reason for hiding this comment

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

Suggested change
configsPath = homePath.resolve(CONFIGS_DIR);
configsPath = Paths.get(CONFIGS_DIR);

Expand Down
1 change: 1 addition & 0 deletions engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,10 @@
logger.info("{}", TerasologyVersion.getInstance());
logger.info("Home path: {}", PathManager.getInstance().getHomePath());
logger.info("Install path: {}", PathManager.getInstance().getInstallPath());
logger.info("Log path: {}", PathManager.getInstance().getLogPath());

Check warning on line 273 in engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java

View check run for this annotation

Terasology Jenkins.io / PMD

GuardLogStatementJavaUtil 

HIGH:
Logger calls should be surrounded by log level guards.
logger.info("Java: {} in {}", System.getProperty("java.version"), System.getProperty("java.home"));

Check warning on line 274 in engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java

View check run for this annotation

Terasology Jenkins.io / PMD

GuardLogStatementJavaUtil 

HIGH:
Logger calls should be surrounded by log level guards.
logger.info("Java VM: {}, version: {}", System.getProperty("java.vm.name"), System.getProperty("java.vm" +
".version"));

Check warning on line 276 in engine/src/main/java/org/terasology/engine/core/TerasologyEngine.java

View check run for this annotation

Terasology Jenkins.io / PMD

GuardLogStatementJavaUtil 

HIGH:
Logger calls should be surrounded by log level guards.
logger.info("OS: {}, arch: {}, version: {}", System.getProperty("os.name"), System.getProperty("os.arch"),
System.getProperty("os.version"));
logger.info("Max. Memory: {} MiB", Runtime.getRuntime().maxMemory() / ONE_MEBIBYTE);
Expand Down
9 changes: 1 addition & 8 deletions facades/PC/src/main/java/org/terasology/engine/Terasology.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,7 @@ private void handleLaunchArguments() throws IOException {
setMemoryLimit(maxDataSize);
}

if (homeDir != null) {
logger.info("homeDir is {}", homeDir);
PathManager.getInstance().useOverrideHomePath(homeDir);
// TODO: what is this?
// PathManager.getInstance().chooseHomePathManually();
} else {
PathManager.getInstance().useDefaultHomePath();
}
PathManager.getInstance().useDefaultHomePath();
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not have been removed. Having the ability to override the game's home directory is important for testing with multiple clients.

Copy link
Contributor Author

@soloturn soloturn Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

how is this test done, or in other words, why setting XDG environment variables is not good enough?


if (isHeadless) {
crashReportEnabled = false;
Expand Down
Loading