From aae1cd9b35886f1d65ed149dc2b48cfd97545531 Mon Sep 17 00:00:00 2001 From: luther-rotmg Date: Fri, 19 Jun 2026 16:20:41 -0700 Subject: [PATCH 1/2] Support "+X% to Fire Spell Critical Hit Chance" affix The "of Xoph" suffix ("+X% to Fire Spell Critical Hit Chance", mod group FireSpellBaseCriticalChance) had no modNameList entry, so the "fire spell" qualifier was left unparsed and the affix did nothing. Add a modNameList mapping to CritChance (BASE) restricted to fire spells via ModFlag.Spell + KeywordFlag.Fire, matching the codebase's existing "fire spells" convention in preFlagList. Add a parse regression test. Closes #2226 --- spec/System/TestItemParse_spec.lua | 15 +++++++++++++++ src/Modules/ModParser.lua | 1 + 2 files changed, 16 insertions(+) diff --git a/spec/System/TestItemParse_spec.lua b/spec/System/TestItemParse_spec.lua index fe48536c59..2e1d08e12b 100644 --- a/spec/System/TestItemParse_spec.lua +++ b/spec/System/TestItemParse_spec.lua @@ -73,6 +73,21 @@ describe("TestItemParse", function() assert.are.equals(12, item.quality) end) + it("parses '+X% to Fire Spell Critical Hit Chance' (issue #2226)", function() + local item = new("Item", [[ + Rarity: Rare + Xoph's Test Band + Amethyst Ring + Implicits: 0 + +5% to Fire Spell Critical Hit Chance + ]]) + -- grants base critical hit chance to fire spells specifically + assert.are.equals(5, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, "CritChance")) + -- must NOT apply to attacks, nor to non-fire spells + assert.are.equals(0, item.baseModList:Sum("BASE", { flags = ModFlag.Attack, keywordFlags = KeywordFlag.Fire }, "CritChance")) + assert.are.equals(0, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Cold }, "CritChance")) + end) + --TODO: impl sockets for POB2 --it("Sockets", function() --end) diff --git a/src/Modules/ModParser.lua b/src/Modules/ModParser.lua index 3dd2c16b06..8481a51b6f 100644 --- a/src/Modules/ModParser.lua +++ b/src/Modules/ModParser.lua @@ -755,6 +755,7 @@ local modNameList = { ["critical hit chance"] = "CritChance", ["attack critical hit chance"] = { "CritChance", flags = ModFlag.Attack }, ["thorns critical hit chance"] = { "CritChance", flags = ModFlag.Thorns }, + ["fire spell critical hit chance"] = { "CritChance", flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, ["critical damage bonus"] = "CritMultiplier", ["attack critical damage bonus"] = { "CritMultiplier", flags = ModFlag.Attack }, ["critical spell damage bonus"] = { "CritMultiplier", flags = ModFlag.Spell }, From a6fc4bb2a024b8542ffda438e8d5c4802d265f43 Mon Sep 17 00:00:00 2001 From: luther-rotmg Date: Sun, 21 Jun 2026 09:40:34 -0700 Subject: [PATCH 2/2] Address review: composable " spell" tags instead of one stat entry Per review feedback: rather than a single modNameList entry for "fire spell critical hit chance", add fire/cold/lightning/chaos spell to modFlagList's Skill types section. These compose with any stat (critical hit chance, critical damage bonus, ...) through the normal modName parse, and future-proof the other elements. Update the regression test to assert the tag composes with both CritChance and CritMultiplier and works per element. --- spec/System/TestItemParse_spec.lua | 13 +++++++++---- src/Modules/ModParser.lua | 5 ++++- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/spec/System/TestItemParse_spec.lua b/spec/System/TestItemParse_spec.lua index 2e1d08e12b..8654ce88d4 100644 --- a/spec/System/TestItemParse_spec.lua +++ b/spec/System/TestItemParse_spec.lua @@ -73,19 +73,24 @@ describe("TestItemParse", function() assert.are.equals(12, item.quality) end) - it("parses '+X% to Fire Spell Critical Hit Chance' (issue #2226)", function() + it("parses ' spell' as a composable Spell + element tag (issue #2226)", function() local item = new("Item", [[ Rarity: Rare Xoph's Test Band Amethyst Ring Implicits: 0 +5% to Fire Spell Critical Hit Chance + +30% to Fire Spell Critical Damage Bonus + +7% to Cold Spell Critical Hit Chance ]]) - -- grants base critical hit chance to fire spells specifically + -- the "fire spell" tag composes with any crit stat (chance and damage bonus) assert.are.equals(5, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, "CritChance")) - -- must NOT apply to attacks, nor to non-fire spells + assert.are.equals(30, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, "CritMultiplier")) + -- ...and works per element + assert.are.equals(7, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Cold }, "CritChance")) + -- still correctly scoped: not attacks, and not the wrong element assert.are.equals(0, item.baseModList:Sum("BASE", { flags = ModFlag.Attack, keywordFlags = KeywordFlag.Fire }, "CritChance")) - assert.are.equals(0, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Cold }, "CritChance")) + assert.are.equals(0, item.baseModList:Sum("BASE", { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Cold }, "CritMultiplier")) end) --TODO: impl sockets for POB2 diff --git a/src/Modules/ModParser.lua b/src/Modules/ModParser.lua index 8481a51b6f..01f20600cd 100644 --- a/src/Modules/ModParser.lua +++ b/src/Modules/ModParser.lua @@ -755,7 +755,6 @@ local modNameList = { ["critical hit chance"] = "CritChance", ["attack critical hit chance"] = { "CritChance", flags = ModFlag.Attack }, ["thorns critical hit chance"] = { "CritChance", flags = ModFlag.Thorns }, - ["fire spell critical hit chance"] = { "CritChance", flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, ["critical damage bonus"] = "CritMultiplier", ["attack critical damage bonus"] = { "CritMultiplier", flags = ModFlag.Attack }, ["critical spell damage bonus"] = { "CritMultiplier", flags = ModFlag.Spell }, @@ -1038,6 +1037,10 @@ local modFlagList = { ["with ranged weapons"] = { flags = bor(ModFlag.WeaponRanged, ModFlag.Hit) }, -- Skill types ["spell"] = { flags = ModFlag.Spell }, + ["fire spell"] = { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Fire }, + ["cold spell"] = { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Cold }, + ["lightning spell"] = { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Lightning }, + ["chaos spell"] = { flags = ModFlag.Spell, keywordFlags = KeywordFlag.Chaos }, ["for spells"] = { flags = ModFlag.Spell }, ["for spell skills"] = { flags = ModFlag.Spell }, ["for spell damage"] = { flags = ModFlag.Spell },