Fix character chooser opening in non-writable Program Files folder#7646
Open
Vest wants to merge 2 commits into
Open
Fix character chooser opening in non-writable Program Files folder#7646Vest wants to merge 2 commits into
Vest wants to merge 2 commits into
Conversation
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 <USER_HOME>/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.
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).
Contributor
Author
|
I have had feedback from my friend — he is tired opening the last opened character from a wrong folder. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes for character file load/save UX.
1. Default folder no longer lands in Program Files
PCGenSettingsseededPCG_SAVE_PATH,PCP_SAVE_PATH,CHAR_PORTRAITS_PATHandBACKUP_PCG_PATHfromConfigurationSettings.getUserDir(), which isSystem.getProperty("user.dir")— the JVM working directory. For a default jpackage.exeinstall on Windows, that resolves toC:\Program Files\PcGen\…, which is not user-writable. Saving a character from there triggers UAC / VirtualStore redirection (or fails outright).The four properties now seed from a new helper
PCGenSettings.defaultCharactersDir():%USERPROFILE%\Documents\PCGen\charactersifDocumentsexists, otherwiseUSER_HOME\PCGen\characters.USER_HOME/PCGen/characters.Read-only
@-prefixed resources (@data,@system,@outputsheets,@vendordata,@homebrewdata,@docs,@plugins,@preview) are untouched — they correctly anchor on the install tree viauser.dir.2. Open/Save chooser remembers the last-used folder across launches
PCGenFramekeptlastCharacterPathin an instance field only — it died with the JVM, so the chooser reset to the configured default on every launch. A new propertypcgen.files.characters.lastUsed(constantPCGenSettings.LAST_CHARACTER_PATH) now captures the parent folder of the last file successfully opened or saved.Resolution order for the chooser's initial folder is now:
lastCharacterPath(this session's most recent pick).LAST_CHARACTER_PATH(most recent pick from any prior session).PCG_SAVE_PATH(the user's preference in Preferences → Locations).The user's preference is never overwritten — it stays the final fallback.
When the
LAST_CHARACTER_PATHvalue gets written tooptions.iniOnly when the application is closed gracefully — i.e. right after a successful Open / Save dialog, via
PropertyContextFactory.savePropertyContexts(). This covers:Main.shutdown).It does not cover:
Ctrl-Cin a terminal-launched gradle run (SIGINT before the write).kill -9/SIGKILL.In those forced-termination cases the most recent folder choice may be lost. That's an existing limitation of PCGen's settings layer (settings are only flushed on clean shutdown today). Doing the write immediately after a successful chooser pick is what bounds the loss window to "since the last open/save action."
Migration
None. Existing users have these properties already written to
options.ini; the new seeding only affects fresh installs or users who deleteoptions.ini. The newLAST_CHARACTER_PATHkey starts empty and populates the first time a user picks a file.Test Plan
Automated:
./gradlew compileJava compileTestJavapasses../gradlew :test --tests pcgen.system.PCGenSettingsTest— newPCGenSettingsTest(2 tests) passes:defaultCharactersDirIsRootedInUserHome— locks theUSER_HOMErooting.defaultCharactersDirIsNotInsideInstallDir— locks the regression: default ≠user.dir/characters.Manual (recommended on Windows):
options.inifrom the settings dir (e.g.~/Library/Preferences/pcgen/options.inion macOS; the chosen settings dir on Windows)..exe(or./gradlew run).<USER_HOME>/PCGen/characters(or WindowsDocuments\PCGen\characters)..pcgfile.options.ininow containspcgen.files.characters.lastUsed=<the folder you browsed to>.