From 062189e315cd8fc7248ad45c962a1ae98e14a82d Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sat, 13 Dec 2025 20:34:36 -0600 Subject: [PATCH] Add crit threshold modifier tests and fix effect application MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add comprehensive Quench tests for crit threshold system: - Skill crit thresholds modified by Active Effects - Attack crit thresholds (melee, ranged, brawl, finesse) - Effect stacking, disabled effects, minimum threshold enforcement - Helper function validation - Fix crit threshold clamping in CharacterData.prepareDerivedData(): - Skills: clamp in _calculateSkillDifficulties() after effects apply - Attacks: new _clampAttackCritThresholds() method - Ensures thresholds stay within 1-20 range after Active Effects - Note: Foundry v13 auto-prepares data after createEmbeddedDocuments, manual prepareData() calls cause double effect application 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- module/data/actor/character.mjs | 20 ++- module/tests/crit-threshold.test.mjs | 182 +++++++++++++++++++++++++++ module/tests/quench-init.mjs | 2 + 3 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 module/tests/crit-threshold.test.mjs diff --git a/module/data/actor/character.mjs b/module/data/actor/character.mjs index dd9e7e2..b1e63d2 100644 --- a/module/data/actor/character.mjs +++ b/module/data/actor/character.mjs @@ -650,8 +650,11 @@ export default class CharacterData extends VagabondActorBase { this.saves.will.difficulty = 20 - (stats.reason.value + stats.presence.value) - this.saves.will.bonus; - // Calculate Skill Difficulties + // Calculate Skill Difficulties (also clamps skill crit thresholds) this._calculateSkillDifficulties(); + + // Clamp attack crit thresholds after Active Effects + this._clampAttackCritThresholds(); } /** @@ -674,6 +677,21 @@ export default class CharacterData extends VagabondActorBase { // Calculate difficulty: 20 - stat (untrained) or 20 - stat×2 (trained) skillData.difficulty = trained ? 20 - statValue * 2 : 20 - statValue; + + // Clamp crit threshold after Active Effects (schema min doesn't apply to effect-modified data) + skillData.critThreshold = Math.max(1, Math.min(20, skillData.critThreshold)); + } + } + + /** + * Clamp attack crit thresholds after Active Effects are applied. + * Schema constraints don't apply to effect-modified data. + * + * @private + */ + _clampAttackCritThresholds() { + for (const attackData of Object.values(this.attacks)) { + attackData.critThreshold = Math.max(1, Math.min(20, attackData.critThreshold)); } } diff --git a/module/tests/crit-threshold.test.mjs b/module/tests/crit-threshold.test.mjs new file mode 100644 index 0000000..f8e0d64 --- /dev/null +++ b/module/tests/crit-threshold.test.mjs @@ -0,0 +1,182 @@ +/** + * Crit Threshold Modifier System Tests + * + * Tests that Active Effects can modify crit thresholds for: + * - Individual skills (system.skills..critThreshold) + * - Attack types (system.attacks..critThreshold) + * + * Use cases: + * - Fighter's Valor: reduces melee crit threshold by 1/2/3 + * - Gunslinger's Deadeye: reduces ranged crit threshold dynamically + * + * @module tests/crit-threshold + */ + +import { createCritReductionEffect, EFFECT_MODES } from "../helpers/effects.mjs"; + +/** + * Register crit threshold tests with Quench + * @param {Quench} quenchRunner - The Quench test runner + */ +export function registerCritThresholdTests(quenchRunner) { + quenchRunner.registerBatch( + "vagabond.critThreshold.skills", + (context) => { + const { describe, it, expect, beforeEach, afterEach } = context; + + describe("Skill Crit Threshold Modifiers", () => { + let actor; + + beforeEach(async () => { + actor = await Actor.create({ + name: "Test Fighter", + type: "character", + }); + }); + + afterEach(async () => { + await actor?.delete(); + }); + + it("should have default crit threshold of 20 for all skills", () => { + expect(actor.system.skills.arcana.critThreshold).to.equal(20); + expect(actor.system.skills.brawl.critThreshold).to.equal(20); + expect(actor.system.skills.finesse.critThreshold).to.equal(20); + }); + + it("should reduce skill crit threshold with Active Effect", async () => { + // Create an effect that reduces brawl crit by 2 (like Fighter's Valor at level 4) + const effectData = createCritReductionEffect("brawl", 2, "Valor (Brawl)"); + await actor.createEmbeddedDocuments("ActiveEffect", [effectData]); + + // Foundry automatically prepares data after embedded document creation + // Crit threshold should now be 18 + expect(actor.system.skills.brawl.critThreshold).to.equal(18); + // Other skills should be unaffected + expect(actor.system.skills.arcana.critThreshold).to.equal(20); + }); + + it("should stack multiple crit reductions on same skill", async () => { + // Create two effects reducing brawl crit + const effect1 = createCritReductionEffect("brawl", 1, "Effect 1"); + const effect2 = createCritReductionEffect("brawl", 2, "Effect 2"); + await actor.createEmbeddedDocuments("ActiveEffect", [effect1, effect2]); + + // Total reduction: 1 + 2 = 3, so threshold is 17 + expect(actor.system.skills.brawl.critThreshold).to.equal(17); + }); + + it("should allow different skills to have different thresholds", async () => { + // Reduce brawl by 2, finesse by 1 + const brawlEffect = createCritReductionEffect("brawl", 2, "Brawl Crit"); + const finesseEffect = createCritReductionEffect("finesse", 1, "Finesse Crit"); + await actor.createEmbeddedDocuments("ActiveEffect", [brawlEffect, finesseEffect]); + + expect(actor.system.skills.brawl.critThreshold).to.equal(18); + expect(actor.system.skills.finesse.critThreshold).to.equal(19); + expect(actor.system.skills.arcana.critThreshold).to.equal(20); + }); + + it("should not reduce crit threshold below 1", async () => { + // Create an absurd reduction + const effectData = createCritReductionEffect("brawl", 25, "Overpowered"); + await actor.createEmbeddedDocuments("ActiveEffect", [effectData]); + + // Crit threshold should be clamped to minimum of 1 + expect(actor.system.skills.brawl.critThreshold).to.be.at.least(1); + }); + + it("should not affect threshold when effect is disabled", async () => { + const effectData = createCritReductionEffect("brawl", 2, "Disabled Effect"); + effectData.disabled = true; + await actor.createEmbeddedDocuments("ActiveEffect", [effectData]); + + // Should remain at default since effect is disabled + expect(actor.system.skills.brawl.critThreshold).to.equal(20); + }); + }); + }, + { displayName: "Vagabond: Skill Crit Thresholds" } + ); + + quenchRunner.registerBatch( + "vagabond.critThreshold.attacks", + (context) => { + const { describe, it, expect, beforeEach, afterEach } = context; + + describe("Attack Crit Threshold Modifiers", () => { + let actor; + + beforeEach(async () => { + actor = await Actor.create({ + name: "Test Fighter", + type: "character", + }); + }); + + afterEach(async () => { + await actor?.delete(); + }); + + it("should have default crit threshold of 20 for all attack types", () => { + expect(actor.system.attacks.melee.critThreshold).to.equal(20); + expect(actor.system.attacks.brawl.critThreshold).to.equal(20); + expect(actor.system.attacks.ranged.critThreshold).to.equal(20); + expect(actor.system.attacks.finesse.critThreshold).to.equal(20); + }); + + it("should reduce attack crit threshold with Active Effect", async () => { + // Create effect reducing melee crit by 1 (Fighter's Valor level 1) + const effectData = createCritReductionEffect("attack.melee", 1, "Valor (Melee)"); + await actor.createEmbeddedDocuments("ActiveEffect", [effectData]); + + expect(actor.system.attacks.melee.critThreshold).to.equal(19); + expect(actor.system.attacks.ranged.critThreshold).to.equal(20); + }); + + it("should reduce ranged crit threshold (Gunslinger style)", async () => { + // Simulate Gunslinger's Deadeye reducing ranged crit + const effectData = createCritReductionEffect("attack.ranged", 3, "Deadeye"); + await actor.createEmbeddedDocuments("ActiveEffect", [effectData]); + + expect(actor.system.attacks.ranged.critThreshold).to.equal(17); + }); + }); + }, + { displayName: "Vagabond: Attack Crit Thresholds" } + ); + + quenchRunner.registerBatch( + "vagabond.critThreshold.effectHelpers", + (context) => { + const { describe, it, expect } = context; + + describe("Crit Effect Helper Functions", () => { + it("should create valid crit reduction effect data", () => { + const effectData = createCritReductionEffect("brawl", 2, "Test Effect"); + + expect(effectData.name).to.equal("Test Effect"); + expect(effectData.changes).to.have.length(1); + expect(effectData.changes[0].key).to.equal("system.skills.brawl.critThreshold"); + expect(effectData.changes[0].value).to.equal("-2"); + expect(effectData.changes[0].mode).to.equal(EFFECT_MODES.ADD); + }); + + it("should create attack crit reduction with correct key", () => { + const effectData = createCritReductionEffect("attack.melee", 1, "Valor"); + + expect(effectData.changes[0].key).to.equal("system.attacks.melee.critThreshold"); + expect(effectData.changes[0].value).to.equal("-1"); + }); + + it("should always use negative value for reduction", () => { + // Even if positive is passed, should be negative + const effectData = createCritReductionEffect("brawl", 3, "Test"); + + expect(effectData.changes[0].value).to.equal("-3"); + }); + }); + }, + { displayName: "Vagabond: Crit Effect Helpers" } + ); +} diff --git a/module/tests/quench-init.mjs b/module/tests/quench-init.mjs index 121d7e6..ef5716f 100644 --- a/module/tests/quench-init.mjs +++ b/module/tests/quench-init.mjs @@ -11,6 +11,7 @@ import { registerActorTests } from "./actor.test.mjs"; import { registerDiceTests } from "./dice.test.mjs"; import { registerSpellTests } from "./spell.test.mjs"; +import { registerCritThresholdTests } from "./crit-threshold.test.mjs"; // import { registerItemTests } from "./item.test.mjs"; // import { registerEffectTests } from "./effects.test.mjs"; @@ -68,6 +69,7 @@ export function registerQuenchTests(quenchRunner) { registerActorTests(quenchRunner); registerDiceTests(quenchRunner); registerSpellTests(quenchRunner); + registerCritThresholdTests(quenchRunner); // registerItemTests(quenchRunner); // registerEffectTests(quenchRunner);