diff --git a/.gitignore b/.gitignore index d047a0e34f4..5f86e38248b 100644 --- a/.gitignore +++ b/.gitignore @@ -119,3 +119,5 @@ jdks/ mods/ code/manifest-tests .DS_Store + +.superset/ diff --git a/AGENTS.md b/AGENTS.md index 631746d751c..fb92a3c4637 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,13 +2,18 @@ This document captures the concrete commands, structure, conventions, and gotchas observed in this codebase so future agents can work productively without guesswork. +> **⚠️ IMPORTANT: Keep this file up to date!** +> Any time LLM-based development (Copilot, ChatGPT, Claude, or any AI coding assistant) is used to make changes to this repository, the developer (or the LLM agent itself) **must** update this file to reflect any relevant changes. This includes but is not limited to: new build commands, dependency changes, structural changes, new CI workflows, convention changes, new source sets, or anything else that would help a future agent work effectively. Treat this file as living documentation that evolves with the codebase. + ## Project Overview - Project: PCGen — Java desktop application to create/manage RPG player characters -- Build tool: Gradle (via wrapper) -- Java toolchain: Java 25 (Temurin) -- UI: JavaFX; headless testing uses TestFX/Monocle +- Build tool: Gradle 9.5.0 (via wrapper `./gradlew`) +- Java toolchain: Java 25 (Eclipse Temurin) +- UI: JavaFX (OpenJFX, version matched to Java 25); headless testing uses TestFX/Monocle - Packaging: jlink (org.beryx.jlink 4.0.0) / jpackage with custom runtimes and native installers +- Current version: defined in `gradle.properties` (e.g. `6.09.08.RC1`) +- Configuration cache: enabled (`org.gradle.configuration-cache=true` in gradle.properties) Key entry point: `pcgen.system.Main` (code/src/java/pcgen/system/Main.java) @@ -16,62 +21,56 @@ Key entry point: `pcgen.system.Main` (code/src/java/pcgen/system/Main.java) - code/ - src/java … main Java sources (pcgen.* packages) - - src/utest … unit tests - - src/test … slow tests - - src/itest … integration tests - - src/testcommon, src/testResources … shared test fixtures - - gradle/ … build logic split across .gradle files + - src/utest … unit tests (fast, run by `test` task) + - src/test … slow tests (run by `slowtest` task) + - src/itest … integration tests (run by `itest` task) + - src/testcommon … shared test utilities (compiled once, consumed by test/itest/slowtest) + - src/testResources … shared test resources + - src/resources … main resources (includes pcgen/system/prop/PCGenProp.properties) + - gradle/ … build logic split across .gradle files (reporting, distribution, autobuild, release, plugins) + - standards/ … checkstyle.xml, ruleset.xml (PMD), spotbugs_ignore.xml - LICENSE - - scripts: pcgen.sh, pcgen.bat (also duplicated in repo root) -- plugins/ … plugin jars and plugin folders used at runtime + - scripts: pcgen.sh, pcgen.bat +- plugins/ … plugin jars built from compiled classes at build time - data/, outputsheets/, system/, preview/, vendordata/, homebrewdata/ … runtime datasets and assets - docs/ … user documentation (Jekyll site, FAQs, listfile docs) - installers/ … installer assets (mac/win/linux), release notes -- PCGen-base/, PCGen-Formula/ … separate Gradle builds for base/formula modules +- PCGen-base/, PCGen-Formula/ … separate Gradle builds for base/formula modules (consumed as Maven dependencies) - Root Gradle files: build.gradle, settings.gradle, gradle.properties, gradlew* -- CI: `.github/workflows/gradle-test.yml`, `gradle-release.yml`, `codeql-analysis.yml` +- CI: `.github/workflows/gradle-test.yml`, `gradle-release.yml`, `gradle-release-manual.yml`, `codeql-analysis.yml` ## Essential Commands -Always use the wrapper (./gradlew). Java 25 is required; Gradle will fetch dependencies and JavaFX modules as part of tasks. - -- List tasks - - ./gradlew tasks -- Build (default) - - ./gradlew build -- Run unit tests (JUnit 6; headless JavaFX) - - ./gradlew test -- Integration tests (defined source set) - - ./gradlew itest -- Slow tests (longer-running suite) - - ./gradlew slowtest -- Data tests - - ./gradlew datatest -- Pathfinder integration tests - - ./gradlew pfinttest -- Additional inttest variants - - ./gradlew inttest sfinttest rsrdinttest srdinttest msrdinttest -- Coverage (Jacoco report in build/reports/jacoco/testCoverage/html) - - ./gradlew testCoverage -- Assemble files for distribution (zips for data/docs/program/libs + runtime) - - ./gradlew buildDist -- Quick dev binary to output/ - - ./gradlew qbuild -- Run the app (ensures JavaFX modules for host platform) - - ./gradlew run -- Create native app image/installer via jpackage - - ./gradlew fullJpackage -- Clean outputs and auxiliary folders (extended) - - ./gradlew clean (also triggers cleanPlugins, cleanOutput, cleanJdks, cleanMods, cleanMasterSheets) +Always use the wrapper (`./gradlew`). Java 25 (Temurin) is required; Gradle will fetch dependencies and JavaFX modules as part of tasks. + +| Task | Description | +|----------------------------------------------------------|----------------------------------------------------------------------------------------------| +| `./gradlew build` | Compile + unit tests (includes `test` task) | +| `./gradlew test` | Unit tests only (JUnit 6 Jupiter + JUnit 4 vintage; headless JavaFX) | +| `./gradlew itest` | Integration tests | +| `./gradlew slowtest` | Slow/long-running tests (depends on itest) | +| `./gradlew datatest` | Data loading/validation tests | +| `./gradlew pfinttest` | Pathfinder integration tests | +| `./gradlew inttest` | All character integration tests | +| `./gradlew sfinttest rsrdinttest srdinttest msrdinttest` | Per-game-mode integration test variants | +| `./gradlew testCoverage` | Jacoco coverage report (build/reports/jacoco/testCoverage/html) | +| `./gradlew allReports` | Checkstyle + PMD + SpotBugs reports | +| `./gradlew buildDist` | Assemble distribution zips (data/docs/program/libs + runtime) | +| `./gradlew qbuild` | Quick dev binary to output/ | +| `./gradlew run` | Run the app (JavaFX modules configured automatically) | +| `./gradlew fullJpackage` | Create native app image/installer via jpackage | +| `./gradlew clean` | Clean all (also triggers cleanPlugins, cleanOutput, cleanJdks, cleanMods, cleanMasterSheets) | +| `./gradlew tasks` | List all available tasks | Notes - jlink/jpackage build only the host platform. The download/extract tasks (downloadJdk, extractJdk, downloadJfxMods, extractJfxMods) target the host OS/arch automatically; the host SDK helper for local dev is downloadJavaFXLocal/extractJavaFXLocal. CI caches build/jre and build/libs. - Runtime bundles expect assets in data/, system/, outputsheets/, preview/, vendordata/, homebrewdata/. +- Test tasks use parallel forks: `Runtime.runtime.availableProcessors().intdiv(2) ?: 1` for test/itest; `forkEvery = 1` for slowtest/inttest variants. ## Running From Source - Entry point: application plugin sets main class to `pcgen.system.Main`. -- ./gradlew run sets module-path and add-modules for JavaFX automatically and enables preview features. +- `./gradlew run` sets module-path and add-modules for JavaFX automatically. Command-line flags (see code/src/java/pcgen/system/CommandLineArguments.java; tests in code/src/utest/pcgen/system/CommandLineArgumentsTest.java): - --verbose | -v (counted, interpreted as boolean) @@ -90,54 +89,72 @@ Batch export path exists in Main.startupWithoutGUI(). Tests demonstrate usage in ## Testing Approach -- JUnit 6 with Jupiter, xmlunit for XML comparisons, TestFX for JavaFX UI components. +- JUnit 6 (BOM 6.0.3) with Jupiter, JUnit 4 legacy tests via vintage engine (~870 tests still use JUnit 4). +- xmlunit 2.11.0 for XML comparisons, TestFX 4.0.18 + Monocle 21.0.2 for JavaFX UI components. - Source sets: - - test: code/src/utest + testcommon - - itest: code/src/itest + testcommon - - slowtest: code/src/test + testcommon -- CI runs: build, then test itest datatest slowtest, then coverage. -- Headless settings applied to Test tasks: - - -Dtestfx.robot=glass, -Dtestfx.headless=true - - JavaFX module-path and add-modules configured - - Assertions enabled; 1024m heap; maxParallelForks=1; forkEvery used for slow/int tests + - test: code/src/utest + testcommon (fast unit tests) + - itest: code/src/itest + testcommon (integration tests) + - slowtest: code/src/test + testcommon (slow/character integration tests) +- CI runs: `build` → `itest datatest slowtest` → `testCoverage` (coverage report posted to PR). +- Headless settings applied to all Test tasks: + - `-Djava.awt.headless=true`, `-Djavafx.macosx.embedded=true` + - `--module-path mods/lib --add-modules javafx.controls,javafx.web,javafx.swing,javafx.fxml,javafx.graphics` + - `--enable-native-access=javafx.graphics` + - Assertions enabled; 1024m heap; maxParallelForks=1 (global default, overridden for test/itest) +- Unit test task additionally sets: `-Dtestfx.robot=glass`, `-Dtestfx.headless=true`, `-Dprism.order=sw` ## Code Quality and Style -- Checkstyle config: code/standards/checkstyle.xml (enforced via reporting.gradle; toolVersion 13.2.0). Newline at EOF; 201 char line length; prohibits `System.exit` (use pcgen.util.GracefulExit.exit). -- PMD: ruleset at code/standards/ruleset.xml (referenced from reporting.gradle); toolVersion 7.21.0; dependencies pmd-java 7.24.0 and pmd-ant 7.24.0. -- SpotBugs: plugin 6.5.4; toolVersion 4.9.8; exclude filter code/standards/spotbugs_ignore.xml; ignoreFailures true; extra findsecbugs plugin. -- Aggregate quality task: ./gradlew allReports +- **Checkstyle**: config at code/standards/checkstyle.xml; toolVersion 13.2.0. Newline at EOF; 201 char line length; prohibits `System.exit` (use `pcgen.util.GracefulExit.exit`). +- **PMD**: ruleset at code/standards/ruleset.xml; toolVersion 7.21.0; dependencies pmd-java 7.24.0 and pmd-ant 7.24.0. Incremental analysis enabled; ignoreFailures true. +- **SpotBugs**: plugin 6.5.4; toolVersion 4.9.8; exclude filter code/standards/spotbugs_ignore.xml; omitVisitors: Naming, CrossSiteScripting, DontUseEnum, DoInsideDoPrivileged; ignoreFailures true; extra findsecbugs plugin 1.14.0. +- **Jacoco**: toolVersion 0.8.14. +- Aggregate quality task: `./gradlew allReports` (runs checkstyleMain, pmdMain, spotbugsMain) -Conventions/gotchas observed -- Use `GracefulExit.exit` instead of `System.exit` (Checkstyle enforces via RegexpMultiline) -- Java version and JavaFX are tightly coupled to project.ext.javaVersion (25). Tests and run tasks add the needed JavaFX modules explicitly. -- Source sets are nonstandard (itest, slowtest); when adding new tests, place them in the correct source set to be picked up by the corresponding Gradle task. -- Plugins are built from compiled classes into plugin jars via tasks in code/gradle/plugins.gradle; main jar depends on jarAllPlugins. -- Some ivy/maven repos are over HTTP (allowInsecureProtocol true). Do not change without coordinating with maintainers. +Conventions/gotchas observed: +- Use `GracefulExit.exit` instead of `System.exit` (Checkstyle enforces via RegexpMultiline). +- Java version and JavaFX are tightly coupled to `project.ext.javaVersion` (25). Tests and run tasks add the needed JavaFX modules explicitly. +- Source sets are nonstandard (itest, slowtest, testcommon); when adding new tests, place them in the correct source set to be picked up by the corresponding Gradle task. +- Plugins are built from compiled classes into plugin jars via tasks in code/gradle/plugins.gradle; main jar depends on `jarAllPlugins`. +- Some ivy/maven repos are over HTTP (`allowInsecureProtocol true`). Do not change without coordinating with maintainers. +- Gradle configuration cache is enabled — tasks that are not compatible should declare `notCompatibleWithConfigurationCache(...)`. ## Build/Release Flow -- For releases, see `code/gradle/release.gradle` and CI workflow `.github/workflows/gradle-release.yml`. +- For releases, see `code/gradle/release.gradle` and CI workflows: + - `.github/workflows/gradle-release.yml` — triggered on tag push `v*.*.*` or manual dispatch with existing tag + - `.github/workflows/gradle-release-manual.yml` — manual dispatch that creates a new tag on the selected branch +- Both release workflows validate that the tag matches `gradle.properties` version. - Release tasks: - - `prepareRelease` (build; version handling is done by helper groovy script applied as `releaseUtils.groovy`) - - `pcgenRelease` (assemble artifacts, checksum) + - `prepareRelease` (build; version handling via `releaseUtils.groovy`) + - `pcgenRelease` (prepareRelease + assembleArtifacts + checksum) - `pcgenReleaseOfficial` (pcgenRelease + updateVersionRelease) - Artifacts collected into build/release; jpackage produces platform installers under build/jpackage. +- Release CI builds on: ubuntu-latest (x64), ubuntu-24.04-arm, macos-latest, windows-latest. +- CI updates `PCGenProp.properties` with version number and release date at build time. ## Data and Outputsheets - Distribution zips combine program, data, docs, libs; copy specs in code/gradle/distribution.gradle control inclusion/exclusion. -- copyMasterSheets/cleanMasterSheets manage templated outputsheets across genre folders. +- copyMasterSheets/cleanMasterSheets manage templated outputsheets across genre folders (historical, horror, sciencefiction, western). ## CI Details -- gradle-test.yml - - Java 25 (temurin) - - ./gradlew build - - ./gradlew test itest datatest slowtest - - ./gradlew testCoverage and publish report -- codeql-analysis.yml analyzes Java -- gradle-release.yml builds jpackage artifacts for macOS, Windows, Ubuntu (x64) and Ubuntu ARM on tag pushes `v*.*.*` +- **gradle-test.yml** (runs on pull_request): + - Java 25 (temurin), Gradle cache via `gradle/actions/setup-gradle@v4` + - `./gradlew build` + - `./gradlew itest datatest slowtest` + - `./gradlew testCoverage` + - Publishes test results via `EnricoMi/publish-unit-test-result-action@v2` + - Coverage report uploaded as artifact and posted to PR via `madrapps/jacoco-report@v1.7.2` +- **codeql-analysis.yml** (push/PR to master + weekly schedule): + - Analyzes Java via CodeQL autobuild +- **gradle-release.yml** (tag push `v*.*.*` or workflow_dispatch with existing tag): + - Validates tag vs gradle.properties version + - Creates GitHub release, builds on 4 platforms, attaches artifacts +- **gradle-release-manual.yml** (workflow_dispatch — creates tag then releases): + - Creates and pushes a new tag on the current branch + - Updates gradle.properties to match, then delegates to same build matrix ## How to Add/Modify Code @@ -146,42 +163,71 @@ Conventions/gotchas observed - Fast unit tests → code/src/utest - Integration tests → code/src/itest - Slow/long-running → code/src/test -- Run: ./gradlew test itest slowtest locally; fix failing tests before PRs. -- Follow style checks; do not use `System.exit` directly. +- Run: `./gradlew test itest slowtest` locally; fix failing tests before PRs. +- Follow style checks; do not use `System.exit` directly — use `pcgen.util.GracefulExit.exit`. +- Run `./gradlew allReports` to check Checkstyle/PMD/SpotBugs before submitting. +- JavaFX/FXML dialogs: + - For **standalone Stage dialogs** (loaded from FXML and shown as a top-level window via `showAsStage(title)` or modally via `showAndBlock(title)`), use `pcgen.gui3.PanelFromResource`. Both methods are thread-safe (`showAsStage` auto-dispatches to the FX thread; `showAndBlock` must be called off the FX thread and blocks the caller until the dialog closes). + - For **embedding FXML scenes inside Swing** containers, use `pcgen.gui3.JFXPanelFromResource` (it extends `JFXPanel`). Do **not** re-use it for standalone dialogs — re-parenting its embedded `Scene` onto a real `Stage` corrupts the quantum peer and triggers an NPE in `com.sun.javafx.tk.quantum.GlassScene#updateSceneState` on macOS HiDPI displays. ## Useful Paths -- Main: code/src/java/pcgen/system/Main.java -- CLI parsing: code/src/java/pcgen/system/CommandLineArguments.java -- CLI tests: code/src/utest/pcgen/system/CommandLineArgumentsTest.java -- FTL export test base: code/src/test/pcgen/inttest/PcgenFtlTestCase.java +| Purpose | Path | +|---------------------------------|-------------------------------------------------------------| +| Main entry point | code/src/java/pcgen/system/Main.java | +| CLI parsing | code/src/java/pcgen/system/CommandLineArguments.java | +| CLI tests | code/src/utest/pcgen/system/CommandLineArgumentsTest.java | +| GracefulExit | code/src/java/pcgen/util/GracefulExit.java | +| FTL export test base | code/src/test/pcgen/inttest/PcgenFtlTestCase.java | +| Standalone Stage dialogs (FXML) | code/src/java/pcgen/gui3/PanelFromResource.java | +| Swing-embedded FXML panels | code/src/java/pcgen/gui3/JFXPanelFromResource.java | +| Thread assertions for UI code | code/src/java/pcgen/gui3/GuiAssertions.java | +| Build config | build.gradle | +| Version | gradle.properties | +| Checkstyle rules | code/standards/checkstyle.xml | +| PMD ruleset | code/standards/ruleset.xml | +| SpotBugs exclusions | code/standards/spotbugs_ignore.xml | +| Release logic | code/gradle/release.gradle, code/gradle/releaseUtils.groovy | +| Distribution logic | code/gradle/distribution.gradle | +| Plugin jar building | code/gradle/plugins.gradle | +| Reporting (quality) | code/gradle/reporting.gradle | +| CI test workflow | .github/workflows/gradle-test.yml | +| CI release workflow | .github/workflows/gradle-release.yml | ## Local Run Examples - Launch GUI: - - ./gradlew run + - `./gradlew run` - Run name generator directly: - - ./gradlew run --args="--name-generator" + - `./gradlew run --args="--name-generator"` - Export a character to XML (batch mode example from tests): - - java -jar build/libs/pcgen-.jar --character /path/to/char.pcg --exportsheet code/testsuite/base-xml.ftl --outputfile /tmp/char.xml --configfilename config.ini.junit - - Or via Main in Gradle: ./gradlew run --args="--character ... --exportsheet ... --outputfile ... --configfilename ..." + - `java -jar build/libs/pcgen-.jar --character /path/to/char.pcg --exportsheet code/testsuite/base-xml.ftl --outputfile /tmp/char.xml --configfilename config.ini.junit` + - Or via Gradle: `./gradlew run --args="--character ... --exportsheet ... --outputfile ... --configfilename ..."` ## Security/Defensive Notes -- CodeQL workflow is enabled. -- SpotBugs runs with findsecbugs; some visitors are omitted; failures ignored in reporting task (manual review advised). +- CodeQL workflow is enabled (runs on push/PR to master + weekly schedule). +- SpotBugs runs with findsecbugs 1.14.0; some visitors are omitted; failures ignored in reporting task (manual review advised). - Avoid introducing insecure HTTP endpoints unless required by existing build constraints noted in build.gradle. +- Java security manager is disallowed in tests (`-Djava.security.manager=disallow`). -## Project-Specific “Don’t Break” Items +## Project-Specific "Don't Break" Items - Java version and JavaFX module handling are intertwined across build.gradle and distribution tasks — changing one often requires adjusting tasks (run, test, JavaCompile, runtime/jpackage) and CI. - The distribution relies on file layout in data/, outputsheets/, system/, preview/ — deletions or renames will break runtime validation in Main.validateEnvironment(). - GracefulExit should be used for controlled termination (tests hook the exit function). -- Module compilation: PCGen-base and PCGen-Formula jars are placed on `--module-path` while all other dependencies are merged into the pcgen module via `--patch-module`. This means **no source file in the pcgen module may share a package with classes in PCGen-base or PCGen-Formula jars** (Java forbids split packages across modules). Currently conflicting packages (`pcgen.base.util`, `pcgen.base.format`) have been relocated to `pcgen.util` and `pcgen.format` respectively. If adding new classes whose package exists in either jar, place them in a non-overlapping package. +- Module compilation: PCGen-base and PCGen-Formula jars are placed on `--module-path` while all other dependencies are merged into the pcgen module via `--patch-module` (jlink `forceMerge`). This means **no source file in the pcgen module may share a package with classes in PCGen-base or PCGen-Formula jars** (Java forbids split packages across modules). Currently conflicting packages (`pcgen.base.util`, `pcgen.base.format`) have been relocated to `pcgen.util` and `pcgen.format` respectively. If adding new classes whose package exists in either jar, place them in a non-overlapping package. +- Gradle configuration cache is enabled — ensure new tasks are compatible or explicitly opt out. +- The `testcommon` source set extends test configurations — changes to test dependencies are automatically available there. +- Release tag must match `gradle.properties` version exactly (CI validates this). +- Never re-attach a `Scene` loaded into a `JFXPanel` onto a standalone `Stage`. The embedded scene peer stays bound to the JFXPanel's host and the orphaned `EmbeddedScene` will eventually fire `setPixelScaleFactors` against a null `sceneState`, throwing an NPE in `GlassScene#updateSceneState` on macOS HiDPI displays. Use `PanelFromResource` for top-level dialogs and reserve `JFXPanelFromResource` for Swing embedding only. ## Maintainer/Issue Tracking Context - Primary docs: README.md (development setup, essential Gradle tasks) - Issue tracker: Jira at https://pcgenorg.atlassian.net (CODE/DATA/etc.) as referenced in README and docs/faqpages/faqsubmittingabugreport.md +- Community: Discord (https://discord.gg/M7GH5BS), Slack (by invitation) + +--- -This file documents only observed behavior and commands present in this repository as of the current state. +_This file documents observed behavior and commands present in this repository as of the current state. It should be updated whenever significant changes are made — especially during LLM-assisted development sessions._ diff --git a/code/src/java/pcgen/gui2/PCGenActionMap.java b/code/src/java/pcgen/gui2/PCGenActionMap.java index 4f28d28175e..fbcdde674aa 100644 --- a/code/src/java/pcgen/gui2/PCGenActionMap.java +++ b/code/src/java/pcgen/gui2/PCGenActionMap.java @@ -40,7 +40,7 @@ import pcgen.gui2.tools.Icons; import pcgen.gui2.tools.PCGenAction; import pcgen.gui3.GuiAssertions; -import pcgen.gui3.JFXPanelFromResource; +import pcgen.gui3.PanelFromResource; import pcgen.gui3.dialog.CalculatorDialogController; import pcgen.gui3.dialog.DebugDialog; import pcgen.gui3.dialog.ExportDialogController; @@ -258,8 +258,6 @@ public void actionPerformed(ActionEvent e) private static final class CalculatorAction extends PCGenAction { - private JFXPanelFromResource dialog; - private CalculatorAction() { super("mnuToolsCalculator", CALCULATOR_COMMAND, "F11"); @@ -268,10 +266,7 @@ private CalculatorAction() @Override public void actionPerformed(ActionEvent e) { - if (dialog == null) - { - dialog = new JFXPanelFromResource<>(CalculatorDialogController.class, "CalculatorDialog.fxml"); - } + var dialog = new PanelFromResource<>(CalculatorDialogController.class, "CalculatorDialog.fxml"); dialog.showAsStage(LanguageBundle.getString("mnuToolsCalculator")); } } @@ -656,9 +651,9 @@ private ExportAction() public void actionPerformed(ActionEvent e) { GuiAssertions.assertIsNotJavaFXThread(); - JFXPanelFromResource - jfxPanelFromResource = new JFXPanelFromResource(ExportDialogController.class, "ExportDialog.fxml"); - jfxPanelFromResource.showAsStage("Export a PC or Party"); + PanelFromResource panel = + new PanelFromResource<>(ExportDialogController.class, "ExportDialog.fxml"); + panel.showAsStage("Export a PC or Party"); } } diff --git a/code/src/java/pcgen/gui2/prefs/PurchaseModeFrame.java b/code/src/java/pcgen/gui2/prefs/PurchaseModeFrame.java index 280a86683a2..cd64c476af4 100644 --- a/code/src/java/pcgen/gui2/prefs/PurchaseModeFrame.java +++ b/code/src/java/pcgen/gui2/prefs/PurchaseModeFrame.java @@ -58,7 +58,7 @@ import pcgen.core.utils.MessageType; import pcgen.core.utils.ShowMessageDelegate; import pcgen.gui3.GuiUtility; -import pcgen.gui3.JFXPanelFromResource; +import pcgen.gui3.PanelFromResource; import pcgen.gui3.component.OKCloseButtonBar; import pcgen.gui3.dialog.NewPurchaseMethodDialogController; import pcgen.rules.context.AbstractReferenceContext; @@ -102,7 +102,7 @@ public final class PurchaseModeFrame extends JDialog // private void addMethodButtonActionPerformed() { - var npmd = new JFXPanelFromResource<>( + var npmd = new PanelFromResource<>( NewPurchaseMethodDialogController.class, "NewPurchaseMethodDialog.fxml" ); diff --git a/code/src/java/pcgen/gui3/JFXPanelFromResource.java b/code/src/java/pcgen/gui3/JFXPanelFromResource.java index ad2391afa56..9f6a1d86011 100644 --- a/code/src/java/pcgen/gui3/JFXPanelFromResource.java +++ b/code/src/java/pcgen/gui3/JFXPanelFromResource.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.net.URL; import java.util.Objects; -import java.util.concurrent.CompletableFuture; import pcgen.system.LanguageBundle; import pcgen.util.Logging; @@ -30,10 +29,17 @@ import javafx.embed.swing.JFXPanel; import javafx.fxml.FXMLLoader; import javafx.scene.Scene; -import javafx.stage.Stage; /** - * Displays HTML content as a "panel". + * Embeds an FXML-defined JavaFX scene inside a Swing {@link JFXPanel}. + * + *

This class is only suitable for the JFX-in-Swing embedding use case (i.e. + * the panel will be added to a Swing container). To open the FXML resource + * as a top-level JavaFX {@link javafx.stage.Stage} dialog, use + * {@link PanelFromResource} instead — re-parenting an embedded scene onto a + * standalone {@code Stage} corrupts the scene's quantum peer and triggers an + * NPE in {@code com.sun.javafx.tk.quantum.GlassScene#updateSceneState} on + * macOS HiDPI displays. * * @param The class of the controller */ @@ -84,31 +90,4 @@ public T getControllerFromJavaFXThread() GuiAssertions.assertIsJavaFXThread(); return fxmlLoader.getController(); } - - public void showAsStage(String title) - { - Platform.runLater(() -> { - Stage stage = new Stage(); - stage.setTitle(title); - stage.setScene(getScene()); - stage.sizeToScene(); - stage.show(); - }); - } - - public void showAndBlock(String title) - { - GuiAssertions.assertIsNotJavaFXThread(); - CompletableFuture lock = new CompletableFuture<>(); - Platform.runLater(() -> { - Stage stage = new Stage(); - stage.setTitle(title); - stage.setScene(getScene()); - stage.sizeToScene(); - stage.showAndWait(); - lock.completeAsync(() -> 0); - - }); - lock.join(); - } } diff --git a/code/src/java/pcgen/gui3/PanelFromResource.java b/code/src/java/pcgen/gui3/PanelFromResource.java index be8ca562757..b7a1dc28a0b 100644 --- a/code/src/java/pcgen/gui3/PanelFromResource.java +++ b/code/src/java/pcgen/gui3/PanelFromResource.java @@ -1,11 +1,13 @@ package pcgen.gui3; +import javafx.application.Platform; import javafx.fxml.FXMLLoader; import javafx.scene.Scene; import javafx.stage.Stage; import pcgen.system.LanguageBundle; import java.text.MessageFormat; +import java.util.concurrent.CompletableFuture; import java.util.logging.Level; import java.util.logging.Logger; @@ -18,6 +20,13 @@ * This class provides functionality to load FXML files, assign their controllers, * and display the loaded scenes in a new JavaFX stage. * + *

Unlike {@link JFXPanelFromResource}, this class does not extend + * {@code JFXPanel}, so the loaded {@link Scene} is never wrapped in an embedded + * scene peer. This avoids the {@code EmbeddedScene.sceneState} NPE that occurs + * on macOS HiDPI displays when an embedded scene is later re-parented onto a + * top-level {@link Stage}. Use this class for any dialog that is shown via + * {@link #showAsStage(String)} or {@link #showAndBlock(String)}. + * * @param The type of the controller associated with the FXML resource. */ public class PanelFromResource implements Controllable @@ -47,33 +56,52 @@ public PanelFromResource(Class klass, String resourceName) /** * Retrieves the controller associated with the loaded FXML resource. + * The controller is only available after one of the {@code show*} methods has + * been invoked (which is when the FXML file is actually loaded). + * + *

This method may be called from any thread; if invoked off the JavaFX + * application thread it will bounce to it and wait for the result. * - * @return The controller instance. - * @throws IllegalStateException if this method is called outside the JavaFX application thread. + * @return The controller instance, or {@code null} if no FXML has been loaded yet. */ @Override public T getController() { - GuiAssertions.assertIsJavaFXThread(); - return fxmlLoader.getController(); + if (Platform.isFxApplicationThread()) + { + return fxmlLoader.getController(); + } + return GuiUtility.runOnJavaFXThreadNow(fxmlLoader::getController); } /** - * Displays the loaded FXML resource as a new JavaFX stage. + * Displays the loaded FXML resource as a new non-modal JavaFX stage. + * + *

This method may be called from any thread. If invoked off the JavaFX + * application thread, the stage creation and display are dispatched there + * via {@link Platform#runLater(Runnable)} and this call returns immediately. * * @param title The title of the stage to be displayed. - * @throws IllegalStateException if this method is called outside the JavaFX application thread. */ public void showAsStage(String title) { - GuiAssertions.assertIsJavaFXThread(); + if (Platform.isFxApplicationThread()) + { + showAsStageOnFxThread(title); + } + else + { + ensureToolkitInitialized(); + Platform.runLater(() -> showAsStageOnFxThread(title)); + } + } + private void showAsStageOnFxThread(String title) + { try { - // Load the scene from the FXML resource. Scene scene = fxmlLoader.load(); - // Create and configure a new stage. Stage stage = new Stage(); stage.setTitle(title); stage.setScene(scene); @@ -86,4 +114,108 @@ public void showAsStage(String title) e); } } + + /** + * Displays the loaded FXML resource as a JavaFX stage, blocking the + * calling thread until the user closes the dialog. + * + *

Must not be called from the JavaFX application thread; + * doing so would deadlock because {@code Stage.showAndWait()} requires the + * FX thread to remain available to pump events. + * + * @param title The title of the stage to be displayed. + * @throws RuntimeException if this method is called on the JavaFX application thread. + */ + public void showAndBlock(String title) + { + GuiAssertions.assertIsNotJavaFXThread(); + ensureToolkitInitialized(); + CompletableFuture lock = new CompletableFuture<>(); + Platform.runLater(() -> { + try + { + Scene scene = fxmlLoader.load(); + + Stage stage = new Stage(); + stage.setTitle(title); + stage.setScene(scene); + stage.sizeToScene(); + stage.showAndWait(); + } catch (IOException e) + { + LOG.log(Level.SEVERE, + MessageFormat.format("Failed to load stream fxml from location {0}", fxmlLoader.getLocation()), + e); + } finally + { + lock.complete(0); + } + }); + lock.join(); + } + + /** + * Ensures the JavaFX toolkit has been initialized before this class + * schedules any work on the FX application thread. + * + *

Why this exists. Historically the OptionsPathDialog + * (and a handful of other early dialogs) were opened via + * {@link JFXPanelFromResource}, which extends + * {@link javafx.embed.swing.JFXPanel}. Constructing a {@code JFXPanel} is + * one of the three documented ways to bootstrap the JavaFX runtime — it + * implicitly initialises the toolkit (and sets implicit-exit to + * {@code false}) as a side effect, so callers never had to think about + * lifecycle. + * + *

When {@code PanelFromResource} was introduced — to avoid the macOS + * HiDPI {@code com.sun.javafx.tk.quantum.GlassScene#updateSceneState} NPE + * caused by re-parenting a {@code JFXPanel}'s embedded {@link Scene} onto + * a top-level {@link Stage} — it intentionally stopped extending + * {@code JFXPanel}. That deliberate change removed the implicit toolkit + * bootstrap, which in turn exposed a latent ordering bug in + * {@link pcgen.system.Main#startupWithGUI()}: {@code loadProperties(true)} + * runs before {@code new JFXPanel()} on that path. On a fresh + * installation with no {@code settings.files.path} and no {@code -s} CLI + * argument, {@code loadProperties} needs to show the OptionsPathDialog, + * and the first call into {@link Platform#runLater(Runnable)} would throw + * {@code IllegalStateException: Toolkit not initialized}. The JVM would + * log SEVERE and exit before the dialog could be displayed, so the + * application could never start fresh. + * + *

This helper makes {@code PanelFromResource} self-sufficient: if the + * toolkit has not yet been started we start it; if it already has, the + * {@link IllegalStateException} from {@link Platform#startup(Runnable)} is + * swallowed — that exception is the documented signal that initialisation + * has already happened (e.g. from a prior {@code PanelFromResource} use, + * a {@code JFXPanel} construction, or an explicit {@code Platform.startup} + * elsewhere). {@link Platform#setImplicitExit(boolean)} is then forced to + * {@code false} so that closing this dialog does not tear down the toolkit + * before the rest of the GUI gets a chance to bring up its own Stages — + * matching the previous {@code JFXPanel} behaviour and preventing a + * regression where dismissing an early dialog would shut JavaFX down. + * + *

{@link Platform#startup(Runnable)} is internally guarded by an atomic + * flag in {@code com.sun.javafx.application.PlatformImpl}, so concurrent + * callers race safely: exactly one wins and starts the toolkit, the rest + * see {@code IllegalStateException} and fall through. No additional + * synchronisation is required here. + */ + private static void ensureToolkitInitialized() + { + try + { + Platform.startup(() -> { + // The toolkit is considered initialised the moment this Runnable + // is queued onto the FX application thread; we have no work to + // perform here ourselves. + }); + } + catch (IllegalStateException alreadyStarted) + { + LOG.log(Level.FINEST, + "JavaFX toolkit was already initialised before PanelFromResource bootstrap; continuing.", + alreadyStarted); + } + Platform.setImplicitExit(false); + } } diff --git a/code/src/java/pcgen/gui3/component/PCGenToolBar.java b/code/src/java/pcgen/gui3/component/PCGenToolBar.java index 9754d6b95ad..f90c313bb39 100644 --- a/code/src/java/pcgen/gui3/component/PCGenToolBar.java +++ b/code/src/java/pcgen/gui3/component/PCGenToolBar.java @@ -26,7 +26,7 @@ import pcgen.gui2.PCGenUIManager; import pcgen.gui2.dialog.PrintPreviewDialog; import pcgen.gui2.tools.Icons; -import pcgen.gui3.JFXPanelFromResource; +import pcgen.gui3.PanelFromResource; import pcgen.gui3.behavior.EnabledOnlyWithCharacter; import pcgen.gui3.behavior.EnabledOnlyWithSources; import pcgen.gui3.dialog.ExportDialogController; @@ -136,10 +136,9 @@ private void onPrint(final ActionEvent actionEvent) private void onExport(final ActionEvent actionEvent) { - //GuiAssertions.assertIsNotJavaFXThread(); - JFXPanelFromResource - jfxPanelFromResource = new JFXPanelFromResource<>(ExportDialogController.class, "ExportDialog.fxml"); - jfxPanelFromResource.showAsStage("Export a PC or Party"); + PanelFromResource panel = + new PanelFromResource<>(ExportDialogController.class, "ExportDialog.fxml"); + panel.showAsStage("Export a PC or Party"); } private void onPreferences(final ActionEvent actionEvent) diff --git a/code/src/java/pcgen/system/Main.java b/code/src/java/pcgen/system/Main.java index 272dba791c6..1afef38363a 100644 --- a/code/src/java/pcgen/system/Main.java +++ b/code/src/java/pcgen/system/Main.java @@ -40,7 +40,7 @@ import pcgen.gui2.PCGenUIManager; import pcgen.gui2.UIPropertyContext; import pcgen.gui2.converter.TokenConverter; -import pcgen.gui3.JFXPanelFromResource; +import pcgen.gui3.PanelFromResource; import pcgen.gui3.namegen.RandomNameDialog; import pcgen.gui3.dialog.OptionsPathDialogController; import pcgen.gui3.preloader.PCGenPreloader; @@ -258,7 +258,7 @@ public static void loadProperties(boolean useGui) Logging.errorPrint("No settingsDir specified via -s in batch mode and no default exists."); GracefulExit.exit(1); } - var panel = new JFXPanelFromResource<>( + var panel = new PanelFromResource<>( OptionsPathDialogController.class, "OptionsPathDialog.fxml" );