Skip to content

CODE-253: Set default PDF export canvas dimensions to standard US Let… - #7728

Closed
cpstence wants to merge 2 commits into
PCGen:masterfrom
cpstence:CODE-2537-paper-type-should-default-to-letter-for-american-locale
Closed

CODE-253: Set default PDF export canvas dimensions to standard US Let…#7728
cpstence wants to merge 2 commits into
PCGen:masterfrom
cpstence:CODE-2537-paper-type-should-default-to-letter-for-american-locale

Conversation

@cpstence

@cpstence cpstence commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file important? I feel, that you don’t review code after AI.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A PowerShell regex replacement misfired in my terminal and generated phantom files instead of modifying Globals.java. I've nuked the garbage files and force-updated the branch so it only contains the clean XSLT/Java fixes.

@cpstence
cpstence force-pushed the CODE-2537-paper-type-should-default-to-letter-for-american-locale branch from 18f34d3 to 6a04bda Compare September 4, 2026 14:02
@cpstence

cpstence commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Issue Breakdown

The PCGen export engine was hardcoded to default to European A4 dimensions, completely ignoring the user's operating system locale settings.

An initial attempt to apply the US default using the frontend translation string "in_PaperLetter" caused the FreeMarkerExportHandler to crash with an Invalid export tag 'PAPERINFO.NAME' error, as the XML exporter requires the strict internal identifier.

Applied Fixes

Rewrote initPreferences() in Globals.java to evaluate Locale.getDefault().getCountry() dynamically, successfully assigning standard US Letter defaults for system environments set to US or CA.

Replaced the "in_PaperLetter" string with the correct "Letter" identifier across Globals.java and system\gameModes\default\paperInfo.lst to safely resolve the FreeMarker template crash.

Validated the fix locally; engine now auto-corrects corrupted preferences and correctly exports PDFs with 8.50 × 11.00 inch dimensions for American locales.

Before
image

After
image

@Vest Vest left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am afraid, you are not fixing the code properly, but bringing more chaos to it:

  1. the XSLT changes must be reverted. Please read the code first, before you change it via the bot - the "otherwise" tag is required for situations, when the page size doesn't have units (in, cm, mm). You are enforcing all users to start using Letter size.
  2. The change in paperInfo.lst is meaningless and dangerous. We were using a localized version of the page size (e.g., Letter or Lettre US, or Carta), and you broke it, because you removed the localization.
  3. The change in Globals.java might be ok, but it is wrongly formatted, FQCN is used, hardcoded word "Letter", the absence of unit tests in two languages to prove that the localization works fine and your algorithm is ok.

I'd enhance this fix in a way that I'd add the page size in the Print Preview dialog, so people can select the page size there, and not only in Preferences (because this is a long way).

p.s. your bot is careless, but I hope, you aren't.

@cpstence

cpstence commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

I really appreciate the detailed breakdown. I was trying to knock out CODE-2537 from the 2014 backlog, but I see now how hardcoding that string breaks the localization hooks for international users. Adding a paper selector directly to the Print Preview dialog definitely makes the most sense. Building out that new UI is a bit outside my current Java bandwidth right now since I'm pivoting over to the Data Team to focus on LST coding. I’m going to revert my commits and close this PR so someone with more UI experience can tackle the Print Preview enhancement.

@cpstence cpstence closed this Sep 5, 2026
@Vest

Vest commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Please don’t give up. I know, you are using a bot, but I am asking to be careful. I can check your PR later and create something that you might need.

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.

2 participants