League-of-Foundry-Developers / fvtt-module-lmrtfy

Let Me Roll That For You!
27 stars 51 forks source link

PF2 Skill Rolls Fail #168

Closed timingila closed 1 year ago

timingila commented 1 year ago

In PF2 system, when a player clicks a Skill request, there is no roll and we get the following error in the console. Perception and Saves work fine.

Uncaught (in promise) TypeError: actor.system.data is undefined _makeRoll https://assets.forge-vtt.com/bazaar/modules/lmrtfy/3.0.3/src/roller.js:187 _onSkillCheck https://assets.forge-vtt.com/bazaar/modules/lmrtfy/3.0.3/src/roller.js:400

timingila commented 1 year ago

Trying to learn JS, and the PF2 system, I started poking around.

It looks like this only needs a small change to handle the new data model. I also found an issue with handling DCs, but I'm not sure the solution of using parseInt is the "proper" way.

Here's the diff of my fix vs the main branch code

index 710fc70..ce59b77 100644
--- a/src/requestor.js
+++ b/src/requestor.js
@@ -327,12 +327,11 @@ class LMRTFYRequestor extends FormApplication {
             ui.notifications.warn(game.i18n.localize("LMRTFY.NothingNotification"));
             return;
         }
-
         let dc = undefined;
         if (game.system.id === 'pf2e') {
-            if (Number.isInteger(formData.dc)) {
+            if (Number.isInteger(parseInt(formData.dc))) {
                 dc = {
-                    value: formData.dc,
+                    value: parseInt(formData.dc),
                     visibility: formData.visibility
                 }
             }
diff --git a/src/roller.js b/src/roller.js
index 34f87f2..f6cfdbd 100644
--- a/src/roller.js
+++ b/src/roller.js
@@ -184,7 +184,7 @@ class LMRTFYRoller extends Application {

                         case this.pf2eRollFor.SKILL:
                             // system specific roll handling
-                            const skill = actor.system.data.skills[args[0]];
+                            const skill = actor.skills[args[0]];
                             // roll lore skills only for actors who have them ...
                             if (!skill) continue;

@@ -194,7 +194,7 @@ class LMRTFYRoller extends Application {

                         case this.pf2eRollFor.PERCEPTION:
                             const precOptions = actor.getRollOptions(['all', 'wis-based', 'perception']);
-                            actor.system.data.attributes.perception.roll({ event, precOptions, dc: this.dc });
+                            actor.perception.roll({ event, precOptions, dc: this.dc });
                             break;
                     }
jdip commented 1 year ago

These changes fixed the same issue for me. Thanks!

vtt-lair commented 1 year ago

@timingila would you be able to send a merge request for this? if not, we can possible chat on discord (I'm in the Foundry VTT server) and you can send your file to me and I can check it and merge it into the application.

timingila commented 1 year ago

@vtt-lair I don't see a way for me to create a merge request. I messaged you on Discord. Thank you for your help!

vtt-lair commented 1 year ago

Thanks @timingila, I've added your changes to the code base and will be in the next release.