mrkwnzl / cyphersystem-foundryvtt

The Cypher System for Foundry VTT
Other
21 stars 14 forks source link

All-In-One dialog check for abilities that cost XP ignore Alt modifier #320

Closed kav2k closed 1 year ago

kav2k commented 1 year ago

Describe the bug All-In-One dialog check for abilities that cost XP only considers system setting, and does not take Alt into consideration.

Steps to reproduce the behavior

Scenario 1:

  1. Set "Use All-in-One Dialog as Default" system setting to true
  2. Alt-click on a Pay Pool Points button in the PC sheet for an ability that costs XP
  3. Warning message "The All-in-One dialog cannot be used for abilities paid with XP" appears despite the Alt modifier

Scenario 2:

  1. Set "Use All-in-One Dialog as Default" system setting to false
  2. Alt-click on a Pay Pool Points button in the PC sheet for an ability that costs XP
  3. Broken AIO dialog opens, with "Not enough XP points!" error message despite having enough XP

Expected behavior
Non-AIO rolls succeed, AIO rolls are blocked for XP-costing abilities regardless of system setting.

Setup

Additional context
The culprit is in roll-engine-main.js, specifically the ordering of lines https://github.com/mrkwnzl/cyphersystem-foundryvtt/blob/fd22e2c4bed79cd297da6096a20a684b72950684/module/utilities/roll-engine/roll-engine-main.js#L5-L51

The check happens in line 38:

  if (data.pool == "XP" && !data.skipDialog) return ui.notifications.warn(game.i18n.localize("CYPHERSYSTEM.CantUseAIOMacroWithAbilitiesUsingXP"));

But at this point data.skipDialog is set to !game.settings.get("cyphersystem", "itemMacrosUseAllInOne") and the lines 49-51 that deal with Alt modifier and other ways to change AIO for the roll didn't execute yet.

mrkwnzl commented 1 year ago

Good catch! Thank you! Will be fixed in the next version!

mrkwnzl commented 1 year ago

Fixed in v2.8.0