From 5b54cedf9850fb8efa803c6169f6883229dddb81 Mon Sep 17 00:00:00 2001 From: Vest Date: Sat, 27 Jun 2026 01:02:22 +0200 Subject: [PATCH 1/2] Fix character chooser opening in non-writable Program Files folder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes: 1. Seed the character/portraits/backup folder defaults from the user's home directory instead of user.dir. user.dir resolves to the install tree, which on a default jpackage Windows install is under Program Files — non-writable without elevation. The new default is /PCGen/characters (Documents\PCGen\characters on Windows when Documents exists). Read-only @-prefixed resources (data, system, outputsheets, ...) still anchor on user.dir; only the writable user- document defaults move. 2. Persist the last-used folder of the character Open/Save chooser to options.ini so the chooser remembers it across launches. Today only an in-memory field is kept, so every new session reverts to the configured default. A new property pcgen.files.characters.lastUsed captures this; the chooser falls back to PCG_SAVE_PATH when not set. The write happens immediately after a successful open/save (via PropertyContextFactory.savePropertyContexts), so the value survives a normal window close. A forced kill (Ctrl-C, SIGKILL) before the write is unavoidable. Existing options.ini values are not migrated; the new defaults only affect fresh installs or users who delete options.ini. --- code/src/java/pcgen/gui2/PCGenFrame.java | 50 ++++++++++++++---- code/src/java/pcgen/system/PCGenSettings.java | 37 +++++++++++--- .../test/pcgen/system/PCGenSettingsTest.java | 51 +++++++++++++++++++ 3 files changed, 119 insertions(+), 19 deletions(-) create mode 100644 code/src/test/pcgen/system/PCGenSettingsTest.java diff --git a/code/src/java/pcgen/gui2/PCGenFrame.java b/code/src/java/pcgen/gui2/PCGenFrame.java index fd5c17e9e41..1968bb8a32e 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(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(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 lastCharacterPath}; the persisted + * {@link PCGenSettings#LAST_CHARACTER_PATH} from prior runs; the user's + * configured {@link PCGenSettings#PCG_SAVE_PATH} default. + */ + private String resolveCharacterChooserDir(PropertyContext context) + { + String path = lastCharacterPath; + 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/system/PCGenSettingsTest.java b/code/src/test/pcgen/system/PCGenSettingsTest.java new file mode 100644 index 00000000000..43de1946866 --- /dev/null +++ b/code/src/test/pcgen/system/PCGenSettingsTest.java @@ -0,0 +1,51 @@ +/* + * 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.assertTrue; + +import java.io.File; + +import org.apache.commons.lang3.SystemUtils; +import org.junit.jupiter.api.Test; + +/** + * Verifies that the default character/portraits/backup directory is rooted in + * the user's home rather than {@code user.dir}. + */ +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"; + + assertTrue(!dir.equals(installRootedDefault), + "Default characters dir must not equal user.dir/characters but was: " + dir); + } +} From 9eab51db2b9578afdd9d08fd0cae393de7a8380a Mon Sep 17 00:00:00 2001 From: Vest Date: Sat, 27 Jun 2026 01:07:38 +0200 Subject: [PATCH 2/2] Test LAST_CHARACTER_PATH round-trip and chooser dir fallback Add coverage for the persistence + fallback logic introduced earlier: - PCGenSettingsTest.lastCharacterPathRoundTripsViaOptionsIni: writes the property in one factory session, re-loads it in a fresh one against a @TempDir, and asserts the value survives. Locks the durability claim end-to-end through PropertyContextFactory. - PCGenFrameResolveCharacterChooserDirTest: covers the three-step fallback (in-session -> persisted LAST_CHARACTER_PATH -> configured PCG_SAVE_PATH) plus null/empty in-session edge cases. Catches any accidental reorder of the chain. To make the fallback testable, resolveCharacterChooserDir is now a static pure function that takes the in-session path as a parameter (was a private instance method reading lastCharacterPath directly). --- code/src/java/pcgen/gui2/PCGenFrame.java | 10 +-- ...enFrameResolveCharacterChooserDirTest.java | 80 +++++++++++++++++++ .../test/pcgen/system/PCGenSettingsTest.java | 46 ++++++++++- 3 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 code/src/test/pcgen/gui2/PCGenFrameResolveCharacterChooserDirTest.java diff --git a/code/src/java/pcgen/gui2/PCGenFrame.java b/code/src/java/pcgen/gui2/PCGenFrame.java index 1968bb8a32e..fbe8dd2aa7d 100644 --- a/code/src/java/pcgen/gui2/PCGenFrame.java +++ b/code/src/java/pcgen/gui2/PCGenFrame.java @@ -824,7 +824,7 @@ boolean saveAllCharacters() boolean showSaveCharacterChooser(CharacterFacade character) { PCGenSettings context = PCGenSettings.getInstance(); - String parentPath = resolveCharacterChooserDir(context); + String parentPath = resolveCharacterChooserDir(lastCharacterPath, context); FileChooser fileChooser = new FileChooser(); fileChooser.setTitle("Save PCGen Character File"); @@ -925,7 +925,7 @@ public void showOpenCharacterChooser() { GuiAssertions.assertIsNotJavaFXThread(); PCGenSettings context = PCGenSettings.getInstance(); - String path = resolveCharacterChooserDir(context); + String path = resolveCharacterChooserDir(lastCharacterPath, context); FileChooser fileChooser = new FileChooser(); fileChooser.setTitle("Open PCGen Character"); @@ -962,13 +962,13 @@ private static void rememberCharacterChooserDir(String dir) /** * Resolve the initial folder for the character Open/Save chooser, in order: - * the in-session {@code lastCharacterPath}; the persisted + * 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. */ - private String resolveCharacterChooserDir(PropertyContext context) + static String resolveCharacterChooserDir(String lastInSessionPath, PropertyContext context) { - String path = lastCharacterPath; + String path = lastInSessionPath; if (path == null || path.isEmpty()) { path = context.getProperty(PCGenSettings.LAST_CHARACTER_PATH); 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 index 43de1946866..19d862909b1 100644 --- a/code/src/test/pcgen/system/PCGenSettingsTest.java +++ b/code/src/test/pcgen/system/PCGenSettingsTest.java @@ -13,16 +13,20 @@ */ 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; /** - * Verifies that the default character/portraits/backup directory is rooted in - * the user's home rather than {@code user.dir}. + * Tests the writable default and the round-trip persistence of + * {@link PCGenSettings#LAST_CHARACTER_PATH}. */ class PCGenSettingsTest { @@ -45,7 +49,41 @@ void defaultCharactersDirIsNotInsideInstallDir() String dir = PCGenSettings.defaultCharactersDir(); String installRootedDefault = SystemUtils.USER_DIR + File.separator + "characters"; - assertTrue(!dir.equals(installRootedDefault), - "Default characters dir must not equal user.dir/characters but was: " + dir); + 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"); + } } }