Skip to content

Fix character chooser opening in non-writable Program Files folder#7646

Open
Vest wants to merge 2 commits into
PCGen:masterfrom
Vest:fix-program-files-default-and-last-folder
Open

Fix character chooser opening in non-writable Program Files folder#7646
Vest wants to merge 2 commits into
PCGen:masterfrom
Vest:fix-program-files-default-and-last-folder

Conversation

@Vest

@Vest Vest commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Two related fixes for character file load/save UX.

1. Default folder no longer lands in Program Files

PCGenSettings seeded PCG_SAVE_PATH, PCP_SAVE_PATH, CHAR_PORTRAITS_PATH and BACKUP_PCG_PATH from ConfigurationSettings.getUserDir(), which is System.getProperty("user.dir") — the JVM working directory. For a default jpackage .exe install on Windows, that resolves to C:\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():

  • Windows: %USERPROFILE%\Documents\PCGen\characters if Documents exists, otherwise USER_HOME\PCGen\characters.
  • macOS / Linux / other: 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 via user.dir.

2. Open/Save chooser remembers the last-used folder across launches

PCGenFrame kept lastCharacterPath in an instance field only — it died with the JVM, so the chooser reset to the configured default on every launch. A new property pcgen.files.characters.lastUsed (constant PCGenSettings.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:

  1. In-session lastCharacterPath (this session's most recent pick).
  2. Persisted LAST_CHARACTER_PATH (most recent pick from any prior session).
  3. Configured 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_PATH value gets written to options.ini

Only when the application is closed gracefully — i.e. right after a successful Open / Save dialog, via PropertyContextFactory.savePropertyContexts(). This covers:

  • Normal File → Exit / Cmd-Q / window close (calls Main.shutdown).
  • The window manager closing the app cleanly.

It does not cover:

  • Ctrl-C in a terminal-launched gradle run (SIGINT before the write).
  • kill -9 / SIGKILL.
  • OS crash, power loss.

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 delete options.ini. The new LAST_CHARACTER_PATH key starts empty and populates the first time a user picks a file.

Test Plan

Automated:

  • ./gradlew compileJava compileTestJava passes.
  • ./gradlew :test --tests pcgen.system.PCGenSettingsTest — new PCGenSettingsTest (2 tests) passes:
    • defaultCharactersDirIsRootedInUserHome — locks the USER_HOME rooting.
    • defaultCharactersDirIsNotInsideInstallDir — locks the regression: default ≠ user.dir/characters.

Manual (recommended on Windows):

  1. Delete options.ini from the settings dir (e.g. ~/Library/Preferences/pcgen/options.ini on macOS; the chosen settings dir on Windows).
  2. Launch PCGen via the installer-built .exe (or ./gradlew run).
  3. File → Open Character — verify the dialog opens at <USER_HOME>/PCGen/characters (or Windows Documents\PCGen\characters).
  4. Navigate elsewhere, open any .pcg file.
  5. File → Exit (clean exit).
  6. Confirm options.ini now contains pcgen.files.characters.lastUsed=<the folder you browsed to>.
  7. Re-launch PCGen, File → Open Character — verify the dialog opens at that remembered folder.

Vest added 2 commits June 27, 2026 01:02
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).
@Vest

Vest commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

I have had feedback from my friend — he is tired opening the last opened character from a wrong folder.
When the user closes the application gracefully, I store the last accessed folder into option.ini and re-use it in the second run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant