diff --git a/code/src/java/pcgen/gui2/PCGenFrame.java b/code/src/java/pcgen/gui2/PCGenFrame.java index fd5c17e9e41..fbe8dd2aa7d 100644 --- a/code/src/java/pcgen/gui2/PCGenFrame.java +++ b/code/src/java/pcgen/gui2/PCGenFrame.java @@ -100,6 +100,7 @@ import pcgen.system.PCGenPropBundle; import pcgen.system.PCGenSettings; import pcgen.system.PropertyContext; +import pcgen.system.PropertyContextFactory; import pcgen.util.Logging; import pcgen.util.chooser.ChooserFactory; import pcgen.util.chooser.RandomChooser; @@ -823,11 +824,7 @@ boolean saveAllCharacters() boolean showSaveCharacterChooser(CharacterFacade character) { PCGenSettings context = PCGenSettings.getInstance(); - String parentPath = lastCharacterPath; - if (parentPath == null) - { - parentPath = context.getProperty(PCGenSettings.PCG_SAVE_PATH); - } + String parentPath = resolveCharacterChooserDir(lastCharacterPath, context); FileChooser fileChooser = new FileChooser(); fileChooser.setTitle("Save PCGen Character File"); @@ -881,6 +878,7 @@ boolean showSaveCharacterChooser(CharacterFacade character) } lastCharacterPath = file.getParent(); + rememberCharacterChooserDir(lastCharacterPath); return true; } catch (Exception e) @@ -926,12 +924,8 @@ public void revertCharacter(CharacterFacade character) public void showOpenCharacterChooser() { GuiAssertions.assertIsNotJavaFXThread(); - PropertyContext context = PCGenSettings.getInstance(); - String path = lastCharacterPath; - if (path == null) - { - path = context.getProperty(PCGenSettings.PCG_SAVE_PATH); - } + PCGenSettings context = PCGenSettings.getInstance(); + String path = resolveCharacterChooserDir(lastCharacterPath, context); FileChooser fileChooser = new FileChooser(); fileChooser.setTitle("Open PCGen Character"); @@ -948,10 +942,44 @@ public void showOpenCharacterChooser() if (file != null) { lastCharacterPath = file.getAbsoluteFile().getParent(); + rememberCharacterChooserDir(lastCharacterPath); loadCharacterFromFile(file); } } + /** + * Persists the last-used character folder to {@code options.ini} so it + * survives a clean shutdown — and immediately flushes it so it also + * survives an abrupt exit (window close from a desktop manager, crash). + * Note: a forced kill (Ctrl-C, SIGKILL) before this point cannot be + * caught, so the most recent folder may still be lost in that case. + */ + private static void rememberCharacterChooserDir(String dir) + { + PCGenSettings.getInstance().setProperty(PCGenSettings.LAST_CHARACTER_PATH, dir); + PropertyContextFactory.getDefaultFactory().savePropertyContexts(); + } + + /** + * Resolve the initial folder for the character Open/Save chooser, in order: + * the in-session {@code lastInSessionPath}; the persisted + * {@link PCGenSettings#LAST_CHARACTER_PATH} from prior runs; the user's + * configured {@link PCGenSettings#PCG_SAVE_PATH} default. + */ + static String resolveCharacterChooserDir(String lastInSessionPath, PropertyContext context) + { + String path = lastInSessionPath; + if (path == null || path.isEmpty()) + { + path = context.getProperty(PCGenSettings.LAST_CHARACTER_PATH); + } + if (path == null || path.isEmpty()) + { + path = context.getProperty(PCGenSettings.PCG_SAVE_PATH); + } + return path; + } + void showOpenPartyChooser() { PropertyContext context = PCGenSettings.getInstance(); diff --git a/code/src/java/pcgen/system/PCGenSettings.java b/code/src/java/pcgen/system/PCGenSettings.java index 3badedbb3c9..97f9a990067 100644 --- a/code/src/java/pcgen/system/PCGenSettings.java +++ b/code/src/java/pcgen/system/PCGenSettings.java @@ -61,6 +61,8 @@ public final class PCGenSettings extends PropertyContext public static final String PCP_SAVE_PATH = "pcgen.files.parties"; public static final String CHAR_PORTRAITS_PATH = "pcgen.files.portaits"; public static final String BACKUP_PCG_PATH = "pcgen.files.characters.backup"; + /** Last folder used in the character Open/Save chooser; remembers across launches. */ + public static final String LAST_CHARACTER_PATH = "pcgen.files.characters.lastUsed"; public static final String SELECTED_SPELL_SHEET_PATH = "pcgen.files.selectedSpellOutputSheet"; public static final String RECENT_CHARACTERS = "recentCharacters"; public static final String RECENT_PARTIES = "recentParties"; @@ -84,20 +86,39 @@ public final class PCGenSettings extends PropertyContext private PCGenSettings() { super("options.ini"); - setProperty(PCG_SAVE_PATH, - (ConfigurationSettings.getUserDir() + "/characters").replace('/', File.separatorChar)); - setProperty(PCP_SAVE_PATH, - (ConfigurationSettings.getUserDir() + "/characters").replace('/', File.separatorChar)); - setProperty(CHAR_PORTRAITS_PATH, - (ConfigurationSettings.getUserDir() + "/characters").replace('/', File.separatorChar)); - setProperty(BACKUP_PCG_PATH, - (ConfigurationSettings.getUserDir() + "/characters").replace('/', File.separatorChar)); + String defaultCharsDir = defaultCharactersDir(); + setProperty(PCG_SAVE_PATH, defaultCharsDir); + setProperty(PCP_SAVE_PATH, defaultCharsDir); + setProperty(CHAR_PORTRAITS_PATH, defaultCharsDir); + setProperty(BACKUP_PCG_PATH, defaultCharsDir); setProperty(VENDOR_DATA_DIR, "@vendordata"); setProperty(HOMEBREW_DATA_DIR, "@homebrewdata"); setProperty(CUSTOM_DATA_DIR, "@data/customsources".replace('/', File.separatorChar)); OutputDB.registerBooleanPreference(OPTION_SHOW_OUTPUT_NAME_FOR_OTHER_ITEMS, false); } + /** + * Default characters/portraits/backups dir, rooted in the user's home so it + * stays writable when PCGen is installed under Program Files. + */ + static String defaultCharactersDir() + { + String root = SystemUtils.USER_HOME; + if (SystemUtils.IS_OS_WINDOWS) + { + String userProfile = System.getenv("USERPROFILE"); + if (userProfile != null && !userProfile.isEmpty()) + { + File documents = new File(userProfile, "Documents"); + if (documents.isDirectory()) + { + root = documents.getAbsolutePath(); + } + } + } + return root + File.separator + "PCGen" + File.separator + "characters"; + } + @Override protected void beforePropertiesSaved() { diff --git a/code/src/test/pcgen/gui2/PCGenFrameResolveCharacterChooserDirTest.java b/code/src/test/pcgen/gui2/PCGenFrameResolveCharacterChooserDirTest.java new file mode 100644 index 00000000000..b8b0b37af9f --- /dev/null +++ b/code/src/test/pcgen/gui2/PCGenFrameResolveCharacterChooserDirTest.java @@ -0,0 +1,80 @@ +/* + * Copyright 2026 Vest + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + */ +package pcgen.gui2; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import pcgen.system.PCGenSettings; +import pcgen.system.PropertyContext; + +import org.junit.jupiter.api.Test; + +/** + * Verifies the fallback order of {@link PCGenFrame#resolveCharacterChooserDir}: + * in-session path → persisted LAST_CHARACTER_PATH → configured PCG_SAVE_PATH. + */ +class PCGenFrameResolveCharacterChooserDirTest +{ + private static final String IN_SESSION = "/session/path"; + private static final String LAST_USED = "/persisted/path"; + private static final String DEFAULT_PCG = "/default/path"; + + @Test + void inSessionPathWinsWhenSet() + { + PropertyContext ctx = new TestContext(); + ctx.setProperty(PCGenSettings.LAST_CHARACTER_PATH, LAST_USED); + ctx.setProperty(PCGenSettings.PCG_SAVE_PATH, DEFAULT_PCG); + + assertEquals(IN_SESSION, PCGenFrame.resolveCharacterChooserDir(IN_SESSION, ctx)); + } + + @Test + void persistedPathWinsWhenInSessionIsNull() + { + PropertyContext ctx = new TestContext(); + ctx.setProperty(PCGenSettings.LAST_CHARACTER_PATH, LAST_USED); + ctx.setProperty(PCGenSettings.PCG_SAVE_PATH, DEFAULT_PCG); + + assertEquals(LAST_USED, PCGenFrame.resolveCharacterChooserDir(null, ctx)); + } + + @Test + void persistedPathWinsWhenInSessionIsEmpty() + { + PropertyContext ctx = new TestContext(); + ctx.setProperty(PCGenSettings.LAST_CHARACTER_PATH, LAST_USED); + ctx.setProperty(PCGenSettings.PCG_SAVE_PATH, DEFAULT_PCG); + + assertEquals(LAST_USED, PCGenFrame.resolveCharacterChooserDir("", ctx)); + } + + @Test + void defaultPathIsUsedWhenNothingElseSet() + { + PropertyContext ctx = new TestContext(); + ctx.setProperty(PCGenSettings.PCG_SAVE_PATH, DEFAULT_PCG); + + assertEquals(DEFAULT_PCG, PCGenFrame.resolveCharacterChooserDir(null, ctx)); + } + + /** Plain {@link PropertyContext} so the test doesn't touch the singleton. */ + private static final class TestContext extends PropertyContext + { + TestContext() + { + super("test-options.ini"); + } + } +} diff --git a/code/src/test/pcgen/system/PCGenSettingsTest.java b/code/src/test/pcgen/system/PCGenSettingsTest.java new file mode 100644 index 00000000000..19d862909b1 --- /dev/null +++ b/code/src/test/pcgen/system/PCGenSettingsTest.java @@ -0,0 +1,89 @@ +/* + * Copyright 2026 Vest + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + */ +package pcgen.system; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.File; +import java.nio.file.Path; + +import org.apache.commons.lang3.SystemUtils; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Tests the writable default and the round-trip persistence of + * {@link PCGenSettings#LAST_CHARACTER_PATH}. + */ +class PCGenSettingsTest +{ + @Test + void defaultCharactersDirIsRootedInUserHome() + { + String dir = PCGenSettings.defaultCharactersDir(); + + assertTrue(dir.startsWith(SystemUtils.USER_HOME), + "Expected default characters dir to live under USER_HOME but was: " + dir); + assertTrue(dir.endsWith(File.separator + "PCGen" + File.separator + "characters"), + "Expected default characters dir to end with PCGen/characters but was: " + dir); + } + + @Test + void defaultCharactersDirIsNotInsideInstallDir() + { + // USER_DIR is the JVM working directory; under Program Files installs it is the + // install root, which is not user-writable. The fix decouples the default from it. + String dir = PCGenSettings.defaultCharactersDir(); + String installRootedDefault = SystemUtils.USER_DIR + File.separator + "characters"; + + assertNotEquals(installRootedDefault, dir, + "Default characters dir must not equal user.dir/characters"); + } + + /** + * Locks the durability claim: {@code LAST_CHARACTER_PATH} written in one + * "session" is read back in the next via {@code options.ini}. + */ + @Test + void lastCharacterPathRoundTripsViaOptionsIni(@TempDir Path settingsDir) + { + String chosenFolder = "/some/folder/the/user/picked"; + + // Session 1: write the property and flush options.ini to the temp dir. + PropertyContextFactory factory1 = new PropertyContextFactory(settingsDir.toString()); + TestableSettings session1 = new TestableSettings(); + factory1.registerAndLoadPropertyContext(session1); + session1.setProperty(PCGenSettings.LAST_CHARACTER_PATH, chosenFolder); + factory1.savePropertyContexts(); + + // Session 2: a fresh factory + context reads the same file back. + PropertyContextFactory factory2 = new PropertyContextFactory(settingsDir.toString()); + TestableSettings session2 = new TestableSettings(); + factory2.registerAndLoadPropertyContext(session2); + + assertEquals(chosenFolder, session2.getProperty(PCGenSettings.LAST_CHARACTER_PATH), + "Persisted folder must survive a save+load cycle through options.ini"); + } + + /** Plain {@link PropertyContext} so we don't touch the {@code PCGenSettings} singleton. */ + private static final class TestableSettings extends PropertyContext + { + TestableSettings() + { + super("options.ini"); + } + } +}