[Bug]: Export button not displayed in Cypher System

trevorschadt commented 1 year ago

Expected Behavior

Given:

A world is created using the Cypher System;
The "Export Sheet to PDF" module has been activated;
The "Cypher System" mapping included in the settings for the module has been set;

When:

A GM or an owning player opens a character's sheet;

Then:

A link should appear on the title bar of the character sheet that allows the user to export the character's data to PDF.

Current Behavior

Given: as above When: as above Then:

No such button, link, or other access method is displayed on the title bar.

Steps to Reproduce

Launch a world that uses the Cypher System game system.
Activate the "Export Sheet to PDF" module, click the "Save Module Settings" button and select "Yes" on the "Reload Application?" dialog that appears.
In the "Export Sheet to PDF" section of the "Configure Game Settings" dialog and select "cyphersystem" from the "Mapping" drop down. Click "Save Changes."
In the "Actors" tab of the sidebar, create a new Actor.
Observe that no link, button or other methodology to access the Export functionality appears on the title bar.

Context

No response

Version

0.22.4

Foundry VTT Version

11.306

Operating System

Windows

Browser / App

Native Electron App

Game System

cyphersystem

Modules Disabled

[X] I can confirm that disabling all of my other modules doesn't fix this issue.

mrkwnzl commented 1 year ago

I found the error. There’s a check for the right actor types, and it looks for PC, but in the Cypher System, all actor types are lower case, i. e. pc.

Adding "pc" here (or transforming everything to lower case) will solve this issue: https://github.com/arcanistzed/pdf-sheet/blob/3c4dd403c9c72d738f81ea51a0bed2d0e4e2ab54/scripts/pdfsheet.js#L249

As a system agnostic and future-proof way, let the systems define what actor types should be able to export the sheet. For example, having NPCs have that sheet doesn’t really make sense for the Cypher System (NPCs don’t have character sheets), but the Companion actor type might work with different mappings.

gioppoluca commented 1 year ago

In 5e I have recently added the option to have sheets also for NPC. I need to check if there is an API for getting the actor sheet. Will check. For the moment could be a solution for you if I add also lower case "pc" in the next build?

mrkwnzl commented 1 year ago

Yeah, lower case pc will work.

arcanistzed / pdf-sheet