Fix NPE on load and improve row clicking

[karianna]

• build.gradle: fix clean/download race and merged-module missing JDK requires (build.gradle: fix clean/download race and merged-module missing JDK requires #7583)
• build.gradle, reporting.gradle: small cleanups (build.gradle, reporting.gradle: small cleanups #7584)
• Build cleanup: distribution / plugins / release.gradle drive-by fixes (Build cleanup: distribution / plugins / release.gradle drive-by fixes #7585)
• fix NPE on PCGen load, update Agent instructions, stabalize some UI elements (row open/close)

[Vest]

Independent review — verdict: merge with nits

NPE fix looks structurally correct. Splitting JFXPanelFromResource (Swing-embedded, JFXPanel auto-inits FX runtime) from PanelFromResource (pure FXML→Stage, no embedded peer) is the right shape. All showAsStage/showAndBlock call sites are migrated; remaining JFXPanelFromResource references in the tree (InfoGuidePane, DataInstaller html pane, the various InfoTab html panes, HtmlSheetSupport, ConvertedJavaFXPanel, PCGenPreferencesModel) are all genuine Swing-embed uses — verified.

Findings

None of these block merge.

1. AGENTS.md version table is stale on day one. This is a bit awkward given the PR's own preamble says "keep this file up to date":

   • Gradle 9.5.0 → actual 9.5.1
   • JUnit BOM 6.0.3 → 6.1.0
   • Saxon 12.9 → 13.0
   • SpotBugs 6.5.4 → 6.5.5
   • PMD 7.21.0 / 7.24.0 → 7.25.0
   • jlink 4.0.0 → 4.0.2

   Either fix the numbers or drop the version table entirely — otherwise it'll rot fast and become a misleading reference.

2. PanelFromResource.showAsStage is not idempotent on the same instance (code/src/java/pcgen/gui3/PanelFromResource.java:80–96). It calls fxmlLoader.load() per invocation, so calling showAsStage twice on one instance re-parses the FXML and replaces the controller. No current caller does this (every site builds a fresh instance), but worth a javadoc warning, or load-once into a Parent and build a fresh Scene per show.

3. getController() returns null on a never-shown instance (PanelFromResource.java:64–72). Old JFXPanelFromResource loaded FXML in the constructor, so getController() was always usable. The new javadoc documents this, and no migrated site reads the controller before show* (verified for Main.loadProperties and PurchaseModeFrame.addMethodButtonActionPerformed, which reads it after showAndBlock returns — i.e. after the inline fxmlLoader.load() inside the Platform.runLater body has run). Behavior is correct, but any future caller doing new PanelFromResource(...).getController().setX(...) to pre-configure before show will silently NPE.

4. Calculator dialog is no longer a singleton across F11 presses (PCGenActionMap.java:258–280). Worth flagging in the commit message that this is intentional: the old singleton reused a Scene across Stages, which is precisely the embedded-scene-reparenting bug being fixed. A Scene can only be attached to one Stage at a time. State loss between presses is expected and matches the existing Export-dialog pattern.

5. .gitignore additions (.gitignore:120–122). .superset/ appears to be local tooling/IDE noise — no references in build.gradle, CI, or anywhere else. Harmless to ignore. The leading blank line above it is gratuitous; consider tightening.

6. Javadoc nit on PanelFromResource.showAndBlock: verify the documented throw type matches what GuiAssertions.assertIsNotJavaFXThread() actually throws.

Recommendation

Merge as-is is fine; ideally fix nit #1 first (or remove the version table) since the PR explicitly establishes the rule that AGENTS.md should stay accurate. Nits 2 and 3 are good follow-up material.

(Reviewed by an LLM agent; happy to clarify any of the above.)

[Vest]

Update: ran the repro locally — PR introduces a startup regression

Followed up on my own review by actually running both the parent commit and this PR's HEAD on macOS Retina (Built-in Liquid Retina XDR, 3456×2234). Setup: moved repo config.ini aside so Main.loadProperties falls into the OptionsPathDialog flow on next launch (mimics a fresh install).

Old code (ad6faf0df6, parent of this PR) — bug reproduces

java.lang.NullPointerException: Cannot invoke "com.sun.javafx.tk.quantum.SceneState.update()" because "this.sceneState" is null
    at javafx.graphics@25.0.3/com.sun.javafx.tk.quantum.GlassScene.updateSceneState(GlassScene.java:253)
    at javafx.graphics@25.0.3/com.sun.javafx.tk.quantum.EmbeddedScene.lambda$setPixelScaleFactors$1(EmbeddedScene.java:158)
    at javafx.graphics@25.0.3/com.sun.javafx.tk.quantum.QuantumToolkit.runWithRenderLock(QuantumToolkit.java:442)
    at javafx.graphics@25.0.3/com.sun.javafx.tk.quantum.EmbeddedScene.lambda$setPixelScaleFactors$0(EmbeddedScene.java:157)
    at javafx.graphics@25.0.3/com.sun.javafx.application.PlatformImpl.lambda$runLater$0(PlatformImpl.java:424)
    ...
    at javafx.graphics@25.0.3/javafx.stage.Stage.showAndWait(Stage.java:454)
    at pcgen.gui3.JFXPanelFromResource.lambda$showAndBlock$0(JFXPanelFromResource.java:108)

Diagnosis confirmed: Stage.showAndWait enters the nested event loop, EmbeddedScene.setPixelScaleFactors runs (HiDPI scale recompute on the now-orphaned embedded scene), sceneState is null, NPE. Fires twice during dialog open and twice more during normal UI operation. JavaFX's LoggingUncaughtExceptionHandler swallows them, so the app limps along.

PR HEAD (8461bd4c68) — new regression on the same code path

java.lang.IllegalStateException: Toolkit not initialized
    at javafx.graphics@25.0.3/com.sun.javafx.application.PlatformImpl.runLater(PlatformImpl.java:408)
    at javafx.graphics@25.0.3/com.sun.javafx.application.PlatformImpl.runLater(PlatformImpl.java:403)
    at javafx.graphics@25.0.3/javafx.application.Platform.runLater(Platform.java:171)
    at pcgen.gui3.PanelFromResource.showAndBlock(PanelFromResource.java:132)
    at pcgen.system.Main.loadProperties(Main.java:265)
    at pcgen.system.Main.startupWithGUI(Main.java:185)
    at pcgen.system.Main.main(Main.java:136)

The JVM exits after the SEVERE log line — OptionsPathDialog never opens, Main.loadProperties aborts. A fresh install (no config.ini/settingsPath) cannot start the app.

Cause

JFXPanelFromResource extends JFXPanel. Instantiating JFXPanel has a critical side effect: JFXPanel.<init> calls PlatformImpl.startup(...) and bootstraps the JavaFX toolkit. The old startup path worked from the main thread — even with the NPE in HiDPI rendering — because new JFXPanelFromResource(...) had already brought up the toolkit.

The new PanelFromResource is a plain Object and does no such bootstrapping. The first Platform.runLater from main thread throws IllegalStateException: Toolkit not initialized, full stop. The PR removed the implicit init without replacing it.

Suggested fix

In Main.startupWithGUI (or wherever the first FX call may originate), explicitly initialize the toolkit before any Platform.runLater. Either:

// Option 1: cheap, well-defined
javafx.embed.swing.JFXPanel.<init>();   // discard the panel; just need the side effect

or, more cleanly:

// Option 2: explicit, no Swing dependency
javafx.application.Platform.startup(() -> { /* no-op */ });

Platform.startup is idempotent against IllegalStateException only if you guard it (it throws if already started), so Platform.isFxApplicationThread() doesn't help here — a private static volatile boolean fxStarted flag, or catching the IllegalStateException from startup, is the cleanest pattern.

Repro recipe

For anyone wanting to confirm:

mv config.ini /tmp/config.ini.bak       # forces OptionsPathDialog flow
git checkout ad6faf0df6                 # parent of this PR — old code
./gradlew run                           # see GlassScene.updateSceneState NPE in pcgen.log
# kill app
git checkout 8461bd4c68                 # this PR's HEAD
./gradlew run                           # see Toolkit not initialized in pcgen.log; app exits
mv /tmp/config.ini.bak config.ini       # restore

Reverting my earlier "merge with nits" recommendation to hold — please address the toolkit-init regression before merging. The split design is right; the implementation just needs the missing toolkit-bootstrap call.

(LLM-assisted review; happy to clarify or test a proposed fix.)

[Vest]

@karianna my bot agrees with you. I think, you can merge.

[Copilot]

Pull request overview

This PR replaces use of JFXPanel-backed FXML loaders for standalone dialogs with a new PanelFromResource helper to avoid a macOS HiDPI JavaFX embedded-scene NPE, and updates developer/agent documentation accordingly.

Changes:

• Introduce pcgen.gui3.PanelFromResource for opening FXML dialogs as standalone Stages (non-embedded) and migrate several call sites to it.
• Remove stage-opening APIs from JFXPanelFromResource and clarify its intended Swing-embedding-only usage.
• Update AGENTS.md and .gitignore.

Show a summary per file

File	Description
code/src/java/pcgen/system/Main.java	Switches options-path prompt to PanelFromResource during startup.
code/src/java/pcgen/gui3/PanelFromResource.java	New helper for loading/showing FXML as a standalone Stage; adds blocking show API.
code/src/java/pcgen/gui3/JFXPanelFromResource.java	Removes stage APIs and documents Swing-embedding-only constraint.
code/src/java/pcgen/gui3/component/PCGenToolBar.java	Uses PanelFromResource for the export dialog.
code/src/java/pcgen/gui2/prefs/PurchaseModeFrame.java	Uses PanelFromResource for the “New Purchase Method” dialog.
code/src/java/pcgen/gui2/PCGenActionMap.java	Uses PanelFromResource for calculator/export dialogs.
AGENTS.md	Expands build/test guidance and JavaFX dialog guidance.
.gitignore	Ignores .superset/.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (1)

code/src/java/pcgen/system/Main.java:266

• startupWithGUI() calls loadProperties(true) before JavaFX is initialized (new JFXPanel() happens later). With this change, loadProperties may call PanelFromResource.showAndBlock(...), which uses Platform.runLater(...) and will throw IllegalStateException: Toolkit not initialized when the settings dir prompt is needed. Previously the JFXPanelFromResource constructor implicitly initialized JavaFX via JFXPanel.

			var panel = new PanelFromResource<>(
					OptionsPathDialogController.class,
					"OptionsPathDialog.fxml"
			);
			panel.showAndBlock("Directory for options.ini location");
		}

• Files reviewed: 7/8 changed files
• Comments generated: 4